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



server/src/com/cloud/api/ApiResponseHelper.java
<https://reviews.apache.org/r/9541/#comment35773>

    Good to take ceiling value to roundoff this division operation. Or do +1 to 
result. 



server/src/com/cloud/api/query/dao/AccountJoinDaoImpl.java
<https://reviews.apache.org/r/9541/#comment35774>

    Need to see if the division result might cause skew in calculations 
involving these variables.



server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
<https://reviews.apache.org/r/9541/#comment35775>

    Why is the dao object protected?



server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
<https://reviews.apache.org/r/9541/#comment35776>

    change variable name to start with small case letter.



server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
<https://reviews.apache.org/r/9541/#comment35777>

    Good amount of comman code exists between public long 
findCorrectResourceLimitForAccount(Account account, ResourceType type) and 
public long findCorrectResourceLimitForAccount(short accountType, Long limit, 
ResourceType type). Is this something we can optimize?



server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java
<https://reviews.apache.org/r/9541/#comment35778>

    This statement is not required as default value would suffice.



server/src/com/cloud/storage/dao/VolumeDao.java
<https://reviews.apache.org/r/9541/#comment35779>

    What is the unit of storage space here? bytes of Gib. Please update the 
comment to bring more clarity.



server/src/com/cloud/storage/dao/VolumeDao.java
<https://reviews.apache.org/r/9541/#comment35780>

    Specify storage units here.



server/src/com/cloud/vm/UserVmManagerImpl.java
<https://reviews.apache.org/r/9541/#comment35781>

    how about snapshot value null here?



utils/src/com/cloud/utils/UriUtils.java
<https://reviews.apache.org/r/9541/#comment35782>

    Better to add finally block with connection cleanup code.


Would like to know if there are upgrade scenarios considered.

- Sateesh Chodapuneedi


On Feb. 21, 2013, 1:20 p.m., Sanjay Tripathi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9541/
> -----------------------------------------------------------
> 
> (Updated Feb. 21, 2013, 1:20 p.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh and Min Chen.
> 
> 
> Description
> -------
> 
> CLOUDSTACK-1156: Limit Primary and Secondary storage for domain/accounts
>     
>     Addition of two new resource types i.e. Primary and Secondary storage 
> space in the existing pool of
>     resource types.
>     Added methods to set the limits on these resources using 
> updateResourceLimit
>     API command and to get a count using updateResourceCount. Also added 
> calls in the
>     Templates, Volumes, Snapshots life cycle to check these limits and to 
> increment/decrement the new
>     resource types
>     
>     Resource Name          :: Resource type number
>         Primary Storage               10
>         Secondary Storage             11
>     
>     Also added jUnit Tests for the same.
> 
> 
> This addresses bug CLOUDSTACK-1156.
> 
> 
> Diffs
> -----
> 
>   api/src/com/cloud/configuration/Resource.java 7614c8a 
>   
> api/src/org/apache/cloudstack/api/command/user/resource/UpdateResourceCountCmd.java
>  f6d3a98 
>   
> api/src/org/apache/cloudstack/api/command/user/resource/UpdateResourceLimitCmd.java
>  0039f62 
>   api/src/org/apache/cloudstack/api/response/AccountResponse.java 9a98a35 
>   api/src/org/apache/cloudstack/api/response/ResourceCountResponse.java 
> a7fbbf2 
>   api/src/org/apache/cloudstack/api/response/ResourceLimitResponse.java 
> b444e7a 
>   server/src/com/cloud/api/ApiResponseHelper.java a94e935 
>   server/src/com/cloud/api/query/dao/AccountJoinDaoImpl.java 898bafc 
>   server/src/com/cloud/api/query/vo/AccountJoinVO.java cd7231c 
>   server/src/com/cloud/baremetal/BareMetalTemplateAdapter.java 4440b7a 
>   server/src/com/cloud/baremetal/BareMetalVmManagerImpl.java 5de5ccd 
>   server/src/com/cloud/configuration/Config.java c0c23b6 
>   server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java 7ff06af 
>   server/src/com/cloud/storage/StorageManagerImpl.java 05e0cfe 
>   server/src/com/cloud/storage/dao/SnapshotDao.java 3b961f6 
>   server/src/com/cloud/storage/dao/SnapshotDaoImpl.java a8a07dc 
>   server/src/com/cloud/storage/dao/VolumeDao.java d7a2667 
>   server/src/com/cloud/storage/dao/VolumeDaoImpl.java a189d00 
>   server/src/com/cloud/storage/download/DownloadMonitorImpl.java 6d3cf2a 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java e06da75 
>   server/src/com/cloud/template/HyervisorTemplateAdapter.java fe6bc2a 
>   server/src/com/cloud/template/TemplateAdapterBase.java fa677ac 
>   server/src/com/cloud/template/TemplateManagerImpl.java f9cf277 
>   server/src/com/cloud/vm/UserVmManagerImpl.java df97609 
>   server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java 
> d311ad3 
>   server/test/com/cloud/vpc/MockResourceLimitManagerImpl.java b9fc861 
>   setup/db/db/schema-40to410.sql 9a59318 
>   setup/db/db/schema-410to420.sql 65add75 
>   utils/src/com/cloud/utils/UriUtils.java a8b5ccb 
> 
> Diff: https://reviews.apache.org/r/9541/diff/
> 
> 
> Testing
> -------
> 
> Tested life cycle of templates, volumes, snapshots, vm on my local CloudStack 
> setup.
> 
> 
> Thanks,
> 
> Sanjay Tripathi
> 
>

Reply via email to