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



##########
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 @weizhouapache is right; If you have unlimited 
resources that would be still limited to the hierarchy above you, mainly your 
domain/project or what ever you are in. There is also a posibility that it 
would be limited by any domain above it, and in the end by the capacity of the 
cloud of course. A domain limit would otherwise not be useful at all.
   say Dom1 has a limit of 10 and has two subdomains Dom2 and Dom3 which each 
have a limit of 6. Now if Dom2 uses all 6, Dom3 has an effective limit of 4 
until Dom2 releases some. I think the same should be true for unlimited.
   I thought `findCorrectResourceLimitForAccount()` already took care of that, 
but it seems it doesn't.  I am also sure we can find more bugs in this respect 
somewhere ;)




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