Skip to content

Commit 3e30283

Browse files
JoaoJandreJoaoJandre
authored
Fix migration from local storage to NFS in KVM (#8909)
* fix migration from local to nfs * remove unused imports * remove dead code
1 parent f944d4c commit 3e30283

3 files changed

Lines changed: 8 additions & 123 deletions

File tree

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

Lines changed: 4 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
7171
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
7272
import org.apache.cloudstack.storage.to.VolumeObjectTO;
73-
import org.apache.commons.collections.MapUtils;
7473
import org.apache.commons.lang3.StringUtils;
7574
import org.apache.log4j.Logger;
7675

@@ -82,7 +81,6 @@
8281
import com.cloud.agent.api.ModifyTargetsAnswer;
8382
import com.cloud.agent.api.ModifyTargetsCommand;
8483
import com.cloud.agent.api.PrepareForMigrationCommand;
85-
import com.cloud.agent.api.storage.CheckStorageAvailabilityCommand;
8684
import com.cloud.agent.api.storage.CopyVolumeAnswer;
8785
import com.cloud.agent.api.storage.CopyVolumeCommand;
8886
import com.cloud.agent.api.storage.MigrateVolumeAnswer;
@@ -1909,7 +1907,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
19091907
throw new CloudRuntimeException("Invalid hypervisor type (only KVM supported for this operation at the time being)");
19101908
}
19111909

1912-
verifyLiveMigrationForKVM(volumeDataStoreMap, destHost);
1910+
verifyLiveMigrationForKVM(volumeDataStoreMap);
19131911

19141912
VMInstanceVO vmInstance = _vmDao.findById(vmTO.getId());
19151913
vmTO.setState(vmInstance.getState());
@@ -1983,8 +1981,8 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
19831981

19841982
MigrateCommand.MigrateDiskInfo migrateDiskInfo;
19851983

1986-
boolean isNonManagedNfsToNfsOrSharedMountPointToNfs = supportStoragePoolType(sourceStoragePool.getPoolType()) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
1987-
if (isNonManagedNfsToNfsOrSharedMountPointToNfs) {
1984+
boolean isNonManagedToNfs = supportStoragePoolType(sourceStoragePool.getPoolType(), StoragePoolType.Filesystem) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
1985+
if (isNonManagedToNfs) {
19881986
migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(),
19891987
MigrateCommand.MigrateDiskInfo.DiskType.FILE,
19901988
MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
@@ -2363,9 +2361,8 @@ protected void prepareDiskWithSecretConsumerDetail(VirtualMachineTO vmTO, Volume
23632361
* At a high level: The source storage cannot be managed and
23642362
* the destination storages can be all managed or all not managed, not mixed.
23652363
*/
2366-
protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataStoreMap, Host destHost) {
2364+
protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataStoreMap) {
23672365
Boolean storageTypeConsistency = null;
2368-
Map<String, Storage.StoragePoolType> sourcePools = new HashMap<>();
23692366
for (Map.Entry<VolumeInfo, DataStore> entry : volumeDataStoreMap.entrySet()) {
23702367
VolumeInfo volumeInfo = entry.getKey();
23712368

@@ -2392,47 +2389,6 @@ protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataSt
23922389
} else if (storageTypeConsistency != destStoragePoolVO.isManaged()) {
23932390
throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed");
23942391
}
2395-
2396-
addSourcePoolToPoolsMap(sourcePools, srcStoragePoolVO, destStoragePoolVO);
2397-
}
2398-
verifyDestinationStorage(sourcePools, destHost);
2399-
}
2400-
2401-
/**
2402-
* Adds source storage pool to the migration map if the destination pool is not managed and it is NFS.
2403-
*/
2404-
protected void addSourcePoolToPoolsMap(Map<String, Storage.StoragePoolType> sourcePools, StoragePoolVO srcStoragePoolVO, StoragePoolVO destStoragePoolVO) {
2405-
if (destStoragePoolVO.isManaged() || !StoragePoolType.NetworkFilesystem.equals(destStoragePoolVO.getPoolType())) {
2406-
LOGGER.trace(String.format("Skipping adding source pool [%s] to map due to destination pool [%s] is managed or not NFS.", srcStoragePoolVO, destStoragePoolVO));
2407-
return;
2408-
}
2409-
2410-
String sourceStoragePoolUuid = srcStoragePoolVO.getUuid();
2411-
if (!sourcePools.containsKey(sourceStoragePoolUuid)) {
2412-
sourcePools.put(sourceStoragePoolUuid, srcStoragePoolVO.getPoolType());
2413-
}
2414-
}
2415-
2416-
/**
2417-
* Perform storage validation on destination host for KVM live storage migrations.
2418-
* Validate that volume source storage pools are mounted on the destination host prior the migration
2419-
* @throws CloudRuntimeException if any source storage pool is not mounted on the destination host
2420-
*/
2421-
private void verifyDestinationStorage(Map<String, Storage.StoragePoolType> sourcePools, Host destHost) {
2422-
if (MapUtils.isNotEmpty(sourcePools)) {
2423-
LOGGER.debug("Verifying source pools are already available on destination host " + destHost.getUuid());
2424-
CheckStorageAvailabilityCommand cmd = new CheckStorageAvailabilityCommand(sourcePools);
2425-
try {
2426-
Answer answer = agentManager.send(destHost.getId(), cmd);
2427-
if (answer == null || !answer.getResult()) {
2428-
throw new CloudRuntimeException("Storage verification failed on host "
2429-
+ destHost.getUuid() +": " + answer.getDetails());
2430-
}
2431-
} catch (AgentUnavailableException | OperationTimedoutException e) {
2432-
e.printStackTrace();
2433-
throw new CloudRuntimeException("Cannot perform storage verification on host " + destHost.getUuid() +
2434-
"due to: " + e.getMessage());
2435-
}
24362392
}
24372393
}
24382394

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,19 +476,19 @@ public void testCanHandleLiveMigrationUnmanagedStorage() {
476476

477477
@Test
478478
public void testVerifyLiveMigrationMapForKVM() {
479-
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
479+
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
480480
}
481481

482482
@Test(expected = CloudRuntimeException.class)
483483
public void testVerifyLiveMigrationMapForKVMNotExistingSource() {
484484
when(primaryDataStoreDao.findById(POOL_1_ID)).thenReturn(null);
485-
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
485+
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
486486
}
487487

488488
@Test(expected = CloudRuntimeException.class)
489489
public void testVerifyLiveMigrationMapForKVMNotExistingDest() {
490490
when(primaryDataStoreDao.findById(POOL_2_ID)).thenReturn(null);
491-
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
491+
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
492492
}
493493

494494
@Test(expected = CloudRuntimeException.class)
@@ -497,7 +497,7 @@ public void testVerifyLiveMigrationMapForKVMMixedManagedUnmagedStorage() {
497497
when(pool1.getId()).thenReturn(POOL_1_ID);
498498
when(pool2.getId()).thenReturn(POOL_2_ID);
499499
lenient().when(pool2.isManaged()).thenReturn(false);
500-
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
500+
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap);
501501
}
502502

503503
@Test

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

Lines changed: 0 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import static org.mockito.Mockito.doReturn;
2424
import static org.mockito.Mockito.lenient;
2525
import static org.mockito.Mockito.mock;
26-
import static org.mockito.Mockito.never;
2726
import static org.mockito.MockitoAnnotations.initMocks;
2827

2928
import java.util.HashMap;
@@ -48,7 +47,6 @@
4847
import org.mockito.Mockito;
4948
import org.mockito.Spy;
5049
import org.mockito.runners.MockitoJUnitRunner;
51-
import org.mockito.verification.VerificationMode;
5250

5351
import com.cloud.agent.api.MigrateCommand;
5452
import com.cloud.host.HostVO;
@@ -62,7 +60,6 @@
6260
import java.util.AbstractMap;
6361
import java.util.Arrays;
6462
import java.util.HashSet;
65-
import java.util.LinkedList;
6663
import java.util.List;
6764
import java.util.Set;
6865

@@ -372,72 +369,4 @@ public void validateIsStoragePoolTypeInListReturnsFalse() {
372369

373370
assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes));
374371
}
375-
376-
@Test
377-
public void validateAddSourcePoolToPoolsMapDestinationPoolIsManaged() {
378-
Mockito.doReturn(true).when(destinationStoragePoolVoMock).isManaged();
379-
strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
380-
381-
Mockito.verify(destinationStoragePoolVoMock).isManaged();
382-
Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
383-
}
384-
385-
@Test
386-
public void validateAddSourcePoolToPoolsMapDestinationPoolIsNotNFS() {
387-
List<StoragePoolType> storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values()));
388-
storagePoolTypes.remove(StoragePoolType.NetworkFilesystem);
389-
390-
Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged();
391-
storagePoolTypes.forEach(poolType -> {
392-
Mockito.doReturn(poolType).when(destinationStoragePoolVoMock).getPoolType();
393-
strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
394-
});
395-
396-
VerificationMode times = Mockito.times(storagePoolTypes.size());
397-
Mockito.verify(destinationStoragePoolVoMock, times).isManaged();
398-
Mockito.verify(destinationStoragePoolVoMock, times).getPoolType();
399-
Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
400-
}
401-
402-
@Test
403-
public void validateAddSourcePoolToPoolsMapMapContainsKey() {
404-
Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged();
405-
Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType();
406-
Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid();
407-
Mockito.doReturn(true).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString());
408-
strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
409-
410-
Mockito.verify(destinationStoragePoolVoMock, never()).getScope();
411-
Mockito.verify(destinationStoragePoolVoMock).isManaged();
412-
Mockito.verify(destinationStoragePoolVoMock).getPoolType();
413-
Mockito.verify(sourceStoragePoolVoMock).getUuid();
414-
Mockito.verify(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString());
415-
Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
416-
}
417-
418-
@Test
419-
public void validateAddSourcePoolToPoolsMapMapDoesNotContainsKey() {
420-
List<StoragePoolType> storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values()));
421-
422-
Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged();
423-
Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType();
424-
Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid();
425-
Mockito.doReturn(false).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString());
426-
Mockito.doReturn(null).when(mapStringStoragePoolTypeMock).put(Mockito.anyString(), Mockito.any());
427-
428-
storagePoolTypes.forEach(poolType -> {
429-
Mockito.doReturn(poolType).when(sourceStoragePoolVoMock).getPoolType();
430-
strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
431-
});
432-
433-
VerificationMode times = Mockito.times(storagePoolTypes.size());
434-
Mockito.verify(destinationStoragePoolVoMock, never()).getScope();
435-
Mockito.verify(destinationStoragePoolVoMock, times).isManaged();
436-
Mockito.verify(destinationStoragePoolVoMock, times).getPoolType();
437-
Mockito.verify(sourceStoragePoolVoMock, times).getUuid();
438-
Mockito.verify(mapStringStoragePoolTypeMock, times).containsKey(Mockito.anyString());
439-
Mockito.verify(sourceStoragePoolVoMock, times).getPoolType();
440-
Mockito.verify(mapStringStoragePoolTypeMock, times).put(Mockito.anyString(), Mockito.any());
441-
Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock);
442-
}
443372
}

0 commit comments

Comments
 (0)