diff --git a/core/src/main/java/com/cloud/agent/api/ConvertInstanceAnswer.java b/core/src/main/java/com/cloud/agent/api/ConvertInstanceAnswer.java new file mode 100644 index 000000000000..4e0ab0d51f60 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/ConvertInstanceAnswer.java @@ -0,0 +1,40 @@ +// 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.api; + +import org.apache.cloudstack.vm.UnmanagedInstanceTO; + +public class ConvertInstanceAnswer extends Answer { + + public ConvertInstanceAnswer() { + super(); + } + private String temporaryConvertUuid; + + public ConvertInstanceAnswer(Command command, boolean success, String details) { + super(command, success, details); + } + + public ConvertInstanceAnswer(Command command, String temporaryConvertUuid) { + super(command, true, ""); + this.temporaryConvertUuid = temporaryConvertUuid; + } + + public String getTemporaryConvertUuid() { + return temporaryConvertUuid; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/ImportConvertedInstanceCommand.java b/core/src/main/java/com/cloud/agent/api/ImportConvertedInstanceCommand.java index fb4b5b6db52d..9d50e852ced4 100644 --- a/core/src/main/java/com/cloud/agent/api/ImportConvertedInstanceCommand.java +++ b/core/src/main/java/com/cloud/agent/api/ImportConvertedInstanceCommand.java @@ -26,15 +26,18 @@ public class ImportConvertedInstanceCommand extends Command { private RemoteInstanceTO sourceInstance; private List destinationStoragePools; private DataStoreTO conversionTemporaryLocation; + private String temporaryConvertUuid; public ImportConvertedInstanceCommand() { } public ImportConvertedInstanceCommand(RemoteInstanceTO sourceInstance, - List destinationStoragePools, DataStoreTO conversionTemporaryLocation) { + List destinationStoragePools, + DataStoreTO conversionTemporaryLocation, String temporaryConvertUuid) { this.sourceInstance = sourceInstance; this.destinationStoragePools = destinationStoragePools; this.conversionTemporaryLocation = conversionTemporaryLocation; + this.temporaryConvertUuid = temporaryConvertUuid; } public RemoteInstanceTO getSourceInstance() { @@ -49,6 +52,10 @@ public DataStoreTO getConversionTemporaryLocation() { return conversionTemporaryLocation; } + public String getTemporaryConvertUuid() { + return temporaryConvertUuid; + } + @Override public boolean executeInSequence() { return false; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java index b00ae3ce20e8..704ed5b9a278 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java @@ -122,6 +122,7 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve final String temporaryConvertUuid = UUID.randomUUID().toString(); boolean verboseModeEnabled = serverResource.isConvertInstanceVerboseModeEnabled(); + boolean cleanupSecondaryStorage = false; try { boolean result = performInstanceConversion(sourceOVFDirPath, temporaryConvertPath, temporaryConvertUuid, timeout, verboseModeEnabled); @@ -131,11 +132,12 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve s_logger.error(err); return new Answer(cmd, false, err); } - return new Answer(cmd, false, null); + return new Answer(cmd, true, null); } catch (Exception e) { String error = String.format("Error converting instance %s from %s, due to: %s", sourceInstanceName, sourceHypervisorType, e.getMessage()); s_logger.error(error, e); + cleanupSecondaryStorage = true; return new Answer(cmd, false, error); } finally { if (ovfExported && StringUtils.isNotBlank(ovfTemplateDirOnConversionLocation)) { @@ -143,6 +145,10 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve s_logger.debug("Cleaning up exported OVA at dir " + sourceOVFDir); FileUtil.deletePath(sourceOVFDir); } + if (cleanupSecondaryStorage && conversionTemporaryLocation instanceof NfsTO) { + s_logger.debug("Cleaning up secondary storage temporary location"); + storagePoolMgr.deleteStoragePool(temporaryStoragePool.getType(), temporaryStoragePool.getUuid()); + } } } @@ -183,44 +189,6 @@ private String getExportOVAUrlFromRemoteInstance(RemoteInstanceTO vmwareInstance encodedUsername, encodedPassword, vcenter, datacenter, vm); } - protected List getTemporaryDisksFromParsedXml(KVMStoragePool pool, LibvirtDomainXMLParser xmlParser, String convertedBasePath) { - List disksDefs = xmlParser.getDisks(); - disksDefs = disksDefs.stream().filter(x -> x.getDiskType() == LibvirtVMDef.DiskDef.DiskType.FILE && - x.getDeviceType() == LibvirtVMDef.DiskDef.DeviceType.DISK).collect(Collectors.toList()); - if (CollectionUtils.isEmpty(disksDefs)) { - String err = String.format("Cannot find any disk defined on the converted XML domain %s.xml", convertedBasePath); - s_logger.error(err); - throw new CloudRuntimeException(err); - } - sanitizeDisksPath(disksDefs); - return getPhysicalDisksFromDefPaths(disksDefs, pool); - } - - private List getPhysicalDisksFromDefPaths(List disksDefs, KVMStoragePool pool) { - List disks = new ArrayList<>(); - for (LibvirtVMDef.DiskDef diskDef : disksDefs) { - KVMPhysicalDisk physicalDisk = pool.getPhysicalDisk(diskDef.getDiskPath()); - disks.add(physicalDisk); - } - return disks; - } - - protected List getTemporaryDisksWithPrefixFromTemporaryPool(KVMStoragePool pool, String path, String prefix) { - String msg = String.format("Could not parse correctly the converted XML domain, checking for disks on %s with prefix %s", path, prefix); - s_logger.info(msg); - pool.refresh(); - List disksWithPrefix = pool.listPhysicalDisks() - .stream() - .filter(x -> x.getName().startsWith(prefix) && !x.getName().endsWith(".xml")) - .collect(Collectors.toList()); - if (CollectionUtils.isEmpty(disksWithPrefix)) { - msg = String.format("Could not find any converted disk with prefix %s on temporary location %s", prefix, path); - s_logger.error(msg); - throw new CloudRuntimeException(msg); - } - return disksWithPrefix; - } - protected void sanitizeDisksPath(List disks) { for (LibvirtVMDef.DiskDef disk : disks) { String[] diskPathParts = disk.getDiskPath().split("/"); @@ -229,89 +197,6 @@ protected void sanitizeDisksPath(List disks) { } } - protected List moveTemporaryDisksToDestination(List temporaryDisks, - List destinationStoragePools, - KVMStoragePoolManager storagePoolMgr) { - List targetDisks = new ArrayList<>(); - if (temporaryDisks.size() != destinationStoragePools.size()) { - String warn = String.format("Discrepancy between the converted instance disks (%s) " + - "and the expected number of disks (%s)", temporaryDisks.size(), destinationStoragePools.size()); - s_logger.warn(warn); - } - for (int i = 0; i < temporaryDisks.size(); i++) { - String poolPath = destinationStoragePools.get(i); - KVMStoragePool destinationPool = storagePoolMgr.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, poolPath); - if (destinationPool == null) { - String err = String.format("Could not find a storage pool by URI: %s", poolPath); - s_logger.error(err); - continue; - } - if (destinationPool.getType() != Storage.StoragePoolType.NetworkFilesystem) { - String err = String.format("Storage pool by URI: %s is not an NFS storage", poolPath); - s_logger.error(err); - continue; - } - KVMPhysicalDisk sourceDisk = temporaryDisks.get(i); - if (s_logger.isDebugEnabled()) { - String msg = String.format("Trying to copy converted instance disk number %s from the temporary location %s" + - " to destination storage pool %s", i, sourceDisk.getPool().getLocalPath(), destinationPool.getUuid()); - s_logger.debug(msg); - } - - String destinationName = UUID.randomUUID().toString(); - - KVMPhysicalDisk destinationDisk = storagePoolMgr.copyPhysicalDisk(sourceDisk, destinationName, destinationPool, 7200 * 1000); - targetDisks.add(destinationDisk); - } - return targetDisks; - } - - protected List getUnmanagedInstanceDisks(List vmDisks, LibvirtDomainXMLParser xmlParser) { - List instanceDisks = new ArrayList<>(); - List diskDefs = xmlParser != null ? xmlParser.getDisks() : null; - for (int i = 0; i< vmDisks.size(); i++) { - KVMPhysicalDisk physicalDisk = vmDisks.get(i); - KVMStoragePool storagePool = physicalDisk.getPool(); - UnmanagedInstanceTO.Disk disk = new UnmanagedInstanceTO.Disk(); - disk.setPosition(i); - Pair storagePoolHostAndPath = getNfsStoragePoolHostAndPath(storagePool); - disk.setDatastoreHost(storagePoolHostAndPath.first()); - disk.setDatastorePath(storagePoolHostAndPath.second()); - disk.setDatastoreName(storagePool.getUuid()); - disk.setDatastoreType(storagePool.getType().name()); - disk.setCapacity(physicalDisk.getVirtualSize()); - disk.setFileBaseName(physicalDisk.getName()); - if (CollectionUtils.isNotEmpty(diskDefs)) { - LibvirtVMDef.DiskDef diskDef = diskDefs.get(i); - disk.setController(diskDef.getBusType() != null ? diskDef.getBusType().toString() : LibvirtVMDef.DiskDef.DiskBus.VIRTIO.toString()); - } else { - // If the job is finished but we cannot parse the XML, the guest VM can use the virtio driver - disk.setController(LibvirtVMDef.DiskDef.DiskBus.VIRTIO.toString()); - } - instanceDisks.add(disk); - } - return instanceDisks; - } - - protected Pair getNfsStoragePoolHostAndPath(KVMStoragePool storagePool) { - String sourceHostIp = null; - String sourcePath = null; - List commands = new ArrayList<>(); - commands.add(new String[]{Script.getExecutableAbsolutePath("mount")}); - commands.add(new String[]{Script.getExecutableAbsolutePath("grep"), storagePool.getLocalPath()}); - String storagePoolMountPoint = Script.executePipedCommands(commands, 0).second(); - s_logger.debug(String.format("NFS Storage pool: %s - local path: %s, mount point: %s", storagePool.getUuid(), storagePool.getLocalPath(), storagePoolMountPoint)); - if (StringUtils.isNotEmpty(storagePoolMountPoint)) { - String[] res = storagePoolMountPoint.strip().split(" "); - res = res[0].split(":"); - if (res.length > 1) { - sourceHostIp = res[0].strip(); - sourcePath = res[1].strip(); - } - } - return new Pair<>(sourceHostIp, sourcePath); - } - private boolean exportOVAFromVMOnVcenter(String vmExportUrl, String targetOvfDir, int noOfThreads, diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java index b0411ed5cdfd..3af41253b92a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java @@ -69,13 +69,12 @@ public Answer execute(ImportConvertedInstanceCommand cmd, LibvirtComputingResour String sourceInstanceName = sourceInstance.getInstanceName(); List destinationStoragePools = cmd.getDestinationStoragePools(); DataStoreTO conversionTemporaryLocation = cmd.getConversionTemporaryLocation(); + final String temporaryConvertUuid = cmd.getTemporaryConvertUuid(); final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr(); KVMStoragePool temporaryStoragePool = getTemporaryStoragePool(conversionTemporaryLocation, storagePoolMgr); final String temporaryConvertPath = temporaryStoragePool.getLocalPath(); - final String temporaryConvertUuid = UUID.randomUUID().toString(); - try { String convertedBasePath = String.format("%s/%s", temporaryConvertPath, temporaryConvertUuid); LibvirtDomainXMLParser xmlParser = parseMigratedVMXmlDomain(convertedBasePath); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java index 1cc2a60e380c..5ea3a64ea1b8 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapperTest.java @@ -23,7 +23,6 @@ import java.util.UUID; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; -import org.apache.cloudstack.vm.UnmanagedInstanceTO; import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -41,13 +40,10 @@ import com.cloud.agent.api.to.RemoteInstanceTO; import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; -import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser; import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk; import com.cloud.hypervisor.kvm.storage.KVMStoragePool; import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; -import com.cloud.storage.Storage; -import com.cloud.utils.Pair; import com.cloud.utils.script.Script; @RunWith(MockitoJUnitRunner.class) @@ -80,7 +76,6 @@ public void setUp() { Mockito.when(storagePoolManager.getStoragePoolByURI(secondaryPoolUrl)).thenReturn(temporaryPool); KVMPhysicalDisk physicalDisk1 = Mockito.mock(KVMPhysicalDisk.class); KVMPhysicalDisk physicalDisk2 = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(temporaryPool.listPhysicalDisks()).thenReturn(Arrays.asList(physicalDisk1, physicalDisk2)); } @Test @@ -107,51 +102,6 @@ public void testGetTemporaryStoragePool() { Assert.assertNotNull(temporaryStoragePool); } - @Test - public void testGetTemporaryDisksWithPrefixFromTemporaryPool() { - String convertPath = "/xyz"; - String convertPrefix = UUID.randomUUID().toString(); - KVMPhysicalDisk physicalDisk1 = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(physicalDisk1.getName()).thenReturn("disk1"); - KVMPhysicalDisk physicalDisk2 = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(physicalDisk2.getName()).thenReturn("disk2"); - - KVMPhysicalDisk convertedDisk1 = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(convertedDisk1.getName()).thenReturn(String.format("%s-sda", convertPrefix)); - KVMPhysicalDisk convertedDisk2 = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(convertedDisk2.getName()).thenReturn(String.format("%s-sdb", convertPrefix)); - KVMPhysicalDisk convertedXml = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(convertedXml.getName()).thenReturn(String.format("%s.xml", convertPrefix)); - Mockito.when(temporaryPool.listPhysicalDisks()).thenReturn(Arrays.asList(physicalDisk1, physicalDisk2, - convertedDisk1, convertedDisk2, convertedXml)); - - List convertedDisks = convertInstanceCommandWrapper.getTemporaryDisksWithPrefixFromTemporaryPool(temporaryPool, convertPath, convertPrefix); - Assert.assertEquals(2, convertedDisks.size()); - } - - @Test - public void testGetTemporaryDisksFromParsedXml() { - String relativePath = UUID.randomUUID().toString(); - String fullPath = String.format("/mnt/xyz/%s", relativePath); - - LibvirtVMDef.DiskDef diskDef = new LibvirtVMDef.DiskDef(); - LibvirtVMDef.DiskDef.DiskBus bus = LibvirtVMDef.DiskDef.DiskBus.VIRTIO; - LibvirtVMDef.DiskDef.DiskFmtType type = LibvirtVMDef.DiskDef.DiskFmtType.QCOW2; - diskDef.defFileBasedDisk(fullPath, "test", bus, type); - - LibvirtDomainXMLParser parser = Mockito.mock(LibvirtDomainXMLParser.class); - Mockito.when(parser.getDisks()).thenReturn(List.of(diskDef)); - - KVMPhysicalDisk convertedDisk1 = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(convertedDisk1.getName()).thenReturn("disk1"); - Mockito.when(temporaryPool.getPhysicalDisk(relativePath)).thenReturn(convertedDisk1); - - List disks = convertInstanceCommandWrapper.getTemporaryDisksFromParsedXml(temporaryPool, parser, ""); - Mockito.verify(convertInstanceCommandWrapper).sanitizeDisksPath(List.of(diskDef)); - Assert.assertEquals(1, disks.size()); - Assert.assertEquals("disk1", disks.get(0).getName()); - } - @Test public void testSanitizeDisksPath() { String relativePath = UUID.randomUUID().toString(); @@ -165,73 +115,6 @@ public void testSanitizeDisksPath() { Assert.assertEquals(relativePath, diskDef.getDiskPath()); } - @Test - public void testMoveTemporaryDisksToDestination() { - KVMPhysicalDisk sourceDisk = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(sourceDisk.getPool()).thenReturn(temporaryPool); - List disks = List.of(sourceDisk); - String destinationPoolUuid = UUID.randomUUID().toString(); - List destinationPools = List.of(destinationPoolUuid); - - KVMPhysicalDisk destDisk = Mockito.mock(KVMPhysicalDisk.class); - Mockito.when(destDisk.getPath()).thenReturn("xyz"); - Mockito.when(storagePoolManager.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, destinationPoolUuid)) - .thenReturn(destinationPool); - Mockito.when(destinationPool.getType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); - Mockito.when(storagePoolManager.copyPhysicalDisk(Mockito.eq(sourceDisk), Mockito.anyString(), Mockito.eq(destinationPool), Mockito.anyInt())) - .thenReturn(destDisk); - - List movedDisks = convertInstanceCommandWrapper.moveTemporaryDisksToDestination(disks, destinationPools, storagePoolManager); - Assert.assertEquals(1, movedDisks.size()); - Assert.assertEquals("xyz", movedDisks.get(0).getPath()); - } - - @Test - public void testGetUnmanagedInstanceDisks() { - try (MockedStatic