[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 02/13] Add migration capabilities
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v14 02/13] Add migration capabilities |
Date: |
Tue, 03 Jul 2012 12:36:21 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 |
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 address@hidden +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v14 10/13] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions, (continued)
[Qemu-devel] [PATCH v14 05/13] Add uleb encoding/decoding functions, Orit Wasserman, 2012/07/03
[Qemu-devel] [PATCH v14 11/13] Add XBZRLE to ram_save_block and ram_save_live, Orit Wasserman, 2012/07/03
[Qemu-devel] [PATCH v14 02/13] Add migration capabilities, Orit Wasserman, 2012/07/03
- Re: [Qemu-devel] [PATCH v14 02/13] Add migration capabilities,
Eric Blake <=
[Qemu-devel] [PATCH v14 13/13] Add XBZRLE statistics, Orit Wasserman, 2012/07/03
[Qemu-devel] [PATCH v14 12/13] Add set_cachesize command, Orit Wasserman, 2012/07/03
[Qemu-devel] [PATCH v14 09/13] Add migration_end function, Orit Wasserman, 2012/07/03