diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index 71e8c0afc8f..fa52933cebe 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/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; - if (isNonManagedNfsToNfsOrSharedMountPointToNfs) { + boolean isNonManagedToNfs = supportStoragePoolType(sourceStoragePool.getPoolType(), StoragePoolType.Filesystem) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination; + 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 volumeDataStoreMap, Host destHost) { + protected void verifyLiveMigrationForKVM(Map volumeDataStoreMap) { Boolean storageTypeConsistency = null; - Map sourcePools = new HashMap<>(); for (Map.Entry 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 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 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()); - } } } diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index 8f1ada87458..c8a77a42252 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/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 diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java index ea1a221c8c0..e619b40fae0 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java +++ b/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 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 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); - } }