You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2019/08/07 10:11:42 UTC
[cloudstack] branch master updated: KVM local migration issue #3521
(#3533)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 5dc982d KVM local migration issue #3521 (#3533)
5dc982d is described below
commit 5dc982d8bab4ed16533dd56b39e98dadc79eeafe
Author: Gabriel Beims Bräscher <ga...@pcextreme.nl>
AuthorDate: Wed Aug 7 07:11:30 2019 -0300
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
---
.../java/com/cloud/agent/api/MigrateCommand.java | 9 ++++
.../motion/StorageSystemDataMotionStrategy.java | 40 +++++++++++----
.../StorageSystemDataMotionStrategyTest.java | 60 ++++++++++++++++++++++
.../hypervisor/kvm/resource/MigrateKVMAsync.java | 12 ++---
.../wrapper/LibvirtMigrateCommandWrapper.java | 4 +-
5 files changed, 109 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java
index 93324d2..c7aeb69 100644
--- a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java
+++ b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java
@@ -32,6 +32,7 @@ public class MigrateCommand extends Command {
private String destIp;
private Map<String, MigrateDiskInfo> migrateStorage;
private boolean migrateStorageManaged;
+ private boolean migrateNonSharedInc;
private boolean autoConvergence;
private String hostGuid;
private boolean isWindows;
@@ -75,6 +76,14 @@ public class MigrateCommand extends Command {
this.migrateStorageManaged = migrateStorageManaged;
}
+ public boolean isMigrateNonSharedInc() {
+ return migrateNonSharedInc;
+ }
+
+ public void setMigrateNonSharedInc(boolean migrateNonSharedInc) {
+ this.migrateNonSharedInc = migrateNonSharedInc;
+ }
+
public void setAutoConvergence(boolean autoConvergence) {
this.autoConvergence = autoConvergence;
}
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 45cd295..4d3ec18 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
@@ -1828,16 +1828,19 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
String destPath = generateDestPath(destHost, destStoragePool, destVolumeInfo);
MigrateCommand.MigrateDiskInfo migrateDiskInfo;
- if (managedStorageDestination) {
- migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath);
- migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool));
- migrateDiskInfoList.add(migrateDiskInfo);
- } else {
+
+ boolean isNonManagedNfsToNfs = sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem
+ && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
+ if (isNonManagedNfsToNfs) {
migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(),
MigrateCommand.MigrateDiskInfo.DiskType.FILE,
MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
MigrateCommand.MigrateDiskInfo.Source.FILE,
connectHostToVolume(destHost, destVolumeInfo.getPoolId(), volumeIdentifier));
+ } else {
+ migrateDiskInfo = configureMigrateDiskInfo(srcVolumeInfo, destPath);
+ migrateDiskInfo.setSourceDiskOnStorageFileSystem(isStoragePoolTypeOfFile(sourceStoragePool));
+ migrateDiskInfoList.add(migrateDiskInfo);
}
migrateStorage.put(srcVolumeInfo.getPath(), migrateDiskInfo);
@@ -1864,11 +1867,14 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
VMInstanceVO vm = _vmDao.findById(vmTO.getId());
boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows");
+ boolean migrateNonSharedInc = isSourceAndDestinationPoolTypeOfNfs(volumeDataStoreMap);
+
MigrateCommand migrateCommand = new MigrateCommand(vmTO.getName(), destHost.getPrivateIpAddress(), isWindows, vmTO, true);
migrateCommand.setWait(StorageManager.KvmStorageOnlineMigrationWait.value());
migrateCommand.setMigrateStorage(migrateStorage);
migrateCommand.setMigrateDiskInfoList(migrateDiskInfoList);
migrateCommand.setMigrateStorageManaged(managedStorageDestination);
+ migrateCommand.setMigrateNonSharedInc(migrateNonSharedInc);
boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value();
migrateCommand.setAutoConvergence(kvmAutoConvergence);
@@ -1906,6 +1912,23 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
}
/**
+ * Returns true if at least one of the entries on the map 'volumeDataStoreMap' has both source and destination storage pools of Network Filesystem (NFS).
+ */
+ protected boolean isSourceAndDestinationPoolTypeOfNfs(Map<VolumeInfo, DataStore> volumeDataStoreMap) {
+ for (Map.Entry<VolumeInfo, DataStore> entry : volumeDataStoreMap.entrySet()) {
+ VolumeInfo srcVolumeInfo = entry.getKey();
+ DataStore destDataStore = entry.getValue();
+
+ StoragePoolVO destStoragePool = _storagePoolDao.findById(destDataStore.getId());
+ StoragePoolVO sourceStoragePool = _storagePoolDao.findById(srcVolumeInfo.getPoolId());
+ if (sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
* 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.
*/
protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
@@ -2157,7 +2180,7 @@ 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.
*/
@@ -2191,9 +2214,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed");
}
- if (!destStoragePoolVO.isManaged()) {
- if (destStoragePoolVO.getPoolType() == StoragePoolType.NetworkFilesystem &&
- destStoragePoolVO.getScope() != ScopeType.CLUSTER) {
+ if (!destStoragePoolVO.isManaged() && destStoragePoolVO.getPoolType() == StoragePoolType.NetworkFilesystem) {
+ if (destStoragePoolVO.getScope() != ScopeType.CLUSTER) {
throw new CloudRuntimeException("KVM live storage migrations currently support cluster-wide " +
"not managed NFS destination storage");
}
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 1b383d9..288243c 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
@@ -205,4 +205,64 @@ public class StorageSystemDataMotionStrategyTest {
}
}
+ @Test
+ public void isSourceAndDestinationPoolTypeOfNfsTestNfsNfs() {
+ configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem, true);
+ }
+
+ @Test
+ public void isSourceAndDestinationPoolTypeOfNfsTestNfsAny() {
+ StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypeArray.length - 1; i++) {
+ if (storagePoolTypeArray[i] != StoragePoolType.NetworkFilesystem) {
+ configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(StoragePoolType.NetworkFilesystem, storagePoolTypeArray[i], false);
+ }
+ }
+ }
+
+ @Test
+ public void isSourceAndDestinationPoolTypeOfNfsTestAnyNfs() {
+ StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypeArray.length - 1; i++) {
+ if (storagePoolTypeArray[i] != StoragePoolType.NetworkFilesystem) {
+ configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(storagePoolTypeArray[i], StoragePoolType.NetworkFilesystem, false);
+ }
+ }
+ }
+
+ @Test
+ public void isSourceAndDestinationPoolTypeOfNfsTestAnyAny() {
+ StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
+ for (int i = 0; i < storagePoolTypeArray.length - 1; i++) {
+ for (int j = 0; j < storagePoolTypeArray.length - 1; j++) {
+ if (storagePoolTypeArray[i] != StoragePoolType.NetworkFilesystem || storagePoolTypeArray[j] != StoragePoolType.NetworkFilesystem) {
+ configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(storagePoolTypeArray[i], storagePoolTypeArray[j], false);
+ }
+ }
+ }
+ }
+
+ private void configureAndVerifyIsSourceAndDestinationPoolTypeOfNfs(StoragePoolType destStoragePoolType, StoragePoolType sourceStoragePoolType, boolean expected) {
+ VolumeInfo srcVolumeInfo = Mockito.mock(VolumeObject.class);
+ Mockito.when(srcVolumeInfo.getId()).thenReturn(0l);
+
+ DataStore destDataStore = Mockito.mock(PrimaryDataStoreImpl.class);
+ Mockito.when(destDataStore.getId()).thenReturn(1l);
+
+ StoragePoolVO destStoragePool = Mockito.mock(StoragePoolVO.class);
+ Mockito.when(destStoragePool.getPoolType()).thenReturn(destStoragePoolType);
+
+ StoragePoolVO sourceStoragePool = Mockito.mock(StoragePoolVO.class);
+ Mockito.when(sourceStoragePool.getPoolType()).thenReturn(sourceStoragePoolType);
+
+ Map<VolumeInfo, DataStore> volumeDataStoreMap = new HashMap<>();
+ volumeDataStoreMap.put(srcVolumeInfo, destDataStore);
+
+ Mockito.doReturn(sourceStoragePool).when(primaryDataStoreDao).findById(0l);
+ Mockito.doReturn(destStoragePool).when(primaryDataStoreDao).findById(1l);
+
+ boolean result = strategy.isSourceAndDestinationPoolTypeOfNfs(volumeDataStoreMap);
+ Assert.assertEquals(expected, result);
+ }
+
}
\ No newline at end of file
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java
index c3e3e6e..c391731 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java
@@ -34,7 +34,7 @@ public class MigrateKVMAsync implements Callable<Domain> {
private String vmName = "";
private String destIp = "";
private boolean migrateStorage;
- private boolean migrateStorageManaged;
+ private boolean migrateNonSharedInc;
private boolean autoConvergence;
// Libvirt Migrate Flags reference:
@@ -87,14 +87,14 @@ public class MigrateKVMAsync implements Callable<Domain> {
private static final int LIBVIRT_VERSION_SUPPORTS_AUTO_CONVERGE = 1002003;
public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, final Domain dm, final Connect dconn, final String dxml,
- final boolean migrateStorage, final boolean migrateStorageManaged, final boolean autoConvergence, final String vmName, final String destIp) {
+ final boolean migrateStorage, final boolean migrateNonSharedInc, final boolean autoConvergence, final String vmName, final String destIp) {
this.libvirtComputingResource = libvirtComputingResource;
this.dm = dm;
this.dconn = dconn;
this.dxml = dxml;
this.migrateStorage = migrateStorage;
- this.migrateStorageManaged = migrateStorageManaged;
+ this.migrateNonSharedInc = migrateNonSharedInc;
this.autoConvergence = autoConvergence;
this.vmName = vmName;
this.destIp = destIp;
@@ -109,11 +109,11 @@ public class MigrateKVMAsync implements Callable<Domain> {
}
if (migrateStorage) {
- if (migrateStorageManaged) {
- flags |= VIR_MIGRATE_NON_SHARED_DISK;
- } else {
+ if (migrateNonSharedInc) {
flags |= VIR_MIGRATE_PERSIST_DEST;
flags |= VIR_MIGRATE_NON_SHARED_INC;
+ } else {
+ flags |= VIR_MIGRATE_NON_SHARED_DISK;
}
}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
index e6d366e..f0eb287 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java
@@ -164,8 +164,10 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper<MigrateCo
//run migration in thread so we can monitor it
s_logger.info("Live migration of instance " + vmName + " initiated to destination host: " + dconn.getURI());
final ExecutorService executor = Executors.newFixedThreadPool(1);
+ boolean migrateNonSharedInc = command.isMigrateNonSharedInc() && !migrateStorageManaged;
+
final Callable<Domain> worker = new MigrateKVMAsync(libvirtComputingResource, dm, dconn, xmlDesc,
- migrateStorage, migrateStorageManaged,
+ migrateStorage, migrateNonSharedInc,
command.isAutoConvergence(), vmName, command.getDestinationIp());
final Future<Domain> migrateThread = executor.submit(worker);
executor.shutdown();