Damans227 commented on code in PR #14137:
URL: https://github.com/apache/cloudstack/pull/14137#discussion_r4259225435
##########
server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java:
##########
@@ -455,8 +462,10 @@ public VMSnapshot allocVMSnapshot(Long vmId, String
vsDisplayName, String vsDesc
}
try {
+ resourceLimitMgr.incrementResourceCount(caller.getAccountId(),
Resource.ResourceType.instance_snapshot);
Review Comment:
should this count go on the owner of the instance instead of the caller?
when an admin snapshots a users instance the admins count goes up but the limit
is checked on the user
##########
server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java:
##########
@@ -700,10 +711,13 @@ else if (jobResult instanceof Throwable)
}
if (jobResult instanceof Boolean)
- return ((Boolean)jobResult).booleanValue();
+ success = ((Boolean)jobResult).booleanValue();
+ }
- return false;
+ if (success) {
+ resourceLimitMgr.decrementResourceCount(caller.getAccountId(),
Resource.ResourceType.instance_snapshot);
Review Comment:
what happens when an instance is destroyed with snapshots still on it? that
path removes them without lowering the count, so it only goes up
--
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]
