Damans227 commented on code in PR #14497:
URL: https://github.com/apache/cloudstack/pull/14497#discussion_r4388376911
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1928,23 +1943,13 @@ public Volume destroyVolume(long volumeId, Account
caller, boolean expunge, bool
validateDestroyVolume(volume, caller, expunge, forceExpunge);
+ if (expunge && removeAllocatedVolumeWithoutStorage(volume,
enforceResourceLimitOnDisplayFalse)) {
Review Comment:
@nvazquez in the failed attach case, is the volume path saved before it
fails? if its still empty the delete thinks it was never created and only drops
the record, so the disk stays on the storage
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -1899,6 +1899,21 @@ private void removeVolume(long volumeId) {
}
}
+ /**
+ * Directly soft-deletes an {@link Volume.State#Allocated} volume that has
no backing storage pool and releases its
+ * resource count. Volumes with poolID are left to the normal
destroy/expunge flow
+ * @return true when the volume was handled here and no further destroy
processing is required.
+ */
+ protected boolean removeAllocatedVolumeWithoutStorage(VolumeVO volume,
Boolean enforceResourceLimitOnDisplayFalse) {
+ if (volume.getState() != Volume.State.Allocated || volume.getPoolId()
!= null) {
+ return false;
+ }
+ removeVolume(volume.getId());
Review Comment:
@nvazquez does this still write the usage delete record? the old code moved
the volume to destroyed, which is what records it. without that a volume
deleted before it was ever created stays billed
--
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]
