Re: [Qemu-devel] [RFC PATCH v2 10/12] mc: expose tunable parameter for checkpointing frequency

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

On 02/18/2014 01:50 AM, mrhines@linux.vnet.ibm.com wrote:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
> 
> This exposes a QMP command that allows the management software
> or policy to control the frequency of micro-checkpointing.
> 
> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
> ---
>  hmp-commands.hx  | 16 +++++++++++++++-
>  hmp.c            |  6 ++++++
>  hmp.h            |  1 +
>  qapi-schema.json | 13 +++++++++++++
>  qmp-commands.hx  | 23 +++++++++++++++++++++++
>  5 files changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f3fc514..2066c76 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -888,7 +888,7 @@ ETEXI
>  		      "\n\t\t\t -b for migration without shared storage with"
>  		      " full copy of disk\n\t\t\t -i for migration without "
>  		      "shared storage with incremental copy of disk "
> -		      "(base image shared between src and destination)",
> + 		      "(base image shared between src and destination)",

Spurious hunk.  Oh, I see - you managed to take TAB damage and make it
worse with a space-TAB (I guess this file isn't tab-clean, like the
.json file is).  Eww.

>          .mhandler.cmd = hmp_migrate,
>      },
>  
> @@ -965,6 +965,20 @@ Set maximum tolerated downtime (in seconds) for migration.
>  ETEXI
>  
>      {
> +        .name       = "migrate-set-mc-delay",

We're building up a LOT of migrate- tunable commands.  Maybe it's time
to think about building a more generic migrate-set-parameter, which
takes both the name of the parameter to set and its value, so that a
single command serves all parameters, instead of needing a proliferation
of commands.  Of course, for that to be useful, we also need a way to
introspect which parameters can be tuned; whereas with the current
approach of one command per parameter (well, 2 for set vs. get) the
introspection is based on whether the command exists.


> +++ b/qapi-schema.json
> @@ -2160,6 +2160,19 @@
>  { 'command': 'migrate_set_downtime', 'data': {'value': 'number'} }
>  
>  ##
> +# @migrate-set-mc-delay
> +#
> +# Set delay (in milliseconds) between micro checkpoints.
> +#
> +# @value: maximum delay in milliseconds 
> +#
> +# Returns: nothing on success
> +#
> +# Since: 2.x
> +##
> +{ 'command': 'migrate-set-mc-delay', 'data': {'value': 'int'} }
> +
> +##

I hate write-only interfaces.  If I can set the parameter, I _also_ need
a way to query the current value of the parameter.  Either an existing
migration statistics output should be modified to include this new
information, or you need to add a counterpart migrate-get-mc-delay command.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]