Merge ce87ce4ae3 into b7d4df0a11

core/src/main/java/com/cloud/agent/api/CleanupConvertedInstanceDisksCommand.java

@@ -0,0 +1,57 @@
+//
+// 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 com.cloud.agent.api.to.DataStoreTO;
+
+/**
+ * This command is used to cleanup the converted instance disks from the storage pool: vmVolumesStore and the prefix: vmVolumesPrefix.
+ */
+public class CleanupConvertedInstanceDisksCommand extends Command {
+
+    private DataStoreTO vmVolumesStore;
+    private String vmVolumesPrefix;
+
+    public CleanupConvertedInstanceDisksCommand(DataStoreTO vmVolumesStore, String vmVolumesPrefix) {
+        this.vmVolumesStore = vmVolumesStore;
+        this.vmVolumesPrefix = vmVolumesPrefix;
+    }
+
+    public DataStoreTO getVmVolumesStore() {
+        return vmVolumesStore;
+    }
+
+    public void setVmVolumesStore(DataStoreTO vmVolumesStore) {
+        this.vmVolumesStore = vmVolumesStore;
+    }
+
+    public String getVmVolumesPrefix() {
+        return vmVolumesPrefix;
+    }
+
+    public void setVmVolumesPrefix(String vmVolumesPrefix) {
+        this.vmVolumesPrefix = vmVolumesPrefix;
+    }
+
+    @Override
+    public boolean executeInSequence() {
+        return false;
+    }
+}

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBaseConvertCommandWrapper.java

@@ -0,0 +1,283 @@
+//
+// 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.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.Command;
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.agent.api.to.NfsTO;
+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.resource.CommandWrapper;
+import com.cloud.resource.ServerResource;
+import com.cloud.storage.Storage;
+import com.cloud.utils.FileUtil;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.cloudstack.vm.UnmanagedInstanceTO;
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
+
+import java.io.BufferedInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.Charset;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.stream.Collectors;
+
+public abstract class LibvirtBaseConvertCommandWrapper <T extends Command, A extends Answer, R extends ServerResource> extends CommandWrapper<T, A, R> {
+
+    protected KVMStoragePool getTemporaryStoragePool(DataStoreTO conversionTemporaryLocation, KVMStoragePoolManager storagePoolMgr) {
+        if (conversionTemporaryLocation instanceof NfsTO) {
+            NfsTO nfsTO = (NfsTO) conversionTemporaryLocation;
+            return storagePoolMgr.getStoragePoolByURI(nfsTO.getUrl());
+        } else {
+            PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) conversionTemporaryLocation;
+            return storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
+        }
+    }
+
+    protected List<KVMPhysicalDisk> getTemporaryDisksFromParsedXml(KVMStoragePool pool, LibvirtDomainXMLParser xmlParser,
+                                                                   String convertedBasePath,
+                                                                   String conversionPoolPath, String vmVolumesPrefix) {
+        List<LibvirtVMDef.DiskDef> 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, " +
+                    "checking disks at: %s with prefix: %s", convertedBasePath, conversionPoolPath, vmVolumesPrefix);
+            logger.warn(err);
+            return getTemporaryDisksWithPrefixFromTemporaryPool(pool, conversionPoolPath, vmVolumesPrefix);
+        }
+        sanitizeDisksPath(disksDefs);
+        return getPhysicalDisksFromDefPaths(disksDefs, pool);
+    }
+
+    private List<KVMPhysicalDisk> getPhysicalDisksFromDefPaths(List<LibvirtVMDef.DiskDef> disksDefs, KVMStoragePool pool) {
+        List<KVMPhysicalDisk> disks = new ArrayList<>();
+        for (LibvirtVMDef.DiskDef diskDef : disksDefs) {
+            KVMPhysicalDisk physicalDisk = pool.getPhysicalDisk(diskDef.getDiskPath());
+            disks.add(physicalDisk);
+        }
+        return disks;
+    }
+
+    protected List<KVMPhysicalDisk> 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);
+        logger.info(msg);
+        pool.refresh();
+        List<KVMPhysicalDisk> 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);
+            logger.error(msg);
+            throw new CloudRuntimeException(msg);
+        }
+        return disksWithPrefix;
+    }
+
+    protected void cleanupDisksAndDomainFromTemporaryLocation(List<KVMPhysicalDisk> disks,
+                                                            KVMStoragePool temporaryStoragePool,
+                                                            String temporaryConvertUuid, boolean xmlExists) {
+        for (KVMPhysicalDisk disk : disks) {
+            logger.info(String.format("Cleaning up temporary disk %s after conversion from temporary location", disk.getName()));
+            temporaryStoragePool.deletePhysicalDisk(disk.getName(), Storage.ImageFormat.QCOW2);
+        }
+        if (xmlExists) {
+            logger.info(String.format("Cleaning up temporary domain %s after conversion from temporary location", temporaryConvertUuid));
+            FileUtil.deleteFiles(temporaryStoragePool.getLocalPath(), temporaryConvertUuid, ".xml");
+        }
+    }
+
+    protected void sanitizeDisksPath(List<LibvirtVMDef.DiskDef> disks) {
+        for (LibvirtVMDef.DiskDef disk : disks) {
+            String[] diskPathParts = disk.getDiskPath().split("/");
+            String relativePath = diskPathParts[diskPathParts.length - 1];
+            disk.setDiskPath(relativePath);
+        }
+    }
+
+    protected List<KVMPhysicalDisk> moveTemporaryDisksToDestination(List<KVMPhysicalDisk> temporaryDisks,
+                                                                    List<String> destinationStoragePools,
+                                                                    KVMStoragePoolManager storagePoolMgr) {
+        List<KVMPhysicalDisk> 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());
+            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);
+                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);
+                logger.error(err);
+                continue;
+            }
+            KVMPhysicalDisk sourceDisk = temporaryDisks.get(i);
+            if (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());
+                logger.debug(msg);
+            }
+
+            String destinationName = UUID.randomUUID().toString();
+
+            try {
+                if (destinationPool.getAvailable() < sourceDisk.getSize()) {
+                    String msg = String.format("Not enough space on destination pool %s (%s bytes) to copy disk %s (size %s)",
+                            destinationPool.getUuid(), destinationPool.getAvailable(), sourceDisk.getName(), sourceDisk.getSize());
+                    logger.error(msg);
+                    throw new CloudRuntimeException(msg);
+                }
+
+                KVMPhysicalDisk destinationDisk = storagePoolMgr.copyPhysicalDisk(sourceDisk, destinationName, destinationPool, 7200 * 1000);
+                targetDisks.add(destinationDisk);
+            } catch (Exception e) {
+                String err = String.format("Error copying converted instance disk number %s from the temporary location %s" +
+                        " to destination storage pool %s: %s", i, sourceDisk.getPool().getLocalPath(), destinationPool.getUuid(), e.getMessage());
+                logger.error(err, e);
+                cleanupMovedDisksOnDestinationPool(targetDisks);
+                return null;
+            }
+        }
+        return targetDisks;
+    }
+
+    private void cleanupMovedDisksOnDestinationPool(List<KVMPhysicalDisk> targetDisks) {
+        if (CollectionUtils.isEmpty(targetDisks)) {
+            return;
+        }
+        for (KVMPhysicalDisk disk : targetDisks) {
+            logger.info(String.format("Cleaning up disk %s from pool %s after conversion", disk.getName(), disk.getPool().getUuid()));
+            disk.getPool().deletePhysicalDisk(disk.getName(), Storage.ImageFormat.QCOW2);
+        }
+    }
+
+    protected UnmanagedInstanceTO getConvertedUnmanagedInstance(String baseName,
+                                                              List<KVMPhysicalDisk> vmDisks,
+                                                              LibvirtDomainXMLParser xmlParser) {
+        UnmanagedInstanceTO instanceTO = new UnmanagedInstanceTO();
+        instanceTO.setName(baseName);
+        instanceTO.setDisks(getUnmanagedInstanceDisks(vmDisks, xmlParser));
+        instanceTO.setNics(getUnmanagedInstanceNics(xmlParser));
+        return instanceTO;
+    }
+
+    private List<UnmanagedInstanceTO.Nic> getUnmanagedInstanceNics(LibvirtDomainXMLParser xmlParser) {
+        List<UnmanagedInstanceTO.Nic> nics = new ArrayList<>();
+        if (xmlParser != null) {
+            List<LibvirtVMDef.InterfaceDef> interfaces = xmlParser.getInterfaces();
+            for (LibvirtVMDef.InterfaceDef interfaceDef : interfaces) {
+                UnmanagedInstanceTO.Nic nic = new UnmanagedInstanceTO.Nic();
+                nic.setMacAddress(interfaceDef.getMacAddress());
+                nic.setNicId(interfaceDef.getBrName());
+                nic.setAdapterType(interfaceDef.getModel().toString());
+                nics.add(nic);
+            }
+        }
+        return nics;
+    }
+
+    protected List<UnmanagedInstanceTO.Disk> getUnmanagedInstanceDisks(List<KVMPhysicalDisk> vmDisks, LibvirtDomainXMLParser xmlParser) {
+        List<UnmanagedInstanceTO.Disk> instanceDisks = new ArrayList<>();
+        List<LibvirtVMDef.DiskDef> 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<String, String> 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<String, String> getNfsStoragePoolHostAndPath(KVMStoragePool storagePool) {
+        String sourceHostIp = null;
+        String sourcePath = null;
+        List<String[]> 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();
+        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);
+    }
+
+    protected LibvirtDomainXMLParser parseMigratedVMXmlDomain(String installPath) throws IOException {
+        String xmlPath = String.format("%s.xml", installPath);
+        if (!new File(xmlPath).exists()) {
+            String err = String.format("Conversion failed. Unable to find the converted XML domain, expected %s", xmlPath);
+            logger.error(err);
+            throw new CloudRuntimeException(err);
+        }
+        String xml;
+        try (InputStream is = new BufferedInputStream(new FileInputStream(xmlPath))) {
+            xml = IOUtils.toString(is, Charset.defaultCharset());
+        }
+        final LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
+        try {
+            parser.parseDomainXML(xml);
+            return parser;
+        } catch (RuntimeException e) {
+            String err = String.format("Error parsing the converted instance XML domain at %s: %s", xmlPath, e.getMessage());
+            logger.error(err, e);
+            logger.debug(xml);
+            return null;
+        }
+    }
+}

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupConvertedInstanceDisksCommandWrapper.java

@@ -0,0 +1,67 @@
+//
+// 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.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CleanupConvertedInstanceDisksCommand;
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
+import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.resource.ResourceWrapper;
+
+import java.io.File;
+import java.util.List;
+
+@ResourceWrapper(handles = CleanupConvertedInstanceDisksCommand.class)
+public class LibvirtCleanupConvertedInstanceDisksCommandWrapper extends LibvirtBaseConvertCommandWrapper<CleanupConvertedInstanceDisksCommand, Answer, LibvirtComputingResource> {
+
+    @Override
+    public Answer execute(CleanupConvertedInstanceDisksCommand command, LibvirtComputingResource serverResource) {
+        DataStoreTO vmVolumesStore = command.getVmVolumesStore();
+        String vmVolumesPrefix = command.getVmVolumesPrefix();
+
+        final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
+        KVMStoragePool conversionPool = getTemporaryStoragePool(vmVolumesStore, storagePoolMgr);
+        final String conversionPoolPath = conversionPool.getLocalPath();
+
+        try {
+            String volumesBasePath = String.format("%s/%s", conversionPoolPath, vmVolumesPrefix);
+            String xmlPath = String.format("%s.xml", volumesBasePath);
+            boolean xmlExists = new File(xmlPath).exists();
+
+            LibvirtDomainXMLParser xmlParser = xmlExists ? parseMigratedVMXmlDomain(volumesBasePath) : null;
+            List<KVMPhysicalDisk> temporaryDisks = xmlExists && xmlParser != null ?
+                    getTemporaryDisksFromParsedXml(conversionPool, xmlParser, volumesBasePath, conversionPoolPath, vmVolumesPrefix) :
+                    getTemporaryDisksWithPrefixFromTemporaryPool(conversionPool, conversionPoolPath, vmVolumesPrefix);
+
+            cleanupDisksAndDomainFromTemporaryLocation(temporaryDisks, conversionPool, vmVolumesPrefix, xmlExists);
+
+        } catch (Exception e) {
+            String error = String.format("Error cleaning up converted disks with prefix %s from %s, due to: %s",
+                    vmVolumesPrefix, conversionPoolPath, e.getMessage());
+            logger.error(error, e);
+            return new Answer(command, false, error);
+        }
+
+        return new Answer(command);
+    }
+}

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java

@@ -18,22 +18,9 @@
 //
 package com.cloud.hypervisor.kvm.resource.wrapper;
 
-import java.io.BufferedInputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.nio.charset.Charset;
-import java.util.ArrayList;
 import java.util.List;
-import java.util.UUID;
-import java.util.stream.Collectors;
 
-import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.cloudstack.vm.UnmanagedInstanceTO;
-import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.io.IOUtils;
-import org.apache.commons.lang3.StringUtils;
 
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.ImportConvertedInstanceAnswer;
@@ -44,20 +31,14 @@ 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.resource.CommandWrapper;
 import com.cloud.resource.ResourceWrapper;
-import com.cloud.storage.Storage;
-import com.cloud.utils.FileUtil;
-import com.cloud.utils.Pair;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
+import org.apache.commons.collections4.CollectionUtils;
 
 @ResourceWrapper(handles =  ImportConvertedInstanceCommand.class)
-public class LibvirtImportConvertedInstanceCommandWrapper extends CommandWrapper<ImportConvertedInstanceCommand, Answer, LibvirtComputingResource> {
+public class LibvirtImportConvertedInstanceCommandWrapper extends LibvirtBaseConvertCommandWrapper<ImportConvertedInstanceCommand, Answer, LibvirtComputingResource> {
 
     @Override
     public Answer execute(ImportConvertedInstanceCommand cmd, LibvirtComputingResource serverResource) {
@@ -79,16 +60,22 @@ public class LibvirtImportConvertedInstanceCommandWrapper extends CommandWrapper
 
             List<KVMPhysicalDisk> temporaryDisks = xmlParser == null ?
                     getTemporaryDisksWithPrefixFromTemporaryPool(temporaryStoragePool, temporaryConvertPath, temporaryConvertUuid) :
-                    getTemporaryDisksFromParsedXml(temporaryStoragePool, xmlParser, convertedBasePath);
+                    getTemporaryDisksFromParsedXml(temporaryStoragePool, xmlParser, convertedBasePath, temporaryConvertPath, temporaryConvertUuid);
 
-            List<KVMPhysicalDisk> disks = null;
+            List<KVMPhysicalDisk> disks;
             if (forceConvertToPool) {
                 // Force flag to use the conversion path, no need to move disks
                 disks = temporaryDisks;
             } else {
                 disks = moveTemporaryDisksToDestination(temporaryDisks,
                         destinationStoragePools, storagePoolMgr);
-                cleanupDisksAndDomainFromTemporaryLocation(temporaryDisks, temporaryStoragePool, temporaryConvertUuid);
+                cleanupDisksAndDomainFromTemporaryLocation(temporaryDisks, temporaryStoragePool, temporaryConvertUuid, true);
+            }
+
+            if (CollectionUtils.isEmpty(disks)) {
+                String msg = String.format("Unable to import the converted disks for VM %s from destination pools", sourceInstanceName);
+                logger.error(msg);
+                return new ImportConvertedInstanceAnswer(cmd, false, msg);
             }
 
             UnmanagedInstanceTO convertedInstanceTO = getConvertedUnmanagedInstance(temporaryConvertUuid,
@@ -106,200 +93,4 @@ public class LibvirtImportConvertedInstanceCommandWrapper extends CommandWrapper
             }
         }
     }
-
-    protected KVMStoragePool getTemporaryStoragePool(DataStoreTO conversionTemporaryLocation, KVMStoragePoolManager storagePoolMgr) {
-        if (conversionTemporaryLocation instanceof NfsTO) {
-            NfsTO nfsTO = (NfsTO) conversionTemporaryLocation;
-            return storagePoolMgr.getStoragePoolByURI(nfsTO.getUrl());
-        } else {
-            PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) conversionTemporaryLocation;
-            return storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
-        }
-    }
-
-    protected List<KVMPhysicalDisk> getTemporaryDisksFromParsedXml(KVMStoragePool pool, LibvirtDomainXMLParser xmlParser, String convertedBasePath) {
-        List<LibvirtVMDef.DiskDef> 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);
-            logger.error(err);
-            throw new CloudRuntimeException(err);
-        }
-        sanitizeDisksPath(disksDefs);
-        return getPhysicalDisksFromDefPaths(disksDefs, pool);
-    }
-
-    private List<KVMPhysicalDisk> getPhysicalDisksFromDefPaths(List<LibvirtVMDef.DiskDef> disksDefs, KVMStoragePool pool) {
-        List<KVMPhysicalDisk> disks = new ArrayList<>();
-        for (LibvirtVMDef.DiskDef diskDef : disksDefs) {
-            KVMPhysicalDisk physicalDisk = pool.getPhysicalDisk(diskDef.getDiskPath());
-            disks.add(physicalDisk);
-        }
-        return disks;
-    }
-
-    protected List<KVMPhysicalDisk> 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);
-        logger.info(msg);
-        pool.refresh();
-        List<KVMPhysicalDisk> 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);
-            logger.error(msg);
-            throw new CloudRuntimeException(msg);
-        }
-        return disksWithPrefix;
-    }
-
-    private void cleanupDisksAndDomainFromTemporaryLocation(List<KVMPhysicalDisk> disks,
-                                                            KVMStoragePool temporaryStoragePool,
-                                                            String temporaryConvertUuid) {
-        for (KVMPhysicalDisk disk : disks) {
-            logger.info(String.format("Cleaning up temporary disk %s after conversion from temporary location", disk.getName()));
-            temporaryStoragePool.deletePhysicalDisk(disk.getName(), Storage.ImageFormat.QCOW2);
-        }
-        logger.info(String.format("Cleaning up temporary domain %s after conversion from temporary location", temporaryConvertUuid));
-        FileUtil.deleteFiles(temporaryStoragePool.getLocalPath(), temporaryConvertUuid, ".xml");
-    }
-
-    protected void sanitizeDisksPath(List<LibvirtVMDef.DiskDef> disks) {
-        for (LibvirtVMDef.DiskDef disk : disks) {
-            String[] diskPathParts = disk.getDiskPath().split("/");
-            String relativePath = diskPathParts[diskPathParts.length - 1];
-            disk.setDiskPath(relativePath);
-        }
-    }
-
-    protected List<KVMPhysicalDisk> moveTemporaryDisksToDestination(List<KVMPhysicalDisk> temporaryDisks,
-                                                                    List<String> destinationStoragePools,
-                                                                    KVMStoragePoolManager storagePoolMgr) {
-        List<KVMPhysicalDisk> 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());
-            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);
-                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);
-                logger.error(err);
-                continue;
-            }
-            KVMPhysicalDisk sourceDisk = temporaryDisks.get(i);
-            if (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());
-                logger.debug(msg);
-            }
-
-            String destinationName = UUID.randomUUID().toString();
-
-            KVMPhysicalDisk destinationDisk = storagePoolMgr.copyPhysicalDisk(sourceDisk, destinationName, destinationPool, 7200 * 1000);
-            targetDisks.add(destinationDisk);
-        }
-        return targetDisks;
-    }
-
-    private UnmanagedInstanceTO getConvertedUnmanagedInstance(String baseName,
-                                                              List<KVMPhysicalDisk> vmDisks,
-                                                              LibvirtDomainXMLParser xmlParser) {
-        UnmanagedInstanceTO instanceTO = new UnmanagedInstanceTO();
-        instanceTO.setName(baseName);
-        instanceTO.setDisks(getUnmanagedInstanceDisks(vmDisks, xmlParser));
-        instanceTO.setNics(getUnmanagedInstanceNics(xmlParser));
-        return instanceTO;
-    }
-
-    private List<UnmanagedInstanceTO.Nic> getUnmanagedInstanceNics(LibvirtDomainXMLParser xmlParser) {
-        List<UnmanagedInstanceTO.Nic> nics = new ArrayList<>();
-        if (xmlParser != null) {
-            List<LibvirtVMDef.InterfaceDef> interfaces = xmlParser.getInterfaces();
-            for (LibvirtVMDef.InterfaceDef interfaceDef : interfaces) {
-                UnmanagedInstanceTO.Nic nic = new UnmanagedInstanceTO.Nic();
-                nic.setMacAddress(interfaceDef.getMacAddress());
-                nic.setNicId(interfaceDef.getBrName());
-                nic.setAdapterType(interfaceDef.getModel().toString());
-                nics.add(nic);
-            }
-        }
-        return nics;
-    }
-
-    protected List<UnmanagedInstanceTO.Disk> getUnmanagedInstanceDisks(List<KVMPhysicalDisk> vmDisks, LibvirtDomainXMLParser xmlParser) {
-        List<UnmanagedInstanceTO.Disk> instanceDisks = new ArrayList<>();
-        List<LibvirtVMDef.DiskDef> 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<String, String> 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<String, String> getNfsStoragePoolHostAndPath(KVMStoragePool storagePool) {
-        String sourceHostIp = null;
-        String sourcePath = null;
-        List<String[]> 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();
-        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);
-    }
-
-    protected LibvirtDomainXMLParser parseMigratedVMXmlDomain(String installPath) throws IOException {
-        String xmlPath = String.format("%s.xml", installPath);
-        if (!new File(xmlPath).exists()) {
-            String err = String.format("Conversion failed. Unable to find the converted XML domain, expected %s", xmlPath);
-            logger.error(err);
-            throw new CloudRuntimeException(err);
-        }
-        InputStream is = new BufferedInputStream(new FileInputStream(xmlPath));
-        String xml = IOUtils.toString(is, Charset.defaultCharset());
-        final LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
-        try {
-            parser.parseDomainXML(xml);
-            return parser;
-        } catch (RuntimeException e) {
-            String err = String.format("Error parsing the converted instance XML domain at %s: %s", xmlPath, e.getMessage());
-            logger.error(err, e);
-            logger.debug(xml);
-            return null;
-        }
-    }
 }

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapperTest.java

@@ -127,7 +127,7 @@ public class LibvirtImportConvertedInstanceCommandWrapperTest {
         Mockito.when(convertedDisk1.getName()).thenReturn("disk1");
         Mockito.when(temporaryPool.getPhysicalDisk(relativePath)).thenReturn(convertedDisk1);
 
-        List<KVMPhysicalDisk> disks = importInstanceCommandWrapper.getTemporaryDisksFromParsedXml(temporaryPool, parser, "");
+        List<KVMPhysicalDisk> disks = importInstanceCommandWrapper.getTemporaryDisksFromParsedXml(temporaryPool, parser, "", "", "prefix");
         Mockito.verify(importInstanceCommandWrapper).sanitizeDisksPath(List.of(diskDef));
         Assert.assertEquals(1, disks.size());
         Assert.assertEquals("disk1", disks.get(0).getName());

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -23,6 +23,7 @@ import com.cloud.agent.api.CheckConvertInstanceAnswer;
 import com.cloud.agent.api.CheckConvertInstanceCommand;
 import com.cloud.agent.api.CheckVolumeAnswer;
 import com.cloud.agent.api.CheckVolumeCommand;
+import com.cloud.agent.api.CleanupConvertedInstanceDisksCommand;
 import com.cloud.agent.api.ConvertInstanceAnswer;
 import com.cloud.agent.api.ConvertInstanceCommand;
 import com.cloud.agent.api.CopyRemoteVolumeAnswer;
@@ -1717,13 +1718,14 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
             logger.debug("The host {}  is selected to execute the conversion of the " +
                     "instance {} from VMware to KVM ", convertHost, sourceVMName);
 
+            long importStartTime = System.currentTimeMillis();
+            importVMTask = importVmTasksManager.createImportVMTaskRecord(zone, owner, userId, displayName, vcenter, datacenterName, sourceVMName,
+                    convertHost, importHost);
+
             temporaryConvertLocation = selectInstanceConversionTemporaryLocation(
                     destinationCluster, convertHost, importHost, convertStoragePoolId, forceConvertToPool);
             List<StoragePoolVO> convertStoragePools = findInstanceConversionDestinationStoragePoolsInCluster(destinationCluster, serviceOffering, dataDiskOfferingMap, temporaryConvertLocation, forceConvertToPool);
 
-            long importStartTime = System.currentTimeMillis();
-            importVMTask = importVmTasksManager.createImportVMTaskRecord(zone, owner, userId, displayName, vcenter, datacenterName, sourceVMName,
-                    convertHost, importHost);
             importVmTasksManager.updateImportVMTaskStep(importVMTask, zone, owner, convertHost, importHost, null, CloningInstance);
 
             // sourceVMwareInstance could be a cloned instance from sourceVMName, of the sourceVMName itself if its powered off.
@@ -2214,31 +2216,56 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
             throw new CloudRuntimeException(err);
         }
 
+        boolean cleanupConvertedDisks = false;
+        String convertedDisksPrefix = null;
         Answer importAnswer;
         try {
+            convertedDisksPrefix = ((ConvertInstanceAnswer)convertAnswer).getTemporaryConvertUuid();
             ImportConvertedInstanceCommand importCmd = new ImportConvertedInstanceCommand(
                     remoteInstanceTO, destinationStoragePools, temporaryConvertLocation,
-                    ((ConvertInstanceAnswer)convertAnswer).getTemporaryConvertUuid(), forceConvertToPool);
+                    convertedDisksPrefix, forceConvertToPool);
             importAnswer = agentManager.send(importHost.getId(), importCmd);
+
+            if (!importAnswer.getResult()) {
+                cleanupConvertedDisks = true;
+                String err = String.format(
+                        "The import process failed for instance %s from VMware to KVM on host %s: %s",
+                        sourceVM, importHost, importAnswer.getDetails());
+                logger.error(err);
+                throw new CloudRuntimeException(err);
+            }
         } catch (AgentUnavailableException | OperationTimedoutException e) {
+            cleanupConvertedDisks = true;
             String err = String.format(
                     "Could not send the import converted instance command to host %s due to: %s",
                     importHost, e.getMessage());
             logger.error(err, e);
             throw new CloudRuntimeException(err);
-        }
-
-        if (!importAnswer.getResult()) {
-            String err = String.format(
-                    "The import process failed for instance %s from VMware to KVM on host %s: %s",
-                    sourceVM, importHost, importAnswer.getDetails());
-            logger.error(err);
-            throw new CloudRuntimeException(err);
+        } finally {
+            if (cleanupConvertedDisks) {
+                cleanupConvertedDisks(sourceVM, convertHost, temporaryConvertLocation, convertedDisksPrefix);
+            }
         }
 
         return ((ImportConvertedInstanceAnswer) importAnswer).getConvertedInstance();
     }
 
+    private void cleanupConvertedDisks(String sourceVM, HostVO convertHost, DataStoreTO temporaryConvertLocation, String convertedDisksPrefix) {
+        logger.debug("Cleaning up the converted disks for the VM {} through the conversion host {}", sourceVM, convertHost.getName());
+        CleanupConvertedInstanceDisksCommand cleanupCommand =
+                new CleanupConvertedInstanceDisksCommand(temporaryConvertLocation, convertedDisksPrefix);
+        try {
+            Answer cleanupAnswer = agentManager.send(convertHost.getId(), cleanupCommand);
+            if (!cleanupAnswer.getResult()) {
+                logger.warn("Failed to cleanup the converted disks for the VM {} through " +
+                        "the conversion host {}: {}", sourceVM, convertHost.getName(), cleanupAnswer.getDetails());
+            }
+        } catch (AgentUnavailableException | OperationTimedoutException e) {
+            logger.error("Error cleaning up converted disks for VM {} through the conversion host {}",
+                    sourceVM, convertHost.getName(), e);
+        }
+    }
+
     private List<StoragePoolVO> findInstanceConversionDestinationStoragePoolsInCluster(
             Cluster destinationCluster, ServiceOfferingVO serviceOffering,
             Map<String, Long> dataDiskOfferingMap,