incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Devdeep Singh" <devdeep.si...@citrix.com>
Subject Re: Review Request: CLOUDSTACK-1156: Limit Primary and Secondary storage for domain/accounts
Date Tue, 12 Mar 2013 11:33:03 GMT

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



server/src/com/cloud/baremetal/BareMetalTemplateAdapter.java
<https://reviews.apache.org/r/9541/#comment37642>

    I see another file ./plugins/hypervisors/baremetal/src/com/cloud/baremetal/manager/BareMetalTemplateAdapter.java
present by the same name. Should the changes be made to this file instead? Can you confirm?



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

    Instead of adding routines for calculating secondary storage used by an account in volumeDao
and snapshotDao, would it make sense to keep them in ResourceManagerImpl itself? Same comment
for primaryStorageUsedForAccount in volumeDao



server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/9541/#comment37644>

    Should the check be made outside the validateVolume function, that is in uploadVolume?



server/src/com/cloud/storage/VolumeManagerImpl.java
<https://reviews.apache.org/r/9541/#comment37645>

    Can you put a comment here explaining why you are decrementing the count from secondary
storage. I understand it to to handle the scenario when the volume has been uploaded. Please
put that in a comments here.



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

    Why is the instanceid not null check needed?



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

    Similarly why is the instanceid is null check needed here?


- Devdeep Singh


On March 11, 2013, 8:15 a.m., Sanjay Tripathi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9541/
> -----------------------------------------------------------
> 
> (Updated March 11, 2013, 8:15 a.m.)
> 
> 
> Review request for cloudstack, Devdeep Singh, Nitin Mehta, Sateesh Chodapuneedi, mice
xia, 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/com/cloud/storage/VolumeApiService.java 8517988 
>   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/command/user/volume/ResizeVolumeCmd.java 955727a

>   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 fbfc955 
>   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 d902dc4 
>   server/src/com/cloud/configuration/Config.java 64465a2 
>   server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java 23c0796 
>   server/src/com/cloud/storage/VolumeManager.java af3cbbf 
>   server/src/com/cloud/storage/VolumeManagerImpl.java f0e6028 
>   server/src/com/cloud/storage/dao/SnapshotDao.java 0e378a7 
>   server/src/com/cloud/storage/dao/SnapshotDaoImpl.java 825b6d5 
>   server/src/com/cloud/storage/dao/VolumeDao.java d7a2667 
>   server/src/com/cloud/storage/dao/VolumeDaoImpl.java 40ed875 
>   server/src/com/cloud/storage/download/DownloadMonitorImpl.java 0bc89e3 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java bacca01 
>   server/src/com/cloud/template/HypervisorTemplateAdapter.java 1426421 
>   server/src/com/cloud/template/TemplateManagerImpl.java d843dbc 
>   server/src/com/cloud/vm/UserVmManagerImpl.java 6b2f762 
>   server/test/com/cloud/resourcelimit/ResourceLimitManagerImplTest.java d311ad3 
>   server/test/com/cloud/vpc/MockResourceLimitManagerImpl.java b9fc861 
>   setup/db/db/schema-40to410.sql b9bfe1a 
>   setup/db/db/schema-410to420.sql ca15bda 
>   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
> 
>


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