[libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
ajia
ajia at redhat.com
Thu Jul 28 14:25:03 UTC 2011
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;
>> }
More information about the libvir-list
mailing list