[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration para
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter |
Date: |
Fri, 23 Jan 2015 08:39:41 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 12/11/2014 06:29 PM, Liang Li wrote:
> Add the qmp and hmp commands to tune the parameters used in live
> migration.
>
> Signed-off-by: Liang Li <address@hidden>
> Signed-off-by: Yang Zhang <address@hidden>
> ---
> hmp-commands.hx | 15 ++++++++++
> hmp.c | 32 +++++++++++++++++++++
> hmp.h | 3 ++
> include/migration/migration.h | 4 +--
> migration.c | 66
> +++++++++++++++++++++++++++++++++++--------
> monitor.c | 18 ++++++++++++
> qapi-schema.json | 44 +++++++++++++++++++++++++++++
> qmp-commands.hx | 23 +++++++++++++++
> 8 files changed, 190 insertions(+), 15 deletions(-)
>
> +++ b/hmp.h
> @@ -63,6 +63,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
> void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
> void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
> void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict);
> void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict);
> void hmp_set_password(Monitor *mon, const QDict *qdict);
> void hmp_expire_password(Monitor *mon, const QDict *qdict);
> @@ -111,6 +112,8 @@ void watchdog_action_completion(ReadLineState *rs, int
> nb_args,
> const char *str);
> void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
> const char *str);
> +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
> + const char *str);
Off-by-one on indentation.
> @@ -292,6 +295,41 @@ void
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> }
> }
>
> +void qmp_migrate_set_parameters(MigrationParameterStatusList *params,
> + Error **errp)
Indentation is off.
> +{
> + MigrationState *s = migrate_get_current();
> + MigrationParameterStatusList *p;
> +
> + for (p = params; p; p = p->next) {
> + switch (p->value->parameter) {
> + case MIGRATION_PARAMETER_COMPRESS_LEVEL:
> + if (p->value->value < 0 || p->value->value > 9) {
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> "compress_level",
> + "is invalied, it should be in the rang of 0 to 9");
s/invalied/invalid/; s/rang/range/
> + if (p->value->value < 1 || p->value->value > 255) {
> + error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> + "(de)compress_threads",
> + "is invalied, it should be in the rang of 1 to 255");
and again
> +++ b/monitor.c
> @@ -4544,6 +4544,24 @@ void migrate_set_capability_completion(ReadLineState
> *rs, int nb_args,
> }
> }
>
> +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
> + const char *str)
Indentation is off.
> +++ b/qapi-schema.json
> @@ -540,6 +540,50 @@
> ##
> { 'command': 'query-migrate-capabilities', 'returns':
> ['MigrationCapabilityStatus']}
>
> +# @MigrationParameter
> +#
> +# Migration parameters enumeration
> +#
> +# @compress-level:Set the compression level to be used in live migration,
s/:/: /
> +# 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.
> +#
> +# Since: 2.3
> +##
> +{ 'enum': 'MigrationParameter',
> + 'data': ['compress-level', 'compress-threads', 'decompress-threads'] }
> +##
> +# @MigrationParameterStatus
> +#
> +# Migration parameter information
> +#
> +# @parameter: parameter enum
> +#
> +# @value: parameter value int
> +#
> +# Since: 2.3
> +##
> +{ 'type': 'MigrationParameterStatus',
> + 'data': { 'parameter' : 'MigrationParameter', 'value' : 'int' } }
Doesn't allow for non-integer parameters. Not necessarily fatal for
input only (I think a flat union could be utilized with a
MigrationParameter as the discriminator to allow non-int values later on)...
> +##
> +# @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'] } }
...but on output, we might confuse callers by outputting non-int values
unless we plan _from the start_ to support them. That is, I think we want:
{ 'type': 'MigrationParameterBase',
'data': { 'parameter': 'MigrationParameter' } }
{ 'type': 'MigrationParameterInt',
'data': { 'value': 'int' } }
{ 'union': 'MigrationParameterStatus',
'base': 'MigrationParameterBase',
'discriminator': 'parameter',
'data': { 'compress-level': 'MigrationParameterInt',
'compress-threads': 'MigrationParameterInt',
'decompress-threads': 'MigrationParameterInt' } }
to make it obvious to callers that they must be prepared to accept
non-int values down the road if we ever extend the union to add another
parameter with a different type.
> SQMP
> +migrate-set-parameters
> +----------------------
> +
> +Set migration parameters
> +
> +- "compress-leve": multiple compression thread support
s/leve/level/
Missing two of the three defined parameters.
I hate write-only interfaces. I'm hoping you add the query of
parameters in a later patch - but I'd prefer you squash it into one patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature