----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/11782/#review21865 -----------------------------------------------------------
api/src/com/cloud/agent/api/to/VolumeTO.java <https://reviews.apache.org/r/11782/#comment45155> Since the bytesReadRate, bytesWriteRate, iopsReadRate, iopsWriteRate are optional, how are they represented when they are not specified? Given that the type is long, I am concerned that a magic value is being used. Consider changing the type to Long which will allow null to be used to clearly represent the optional nature of the value. api/src/com/cloud/vm/DiskProfile.java <https://reviews.apache.org/r/11782/#comment45150> How are these rate value represented when they are not specified? I am concerned that magic values are used in this scenario. Consider using a Long instead of a long would allow a clear expression of optionality of the value. api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java <https://reviews.apache.org/r/11782/#comment45156> See note above about magic values. It appears that 0 is being used as a magic value to represent no value specified. server/src/com/cloud/test/DatabaseConfig.java <https://reviews.apache.org/r/11782/#comment45157> Coding standard is to wrap all if blocks with curly braces. setup/db/db/schema-410to420.sql <https://reviews.apache.org/r/11782/#comment45158> Concerned again about using 0 as a magic value to represent no value specified. Consider converting a nullable column with a default value of null since null is the natural idiom for representing optional data. - John Burwell On June 12, 2013, 9:13 a.m., Wei Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/11782/ > ----------------------------------------------------------- > > (Updated June 12, 2013, 9:13 a.m.) > > > Review request for cloudstack, Wido den Hollander and John Burwell. > > > Description > ------- > > The patch for VM Disk I/O throttling based on commit > 3f3c6aa35f64c4129c203d54840524e6aa2c4621 > > > This addresses bug CLOUDSTACK-1301. > > > Diffs > ----- > > api/src/com/cloud/agent/api/to/VolumeTO.java 4cbe82b > api/src/com/cloud/offering/DiskOffering.java dd77c70 > api/src/com/cloud/vm/DiskProfile.java e3a3386 > api/src/org/apache/cloudstack/api/ApiConstants.java ab1402c > > api/src/org/apache/cloudstack/api/command/admin/offering/CreateDiskOfferingCmd.java > aa11599 > > api/src/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java > 4c54a4e > api/src/org/apache/cloudstack/api/response/DiskOfferingResponse.java > 377e66e > api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java > 31533f8 > api/src/org/apache/cloudstack/api/response/VolumeResponse.java 21d7d1a > client/WEB-INF/classes/resources/messages.properties 2b17359 > core/src/com/cloud/agent/api/AttachVolumeCommand.java 302b8f8 > engine/schema/src/com/cloud/storage/DiskOfferingVO.java 909d7fe > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java > bab53bc > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParser.java > b8645e1 > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java > 9cddb2e > server/src/com/cloud/api/query/dao/DiskOfferingJoinDaoImpl.java 283181f > server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 56e4d0a > server/src/com/cloud/api/query/dao/VolumeJoinDaoImpl.java e27e2d9 > server/src/com/cloud/api/query/vo/DiskOfferingJoinVO.java 6d3cdcb > server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java e87a101 > server/src/com/cloud/api/query/vo/VolumeJoinVO.java 6ef8c91 > server/src/com/cloud/configuration/Config.java 5ee0fad > server/src/com/cloud/configuration/ConfigurationManager.java 8db037b > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 59e70cf > server/src/com/cloud/storage/StorageManager.java d49a7f8 > server/src/com/cloud/storage/StorageManagerImpl.java d38b35e > server/src/com/cloud/storage/VolumeManagerImpl.java 43f3681 > server/src/com/cloud/test/DatabaseConfig.java 70c8178 > server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 21b3590 > setup/db/db/schema-410to420.sql bcfbcc9 > ui/dictionary.jsp a5f0662 > ui/scripts/configuration.js cb15598 > > Diff: https://reviews.apache.org/r/11782/diff/ > > > Testing > ------- > > testing ok. > > > Thanks, > > Wei Zhou > >
