This is an automated email from the ASF dual-hosted git repository. dahn pushed a commit to branch 4.20 in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 497266270b9f0a238f9712e9c8cd82a99024ff76 Author: Fabricio Duarte <[email protected]> AuthorDate: Sun Mar 8 13:25:27 2026 -0300 Cleanup imported VM from disk on failure due to volume allocation + prevent duplicate volume and primary storage increment on import --- .../service/VolumeOrchestrationService.java | 2 +- .../com/cloud/vm/VirtualMachineManagerImpl.java | 6 +++--- .../engine/orchestration/VolumeOrchestrator.java | 4 ++-- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 21 ++++++++++----------- .../cloudstack/vm/UnmanagedVMsManagerImplTest.java | 4 ++-- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 7950dda4d68..84e110a8940 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java @@ -107,7 +107,7 @@ public interface VolumeOrchestrationService { void destroyVolume(Volume volume); DiskProfile allocateRawVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template, - Account owner, Long deviceId); + Account owner, Long deviceId, boolean incrementResourceCount); VolumeInfo createVolumeOnPrimaryStorage(VirtualMachine vm, VolumeInfo volume, HypervisorType rootDiskHyperType, StoragePool storagePool) throws NoTransitionException; diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 2dcb8fa2059..74ff3801b6a 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -536,7 +536,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac if (dataDiskOfferings != null) { for (final DiskOfferingInfo dataDiskOfferingInfo : dataDiskOfferings) { volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId(), dataDiskOfferingInfo.getDiskOffering(), dataDiskOfferingInfo.getSize(), - dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, null); + dataDiskOfferingInfo.getMinIops(), dataDiskOfferingInfo.getMaxIops(), persistedVm, template, owner, null, true); } } if (datadiskTemplateToDiskOfferingMap != null && !datadiskTemplateToDiskOfferingMap.isEmpty()) { @@ -546,7 +546,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac long diskOfferingSize = diskOffering.getDiskSize() / (1024 * 1024 * 1024); VMTemplateVO dataDiskTemplate = _templateDao.findById(dataDiskTemplateToDiskOfferingMap.getKey()); volumeMgr.allocateRawVolume(Type.DATADISK, "DATA-" + persistedVm.getId() + "-" + String.valueOf(diskNumber), diskOffering, diskOfferingSize, null, null, - persistedVm, dataDiskTemplate, owner, Long.valueOf(diskNumber)); + persistedVm, dataDiskTemplate, owner, Long.valueOf(diskNumber), true); diskNumber++; } } @@ -576,7 +576,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac String rootVolumeName = String.format("ROOT-%s", vm.getId()); if (template.getFormat() == ImageFormat.ISO) { volumeMgr.allocateRawVolume(Type.ROOT, rootVolumeName, rootDiskOfferingInfo.getDiskOffering(), rootDiskOfferingInfo.getSize(), - rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), vm, template, owner, null); + rootDiskOfferingInfo.getMinIops(), rootDiskOfferingInfo.getMaxIops(), vm, template, owner, null, true); } else if (template.getFormat() == ImageFormat.BAREMETAL) { logger.debug("%s has format [{}]. Skipping ROOT volume [{}] allocation.", template.toString(), ImageFormat.BAREMETAL, rootVolumeName); } else { diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 0fc61d81588..5eef84e354c 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -835,7 +835,7 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati @ActionEvent(eventType = EventTypes.EVENT_VOLUME_CREATE, eventDescription = "creating volume", create = true) @Override public DiskProfile allocateRawVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template, Account owner, - Long deviceId) { + Long deviceId, boolean incrementResourceCount) { if (size == null) { size = offering.getDiskSize(); } else { @@ -874,7 +874,7 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati saveVolumeDetails(offering.getId(), vol.getId()); // Save usage event and update resource count for user vm volumes - if (vm.getType() == VirtualMachine.Type.User) { + if (vm.getType() == VirtualMachine.Type.User && incrementResourceCount) { UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, vol.getAccountId(), vol.getDataCenterId(), vol.getId(), vol.getName(), offering.getId(), null, size, Volume.class.getName(), vol.getUuid(), vol.isDisplayVolume()); _resourceLimitMgr.incrementVolumeResourceCount(vm.getAccountId(), vol.isDisplayVolume(), vol.getSize(), offering); diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 1917804fb8d..a32459ed059 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -2489,13 +2489,13 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import vm name: %s", instanceName)); } String rootVolumeName = String.format("ROOT-%s", userVm.getId()); - DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null); + DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null, false); DiskProfile[] dataDiskProfiles = new DiskProfile[dataDisks.size()]; int diskSeq = 0; for (UnmanagedInstanceTO.Disk disk : dataDisks) { DiskOffering offering = diskOfferingDao.findById(dataDiskOfferingMap.get(disk.getDiskId())); - DiskProfile dataDiskProfile = volumeManager.allocateRawVolume(Volume.Type.DATADISK, String.format("DATA-%d-%s", userVm.getId(), disk.getDiskId()), offering, null, null, null, userVm, template, owner, null); + 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); dataDiskProfiles[diskSeq++] = dataDiskProfile; } @@ -2653,7 +2653,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { reservations.add(volumeReservation); String rootVolumeName = String.format("ROOT-%s", userVm.getId()); - DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null); + DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null, false); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId()); @@ -2696,14 +2696,10 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { throw new CloudRuntimeException("Disk not found or is invalid"); } diskProfile.setSize(checkVolumeAnswer.getSize()); - try { - CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags, - CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService); - reservations.add(primaryStorageReservation); - } catch (ResourceAllocationException e) { - cleanupFailedImportVM(userVm); - throw e; - } + + CheckedReservation primaryStorageReservation = new CheckedReservation(owner, Resource.ResourceType.primary_storage, resourceLimitStorageTags, + CollectionUtils.isNotEmpty(resourceLimitStorageTags) ? diskProfile.getSize() : 0L, reservationDao, resourceLimitService); + reservations.add(primaryStorageReservation); List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>(); try { @@ -2724,6 +2720,9 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager { publishVMUsageUpdateResourceCount(userVm, dummyOffering, template); return userVm; + } catch (ResourceAllocationException e) { + cleanupFailedImportVM(userVm); + throw e; } finally { ReservationHelper.closeAll(reservations); } diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 282f3fb3a70..d98eebebd62 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -508,7 +508,7 @@ public class UnmanagedVMsManagerImplTest { DeployDestination mockDest = Mockito.mock(DeployDestination.class); when(deploymentPlanningManager.planDeployment(any(), any(), any(), any())).thenReturn(mockDest); DiskProfile diskProfile = Mockito.mock(DiskProfile.class); - when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(diskProfile); Map<Volume, StoragePool> storage = new HashMap<>(); VolumeVO volume = Mockito.mock(VolumeVO.class); @@ -743,7 +743,7 @@ public class UnmanagedVMsManagerImplTest { DeployDestination mockDest = Mockito.mock(DeployDestination.class); when(deploymentPlanningManager.planDeployment(any(), any(), any(), any())).thenReturn(mockDest); DiskProfile diskProfile = Mockito.mock(DiskProfile.class); - when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) + when(volumeManager.allocateRawVolume(any(), any(), any(), any(), any(), any(), any(), any(), any(), any(), any())) .thenReturn(diskProfile); Map<Volume, StoragePool> storage = new HashMap<>(); VolumeVO volume = Mockito.mock(VolumeVO.class);
