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

["Michael R. Hines" <mrhines@linux.vnet.ibm.com>]

On 03/12/2014 05:49 AM, Eric Blake wrote:
> 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.

Ooops. =)

>>           .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.
>

Well, unless there's anymore strong objection, I didn't find it too
difficult to add a new command in QEMU, although I did find it quite
painful to expose this command in Libvirt - I had to modify something
like 5 or 6 (IIRC) different files in libvirt to accomplish the same goal.

Could we "merge" the commands into a single command at the
libvirt level instead of the QEMU level?

Is there any other "pressing" reason to merge them at the QEMU
level?


>> +++ 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.
>

Totally forgot about that - will get a 'get' command in there ASAP.

- Michael