incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chiradeep Vittal" <chirade...@gmail.com>
Subject Re: Review Request: CLOUDSTACK-713 Limit Resources to domain/accounts
Date Tue, 29 Jan 2013 20:56:55 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9110/#review15804
-----------------------------------------------------------



server/src/com/cloud/baremetal/BareMetalVmManagerImpl.java
<https://reviews.apache.org/r/9110/#comment34020>

    There seems to be a lot of duplication of _resourceLimitMgr.checkResourceLimit(), increment
and decrement. Can these be made generic?



server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
<https://reviews.apache.org/r/9110/#comment34015>

    You can use @Ignore("Explanation") if you need the test to be ignored during mvn build



server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
<https://reviews.apache.org/r/9110/#comment34018>

    How about testing whether you can deploy a VM when the limits are exceeded?



server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
<https://reviews.apache.org/r/9110/#comment34019>

    How about testing your DAOs?



server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
<https://reviews.apache.org/r/9110/#comment34014>

    Why are these commented out? They will not run  any tests?



server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
<https://reviews.apache.org/r/9110/#comment34016>

    You have to use junit Assert utilities to assert failure/success.



server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java
<https://reviews.apache.org/r/9110/#comment34017>

    Use Assert


- Chiradeep Vittal


On Jan. 29, 2013, 12:26 p.m., Sanjay Tripathi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9110/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2013, 12:26 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, and Min Chen.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-713: Limit Resources to domain/accountsi(CPU and RAM)
>     
>     Addition of two new resource types i.e. CPU and RAM in the existing pool of
>     resource types.
>     Added some methods to set the limits on these resources using updateResource
>     API command and to get a count using updateResourceCount. Also added calls in
>     Virtual machine life cycle to check these limits and to increment/decrement 
>     resource count
>     
>     Resource Name  :: Resource type number
>         CPU               8
>         RAM               9
> 
> 
> This addresses bug CLOUDSTACK-713.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/configuration/Resource.java 7f551d6 
>   api/src/org/apache/cloudstack/api/command/user/resource/UpdateResourceCountCmd.java
4aa694b 
>   api/src/org/apache/cloudstack/api/command/user/resource/UpdateResourceLimitCmd.java
9b6359f 
>   api/src/org/apache/cloudstack/api/response/AccountResponse.java 51d3352 
>   api/src/org/apache/cloudstack/api/response/ResourceCountResponse.java 7a29194 
>   api/src/org/apache/cloudstack/api/response/ResourceLimitResponse.java c01e12f 
>   server/src/com/cloud/api/query/dao/AccountJoinDaoImpl.java 6268724 
>   server/src/com/cloud/api/query/vo/AccountJoinVO.java 6d37f4d 
>   server/src/com/cloud/baremetal/BareMetalVmManagerImpl.java 57cfb39 
>   server/src/com/cloud/configuration/Config.java 4ae144e 
>   server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java c17b0ea 
>   server/src/com/cloud/vm/UserVmManagerImpl.java ecf1242 
>   server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java PRE-CREATION

>   server/test/com/cloud/vpc/MockResourceLimitManagerImpl.java a0c7b70 
>   setup/db/create-schema-view.sql f68a6ca 
>   setup/db/db/schema-40to410.sql ed4946e 
> 
> Diff: https://reviews.apache.org/r/9110/diff/
> 
> 
> Testing
> -------
> 
> Manually tested on my local CloudStack setup. Also added unit tests file in the patch.
> 
> 
> Thanks,
> 
> Sanjay Tripathi
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message