On 07/28/2011 05:16 PM, Michal Privoznik wrote:
On 28.07.2011 11:04, 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@xxxxxxxxxx>
---
tools/virsh.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
I liked the first verion more than this. Although it is not shown in
this snippet,
nparams is initialized to zero
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;
}
and therefore doing this
+ nparams++;
+
will invalidate this test.
if (nparams == 0) {
/* get the number of blkio parameters */
if (virDomainGetBlkioParameters(dom, NULL,&nparams, flags)
!= 0) {
The better way of dealing with this issue IMO is to switch from
if (weight) {
to test if 'weight' parameter was given (vshCommandOptInt() returns
>0) and after this
check for impermissible values. And after the latter test (still
inside 'weight argument
given') increment nparams:
if (weight_given) {
if (weight<=0) {
report error; goto cleanup;
nparams++;
}
Michal
I aware of this , absolutely agree, thanks for your nice comment.
Regards,
Alex
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list