CloudStack fails to migrate VM with volume when there are datadisks attatched (#5410)

* Check if should map volume in createStoragePoolMappingsForVolumes

* Invert conditional at internalCanHandle

Author: GabrielBrascher

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -2947,8 +2947,8 @@ protected void executeManagedStorageChecksWhenTargetStoragePoolProvided(StorageP
      * For each one of the volumes we will map it to a storage pool that is available via the target host.
      * An exception is thrown if we cannot find a storage pool that is accessible in the target host to migrate the volume to.
      */
-    protected void createStoragePoolMappingsForVolumes(VirtualMachineProfile profile, DataCenterDeployment plan, Map<Volume, StoragePool> volumeToPoolObjectMap, List<Volume> allVolumes) {
-        for (Volume volume : allVolumes) {
+    protected void createStoragePoolMappingsForVolumes(VirtualMachineProfile profile, DataCenterDeployment plan, Map<Volume, StoragePool> volumeToPoolObjectMap, List<Volume> volumesNotMapped) {
+        for (Volume volume : volumesNotMapped) {
             StoragePoolVO currentPool = _storagePoolDao.findById(volume.getPoolId());
 
             Host targetHost = null;
@@ -2958,12 +2958,25 @@ protected void createStoragePoolMappingsForVolumes(VirtualMachineProfile profile
             executeManagedStorageChecksWhenTargetStoragePoolNotProvided(targetHost, currentPool, volume);
             if (ScopeType.HOST.equals(currentPool.getScope()) || isStorageCrossClusterMigration(plan.getClusterId(), currentPool)) {
                 createVolumeToStoragePoolMappingIfPossible(profile, plan, volumeToPoolObjectMap, volume, currentPool);
-            } else {
+            } else if (shouldMapVolume(profile, volume, currentPool)){
                 volumeToPoolObjectMap.put(volume, currentPool);
             }
         }
     }
 
+    /**
+     * Returns true if it should map the volume for a storage pool to migrate.
+     * <br><br>
+     * Some context: VMware migration workflow requires all volumes to be mapped (even if volume stays on its current pool);
+     *  however, this is not necessary/desirable for the KVM flow.
+     */
+    protected boolean shouldMapVolume(VirtualMachineProfile profile, Volume volume, StoragePoolVO currentPool) {
+        boolean isManaged = currentPool.isManaged();
+        boolean isNotKvm = HypervisorType.KVM != profile.getHypervisorType();
+        boolean isNotDatadisk = Type.DATADISK != volume.getVolumeType();
+        return isNotKvm || isNotDatadisk || isManaged;
+    }
+
     /**
      *  Executes the managed storage checks for the volumes that the user has not entered a mapping of <volume, storage pool>. The following checks are performed.
      *   <ul>

engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java

@@ -82,21 +82,19 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot
      */
     @Override
     protected StrategyPriority internalCanHandle(Map<VolumeInfo, DataStore> volumeMap, Host srcHost, Host destHost) {
-        if (super.internalCanHandle(volumeMap, srcHost, destHost) == StrategyPriority.CANT_HANDLE) {
-            if (canHandleKVMNonManagedLiveNFSStorageMigration(volumeMap, srcHost, destHost) == StrategyPriority.CANT_HANDLE) {
-                Set<VolumeInfo> volumeInfoSet = volumeMap.keySet();
-
-                for (VolumeInfo volumeInfo : volumeInfoSet) {
-                    StoragePoolVO storagePoolVO = _storagePoolDao.findById(volumeInfo.getPoolId());
+        if (super.internalCanHandle(volumeMap, srcHost, destHost) != StrategyPriority.CANT_HANDLE
+                || canHandleKVMNonManagedLiveNFSStorageMigration(volumeMap, srcHost, destHost) != StrategyPriority.CANT_HANDLE) {
+            return StrategyPriority.CANT_HANDLE;
+        }
 
-                    if (!supportStoragePoolType(storagePoolVO.getPoolType())) {
-                        return StrategyPriority.CANT_HANDLE;
-                    }
-                }
+        Set<VolumeInfo> volumeInfoSet = volumeMap.keySet();
+        for (VolumeInfo volumeInfo : volumeInfoSet) {
+            StoragePoolVO storagePoolVO = _storagePoolDao.findById(volumeInfo.getPoolId());
+            if (!supportStoragePoolType(storagePoolVO.getPoolType())) {
+                return StrategyPriority.CANT_HANDLE;
             }
-            return StrategyPriority.HYPERVISOR;
         }
-        return StrategyPriority.CANT_HANDLE;
+        return StrategyPriority.HYPERVISOR;
     }
 
     /**