Skip to content

Commit 65a48dc

Browse files
GutoVeroneziDaniel Augusto Veronezi Salvador
GutoVeronezi
and
Daniel Augusto Veronezi Salvador
authored
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 <daniel@scclouds.com.br>
1 parent 664a46a commit 65a48dc

5 files changed

Lines changed: 161 additions & 27 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ protected StrategyPriority internalCanHandle(Map<VolumeInfo, DataStore> volumeMa
8888

8989
for (VolumeInfo volumeInfo : volumeInfoSet) {
9090
StoragePoolVO storagePoolVO = _storagePoolDao.findById(volumeInfo.getPoolId());
91-
if (storagePoolVO.getPoolType() != StoragePoolType.Filesystem && storagePoolVO.getPoolType() != StoragePoolType.NetworkFilesystem) {
91+
92+
if (!supportStoragePoolType(storagePoolVO.getPoolType())) {
9293
return StrategyPriority.CANT_HANDLE;
9394
}
9495
}
@@ -187,7 +188,7 @@ protected void setVolumePath(VolumeVO volume) {
187188
*/
188189
@Override
189190
protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) {
190-
return sourceStoragePool.getPoolType() == StoragePoolType.Filesystem || sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem;
191+
return supportStoragePoolType(sourceStoragePool.getPoolType());
191192
}
192193

193194
/**
@@ -201,7 +202,7 @@ protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolum
201202
}
202203

203204
VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId(), null);
204-
if (sourceVolumeTemplateStoragePoolVO == null && destStoragePool.getPoolType() == StoragePoolType.Filesystem) {
205+
if (sourceVolumeTemplateStoragePoolVO == null && (isStoragePoolTypeInList(destStoragePool.getPoolType(), StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint))) {
205206
DataStore sourceTemplateDataStore = dataStoreManagerImpl.getRandomImageStore(srcVolumeInfo.getDataCenterId());
206207
if (sourceTemplateDataStore != null) {
207208
TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore);
@@ -270,4 +271,8 @@ protected void logInCaseOfTemplateCopyFailure(Answer copyCommandAnswer, Template
270271
LOGGER.error(generateFailToCopyTemplateMessage(sourceTemplate, destDataStore) + failureDetails);
271272
}
272273
}
274+
275+
protected Boolean supportStoragePoolType(StoragePoolType storagePoolType) {
276+
return super.supportStoragePoolType(storagePoolType, StoragePoolType.Filesystem);
277+
}
273278
}

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@
134134
import com.cloud.vm.VirtualMachineManager;
135135
import com.cloud.vm.dao.VMInstanceDao;
136136
import com.google.common.base.Preconditions;
137+
import java.util.Arrays;
138+
import java.util.HashSet;
137139
import java.util.stream.Collectors;
140+
import org.apache.commons.collections.CollectionUtils;
138141

139142
public class StorageSystemDataMotionStrategy implements DataMotionStrategy {
140143
private static final Logger LOGGER = Logger.getLogger(StorageSystemDataMotionStrategy.class);
@@ -1861,9 +1864,8 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
18611864

18621865
MigrateCommand.MigrateDiskInfo migrateDiskInfo;
18631866

1864-
boolean isNonManagedNfsToNfs = sourceStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem
1865-
&& destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
1866-
if (isNonManagedNfsToNfs) {
1867+
boolean isNonManagedNfsToNfsOrSharedMountPointToNfs = supportStoragePoolType(sourceStoragePool.getPoolType()) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination;
1868+
if (isNonManagedNfsToNfsOrSharedMountPointToNfs) {
18671869
migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(),
18681870
MigrateCommand.MigrateDiskInfo.DiskType.FILE,
18691871
MigrateCommand.MigrateDiskInfo.DriverType.QCOW2,
@@ -2897,4 +2899,27 @@ private CopyCmdAnswer performCopyOfVdi(VolumeInfo volumeInfo, SnapshotInfo snaps
28972899

28982900
return copyCmdAnswer;
28992901
}
2902+
2903+
protected Boolean supportStoragePoolType(StoragePoolType storagePoolTypeToValidate, StoragePoolType... extraAcceptedValues) {
2904+
List<StoragePoolType> values = new ArrayList<>();
2905+
2906+
values.add(StoragePoolType.NetworkFilesystem);
2907+
values.add(StoragePoolType.SharedMountPoint);
2908+
2909+
if (extraAcceptedValues != null) {
2910+
CollectionUtils.addAll(values, extraAcceptedValues);
2911+
}
2912+
2913+
return isStoragePoolTypeInList(storagePoolTypeToValidate, values.toArray(new StoragePoolType[values.size()]));
2914+
}
2915+
2916+
protected Boolean isStoragePoolTypeInList(StoragePoolType storagePoolTypeToValidate, StoragePoolType... acceptedValues){
2917+
Set<StoragePoolType> supportedTypes = new HashSet<>();
2918+
2919+
if (acceptedValues != null) {
2920+
supportedTypes.addAll(Arrays.asList(acceptedValues));
2921+
}
2922+
2923+
return supportedTypes.contains(storagePoolTypeToValidate);
2924+
};
29002925
}

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

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@
7575
import com.cloud.storage.dao.VMTemplatePoolDao;
7676
import com.cloud.utils.exception.CloudRuntimeException;
7777
import com.cloud.vm.VirtualMachineManager;
78+
import java.util.HashSet;
79+
import java.util.Set;
80+
import static org.junit.Assert.assertFalse;
81+
import static org.junit.Assert.assertTrue;
7882

7983
@RunWith(MockitoJUnitRunner.class)
8084
public class KvmNonManagedStorageSystemDataMotionTest {
@@ -159,13 +163,23 @@ private void canHandleExpectCannotHandle(HypervisorType hypervisorType, int time
159163
Assert.assertEquals(expectedStrategyPriority, strategyPriority);
160164
}
161165

166+
public Boolean supportStoragePoolType(StoragePoolType storagePoolType) {
167+
Set<StoragePoolType> supportedTypes = new HashSet<>();
168+
supportedTypes.add(StoragePoolType.Filesystem);
169+
supportedTypes.add(StoragePoolType.NetworkFilesystem);
170+
supportedTypes.add(StoragePoolType.SharedMountPoint);
171+
172+
return supportedTypes.contains(storagePoolType);
173+
}
174+
162175
@Test
163176
public void internalCanHandleTestNonManaged() {
164177
StoragePoolType[] storagePoolTypeArray = StoragePoolType.values();
165178
for (int i = 0; i < storagePoolTypeArray.length; i++) {
166179
Map<VolumeInfo, DataStore> volumeMap = configureTestInternalCanHandle(false, storagePoolTypeArray[i]);
167180
StrategyPriority strategyPriority = kvmNonManagedStorageDataMotionStrategy.internalCanHandle(volumeMap, new HostVO("sourceHostUuid"), new HostVO("destHostUuid"));
168-
if (storagePoolTypeArray[i] == StoragePoolType.Filesystem || storagePoolTypeArray[i] == StoragePoolType.NetworkFilesystem) {
181+
182+
if (supportStoragePoolType(storagePoolTypeArray[i])) {
169183
Assert.assertEquals(StrategyPriority.HYPERVISOR, strategyPriority);
170184
} else {
171185
Assert.assertEquals(StrategyPriority.CANT_HANDLE, strategyPriority);
@@ -243,7 +257,7 @@ public void shouldMigrateVolumeTest() {
243257
for (int i = 0; i < storagePoolTypes.length; i++) {
244258
Mockito.doReturn(storagePoolTypes[i]).when(sourceStoragePool).getPoolType();
245259
boolean result = kvmNonManagedStorageDataMotionStrategy.shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool);
246-
if (storagePoolTypes[i] == StoragePoolType.Filesystem || storagePoolTypes[i] == StoragePoolType.NetworkFilesystem) {
260+
if (supportStoragePoolType(storagePoolTypes[i])) {
247261
Assert.assertTrue(result);
248262
} else {
249263
Assert.assertFalse(result);
@@ -472,4 +486,22 @@ public void testVerifyLiveMigrationMapForKVMMixedManagedUnmagedStorage() {
472486
lenient().when(pool2.isManaged()).thenReturn(false);
473487
kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2);
474488
}
489+
490+
@Test
491+
public void validateSupportStoragePoolType() {
492+
Set<StoragePoolType> supportedTypes = new HashSet<>();
493+
supportedTypes.add(StoragePoolType.Filesystem);
494+
supportedTypes.add(StoragePoolType.NetworkFilesystem);
495+
supportedTypes.add(StoragePoolType.SharedMountPoint);
496+
497+
for (StoragePoolType poolType : StoragePoolType.values()) {
498+
boolean isSupported = kvmNonManagedStorageDataMotionStrategy.supportStoragePoolType(poolType);
499+
if (supportedTypes.contains(poolType)) {
500+
assertTrue(isSupported);
501+
} else {
502+
assertFalse(isSupported);
503+
}
504+
}
505+
}
506+
475507
}

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.cloudstack.storage.motion;
2020

2121
import static org.junit.Assert.assertTrue;
22+
import static org.junit.Assert.assertFalse;
2223
import static org.mockito.Mockito.doReturn;
2324
import static org.mockito.Mockito.lenient;
2425
import static org.mockito.Mockito.mock;
@@ -56,6 +57,8 @@
5657
import com.cloud.storage.Volume;
5758
import com.cloud.storage.VolumeVO;
5859
import java.util.AbstractMap;
60+
import java.util.HashSet;
61+
import java.util.Set;
5962

6063
@RunWith(MockitoJUnitRunner.class)
6164
public class StorageSystemDataMotionStrategyTest {
@@ -288,4 +291,58 @@ public void formatEntryOfVolumesAndStoragesAsJsonToDisplayOnLogValidateFormat(){
288291

289292
Assert.assertEquals(String.format("{volume: \"%s\", from: \"%s\", to:\"%s\"}", volume, from, to), strategy.formatEntryOfVolumesAndStoragesAsJsonToDisplayOnLog(new AbstractMap.SimpleEntry<>(volumeInfo, dataStore)));
290293
}
294+
295+
@Test
296+
public void validateSupportStoragePoolTypeDefaultValues() {
297+
Set<StoragePoolType> supportedTypes = new HashSet<>();
298+
supportedTypes.add(StoragePoolType.NetworkFilesystem);
299+
supportedTypes.add(StoragePoolType.SharedMountPoint);
300+
301+
for (StoragePoolType poolType : StoragePoolType.values()) {
302+
boolean isSupported = strategy.supportStoragePoolType(poolType);
303+
if (supportedTypes.contains(poolType)) {
304+
assertTrue(isSupported);
305+
} else {
306+
assertFalse(isSupported);
307+
}
308+
}
309+
}
310+
311+
@Test
312+
public void validateSupportStoragePoolTypeExtraValues() {
313+
Set<StoragePoolType> supportedTypes = new HashSet<>();
314+
supportedTypes.add(StoragePoolType.NetworkFilesystem);
315+
supportedTypes.add(StoragePoolType.SharedMountPoint);
316+
supportedTypes.add(StoragePoolType.Iscsi);
317+
supportedTypes.add(StoragePoolType.CLVM);
318+
319+
for (StoragePoolType poolType : StoragePoolType.values()) {
320+
boolean isSupported = strategy.supportStoragePoolType(poolType, StoragePoolType.Iscsi, StoragePoolType.CLVM);
321+
if (supportedTypes.contains(poolType)) {
322+
assertTrue(isSupported);
323+
} else {
324+
assertFalse(isSupported);
325+
}
326+
}
327+
}
328+
329+
@Test
330+
public void validateIsStoragePoolTypeInListReturnsTrue() {
331+
StoragePoolType[] listTypes = new StoragePoolType[3];
332+
listTypes[0] = StoragePoolType.LVM;
333+
listTypes[1] = StoragePoolType.NetworkFilesystem;
334+
listTypes[2] = StoragePoolType.SharedMountPoint;
335+
336+
assertTrue(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes));
337+
}
338+
339+
@Test
340+
public void validateIsStoragePoolTypeInListReturnsFalse() {
341+
StoragePoolType[] listTypes = new StoragePoolType[3];
342+
listTypes[0] = StoragePoolType.LVM;
343+
listTypes[1] = StoragePoolType.NetworkFilesystem;
344+
listTypes[2] = StoragePoolType.RBD;
345+
346+
assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes));
347+
}
291348
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java‎

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464
import com.cloud.utils.script.Script;
6565

6666
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
67+
import java.util.Arrays;
68+
import java.util.HashSet;
69+
import java.util.Set;
70+
import java.util.stream.Collectors;
6771

6872
public class LibvirtStorageAdaptor implements StorageAdaptor {
6973
private static final Logger s_logger = Logger.getLogger(LibvirtStorageAdaptor.class);
@@ -80,6 +84,9 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
8084
private int rbdFeatures = RBD_FEATURE_LAYERING + RBD_FEATURE_EXCLUSIVE_LOCK + RBD_FEATURE_OBJECT_MAP + RBD_FEATURE_FAST_DIFF + RBD_FEATURE_DEEP_FLATTEN;
8185
private int rbdOrder = 0; /* Order 0 means 4MB blocks (the default) */
8286

87+
private static final Set<StoragePoolType> poolTypesThatEnableCreateDiskFromTemplateBacking = new HashSet<>(Arrays.asList(StoragePoolType.NetworkFilesystem,
88+
StoragePoolType.Filesystem));
89+
8390
public LibvirtStorageAdaptor(StorageLayer storage) {
8491
_storageLayer = storage;
8592
_manageSnapshotPath = Script.findScript("scripts/storage/qcow2/", "managesnapshot.sh");
@@ -98,28 +105,36 @@ public boolean createFolder(String uuid, String path) {
98105
@Override
99106
public KVMPhysicalDisk createDiskFromTemplateBacking(KVMPhysicalDisk template, String name, PhysicalDiskFormat format, long size,
100107
KVMStoragePool destPool, int timeout) {
101-
s_logger.info("Creating volume " + name + " with template backing " + template.getName() + " in pool " + destPool.getUuid() +
102-
" (" + destPool.getType().toString() + ") with size " + size);
108+
String volumeDesc = String.format("volume [%s], with template backing [%s], in pool [%s] (%s), with size [%s]", name, template.getName(), destPool.getUuid(),
109+
destPool.getType(), size);
103110

104-
KVMPhysicalDisk disk = null;
105-
String destPath = destPool.getLocalPath().endsWith("/") ?
106-
destPool.getLocalPath() + name :
107-
destPool.getLocalPath() + "/" + name;
111+
if (!poolTypesThatEnableCreateDiskFromTemplateBacking.contains(destPool.getType())) {
112+
s_logger.info(String.format("Skipping creation of %s due to pool type is none of the following types %s.", volumeDesc, poolTypesThatEnableCreateDiskFromTemplateBacking.stream()
113+
.map(type -> type.toString()).collect(Collectors.joining(", "))));
108114

109-
if (destPool.getType() == StoragePoolType.NetworkFilesystem) {
110-
try {
111-
if (format == PhysicalDiskFormat.QCOW2) {
112-
QemuImg qemu = new QemuImg(timeout);
113-
QemuImgFile destFile = new QemuImgFile(destPath, format);
114-
destFile.setSize(size);
115-
QemuImgFile backingFile = new QemuImgFile(template.getPath(), template.getFormat());
116-
qemu.create(destFile, backingFile);
117-
}
118-
} catch (QemuImgException e) {
119-
s_logger.error("Failed to create " + destPath + " due to a failed executing of qemu-img: " + e.getMessage());
120-
}
115+
return null;
121116
}
122-
return disk;
117+
118+
if (format != PhysicalDiskFormat.QCOW2) {
119+
s_logger.info(String.format("Skipping creation of %s due to format [%s] is not [%s].", volumeDesc, format, PhysicalDiskFormat.QCOW2));
120+
return null;
121+
}
122+
123+
s_logger.info(String.format("Creating %s.", volumeDesc));
124+
125+
String destPoolLocalPath = destPool.getLocalPath();
126+
String destPath = String.format("%s%s%s", destPoolLocalPath, destPoolLocalPath.endsWith("/") ? "" : "/", name);
127+
128+
try {
129+
QemuImgFile destFile = new QemuImgFile(destPath, format);
130+
destFile.setSize(size);
131+
QemuImgFile backingFile = new QemuImgFile(template.getPath(), template.getFormat());
132+
new QemuImg(timeout).create(destFile, backingFile);
133+
} catch (QemuImgException e) {
134+
s_logger.error(String.format("Failed to create %s in [%s] due to [%s].", volumeDesc, destPath, e.getMessage()), e);
135+
}
136+
137+
return null;
123138
}
124139

125140
/**

0 commit comments

Comments
 (0)