cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ro...@apache.org
Subject [cloudstack] branch master updated: KVM local migration issue #3521 (#3533)
Date Wed, 07 Aug 2019 10:11:42 GMT
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 <gabriel@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();


Mime
View raw message