Skip to content

Commit e2d790c

Browse files
rajujithrajujith
committed
Added support for skipping volume backing when importing unmanaged volumes and VMs. This allows users to import volumes and VMs without creating a backing volume, which can be useful in certain scenarios where the backing volume is not needed or desired.
1 parent 5caf6cd commit e2d790c

File tree

4 files changed

+73
-5
lines changed

4 files changed

+73
-5
lines changed

‎server/src/main/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImpl.java‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@
7373
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
7474
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
7575
import org.apache.commons.collections.CollectionUtils;
76+
import org.apache.cloudstack.framework.config.ConfigKey;
77+
import org.apache.cloudstack.framework.config.Configurable;
7678
import org.apache.commons.lang3.StringUtils;
7779
import org.apache.logging.log4j.LogManager;
7880
import org.apache.logging.log4j.Logger;
@@ -84,7 +86,7 @@
8486
import java.util.List;
8587
import java.util.Map;
8688

87-
public class VolumeImportUnmanageManagerImpl implements VolumeImportUnmanageService {
89+
public class VolumeImportUnmanageManagerImpl implements VolumeImportUnmanageService, Configurable {
8890
protected Logger logger = LogManager.getLogger(VolumeImportUnmanageManagerImpl.class);
8991

9092
@Inject
@@ -123,6 +125,15 @@ public class VolumeImportUnmanageManagerImpl implements VolumeImportUnmanageServ
123125
static final String DISK_OFFERING_NAME_SUFFIX_LOCAL = " - Local Storage";
124126
static final String DISK_OFFERING_UNIQUE_NAME_SUFFIX_LOCAL = "-Local";
125127

128+
ConfigKey<Boolean> AllowImportVolumeWithBackingFile = new ConfigKey<>(Boolean.class,
129+
"allow.import.volume.with.backing.file",
130+
"Advanced",
131+
"false",
132+
"If enabled, allows QCOW2 volumes with backing files to be imported or unmanaged",
133+
true,
134+
ConfigKey.Scope.Global,
135+
null);
136+
126137
protected void logFailureAndThrowException(String msg) {
127138
logger.error(msg);
128139
throw new CloudRuntimeException(msg);
@@ -394,7 +405,7 @@ protected void checkIfVolumeHasBackingFile(VolumeOnStorageTO volume) {
394405
Map<VolumeOnStorageTO.Detail, String> volumeDetails = volume.getDetails();
395406
if (volumeDetails != null && volumeDetails.containsKey(VolumeOnStorageTO.Detail.BACKING_FILE)) {
396407
String backingFile = volumeDetails.get(VolumeOnStorageTO.Detail.BACKING_FILE);
397-
if (StringUtils.isNotBlank(backingFile)) {
408+
if (StringUtils.isNotBlank(backingFile) && !AllowImportVolumeWithBackingFile.value()) {
398409
logFailureAndThrowException("Volume with backing file cannot be imported or unmanaged.");
399410
}
400411
}
@@ -513,4 +524,14 @@ private void unmanageVolumeFromDatabase(VolumeVO volume) {
513524
volume.setRemoved(new Date());
514525
volumeDao.update(volume.getId(), volume);
515526
}
527+
528+
@Override
529+
public String getConfigComponentName() {
530+
return VolumeImportUnmanageManagerImpl.class.getSimpleName();
531+
}
532+
533+
@Override
534+
public ConfigKey<?>[] getConfigKeys() {
535+
return new ConfigKey<?>[]{ AllowImportVolumeWithBackingFile };
536+
}
516537
}

‎server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,15 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
220220
ConfigKey.Scope.Global,
221221
null);
222222

223+
ConfigKey<Boolean> AllowImportVolumeWithBackingFile = new ConfigKey<>(Boolean.class,
224+
"allow.import.volume.with.backing.file",
225+
"Advanced",
226+
"false",
227+
"If enabled, allows QCOW2 volumes with backing files to be imported or unmanaged",
228+
true,
229+
ConfigKey.Scope.Global,
230+
null);
231+
223232
ConfigKey<String> ConvertVmwareInstanceToKvmExtraParamsAllowedList = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED,
224233
String.class,
225234
"convert.vmware.instance.to.kvm.extra.params.allowed.list",
@@ -2889,7 +2898,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
28892898
return userVm;
28902899
}
28912900

2892-
private void checkVolume(Map<VolumeOnStorageTO.Detail, String> volumeDetails) {
2901+
protected void checkVolume(Map<VolumeOnStorageTO.Detail, String> volumeDetails) {
28932902
if (MapUtils.isEmpty(volumeDetails)) {
28942903
return;
28952904
}
@@ -2908,7 +2917,7 @@ private void checkVolume(Map<VolumeOnStorageTO.Detail, String> volumeDetails) {
29082917
}
29092918
if (volumeDetails.containsKey(VolumeOnStorageTO.Detail.BACKING_FILE)) {
29102919
String backingFile = volumeDetails.get(VolumeOnStorageTO.Detail.BACKING_FILE);
2911-
if (StringUtils.isNotBlank(backingFile)) {
2920+
if (StringUtils.isNotBlank(backingFile) && !AllowImportVolumeWithBackingFile.value()) {
29122921
logFailureAndThrowException("Volume with backing file cannot be imported or unmanaged.");
29132922
}
29142923
}
@@ -3037,7 +3046,8 @@ public ConfigKey<?>[] getConfigKeys() {
30373046
ThreadsOnMSToImportVMwareVMFiles,
30383047
ThreadsOnKVMHostToImportVMwareVMFiles,
30393048
ConvertVmwareInstanceToKvmExtraParamsAllowed,
3040-
ConvertVmwareInstanceToKvmExtraParamsAllowedList
3049+
ConvertVmwareInstanceToKvmExtraParamsAllowedList,
3050+
AllowImportVolumeWithBackingFile
30413051
};
30423052
}
30433053
}

‎server/src/test/java/org/apache/cloudstack/storage/volume/VolumeImportUnmanageManagerImplTest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import org.apache.cloudstack.api.response.VolumeForImportResponse;
6262
import org.apache.cloudstack.api.response.VolumeResponse;
6363
import org.apache.cloudstack.context.CallContext;
64+
import org.apache.cloudstack.framework.config.ConfigKey;
6465
import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
6566
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
6667
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
@@ -134,6 +135,8 @@ public class VolumeImportUnmanageManagerImplTest {
134135
private VolumeApiService volumeApiService;
135136
@Mock
136137
private SnapshotDataStoreDao snapshotDataStoreDao;
138+
@Mock
139+
private ConfigKey<Boolean> mockAllowImportVolumeWithBackingFile;
137140

138141
@Mock
139142
StoragePoolVO storagePoolVO;
@@ -343,6 +346,8 @@ public void testCheckIfVolumeIsEncrypted() {
343346

344347
@Test
345348
public void testCheckIfVolumeHasBackingFile() {
349+
volumeImportUnmanageManager.AllowImportVolumeWithBackingFile = mockAllowImportVolumeWithBackingFile;
350+
Mockito.when(mockAllowImportVolumeWithBackingFile.value()).thenReturn(false);
346351
try {
347352
VolumeOnStorageTO volumeOnStorageTO = new VolumeOnStorageTO(hypervisorType, path, name, fullPath,
348353
format, size, virtualSize);
@@ -356,6 +361,17 @@ public void testCheckIfVolumeHasBackingFile() {
356361
}
357362
}
358363

364+
@Test
365+
public void testCheckIfVolumeHasBackingFileAllowEnabled() {
366+
volumeImportUnmanageManager.AllowImportVolumeWithBackingFile = mockAllowImportVolumeWithBackingFile;
367+
Mockito.when(mockAllowImportVolumeWithBackingFile.value()).thenReturn(true);
368+
VolumeOnStorageTO volumeOnStorageTO = new VolumeOnStorageTO(hypervisorType, path, name, fullPath,
369+
format, size, virtualSize);
370+
volumeOnStorageTO.addDetail(VolumeOnStorageTO.Detail.BACKING_FILE, BACKING_FILE);
371+
volumeOnStorageTO.addDetail(VolumeOnStorageTO.Detail.BACKING_FILE_FORMAT, BACKING_FILE_FORMAT);
372+
volumeImportUnmanageManager.checkIfVolumeHasBackingFile(volumeOnStorageTO);
373+
}
374+
359375
@Test
360376
public void testUnmanageVolume() {
361377
when(volumeVO.getState()).thenReturn(Volume.State.Ready);

‎server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@
171171
import com.cloud.vm.dao.UserVmDao;
172172
import com.cloud.vm.dao.VMInstanceDao;
173173
import com.cloud.vm.dao.VMInstanceDetailsDao;
174+
import org.apache.cloudstack.storage.volume.VolumeOnStorageTO;
174175

175176
@RunWith(MockitoJUnitRunner.class)
176177
public class UnmanagedVMsManagerImplTest {
@@ -257,6 +258,8 @@ public class UnmanagedVMsManagerImplTest {
257258
private ConfigKey<Boolean> configKeyMockParamsAllowed;
258259
@Mock
259260
private ConfigKey<String> configKeyMockParamsAllowedList;
261+
@Mock
262+
private ConfigKey<Boolean> configKeyMockAllowBackingFile;
260263

261264
private static final long virtualMachineId = 1L;
262265

@@ -1351,4 +1354,22 @@ public void testAddServiceOfferingDetailsToParamsCustomUnconstrainedOffering() {
13511354
Assert.assertFalse(params.containsKey(VmDetailConstants.CPU_SPEED));
13521355
Assert.assertFalse(params.containsKey(VmDetailConstants.MEMORY));
13531356
}
1357+
1358+
@Test(expected = CloudRuntimeException.class)
1359+
public void testCheckVolumeWithBackingFileSettingDisabled() {
1360+
unmanagedVMsManager.AllowImportVolumeWithBackingFile = configKeyMockAllowBackingFile;
1361+
Mockito.when(configKeyMockAllowBackingFile.value()).thenReturn(false);
1362+
Map<VolumeOnStorageTO.Detail, String> details = new HashMap<>();
1363+
details.put(VolumeOnStorageTO.Detail.BACKING_FILE, "backing.qcow2");
1364+
unmanagedVMsManager.checkVolume(details);
1365+
}
1366+
1367+
@Test
1368+
public void testCheckVolumeWithBackingFileSettingEnabled() {
1369+
unmanagedVMsManager.AllowImportVolumeWithBackingFile = configKeyMockAllowBackingFile;
1370+
Mockito.when(configKeyMockAllowBackingFile.value()).thenReturn(true);
1371+
Map<VolumeOnStorageTO.Detail, String> details = new HashMap<>();
1372+
details.put(VolumeOnStorageTO.Detail.BACKING_FILE, "backing.qcow2");
1373+
unmanagedVMsManager.checkVolume(details);
1374+
}
13541375
}

0 commit comments

Comments
 (0)