weizhouapache commented on a change in pull request #5687:
URL: https://github.com/apache/cloudstack/pull/5687#discussion_r754089656



##########
File path: 
server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
##########
@@ -521,13 +520,21 @@ public void checkResourceLimit(final Account account, 
final ResourceType type, l
         }
 
         final Project projectFinal = project;
+
+        // Check account limits. If it's unlimited then don't lock the db rows
+        long accountResourceLimit = 
findCorrectResourceLimitForAccount(account, type);
+        if (Resource.RESOURCE_UNLIMITED == accountResourceLimit) {

Review comment:
       @ravening 
   I think I do not need to explain you again. You know what I mean.
   Assume the domain resource limit is 20, account resource limit is 80. what 
is the max resource the account can use ? it is 20, not 80. -1 just means the 
account can use all resources of the domain.
   there is no special account in a domain, who can use real-unlimited resource 
(ignore the domain resource limit) and domain resource count does not take the 
account resource count into calculation. It is very strange. This change is 
unacceptable in my opinion.
   If you want to have such account, add it to `ROOT` domain or update the 
domain resource limit to -1.
   




-- 
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]


Reply via email to