Fix migration from local storage to NFS in KVM (#8909)

* fix migration from local to nfs

* remove unused imports

* remove dead code

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

@@ -70,7 +70,6 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
-import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.log4j.Logger;
 
@@ -82,7 +81,6 @@ import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo;
 import com.cloud.agent.api.ModifyTargetsAnswer;
 import com.cloud.agent.api.ModifyTargetsCommand;
 import com.cloud.agent.api.PrepareForMigrationCommand;
-import com.cloud.agent.api.storage.CheckStorageAvailabilityCommand;
 import com.cloud.agent.api.storage.CopyVolumeAnswer;
 import com.cloud.agent.api.storage.CopyVolumeCommand;
 import com.cloud.agent.api.storage.MigrateVolumeAnswer;
@@ -1909,7 +1907,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
                 throw new CloudRuntimeException("Invalid hypervisor type (only KVM supported for this operation at the time being)");
             }
 
-            verifyLiveMigrationForKVM(volumeDataStoreMap, destHost);
+            verifyLiveMigrationForKVM(volumeDataStoreMap);
 
             VMInstanceVO vmInstance = _vmDao.findById(vmTO.getId());
             vmTO.setState(vmInstance.getState());
@@ -1983,8 +1981,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
 
                 MigrateCommand.MigrateDiskInfo migrateDiskInfo;
 
-                boolean isNonManagedNfsToNfsOrSharedMountPointToNfs = supportStoragePoolType(sourceStoragePool.getPoolType()) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
+                boolean isNonManagedToNfs = supportStoragePoolType(sourceStoragePool.getPoolType(), StoragePoolType.Filesystem) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
-                if (isNonManagedNfsToNfsOrSharedMountPointToNfs) {
+                if (isNonManagedToNfs) {
                     migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(),
                             MigrateCommand.MigrateDiskInfo.DiskType.FILE,
                             MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
@@ -2363,9 +2361,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
     * At a high level: The source storage cannot be managed and
     *                  the destination storages can be all managed or all not managed, not mixed.
     */
-    protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataStoreMap, Host destHost) {
+    protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataStoreMap) {
         Boolean storageTypeConsistency = null;
-        Map<String, Storage.StoragePoolType> sourcePools = new HashMap<>();
         for (Map.Entry<VolumeInfo, DataStore> entry : volumeDataStoreMap.entrySet()) {
             VolumeInfo volumeInfo = entry.getKey();
 
@@ -2392,47 +2389,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
             } else if (storageTypeConsistency != destStoragePoolVO.isManaged()) {
                 throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed");
             }
-
-            addSourcePoolToPoolsMap(sourcePools, srcStoragePoolVO, destStoragePoolVO);
-        }
-        verifyDestinationStorage(sourcePools, destHost);
-    }
-
-    /**
-     * Adds source storage pool to the migration map if the destination pool is not managed and it is NFS.
-     */
-    protected void addSourcePoolToPoolsMap(Map<String, Storage.StoragePoolType> sourcePools, StoragePoolVO srcStoragePoolVO, StoragePoolVO destStoragePoolVO) {
-        if (destStoragePoolVO.isManaged() || !StoragePoolType.NetworkFilesystem.equals(destStoragePoolVO.getPoolType())) {
-            LOGGER.trace(String.format("Skipping adding source pool [%s] to map due to destination pool [%s] is managed or not NFS.", srcStoragePoolVO, destStoragePoolVO));
-            return;
-        }
-
-        String sourceStoragePoolUuid = srcStoragePoolVO.getUuid();
-        if (!sourcePools.containsKey(sourceStoragePoolUuid)) {
-            sourcePools.put(sourceStoragePoolUuid, srcStoragePoolVO.getPoolType());
-        }
-    }
-
-    /**
-     * Perform storage validation on destination host for KVM live storage migrations.
-     * Validate that volume source storage pools are mounted on the destination host prior the migration
-     * @throws CloudRuntimeException if any source storage pool is not mounted on the destination host
-     */
-    private void verifyDestinationStorage(Map<String, Storage.StoragePoolType> sourcePools, Host destHost) {
-        if (MapUtils.isNotEmpty(sourcePools)) {
-            LOGGER.debug("Verifying source pools are already available on destination host " + destHost.getUuid());
-            CheckStorageAvailabilityCommand cmd = new CheckStorageAvailabilityCommand(sourcePools);
-            try {
-                Answer answer = agentManager.send(destHost.getId(), cmd);
-                if (answer == null || !answer.getResult()) {
-                    throw new CloudRuntimeException("Storage verification failed on host "
-                            + destHost.getUuid() +": " + answer.getDetails());
-                }
-            } catch (AgentUnavailableException | OperationTimedoutException e) {
-                e.printStackTrace();
-                throw new CloudRuntimeException("Cannot perform storage verification on host " + destHost.getUuid() +
-                        "due to: " + e.getMessage());
-            }
         }
     }
 

engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java

@@ -476,19 +476,19 @@ public class KvmNonManagedStorageSystemDataMotionTest {
 
     @Test
     public void testVerifyLiveMigrationMapForKVM() {
-        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
+        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
     }
 
     @Test(expected = CloudRuntimeException.class)
     public void testVerifyLiveMigrationMapForKVMNotExistingSource() {
         when(primaryDataStoreDao.findById(POOL_1_ID)).thenReturn(null);
-        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
+        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
     }
 
     @Test(expected = CloudRuntimeException.class)
     public void testVerifyLiveMigrationMapForKVMNotExistingDest() {
         when(primaryDataStoreDao.findById(POOL_2_ID)).thenReturn(null);
-        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
+        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
     }
 
     @Test(expected = CloudRuntimeException.class)
@@ -497,7 +497,7 @@ public class KvmNonManagedStorageSystemDataMotionTest {
         when(pool1.getId()).thenReturn(POOL_1_ID);
         when(pool2.getId()).thenReturn(POOL_2_ID);
         lenient().when(pool2.isManaged()).thenReturn(false);
-        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
+        kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
     }
 
     @Test

engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java

@@ -23,7 +23,6 @@ import static org.junit.Assert.assertFalse;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.MockitoAnnotations.initMocks;
 
 import java.util.HashMap;
@@ -48,7 +47,6 @@ import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.runners.MockitoJUnitRunner;
-import org.mockito.verification.VerificationMode;
 
 import com.cloud.agent.api.MigrateCommand;
 import com.cloud.host.HostVO;
@@ -62,7 +60,6 @@ import com.cloud.storage.VolumeVO;
 import java.util.AbstractMap;
 import java.util.Arrays;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 
@@ -372,72 +369,4 @@ public class StorageSystemDataMotionStrategyTest {
 
         assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes));
     }
-
-    @Test
-    public void validateAddSourcePoolToPoolsMapDestinationPoolIsManaged() {
-        Mockito.doReturn(true).when(destinationStoragePoolVoMock).isManaged();
-        strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-
-        Mockito.verify(destinationStoragePoolVoMock).isManaged();
-        Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-    }
-
-    @Test
-    public void validateAddSourcePoolToPoolsMapDestinationPoolIsNotNFS() {
-        List<StoragePoolType> storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values()));
-        storagePoolTypes.remove(StoragePoolType.NetworkFilesystem);
-
-        Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged();
-        storagePoolTypes.forEach(poolType -> {
-            Mockito.doReturn(poolType).when(destinationStoragePoolVoMock).getPoolType();
-            strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-        });
-
-        VerificationMode times = Mockito.times(storagePoolTypes.size());
-        Mockito.verify(destinationStoragePoolVoMock, times).isManaged();
-        Mockito.verify(destinationStoragePoolVoMock, times).getPoolType();
-        Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-    }
-
-    @Test
-    public void validateAddSourcePoolToPoolsMapMapContainsKey() {
-        Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged();
-        Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType();
-        Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid();
-        Mockito.doReturn(true).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString());
-        strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-
-        Mockito.verify(destinationStoragePoolVoMock, never()).getScope();
-        Mockito.verify(destinationStoragePoolVoMock).isManaged();
-        Mockito.verify(destinationStoragePoolVoMock).getPoolType();
-        Mockito.verify(sourceStoragePoolVoMock).getUuid();
-        Mockito.verify(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString());
-        Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-    }
-
-    @Test
-    public void validateAddSourcePoolToPoolsMapMapDoesNotContainsKey() {
-        List<StoragePoolType> storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values()));
-
-        Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged();
-        Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType();
-        Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid();
-        Mockito.doReturn(false).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString());
-        Mockito.doReturn(null).when(mapStringStoragePoolTypeMock).put(Mockito.anyString(), Mockito.any());
-
-        storagePoolTypes.forEach(poolType -> {
-            Mockito.doReturn(poolType).when(sourceStoragePoolVoMock).getPoolType();
-            strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-        });
-
-        VerificationMode times = Mockito.times(storagePoolTypes.size());
-        Mockito.verify(destinationStoragePoolVoMock, never()).getScope();
-        Mockito.verify(destinationStoragePoolVoMock, times).isManaged();
-        Mockito.verify(destinationStoragePoolVoMock, times).getPoolType();
-        Mockito.verify(sourceStoragePoolVoMock, times).getUuid();
-        Mockito.verify(mapStringStoragePoolTypeMock, times).containsKey(Mockito.anyString());
-        Mockito.verify(sourceStoragePoolVoMock, times).getPoolType();
-        Mockito.verify(mapStringStoragePoolTypeMock, times).put(Mockito.anyString(), Mockito.any());
-        Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
-    }
 }