> On June 13, 2013, 6:01 p.m., John Burwell wrote:
> >
>
> Wei Zhou wrote:
> John,
>
> The validation of input fields is in CreateDiskOfferingCmd.java and
> CreateServiceOfferingCmd.java, like:
> public long getBytesReadRate() {
> return (bytesReadRate == null) || (bytesReadRate < 0) ? 0 :
> bytesReadRate;
> }
> It can avoid setting a variaty with "long" type to "null" in other java
> files, for instance VolumeTO.java and DiskProfile.java.
>
> I think it is better to set the default value to 0 which means no
> limitation.
> What do you think if the default value is null? Maybe it also means no
> limitation, right?
>
> -Wei
null is the Java idiom for representing the lack of a optional value across all
types. Using zero for this purpose feels like a magic value. It also raises
the question of a client's intent when zero is passed -- did they make a
mistake or did they intend not specify a value? To my way of thinking, any
non-null value for the rates should be greater than zero and less than a
maximum. Values that fall out of this range cause an exception ...
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11782/#review21865
-----------------------------------------------------------
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
>
>
