> On March 12, 2013, 11:33 a.m., Devdeep Singh wrote: > > server/src/com/cloud/baremetal/BareMetalTemplateAdapter.java, line 43 > > <https://reviews.apache.org/r/9541/diff/7/?file=269109#file269109line43> > > > > 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?
As this is the file which is getting complied, so made corresponding changes in this file under plugin package. > On March 12, 2013, 11:33 a.m., Devdeep Singh wrote: > > server/src/com/cloud/resourcelimit/ResourceLimitManagerImpl.java, line 887 > > <https://reviews.apache.org/r/9541/diff/7/?file=269111#file269111line887> > > > > 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 To get the resource count of all the existing resources, the method was written in the DAOs files, i have followed similar approach and in this way the methods to get primary and secondary storage space count are more generic and can be used in other parts also if required. > On March 12, 2013, 11:33 a.m., Devdeep Singh wrote: > > server/src/com/cloud/storage/VolumeManagerImpl.java, line 471 > > <https://reviews.apache.org/r/9541/diff/7/?file=269113#file269113line471> > > > > Should the check be made outside the validateVolume function, that is > > in uploadVolume? As ResourceLimitCheck to check template is in validateVolume, in the similar manner I am also checking the resource limit for secondary storage in the same method. If you feel that both the checks and accountAccess check (which is also called from the same method) should not be in validateVolume, then I will move these calls in the uploadVolume method. > On March 12, 2013, 11:33 a.m., Devdeep Singh wrote: > > server/src/com/cloud/storage/VolumeManagerImpl.java, line 1274 > > <https://reviews.apache.org/r/9541/diff/7/?file=269113#file269113line1274> > > > > 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. Thanks for suggestion, I'll add appropriate comment in the code. - Sanjay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9541/#review17720 ----------------------------------------------------------- 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 > >
