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
<[email protected]>
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 <[email protected]>
---
.../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;
}
/**
