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

Author: sureshanaparti

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

@@ -169,6 +169,14 @@ Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, Account acc
      *      </body>
      *   </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;
-    }
-}
+}

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

@@ -18,11 +18,11 @@
 //
 package com.cloud.hypervisor.kvm.resource.wrapper;
 
-
 import java.net.URLEncoder;
 import java.nio.charset.Charset;
 import java.util.List;
 import java.util.UUID;
+
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.commons.lang3.StringUtils;
 
@@ -62,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();
@@ -88,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();
@@ -102,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 {
@@ -124,14 +124,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);
-            return new ConvertInstanceAnswer(cmd, false, error);
+            cleanupSecondaryStorage = true;
+            return new Answer(cmd, false, error);
         } finally {
             if (ovfExported && StringUtils.isNotBlank(ovfTemplateDirOnConversionLocation)) {
                 String sourceOVFDir = String.format("%s/%s", temporaryConvertPath, ovfTemplateDirOnConversionLocation);

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

@@ -2091,39 +2091,8 @@ private List<StoragePoolVO> findInstanceConversionStoragePoolsInCluster(
                 logger.error(msg);
                 throw new CloudRuntimeException(msg);
             }
-            diskOfferingTags.add(diskOffering.getTags());
-        }
-        if (serviceOffering.getDiskOfferingId() != null) {
-            DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
-            if (diskOffering != null) {
-                diskOfferingTags.add(diskOffering.getTags());
-            }
-        }
-
-        pools = getPoolsWithMatchingTags(pools, diskOfferingTags);
-        if (pools.isEmpty()) {
-            String msg = String.format("Cannot find suitable storage pools in cluster %s for the conversion", destinationCluster);
-            logger.error(msg);
-            throw new CloudRuntimeException(msg);
-        }
-        return pools;
-    }
-
-    private List<StoragePoolVO> getPoolsWithMatchingTags(List<StoragePoolVO> pools, List<String> diskOfferingTags) {
-        if (diskOfferingTags.isEmpty()) {
-            return pools;
-        }
-        List<StoragePoolVO> poolsSupportingTags = new ArrayList<>(pools);
-        for (String tags : diskOfferingTags) {
-            boolean tagsMatched = false;
-            for (StoragePoolVO pool : pools) {
-                if (volumeApiService.doesTargetStorageSupportDiskOffering(pool, tags)) {
-                    poolsSupportingTags.add(pool);
-                    tagsMatched = true;
-                }
-            }
-            if (!tagsMatched) {
-                String msg = String.format("Cannot find suitable storage pools for the conversion with disk offering tags %s", tags);
+            if (getStoragePoolWithTags(pools, diskOffering.getTags()) == null) {
+                String msg = String.format("Cannot find suitable storage pool for disk offering %s", diskOffering.getName());
                 logger.error(msg);
                 throw new CloudRuntimeException(msg);
             }

server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java

@@ -704,11 +704,10 @@ private void baseTestImportVmFromVmwareToKvm(VcenterParameter vcenterParameter,
             when(agentManager.send(Mockito.eq(convertHostId), Mockito.any(CheckConvertInstanceCommand.class))).thenReturn(checkConvertInstanceAnswer);
         }
 
-        when(volumeApiService.doesStoragePoolSupportDiskOfferingTags(any(StoragePool.class), any())).thenReturn(true);
+        when(volumeApiService.doesStoragePoolSupportDiskOffering(any(StoragePool.class), any(DiskOffering.class))).thenReturn(true);
 
         ConvertInstanceAnswer convertInstanceAnswer = mock(ConvertInstanceAnswer.class);
         ImportConvertedInstanceAnswer convertImportedInstanceAnswer = mock(ImportConvertedInstanceAnswer.class);
-        when(convertInstanceAnswer.getConvertedInstance()).thenReturn(instance);
         when(convertInstanceAnswer.getResult()).thenReturn(vcenterParameter != VcenterParameter.CONVERT_FAILURE);
         Mockito.lenient().when(convertImportedInstanceAnswer.getConvertedInstance()).thenReturn(instance);
         Mockito.lenient().when(convertImportedInstanceAnswer.getResult()).thenReturn(vcenterParameter != VcenterParameter.CONVERT_FAILURE);