qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v6] migrate: move max-bandwidth and downtime-lim


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v6] migrate: move max-bandwidth and downtime-limit to migrate_set_parameter
Date: Thu, 15 Sep 2016 10:48:28 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0

On 09/14/2016 12:40 PM, Ashijeet Acharya wrote:
> Mark the old commands 'migrate_set_speed' and 'migrate_set_downtime' as
> deprecated.
> Move max-bandwidth and downtime-limit into migrate-set-parameters for
> setting maximum migration speed and expected downtime limit parameters
> respectively.
> Change downtime units to milliseconds (only for new-command) and set
> its upper bound limit to 2000 seconds.
> Update the query part in both hmp and qmp qemu control interfaces.
> 
> Signed-off-by: Ashijeet Acharya <address@hidden>
> ---

> +++ b/migration/migration.c

> @@ -804,6 +802,20 @@ void qmp_migrate_set_parameters(MigrationParameters 
> *params, Error **errp)
>                     "cpu_throttle_increment",
>                     "an integer in the range of 1 to 99");
>      }
> +    if (params->has_max_bandwidth &&
> +        (params->max_bandwidth < 0 || params->max_bandwidth > SIZE_MAX)) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE "%zu",
> +                   "max_bandwidth",
> +                   "an integer in the range of 0 to ", SIZE_MAX);

That looks weird (not necessarily your fault; but it explains why we've
been trying to avoid new QERR_ macros).  No other use of
QERR_INVALID_PARAMETER_VALUE adds additional text.  So I'm thinking it's
better to just follow suit with a shorter non-parameterized message;
such as in this instance:

balloon.c:        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"target", "a size");

Or, just open-code the full error message, instead of having half the
message hidden behind the QERR_INVALID_PARAMETER_VALUE macro.

Otherwise, it looks good to me.  With the error message sorted out, you
can add:

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]