[VMware to KVM Migration] Fix for converted instance npe issue when source vmware instance ovf is exported from management server (#11003)

Author: sureshanaparti

api/src/main/java/com/cloud/storage/VolumeApiService.java

@@ -171,6 +171,13 @@ Volume updateVolume(long volumeId, String path, String state, Long storageId,
      *   </table>
      */
     boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOffering diskOffering);
+
+    /**
+     * Checks if the storage pool supports the required disk offering tags
+     * destPool the storage pool to check the disk offering tags
+     * diskOfferingTags the tags that should be supported
+     * return whether the tags are supported in the storage pool
+     */
     boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags);
 
     Volume destroyVolume(long volumeId, Account caller, boolean expunge, boolean forceExpunge);

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

@@ -16,25 +16,13 @@
 // under the License.
 package com.cloud.agent.api;
 
-import org.apache.cloudstack.vm.UnmanagedInstanceTO;
-
 public class ConvertInstanceAnswer extends Answer {
 
     private String temporaryConvertUuid;
 
     public ConvertInstanceAnswer() {
         super();
     }
-    private UnmanagedInstanceTO convertedInstance;
-
-    public ConvertInstanceAnswer(Command command, boolean success, String details) {
-        super(command, success, details);
-    }
-
-    public ConvertInstanceAnswer(Command command, UnmanagedInstanceTO convertedInstance) {
-        super(command, true, "");
-        this.convertedInstance = convertedInstance;
-    }
 
     public ConvertInstanceAnswer(Command command, String temporaryConvertUuid) {
         super(command, true, "");
@@ -44,8 +32,4 @@ public ConvertInstanceAnswer(Command command, String temporaryConvertUuid) {
     public String getTemporaryConvertUuid() {
         return temporaryConvertUuid;
     }
-
-    public UnmanagedInstanceTO getConvertedInstance() {
-        return convertedInstance;
-    }
 }

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

@@ -20,13 +20,10 @@
 import com.cloud.agent.api.to.RemoteInstanceTO;
 import com.cloud.hypervisor.Hypervisor;
 
-import java.util.List;
-
 public class ConvertInstanceCommand extends Command {
 
     private RemoteInstanceTO sourceInstance;
     private Hypervisor.HypervisorType destinationHypervisorType;
-    private List<String> destinationStoragePools;
     private DataStoreTO conversionTemporaryLocation;
     private String templateDirOnConversionLocation;
     private boolean checkConversionSupport;
@@ -36,12 +33,10 @@ public class ConvertInstanceCommand extends Command {
     public ConvertInstanceCommand() {
     }
 
-    public ConvertInstanceCommand(RemoteInstanceTO sourceInstance, Hypervisor.HypervisorType destinationHypervisorType,
-                                  List<String> destinationStoragePools, DataStoreTO conversionTemporaryLocation,
+    public ConvertInstanceCommand(RemoteInstanceTO sourceInstance, Hypervisor.HypervisorType destinationHypervisorType, DataStoreTO conversionTemporaryLocation,
                                   String templateDirOnConversionLocation, boolean checkConversionSupport, boolean exportOvfToConversionLocation) {
         this.sourceInstance = sourceInstance;
         this.destinationHypervisorType = destinationHypervisorType;
-        this.destinationStoragePools = destinationStoragePools;
         this.conversionTemporaryLocation = conversionTemporaryLocation;
         this.templateDirOnConversionLocation = templateDirOnConversionLocation;
         this.checkConversionSupport = checkConversionSupport;
@@ -56,10 +51,6 @@ public Hypervisor.HypervisorType getDestinationHypervisorType() {
         return destinationHypervisorType;
     }
 
-    public List<String> getDestinationStoragePools() {
-        return destinationStoragePools;
-    }
-
     public DataStoreTO getConversionTemporaryLocation() {
         return conversionTemporaryLocation;
     }

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

@@ -18,22 +18,12 @@
 //
 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.net.URLEncoder;
 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;
@@ -44,17 +34,12 @@
 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.OutputInterpreter;
 import com.cloud.utils.script.Script;
 
@@ -77,15 +62,15 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve
             String msg = String.format("Cannot convert the instance %s from VMware as the virt-v2v binary is not found. " +
                     "Please install virt-v2v%s on the host before attempting the instance conversion.", sourceInstanceName, serverResource.isUbuntuHost()? ", nbdkit" : "");
             logger.info(msg);
-            return new ConvertInstanceAnswer(cmd, false, msg);
+            return new Answer(cmd, false, msg);
         }
 
         if (!areSourceAndDestinationHypervisorsSupported(sourceHypervisorType, destinationHypervisorType)) {
             String err = destinationHypervisorType != Hypervisor.HypervisorType.KVM ?
                     String.format("The destination hypervisor type is %s, KVM was expected, cannot handle it", destinationHypervisorType) :
                     String.format("The source hypervisor type %s is not supported for KVM conversion", sourceHypervisorType);
             logger.error(err);
-            return new ConvertInstanceAnswer(cmd, false, err);
+            return new Answer(cmd, false, err);
         }
 
         final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
@@ -103,7 +88,7 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve
             if (StringUtils.isBlank(exportInstanceOVAUrl)) {
                 String err = String.format("Couldn't export OVA for the VM %s, due to empty url", sourceInstanceName);
                 logger.error(err);
-                return new ConvertInstanceAnswer(cmd, false, err);
+                return new Answer(cmd, false, err);
             }
 
             int noOfThreads = cmd.getThreadsCountToExportOvf();
@@ -117,7 +102,7 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve
             if (!ovfExported) {
                 String err = String.format("Export OVA for the VM %s failed", sourceInstanceName);
                 logger.error(err);
-                return new ConvertInstanceAnswer(cmd, false, err);
+                return new Answer(cmd, false, err);
             }
             sourceOVFDirPath = String.format("%s%s/", sourceOVFDirPath, sourceInstanceName);
         } else {
@@ -140,15 +125,15 @@ public Answer execute(ConvertInstanceCommand cmd, LibvirtComputingResource serve
                                 "has a different virt-v2v version.",
                         ovfTemplateDirOnConversionLocation);
                 logger.error(err);
-                return new ConvertInstanceAnswer(cmd, false, err);
+                return new Answer(cmd, false, err);
             }
             return new ConvertInstanceAnswer(cmd, temporaryConvertUuid);
         } catch (Exception e) {
             String error = String.format("Error converting instance %s from %s, due to: %s",
                     sourceInstanceName, sourceHypervisorType, e.getMessage());
             logger.error(error, e);
             cleanupSecondaryStorage = true;
-            return new ConvertInstanceAnswer(cmd, false, error);
+            return new Answer(cmd, false, error);
         } finally {
             if (ovfExported && StringUtils.isNotBlank(ovfTemplateDirOnConversionLocation)) {
                 String sourceOVFDir = String.format("%s/%s", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
@@ -205,55 +190,6 @@ private String getExportOVAUrlFromRemoteInstance(RemoteInstanceTO vmwareInstance
                 encodedUsername, encodedPassword, vcenter, datacenter, vm);
     }
 
-    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("/");
@@ -262,114 +198,6 @@ protected void sanitizeDisksPath(List<LibvirtVMDef.DiskDef> disks) {
         }
     }
 
-    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);
-    }
-
     private boolean exportOVAFromVMOnVcenter(String vmExportUrl,
                                              String targetOvfDir,
                                              int noOfThreads,
@@ -412,27 +240,6 @@ protected boolean performInstanceConversion(String sourceOVFDirPath,
         return exitValue == 0;
     }
 
-    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;
-        }
-    }
-
     protected String encodeUsername(String username) {
         return URLEncoder.encode(username, Charset.defaultCharset());
     }