From caf6b100b698f3491f749e5dbb192c8909d16e3c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 10 Jan 2020 12:08:12 -0300 Subject: [PATCH 01/23] Remove constraint for NFS storage --- .../com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 979583b13992..0539f3f20915 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1693,9 +1693,6 @@ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDo @Override public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { final PrimaryDataStoreTO pool = cmd.getDestPool(); - if (!pool.getPoolType().equals(StoragePoolType.NetworkFilesystem)) { - return new DirectDownloadAnswer(false, "Unsupported pool type " + pool.getPoolType().toString(), true); - } KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); DirectTemplateDownloader downloader; From 033d4f7bc62a59154dc88a5517a962617ec621fe Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 10 Jan 2020 13:48:23 -0300 Subject: [PATCH 02/23] Add new property on agent.properties --- agent/conf/agent.properties | 3 +++ .../kvm/resource/LibvirtComputingResource.java | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 7472e3922277..24592387b09e 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -115,6 +115,9 @@ domr.scripts.dir=scripts/network/domr/kvm # set the hypervisor type, values are: kvm, lxc hypervisor.type=kvm +# This parameter specifies a directory on the host local storage for temporary storing direct download templates +#direct.download.temporary.download.location=/var/lib/libvirt/images + # set the hypervisor URI. Usually there is no need for changing this # For KVM: qemu:///system # For LXC: lxc:/// diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index f3066cafc07a..35e54d032ae3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -478,6 +478,10 @@ public String getOvsPvlanVmPath() { return _ovsPvlanVmPath; } + public String getDirectDownloadTemporaryDownloadPath() { + return directDownloadTemporaryDownloadPath; + } + public String getResizeVolumePath() { return _resizeVolumePath; } @@ -530,6 +534,7 @@ protected enum BridgeType { protected boolean dpdkSupport = false; protected String dpdkOvsPath; + protected String directDownloadTemporaryDownloadPath; private String getEndIpFromStartIp(final String startIp, final int numIps) { final String[] tokens = startIp.split("[.]"); @@ -577,6 +582,10 @@ private Map getDeveloperProperties() throws ConfigurationExcepti } } + private String getDefaultDirectDownloadTemporaryPath() { + return "/var/lib/libvirt/images"; + } + protected String getDefaultNetworkScriptsDir() { return "scripts/vm/network/vnet"; } @@ -656,6 +665,11 @@ public boolean configure(final String name, final Map params) th } } + directDownloadTemporaryDownloadPath = (String) params.get("direct.download.temporary.download.location"); + if (org.apache.commons.lang.StringUtils.isBlank(directDownloadTemporaryDownloadPath)) { + directDownloadTemporaryDownloadPath = getDefaultDirectDownloadTemporaryPath(); + } + params.put("domr.scripts.dir", domrScriptsDir); _virtRouterResource = new VirtualRoutingResource(this); From e69cd3da2880bd82d3fa9c5f1735a0eb2f2985d7 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 10 Jan 2020 14:35:13 -0300 Subject: [PATCH 03/23] Add free disk space on the host prior template download --- .../directdownload/DirectDownloadCommand.java | 9 +++++++ .../kvm/storage/KVMStorageProcessor.java | 27 +++++++++++++++++++ .../download/DirectDownloadManagerImpl.java | 1 + 3 files changed, 37 insertions(+) diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index 506cd3140de8..fbbc204bfd59 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -38,6 +38,7 @@ public enum DownloadProtocol { private Integer connectTimeout; private Integer soTimeout; private Integer connectionRequestTimeout; + private Long templateSize; protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers, final Integer connectTimeout, final Integer soTimeout, final Integer connectionRequestTimeout) { this.url = url; @@ -94,6 +95,14 @@ public void setConnectionRequestTimeout(Integer connectionRequestTimeout) { this.connectionRequestTimeout = connectionRequestTimeout; } + public Long getTemplateSize() { + return templateSize; + } + + public void setTemplateSize(Long templateSize) { + this.templateSize = templateSize; + } + @Override public void setExecuteInSequence(boolean inSeq) { } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 0539f3f20915..537b41e61c71 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1703,6 +1703,11 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) } try { + s_logger.info("Checking for free space on the host for downloading the template"); + if (!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) { + String msg = "Not enough space on the defined temporary location to download the template " + cmd.getTemplateId(); + return new DirectDownloadAnswer(false, msg, true); + } s_logger.info("Trying to download template"); if (!downloader.downloadTemplate()) { s_logger.warn("Couldn't download template"); @@ -1724,4 +1729,26 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) DirectTemplateInformation info = downloader.getTemplateInformation(); return new DirectDownloadAnswer(true, info.getSize(), info.getInstallPath()); } + + /** + * Perform a free space check on the host for downloading the direct download templates + * @param templateSize template size obtained from remote server when registering the template + */ + private boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { + if (templateSize == null || templateSize == 0L) { + s_logger.info("The server did not provide the template size, assuming there is enough space to download it"); + return true; + } + String cmd = String.format("df --output=avail %s | tail -1", resource.getDirectDownloadTemporaryDownloadPath()); + String result = Script.runSimpleBashScript(cmd); + Long availableBytes; + try { + availableBytes = Long.parseLong(result); + } catch (NumberFormatException e) { + String msg = "Could not parse the output " + result + " as a number, therefore not able to check for free space"; + s_logger.error(msg, e); + return false; + } + return availableBytes >= templateSize; + } } diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index 792a5a30ee84..d9ee0febf61d 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -252,6 +252,7 @@ public void downloadTemplate(long templateId, long poolId, long hostId) { DownloadProtocol protocol = getProtocolFromUrl(url); DirectDownloadCommand cmd = getDirectDownloadCommandFromProtocol(protocol, url, templateId, to, checksum, headers); + cmd.setTemplateSize(template.getSize()); Answer answer = sendDirectDownloadCommand(cmd, template, poolId, host); From 8e11bf255265ea0b629edfbe473351255a77234c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Mon, 13 Jan 2020 14:21:19 -0300 Subject: [PATCH 04/23] Add unit tests for the free space check --- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../kvm/storage/KVMStorageProcessorTest.java | 44 ++++++++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 537b41e61c71..00ec351d55c2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1734,7 +1734,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) * Perform a free space check on the host for downloading the direct download templates * @param templateSize template size obtained from remote server when registering the template */ - private boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { + protected boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { if (templateSize == null || templateSize == 0L) { s_logger.info("The server did not provide the template size, assuming there is enough space to download it"); return true; diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java index 63d46bc87e9c..36d957038a27 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java @@ -21,13 +21,22 @@ import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; import javax.naming.ConfigurationException; +import com.cloud.utils.script.Script; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.InjectMocks; +import org.mockito.Matchers; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.mockito.Spy; +import org.powermock.api.mockito.PowerMockito; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +@PrepareForTest({ Script.class }) +@RunWith(PowerMockRunner.class) public class KVMStorageProcessorTest { @Mock @@ -35,26 +44,47 @@ public class KVMStorageProcessorTest { @Mock LibvirtComputingResource resource; - private static final Long TEMPLATE_ID = 202l; - private static final String EXPECTED_DIRECT_DOWNLOAD_DIR = "template/2/202"; - - @Spy @InjectMocks private KVMStorageProcessor storageProcessor; + private static final String directDownloadTemporaryPath = "/var/lib/libvirt/images/dd"; + private static final long templateSize = 80000L; + @Before public void setUp() throws ConfigurationException { MockitoAnnotations.initMocks(this); storageProcessor = new KVMStorageProcessor(storagePoolManager, resource); + PowerMockito.mockStatic(Script.class); + Mockito.when(resource.getDirectDownloadTemporaryDownloadPath()).thenReturn(directDownloadTemporaryPath); } @Test - public void testCloneVolumeFromBaseTemplate() throws Exception { + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationAssumeEnoughSpaceWhenNotProvided() { + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(null); + Assert.assertTrue(result); + } + @Test + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationNotEnoughSpace() { + String output = String.valueOf(templateSize - 30000L); + Mockito.when(Script.runSimpleBashScript(Matchers.anyString())).thenReturn(output); + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(templateSize); + Assert.assertFalse(result); } @Test - public void testCopyVolumeFromImageCacheToPrimary() throws Exception { + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationEnoughSpace() { + String output = String.valueOf(templateSize + 30000L); + Mockito.when(Script.runSimpleBashScript(Matchers.anyString())).thenReturn(output); + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(templateSize); + Assert.assertTrue(result); + } + @Test + public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationNotExistingLocation() { + String output = String.format("df: ‘%s’: No such file or directory", directDownloadTemporaryPath); + Mockito.when(Script.runSimpleBashScript(Matchers.anyString())).thenReturn(output); + boolean result = storageProcessor.isEnoughSpaceForDownloadTemplateOnTemporaryLocation(templateSize); + Assert.assertFalse(result); } } From cb2ee6f5e168197fe8ed8de3411e70558f780025 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 14 Jan 2020 12:04:24 -0300 Subject: [PATCH 05/23] Fix free space check - retrieve avaiable size in bytes --- .../hypervisor/kvm/storage/KVMStorageProcessor.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 00ec351d55c2..d02257de0303 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1732,20 +1732,20 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) /** * Perform a free space check on the host for downloading the direct download templates - * @param templateSize template size obtained from remote server when registering the template + * @param templateSize template size obtained from remote server when registering the template (in bytes) */ protected boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long templateSize) { if (templateSize == null || templateSize == 0L) { s_logger.info("The server did not provide the template size, assuming there is enough space to download it"); return true; } - String cmd = String.format("df --output=avail %s | tail -1", resource.getDirectDownloadTemporaryDownloadPath()); - String result = Script.runSimpleBashScript(cmd); + String cmd = String.format("df --output=avail %s -B 1 | tail -1", resource.getDirectDownloadTemporaryDownloadPath()); + String resultInBytes = Script.runSimpleBashScript(cmd); Long availableBytes; try { - availableBytes = Long.parseLong(result); + availableBytes = Long.parseLong(resultInBytes); } catch (NumberFormatException e) { - String msg = "Could not parse the output " + result + " as a number, therefore not able to check for free space"; + String msg = "Could not parse the output " + resultInBytes + " as a number, therefore not able to check for free space"; s_logger.error(msg, e); return false; } From bc989e423b4e8997aa90fa7fc9ec59e8428a5e2c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 14 Jan 2020 12:20:10 -0300 Subject: [PATCH 06/23] Update default location for direct download --- agent/conf/agent.properties | 2 +- .../cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 24592387b09e..525c19f8a592 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -116,7 +116,7 @@ domr.scripts.dir=scripts/network/domr/kvm hypervisor.type=kvm # This parameter specifies a directory on the host local storage for temporary storing direct download templates -#direct.download.temporary.download.location=/var/lib/libvirt/images +#direct.download.temporary.download.location=/var/lib/libvirt/images/direct-download # set the hypervisor URI. Usually there is no need for changing this # For KVM: qemu:///system diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 35e54d032ae3..876150028596 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -583,7 +583,7 @@ private Map getDeveloperProperties() throws ConfigurationExcepti } private String getDefaultDirectDownloadTemporaryPath() { - return "/var/lib/libvirt/images"; + return "/var/lib/libvirt/images/direct-download"; } protected String getDefaultNetworkScriptsDir() { From 989e3b8b5196353046f11b035d3d00b2d6852121 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 15 Jan 2020 13:22:03 -0300 Subject: [PATCH 07/23] Improve the method to retrieve hosts to retry on depending on the destination pool type and scope --- .../download/DirectDownloadManagerImpl.java | 68 +++++++++++-------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index d9ee0febf61d..a759f9ccfdf0 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -20,6 +20,32 @@ import static com.cloud.storage.Storage.ImageFormat; +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.event.ActionEventUtils; +import com.cloud.event.EventTypes; +import com.cloud.event.EventVO; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.Status; +import com.cloud.host.dao.HostDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; +import com.cloud.utils.exception.CloudRuntimeException; + import java.net.URI; import java.net.URISyntaxException; import java.security.cert.Certificate; @@ -27,7 +53,6 @@ import java.security.cert.CertificateExpiredException; import java.security.cert.CertificateNotYetValidException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -67,29 +92,6 @@ import org.joda.time.DateTime; import org.joda.time.DateTimeZone; -import com.cloud.agent.AgentManager; -import com.cloud.agent.api.Answer; -import com.cloud.dc.DataCenterVO; -import com.cloud.dc.dao.DataCenterDao; -import com.cloud.event.ActionEventUtils; -import com.cloud.event.EventTypes; -import com.cloud.event.EventVO; -import com.cloud.exception.AgentUnavailableException; -import com.cloud.exception.OperationTimedoutException; -import com.cloud.host.Host; -import com.cloud.host.HostVO; -import com.cloud.host.Status; -import com.cloud.host.dao.HostDao; -import com.cloud.hypervisor.Hypervisor.HypervisorType; -import com.cloud.storage.DataStoreRole; -import com.cloud.storage.VMTemplateStoragePoolVO; -import com.cloud.storage.VMTemplateStorageResourceAssoc; -import com.cloud.storage.VMTemplateVO; -import com.cloud.storage.dao.VMTemplateDao; -import com.cloud.storage.dao.VMTemplatePoolDao; -import com.cloud.utils.component.ManagerBase; -import com.cloud.utils.concurrency.NamedThreadFactory; -import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.security.CertificateHelper; import sun.security.x509.X509CertImpl; @@ -202,7 +204,7 @@ protected List getRunningHostIdsInTheSameCluster(Long clusterId, long data */ protected Long[] createHostIdsList(List hostIds, long hostId) { if (CollectionUtils.isEmpty(hostIds)) { - return Arrays.asList(hostId).toArray(new Long[1]); + return Collections.singletonList(hostId).toArray(new Long[1]); } Long[] ids = new Long[hostIds.size() + 1]; ids[0] = hostId; @@ -215,11 +217,15 @@ protected Long[] createHostIdsList(List hostIds, long hostId) { } /** - * Get hosts to retry download having hostId as the first element + * Get alternative hosts to retry downloading a template. The planner have previously selected a host and a storage pool + * @return array of host ids which can access the storage pool */ - protected Long[] getHostsToRetryOn(Long clusterId, long dataCenterId, HypervisorType hypervisorType, long hostId) { - List hostIds = getRunningHostIdsInTheSameCluster(clusterId, dataCenterId, hypervisorType, hostId); - return createHostIdsList(hostIds, hostId); + protected Long[] getHostsToRetryOn(Host host, StoragePoolVO storagePool) { + List clusterHostIds = new ArrayList<>(); + if (storagePool.getPoolType() != Storage.StoragePoolType.Filesystem || storagePool.getScope() != ScopeType.HOST) { + clusterHostIds = getRunningHostIdsInTheSameCluster(host.getClusterId(), host.getDataCenterId(), host.getHypervisorType(), host.getId()); + } + return createHostIdsList(clusterHostIds, host.getId()); } @Override @@ -285,7 +291,9 @@ public void downloadTemplate(long templateId, long poolId, long hostId) { private Answer sendDirectDownloadCommand(DirectDownloadCommand cmd, VMTemplateVO template, long poolId, HostVO host) { boolean downloaded = false; int retry = 3; - Long[] hostsToRetry = getHostsToRetryOn(host.getClusterId(), host.getDataCenterId(), host.getHypervisorType(), host.getId()); + + StoragePoolVO storagePoolVO = primaryDataStoreDao.findById(poolId); + Long[] hostsToRetry = getHostsToRetryOn(host, storagePoolVO); int hostIndex = 0; Answer answer = null; Long hostToSendDownloadCmd = hostsToRetry[hostIndex]; From cfe19ab9bea17cda54302eb172c7026a97689649 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 15 Jan 2020 14:00:03 -0300 Subject: [PATCH 08/23] Verify location for temporary download exists before checking free space --- .../kvm/storage/KVMStorageProcessor.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index d02257de0303..15600675ab9b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1703,11 +1703,22 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) } try { + s_logger.info("Verifying temporary location for downloading the template exists on the host"); + String temporaryDownloadPath = resource.getDirectDownloadTemporaryDownloadPath(); + if (!isLocationAccessible(temporaryDownloadPath)) { + String msg = "The temporary location path for downloading templates does not exist: " + + temporaryDownloadPath + " on this host"; + s_logger.error(msg); + return new DirectDownloadAnswer(false, msg, true); + } + s_logger.info("Checking for free space on the host for downloading the template"); if (!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) { String msg = "Not enough space on the defined temporary location to download the template " + cmd.getTemplateId(); + s_logger.error(msg); return new DirectDownloadAnswer(false, msg, true); } + s_logger.info("Trying to download template"); if (!downloader.downloadTemplate()) { s_logger.warn("Couldn't download template"); @@ -1730,6 +1741,14 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(true, info.getSize(), info.getInstallPath()); } + /** + * True if location exists + */ + private boolean isLocationAccessible(String temporaryDownloadPath) { + File dir = new File(temporaryDownloadPath); + return dir.exists(); + } + /** * Perform a free space check on the host for downloading the direct download templates * @param templateSize template size obtained from remote server when registering the template (in bytes) From 453bb895cce0cf40bbcee9c19a6eee53da555e8c Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 17 Jan 2020 09:45:39 -0300 Subject: [PATCH 09/23] In progress - refactor and extension --- .../download/DirectTemplateDownloader.java | 6 +- .../DirectTemplateDownloaderImpl.java | 12 ++-- .../HttpDirectTemplateDownloader.java | 33 ++++++----- .../HttpsDirectTemplateDownloader.java | 48 ++++++++-------- .../MetalinkDirectTemplateDownloader.java | 26 +++++---- .../download/NfsDirectTemplateDownloader.java | 9 +-- .../DirectTemplateDownloaderImplTest.java | 8 --- .../kvm/storage/IscsiAdmStorageAdaptor.java | 5 ++ .../kvm/storage/KVMStoragePoolManager.java | 5 ++ .../kvm/storage/KVMStorageProcessor.java | 49 +++++++++------- .../kvm/storage/LibvirtStorageAdaptor.java | 56 ++++++++++++++++++- .../kvm/storage/ManagedNfsStorageAdaptor.java | 5 ++ .../kvm/storage/StorageAdaptor.java | 7 +++ 13 files changed, 180 insertions(+), 89 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java index a88b4526e9d9..db32e31c232a 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java @@ -19,6 +19,8 @@ package com.cloud.agent.direct.download; +import com.cloud.utils.Pair; + public interface DirectTemplateDownloader { class DirectTemplateInformation { @@ -47,9 +49,9 @@ public String getChecksum() { /** * Perform template download to pool specified on downloader creation - * @return true if successful, false if not + * @return (true if successful, false if not, download file path) */ - boolean downloadTemplate(); + Pair downloadTemplate(); /** * Perform extraction (if necessary) and installation of previously downloaded template diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index 419ab7d1bbde..28e6a8cf2ef1 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -39,13 +39,17 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String installPath; private String checksum; private boolean redownload = false; + private String temporaryDownloadPath; + public static final Logger s_logger = Logger.getLogger(DirectTemplateDownloaderImpl.class.getName()); - protected DirectTemplateDownloaderImpl(final String url, final String destPoolPath, final Long templateId, final String checksum) { + protected DirectTemplateDownloaderImpl(final String url, final String destPoolPath, final Long templateId, + final String checksum, final String temporaryDownloadPath) { this.url = url; this.destPoolPath = destPoolPath; this.templateId = templateId; this.checksum = checksum; + this.temporaryDownloadPath = temporaryDownloadPath; } private static String directDownloadDir = "template"; @@ -53,10 +57,10 @@ protected DirectTemplateDownloaderImpl(final String url, final String destPoolPa /** * Return direct download temporary path to download template */ - protected static String getDirectDownloadTempPath(Long templateId) { + protected String getDirectDownloadTempPath(Long templateId) { String templateIdAsString = String.valueOf(templateId); - return directDownloadDir + File.separator + templateIdAsString.substring(0,1) + - File.separator + templateIdAsString; + return this.temporaryDownloadPath + File.separator + directDownloadDir + File.separator + + templateIdAsString.substring(0,1) + File.separator + templateIdAsString; } /** diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java index f91af40dd404..fc2360344048 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpDirectTemplateDownloader.java @@ -27,6 +27,8 @@ import java.util.HashMap; import java.util.Map; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.commons.collections.MapUtils; import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.HttpStatus; @@ -35,8 +37,6 @@ import org.apache.commons.io.IOUtils; import org.apache.log4j.Logger; -import com.cloud.utils.exception.CloudRuntimeException; - public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected HttpClient client; @@ -45,20 +45,25 @@ public class HttpDirectTemplateDownloader extends DirectTemplateDownloaderImpl { protected GetMethod request; protected Map reqHeaders = new HashMap<>(); - public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, Integer connectTimeout, Integer soTimeout) { - super(url, destPoolPath, templateId, checksum); + public HttpDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, + Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath) { + super(url, destPoolPath, templateId, checksum, downloadPath); s_httpClientManager.getParams().setConnectionTimeout(connectTimeout == null ? 5000 : connectTimeout); s_httpClientManager.getParams().setSoTimeout(soTimeout == null ? 5000 : soTimeout); client = new HttpClient(s_httpClientManager); request = createRequest(url, headers); String downloadDir = getDirectDownloadTempPath(templateId); - createTemporaryDirectoryAndFile(downloadDir); + File tempFile = createTemporaryDirectoryAndFile(downloadDir); + setDownloadedFilePath(tempFile.getAbsolutePath()); } - protected void createTemporaryDirectoryAndFile(String downloadDir) { - createFolder(getDestPoolPath() + File.separator + downloadDir); - File f = new File(getDestPoolPath() + File.separator + downloadDir + File.separator + getFileNameFromUrl()); - setDownloadedFilePath(f.getAbsolutePath()); + /** + * Create download directory (if it does not exist) and set the download file + * @return + */ + protected File createTemporaryDirectoryAndFile(String downloadDir) { + createFolder(downloadDir); + return new File(downloadDir + File.separator + getFileNameFromUrl()); } protected GetMethod createRequest(String downloadUrl, Map headers) { @@ -74,12 +79,12 @@ protected GetMethod createRequest(String downloadUrl, Map header } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { try { int status = client.executeMethod(request); if (status != HttpStatus.SC_OK) { s_logger.warn("Not able to download template, status code: " + status); - return false; + return new Pair<>(false, null); } return performDownload(); } catch (IOException e) { @@ -89,7 +94,7 @@ public boolean downloadTemplate() { } } - protected boolean performDownload() { + protected Pair performDownload() { s_logger.info("Downloading template " + getTemplateId() + " from " + getUrl() + " to: " + getDownloadedFilePath()); try ( InputStream in = request.getResponseBodyAsStream(); @@ -98,8 +103,8 @@ protected boolean performDownload() { IOUtils.copy(in, out); } catch (IOException e) { s_logger.error("Error downloading template " + getTemplateId() + " due to: " + e.getMessage()); - return false; + return new Pair<>(false, null); } - return true; + return new Pair<>(true, getDownloadedFilePath()); } } \ No newline at end of file diff --git a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java index f320846e75d9..d788310f68e7 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/HttpsDirectTemplateDownloader.java @@ -19,6 +19,23 @@ package com.cloud.agent.direct.download; +import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.utils.script.Script; +import org.apache.commons.io.IOUtils; +import org.apache.http.HttpEntity; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.commons.collections.MapUtils; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.conn.ssl.TrustSelfSignedStrategy; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.ssl.SSLContexts; + +import javax.net.ssl.SSLContext; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; @@ -32,31 +49,14 @@ import java.security.cert.CertificateException; import java.util.Map; -import javax.net.ssl.SSLContext; - -import org.apache.commons.collections.MapUtils; -import org.apache.commons.io.IOUtils; -import org.apache.http.HttpEntity; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpGet; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; -import org.apache.http.conn.ssl.TrustSelfSignedStrategy; -import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.ssl.SSLContexts; - -import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; - public class HttpsDirectTemplateDownloader extends HttpDirectTemplateDownloader { private CloseableHttpClient httpsClient; private HttpUriRequest req; - public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, Integer connectTimeout, Integer soTimeout, Integer connectionRequestTimeout) { - super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); + public HttpsDirectTemplateDownloader(String url, Long templateId, String destPoolPath, String checksum, Map headers, + Integer connectTimeout, Integer soTimeout, Integer connectionRequestTimeout, String temporaryDownloadPath) { + super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout, temporaryDownloadPath); SSLContext sslcontext = null; try { sslcontext = getSSLContext(); @@ -98,7 +98,7 @@ private SSLContext getSSLContext() throws KeyStoreException, NoSuchAlgorithmExce } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { CloseableHttpResponse response; try { response = httpsClient.execute(req); @@ -111,7 +111,7 @@ public boolean downloadTemplate() { /** * Consume response and persist it on getDownloadedFilePath() file */ - protected boolean consumeResponse(CloseableHttpResponse response) { + protected Pair consumeResponse(CloseableHttpResponse response) { s_logger.info("Downloading template " + getTemplateId() + " from " + getUrl() + " to: " + getDownloadedFilePath()); if (response.getStatusLine().getStatusCode() != 200) { throw new CloudRuntimeException("Error on HTTPS response"); @@ -123,9 +123,9 @@ protected boolean consumeResponse(CloseableHttpResponse response) { IOUtils.copy(in, out); } catch (Exception e) { s_logger.error("Error parsing response for template " + getTemplateId() + " due to: " + e.getMessage()); - return false; + return new Pair<>(false, null); } - return true; + return new Pair<>(true, getDownloadedFilePath()); } } diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 0a571015bee7..2f2d1d2d8611 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -18,17 +18,17 @@ // package com.cloud.agent.direct.download; -import java.io.File; -import java.util.List; -import java.util.Map; -import java.util.Random; - +import com.cloud.utils.Pair; +import com.cloud.utils.UriUtils; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; -import com.cloud.utils.UriUtils; -import com.cloud.utils.exception.CloudRuntimeException; +import java.io.File; +import java.util.List; +import java.util.Map; +import java.util.Random; public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownloader { @@ -38,8 +38,9 @@ public class MetalinkDirectTemplateDownloader extends HttpDirectTemplateDownload private Random random = new Random(); private static final Logger s_logger = Logger.getLogger(MetalinkDirectTemplateDownloader.class.getName()); - public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, Map headers, Integer connectTimeout, Integer soTimeout) { - super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout); + public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long templateId, String checksum, + Map headers, Integer connectTimeout, Integer soTimeout, String downloadPath) { + super(url, templateId, destPoolPath, checksum, headers, connectTimeout, soTimeout, downloadPath); metalinkUrl = url; metalinkUrls = UriUtils.getMetalinkUrls(metalinkUrl); metalinkChecksums = UriUtils.getMetalinkChecksums(metalinkUrl); @@ -53,7 +54,7 @@ public MetalinkDirectTemplateDownloader(String url, String destPoolPath, Long te } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { if (StringUtils.isBlank(getUrl())) { throw new CloudRuntimeException("Download url has not been set, aborting"); } @@ -73,7 +74,8 @@ public boolean downloadTemplate() { } setDownloadedFilePath(f.getAbsolutePath()); request = createRequest(getUrl(), reqHeaders); - downloaded = super.downloadTemplate(); + Pair downloadResult = super.downloadTemplate(); + downloaded = downloadResult.first(); if (downloaded) { s_logger.info("Successfully downloaded template from url: " + getUrl()); } @@ -84,7 +86,7 @@ public boolean downloadTemplate() { i++; } while (!downloaded && !isRedownload() && i < metalinkUrls.size()); - return downloaded; + return new Pair<>(downloaded, getDownloadedFilePath()); } @Override diff --git a/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java index 16901afedf1b..932477031d6a 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/NfsDirectTemplateDownloader.java @@ -18,6 +18,7 @@ // package com.cloud.agent.direct.download; +import com.cloud.utils.Pair; import com.cloud.utils.UriUtils; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; @@ -51,13 +52,13 @@ private void parseUrl() { } } - public NfsDirectTemplateDownloader(String url, String destPool, Long templateId, String checksum) { - super(url, destPool, templateId, checksum); + public NfsDirectTemplateDownloader(String url, String destPool, Long templateId, String checksum, String downloadPath) { + super(url, destPool, templateId, checksum, downloadPath); parseUrl(); } @Override - public boolean downloadTemplate() { + public Pair downloadTemplate() { String mountSrcUuid = UUID.randomUUID().toString(); String mount = String.format(mountCommand, srcHost + ":" + srcPath, "/mnt/" + mountSrcUuid); Script.runSimpleBashScript(mount); @@ -65,6 +66,6 @@ public boolean downloadTemplate() { setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl()); Script.runSimpleBashScript("cp /mnt/" + mountSrcUuid + srcPath + " " + getDownloadedFilePath()); Script.runSimpleBashScript("umount /mnt/" + mountSrcUuid); - return true; + return new Pair<>(true, getDownloadedFilePath()); } } diff --git a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java b/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java index b244d02f4993..42a671301441 100644 --- a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java +++ b/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java @@ -18,8 +18,6 @@ // package com.cloud.agent.direct.download; -import org.junit.Assert; -import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; @@ -27,10 +25,4 @@ public class DirectTemplateDownloaderImplTest { private static final Long templateId = 202l; - - @Test - public void testGetDirectDownloadTempPath() { - String path = DirectTemplateDownloaderImpl.getDirectDownloadTempPath(templateId); - Assert.assertEquals("template/2/202", path); - } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index bad0151ed8f4..90f2f7112d19 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -445,4 +445,9 @@ public boolean createFolder(String uuid, String path) { public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout) { return null; } + + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + return null; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index c1f73d7a088d..32df7ebdeab1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -405,4 +405,9 @@ public KVMPhysicalDisk createDiskWithTemplateBacking(KVMPhysicalDisk template, S return adaptor.createDiskFromTemplateBacking(template, name, format, size, destPool, timeout); } + public KVMPhysicalDisk createPhysicalDiskFromDirectDownloadTemplate(String templateFilePath, KVMStoragePool destPool, int timeout) { + StorageAdaptor adaptor = getStorageAdaptor(destPool.getType()); + return adaptor.createTemplateFromDirectDownloadFile(templateFilePath, destPool); + } + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 15600675ab9b..32d793603023 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -36,6 +36,7 @@ import javax.naming.ConfigurationException; +import com.cloud.utils.Pair; import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; import org.apache.cloudstack.agent.directdownload.HttpDirectDownloadCommand; @@ -89,7 +90,6 @@ import com.cloud.agent.api.to.NfsTO; import com.cloud.agent.api.to.S3TO; import com.cloud.agent.direct.download.DirectTemplateDownloader; -import com.cloud.agent.direct.download.DirectTemplateDownloader.DirectTemplateInformation; import com.cloud.agent.direct.download.HttpDirectTemplateDownloader; import com.cloud.agent.direct.download.HttpsDirectTemplateDownloader; import com.cloud.agent.direct.download.MetalinkDirectTemplateDownloader; @@ -1676,15 +1676,20 @@ public Answer forgetObject(final ForgetObjectCmd cmd) { /** * Get direct template downloader from direct download command and destination pool */ - private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDownloadCommand cmd, KVMStoragePool destPool) { + private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDownloadCommand cmd, + KVMStoragePool destPool, + String temporaryDownloadPath) { if (cmd instanceof HttpDirectDownloadCommand) { - return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout()); + return new HttpDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), + cmd.getConnectTimeout(), cmd.getSoTimeout(), temporaryDownloadPath); } else if (cmd instanceof HttpsDirectDownloadCommand) { - return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout(), cmd.getConnectionRequestTimeout()); + return new HttpsDirectTemplateDownloader(cmd.getUrl(), cmd.getTemplateId(), destPool.getLocalPath(), cmd.getChecksum(), cmd.getHeaders(), + cmd.getConnectTimeout(), cmd.getSoTimeout(), cmd.getConnectionRequestTimeout(), temporaryDownloadPath); } else if (cmd instanceof NfsDirectDownloadCommand) { - return new NfsDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum()); + return new NfsDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), temporaryDownloadPath); } else if (cmd instanceof MetalinkDirectDownloadCommand) { - return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), cmd.getHeaders(), cmd.getConnectTimeout(), cmd.getSoTimeout()); + return new MetalinkDirectTemplateDownloader(cmd.getUrl(), destPool.getLocalPath(), cmd.getTemplateId(), cmd.getChecksum(), cmd.getHeaders(), + cmd.getConnectTimeout(), cmd.getSoTimeout(), temporaryDownloadPath); } else { throw new IllegalArgumentException("Unsupported protocol, please provide HTTP(S), NFS or a metalink"); } @@ -1694,13 +1699,9 @@ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDo public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { final PrimaryDataStoreTO pool = cmd.getDestPool(); KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); - DirectTemplateDownloader downloader; - - try { - downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool); - } catch (IllegalArgumentException e) { - return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); - } + DirectTemplateDownloader downloader = null; + String path; + long size = 0L; try { s_logger.info("Verifying temporary location for downloading the template exists on the host"); @@ -1719,26 +1720,34 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, msg, true); } + downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, temporaryDownloadPath); s_logger.info("Trying to download template"); - if (!downloader.downloadTemplate()) { + Pair result = downloader.downloadTemplate(); + if (!result.first()) { s_logger.warn("Couldn't download template"); return new DirectDownloadAnswer(false, "Unable to download template", true); } - if (!downloader.validateChecksum()) { + String tempFilePath = result.second(); + /**if (!downloader.validateChecksum()) { s_logger.warn("Couldn't validate template checksum"); return new DirectDownloadAnswer(false, "Checksum validation failed", false); - } - if (!downloader.extractAndInstallDownloadedTemplate()) { + }**/ + KVMPhysicalDisk template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); + path = template.getPath(); + size = template.getSize(); + /**if (!downloader.extractAndInstallDownloadedTemplate()) { s_logger.warn("Couldn't extract and install template"); return new DirectDownloadAnswer(false, "Extraction and installation failed", false); - } + }**/ } catch (CloudRuntimeException e) { s_logger.warn("Error downloading template " + cmd.getTemplateId() + " due to: " + e.getMessage()); return new DirectDownloadAnswer(false, "Unable to download template: " + e.getMessage(), true); + } catch (IllegalArgumentException e) { + return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - DirectTemplateInformation info = downloader.getTemplateInformation(); - return new DirectDownloadAnswer(true, info.getSize(), info.getInstallPath()); + //DirectTemplateInformation info = downloader.getTemplateInformation(); + return new DirectDownloadAnswer(true, size, path); } /** diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index f858a4f15772..e290ba94f173 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -122,6 +122,60 @@ public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, S return disk; } + /** + * Checks if downloaded template is extractable + * @return true if it should be extracted, false if not + */ + private boolean isTemplateExtractable(String templatePath) { + String type = Script.runSimpleBashScript("file " + templatePath + " | awk -F' ' '{print $2}'"); + return type.equalsIgnoreCase("bzip2") || type.equalsIgnoreCase("gzip") || type.equalsIgnoreCase("zip"); + } + + /** + * Return extract command to execute given downloaded file + * @param downloadedTemplateFile + * @param templateUuid + */ + private String getExtractCommandForDownloadedFile(String downloadedTemplateFile, String templateUuid) { + if (downloadedTemplateFile.endsWith(".zip")) { + return "unzip -p " + downloadedTemplateFile + " | cat > " + templateUuid; + } else if (downloadedTemplateFile.endsWith(".bz2")) { + return "bunzip2 -c " + downloadedTemplateFile + " > " + templateUuid; + } else if (downloadedTemplateFile.endsWith(".gz")) { + return "gunzip -c " + downloadedTemplateFile + " > " + templateUuid; + } else { + throw new CloudRuntimeException("Unable to extract template " + downloadedTemplateFile); + } + } + + /** + * Extract downloaded template into installPath, remove compressed file + */ + private void extractDownloadedTemplate(String downloadedTemplateFile, KVMStoragePool destPool, String templateUuid) { + String destinationFile = destPool.getLocalPath() + File.separator + templateUuid; + String extractCommand = getExtractCommandForDownloadedFile(downloadedTemplateFile, destinationFile); + Script.runSimpleBashScript(extractCommand); + Script.runSimpleBashScript("rm -f " + downloadedTemplateFile); + } + + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + File sourceFile = new File(templateFilePath); + if (!sourceFile.exists()) { + throw new CloudRuntimeException("Direct download template file " + sourceFile + " does not exist on this host"); + } + String templateUuid = UUID.randomUUID().toString(); + if (destPool.getType() == StoragePoolType.NetworkFilesystem || destPool.getType() == StoragePoolType.Filesystem + || destPool.getType() == StoragePoolType.SharedMountPoint) { + if (isTemplateExtractable(templateFilePath)) { + extractDownloadedTemplate(templateFilePath, destPool, templateUuid); + } else { + Script.runSimpleBashScript("mv " + templateFilePath + " " + templateUuid); + } + } + return destPool.getPhysicalDisk(templateUuid); + } + public StorageVol getVolume(StoragePool pool, String volName) { StorageVol vol = null; @@ -1198,7 +1252,7 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt if (disk.getFormat() == PhysicalDiskFormat.TAR) { newDisk = destPool.createPhysicalDisk(name, PhysicalDiskFormat.DIR, Storage.ProvisioningType.THIN, disk.getVirtualSize()); } else { - newDisk = destPool.createPhysicalDisk(name, Storage.ProvisioningType.THIN, disk.getVirtualSize()); + newDisk = destPool.createPhysicalDisk(name, Storage.ProvisioningType.THIN, disk.getVirtualSize()); } } else { newDisk = new KVMPhysicalDisk(destPool.getSourceDir() + "/" + name, name, destPool); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java index 309308ae9c15..fe583e385cbe 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java @@ -318,6 +318,11 @@ public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, S return null; } + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + return null; + } + @Override public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, PhysicalDiskFormat format, ProvisioningType provisioningType, long size) { return null; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java index a3c1387aa6b6..86392da5ea3d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java @@ -81,4 +81,11 @@ public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template, KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size, KVMStoragePool destPool, int timeout); + + /** + * Create physical disk on Primary Storage from direct download template on the host (in temporary location) + * @param templateFilePath + * @param destPool + */ + KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool); } From 6ce0a47aaab5cc88a3d3741d1b9f6be7b8aa760a Mon Sep 17 00:00:00 2001 From: nvazquez Date: Tue, 21 Jan 2020 00:07:15 -0300 Subject: [PATCH 10/23] Refactor and fix --- .../download/DirectTemplateDownloader.java | 36 ----------- .../DirectTemplateDownloaderImpl.java | 61 ------------------- .../kvm/storage/KVMStorageProcessor.java | 22 +++---- 3 files changed, 7 insertions(+), 112 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java index db32e31c232a..32b84a34436d 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloader.java @@ -23,48 +23,12 @@ public interface DirectTemplateDownloader { - class DirectTemplateInformation { - private String installPath; - private Long size; - private String checksum; - - public DirectTemplateInformation(String installPath, Long size, String checksum) { - this.installPath = installPath; - this.size = size; - this.checksum = checksum; - } - - public String getInstallPath() { - return installPath; - } - - public Long getSize() { - return size; - } - - public String getChecksum() { - return checksum; - } - } - /** * Perform template download to pool specified on downloader creation * @return (true if successful, false if not, download file path) */ Pair downloadTemplate(); - /** - * Perform extraction (if necessary) and installation of previously downloaded template - * @return true if successful, false if not - */ - boolean extractAndInstallDownloadedTemplate(); - - /** - * Get template information after it is properly installed on pool - * @return template information - */ - DirectTemplateInformation getTemplateInformation(); - /** * Perform checksum validation of previously downloadeed template * @return true if successful, false if not diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index 28e6a8cf2ef1..01603b941024 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -19,7 +19,6 @@ package com.cloud.agent.direct.download; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; import org.apache.cloudstack.utils.security.DigestHelper; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; @@ -28,7 +27,6 @@ import java.io.FileInputStream; import java.io.IOException; import java.security.NoSuchAlgorithmException; -import java.util.UUID; public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDownloader { @@ -36,7 +34,6 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String destPoolPath; private Long templateId; private String downloadedFilePath; - private String installPath; private String checksum; private boolean redownload = false; private String temporaryDownloadPath; @@ -117,64 +114,6 @@ public String getFileNameFromUrl() { return urlParts[urlParts.length - 1]; } - /** - * Checks if downloaded template is extractable - * @return true if it should be extracted, false if not - */ - private boolean isTemplateExtractable() { - String type = Script.runSimpleBashScript("file " + downloadedFilePath + " | awk -F' ' '{print $2}'"); - return type.equalsIgnoreCase("bzip2") || type.equalsIgnoreCase("gzip") || type.equalsIgnoreCase("zip"); - } - - @Override - public boolean extractAndInstallDownloadedTemplate() { - installPath = UUID.randomUUID().toString(); - if (isTemplateExtractable()) { - extractDownloadedTemplate(); - } else { - Script.runSimpleBashScript("mv " + downloadedFilePath + " " + getInstallFullPath()); - } - return true; - } - - /** - * Return install full path - */ - private String getInstallFullPath() { - return destPoolPath + File.separator + installPath; - } - - /** - * Return extract command to execute given downloaded file - */ - private String getExtractCommandForDownloadedFile() { - if (downloadedFilePath.endsWith(".zip")) { - return "unzip -p " + downloadedFilePath + " | cat > " + getInstallFullPath(); - } else if (downloadedFilePath.endsWith(".bz2")) { - return "bunzip2 -c " + downloadedFilePath + " > " + getInstallFullPath(); - } else if (downloadedFilePath.endsWith(".gz")) { - return "gunzip -c " + downloadedFilePath + " > " + getInstallFullPath(); - } else { - throw new CloudRuntimeException("Unable to extract template " + templateId + " on " + downloadedFilePath); - } - } - - /** - * Extract downloaded template into installPath, remove compressed file - */ - private void extractDownloadedTemplate() { - String extractCommand = getExtractCommandForDownloadedFile(); - Script.runSimpleBashScript(extractCommand); - Script.runSimpleBashScript("rm -f " + downloadedFilePath); - } - - @Override - public DirectTemplateInformation getTemplateInformation() { - String sizeResult = Script.runSimpleBashScript("ls -als " + getInstallFullPath() + " | awk '{print $1}'"); - long size = Long.parseLong(sizeResult); - return new DirectTemplateInformation(installPath, size, checksum); - } - @Override public boolean validateChecksum() { if (StringUtils.isNotBlank(checksum)) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 32d793603023..7a54c0c02b82 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1698,10 +1698,8 @@ private DirectTemplateDownloader getDirectTemplateDownloaderFromCommand(DirectDo @Override public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { final PrimaryDataStoreTO pool = cmd.getDestPool(); - KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); - DirectTemplateDownloader downloader = null; - String path; - long size = 0L; + DirectTemplateDownloader downloader; + KVMPhysicalDisk template; try { s_logger.info("Verifying temporary location for downloading the template exists on the host"); @@ -1720,6 +1718,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, msg, true); } + KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, temporaryDownloadPath); s_logger.info("Trying to download template"); Pair result = downloader.downloadTemplate(); @@ -1728,17 +1727,11 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, "Unable to download template", true); } String tempFilePath = result.second(); - /**if (!downloader.validateChecksum()) { + if (!downloader.validateChecksum()) { s_logger.warn("Couldn't validate template checksum"); return new DirectDownloadAnswer(false, "Checksum validation failed", false); - }**/ - KVMPhysicalDisk template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); - path = template.getPath(); - size = template.getSize(); - /**if (!downloader.extractAndInstallDownloadedTemplate()) { - s_logger.warn("Couldn't extract and install template"); - return new DirectDownloadAnswer(false, "Extraction and installation failed", false); - }**/ + } + template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); } catch (CloudRuntimeException e) { s_logger.warn("Error downloading template " + cmd.getTemplateId() + " due to: " + e.getMessage()); return new DirectDownloadAnswer(false, "Unable to download template: " + e.getMessage(), true); @@ -1746,8 +1739,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - //DirectTemplateInformation info = downloader.getTemplateInformation(); - return new DirectDownloadAnswer(true, size, path); + return new DirectDownloadAnswer(true, template.getSize(), template.getPath()); } /** From ba5596851858e2682a37f4b5ad3058c47ca41704 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 22 Jan 2020 02:29:24 -0300 Subject: [PATCH 11/23] Last fixes and marvin tests --- .../DirectTemplateDownloaderImpl.java | 2 +- .../MetalinkDirectTemplateDownloader.java | 3 +- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../integration/smoke/test_direct_download.py | 148 ++++++++++++++++-- 4 files changed, 138 insertions(+), 17 deletions(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java index 01603b941024..9c150e92a989 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImpl.java @@ -36,7 +36,7 @@ public abstract class DirectTemplateDownloaderImpl implements DirectTemplateDown private String downloadedFilePath; private String checksum; private boolean redownload = false; - private String temporaryDownloadPath; + protected String temporaryDownloadPath; public static final Logger s_logger = Logger.getLogger(DirectTemplateDownloaderImpl.class.getName()); diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 2f2d1d2d8611..3a04bd2b5a01 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -58,7 +58,6 @@ public Pair downloadTemplate() { if (StringUtils.isBlank(getUrl())) { throw new CloudRuntimeException("Download url has not been set, aborting"); } - String downloadDir = getDirectDownloadTempPath(getTemplateId()); boolean downloaded = false; int i = 0; do { @@ -67,7 +66,7 @@ public Pair downloadTemplate() { } s_logger.info("Trying to download template from url: " + getUrl()); try { - File f = new File(getDestPoolPath() + File.separator + downloadDir + File.separator + getFileNameFromUrl()); + File f = new File(getDownloadedFilePath()); if (f.exists()) { f.delete(); f.createNewFile(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 7a54c0c02b82..6ed952c08782 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1739,7 +1739,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, "Unable to create direct downloader: " + e.getMessage(), true); } - return new DirectDownloadAnswer(true, template.getSize(), template.getPath()); + return new DirectDownloadAnswer(true, template.getSize(), template.getName()); } /** diff --git a/test/integration/smoke/test_direct_download.py b/test/integration/smoke/test_direct_download.py index 132deb4f6989..a6c98e52286e 100644 --- a/test/integration/smoke/test_direct_download.py +++ b/test/integration/smoke/test_direct_download.py @@ -23,7 +23,8 @@ NetworkOffering, Network, Template, - VirtualMachine) + VirtualMachine, + StoragePool) from marvin.lib.common import (get_pod, get_zone) from nose.plugins.attrib import attr @@ -160,11 +161,15 @@ def setUpClass(cls): cls.services = cls.testClient.getParsedTestDataConfig() cls._cleanup = [] - cls.hypervisorNotSupported = False - if cls.hypervisor.lower() not in ['kvm', 'lxc']: - cls.hypervisorNotSupported = True + cls.hypervisorSupported = False + cls.nfsStorageFound = False + cls.localStorageFound = False + cls.sharedMountPointFound = False - if not cls.hypervisorNotSupported: + if cls.hypervisor.lower() in ['kvm', 'lxc']: + cls.hypervisorSupported = True + + if cls.hypervisorSupported: cls.services["test_templates"]["kvm"]["directdownload"] = "true" cls.template = Template.register(cls.apiclient, cls.services["test_templates"]["kvm"], zoneid=cls.zone.id, hypervisor=cls.hypervisor) @@ -192,6 +197,25 @@ def setUpClass(cls): ) cls._cleanup.append(cls.l2_network) cls._cleanup.append(cls.network_offering) + + storage_pools = StoragePool.list( + cls.apiclient, + zoneid=cls.zone.id + ) + for pool in storage_pools: + if not cls.nfsStorageFound and pool.type == "NetworkFilesystem": + cls.nfsStorageFound = True + cls.nfsPoolId = pool.id + elif not cls.localStorageFound and pool.type == "Filesystem": + cls.localStorageFound = True + cls.localPoolId = pool.id + elif not cls.sharedMountPointFound and pool.type == "SharedMountPoint": + cls.sharedMountPointFound = True + cls.sharedPoolId = pool.id + + cls.nfsKvmNotAvailable = not cls.hypervisorSupported or not cls.nfsStorageFound + cls.localStorageKvmNotAvailable = not cls.hypervisorSupported or not cls.localStorageFound + cls.sharedMountPointKvmNotAvailable = not cls.hypervisorSupported or not cls.sharedMountPointFound return @classmethod @@ -215,26 +239,124 @@ def tearDown(self): raise Exception("Warning: Exception during cleanup : %s" % e) return - @skipTestIf("hypervisorNotSupported") + def getCurrentStoragePoolTags(self, poolId): + local_pool = StoragePool.list( + self.apiclient, + id=poolId + ) + return local_pool[0].tags + + def updateStoragePoolTags(self, poolId, tags): + StoragePool.update( + self.apiclient, + id=poolId, + tags=tags + ) + + def createServiceOffering(self, name, type, tags): + services = { + "cpunumber": 1, + "cpuspeed": 512, + "memory": 256, + "displaytext": name, + "name": name, + "storagetype": type + } + return ServiceOffering.create( + self.apiclient, + services, + tags=tags + ) + + + @skipTestIf("nfsKvmNotAvailable") @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") - def test_01_deploy_vm_from_direct_download_template(self): - """Test Deploy VM from direct download template + def test_01_deploy_vm_from_direct_download_template_nfs_storage(self): + """Test Deploy VM from direct download template on NFS storage """ - # Validate the following - # 1. Register direct download template - # 2. Deploy VM from direct download template + # Create service offering for local storage using storage tags + tags = self.getCurrentStoragePoolTags(self.nfsPoolId) + test_tag = "marvin_test_nfs_storage_direct_download" + self.updateStoragePoolTags(self.nfsPoolId, test_tag) + nfs_storage_offering = self.createServiceOffering("TestNFSStorageDirectDownload", "shared", test_tag) vm = VirtualMachine.create( self.apiclient, self.services["virtual_machine"], - serviceofferingid=self.service_offering.id, + serviceofferingid=nfs_storage_offering.id, networkids=self.l2_network.id ) self.assertEqual( vm.state, "Running", - "Check VM deployed from direct download template is running" + "Check VM deployed from direct download template is running on NFS storage" ) + + # Revert storage tags for the storage pool used in this test + self.updateStoragePoolTags(self.nfsPoolId, tags) + self.cleanup.append(vm) + self.cleanup.append(nfs_storage_offering) + return + + @skipTestIf("localStorageKvmNotAvailable") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_02_deploy_vm_from_direct_download_template_local_storage(self): + """Test Deploy VM from direct download template on local storage + """ + + # Create service offering for local storage using storage tags + tags = self.getCurrentStoragePoolTags(self.localPoolId) + test_tag = "marvin_test_local_storage_direct_download" + self.updateStoragePoolTags(self.localPoolId, test_tag) + local_storage_offering = self.createServiceOffering("TestLocalStorageDirectDownload", "local", test_tag) + + # Deploy VM + vm = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + serviceofferingid=local_storage_offering.id, + networkids=self.l2_network.id, + ) + self.assertEqual( + vm.state, + "Running", + "Check VM deployed from direct download template is running on local storage" + ) + + # Revert storage tags for the storage pool used in this test + self.updateStoragePoolTags(self.localPoolId, tags) + self.cleanup.append(vm) + self.cleanup.append(local_storage_offering) + return + + @skipTestIf("sharedMountPointKvmNotAvailable") + @attr(tags=["advanced", "basic", "eip", "advancedns", "sg"], required_hardware="false") + def test_03_deploy_vm_from_direct_download_template_shared_mount_point_storage(self): + """Test Deploy VM from direct download template on shared mount point + """ + + # Create service offering for local storage using storage tags + tags = self.getCurrentStoragePoolTags(self.sharedPoolId) + test_tag = "marvin_test_shared_mount_point_storage_direct_download" + self.updateStoragePoolTags(self.sharedPoolId, test_tag) + shared_offering = self.createServiceOffering("TestSharedMountPointStorageDirectDownload", "shared", test_tag) + + # Deploy VM + vm = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + serviceofferingid=shared_offering.id, + networkids=self.l2_network.id, + ) + self.assertEqual( + vm.state, + "Running", + "Check VM deployed from direct download template is running on shared mount point" + ) + + # Revert storage tags for the storage pool used in this test + self.updateStoragePoolTags(self.sharedPoolId, tags) self.cleanup.append(vm) + self.cleanup.append(shared_offering) return From 83930c899b4aedfce4f8018bda417949b342d26f Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 22 Jan 2020 02:50:59 -0300 Subject: [PATCH 12/23] Remove unused test file --- .../DirectTemplateDownloaderImplTest.java | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java diff --git a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java b/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java deleted file mode 100644 index 42a671301441..000000000000 --- a/agent/src/test/java/com/cloud/agent/direct/download/DirectTemplateDownloaderImplTest.java +++ /dev/null @@ -1,28 +0,0 @@ -// -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -// -package com.cloud.agent.direct.download; - -import org.junit.runner.RunWith; -import org.mockito.runners.MockitoJUnitRunner; - -@RunWith(MockitoJUnitRunner.class) -public class DirectTemplateDownloaderImplTest { - - private static final Long templateId = 202l; -} From c8264584c3f0e612bbf50c3da5d7ccfa32098e2f Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 22 Jan 2020 07:19:37 -0300 Subject: [PATCH 13/23] Improve logging --- .../cloud/hypervisor/kvm/storage/KVMStorageProcessor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 6ed952c08782..7624316e2c95 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1702,7 +1702,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) KVMPhysicalDisk template; try { - s_logger.info("Verifying temporary location for downloading the template exists on the host"); + s_logger.debug("Verifying temporary location for downloading the template exists on the host"); String temporaryDownloadPath = resource.getDirectDownloadTemporaryDownloadPath(); if (!isLocationAccessible(temporaryDownloadPath)) { String msg = "The temporary location path for downloading templates does not exist: " + @@ -1711,7 +1711,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(false, msg, true); } - s_logger.info("Checking for free space on the host for downloading the template"); + s_logger.debug("Checking for free space on the host for downloading the template"); if (!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) { String msg = "Not enough space on the defined temporary location to download the template " + cmd.getTemplateId(); s_logger.error(msg); @@ -1720,7 +1720,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) KVMStoragePool destPool = storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid()); downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, temporaryDownloadPath); - s_logger.info("Trying to download template"); + s_logger.debug("Trying to download template"); Pair result = downloader.downloadTemplate(); if (!result.first()) { s_logger.warn("Couldn't download template"); From e8a20d684dc85075184ba03f3a160489e54a21ce Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 29 Jan 2020 01:24:53 -0300 Subject: [PATCH 14/23] Change default path for direct download --- agent/conf/agent.properties | 2 +- .../cloud/hypervisor/kvm/resource/LibvirtComputingResource.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/conf/agent.properties b/agent/conf/agent.properties index 525c19f8a592..24592387b09e 100644 --- a/agent/conf/agent.properties +++ b/agent/conf/agent.properties @@ -116,7 +116,7 @@ domr.scripts.dir=scripts/network/domr/kvm hypervisor.type=kvm # This parameter specifies a directory on the host local storage for temporary storing direct download templates -#direct.download.temporary.download.location=/var/lib/libvirt/images/direct-download +#direct.download.temporary.download.location=/var/lib/libvirt/images # set the hypervisor URI. Usually there is no need for changing this # For KVM: qemu:///system diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 876150028596..35e54d032ae3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -583,7 +583,7 @@ private Map getDeveloperProperties() throws ConfigurationExcepti } private String getDefaultDirectDownloadTemporaryPath() { - return "/var/lib/libvirt/images/direct-download"; + return "/var/lib/libvirt/images"; } protected String getDefaultNetworkScriptsDir() { From ef7f626ce3cd9cc47fb827ed32530fe393f1aec7 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 29 Jan 2020 01:41:12 -0300 Subject: [PATCH 15/23] Fix upload certificate --- test/integration/smoke/test_direct_download.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/integration/smoke/test_direct_download.py b/test/integration/smoke/test_direct_download.py index a6c98e52286e..324fb5972cb2 100644 --- a/test/integration/smoke/test_direct_download.py +++ b/test/integration/smoke/test_direct_download.py @@ -30,6 +30,7 @@ from nose.plugins.attrib import attr from marvin.cloudstackAPI import (uploadTemplateDirectDownloadCertificate, revokeTemplateDirectDownloadCertificate) from marvin.lib.decoratorGenerators import skipTestIf +import uuid class TestUploadDirectDownloadCertificates(cloudstackTestCase): @@ -91,7 +92,7 @@ def test_01_sanity_check_on_certificates(self): cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() cmd.hypervisor = self.hypervisor - cmd.name = "marvin-test-verify-certs" + cmd.name = "marvin-test-verify-certs" + str(uuid.uuid1()) cmd.certificate = self.certificates["invalid"] cmd.zoneid = self.zone.id @@ -126,7 +127,7 @@ def test_02_upload_direct_download_certificates(self): cmd = uploadTemplateDirectDownloadCertificate.uploadTemplateDirectDownloadCertificateCmd() cmd.hypervisor = self.hypervisor - cmd.name = "marvin-test-verify-certs" + cmd.name = "marvin-test-verify-certs" + str(uuid.uuid1()) cmd.certificate = self.certificates["valid"] cmd.zoneid = self.zone.id From 09c2636a75adb0a6f9bc939e0d9275a524e894d2 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 20 Feb 2020 09:44:56 -0300 Subject: [PATCH 16/23] Fix ISO failure after retry --- .../src/main/java/com/cloud/template/TemplateManagerImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 93905fc0f3fb..769a814fd31d 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -587,6 +587,10 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), poolId); } else { template = _tmplFactory.getTemplate(vm.getIsoId(), DataStoreRole.Primary, dest.getDataCenter().getId()); + if (template.isDirectDownload() && template.getInstallPath() == null) { + s_logger.error("Error getting ISO for direct download"); + throw new CloudRuntimeException("Error getting ISO for direct download"); + } } if (template == null){ From ba202a3827acc42a2b9ac6b9706e61a12b056712 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 21 Feb 2020 00:33:00 -0300 Subject: [PATCH 17/23] Fix metalink filename mismatch error --- .../direct/download/MetalinkDirectTemplateDownloader.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java index 3a04bd2b5a01..c8e85277913b 100644 --- a/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java +++ b/agent/src/main/java/com/cloud/agent/direct/download/MetalinkDirectTemplateDownloader.java @@ -60,18 +60,19 @@ public Pair downloadTemplate() { } boolean downloaded = false; int i = 0; + String downloadDir = getDirectDownloadTempPath(getTemplateId()); do { if (!isRedownload()) { setUrl(metalinkUrls.get(i)); } s_logger.info("Trying to download template from url: " + getUrl()); try { + setDownloadedFilePath(downloadDir + File.separator + getFileNameFromUrl()); File f = new File(getDownloadedFilePath()); if (f.exists()) { f.delete(); f.createNewFile(); } - setDownloadedFilePath(f.getAbsolutePath()); request = createRequest(getUrl(), reqHeaders); Pair downloadResult = super.downloadTemplate(); downloaded = downloadResult.first(); From 2923518fe01808eb79a5034a8f5de78daf9598d9 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 21 Feb 2020 03:01:16 -0300 Subject: [PATCH 18/23] Fix iso direct download --- .../directdownload/DirectDownloadCommand.java | 9 +++++++++ .../kvm/storage/IscsiAdmStorageAdaptor.java | 2 +- .../kvm/storage/KVMStoragePoolManager.java | 4 ++-- .../kvm/storage/KVMStorageProcessor.java | 2 +- .../kvm/storage/LibvirtStorageAdaptor.java | 16 ++++++++++------ .../kvm/storage/ManagedNfsStorageAdaptor.java | 2 +- .../hypervisor/kvm/storage/StorageAdaptor.java | 3 ++- .../download/DirectDownloadManagerImpl.java | 1 + 8 files changed, 27 insertions(+), 12 deletions(-) diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index fbbc204bfd59..aafcb5370a57 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -39,6 +39,7 @@ public enum DownloadProtocol { private Integer soTimeout; private Integer connectionRequestTimeout; private Long templateSize; + private boolean iso; protected DirectDownloadCommand (final String url, final Long templateId, final PrimaryDataStoreTO destPool, final String checksum, final Map headers, final Integer connectTimeout, final Integer soTimeout, final Integer connectionRequestTimeout) { this.url = url; @@ -103,6 +104,14 @@ public void setTemplateSize(Long templateSize) { this.templateSize = templateSize; } + public boolean isIso() { + return iso; + } + + public void setIso(boolean iso) { + this.iso = iso; + } + @Override public void setExecuteInSequence(boolean inSeq) { } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java index 90f2f7112d19..0418dbbb0000 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java @@ -447,7 +447,7 @@ public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, S } @Override - public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool, boolean isIso) { return null; } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index 32df7ebdeab1..544c47f07e57 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -405,9 +405,9 @@ public KVMPhysicalDisk createDiskWithTemplateBacking(KVMPhysicalDisk template, S return adaptor.createDiskFromTemplateBacking(template, name, format, size, destPool, timeout); } - public KVMPhysicalDisk createPhysicalDiskFromDirectDownloadTemplate(String templateFilePath, KVMStoragePool destPool, int timeout) { + public KVMPhysicalDisk createPhysicalDiskFromDirectDownloadTemplate(String templateFilePath, KVMStoragePool destPool, boolean isIso) { StorageAdaptor adaptor = getStorageAdaptor(destPool.getType()); - return adaptor.createTemplateFromDirectDownloadFile(templateFilePath, destPool); + return adaptor.createTemplateFromDirectDownloadFile(templateFilePath, destPool, isIso); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 7624316e2c95..9ff783bd5d66 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1731,7 +1731,7 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) s_logger.warn("Couldn't validate template checksum"); return new DirectDownloadAnswer(false, "Checksum validation failed", false); } - template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, 100); + template = storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, destPool, cmd.isIso()); } catch (CloudRuntimeException e) { s_logger.warn("Error downloading template " + cmd.getTemplateId() + " due to: " + e.getMessage()); return new DirectDownloadAnswer(false, "Unable to download template: " + e.getMessage(), true); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index e290ba94f173..ce2199cc28d3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -151,26 +151,30 @@ private String getExtractCommandForDownloadedFile(String downloadedTemplateFile, /** * Extract downloaded template into installPath, remove compressed file */ - private void extractDownloadedTemplate(String downloadedTemplateFile, KVMStoragePool destPool, String templateUuid) { - String destinationFile = destPool.getLocalPath() + File.separator + templateUuid; + private void extractDownloadedTemplate(String downloadedTemplateFile, KVMStoragePool destPool, String destinationFile) { String extractCommand = getExtractCommandForDownloadedFile(downloadedTemplateFile, destinationFile); Script.runSimpleBashScript(extractCommand); Script.runSimpleBashScript("rm -f " + downloadedTemplateFile); } @Override - public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool, boolean isIso) { File sourceFile = new File(templateFilePath); if (!sourceFile.exists()) { throw new CloudRuntimeException("Direct download template file " + sourceFile + " does not exist on this host"); } String templateUuid = UUID.randomUUID().toString(); + if (isIso) { + templateUuid += ".iso"; + } + String destinationFile = destPool.getLocalPath() + File.separator + templateUuid; + if (destPool.getType() == StoragePoolType.NetworkFilesystem || destPool.getType() == StoragePoolType.Filesystem || destPool.getType() == StoragePoolType.SharedMountPoint) { - if (isTemplateExtractable(templateFilePath)) { - extractDownloadedTemplate(templateFilePath, destPool, templateUuid); + if (!isIso && isTemplateExtractable(templateFilePath)) { + extractDownloadedTemplate(templateFilePath, destPool, destinationFile); } else { - Script.runSimpleBashScript("mv " + templateFilePath + " " + templateUuid); + Script.runSimpleBashScript("mv " + templateFilePath + " " + destinationFile); } } return destPool.getPhysicalDisk(templateUuid); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java index fe583e385cbe..1ea4f6262263 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java @@ -319,7 +319,7 @@ public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, S } @Override - public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool) { + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool, boolean isIso) { return null; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java index 86392da5ea3d..99f2876915c0 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java @@ -86,6 +86,7 @@ KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, * Create physical disk on Primary Storage from direct download template on the host (in temporary location) * @param templateFilePath * @param destPool + * @param isIso */ - KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool); + KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, KVMStoragePool destPool, boolean isIso); } diff --git a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java index a759f9ccfdf0..a05c4b9e4aa5 100644 --- a/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java @@ -259,6 +259,7 @@ public void downloadTemplate(long templateId, long poolId, long hostId) { DownloadProtocol protocol = getProtocolFromUrl(url); DirectDownloadCommand cmd = getDirectDownloadCommandFromProtocol(protocol, url, templateId, to, checksum, headers); cmd.setTemplateSize(template.getSize()); + cmd.setIso(template.getFormat() == ImageFormat.ISO); Answer answer = sendDirectDownloadCommand(cmd, template, poolId, host); From 4f7a27176a57eed5c383205eec7ed8b5bb183fcc Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 27 Feb 2020 03:10:26 -0300 Subject: [PATCH 19/23] Fix for direct download ISOs on local storage and shared mount point --- .../kvm/resource/LibvirtComputingResource.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 35e54d032ae3..0d2cccde0328 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2334,11 +2334,17 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { if (dataStore instanceof NfsTO) { NfsTO nfsStore = (NfsTO)data.getDataStore(); dataStoreUrl = nfsStore.getUrl(); - } else if (dataStore instanceof PrimaryDataStoreTO && ((PrimaryDataStoreTO) dataStore).getPoolType().equals(StoragePoolType.NetworkFilesystem)) { + } else if (dataStore instanceof PrimaryDataStoreTO) { //In order to support directly downloaded ISOs - String psHost = ((PrimaryDataStoreTO) dataStore).getHost(); - String psPath = ((PrimaryDataStoreTO) dataStore).getPath(); - dataStoreUrl = "nfs://" + psHost + File.separator + psPath; + PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStore; + if (primaryDataStoreTO.getPoolType().equals(StoragePoolType.NetworkFilesystem)) { + String psHost = primaryDataStoreTO.getHost(); + String psPath = primaryDataStoreTO.getPath(); + dataStoreUrl = "nfs://" + psHost + File.separator + psPath; + } else if (primaryDataStoreTO.getPoolType().equals(StoragePoolType.SharedMountPoint) || + primaryDataStoreTO.getPoolType().equals(StoragePoolType.Filesystem)) { + dataStoreUrl = primaryDataStoreTO.getPath(); + } } final String volPath = dataStoreUrl + File.separator + data.getPath(); final int index = volPath.lastIndexOf("/"); From f3d814d8992f5caf7a243063f5a9a67873e0bfbc Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 27 Feb 2020 10:59:55 -0300 Subject: [PATCH 20/23] Last fix iso --- .../resource/LibvirtComputingResource.java | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 0d2cccde0328..660666c4b8b3 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2334,6 +2334,7 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { if (dataStore instanceof NfsTO) { NfsTO nfsStore = (NfsTO)data.getDataStore(); dataStoreUrl = nfsStore.getUrl(); + physicalDisk = getPhysicalDiskFromNfsStore(dataStoreUrl, data); } else if (dataStore instanceof PrimaryDataStoreTO) { //In order to support directly downloaded ISOs PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStore; @@ -2341,17 +2342,12 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { String psHost = primaryDataStoreTO.getHost(); String psPath = primaryDataStoreTO.getPath(); dataStoreUrl = "nfs://" + psHost + File.separator + psPath; + physicalDisk = getPhysicalDiskFromNfsStore(dataStoreUrl, data); } else if (primaryDataStoreTO.getPoolType().equals(StoragePoolType.SharedMountPoint) || primaryDataStoreTO.getPoolType().equals(StoragePoolType.Filesystem)) { - dataStoreUrl = primaryDataStoreTO.getPath(); + physicalDisk = getPhysicalDiskPrimaryStore(primaryDataStoreTO, data); } } - final String volPath = dataStoreUrl + File.separator + data.getPath(); - final int index = volPath.lastIndexOf("/"); - final String volDir = volPath.substring(0, index); - final String volName = volPath.substring(index + 1); - final KVMStoragePool secondaryStorage = _storagePoolMgr.getStoragePoolByURI(volDir); - physicalDisk = secondaryStorage.getPhysicalDisk(volName); } else if (volume.getType() != Volume.Type.ISO) { final PrimaryDataStoreTO store = (PrimaryDataStoreTO)data.getDataStore(); physicalDisk = _storagePoolMgr.getPhysicalDisk(store.getPoolType(), store.getUuid(), data.getPath()); @@ -2492,6 +2488,20 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { } + private KVMPhysicalDisk getPhysicalDiskPrimaryStore(PrimaryDataStoreTO primaryDataStoreTO, DataTO data) { + KVMStoragePool storagePool = _storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); + return storagePool.getPhysicalDisk(data.getPath()); + } + + private KVMPhysicalDisk getPhysicalDiskFromNfsStore(String dataStoreUrl, DataTO data) { + final String volPath = dataStoreUrl + File.separator + data.getPath(); + final int index = volPath.lastIndexOf("/"); + final String volDir = volPath.substring(0, index); + final String volName = volPath.substring(index + 1); + final KVMStoragePool storage = _storagePoolMgr.getStoragePoolByURI(volDir); + return storage.getPhysicalDisk(volName); + } + private void setBurstProperties(final VolumeObjectTO volumeObjectTO, final DiskDef disk ) { if (volumeObjectTO.getBytesReadRate() != null && volumeObjectTO.getBytesReadRate() > 0) { disk.setBytesReadRate(volumeObjectTO.getBytesReadRate()); From 2102864e8a57ae39a71d4839928923e43554bdff Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 27 Feb 2020 13:50:21 -0300 Subject: [PATCH 21/23] Fix VM migration with ISO --- .../cloudstack/storage/image/TemplateDataFactoryImpl.java | 2 +- .../main/java/com/cloud/template/TemplateManagerImpl.java | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java index afce5d2d27dd..8343a74d60b7 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateDataFactoryImpl.java @@ -68,7 +68,7 @@ public class TemplateDataFactoryImpl implements TemplateDataFactory { @Override public TemplateInfo getTemplate(long templateId, DataStore store) { VMTemplateVO templ = imageDataDao.findById(templateId); - if (store == null) { + if (store == null && !templ.isDirectDownload()) { TemplateObject tmpl = TemplateObject.getTemplate(templ, null); return tmpl; } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 769a814fd31d..c5472e6dea19 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -586,11 +586,7 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat } template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), poolId); } else { - template = _tmplFactory.getTemplate(vm.getIsoId(), DataStoreRole.Primary, dest.getDataCenter().getId()); - if (template.isDirectDownload() && template.getInstallPath() == null) { - s_logger.error("Error getting ISO for direct download"); - throw new CloudRuntimeException("Error getting ISO for direct download"); - } + template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), null); } if (template == null){ From 9034a8a0b658cb6a1906a8b6e5701b71b314fe01 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 4 Mar 2020 01:49:19 -0300 Subject: [PATCH 22/23] Refactor volume migration to remove secondary storage intermediate --- .../storage/resource/StorageProcessor.java | 2 ++ .../StorageSubsystemCommandHandlerBase.java | 5 ++- .../cloudstack/storage/to/VolumeObjectTO.java | 6 ++++ .../subsystem/api/storage/VolumeInfo.java | 4 +++ .../motion/AncientDataMotionStrategy.java | 18 ++++++++++ .../endpoint/DefaultEndPointSelector.java | 8 +++++ .../storage/volume/VolumeDataFactoryImpl.java | 10 ++++++ .../storage/volume/VolumeObject.java | 11 ++++++ .../kvm/storage/KVMStorageProcessor.java | 35 +++++++++++++++++++ .../ovm3/resources/Ovm3StorageProcessor.java | 5 +++ .../resource/VmwareStorageProcessor.java | 5 +++ .../resource/XenServerStorageProcessor.java | 5 +++ 12 files changed, 113 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/com/cloud/storage/resource/StorageProcessor.java b/core/src/main/java/com/cloud/storage/resource/StorageProcessor.java index 5d57616b7a43..f940e22f45a1 100644 --- a/core/src/main/java/com/cloud/storage/resource/StorageProcessor.java +++ b/core/src/main/java/com/cloud/storage/resource/StorageProcessor.java @@ -74,4 +74,6 @@ public interface StorageProcessor { public Answer resignature(ResignatureCommand cmd); public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd); + + Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd); } diff --git a/core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java b/core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java index 8c0399e9e19d..17b9b700d6c0 100644 --- a/core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java +++ b/core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java @@ -20,6 +20,7 @@ package com.cloud.storage.resource; import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand; +import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.log4j.Logger; import org.apache.cloudstack.storage.command.AttachCommand; @@ -95,7 +96,9 @@ protected Answer execute(CopyCommand cmd) { //copy volume from image cache to primary return processor.copyVolumeFromImageCacheToPrimary(cmd); } else if (srcData.getObjectType() == DataObjectType.VOLUME && srcData.getDataStore().getRole() == DataStoreRole.Primary) { - if (destData.getObjectType() == DataObjectType.VOLUME) { + if (destData.getObjectType() == DataObjectType.VOLUME && srcData instanceof VolumeObjectTO && ((VolumeObjectTO)srcData).isDirectDownload()) { + return processor.copyVolumeFromPrimaryToPrimary(cmd); + } else if (destData.getObjectType() == DataObjectType.VOLUME) { return processor.copyVolumeFromPrimaryToSecondary(cmd); } else if (destData.getObjectType() == DataObjectType.TEMPLATE) { return processor.createTemplateFromVolume(cmd); diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java index 5a9ff21cc121..e47d13ed6693 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/VolumeObjectTO.java @@ -61,6 +61,7 @@ public class VolumeObjectTO implements DataTO { private DiskCacheMode cacheMode; private Hypervisor.HypervisorType hypervisorType; private MigrationOptions migrationOptions; + private boolean directDownload; public VolumeObjectTO() { @@ -100,6 +101,7 @@ public VolumeObjectTO(VolumeInfo volume) { hypervisorType = volume.getHypervisorType(); setDeviceId(volume.getDeviceId()); this.migrationOptions = volume.getMigrationOptions(); + this.directDownload = volume.isDirectDownload(); } public String getUuid() { @@ -307,4 +309,8 @@ public DiskCacheMode getCacheMode() { public MigrationOptions getMigrationOptions() { return migrationOptions; } + + public boolean isDirectDownload() { + return directDownload; + } } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java index 99e47df81492..f4a738109013 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java @@ -79,4 +79,8 @@ public interface VolumeInfo extends DataObject, Volume { MigrationOptions getMigrationOptions(); void setMigrationOptions(MigrationOptions migrationOptions); + + boolean isDirectDownload(); + + void setDirectDownload(boolean directDownload); } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index bc49a53db581..9c8f4df3b6e2 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -325,7 +325,25 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) Scope destScope = getZoneScope(destData.getDataStore().getScope()); DataStore cacheStore = cacheMgr.getCacheStorage(destScope); + boolean bypassSecondaryStorage = false; + if (srcData instanceof VolumeInfo && ((VolumeInfo)srcData).isDirectDownload()) { + bypassSecondaryStorage = true; + } + if (cacheStore == null) { + if (bypassSecondaryStorage) { + CopyCommand cmd = new CopyCommand(srcData.getTO(), destData.getTO(), _copyvolumewait, VirtualMachineManager.ExecuteInSequence.value()); + EndPoint ep = selector.select(srcData, destData); + Answer answer = null; + if (ep == null) { + String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; + s_logger.error(errMsg); + answer = new Answer(cmd, false, errMsg); + } else { + answer = ep.sendMessage(cmd); + } + return answer; + } // need to find a nfs or cifs image store, assuming that can't copy volume // directly to s3 ImageStoreEntity imageStore = (ImageStoreEntity)dataStoreMgr.getImageStoreWithFreeCapacity(destScope.getScopeId()); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java index 9879f1bcf6a6..6e8bdaf4b8c1 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java @@ -84,6 +84,12 @@ protected boolean moveBetweenPrimaryImage(DataStore srcStore, DataStore destStor } } + private boolean moveBetweenPrimaryDirectDownload(DataStore srcStore, DataStore destStore) { + DataStoreRole srcRole = srcStore.getRole(); + DataStoreRole destRole = destStore.getRole(); + return srcRole == DataStoreRole.Primary && destRole == DataStoreRole.Primary; + } + protected boolean moveBetweenCacheAndImage(DataStore srcStore, DataStore destStore) { DataStoreRole srcRole = srcStore.getRole(); DataStoreRole destRole = destStore.getRole(); @@ -182,6 +188,8 @@ public EndPoint select(DataObject srcData, DataObject destData) { DataStore destStore = destData.getDataStore(); if (moveBetweenPrimaryImage(srcStore, destStore)) { return findEndPointForImageMove(srcStore, destStore); + } else if (moveBetweenPrimaryDirectDownload(srcStore, destStore)) { + return findEndPointForImageMove(srcStore, destStore); } else if (moveBetweenCacheAndImage(srcStore, destStore)) { // pick ssvm based on image cache dc DataStore selectedStore = null; diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java index 692f3cc97915..53fa21f3a794 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java @@ -23,6 +23,8 @@ import javax.inject.Inject; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateDao; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; @@ -42,6 +44,8 @@ public class VolumeDataFactoryImpl implements VolumeDataFactory { VolumeDataStoreDao volumeStoreDao; @Inject DataStoreManager storeMgr; + @Inject + VMTemplateDao templateDao; @Override public VolumeInfo getVolume(long volumeId, DataStore store) { @@ -90,6 +94,12 @@ public VolumeInfo getVolume(long volumeId) { DataStore store = storeMgr.getDataStore(volumeVO.getPoolId(), DataStoreRole.Primary); vol = VolumeObject.getVolumeObject(store, volumeVO); } + if (vol.getTemplateId() != null) { + VMTemplateVO template = templateDao.findById(vol.getTemplateId()); + if (template != null) { + vol.setDirectDownload(template.isDirectDownload()); + } + } return vol; } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java index d62a0baa04af..690a1124402d 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java @@ -74,6 +74,7 @@ public class VolumeObject implements VolumeInfo { DiskOfferingDao diskOfferingDao; private Object payload; private MigrationOptions migrationOptions; + private boolean directDownload; public VolumeObject() { _volStateMachine = Volume.State.getStateMachine(); @@ -327,6 +328,16 @@ public void setMigrationOptions(MigrationOptions migrationOptions) { this.migrationOptions = migrationOptions; } + @Override + public boolean isDirectDownload() { + return directDownload; + } + + @Override + public void setDirectDownload(boolean directDownload) { + this.directDownload = directDownload; + } + public void update() { volumeDao.update(volumeVO.getId(), volumeVO); volumeVO = volumeDao.findById(volumeVO.getId()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 9ff783bd5d66..17503ffd49bb 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1742,6 +1742,41 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return new DirectDownloadAnswer(true, template.getSize(), template.getName()); } + @Override + public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { + final DataTO srcData = cmd.getSrcTO(); + final DataTO destData = cmd.getDestTO(); + final VolumeObjectTO srcVol = (VolumeObjectTO)srcData; + final VolumeObjectTO destVol = (VolumeObjectTO)destData; + final ImageFormat srcFormat = srcVol.getFormat(); + final ImageFormat destFormat = destVol.getFormat(); + final DataStoreTO srcStore = srcData.getDataStore(); + final DataStoreTO destStore = destData.getDataStore(); + final PrimaryDataStoreTO primaryStore = (PrimaryDataStoreTO)srcStore; + final PrimaryDataStoreTO primaryStoreDest = (PrimaryDataStoreTO)destStore; + final String srcVolumePath = srcData.getPath(); + final String destVolumePath = destData.getPath(); + KVMStoragePool destPool = null; + + try { + final String volumeName = UUID.randomUUID().toString(); + + final String destVolumeName = volumeName + "." + destFormat.getFileExtension(); + final KVMPhysicalDisk volume = storagePoolMgr.getPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), srcVolumePath); + volume.setFormat(PhysicalDiskFormat.valueOf(srcFormat.toString())); + + destPool = storagePoolMgr.getStoragePool(primaryStoreDest.getPoolType(), primaryStoreDest.getUuid()); + storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds()); + final VolumeObjectTO newVol = new VolumeObjectTO(); + newVol.setPath(destVolumePath + File.separator + destVolumeName); + newVol.setFormat(destFormat); + return new CopyCmdAnswer(newVol); + } catch (final CloudRuntimeException e) { + s_logger.debug("Failed to copyVolumeFromPrimaryToPrimary: ", e); + return new CopyCmdAnswer(e.toString()); + } + } + /** * True if location exists */ diff --git a/plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessor.java b/plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessor.java index 5f43c38f6075..7915586fca3f 100644 --- a/plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessor.java +++ b/plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3StorageProcessor.java @@ -826,6 +826,11 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return null; } + @Override + public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { + return null; + } + /** * Attach disks * @param cmd diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index f67b242ff1c3..3755f2e2e1a4 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -3554,4 +3554,9 @@ void setFullCloneFlag(boolean value){ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { return null; } + + @Override + public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { + return null; + } } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java index 458b7d6377a2..e4c07d4ba79e 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java @@ -209,6 +209,11 @@ public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) return null; } + @Override + public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { + return null; + } + @Override public AttachAnswer attachIso(final AttachCommand cmd) { final DiskTO disk = cmd.getDisk(); From eef4c637b57c67c5e1ab1a12348b846ab9bd8159 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Thu, 5 Mar 2020 09:38:26 -0300 Subject: [PATCH 23/23] Fix simulator issue --- .../java/com/cloud/resource/SimulatorStorageProcessor.java | 5 +++++ .../main/java/com/cloud/template/TemplateManagerImpl.java | 4 +--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/hypervisors/simulator/src/main/java/com/cloud/resource/SimulatorStorageProcessor.java b/plugins/hypervisors/simulator/src/main/java/com/cloud/resource/SimulatorStorageProcessor.java index c2dfdbd4e565..e4ef4dfc1f63 100644 --- a/plugins/hypervisors/simulator/src/main/java/com/cloud/resource/SimulatorStorageProcessor.java +++ b/plugins/hypervisors/simulator/src/main/java/com/cloud/resource/SimulatorStorageProcessor.java @@ -264,4 +264,9 @@ public Answer forgetObject(ForgetObjectCmd cmd) { // TODO Auto-generated method stub return null; } + + @Override + public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) { + return null; + } } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index c5472e6dea19..749f272bf361 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -584,10 +584,8 @@ public void prepareIsoForVmProfile(VirtualMachineProfile profile, DeployDestinat } poolId = storagePool.getId(); } - template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), poolId); - } else { - template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), null); } + template = prepareIso(vm.getIsoId(), vm.getDataCenterId(), dest.getHost().getId(), poolId); if (template == null){ s_logger.error("Failed to prepare ISO on secondary or cache storage");