[libvirt] [PATCH v2] virsh: avoid missing zero value judgement in cmdBlkiotune
Michal Privoznik
mprivozn at redhat.com
Thu Jul 28 09:16:42 UTC 2011
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 at redhat.com>
> ---
> 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
More information about the libvir-list
mailing list