From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEebC-0005Xh-7G for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:48:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YEeb8-00018G-4o for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:48:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60631) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEeb7-00017q-RO for qemu-devel@nongnu.org; Fri, 23 Jan 2015 08:48:30 -0500 Date: Fri, 23 Jan 2015 13:48:22 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150123134821.GL2370@work-vm> References: <1418347746-15829-1-git-send-email-liang.z.li@intel.com> <1418347746-15829-13-git-send-email-liang.z.li@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418347746-15829-13-git-send-email-liang.z.li@intel.com> Subject: Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liang Li Cc: quintela@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, lcapitulino@redhat.com, yang.z.zhang@intel.com * Liang Li (liang.z.li@intel.com) wrote: > Add the qmp and hmp commands to tune the parameters used in live > migration. If I understand correctly on the destination side we need to set the number of decompression threads very early on an incoming migration - I'm not clear how early that needs to be - especially if you're using fd: so it's not waiting for a connect ? Eric: How would libvirt do that? > > Signed-off-by: Liang Li > Signed-off-by: Yang Zhang > --- > 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(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index e37bc8b..535b5ba 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -985,6 +985,21 @@ Enable/Disable the usage of a capability @var{capability} for migration. > ETEXI > > { > + .name = "migrate_set_parameter", > + .args_type = "parameter:s,value:i", > + .params = "parameter value", > + .help = "Set the parameter for migration", > + .mhandler.cmd = hmp_migrate_set_parameter, > + .command_completion = migrate_set_parameter_completion, > + }, > + > +STEXI > +@item migrate_set_parameter @var{parameter} @var{value} > +@findex migrate_set_parameter > +Set the parameter @var{parameter} for migration. > +ETEXI > + > + { > .name = "client_migrate_info", > .args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?", > .params = "protocol hostname port tls-port cert-subject", > diff --git a/hmp.c b/hmp.c > index 63d7686..965c037 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1079,6 +1079,38 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict) > } > } > > +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > +{ > + const char *param = qdict_get_str(qdict, "parameter"); > + int value = qdict_get_int(qdict, "value"); > + Error *err = NULL; > + MigrationParameterStatusList *params = g_malloc0(sizeof(*params)); > + int i; > + > + for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) { > + if (strcmp(param, MigrationParameter_lookup[i]) == 0) { > + params->value = g_malloc0(sizeof(*params->value)); > + params->value->parameter = i; > + params->value->value = value; > + params->next = NULL; > + qmp_migrate_set_parameters(params, &err); > + break; > + } > + } > + > + if (i == MIGRATION_PARAMETER_MAX) { > + error_set(&err, QERR_INVALID_PARAMETER, param); > + } > + > + qapi_free_MigrationParameterStatusList(params); > + > + if (err) { > + monitor_printf(mon, "migrate_set_parameter: %s\n", > + error_get_pretty(err)); > + error_free(err); > + } > +} > + > void hmp_set_password(Monitor *mon, const QDict *qdict) > { > const char *protocol = qdict_get_str(qdict, "protocol"); > diff --git a/hmp.h b/hmp.h > index 4bb5dca..bd1b203 100644 > --- a/hmp.h > +++ 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); > void host_net_add_completion(ReadLineState *rs, int nb_args, const char *str); > void host_net_remove_completion(ReadLineState *rs, int nb_args, > const char *str); > diff --git a/include/migration/migration.h b/include/migration/migration.h > index 0c4f21c..8e09b42 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -50,9 +50,7 @@ struct MigrationState > QEMUBH *cleanup_bh; > QEMUFile *file; > QemuThread *compress_thread; > - int compress_thread_count; > - int decompress_thread_count; > - int compress_level; > + int parameters[MIGRATION_PARAMETER_MAX]; > > int state; > MigrationParams params; > diff --git a/migration.c b/migration.c > index 9d1613d..d3d377e 100644 > --- a/migration.c > +++ b/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, > + .parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] = > + DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT, > + .parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] = > + DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT, > }; > > return ¤t_migration; > @@ -292,6 +295,41 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params, > } > } > > +void qmp_migrate_set_parameters(MigrationParameterStatusList *params, > + Error **errp) > +{ > + 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"); Typo: 'rang' > + return; > + } > + break; > + case MIGRATION_PARAMETER_COMPRESS_THREADS: > + case MIGRATION_PARAMETER_DECOMPRESS_THREADS: > + if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP) { > + error_set(errp, QERR_MIGRATION_ACTIVE); > + return; > + } > + 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"); Typo: 'rang' > + return; > + } > + break; > + default: > + return; > + } > + s->parameters[p->value->parameter] = p->value->value; > + } > +} Do you need the same check here that is in qmp_migrate_set_capabilities; to stop someone changing a parameter (e.g. the number of threads) while it's running. Dave > + > /* shared migration helpers */ > > static void migrate_set_state(MigrationState *s, int old_state, int new_state) > @@ -386,9 +424,11 @@ static MigrationState *migrate_init(const MigrationParams *params) > int64_t bandwidth_limit = s->bandwidth_limit; > bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > int64_t xbzrle_cache_size = s->xbzrle_cache_size; > - int compress_level = s->compress_level; > - int compress_thread_count = s->compress_thread_count; > - int decompress_thread_count = s->decompress_thread_count; > + int compress_level = s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL]; > + int compress_thread_count = > + s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS]; > + int decompress_thread_count = > + s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS]; > > memcpy(enabled_capabilities, s->enabled_capabilities, > sizeof(enabled_capabilities)); > @@ -399,9 +439,11 @@ static MigrationState *migrate_init(const MigrationParams *params) > sizeof(enabled_capabilities)); > s->xbzrle_cache_size = xbzrle_cache_size; > > - s->compress_level = compress_level; > - s->compress_thread_count = compress_thread_count; > - s->decompress_thread_count = decompress_thread_count; > + s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level; > + s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] = > + compress_thread_count; > + s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] = > + decompress_thread_count; > s->bandwidth_limit = bandwidth_limit; > s->state = MIG_STATE_SETUP; > trace_migrate_set_state(MIG_STATE_SETUP); > @@ -589,7 +631,7 @@ int migrate_compress_level(void) > > s = migrate_get_current(); > > - return s->compress_level; > + return s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL]; > } > > int migrate_compress_threads(void) > @@ -598,7 +640,7 @@ int migrate_compress_threads(void) > > s = migrate_get_current(); > > - return s->compress_thread_count; > + return s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS]; > } > > int migrate_decompress_threads(void) > @@ -607,7 +649,7 @@ int migrate_decompress_threads(void) > > s = migrate_get_current(); > > - return s->decompress_thread_count; > + return s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS]; > } > > int migrate_use_xbzrle(void) > diff --git a/monitor.c b/monitor.c > index f1031a1..4cf62b6 100644 > --- a/monitor.c > +++ 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) > +{ > + size_t len; > + > + len = strlen(str); > + readline_set_completion_index(rs, len); > + if (nb_args == 2) { > + int i; > + for (i = 0; i < MIGRATION_PARAMETER_MAX; i++) { > + const char *name = MigrationParameter_lookup[i]; > + if (!strncmp(str, name, len)) { > + readline_add_completion(rs, name); > + } > + } > + } > +} > + > void host_net_add_completion(ReadLineState *rs, int nb_args, const char *str) > { > int i; > diff --git a/qapi-schema.json b/qapi-schema.json > index d371af3..2caeccc 100644 > --- a/qapi-schema.json > +++ 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, > +# 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' } } > +## > +# @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'] } } > +## > ## > # @MouseInfo: > # > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 718dd92..59d2643 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -3225,6 +3225,29 @@ EQMP > }, > > SQMP > +migrate-set-parameters > +---------------------- > + > +Set migration parameters > + > +- "compress-leve": multiple compression thread support Typo: 'compress-leve' > + > +Arguments: > + > +Example: > + > +-> { "execute": "migrate-set-parameters" , "arguments": > + { "parameters": [ { "parameter": "compress-level", "value": 1 } ] } } > + > +EQMP > + > + { > + .name = "migrate-set-parameters", > + .args_type = "parameters:O", > + .params = "parameter:s,value:i", > + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, > + }, > +SQMP > query-balloon > ------------- > > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK