[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 28.07.2011 11:26, Alex Jia wrote:
On 07/28/2011 05:04 PM, Alex Jia wrote:
* 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 | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 8bd22dc..512f2c6 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4037,14 +4037,13 @@ cmdBlkiotune(vshControl * ctl, const vshCmd *
cmd)
goto cleanup;
}

- if (weight) {
- nparams++;
- if (weight< 0) {
- vshError(ctl, _("Invalid value of %d for I/O weight"), weight);
- goto cleanup;
- }
+ if (weight<= 0) {
+ vshError(ctl, _("Invalid value of %d for I/O weight"), weight);
+ goto cleanup;
}

+ nparams++;
+
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL,&nparams, flags) != 0) {
Hmm, the above patch will introduce new issue, if blkiotune without any
weight option,
vshError will also be hit, that is not we expected.

The reason of root is vshCommandOptInt will assign 0 to weight if option
not found
and not required, so we need to specifically deal with 0 value, I will
renew consider this
issue, so cancel this patch.
Actually,  vshCommandOptInt (and all virsh opts parsing functions), touches
given variable only iff found & valid. In all other cases the variable
remains untouched. It's the initialization which set weight to 0.

The reason for such return values is, we can firstly initialize variable,
then call option parsing function over it which will change it iff
everything's fine:

int var = 10;
if (vshCommandOptInt(cmd, "int-attr", &var) < 0) {
    /* error, e.g. string value given */
    goto cleanup;
}

/* here, var may be 10, iff int-attr was not given,
or other value if it was */

Or, if "int-att" is required, we just change the test:

if (vshCommandOptInt(cmd, "int-attr", &var) <= 0) {

Perhaps this will help :)

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]