Re: [PATCH QEMU v5 1/8] softmmu/dirtylimit: Add parameter check for hmp "set_vcpu_dirty_limit"

[Juan Quintela]

Juan Quintela <quintela@redhat.com> wrote:
> ~hyman <hyman@git.sr.ht> wrote:
>> From: Hyman Huang(黄勇) <yong.huang@smartx.com>
>>
>> dirty_rate paraemter of hmp command "set_vcpu_dirty_limit" is invalid
>> if less than 0, so add parameter check for it.
>
> why?

And here I am, making a full of myself.

vcpu_dirty_limit and vcpu_dirty_limit_period are two different things.

So:

Reviewed-by: Juan Quintela <quintela@redhat.com>


> Next patch does it correctly:
>
> +    if (params->has_x_vcpu_dirty_limit_period &&
> +        (params->x_vcpu_dirty_limit_period < 1 ||
> +         params->x_vcpu_dirty_limit_period > 1000)) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> +                   "x-vcpu-dirty-limit-period",
> +                   "a value between 1 and 1000");
> +        return false;
> +    }
> +
>      return true;
>  }
>
> I hate to have to search several places to check for errors in values.
> We get all errors in the functions that set the parameters.
>
> Can you resend with just the monitor command removed?
>
> Or there is any advantage of getting the error message from
> qemu_set_vcpu_dirty_limit()?
>
> Later, Juan.