Skip to content

Commit 3ec5f1a

Browse files
JoaoJandredhslove
JoaoJandre
authored and
dhslove
committed
Fix migration from local storage to NFS in KVM (apache#8909)
* fix migration from local to nfs * remove unused imports * remove dead code
1 parent 1d6b9a4 commit 3ec5f1a

3 files changed

Lines changed: 9 additions & 124 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.logging.log4j.Logger;
7675
import org.apache.logging.log4j.LogManager;
@@ -83,7 +82,6 @@
8382
import com.cloud.agent.api.ModifyTargetsAnswer;
8483
import com.cloud.agent.api.ModifyTargetsCommand;
8584
import com.cloud.agent.api.PrepareForMigrationCommand;
86-
import com.cloud.agent.api.storage.CheckStorageAvailabilityCommand;
8785
import com.cloud.agent.api.storage.CopyVolumeAnswer;
8886
import com.cloud.agent.api.storage.CopyVolumeCommand;
8987
import com.cloud.agent.api.storage.MigrateVolumeAnswer;
@@ -1910,7 +1908,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
19101908
throw new CloudRuntimeException("Invalid hypervisor type (only KVM supported for this operation at the time being)");
19111909
}
19121910

1913-
verifyLiveMigrationForKVM(volumeDataStoreMap, destHost);
1911+
verifyLiveMigrationForKVM(volumeDataStoreMap);
19141912

19151913
VMInstanceVO vmInstance = _vmDao.findById(vmTO.getId());
19161914
vmTO.setState(vmInstance.getState());
@@ -1984,8 +1982,8 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
19841982

19851983
MigrateCommand.MigrateDiskInfo migrateDiskInfo;
19861984

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

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

‎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: 1 addition & 72 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;
@@ -47,8 +46,7 @@
4746
import org.mockito.Mock;
4847
import org.mockito.Mockito;
4948
import org.mockito.Spy;
50-
import org.mockito.junit.MockitoJUnitRunner;
51-
import org.mockito.verification.VerificationMode;
49+
import org.mockito.runners.MockitoJUnitRunner;
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)