[libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune

[ajia]

On 07/28/2011 07:20 PM, Michal Privoznik wrote:
> On 28.07.2011 13:13, Alex Jia wrote:
>> * tools/virsh.c: fix missing zero value judgement in cmdBlkiotune and 
>> correct
>>    vshError information.
>>
>>    when weight is equal to 0, the cmdBlkiotune will not raise any 
>> error information
>>    when judge weight value first time, and execute else branch to 
>> judge weight
>>    value again, strncpy(temp->field, VIR_DOMAIN_BLKIO_WEIGHT, 
>> sizeof(temp->field))
>>    will be not executed for ever. However, if and only if 
>> param->field is equal
>>    to VIR_DOMAIN_BLKIO_WEIGHT, underlying 
>> qemuDomainSetBlkioParameters function
>>    will check whether weight value is in range [100, 1000].
>>
>> * how to reproduce?
>>
>>    % virsh blkiotune ${guestname} --weight 0
>>
>> Signed-off-by: Alex Jia<ajia at redhat.com>
>> ---
>>   tools/virsh.c |    9 +++++----
>>   1 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/virsh.c b/tools/virsh.c
>> index 8bd22dc..feb45de 100644
>> --- a/tools/virsh.c
>> +++ b/tools/virsh.c
>> @@ -4004,6 +4004,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
>>       virDomainPtr dom;
>>       int weight = 0;
>>       int nparams = 0;
>> +    int rv = 0;
>>       unsigned int i = 0;
>>       virTypedParameterPtr params = NULL, temp = NULL;
>>       bool ret = false;
>> @@ -4031,15 +4032,15 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * 
>> cmd)
>>       if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
>>           return false;
>>
>> -    if (vshCommandOptInt(cmd, "weight",&weight)<  0) {
>> +    if ((rv = vshCommandOptInt(cmd, "weight",&weight))<  0) {
>>           vshError(ctl, "%s",
>> -                 _("Unable to parse integer parameter"));
>> +                 _("Unable to parse non-integer parameter"));
> Why this change?
when I give a non-integer to weight value such as
virsh blkiotune $domain --weight demo, vshError will be hit and raise
"Unable to parse integer parameter", 'demo' is a string not integer, it 
should
be only can parse integer parameter, so I think it should be changed 
like above,
please correct me again if I'm wrong again.

BTW, except this, are others okay?

Thanks,
Alex

>>           goto cleanup;
>>       }
>>
>> -    if (weight) {
>> +    if (rv>  0) {
>>           nparams++;
>> -        if (weight<  0) {
>> +        if (weight<= 0) {
>>               vshError(ctl, _("Invalid value of %d for I/O weight"), 
>> weight);
>>               goto cleanup;
>>           }