You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by nv...@apache.org on 2021/08/16 15:32:36 UTC
[cloudstack] branch main updated: Add SharedMountPoint to KVMs
supported storage pool types (#4780)
This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 65a48dc Add SharedMountPoint to KVMs supported storage pool types (#4780)
65a48dc is described below
commit 65a48dcb74e53ea977ea133b46f4acc10378ab34
Author: Daniel Augusto Veronezi Salvador <38...@users.noreply.github.com>
AuthorDate: Mon Aug 16 12:32:19 2021 -0300
Add SharedMountPoint to KVMs supported storage pool types (#4780)
* Add SharedMountPoint to KVMs supported storage pool types
* Fix live migration to iSCSI and improve logs
Co-authored-by: Daniel Augusto Veronezi Salvador <da...@scclouds.com.br>
---
.../KvmNonManagedStorageDataMotionStrategy.java | 11 +++--
.../motion/StorageSystemDataMotionStrategy.java | 31 ++++++++++--
.../KvmNonManagedStorageSystemDataMotionTest.java | 36 +++++++++++++-
.../StorageSystemDataMotionStrategyTest.java | 57 ++++++++++++++++++++++
.../kvm/storage/LibvirtStorageAdaptor.java | 53 ++++++++++++--------
5 files changed, 161 insertions(+), 27 deletions(-)
diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java
index a61b44b..bc951e6 100644
--- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java
+++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java
@@ -88,7 +88,8 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot
for (VolumeInfo volumeInfo : volumeInfoSet) {
StoragePoolVO storagePoolVO = _storagePoolDao.findById(volumeInfo.getPoolId());
- if (storagePoolVO.getPoolType() != StoragePoolType.Filesystem && storagePoolVO.getPoolType() != StoragePoolType.NetworkFilesystem) {
+
+ if (!supportStoragePoolType(storagePoolVO.getPoolType())) {
return StrategyPriority.CANT_HANDLE;
}
}
@@ -187,7 +188,7 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot
*/
@Override
protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
- return sourceStoragePool.getPoolType() == StoragePoolType.Filesystem || sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem;
+ return supportStoragePoolType(sourceStoragePool.getPoolType());
}
/**
@@ -201,7 +202,7 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot
}
VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId(), null);
- if (sourceVolumeTemplateStoragePoolVO == null && destStoragePool.getPoolType() == StoragePoolType.Filesystem) {
+ if (sourceVolumeTemplateStoragePoolVO == null && (isStoragePoolTypeInList(destStoragePool.getPoolType(), StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint))) {
DataStore sourceTemplateDataStore = dataStoreManagerImpl.getRandomImageStore(srcVolumeInfo.getDataCenterId());
if (sourceTemplateDataStore != null) {
TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore);
@@ -270,4 +271,8 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot
LOGGER.error(generateFailToCopyTemplateMessage(sourceTemplate, destDataStore) + failureDetails);
}
}
+
+ protected Boolean supportStoragePoolType(StoragePoolType storagePoolType) {
+ return super.supportStoragePoolType(storagePoolType, StoragePoolType.Filesystem);
+ }
}
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 460c337..3c68793 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
@@ -134,7 +134,10 @@ import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachineManager;
import com.cloud.vm.dao.VMInstanceDao;
import com.google.common.base.Preconditions;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.stream.Collectors;
+import org.apache.commons.collections.CollectionUtils;
public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
private static final Logger LOGGER = Logger.getLogger(StorageSystemDataMotionStrategy.class);
@@ -1861,9 +1864,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
MigrateCommand.MigrateDiskInfo migrateDiskInfo;
- boolean isNonManagedNfsToNfs = sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem
- && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
- if (isNonManagedNfsToNfs) {
+ boolean isNonManagedNfsToNfsOrSharedMountPointToNfs = supportStoragePoolType(sourceStoragePool.getPoolType()) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
+ if (isNonManagedNfsToNfsOrSharedMountPointToNfs) {
migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(),
MigrateCommand.MigrateDiskInfo.DiskType.FILE,
MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
@@ -2897,4 +2899,27 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
return copyCmdAnswer;
}
+
+ protected Boolean supportStoragePoolType(StoragePoolType storagePoolTypeToValidate, StoragePoolType... extraAcceptedValues) {
+ List<StoragePoolType> values = new ArrayList<>();
+
+ values.add(StoragePoolType.NetworkFilesystem);
+ values.add(StoragePoolType.SharedMountPoint);
+
+ if (extraAcceptedValues != null) {
+ CollectionUtils.addAll(values, extraAcceptedValues);
+ }
+
+ return isStoragePoolTypeInList(storagePoolTypeToValidate, values.toArray(new StoragePoolType[values.size()]));
+ }
+
+ protected Boolean isStoragePoolTypeInList(StoragePoolType storagePoolTypeToValidate, StoragePoolType... acceptedValues){
+ Set<StoragePoolType> supportedTypes = new HashSet<>();
+
+ if (acceptedValues != null) {
+ supportedTypes.addAll(Arrays.asList(acceptedValues));
+ }
+
+ return supportedTypes.contains(storagePoolTypeToValidate);
+ };
}
diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java
index 609742b..601f6bb 100644
--- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java
+++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java
@@ -75,6 +75,10 @@ import com.cloud.storage.dao.DiskOfferingDao;
import com.cloud.storage.dao.VMTemplatePoolDao;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VirtualMachineManager;
+import java.util.HashSet;
+import java.util.Set;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
@RunWith(MockitoJUnitRunner.class)
public class KvmNonManagedStorageSystemDataMotionTest {
@@ -159,13 +163,23 @@ public class KvmNonManagedStorageSystemDataMotionTest {
Assert.assertEquals(expectedStrategyPriority, strategyPriority);
}
+ public Boolean supportStoragePoolType(StoragePoolType storagePoolType) {
+ Set<StoragePoolType> supportedTypes = new HashSet<>();
+ supportedTypes.add(StoragePoolType.Filesystem);
+ supportedTypes.add(StoragePoolType.NetworkFilesystem);
+ supportedTypes.add(StoragePoolType.SharedMountPoint);
+
+ return supportedTypes.contains(storagePoolType);
+ }
+
@Test
public void internalCanHandleTestNonManaged() {
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
for (int i = 0; i < storagePoolTypeArray.length; i++) {
Map<VolumeInfo, DataStore> volumeMap = configureTestInternalCanHandle(false, storagePoolTypeArray[i]);
StrategyPriority strategyPriority = kvmNonManagedStorageDataMotionStrategy.internalCanHandle(volumeMap, new HostVO("sourceHostUuid"), new HostVO("destHostUuid"));
- if (storagePoolTypeArray[i] == StoragePoolType.Filesystem || storagePoolTypeArray[i] == StoragePoolType.NetworkFilesystem) {
+
+ if (supportStoragePoolType(storagePoolTypeArray[i])) {
Assert.assertEquals(StrategyPriority.HYPERVISOR, strategyPriority);
} else {
Assert.assertEquals(StrategyPriority.CANT_HANDLE, strategyPriority);
@@ -243,7 +257,7 @@ public class KvmNonManagedStorageSystemDataMotionTest {
for (int i = 0; i < storagePoolTypes.length; i++) {
Mockito.doReturn(storagePoolTypes[i]).when(sourceStoragePool).getPoolType();
boolean result = kvmNonManagedStorageDataMotionStrategy.shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool);
- if (storagePoolTypes[i] == StoragePoolType.Filesystem || storagePoolTypes[i] == StoragePoolType.NetworkFilesystem) {
+ if (supportStoragePoolType(storagePoolTypes[i])) {
Assert.assertTrue(result);
} else {
Assert.assertFalse(result);
@@ -472,4 +486,22 @@ public class KvmNonManagedStorageSystemDataMotionTest {
lenient().when(pool2.isManaged()).thenReturn(false);
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
}
+
+ @Test
+ public void validateSupportStoragePoolType() {
+ Set<StoragePoolType> supportedTypes = new HashSet<>();
+ supportedTypes.add(StoragePoolType.Filesystem);
+ supportedTypes.add(StoragePoolType.NetworkFilesystem);
+ supportedTypes.add(StoragePoolType.SharedMountPoint);
+
+ for (StoragePoolType poolType : StoragePoolType.values()) {
+ boolean isSupported = kvmNonManagedStorageDataMotionStrategy.supportStoragePoolType(poolType);
+ if (supportedTypes.contains(poolType)) {
+ assertTrue(isSupported);
+ } else {
+ assertFalse(isSupported);
+ }
+ }
+ }
+
}
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 dcb7a44..3793a79 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
@@ -19,6 +19,7 @@
package org.apache.cloudstack.storage.motion;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
@@ -56,6 +57,8 @@ import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import java.util.AbstractMap;
+import java.util.HashSet;
+import java.util.Set;
@RunWith(MockitoJUnitRunner.class)
public class StorageSystemDataMotionStrategyTest {
@@ -288,4 +291,58 @@ public class StorageSystemDataMotionStrategyTest {
Assert.assertEquals(String.format("{volume: \"%s\", from: \"%s\", to:\"%s\"}", volume, from, to), strategy.formatEntryOfVolumesAndStoragesAsJsonToDisplayOnLog(new AbstractMap.SimpleEntry<>(volumeInfo, dataStore)));
}
+
+ @Test
+ public void validateSupportStoragePoolTypeDefaultValues() {
+ Set<StoragePoolType> supportedTypes = new HashSet<>();
+ supportedTypes.add(StoragePoolType.NetworkFilesystem);
+ supportedTypes.add(StoragePoolType.SharedMountPoint);
+
+ for (StoragePoolType poolType : StoragePoolType.values()) {
+ boolean isSupported = strategy.supportStoragePoolType(poolType);
+ if (supportedTypes.contains(poolType)) {
+ assertTrue(isSupported);
+ } else {
+ assertFalse(isSupported);
+ }
+ }
+ }
+
+ @Test
+ public void validateSupportStoragePoolTypeExtraValues() {
+ Set<StoragePoolType> supportedTypes = new HashSet<>();
+ supportedTypes.add(StoragePoolType.NetworkFilesystem);
+ supportedTypes.add(StoragePoolType.SharedMountPoint);
+ supportedTypes.add(StoragePoolType.Iscsi);
+ supportedTypes.add(StoragePoolType.CLVM);
+
+ for (StoragePoolType poolType : StoragePoolType.values()) {
+ boolean isSupported = strategy.supportStoragePoolType(poolType, StoragePoolType.Iscsi, StoragePoolType.CLVM);
+ if (supportedTypes.contains(poolType)) {
+ assertTrue(isSupported);
+ } else {
+ assertFalse(isSupported);
+ }
+ }
+ }
+
+ @Test
+ public void validateIsStoragePoolTypeInListReturnsTrue() {
+ StoragePoolType[] listTypes = new StoragePoolType[3];
+ listTypes[0] = StoragePoolType.LVM;
+ listTypes[1] = StoragePoolType.NetworkFilesystem;
+ listTypes[2] = StoragePoolType.SharedMountPoint;
+
+ assertTrue(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes));
+ }
+
+ @Test
+ public void validateIsStoragePoolTypeInListReturnsFalse() {
+ StoragePoolType[] listTypes = new StoragePoolType[3];
+ listTypes[0] = StoragePoolType.LVM;
+ listTypes[1] = StoragePoolType.NetworkFilesystem;
+ listTypes[2] = StoragePoolType.RBD;
+
+ assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes));
+ }
}
\ No newline at end of file
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
index 3532996..85fa367 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
@@ -64,6 +64,10 @@ import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.script.Script;
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Collectors;
public class LibvirtStorageAdaptor implements StorageAdaptor {
private static final Logger s_logger = Logger.getLogger(LibvirtStorageAdaptor.class);
@@ -80,6 +84,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
private int rbdFeatures = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN;
private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */
+ private static final Set<StoragePoolType> poolTypesThatEnableCreateDiskFromTemplateBacking = new HashSet<>(Arrays.asList(StoragePoolType.NetworkFilesystem,
+ StoragePoolType.Filesystem));
+
public LibvirtStorageAdaptor(StorageLayer storage) {
_storageLayer = storage;
_manageSnapshotPath = Script.findScript("scripts/storage/qcow2/", "managesnapshot.sh");
@@ -98,28 +105,36 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
@Override
public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size,
KVMStoragePool destPool, int timeout) {
- s_logger.info("Creating volume " + name + " with template backing " + template.getName() + " in pool " + destPool.getUuid() +
- " (" + destPool.getType().toString() + ") with size " + size);
+ String volumeDesc = String.format("volume [%s], with template backing [%s], in pool [%s] (%s), with size [%s]", name, template.getName(), destPool.getUuid(),
+ destPool.getType(), size);
- KVMPhysicalDisk disk = null;
- String destPath = destPool.getLocalPath().endsWith("/") ?
- destPool.getLocalPath() + name :
- destPool.getLocalPath() + "/" + name;
+ if (!poolTypesThatEnableCreateDiskFromTemplateBacking.contains(destPool.getType())) {
+ s_logger.info(String.format("Skipping creation of %s due to pool type is none of the following types %s.", volumeDesc, poolTypesThatEnableCreateDiskFromTemplateBacking.stream()
+ .map(type -> type.toString()).collect(Collectors.joining(", "))));
- if (destPool.getType() == StoragePoolType.NetworkFilesystem) {
- try {
- if (format == PhysicalDiskFormat.QCOW2) {
- QemuImg qemu = new QemuImg(timeout);
- QemuImgFile destFile = new QemuImgFile(destPath, format);
- destFile.setSize(size);
- QemuImgFile backingFile = new QemuImgFile(template.getPath(), template.getFormat());
- qemu.create(destFile, backingFile);
- }
- } catch (QemuImgException e) {
- s_logger.error("Failed to create " + destPath + " due to a failed executing of qemu-img: " + e.getMessage());
- }
+ return null;
}
- return disk;
+
+ if (format != PhysicalDiskFormat.QCOW2) {
+ s_logger.info(String.format("Skipping creation of %s due to format [%s] is not [%s].", volumeDesc, format, PhysicalDiskFormat.QCOW2));
+ return null;
+ }
+
+ s_logger.info(String.format("Creating %s.", volumeDesc));
+
+ String destPoolLocalPath = destPool.getLocalPath();
+ String destPath = String.format("%s%s%s", destPoolLocalPath, destPoolLocalPath.endsWith("/") ? "" : "/", name);
+
+ try {
+ QemuImgFile destFile = new QemuImgFile(destPath, format);
+ destFile.setSize(size);
+ QemuImgFile backingFile = new QemuImgFile(template.getPath(), template.getFormat());
+ new QemuImg(timeout).create(destFile, backingFile);
+ } catch (QemuImgException e) {
+ s_logger.error(String.format("Failed to create %s in [%s] due to [%s].", volumeDesc, destPath, e.getMessage()), e);
+ }
+
+ return null;
}
/**