Skip to content

Skip stale destroyed volumes in local storage checks #13125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
andrijapanicsb wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Skip stale destroyed volumes in local storage checks #13125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a429813
Skip stale destroyed volumes in local storage checks
andrijapanicsb May 7, 2026
6122c74
generic solution
May 11, 2026
ea1712b
remove tests
May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public interface VolumeDao extends GenericDao<VolumeVO, Long>, StateDao<Volume.S

List<VolumeVO> findByInstance(long id);

List<VolumeVO> findByInstanceAndNotDestroyed(long id);

List<VolumeVO> findByInstanceAndType(long id, Volume.Type vType);

List<VolumeVO> findIncludingRemovedByInstanceAndType(long id, Volume.Type vType);
Expand Down
8 changes: 8 additions & 0 deletions engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ public List<VolumeVO> findByInstance(long id) {
return listBy(sc);
}

@Override
public List<VolumeVO> findByInstanceAndNotDestroyed(long id) {
SearchCriteria<VolumeVO> sc = AllFieldsSearch.create();
sc.setParameters("instanceId", id);
sc.setParameters("notDestroyed", Volume.State.Destroy, Volume.State.Expunged);
return listBy(sc);
}

@Override
public List<VolumeVO> findByInstanceAndDeviceId(long instanceId, long deviceId) {
SearchCriteria<VolumeVO> sc = AllFieldsSearch.create();
Expand Down
16 changes: 13 additions & 3 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@
}

@Override
public UserVm updateNicIpForVirtualMachine(UpdateVmNicIpCmd cmd) {

Check warning on line 1800 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 93 to 64, Complexity from 22 to 14, Nesting Level from 5 to 2, Number of Variables from 18 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCU&open=AZ4DWmx1_sdxmytL1HCU&pullRequest=13125
Long nicId = cmd.getNicId();
String ipaddr = cmd.getIpaddress();
Account caller = CallContext.current().getCallingAccount();
Expand Down Expand Up @@ -2072,7 +2072,7 @@
return upgradeRunningVirtualMachine(vmId, newServiceOfferingId, customParameters);
}

private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingId, Map<String, String> customParameters) throws ResourceUnavailableException,

Check warning on line 2075 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 116 to 64, Complexity from 25 to 14, Nesting Level from 5 to 2, Number of Variables from 35 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCS&open=AZ4DWmx1_sdxmytL1HCS&pullRequest=13125
ConcurrentOperationException, ManagementServerException, VirtualMachineMigrationException {

Account caller = CallContext.current().getCallingAccount();
Expand Down Expand Up @@ -2930,7 +2930,7 @@

@Override
@ActionEvent(eventType = EventTypes.EVENT_VM_UPDATE, eventDescription = "updating Vm")
public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException {

Check warning on line 2933 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 130 to 64, Complexity from 50 to 14, Nesting Level from 5 to 2, Number of Variables from 35 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCR&open=AZ4DWmx1_sdxmytL1HCR&pullRequest=13125
validateInputsAndPermissionForUpdateVirtualMachineCommand(cmd);

String displayName = cmd.getDisplayName();
Expand Down Expand Up @@ -4863,7 +4863,7 @@
return _instance + "-" + uuidName;
}

private UserVmVO commitUserVm(final boolean isImport, final DataCenter zone, final Host host, final Host lastHost, final VirtualMachineTemplate template, final String hostName, final String displayName, final Account owner,

Check warning on line 4866 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 115 to 64, Complexity from 30 to 14, Nesting Level from 3 to 2, Number of Variables from 56 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCX&open=AZ4DWmx1_sdxmytL1HCX&pullRequest=13125
final Long diskOfferingId, final Long diskSize, final String userData, Long userDataId, String userDataDetails, final Boolean isDisplayVm, final String keyboard,
final long accountId, final long userId, final ServiceOffering offering, final boolean isIso, final Long guestOsId, final String sshPublicKeys, final LinkedHashMap<String, List<NicProfile>> networkNicMap,
final long id, final String instanceName, final String uuidName, final HypervisorType hypervisorType, final Map<String, String> customParameters,
Expand Down Expand Up @@ -5554,7 +5554,7 @@
}

@Override
public boolean finalizeStart(VirtualMachineProfile profile, long hostId, Commands cmds, ReservationContext context) {

Check warning on line 5557 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 99 to 64, Complexity from 25 to 14, Nesting Level from 5 to 2, Number of Variables from 32 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCT&open=AZ4DWmx1_sdxmytL1HCT&pullRequest=13125
UserVmVO vm = _vmDao.findById(profile.getId());

Answer[] answersToCmds = cmds.getAnswers();
Expand Down Expand Up @@ -7225,7 +7225,7 @@
}

public boolean isVMUsingLocalStorage(VMInstanceVO vm) {
List<VolumeVO> volumes = _volsDao.findByInstance(vm.getId());
List<VolumeVO> volumes = _volsDao.findByInstanceAndNotDestroyed(vm.getId());
return isAnyVmVolumeUsingLocalStorage(volumes);
}

Expand Down Expand Up @@ -7490,7 +7490,7 @@
return null;
}

public void checkHostsDedication(VMInstanceVO vm, long srcHostId, long destHostId) {

Check warning on line 7493 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 84 to 64, Complexity from 28 to 14, Nesting Level from 5 to 2, Number of Variables from 23 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCV&open=AZ4DWmx1_sdxmytL1HCV&pullRequest=13125
HostVO srcHost = _hostDao.findById(srcHostId);
HostVO destHost = _hostDao.findById(destHostId);
boolean srcExplDedicated = checkIfHostIsDedicated(srcHost);
Expand Down Expand Up @@ -7661,10 +7661,20 @@
protected boolean isAnyVmVolumeUsingLocalStorage(final List<VolumeVO> volumes) {
for (VolumeVO vol : volumes) {
DiskOfferingVO diskOffering = _diskOfferingDao.findById(vol.getDiskOfferingId());
if (diskOffering.isUseLocalStorage()) {
if (diskOffering != null && diskOffering.isUseLocalStorage()) {
return true;
}
StoragePoolVO storagePool = _storagePoolDao.findById(vol.getPoolId());
Long poolId = vol.getPoolId();
Comment on lines 7663 to +7667
if (poolId == null) {
logger.debug("Skipping volume without storage pool while checking local storage usage: {}", vol);
continue;
}
StoragePoolVO storagePool = _storagePoolDao.findById(poolId);
if (storagePool == null || storagePool.getRemoved() != null) {
throw new CloudRuntimeException(String.format(
"Cannot determine local storage usage for active volume %s because storage pool ID %s is missing or removed",
vol, poolId));
}
if (storagePool.isLocal()) {
return true;
}
Expand Down Expand Up @@ -9276,7 +9286,7 @@
}
}

private void handleManagedStorage(UserVmVO vm, VolumeVO root) {

Check warning on line 9289 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 65 to 64, Complexity from 17 to 14, Nesting Level from 6 to 2, Number of Variables from 15 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWmx1_sdxmytL1HCY&open=AZ4DWmx1_sdxmytL1HCY&pullRequest=13125
if (Volume.State.Allocated.equals(root.getState())) {
return;
}
Expand Down
32 changes: 32 additions & 0 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,38 @@
}
}

@Test
public void testIsAnyVmVolumeUsingLocalStorageFailsForActiveVolumeWithMissingPool() {
VolumeVO volume = Mockito.mock(VolumeVO.class);
Mockito.when(volume.getDiskOfferingId()).thenReturn(1L);
Mockito.when(volume.getPoolId()).thenReturn(2L);
DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
Mockito.when(diskOfferingDao.findById(1L)).thenReturn(diskOffering);
Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false);
Mockito.when(primaryDataStoreDao.findById(2L)).thenReturn(null);

CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () ->

Check warning on line 1277 in server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor the code of the lambda to have only one invocation possibly throwing a runtime exception. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWm6H_sdxmytL1HCZ&open=AZ4DWm6H_sdxmytL1HCZ&pullRequest=13125
userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(Collections.singletonList(volume)));
Assert.assertTrue(exception.getMessage().contains("storage pool ID 2 is missing or removed"));
}

@Test
public void testIsAnyVmVolumeUsingLocalStorageFailsForActiveVolumeWithRemovedPool() {
VolumeVO volume = Mockito.mock(VolumeVO.class);
Mockito.when(volume.getDiskOfferingId()).thenReturn(1L);
Mockito.when(volume.getPoolId()).thenReturn(2L);
DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class);
Mockito.when(diskOfferingDao.findById(1L)).thenReturn(diskOffering);
Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false);
StoragePoolVO storagePool = Mockito.mock(StoragePoolVO.class);
Mockito.when(storagePool.getRemoved()).thenReturn(new Date());
Mockito.when(primaryDataStoreDao.findById(2L)).thenReturn(storagePool);

CloudRuntimeException exception = assertThrows(CloudRuntimeException.class, () ->

Check warning on line 1294 in server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor the code of the lambda to have only one invocation possibly throwing a runtime exception. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ4DWm6H_sdxmytL1HCa&open=AZ4DWm6H_sdxmytL1HCa&pullRequest=13125
userVmManagerImpl.isAnyVmVolumeUsingLocalStorage(Collections.singletonList(volume)));
Assert.assertTrue(exception.getMessage().contains("storage pool ID 2 is missing or removed"));
}

private List<VolumeVO> mockVolumesForIsAllVmVolumesOnZoneWideStore(int nullPoolIdVolumes, int nullPoolVolumes, int zoneVolumes, int nonZoneVolumes) {
List<VolumeVO> volumes = new ArrayList<>();
for (int i=0; i< nullPoolIdVolumes + nullPoolVolumes + zoneVolumes + nonZoneVolumes; ++i) {
Expand Down
Loading