Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/02/2017 18:26, Daniel Henrique Barboza wrote:
> The previous error message was displaying the values in miliseconds,
> being misleading with the command that accepts the value in seconds:
> 
> { "execute": "migrate_set_downtime", "arguments": {"value": 3000}}
> {"error": {"class": "GenericError", "desc": "Parameter 'downtime_limit'
> expects an integer in the range of 0 to 2000000 milliseconds"}}
> 
> This patch changes it to '2000 seconds' to keep consistency with
> the expected parameter.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
> ---
>  migration/migration.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index c6ae69d..2dc63b1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -49,6 +49,9 @@
>   * for sending the last part */
>  #define DEFAULT_MIGRATE_SET_DOWNTIME 300
>  
> +/* Maximum migrate downtime set to 2000*1000 miliseconds */
> +#define MAX_MIGRATE_SET_DOWNTIME (2000 * 1000)
> +
>  /* Default compression thread count */
>  #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8
>  /* Default decompression thread count, usually decompression is at
> @@ -843,10 +846,11 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
>          return;
>      }
>      if (params->has_downtime_limit &&
> -        (params->downtime_limit < 0 || params->downtime_limit > 2000000)) {
> +        (params->downtime_limit < 0 ||
> +         params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) {
>          error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>                     "downtime_limit",
> -                   "an integer in the range of 0 to 2000000 milliseconds");
> +                   "an integer in the range of 0 to 2000 seconds");

Perhaps you could use %d and set  MAX_MIGRATE_SET_DOWNTIME to 2000?
Though perhaps the migration maintainers are okay with the patch as is.

Paolo

>          return;
>      }
>      if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) {
>