On 07/03/2012 07:52 AM, Orit Wasserman wrote: > Add migration capabilities that can be queried by the management. > The management can query the source QEMU and the destination QEMU in order to > verify both support some migration capability (currently only XBZRLE). > The management can enable a capability for the next migration by using > migrate_set_parameter command.
Couple of comment nits below. Also, if you don't mind a bit of
bike-shedding...
[roughly translated - feel free to ignore the bulk of this email; the
patch as-is works, if you don't think my alternate design makes sense to
implement]
> +++ b/hmp-commands.hx
> @@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for
> migration.
> ETEXI
>
> {
> + .name = "migrate_set_parameter",
> + .args_type = "capability:s,state:b",
> + .params = "",
> + .help = "Enable the usage of a capability for migration",
> + .mhandler.cmd = hmp_migrate_set_parameter,
This requires the 'state' parameter to be passed. But wouldn't it be
easier to default things to state=true if no parameter was present,
since the most common usage of this command will be to enable, rather
than disable, a migration parameter?
> - /* no migration has happened ever */
> + /* no migration has happened ever show enabled capabilities */
Missing some punctuation, so this was hard to read. Maybe:
/* no migration has ever happened; show enabled capabilities */
> +++ b/qapi-schema.json
> +##
> +# @MigrationCapabilityInfo
> +#
> +# Migration capability information
> +#
> +# @capability: capability enum
> +#
> +# Since: 1.2
> +##
> +{ 'type': 'MigrationCapabilityInfo',
> + 'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }
Back to the bike-shedding - if you would mark this '*state':'bool', then
the enabled parameter becomes optional on input (it should still always
be present on query-migration-capabilities output, though).
> +migrate_set_parameters
> +-------
> +
> +Enable/Disable migration capabilities
> +
> +- "xbzrle": xbzrle support
> +
> +Arguments:
> +
> +Example:
> +
> +-> { "execute": "migrate_set_parameters" , "arguments":
> + { "parameters": { "capability": "xbzrle", "state": true } ] } }
Missing a '[' after "parameters":
--
Eric Blake ebl...@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
