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

[Michal Privoznik]

On 28.07.2011 12:38, Alex Jia wrote:
> On 07/28/2011 05:50 PM, Wen Congyang wrote:
>> At 07/28/2011 05:52 PM, Alex Jia Write:
>>> * tools/virsh.c: avoid missing zero value judgement in cmdBlkiotune,
>>> 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 | 6 +++---
>>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/virsh.c b/tools/virsh.c
>>> index 8bd22dc..f24050d 100644
>>> --- a/tools/virsh.c
>>> +++ b/tools/virsh.c
>>> @@ -4037,12 +4037,12 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
>>> cmd)
>>> goto cleanup;
>>> }
>>>
>>> - if (weight) {
>>> - nparams++;
>>> - if (weight< 0) {
>>> + if (vshCommandOptInt(cmd, "weight",&weight)> 0) {
>> Why you call vshCommandOptInt(cmd, "weight",&weight) again?
> Make sure weight indeed exists and then checking whether its value is
> right.
>
> if do it like this:
>
> if (weight<= 0) {
> vshError(ctl, _("Invalid value of %d for I/O weight"), weight);
> goto cleanup;
> }
>
> nparams++;
> ......
>
> when I run virsh blkiotune ${guestname} --weight 0, I will not hit this
> error,
> and when I get weight value by virsh blkiotune ${guestname}, this error
> will be
> hit, because vshCommandOptInt(cmd, "weight",&weight) will return 0 and
> unchange
> weight value, it means weight will keep initial 0 value, hence vshError
> will be
> raise.
>
> Thanks,
>
> Alex
>

Our point is, you can store the return value of vshCommandOptInt (a few 
lines above)
and then just check against it instead of running the function again.

Michal