[libvirt] [PATCH v4] virsh: avoid missing zero value judgement in cmdBlkiotune
* 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@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")); 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; } -- 1.7.1
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@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? 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; }
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@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
On 07/28/2011 07:20 PM, Michal Privoznik wrote: 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; }
On 07/28/2011 08:25 AM, ajia wrote:
- _("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.
If you wanted to do that, then we need something longer: Unable to parse, non-integer encountered where integer parameter expected But I think the shorter version is accurate - our intent is to tell the user "this option requires an integer parameter, but the input you gave me could not be parsed as an integer". So I don't think the error message should change.
BTW, except this, are others okay?
Yes, so I pushed with that one line fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 07/28/2011 10:31 PM, Eric Blake wrote:
On 07/28/2011 08:25 AM, ajia wrote:
- _("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.
If you wanted to do that, then we need something longer:
Unable to parse, non-integer encountered where integer parameter expected
But I think the shorter version is accurate - our intent is to tell the user "this option requires an integer parameter, but the input you gave me could not be parsed as an integer".
So I don't think the error message should change.
BTW, except this, are others okay?
Yes, so I pushed with that one line fixed.
Eric, thanks :) Regards, Alex
participants (4)
-
ajia -
ajia＠redhat.com
-
Eric Blake -
Michal Privoznik