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