Copilot commented on code in PR #13125:
URL: https://github.com/apache/cloudstack/pull/13125#discussion_r3203243175
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -7660,11 +7660,27 @@ private boolean isImplicitPlannerUsedByOffering(long
offeringId) {
protected boolean isAnyVmVolumeUsingLocalStorage(final List<VolumeVO>
volumes) {
for (VolumeVO vol : volumes) {
+ if (vol == null || vol.getRemoved() != null ||
+ Volume.State.Destroy.equals(vol.getState()) ||
+ Volume.State.Expunged.equals(vol.getState())) {
+ logger.debug("Skipping non-active volume while checking local
storage usage: {}", vol);
+ continue;
+ }
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();
Review Comment:
`_diskOfferingDao.findById(vol.getDiskOfferingId())` can return `null` for
disk offerings that have been removed (soft-deleted), even though existing
volumes may still reference them. In that case this method will skip the
`useLocalStorage` check and can change behavior (e.g., rely solely on the pool
lookup or throw for missing pools even when the disk offering would have been
sufficient to classify local storage). Consider using
`findByIdIncludingRemoved(...)` here and, if it still returns `null` for a
non-skipped (active) volume, fail with a clear exception similar to the
storage-pool check instead of silently continuing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
