Fixup

Author: vishesh92

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

@@ -0,0 +1,37 @@
+// 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 {
+
+    private String temporaryConvertUuid;
+
+    public ConvertInstanceAnswer() {
+        super();
+    }
+
+    public ConvertInstanceAnswer(Command command, String temporaryConvertUuid) {
+        super(command, true, "");
+        this.temporaryConvertUuid = temporaryConvertUuid;
+    }
+
+    public String getTemporaryConvertUuid() {
+        return temporaryConvertUuid;
+    }
+}

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

@@ -26,15 +26,18 @@ public class ImportConvertedInstanceCommand extends Command {
     private RemoteInstanceTO sourceInstance;
     private List<String> destinationStoragePools;
     private DataStoreTO conversionTemporaryLocation;
+    private String temporaryConvertUuid;
 
     public ImportConvertedInstanceCommand() {
     }
 
     public ImportConvertedInstanceCommand(RemoteInstanceTO sourceInstance,
-                                          List<String> destinationStoragePools, DataStoreTO conversionTemporaryLocation) {
+                                          List<String> 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;

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

@@ -25,6 +25,7 @@
 import java.util.UUID;
 import java.util.stream.Collectors;
 
+import com.cloud.agent.api.ConvertInstanceAnswer;
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.cloudstack.vm.UnmanagedInstanceTO;
 import org.apache.commons.collections.CollectionUtils;
@@ -122,6 +123,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,18 +133,23 @@ 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 ConvertInstanceAnswer(cmd, temporaryConvertUuid);
         } 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)) {
                 String sourceOVFDir = String.format("%s/%s", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
                 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 +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);
-            s_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);
-        s_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);
-            s_logger.error(msg);
-            throw new CloudRuntimeException(msg);
-        }
-        return disksWithPrefix;
-    }
-
     protected void sanitizeDisksPath(List<LibvirtVMDef.DiskDef> disks) {
         for (LibvirtVMDef.DiskDef disk : disks) {
             String[] diskPathParts = disk.getDiskPath().split("/");
@@ -229,89 +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());
-            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<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();
-        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,

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<String> 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);