[libvirt] [PATCH v3] virsh: avoid missing zero value judgement in cmdBlkiotune
* 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@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) { + if (weight <= 0) { vshError(ctl, _("Invalid value of %d for I/O weight"), weight); goto cleanup; } + nparams++; } if (nparams == 0) { -- 1.7.1
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@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?
+ if (weight <= 0) { vshError(ctl, _("Invalid value of %d for I/O weight"), weight); goto cleanup; } + nparams++; }
if (nparams == 0) {
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@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
+ if (weight<= 0) { vshError(ctl, _("Invalid value of %d for I/O weight"), weight); goto cleanup; } + nparams++; }
if (nparams == 0) {
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@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
participants (4)
-
ajia＠redhat.com -
Alex Jia
-
Michal Privoznik -
Wen Congyang