6:13 a.m.
* 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(a)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
6:20 a.m.
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(a)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;
}
9:25 a.m.
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(a)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;
> }
9:31 a.m.
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:41 a.m.
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
4901
days inactive
4901
days old
4 comments
4 participants
participants (4)
-
ajia -
ajia＠redhat.com
-
Eric Blake -
Michal Privoznik