Re: [Qemu-devel] [v4 12/13] migration: Add command to set migration parameter

[Eric Blake <eblake@redhat.com>]

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

On 02/02/2015 04:05 AM, Liang Li wrote:
> Add the qmp and hmp commands to tune the parameters used in live
> migration.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> ---
>  hmp-commands.hx               | 15 ++++++++++
>  hmp.c                         | 35 ++++++++++++++++++++++
>  hmp.h                         |  3 ++
>  include/migration/migration.h |  4 +--
>  migration/migration.c         | 69 +++++++++++++++++++++++++++++++++++--------
>  monitor.c                     | 18 +++++++++++
>  qapi-schema.json              | 52 ++++++++++++++++++++++++++++++++
>  qmp-commands.hx               | 25 ++++++++++++++++
>  8 files changed, 206 insertions(+), 15 deletions(-)

> +++ b/migration/migration.c
> @@ -66,9 +66,12 @@ MigrationState *migrate_get_current(void)
>          .bandwidth_limit = MAX_THROTTLE,
>          .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
>          .mbps = -1,
> -        .compress_thread_count = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
> -        .decompress_thread_count = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
> -        .compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
> +        .parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] =
> +                DEFAULT_MIGRATE_COMPRESS_LEVEL,

Looks okay.

> +        .parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
> +                DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
> +        .parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
> +                DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,

Hmm - do we really need two parameters here?  Remember, compress threads
is used only on the source, and decompress threads is used only on the
destination.  Having a single parameter, 'threads', which is set to
compression threads on source and decompression threads on destination,
and which need not be equal between the two machines, should still work,
right?

> +++ b/qapi-schema.json
> @@ -541,6 +541,58 @@
>  ##
>  { 'command': 'query-migrate-capabilities', 'returns':   ['MigrationCapabilityStatus']}
>  
> +# @MigrationParameter
> +#
> +# Migration parameters enumeration
> +#
> +# @compress-level: Set the compression level to be used in live migration,
> +#          the compression level is an integer between 0 and 9, where 0 means
> +#          no compression, 1 means the best compression speed, and 9 means best
> +#          compression ratio which will consume more CPU.
> +#
> +# @compress-threads: Set compression thread count to be used in live migration,
> +#          the compression thread count is an integer between 1 and 255.
> +#
> +# @decompress-threads: Set decompression thread count to be used in live migration,
> +#          the decompression thread count is an integer between 1 and 255.
> +#

Again, I think you could get by with just a single parameter
'compress-threads', and maybe document that the value is typically set
higher on destination than on source.

> +# Since: 2.3
> +##
> +{ 'enum': 'MigrationParameter',
> +  'data': ['compress-level', 'compress-threads', 'decompress-threads'] }
> +##
> +# @MigrationParameterStatus
> +#
> +# Migration parameter information
> +#
> +# @parameter: the parameter of migration
> +#
> +# @data: pointer to the parameter value
> +#
> +# Since: 2.3
> +##
> +{ 'type': 'MigrationParameterBase',
> +  'data': {'parameter': 'MigrationParameter'} }
> +{ 'type': 'MigrationParameterInt',
> +  'data': {'value': 'int'} }
> +{ 'union': 'MigrationParameterStatus',

Is it worth having independent docs for each of these, rather than
cramming all three under one doc text?

> +  'base': 'MigrationParameterBase',
> +  'discriminator': 'parameter',
> +  'data': { 'compress-level': 'MigrationParameterInt',
> +            'compress-threads': 'MigrationParameterInt',
> +            'decompress-threads': 'MigrationParameterInt'} }
> +#
> +# @migrate-set-parameters
> +#
> +# Set the following migration parameters (like compress-level)
> +#
> +# @parameters: json array of parameter modifications to make
> +#
> +# Since: 2.3
> +##
> +{ 'command': 'migrate-set-parameters',
> +  'data': { 'parameters': ['MigrationParameterStatus'] } }
> +##

Interface looks reasonable to me (but I'm biased, as I suggested it).

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