Skip to content

Commit 5dc982d

Browse files
Gabriel Beims Bräscheryadvr
Gabriel Beims Bräscher
authored and
yadvr
committed
KVM local migration issue #3521 (#3533)
Fix regression bug that affects KVM local storage migration. Some of the desired execution flows for KVM local storage migration had been altered to allow only managed storage to execute. Fixed allowing managed and non managed storages to execute. Fixes #3521
1 parent 3dad7f3 commit 5dc982d

5 files changed

Lines changed: 109 additions & 16 deletions

File tree

‎core/src/main/java/com/cloud/agent/api/MigrateCommand.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public class MigrateCommand extends Command {
3232
private String destIp;
3333
private Map<String, MigrateDiskInfo> migrateStorage;
3434
private boolean migrateStorageManaged;
35+
private boolean migrateNonSharedInc;
3536
private boolean autoConvergence;
3637
private String hostGuid;
3738
private boolean isWindows;
@@ -75,6 +76,14 @@ public void setMigrateStorageManaged(boolean migrateStorageManaged) {
7576
this.migrateStorageManaged = migrateStorageManaged;
7677
}
7778

79+
public boolean isMigrateNonSharedInc() {
80+
return migrateNonSharedInc;
81+
}
82+
83+
public void setMigrateNonSharedInc(boolean migrateNonSharedInc) {
84+
this.migrateNonSharedInc = migrateNonSharedInc;
85+
}
86+
7887
public void setAutoConvergence(boolean autoConvergence) {
7988
this.autoConvergence = autoConvergence;
8089
}

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1828,16 +1828,19 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
18281828
String destPath = generateDestPath(destHost, destStoragePool, destVolumeInfo);
18291829

18301830
MigrateCommand.MigrateDiskInfo migrateDiskInfo;
1831-
if (managedStorageDestination) {
1832-
migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath);
1833-
migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool));
1834-
migrateDiskInfoList.add(migrateDiskInfo);
1835-
} else {
1831+
1832+
boolean isNonManagedNfsToNfs = sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem
1833+
&& destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
1834+
if (isNonManagedNfsToNfs) {
18361835
migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(),
18371836
MigrateCommand.MigrateDiskInfo.DiskType.FILE,
18381837
MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
18391838
MigrateCommand.MigrateDiskInfo.Source.FILE,
18401839
connectHostToVolume(destHost, destVolumeInfo.getPoolId(), volumeIdentifier));
1840+
} else {
1841+
migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath);
1842+
migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool));
1843+
migrateDiskInfoList.add(migrateDiskInfo);
18411844
}
18421845

18431846
migrateStorage.put(srcVolumeInfo.getPath(), migrateDiskInfo);
@@ -1864,11 +1867,14 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
18641867
VMInstanceVO vm = _vmDao.findById(vmTO.getId());
18651868
boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows");
18661869

1870+
boolean migrateNonSharedInc = isSourceAndDestinationPoolTypeOfNfs(volumeDataStoreMap);
1871+
18671872
MigrateCommand migrateCommand = new MigrateCommand(vmTO.getName(), destHost.getPrivateIpAddress(), isWindows, vmTO, true);
18681873
migrateCommand.setWait(StorageManager.KvmStorageOnlineMigrationWait.value());
18691874
migrateCommand.setMigrateStorage(migrateStorage);
18701875
migrateCommand.setMigrateDiskInfoList(migrateDiskInfoList);
18711876
migrateCommand.setMigrateStorageManaged(managedStorageDestination);
1877+
migrateCommand.setMigrateNonSharedInc(migrateNonSharedInc);
18721878

18731879
boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value();
18741880
migrateCommand.setAutoConvergence(kvmAutoConvergence);
@@ -1905,6 +1911,23 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
19051911
}
19061912
}
19071913

1914+
/**
1915+
* Returns true if at least one of the entries on the map 'volumeDataStoreMap' has both source and destination storage pools of Network Filesystem (NFS).
1916+
*/
1917+
protected boolean isSourceAndDestinationPoolTypeOfNfs(Map<VolumeInfo, DataStore> volumeDataStoreMap) {
1918+
for (Map.Entry<VolumeInfo, DataStore> entry : volumeDataStoreMap.entrySet()) {
1919+
VolumeInfo srcVolumeInfo = entry.getKey();
1920+
DataStore destDataStore = entry.getValue();
1921+
1922+
StoragePoolVO destStoragePool = _storagePoolDao.findById(destDataStore.getId());
1923+
StoragePoolVO sourceStoragePool = _storagePoolDao.findById(srcVolumeInfo.getPoolId());
1924+
if (sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem) {
1925+
return true;
1926+
}
1927+
}
1928+
return false;
1929+
}
1930+
19081931
/**
19091932
* Returns true. This method was implemented considering the classes that extend this {@link StorageSystemDataMotionStrategy} and cannot migrate volumes from certain types of source storage pools and/or to a different kind of destiny storage pool.
19101933
*/
@@ -2157,7 +2180,7 @@ protected void postVolumeCreationActions(VolumeInfo srcVolumeInfo, VolumeInfo de
21572180
}
21582181
}
21592182

2160-
/*
2183+
/**
21612184
* At a high level: The source storage cannot be managed and
21622185
* the destination storages can be all managed or all not managed, not mixed.
21632186
*/
@@ -2191,9 +2214,8 @@ protected void verifyLiveMigrationForKVM(Map<VolumeInfo, DataStore> volumeDataSt
21912214
throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed");
21922215
}
21932216

2194-
if (!destStoragePoolVO.isManaged()) {
2195-
if (destStoragePoolVO.getPoolType() == StoragePoolType.NetworkFilesystem &&
2196-
destStoragePoolVO.getScope() != ScopeType.CLUSTER) {
2217+
if (!destStoragePoolVO.isManaged() && destStoragePoolVO.getPoolType() == StoragePoolType.NetworkFilesystem) {
2218+
if (destStoragePoolVO.getScope() != ScopeType.CLUSTER) {
21972219
throw new CloudRuntimeException("KVM live storage migrations currently support cluster-wide " +
21982220
"not managed NFS destination storage");
21992221
}

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,4 +205,64 @@ public void shouldMigrateVolumeTest() {
205205
}
206206
}
207207

208+
@Test
209+
public void isSourceAndDestinationPoolTypeOfNfsTestNfsNfs() {
210+
configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem, true);
211+
}
212+
213+
@Test
214+
public void isSourceAndDestinationPoolTypeOfNfsTestNfsAny() {
215+
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
216+
for (int i = 0; i < storagePoolTypeArray.length - 1; i++) {
217+
if (storagePoolTypeArray[i] != StoragePoolType.NetworkFilesystem) {
218+
configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(StoragePoolType.NetworkFilesystem, storagePoolTypeArray[i], false);
219+
}
220+
}
221+
}
222+
223+
@Test
224+
public void isSourceAndDestinationPoolTypeOfNfsTestAnyNfs() {
225+
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
226+
for (int i = 0; i < storagePoolTypeArray.length - 1; i++) {
227+
if (storagePoolTypeArray[i] != StoragePoolType.NetworkFilesystem) {
228+
configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(storagePoolTypeArray[i], StoragePoolType.NetworkFilesystem, false);
229+
}
230+
}
231+
}
232+
233+
@Test
234+
public void isSourceAndDestinationPoolTypeOfNfsTestAnyAny() {
235+
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
236+
for (int i = 0; i < storagePoolTypeArray.length - 1; i++) {
237+
for (int j = 0; j < storagePoolTypeArray.length - 1; j++) {
238+
if (storagePoolTypeArray[i] != StoragePoolType.NetworkFilesystem || storagePoolTypeArray[j] != StoragePoolType.NetworkFilesystem) {
239+
configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(storagePoolTypeArray[i], storagePoolTypeArray[j], false);
240+
}
241+
}
242+
}
243+
}
244+
245+
private void configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(StoragePoolType destStoragePoolType, StoragePoolType sourceStoragePoolType, boolean expected) {
246+
VolumeInfo srcVolumeInfo = Mockito.mock(VolumeObject.class);
247+
Mockito.when(srcVolumeInfo.getId()).thenReturn(0l);
248+
249+
DataStore destDataStore = Mockito.mock(PrimaryDataStoreImpl.class);
250+
Mockito.when(destDataStore.getId()).thenReturn(1l);
251+
252+
StoragePoolVO destStoragePool = Mockito.mock(StoragePoolVO.class);
253+
Mockito.when(destStoragePool.getPoolType()).thenReturn(destStoragePoolType);
254+
255+
StoragePoolVO sourceStoragePool = Mockito.mock(StoragePoolVO.class);
256+
Mockito.when(sourceStoragePool.getPoolType()).thenReturn(sourceStoragePoolType);
257+
258+
Map<VolumeInfo, DataStore> volumeDataStoreMap = new HashMap<>();
259+
volumeDataStoreMap.put(srcVolumeInfo, destDataStore);
260+
261+
Mockito.doReturn(sourceStoragePool).when(primaryDataStoreDao).findById(0l);
262+
Mockito.doReturn(destStoragePool).when(primaryDataStoreDao).findById(1l);
263+
264+
boolean result = strategy.isSourceAndDestinationPoolTypeOfNfs(volumeDataStoreMap);
265+
Assert.assertEquals(expected, result);
266+
}
267+
208268
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class MigrateKVMAsync implements Callable<Domain> {
3434
private String vmName = "";
3535
private String destIp = "";
3636
private boolean migrateStorage;
37-
private boolean migrateStorageManaged;
37+
private boolean migrateNonSharedInc;
3838
private boolean autoConvergence;
3939

4040
// Libvirt Migrate Flags reference:
@@ -87,14 +87,14 @@ public class MigrateKVMAsync implements Callable<Domain> {
8787
private static final int LIBVIRT_VERSION_SUPPORTS_AUTO_CONVERGE = 1002003;
8888

8989
public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, final Domain dm, final Connect dconn, final String dxml,
90-
final boolean migrateStorage, final boolean migrateStorageManaged, final boolean autoConvergence, final String vmName, final String destIp) {
90+
final boolean migrateStorage, final boolean migrateNonSharedInc, final boolean autoConvergence, final String vmName, final String destIp) {
9191
this.libvirtComputingResource = libvirtComputingResource;
9292

9393
this.dm = dm;
9494
this.dconn = dconn;
9595
this.dxml = dxml;
9696
this.migrateStorage = migrateStorage;
97-
this.migrateStorageManaged = migrateStorageManaged;
97+
this.migrateNonSharedInc = migrateNonSharedInc;
9898
this.autoConvergence = autoConvergence;
9999
this.vmName = vmName;
100100
this.destIp = destIp;
@@ -109,11 +109,11 @@ public Domain call() throws LibvirtException {
109109
}
110110

111111
if (migrateStorage) {
112-
if (migrateStorageManaged) {
113-
flags |= VIR_MIGRATE_NON_SHARED_DISK;
114-
} else {
112+
if (migrateNonSharedInc) {
115113
flags |= VIR_MIGRATE_PERSIST_DEST;
116114
flags |= VIR_MIGRATE_NON_SHARED_INC;
115+
} else {
116+
flags |= VIR_MIGRATE_NON_SHARED_DISK;
117117
}
118118
}
119119

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,10 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0.
164164
//run migration in thread so we can monitor it
165165
s_logger.info("Live migration of instance " + vmName + " initiated to destination host: " + dconn.getURI());
166166
final ExecutorService executor = Executors.newFixedThreadPool(1);
167+
boolean migrateNonSharedInc = command.isMigrateNonSharedInc() && !migrateStorageManaged;
168+
167169
final Callable<Domain> worker = new MigrateKVMAsync(libvirtComputingResource, dm, dconn, xmlDesc,
168-
migrateStorage, migrateStorageManaged,
170+
migrateStorage, migrateNonSharedInc,
169171
command.isAutoConvergence(), vmName, command.getDestinationIp());
170172
final Future<Domain> migrateThread = executor.submit(worker);
171173
executor.shutdown();

0 commit comments

Comments
 (0)