Re: [Qemu-devel] [PATCH 02/11] Add migrate-set-capabilities and query-migrate-capabilities

[Eric Blake]

On 08/05/2012 03:13 AM, Orit Wasserman wrote:
> The management can enable/disable a capability for the next migration by using
> migrate-set-apabilities QMP command.

s/set-apabilities/set-capabilities/

> The management can query the current migration capabilities using
> query-migrate-capabilities QMP command.
> The user can use migrate_set_capability and 'info migrate_capabilities' HMP
> commands.
> 
> Signed-off-by: Orit Wasserman <address@hidden>
> Signed-off-by: Juan Quintela <address@hidden>
> ---

> @@ -1419,6 +1433,8 @@ show user network stack connection states
>  show migration status
>  @item info migrate_supported_capabilities
>  show migrate supported capabilities
> address@hidden info migrate_capabilities
> +show current migration capabilities

In HMP, are migrate_supported_capabilities and migrate_capabilities
redundant?  That is, I think I can use either command to answer both
questions "what capabilities exist" and "what is the current state of
all capabilities that exist", since _both_ commands output a list of
capability names as well as an on/off designator.  If my analysis is
right, then we don't need migrate_supported_capabilities.

> +++ b/qapi-schema.json
> @@ -345,6 +345,27 @@
>  { 'command': 'query-migrate-supported-capabilities', 'returns': 
> ['MigrationCapabilityStatus'] }

After all, in QMP, 'query-migrate-supported-capabilities'...

>  
>  ##
> +# @migrate-set-capabilities
> +#
> +# Enable/Disable the following migration capabilities (like xbzrle)
> +#

Missing a line, something like:

# @capabilities: json array of capability modifications to make

> +# Since: 1.2
> +##
> +{ 'command': 'migrate-set-capabilities',
> +  'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
> +
> +##
> +# @query-migrate-capabilities
> +#
> +# Returns information about the current migration capabilities status
> +#
> +# Returns: @MigrationCapabilitiesStatus
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'query-migrate-capabilities', 'returns':   
> ['MigrationCapabilityStatus']}

...and 'query-migrate-capabilities' return the same information.  So we
don't need two commands, just one would do.

Spacing was awkward before that '['.

I would recommend that you s/[_-]supported// in patch 1/11, then removed
query-migrate-capabilities in 2/11 (having moved it to 1/11), leaving
2/11 to just focus on migrate-set-capabilities.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature