Skip to content

Commit 4972662

Browse files
winterhazelDaan Hoogland
winterhazel
authored and
Daan Hoogland
committed
Cleanup imported VM from disk on failure due to volume allocation + prevent duplicate volume and primary storage increment on import
1 parent 0a4b4c6 commit 4972662

5 files changed

Lines changed: 18 additions & 19 deletions

File tree

‎engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ VolumeInfo moveVolume(VolumeInfo volume, long destPoolDcId, Long destPoolPodId,
107107
void destroyVolume(Volume volume);
108108

109109
DiskProfile allocateRawVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template,
110-
Account owner, Long deviceId);
110+
Account owner, Long deviceId, boolean incrementResourceCount);
111111

112112
VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, VolumeInfo volume, HypervisorType rootDiskHyperType, StoragePool storagePool) throws NoTransitionException;
113113

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ public void allocate(final String vmInstanceName, final VirtualMachineTemplate t
536536
if (dataDiskOfferings != null) {
537537
for (final DiskOfferingInfo dataDiskOfferingInfo : dataDiskOfferings) {
538538
volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId(), dataDiskOfferingInfo.getDiskOffering(), dataDiskOfferingInfo.getSize(),
539-
dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, null);
539+
dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, null, true);
540540
}
541541
}
542542
if (datadiskTemplateToDiskOfferingMap != null && !datadiskTemplateToDiskOfferingMap.isEmpty()) {
@@ -546,7 +546,7 @@ public void allocate(final String vmInstanceName, final VirtualMachineTemplate t
546546
long diskOfferingSize = diskOffering.getDiskSize() / (1024 * 1024 * 1024);
547547
VMTemplateVO dataDiskTemplate = _templateDao.findById(dataDiskTemplateToDiskOfferingMap.getKey());
548548
volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId() + "-" + String.valueOf(diskNumber), diskOffering, diskOfferingSize, null, null,
549-
persistedVm, dataDiskTemplate, owner, Long.valueOf(diskNumber));
549+
persistedVm, dataDiskTemplate, owner, Long.valueOf(diskNumber), true);
550550
diskNumber++;
551551
}
552552
}
@@ -576,7 +576,7 @@ private void allocateRootVolume(VMInstanceVO vm, VirtualMachineTemplate template
576576
String rootVolumeName = String.format("ROOT-%s", vm.getId());
577577
if (template.getFormat() == ImageFormat.ISO) {
578578
volumeMgr.allocateRawVolume(Type.ROOT, rootVolumeName, rootDiskOfferingInfo.getDiskOffering(), rootDiskOfferingInfo.getSize(),
579-
rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), vm, template, owner, null);
579+
rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), vm, template, owner, null, true);
580580
} else if (template.getFormat() == ImageFormat.BAREMETAL) {
581581
logger.debug("%s has format [{}]. Skipping ROOT volume [{}] allocation.", template.toString(), ImageFormat.BAREMETAL, rootVolumeName);
582582
} else {

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,7 @@ protected DiskProfile toDiskProfile(Volume vol, DiskOffering offering) {
835835
@ActionEvent(eventType = EventTypes.EVENT_VOLUME_CREATE, eventDescription = "creating volume", create = true)
836836
@Override
837837
public DiskProfile allocateRawVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template, Account owner,
838-
Long deviceId) {
838+
Long deviceId, boolean incrementResourceCount) {
839839
if (size == null) {
840840
size = offering.getDiskSize();
841841
} else {
@@ -874,7 +874,7 @@ public DiskProfile allocateRawVolume(Type type, String name, DiskOffering offeri
874874
saveVolumeDetails(offering.getId(), vol.getId());
875875

876876
// Save usage event and update resource count for user vm volumes
877-
if (vm.getType() == VirtualMachine.Type.User) {
877+
if (vm.getType() == VirtualMachine.Type.User && incrementResourceCount) {
878878
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, vol.getAccountId(), vol.getDataCenterId(), vol.getId(), vol.getName(), offering.getId(), null, size,
879879
Volume.class.getName(), vol.getUuid(), vol.isDisplayVolume());
880880
_resourceLimitMgr.incrementVolumeResourceCount(vm.getAccountId(), vol.isDisplayVolume(), vol.getSize(), offering);

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2489,13 +2489,13 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag
24892489
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName));
24902490
}
24912491
String rootVolumeName = String.format("ROOT-%s", userVm.getId());
2492-
DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null);
2492+
DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null, false);
24932493

24942494
DiskProfile[] dataDiskProfiles = new DiskProfile[dataDisks.size()];
24952495
int diskSeq = 0;
24962496
for (UnmanagedInstanceTO.Disk disk : dataDisks) {
24972497
DiskOffering offering = diskOfferingDao.findById(dataDiskOfferingMap.get(disk.getDiskId()));
2498-
DiskProfile dataDiskProfile = volumeManager.allocateRawVolume(Volume.Type.DATADISK, String.format("DATA-%d-%s", userVm.getId(), disk.getDiskId()), offering, null, null, null, userVm, template, owner, null);
2498+
DiskProfile dataDiskProfile = volumeManager.allocateRawVolume(Volume.Type.DATADISK, String.format("DATA-%d-%s", userVm.getId(), disk.getDiskId()), offering, null, null, null, userVm, template, owner, null, false);
24992499
dataDiskProfiles[diskSeq++] = dataDiskProfile;
25002500
}
25012501

@@ -2653,7 +2653,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
26532653
reservations.add(volumeReservation);
26542654

26552655
String rootVolumeName = String.format("ROOT-%s", userVm.getId());
2656-
DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null);
2656+
DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null, false);
26572657

26582658
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null);
26592659
ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId());
@@ -2696,14 +2696,10 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
26962696
throw new CloudRuntimeException("Disk not found or is invalid");
26972697
}
26982698
diskProfile.setSize(checkVolumeAnswer.getSize());
2699-
try {
2700-
CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags,
2701-
CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService);
2702-
reservations.add(primaryStorageReservation);
2703-
} catch (ResourceAllocationException e) {
2704-
cleanupFailedImportVM(userVm);
2705-
throw e;
2706-
}
2699+
2700+
CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags,
2701+
CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService);
2702+
reservations.add(primaryStorageReservation);
27072703

27082704
List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>();
27092705
try {
@@ -2724,6 +2720,9 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
27242720
publishVMUsageUpdateResourceCount(userVm, dummyOffering, template);
27252721
return userVm;
27262722

2723+
} catch (ResourceAllocationException e) {
2724+
cleanupFailedImportVM(userVm);
2725+
throw e;
27272726
} finally {
27282727
ReservationHelper.closeAll(reservations);
27292728
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ public void testImportFromExternalTest() throws InsufficientServerCapacityExcept
508508
DeployDestination mockDest = Mockito.mock(DeployDestination.class);
509509
when(deploymentPlanningManager.planDeployment(any(), any(), any(), any())).thenReturn(mockDest);
510510
DiskProfile diskProfile = Mockito.mock(DiskProfile.class);
511-
when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()))
511+
when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()))
512512
.thenReturn(diskProfile);
513513
Map<Volume, StoragePool> storage = new HashMap<>();
514514
VolumeVO volume = Mockito.mock(VolumeVO.class);
@@ -743,7 +743,7 @@ private void importFromDisk(String source) throws InsufficientServerCapacityExce
743743
DeployDestination mockDest = Mockito.mock(DeployDestination.class);
744744
when(deploymentPlanningManager.planDeployment(any(), any(), any(), any())).thenReturn(mockDest);
745745
DiskProfile diskProfile = Mockito.mock(DiskProfile.class);
746-
when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()))
746+
when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any()))
747747
.thenReturn(diskProfile);
748748
Map<Volume, StoragePool> storage = new HashMap<>();
749749
VolumeVO volume = Mockito.mock(VolumeVO.class);

0 commit comments

Comments
 (0)