diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index d6325b43134..e39fac147e7 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2999,6 +2999,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic throw new InvalidParameterValueException("Volume cannot be migrated, please remove all VM snapshots for VM to which this volume is attached"); } + StoragePoolVO srcStoragePoolVO = _storagePoolDao.findById(vol.getPoolId()); + // OfflineVmwareMigration: extract this block as method and check if it is subject to regression if (vm != null && State.Running.equals(vm.getState())) { // Check if the VM is GPU enabled. @@ -3020,16 +3022,15 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic liveMigrateVolume = capabilities.isStorageMotionSupported(); } - StoragePoolVO storagePoolVO = _storagePoolDao.findById(vol.getPoolId()); - if (liveMigrateVolume && HypervisorType.KVM.equals(host.getHypervisorType()) && !storagePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { + if (liveMigrateVolume && HypervisorType.KVM.equals(host.getHypervisorType()) && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { StoragePoolVO destinationStoragePoolVo = _storagePoolDao.findById(storagePoolId); - if (isSourceOrDestNotOnStorPool(storagePoolVO, destinationStoragePoolVo)) { + if (isSourceOrDestNotOnStorPool(srcStoragePoolVO, destinationStoragePoolVo)) { throw new InvalidParameterValueException("KVM does not support volume live migration due to the limited possibility to refresh VM XML domain. " + "Therefore, to live migrate a volume between storage pools, one must migrate the VM to a different host as well to force the VM XML domain update. " + "Use 'migrateVirtualMachineWithVolumes' instead."); } - srcAndDestOnStorPool = isSourceAndDestOnStorPool(storagePoolVO, destinationStoragePoolVo); + srcAndDestOnStorPool = isSourceAndDestOnStorPool(srcStoragePoolVO, destinationStoragePoolVo); } } @@ -3043,7 +3044,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic } } - if (vol.getPassphraseId() != null && !srcAndDestOnStorPool) { + if (vol.getPassphraseId() != null && !srcAndDestOnStorPool && !srcStoragePoolVO.getPoolType().equals(Storage.StoragePoolType.PowerFlex)) { throw new InvalidParameterValueException("Migration of encrypted volumes is unsupported"); } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index e5b85c829e4..35b69cf82e4 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -17,6 +17,7 @@ package com.cloud.storage; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyObject; @@ -40,8 +41,11 @@ import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd; import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd; +import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; @@ -52,6 +56,7 @@ import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.dao.AsyncJobJoinMapDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; +import org.apache.cloudstack.snapshot.SnapshotHelper; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -114,6 +119,7 @@ import com.cloud.user.ResourceLimitService; import com.cloud.user.User; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.UserVmManager; @@ -169,6 +175,8 @@ public class VolumeApiServiceImplTest { @Mock private CreateVolumeCmd createVol; @Mock + private MigrateVolumeCmd migrateVolumeCmd; + @Mock private UserVmManager userVmManager; @Mock private DataCenterDao _dcDao; @@ -188,6 +196,10 @@ public class VolumeApiServiceImplTest { private ServiceOfferingDao serviceOfferingDao; @Mock private DiskOfferingDao _diskOfferingDao; + @Mock + private DataStoreManager dataStoreMgr; + @Mock + private SnapshotHelper snapshotHelper; private DetachVolumeCmd detachCmd = new DetachVolumeCmd(); private Class _detachCmdClass = detachCmd.getClass(); @@ -218,6 +230,9 @@ public class VolumeApiServiceImplTest { @Mock private ProjectManager projectManagerMock; + @Mock + private StorageManager storageMgr; + private long accountMockId = 456l; private long volumeMockId = 12313l; private long vmInstanceMockId = 1123l; @@ -1579,4 +1594,41 @@ public class VolumeApiServiceImplTest { Mockito.when(host.getHypervisorType()).thenReturn(HypervisorType.KVM); Assert.assertFalse(volumeApiServiceImpl.isSendCommandForVmVolumeAttachDetach(host, Mockito.mock(StoragePoolVO.class))); } + + // Below test covers both allowing encrypted volume migration for PowerFlex storage and expect error on storagepool compatibility + @Test + public void testStoragePoolCompatibilityAndAllowEncryptedVolumeMigrationForPowerFlexStorage() { + try { + Mockito.when(migrateVolumeCmd.getVolumeId()).thenReturn(1L); + Mockito.when(migrateVolumeCmd.getStoragePoolId()).thenReturn(2L); + VolumeVO vol = Mockito.mock(VolumeVO.class); + Mockito.when(volumeDaoMock.findById(1L)).thenReturn(vol); + Mockito.when(volumeDaoMock.getHypervisorType(1L)).thenReturn(HypervisorType.KVM); + Mockito.when(vol.getState()).thenReturn(Volume.State.Ready); + Mockito.when(vol.getPoolId()).thenReturn(1L); + Mockito.when(vol.getInstanceId()).thenReturn(null); + Mockito.when(vol.getDiskOfferingId()).thenReturn(1L); + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(_diskOfferingDao.findById(1L)).thenReturn(diskOffering); + + StoragePoolVO srcStoragePoolVOMock = Mockito.mock(StoragePoolVO.class); + StoragePool destPool = Mockito.mock(StoragePool.class); + PrimaryDataStore dataStore = Mockito.mock(PrimaryDataStore.class); + + Mockito.when(vol.getPassphraseId()).thenReturn(1L); + Mockito.when(primaryDataStoreDaoMock.findById(1L)).thenReturn(srcStoragePoolVOMock); + Mockito.when(srcStoragePoolVOMock.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.when(dataStoreMgr.getDataStore(2L, DataStoreRole.Primary)).thenReturn( dataStore); + Mockito.doNothing().when(snapshotHelper).checkKvmVolumeSnapshotsOnlyInPrimaryStorage(vol, HypervisorType.KVM); + Mockito.when(destPool.getUuid()).thenReturn("bd525970-3d2a-4230-880d-261892129ef3"); + + Mockito.when(storageMgr.storagePoolCompatibleWithVolumePool(destPool, vol)).thenReturn(false); + + volumeApiServiceImpl.migrateVolume(migrateVolumeCmd); + } catch (InvalidParameterValueException e) { + fail("Unexpected InvalidParameterValueException was thrown"); + } catch (CloudRuntimeException e) { + // test passed + } + } }