Message ID | 1343554983-4195-3-git-send-email-owasserm@redhat.com |
---|---|
State | New |
Headers | show |
On Sun, 29 Jul 2012 12:42:54 +0300 Orit Wasserman <owasserm@redhat.com> wrote: > The management can enable/disable a capability for the next migration by using > migrate_set_parameter command. > The management can query the current migration capabilities using > query-migrate-parameters In general looks good to me, I have a question and a few nitpick comments below. > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > hmp-commands.hx | 16 ++++++++++++ > hmp.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > hmp.h | 2 + > migration.c | 42 +++++++++++++++++++++++++++++++- > migration.h | 2 + > monitor.c | 7 +++++ > qapi-schema.json | 32 ++++++++++++++++++++++++ > qmp-commands.hx | 60 ++++++++++++++++++++++++++++++++++++++++++++- > 8 files changed, 229 insertions(+), 3 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 8786148..3e15338 100644 > --- a/hmp-commands.hx > +++ 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 = "capability state", > + .help = "Enable/Disable the usage of a capability for migration", > + .mhandler.cmd = hmp_migrate_set_parameter, > + }, > + > +STEXI > +@item migrate_set_parameter @var{capability} @var{state} > +@findex migrate_set_parameter > +Enable/Disable the usage of a capability @var{capability} 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", > @@ -1419,6 +1433,8 @@ show user network stack connection states > show migration status > @item info migration_capabilities > show migration capabilities > +@item info migrate_parameters > +show current migration parameters > @item info balloon > show balloon information > @item info qtree > diff --git a/hmp.c b/hmp.c > index 5c7d0be..f2f63fd 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) > void hmp_info_migrate(Monitor *mon) > { > MigrationInfo *info; > + MigrationCapabilityStatusList *cap; > + MigrationParameters *params; > > info = qmp_query_migrate(NULL); > + params = qmp_query_migrate_parameters(NULL); > + > + /* do not display parameters during setup */ > + if (info->has_status && params->capabilities) { > + monitor_printf(mon, "capabilities: "); > + for (cap = params->capabilities; cap; cap = cap->next) { > + monitor_printf(mon, "%s: %s ", > + MigrationCapability_lookup[cap->value->capability], > + cap->value->state ? "on" : "off"); > + } > + monitor_printf(mon, "\n"); > + } > > if (info->has_status) { > monitor_printf(mon, "Migration status: %s\n", info->status); > @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) > } > > qapi_free_MigrationInfo(info); > + qapi_free_MigrationParameters(params); > } > > void hmp_info_migration_capabilities(Monitor *mon) > @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) > qapi_free_MigrationCapabilityStatusList(caps_list); > } > > +void hmp_info_migrate_parameters(Monitor *mon) > +{ > + > + MigrationCapabilityStatusList *cap; > + MigrationParameters *params; > + > + params = qmp_query_migrate_parameters(NULL); > + > + if (params->capabilities) { > + monitor_printf(mon, "capabilities: "); > + for (cap = params->capabilities; cap; cap = cap->next) { > + monitor_printf(mon, "%s: %s ", > + MigrationCapability_lookup[cap->value->capability], > + cap->value->state ? "on" : "off"); > + } > + monitor_printf(mon, "\n"); > + } > + > + qapi_free_MigrationParameters(params); > +} > + > void hmp_info_cpus(Monitor *mon) > { > CpuInfoList *cpu_list, *cpu; > @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) > qmp_migrate_set_speed(value, NULL); > } > > +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > +{ > + const char *cap = qdict_get_str(qdict, "capability"); > + bool state = qdict_get_bool(qdict, "state"); > + Error *err = NULL; > + MigrationCapabilityStatusList *params = NULL; > + int i; > + > + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > + if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { > + if (!params) { > + params = g_malloc0(sizeof(*params)); > + } This if shouldn't be necessary, right? > + params->value = g_malloc0(sizeof(*params->value)); > + params->value->capability = i; > + params->value->state = state; > + params->next = NULL; nitpick: I'd make the code ready for future capabilities. > + qmp_migrate_set_parameters(params, &err); > + break; > + } > + } > + > + if (i == MIGRATION_CAPABILITY_MAX) { > + error_set(&err, QERR_INVALID_PARAMETER, cap); > + } > + > + qapi_free_MigrationCapabilityStatusList(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 2fb44ca..ceb2f06 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon); > void hmp_info_mice(Monitor *mon); > void hmp_info_migrate(Monitor *mon); > void hmp_info_migration_capabilities(Monitor *mon); > +void hmp_info_migrate_parameters(Monitor *mon); > void hmp_info_cpus(Monitor *mon); > void hmp_info_block(Monitor *mon); > void hmp_info_blockstats(Monitor *mon); > @@ -52,6 +53,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); > 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_parameter(Monitor *mon, const QDict *qdict); > void hmp_set_password(Monitor *mon, const QDict *qdict); > void hmp_expire_password(Monitor *mon, const QDict *qdict); > void hmp_eject(Monitor *mon, const QDict *qdict); > diff --git a/migration.c b/migration.c > index 8c27347..e844290 100644 > --- a/migration.c > +++ b/migration.c > @@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void) > return max_downtime; > } > > +MigrationParameters *qmp_query_migrate_parameters(Error **errp) > +{ > + MigrationParameters *params = g_malloc0(sizeof(*params)); > + MigrationState *s = migrate_get_current(); > + int i; > + > + params->capabilities = g_malloc0(sizeof(*params->capabilities)); > + > + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > + params->capabilities->value = > + g_malloc(sizeof(*params->capabilities->value)); > + params->capabilities->value->capability = i; > + params->capabilities->value->state = s->enabled_capabilities[i]; > + params->capabilities->next = NULL; > + } > + > + return params; > +} > + > MigrationInfo *qmp_query_migrate(Error **errp) > { > MigrationInfo *info = g_malloc0(sizeof(*info)); > @@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp) > return caps_list; > } > > +void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params, > + Error **errp) > +{ > + MigrationState *s = migrate_get_current(); > + MigrationCapabilityStatusList *cap; > + > + if (s->state == MIG_STATE_ACTIVE) { > + error_set(errp, QERR_MIGRATION_ACTIVE); > + return; > + } > + > + for (cap = params; cap; cap = cap->next) { > + s->enabled_capabilities[cap->value->capability] = cap->value->state; > + } > +} > + > /* shared migration helpers */ > > static int migrate_fd_cleanup(MigrationState *s) > @@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params) > { > MigrationState *s = migrate_get_current(); > int64_t bandwidth_limit = s->bandwidth_limit; > + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > + > + memcpy(enabled_capabilities, s->enabled_capabilities, > + sizeof(enabled_capabilities)); Is s->enabled_capabilities initialized somewhere or are we making xbzrle disabled by default? > > memset(s, 0, sizeof(*s)); > s->bandwidth_limit = bandwidth_limit; > s->params = *params; > + memcpy(s->enabled_capabilities, enabled_capabilities, > + sizeof(enabled_capabilities)); > > - s->bandwidth_limit = bandwidth_limit; Unrelated change. > s->state = MIG_STATE_SETUP; > s->total_time = qemu_get_clock_ms(rt_clock); > > diff --git a/migration.h b/migration.h > index 57572a6..713aae0 100644 > --- a/migration.h > +++ b/migration.h > @@ -19,6 +19,7 @@ > #include "notify.h" > #include "error.h" > #include "vmstate.h" > +#include "qapi-types.h" > > struct MigrationParams { > bool blk; > @@ -39,6 +40,7 @@ struct MigrationState > void *opaque; > MigrationParams params; > int64_t total_time; > + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > }; > > void process_incoming_migration(QEMUFile *f); > diff --git a/monitor.c b/monitor.c > index fd57c5e..e07af97 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { > .mhandler.info = hmp_info_migration_capabilities, > }, > { > + .name = "migrate-parameters", > + .args_type = "", > + .params = "", > + .help = "show current migration parameters", > + .mhandler.info = hmp_info_migrate_parameters, > + }, > + { > .name = "balloon", > .args_type = "", > .params = "", > diff --git a/qapi-schema.json b/qapi-schema.json > index b4d4dd6..9d6759d 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -345,6 +345,38 @@ > { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } > > ## > +# @migrate-set-parameters > +# > +# Enable/Disable the following migration capabilities (like xbzrle) > +# > +# Since: 1.2 > +## > +{ 'command': 'migrate-set-parameters', > + 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } > + > +## > +# @MigrationParameters > +# > +# @capabilities: @MigrationCapabilityStatus list contain current migration > +# capabilities status > +# Since: 1.2 > +## > +{ 'type': 'MigrationParameters', > + 'data': {'capabilities': ['MigrationCapabilityStatus']} } > + > + > +## > +# @query-migrate-parameters > +# > +# Returns information about the current migration capabilities status > +# > +# Returns: @MigrationParameters > +# > +# Since: 1.2 > +## > +{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' } > + > +## > # @MouseInfo: > # > # Information about a mouse device. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index c0ed14c..7f40e2a 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2081,7 +2081,6 @@ The main json-object contains the following: > - "transferred": amount transferred (json-int) > - "remaining": amount remaining (json-int) > - "total": total (json-int) > - > Examples: > > 1. Before the first migration > @@ -2092,7 +2091,15 @@ Examples: > 2. Migration is done and has succeeded > > -> { "execute": "query-migrate" } > -<- { "return": { "status": "completed" } } > +<- { "return": { > + "status": "completed", > + "ram":{ > + "transferred":123, > + "remaining":123, > + "total":246 > + } > + } > + } > > 3. Migration is done and has failed > > @@ -2165,6 +2172,55 @@ EQMP > }, > > SQMP > +migrate-set-parameters > +------- > + > +Enable/Disable migration capabilities > + > +- "xbzrle": xbzrle support > + > +Arguments: > + > +Example: > + > +-> { "execute": "migrate-set-parameters" , "arguments": > + { "parameters": [ { "capability": "xbzrle", "state": true } ] } } > + > +EQMP > + > + { > + .name = "migrate_set_parameters", > + .args_type = "parameters:O", > + .params = "capability:s,state:b", > + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, > + }, > +SQMP > +query-migrate-parameters > +------- > + > +Query current migration parameters > + > +- "capabilities": migration capabilities state > + - "xbzrle" : XBZRLE state (json-bool) > + > +Arguments: > + > +Example: > + > +-> { "execute": "query-migrate-parameters" } > +<- { "return": { > + "capabilities" : [ { "capability" : "xbzrle", "state" : false } ] > + } > + } > +EQMP > + > + { > + .name = "query-migrate-parameters", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters, > + }, > + > +SQMP > query-balloon > ------------- >
On 07/29/2012 03:42 AM, Orit Wasserman wrote: > The management can enable/disable a capability for the next migration by using > migrate_set_parameter command. > The management can query the current migration capabilities using > query-migrate-parameters In addition to Luiz' nitpicks: Here in the commit message, you are mixing HMP (migrate_set_parameter) with QMP (query-migrate-parameters). It might be better to stick to HMP only (migrate_set_parameter, info migrate_parameters) or QMP only (migrate-set-parameter, query-migrate-parameters), or even list the spellings for both protocols. > +++ b/hmp-commands.hx > @@ -1419,6 +1433,8 @@ show user network stack connection states > show migration status > @item info migration_capabilities > show migration capabilities > +@item info migrate_parameters > +show current migration parameters > +++ b/monitor.c > @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { > .mhandler.info = hmp_info_migration_capabilities, > }, > { > + .name = "migrate-parameters", Is this the correct spelling? Or is this feeding the HMP interface, where you documented it above as 'info migrate_parameters'? > +++ b/qapi-schema.json > @@ -345,6 +345,38 @@ > { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } > > ## > +# @migrate-set-parameters > +# > +# Enable/Disable the following migration capabilities (like xbzrle) > +# > +# Since: 1.2 Same comment as in 1/11 about 1.2 vs. 1.2.0.
On Mon, 30 Jul 2012 12:11:52 -0600 Eric Blake <eblake@redhat.com> wrote: > On 07/29/2012 03:42 AM, Orit Wasserman wrote: > > The management can enable/disable a capability for the next migration by using > > migrate_set_parameter command. > > The management can query the current migration capabilities using > > query-migrate-parameters > > In addition to Luiz' nitpicks: > > Here in the commit message, you are mixing HMP (migrate_set_parameter) > with QMP (query-migrate-parameters). It might be better to stick to HMP > only (migrate_set_parameter, info migrate_parameters) or QMP only > (migrate-set-parameter, query-migrate-parameters), or even list the > spellings for both protocols. > > > +++ b/hmp-commands.hx > > > @@ -1419,6 +1433,8 @@ show user network stack connection states > > show migration status > > @item info migration_capabilities > > show migration capabilities > > +@item info migrate_parameters > > +show current migration parameters > > > +++ b/monitor.c > > @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { > > .mhandler.info = hmp_info_migration_capabilities, > > }, > > { > > + .name = "migrate-parameters", > > Is this the correct spelling? Or is this feeding the HMP interface, > where you documented it above as 'info migrate_parameters'? It's the HMP interface, yes. > > > +++ b/qapi-schema.json > > @@ -345,6 +345,38 @@ > > { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } > > > > ## > > +# @migrate-set-parameters > > +# > > +# Enable/Disable the following migration capabilities (like xbzrle) > > +# > > +# Since: 1.2 > > Same comment as in 1/11 about 1.2 vs. 1.2.0. >
Orit Wasserman <owasserm@redhat.com> wrote: > The management can enable/disable a capability for the next migration by using > migrate_set_parameter command. > The management can query the current migration capabilities using > query-migrate-parameters > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > Signed-off-by: Juan Quintela <quintela@redhat.com> I just noticed .... shouldn't we use migrate_set_parameter cache_size 256M ??? I think that even the old migration parameters could be added to this one? migrate_set_parameter downtime 30ms migrate_set_parameter bandwidth 1G This way everything would be more regular, and easier on libvirt, as new parameters would be trivial to ask for? Notice that I can understand that we have to maintain the old commands for compability, but we can do "regularly" for new ones? Later, Juan.
On 07/30/2012 01:12 PM, Juan Quintela wrote: > Orit Wasserman <owasserm@redhat.com> wrote: >> The management can enable/disable a capability for the next migration by using >> migrate_set_parameter command. >> The management can query the current migration capabilities using >> query-migrate-parameters >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > I just noticed .... shouldn't we use > > migrate_set_parameter cache_size 256M > ??? > > I think that even the old migration parameters could be added to this > one? > > migrate_set_parameter downtime 30ms > migrate_set_parameter bandwidth 1G The existing patch was written under the assumption that parameters were only ever bool; but you are proposing allowing non-bool parameters. > > This way everything would be more regular, and easier on libvirt, as new > parameters would be trivial to ask for? That depends on the QMP interface you come up with for expressing non-bool parameters. This patch series has already been churning for several months now, which makes it feel a bit late to be changing the interface to support non-bool parameters, unless we really like the idea and can afford another delay in getting this series in.
Juan Quintela <quintela@redhat.com> writes: > Orit Wasserman <owasserm@redhat.com> wrote: >> The management can enable/disable a capability for the next migration by using >> migrate_set_parameter command. >> The management can query the current migration capabilities using >> query-migrate-parameters >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > I just noticed .... shouldn't we use > > migrate_set_parameter cache_size 256M > ??? > > I think that even the old migration parameters could be added to this > one? > > migrate_set_parameter downtime 30ms > migrate_set_parameter bandwidth 1G > > This way everything would be more regular, and easier on libvirt, as new > parameters would be trivial to ask for? > > Notice that I can understand that we have to maintain the old commands > for compability, but we can do "regularly" for new ones? I think this is worse. How does libvirt determine which parameters are valid? Let's do migrate_set_cache_size. Please don't introduce commands that multiplex multiple behavior. They do more harm than good. Regards, Anthony Liguori > > Later, Juan.
Orit Wasserman <owasserm@redhat.com> writes: > The management can enable/disable a capability for the next migration by using > migrate_set_parameter command. > The management can query the current migration capabilities using > query-migrate-parameters > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > Signed-off-by: Juan Quintela <quintela@redhat.com> We have a way to add new commands. Let's not invent a new one. Otherwise every subsystem would have it's own approach to querying what's available. Regards, Anthony Liguori > --- > hmp-commands.hx | 16 ++++++++++++ > hmp.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > hmp.h | 2 + > migration.c | 42 +++++++++++++++++++++++++++++++- > migration.h | 2 + > monitor.c | 7 +++++ > qapi-schema.json | 32 ++++++++++++++++++++++++ > qmp-commands.hx | 60 ++++++++++++++++++++++++++++++++++++++++++++- > 8 files changed, 229 insertions(+), 3 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 8786148..3e15338 100644 > --- a/hmp-commands.hx > +++ 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 = "capability state", > + .help = "Enable/Disable the usage of a capability for migration", > + .mhandler.cmd = hmp_migrate_set_parameter, > + }, > + > +STEXI > +@item migrate_set_parameter @var{capability} @var{state} > +@findex migrate_set_parameter > +Enable/Disable the usage of a capability @var{capability} 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", > @@ -1419,6 +1433,8 @@ show user network stack connection states > show migration status > @item info migration_capabilities > show migration capabilities > +@item info migrate_parameters > +show current migration parameters > @item info balloon > show balloon information > @item info qtree > diff --git a/hmp.c b/hmp.c > index 5c7d0be..f2f63fd 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) > void hmp_info_migrate(Monitor *mon) > { > MigrationInfo *info; > + MigrationCapabilityStatusList *cap; > + MigrationParameters *params; > > info = qmp_query_migrate(NULL); > + params = qmp_query_migrate_parameters(NULL); > + > + /* do not display parameters during setup */ > + if (info->has_status && params->capabilities) { > + monitor_printf(mon, "capabilities: "); > + for (cap = params->capabilities; cap; cap = cap->next) { > + monitor_printf(mon, "%s: %s ", > + MigrationCapability_lookup[cap->value->capability], > + cap->value->state ? "on" : "off"); > + } > + monitor_printf(mon, "\n"); > + } > > if (info->has_status) { > monitor_printf(mon, "Migration status: %s\n", info->status); > @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) > } > > qapi_free_MigrationInfo(info); > + qapi_free_MigrationParameters(params); > } > > void hmp_info_migration_capabilities(Monitor *mon) > @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) > qapi_free_MigrationCapabilityStatusList(caps_list); > } > > +void hmp_info_migrate_parameters(Monitor *mon) > +{ > + > + MigrationCapabilityStatusList *cap; > + MigrationParameters *params; > + > + params = qmp_query_migrate_parameters(NULL); > + > + if (params->capabilities) { > + monitor_printf(mon, "capabilities: "); > + for (cap = params->capabilities; cap; cap = cap->next) { > + monitor_printf(mon, "%s: %s ", > + MigrationCapability_lookup[cap->value->capability], > + cap->value->state ? "on" : "off"); > + } > + monitor_printf(mon, "\n"); > + } > + > + qapi_free_MigrationParameters(params); > +} > + > void hmp_info_cpus(Monitor *mon) > { > CpuInfoList *cpu_list, *cpu; > @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) > qmp_migrate_set_speed(value, NULL); > } > > +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > +{ > + const char *cap = qdict_get_str(qdict, "capability"); > + bool state = qdict_get_bool(qdict, "state"); > + Error *err = NULL; > + MigrationCapabilityStatusList *params = NULL; > + int i; > + > + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > + if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { > + if (!params) { > + params = g_malloc0(sizeof(*params)); > + } > + params->value = g_malloc0(sizeof(*params->value)); > + params->value->capability = i; > + params->value->state = state; > + params->next = NULL; > + qmp_migrate_set_parameters(params, &err); > + break; > + } > + } > + > + if (i == MIGRATION_CAPABILITY_MAX) { > + error_set(&err, QERR_INVALID_PARAMETER, cap); > + } > + > + qapi_free_MigrationCapabilityStatusList(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 2fb44ca..ceb2f06 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon); > void hmp_info_mice(Monitor *mon); > void hmp_info_migrate(Monitor *mon); > void hmp_info_migration_capabilities(Monitor *mon); > +void hmp_info_migrate_parameters(Monitor *mon); > void hmp_info_cpus(Monitor *mon); > void hmp_info_block(Monitor *mon); > void hmp_info_blockstats(Monitor *mon); > @@ -52,6 +53,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); > 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_parameter(Monitor *mon, const QDict *qdict); > void hmp_set_password(Monitor *mon, const QDict *qdict); > void hmp_expire_password(Monitor *mon, const QDict *qdict); > void hmp_eject(Monitor *mon, const QDict *qdict); > diff --git a/migration.c b/migration.c > index 8c27347..e844290 100644 > --- a/migration.c > +++ b/migration.c > @@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void) > return max_downtime; > } > > +MigrationParameters *qmp_query_migrate_parameters(Error **errp) > +{ > + MigrationParameters *params = g_malloc0(sizeof(*params)); > + MigrationState *s = migrate_get_current(); > + int i; > + > + params->capabilities = g_malloc0(sizeof(*params->capabilities)); > + > + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > + params->capabilities->value = > + g_malloc(sizeof(*params->capabilities->value)); > + params->capabilities->value->capability = i; > + params->capabilities->value->state = s->enabled_capabilities[i]; > + params->capabilities->next = NULL; > + } > + > + return params; > +} > + > MigrationInfo *qmp_query_migrate(Error **errp) > { > MigrationInfo *info = g_malloc0(sizeof(*info)); > @@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp) > return caps_list; > } > > +void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params, > + Error **errp) > +{ > + MigrationState *s = migrate_get_current(); > + MigrationCapabilityStatusList *cap; > + > + if (s->state == MIG_STATE_ACTIVE) { > + error_set(errp, QERR_MIGRATION_ACTIVE); > + return; > + } > + > + for (cap = params; cap; cap = cap->next) { > + s->enabled_capabilities[cap->value->capability] = cap->value->state; > + } > +} > + > /* shared migration helpers */ > > static int migrate_fd_cleanup(MigrationState *s) > @@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params) > { > MigrationState *s = migrate_get_current(); > int64_t bandwidth_limit = s->bandwidth_limit; > + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > + > + memcpy(enabled_capabilities, s->enabled_capabilities, > + sizeof(enabled_capabilities)); > > memset(s, 0, sizeof(*s)); > s->bandwidth_limit = bandwidth_limit; > s->params = *params; > + memcpy(s->enabled_capabilities, enabled_capabilities, > + sizeof(enabled_capabilities)); > > - s->bandwidth_limit = bandwidth_limit; > s->state = MIG_STATE_SETUP; > s->total_time = qemu_get_clock_ms(rt_clock); > > diff --git a/migration.h b/migration.h > index 57572a6..713aae0 100644 > --- a/migration.h > +++ b/migration.h > @@ -19,6 +19,7 @@ > #include "notify.h" > #include "error.h" > #include "vmstate.h" > +#include "qapi-types.h" > > struct MigrationParams { > bool blk; > @@ -39,6 +40,7 @@ struct MigrationState > void *opaque; > MigrationParams params; > int64_t total_time; > + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > }; > > void process_incoming_migration(QEMUFile *f); > diff --git a/monitor.c b/monitor.c > index fd57c5e..e07af97 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { > .mhandler.info = hmp_info_migration_capabilities, > }, > { > + .name = "migrate-parameters", > + .args_type = "", > + .params = "", > + .help = "show current migration parameters", > + .mhandler.info = hmp_info_migrate_parameters, > + }, > + { > .name = "balloon", > .args_type = "", > .params = "", > diff --git a/qapi-schema.json b/qapi-schema.json > index b4d4dd6..9d6759d 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -345,6 +345,38 @@ > { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } > > ## > +# @migrate-set-parameters > +# > +# Enable/Disable the following migration capabilities (like xbzrle) > +# > +# Since: 1.2 > +## > +{ 'command': 'migrate-set-parameters', > + 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } > + > +## > +# @MigrationParameters > +# > +# @capabilities: @MigrationCapabilityStatus list contain current migration > +# capabilities status > +# Since: 1.2 > +## > +{ 'type': 'MigrationParameters', > + 'data': {'capabilities': ['MigrationCapabilityStatus']} } > + > + > +## > +# @query-migrate-parameters > +# > +# Returns information about the current migration capabilities status > +# > +# Returns: @MigrationParameters > +# > +# Since: 1.2 > +## > +{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' } > + > +## > # @MouseInfo: > # > # Information about a mouse device. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index c0ed14c..7f40e2a 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2081,7 +2081,6 @@ The main json-object contains the following: > - "transferred": amount transferred (json-int) > - "remaining": amount remaining (json-int) > - "total": total (json-int) > - > Examples: > > 1. Before the first migration > @@ -2092,7 +2091,15 @@ Examples: > 2. Migration is done and has succeeded > > -> { "execute": "query-migrate" } > -<- { "return": { "status": "completed" } } > +<- { "return": { > + "status": "completed", > + "ram":{ > + "transferred":123, > + "remaining":123, > + "total":246 > + } > + } > + } > > 3. Migration is done and has failed > > @@ -2165,6 +2172,55 @@ EQMP > }, > > SQMP > +migrate-set-parameters > +------- > + > +Enable/Disable migration capabilities > + > +- "xbzrle": xbzrle support > + > +Arguments: > + > +Example: > + > +-> { "execute": "migrate-set-parameters" , "arguments": > + { "parameters": [ { "capability": "xbzrle", "state": true } ] } } > + > +EQMP > + > + { > + .name = "migrate_set_parameters", > + .args_type = "parameters:O", > + .params = "capability:s,state:b", > + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, > + }, > +SQMP > +query-migrate-parameters > +------- > + > +Query current migration parameters > + > +- "capabilities": migration capabilities state > + - "xbzrle" : XBZRLE state (json-bool) > + > +Arguments: > + > +Example: > + > +-> { "execute": "query-migrate-parameters" } > +<- { "return": { > + "capabilities" : [ { "capability" : "xbzrle", "state" : false } ] > + } > + } > +EQMP > + > + { > + .name = "query-migrate-parameters", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters, > + }, > + > +SQMP > query-balloon > ------------- > > -- > 1.7.7.6
On Mon, 30 Jul 2012 14:45:04 -0500 Anthony Liguori <aliguori@us.ibm.com> wrote: > Orit Wasserman <owasserm@redhat.com> writes: > > > The management can enable/disable a capability for the next migration by using > > migrate_set_parameter command. > > The management can query the current migration capabilities using > > query-migrate-parameters > > > > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > We have a way to add new commands. Let's not invent a new one. > Otherwise every subsystem would have it's own approach to querying > what's available. I think it does make sense for setting/getting migration's capabilities, which are just booleans. And that's what the commands currently do, btw. I'd only recommend to rename them to migrate_set_capability query-migrate-capabilities.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Mon, 30 Jul 2012 14:45:04 -0500 > Anthony Liguori <aliguori@us.ibm.com> wrote: > >> Orit Wasserman <owasserm@redhat.com> writes: >> >> > The management can enable/disable a capability for the next migration by using >> > migrate_set_parameter command. >> > The management can query the current migration capabilities using >> > query-migrate-parameters >> > >> > Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> > Signed-off-by: Juan Quintela <quintela@redhat.com> >> >> We have a way to add new commands. Let's not invent a new one. >> Otherwise every subsystem would have it's own approach to querying >> what's available. > > I think it does make sense for setting/getting migration's capabilities, which > are just booleans. And that's what the commands currently do, btw. > > I'd only recommend to rename them to migrate_set_capability > query-migrate-capabilities. If that's the intent, it should take/return a list of capabilities (expressed as an enum). Regards, Anthony Liguori
On Mon, 30 Jul 2012 15:04:57 -0500 Anthony Liguori <aliguori@us.ibm.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > On Mon, 30 Jul 2012 14:45:04 -0500 > > Anthony Liguori <aliguori@us.ibm.com> wrote: > > > >> Orit Wasserman <owasserm@redhat.com> writes: > >> > >> > The management can enable/disable a capability for the next migration by using > >> > migrate_set_parameter command. > >> > The management can query the current migration capabilities using > >> > query-migrate-parameters > >> > > >> > Signed-off-by: Orit Wasserman <owasserm@redhat.com> > >> > Signed-off-by: Juan Quintela <quintela@redhat.com> > >> > >> We have a way to add new commands. Let's not invent a new one. > >> Otherwise every subsystem would have it's own approach to querying > >> what's available. > > > > I think it does make sense for setting/getting migration's capabilities, which > > are just booleans. And that's what the commands currently do, btw. > > > > I'd only recommend to rename them to migrate_set_capability > > query-migrate-capabilities. > > If that's the intent, it should take/return a list of capabilities > (expressed as an enum). Yes, it already does it iirc. Now, there's something I'm not sure about. This series adds three commands: - migrate-set-parameter (should be renamed to migrate-set-capabilities) - query-migrate-parameters (should be renamed to query-migrate-capabilities) - query-migration-capabilities (should be dropped?) That last command returns the supported capabilities. We should either, rename it to query-migration-supported-capabilities or just drop it, because if a capability appears in (this series') query-migrate-parameters it means that the capability is supported.
Juan Quintela <quintela@redhat.com> wrote: > Orit Wasserman <owasserm@redhat.com> wrote: >> The management can enable/disable a capability for the next migration by using >> migrate_set_parameter command. >> The management can query the current migration capabilities using >> query-migrate-parameters >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > I just noticed .... shouldn't we use > > migrate_set_parameter cache_size 256M > ??? > > I think that even the old migration parameters could be added to this > one? > > migrate_set_parameter downtime 30ms > migrate_set_parameter bandwidth 1G /me nacks myself. It appears that I am the only one that thinks that command explosion is bad O;-) (and yes, it is a different compromise.) > > This way everything would be more regular, and easier on libvirt, as new > parameters would be trivial to ask for? > > Notice that I can understand that we have to maintain the old commands > for compability, but we can do "regularly" for new ones? > > Later, Juan.
On 07/30/2012 08:41 PM, Luiz Capitulino wrote: > On Sun, 29 Jul 2012 12:42:54 +0300 > Orit Wasserman <owasserm@redhat.com> wrote: > >> The management can enable/disable a capability for the next migration by using >> migrate_set_parameter command. >> The management can query the current migration capabilities using >> query-migrate-parameters > > In general looks good to me, I have a question and a few nitpick comments > below. > >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> hmp-commands.hx | 16 ++++++++++++ >> hmp.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> hmp.h | 2 + >> migration.c | 42 +++++++++++++++++++++++++++++++- >> migration.h | 2 + >> monitor.c | 7 +++++ >> qapi-schema.json | 32 ++++++++++++++++++++++++ >> qmp-commands.hx | 60 ++++++++++++++++++++++++++++++++++++++++++++- >> 8 files changed, 229 insertions(+), 3 deletions(-) >> >> diff --git a/hmp-commands.hx b/hmp-commands.hx >> index 8786148..3e15338 100644 >> --- a/hmp-commands.hx >> +++ 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 = "capability state", >> + .help = "Enable/Disable the usage of a capability for migration", >> + .mhandler.cmd = hmp_migrate_set_parameter, >> + }, >> + >> +STEXI >> +@item migrate_set_parameter @var{capability} @var{state} >> +@findex migrate_set_parameter >> +Enable/Disable the usage of a capability @var{capability} 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", >> @@ -1419,6 +1433,8 @@ show user network stack connection states >> show migration status >> @item info migration_capabilities >> show migration capabilities >> +@item info migrate_parameters >> +show current migration parameters >> @item info balloon >> show balloon information >> @item info qtree >> diff --git a/hmp.c b/hmp.c >> index 5c7d0be..f2f63fd 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) >> void hmp_info_migrate(Monitor *mon) >> { >> MigrationInfo *info; >> + MigrationCapabilityStatusList *cap; >> + MigrationParameters *params; >> >> info = qmp_query_migrate(NULL); >> + params = qmp_query_migrate_parameters(NULL); >> + >> + /* do not display parameters during setup */ >> + if (info->has_status && params->capabilities) { >> + monitor_printf(mon, "capabilities: "); >> + for (cap = params->capabilities; cap; cap = cap->next) { >> + monitor_printf(mon, "%s: %s ", >> + MigrationCapability_lookup[cap->value->capability], >> + cap->value->state ? "on" : "off"); >> + } >> + monitor_printf(mon, "\n"); >> + } >> >> if (info->has_status) { >> monitor_printf(mon, "Migration status: %s\n", info->status); >> @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) >> } >> >> qapi_free_MigrationInfo(info); >> + qapi_free_MigrationParameters(params); >> } >> >> void hmp_info_migration_capabilities(Monitor *mon) >> @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) >> qapi_free_MigrationCapabilityStatusList(caps_list); >> } >> >> +void hmp_info_migrate_parameters(Monitor *mon) >> +{ >> + >> + MigrationCapabilityStatusList *cap; >> + MigrationParameters *params; >> + >> + params = qmp_query_migrate_parameters(NULL); >> + >> + if (params->capabilities) { >> + monitor_printf(mon, "capabilities: "); >> + for (cap = params->capabilities; cap; cap = cap->next) { >> + monitor_printf(mon, "%s: %s ", >> + MigrationCapability_lookup[cap->value->capability], >> + cap->value->state ? "on" : "off"); >> + } >> + monitor_printf(mon, "\n"); >> + } >> + >> + qapi_free_MigrationParameters(params); >> +} >> + >> void hmp_info_cpus(Monitor *mon) >> { >> CpuInfoList *cpu_list, *cpu; >> @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) >> qmp_migrate_set_speed(value, NULL); >> } >> >> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> +{ >> + const char *cap = qdict_get_str(qdict, "capability"); >> + bool state = qdict_get_bool(qdict, "state"); >> + Error *err = NULL; >> + MigrationCapabilityStatusList *params = NULL; >> + int i; >> + >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { >> + if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { >> + if (!params) { >> + params = g_malloc0(sizeof(*params)); >> + } > > This if shouldn't be necessary, right? You are right I can always allocate it. > >> + params->value = g_malloc0(sizeof(*params->value)); >> + params->value->capability = i; >> + params->value->state = state; >> + params->next = NULL; > > nitpick: I'd make the code ready for future capabilities. Could you give me a bit more details ? Orit > >> + qmp_migrate_set_parameters(params, &err); >> + break; >> + } >> + } >> + >> + if (i == MIGRATION_CAPABILITY_MAX) { >> + error_set(&err, QERR_INVALID_PARAMETER, cap); >> + } >> + >> + qapi_free_MigrationCapabilityStatusList(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 2fb44ca..ceb2f06 100644 >> --- a/hmp.h >> +++ b/hmp.h >> @@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon); >> void hmp_info_mice(Monitor *mon); >> void hmp_info_migrate(Monitor *mon); >> void hmp_info_migration_capabilities(Monitor *mon); >> +void hmp_info_migrate_parameters(Monitor *mon); >> void hmp_info_cpus(Monitor *mon); >> void hmp_info_block(Monitor *mon); >> void hmp_info_blockstats(Monitor *mon); >> @@ -52,6 +53,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); >> 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_parameter(Monitor *mon, const QDict *qdict); >> void hmp_set_password(Monitor *mon, const QDict *qdict); >> void hmp_expire_password(Monitor *mon, const QDict *qdict); >> void hmp_eject(Monitor *mon, const QDict *qdict); >> diff --git a/migration.c b/migration.c >> index 8c27347..e844290 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void) >> return max_downtime; >> } >> >> +MigrationParameters *qmp_query_migrate_parameters(Error **errp) >> +{ >> + MigrationParameters *params = g_malloc0(sizeof(*params)); >> + MigrationState *s = migrate_get_current(); >> + int i; >> + >> + params->capabilities = g_malloc0(sizeof(*params->capabilities)); >> + >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { >> + params->capabilities->value = >> + g_malloc(sizeof(*params->capabilities->value)); >> + params->capabilities->value->capability = i; >> + params->capabilities->value->state = s->enabled_capabilities[i]; >> + params->capabilities->next = NULL; >> + } >> + >> + return params; >> +} >> + >> MigrationInfo *qmp_query_migrate(Error **errp) >> { >> MigrationInfo *info = g_malloc0(sizeof(*info)); >> @@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp) >> return caps_list; >> } >> >> +void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params, >> + Error **errp) >> +{ >> + MigrationState *s = migrate_get_current(); >> + MigrationCapabilityStatusList *cap; >> + >> + if (s->state == MIG_STATE_ACTIVE) { >> + error_set(errp, QERR_MIGRATION_ACTIVE); >> + return; >> + } >> + >> + for (cap = params; cap; cap = cap->next) { >> + s->enabled_capabilities[cap->value->capability] = cap->value->state; >> + } >> +} >> + >> /* shared migration helpers */ >> >> static int migrate_fd_cleanup(MigrationState *s) >> @@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params) >> { >> MigrationState *s = migrate_get_current(); >> int64_t bandwidth_limit = s->bandwidth_limit; >> + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; >> + >> + memcpy(enabled_capabilities, s->enabled_capabilities, >> + sizeof(enabled_capabilities)); > > Is s->enabled_capabilities initialized somewhere or are we making xbzrle > disabled by default? > >> >> memset(s, 0, sizeof(*s)); >> s->bandwidth_limit = bandwidth_limit; >> s->params = *params; >> + memcpy(s->enabled_capabilities, enabled_capabilities, >> + sizeof(enabled_capabilities)); >> >> - s->bandwidth_limit = bandwidth_limit; > > Unrelated change. > >> s->state = MIG_STATE_SETUP; >> s->total_time = qemu_get_clock_ms(rt_clock); >> >> diff --git a/migration.h b/migration.h >> index 57572a6..713aae0 100644 >> --- a/migration.h >> +++ b/migration.h >> @@ -19,6 +19,7 @@ >> #include "notify.h" >> #include "error.h" >> #include "vmstate.h" >> +#include "qapi-types.h" >> >> struct MigrationParams { >> bool blk; >> @@ -39,6 +40,7 @@ struct MigrationState >> void *opaque; >> MigrationParams params; >> int64_t total_time; >> + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; >> }; >> >> void process_incoming_migration(QEMUFile *f); >> diff --git a/monitor.c b/monitor.c >> index fd57c5e..e07af97 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { >> .mhandler.info = hmp_info_migration_capabilities, >> }, >> { >> + .name = "migrate-parameters", >> + .args_type = "", >> + .params = "", >> + .help = "show current migration parameters", >> + .mhandler.info = hmp_info_migrate_parameters, >> + }, >> + { >> .name = "balloon", >> .args_type = "", >> .params = "", >> diff --git a/qapi-schema.json b/qapi-schema.json >> index b4d4dd6..9d6759d 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -345,6 +345,38 @@ >> { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } >> >> ## >> +# @migrate-set-parameters >> +# >> +# Enable/Disable the following migration capabilities (like xbzrle) >> +# >> +# Since: 1.2 >> +## >> +{ 'command': 'migrate-set-parameters', >> + 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } >> + >> +## >> +# @MigrationParameters >> +# >> +# @capabilities: @MigrationCapabilityStatus list contain current migration >> +# capabilities status >> +# Since: 1.2 >> +## >> +{ 'type': 'MigrationParameters', >> + 'data': {'capabilities': ['MigrationCapabilityStatus']} } >> + >> + >> +## >> +# @query-migrate-parameters >> +# >> +# Returns information about the current migration capabilities status >> +# >> +# Returns: @MigrationParameters >> +# >> +# Since: 1.2 >> +## >> +{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' } >> + >> +## >> # @MouseInfo: >> # >> # Information about a mouse device. >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index c0ed14c..7f40e2a 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -2081,7 +2081,6 @@ The main json-object contains the following: >> - "transferred": amount transferred (json-int) >> - "remaining": amount remaining (json-int) >> - "total": total (json-int) >> - >> Examples: >> >> 1. Before the first migration >> @@ -2092,7 +2091,15 @@ Examples: >> 2. Migration is done and has succeeded >> >> -> { "execute": "query-migrate" } >> -<- { "return": { "status": "completed" } } >> +<- { "return": { >> + "status": "completed", >> + "ram":{ >> + "transferred":123, >> + "remaining":123, >> + "total":246 >> + } >> + } >> + } >> >> 3. Migration is done and has failed >> >> @@ -2165,6 +2172,55 @@ EQMP >> }, >> >> SQMP >> +migrate-set-parameters >> +------- >> + >> +Enable/Disable migration capabilities >> + >> +- "xbzrle": xbzrle support >> + >> +Arguments: >> + >> +Example: >> + >> +-> { "execute": "migrate-set-parameters" , "arguments": >> + { "parameters": [ { "capability": "xbzrle", "state": true } ] } } >> + >> +EQMP >> + >> + { >> + .name = "migrate_set_parameters", >> + .args_type = "parameters:O", >> + .params = "capability:s,state:b", >> + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, >> + }, >> +SQMP >> +query-migrate-parameters >> +------- >> + >> +Query current migration parameters >> + >> +- "capabilities": migration capabilities state >> + - "xbzrle" : XBZRLE state (json-bool) >> + >> +Arguments: >> + >> +Example: >> + >> +-> { "execute": "query-migrate-parameters" } >> +<- { "return": { >> + "capabilities" : [ { "capability" : "xbzrle", "state" : false } ] >> + } >> + } >> +EQMP >> + >> + { >> + .name = "query-migrate-parameters", >> + .args_type = "", >> + .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters, >> + }, >> + >> +SQMP >> query-balloon >> ------------- >> >
On 07/30/2012 08:41 PM, Luiz Capitulino wrote: > On Sun, 29 Jul 2012 12:42:54 +0300 > Orit Wasserman <owasserm@redhat.com> wrote: > >> The management can enable/disable a capability for the next migration by using >> migrate_set_parameter command. >> The management can query the current migration capabilities using >> query-migrate-parameters > > In general looks good to me, I have a question and a few nitpick comments > below. > >> >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> hmp-commands.hx | 16 ++++++++++++ >> hmp.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> hmp.h | 2 + >> migration.c | 42 +++++++++++++++++++++++++++++++- >> migration.h | 2 + >> monitor.c | 7 +++++ >> qapi-schema.json | 32 ++++++++++++++++++++++++ >> qmp-commands.hx | 60 ++++++++++++++++++++++++++++++++++++++++++++- >> 8 files changed, 229 insertions(+), 3 deletions(-) >> >> diff --git a/hmp-commands.hx b/hmp-commands.hx >> index 8786148..3e15338 100644 >> --- a/hmp-commands.hx >> +++ 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 = "capability state", >> + .help = "Enable/Disable the usage of a capability for migration", >> + .mhandler.cmd = hmp_migrate_set_parameter, >> + }, >> + >> +STEXI >> +@item migrate_set_parameter @var{capability} @var{state} >> +@findex migrate_set_parameter >> +Enable/Disable the usage of a capability @var{capability} 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", >> @@ -1419,6 +1433,8 @@ show user network stack connection states >> show migration status >> @item info migration_capabilities >> show migration capabilities >> +@item info migrate_parameters >> +show current migration parameters >> @item info balloon >> show balloon information >> @item info qtree >> diff --git a/hmp.c b/hmp.c >> index 5c7d0be..f2f63fd 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) >> void hmp_info_migrate(Monitor *mon) >> { >> MigrationInfo *info; >> + MigrationCapabilityStatusList *cap; >> + MigrationParameters *params; >> >> info = qmp_query_migrate(NULL); >> + params = qmp_query_migrate_parameters(NULL); >> + >> + /* do not display parameters during setup */ >> + if (info->has_status && params->capabilities) { >> + monitor_printf(mon, "capabilities: "); >> + for (cap = params->capabilities; cap; cap = cap->next) { >> + monitor_printf(mon, "%s: %s ", >> + MigrationCapability_lookup[cap->value->capability], >> + cap->value->state ? "on" : "off"); >> + } >> + monitor_printf(mon, "\n"); >> + } >> >> if (info->has_status) { >> monitor_printf(mon, "Migration status: %s\n", info->status); >> @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) >> } >> >> qapi_free_MigrationInfo(info); >> + qapi_free_MigrationParameters(params); >> } >> >> void hmp_info_migration_capabilities(Monitor *mon) >> @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) >> qapi_free_MigrationCapabilityStatusList(caps_list); >> } >> >> +void hmp_info_migrate_parameters(Monitor *mon) >> +{ >> + >> + MigrationCapabilityStatusList *cap; >> + MigrationParameters *params; >> + >> + params = qmp_query_migrate_parameters(NULL); >> + >> + if (params->capabilities) { >> + monitor_printf(mon, "capabilities: "); >> + for (cap = params->capabilities; cap; cap = cap->next) { >> + monitor_printf(mon, "%s: %s ", >> + MigrationCapability_lookup[cap->value->capability], >> + cap->value->state ? "on" : "off"); >> + } >> + monitor_printf(mon, "\n"); >> + } >> + >> + qapi_free_MigrationParameters(params); >> +} >> + >> void hmp_info_cpus(Monitor *mon) >> { >> CpuInfoList *cpu_list, *cpu; >> @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) >> qmp_migrate_set_speed(value, NULL); >> } >> >> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) >> +{ >> + const char *cap = qdict_get_str(qdict, "capability"); >> + bool state = qdict_get_bool(qdict, "state"); >> + Error *err = NULL; >> + MigrationCapabilityStatusList *params = NULL; >> + int i; >> + >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { >> + if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { >> + if (!params) { >> + params = g_malloc0(sizeof(*params)); >> + } > > This if shouldn't be necessary, right? > >> + params->value = g_malloc0(sizeof(*params->value)); >> + params->value->capability = i; >> + params->value->state = state; >> + params->next = NULL; > > nitpick: I'd make the code ready for future capabilities. > >> + qmp_migrate_set_parameters(params, &err); >> + break; >> + } >> + } >> + >> + if (i == MIGRATION_CAPABILITY_MAX) { >> + error_set(&err, QERR_INVALID_PARAMETER, cap); >> + } >> + >> + qapi_free_MigrationCapabilityStatusList(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 2fb44ca..ceb2f06 100644 >> --- a/hmp.h >> +++ b/hmp.h >> @@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon); >> void hmp_info_mice(Monitor *mon); >> void hmp_info_migrate(Monitor *mon); >> void hmp_info_migration_capabilities(Monitor *mon); >> +void hmp_info_migrate_parameters(Monitor *mon); >> void hmp_info_cpus(Monitor *mon); >> void hmp_info_block(Monitor *mon); >> void hmp_info_blockstats(Monitor *mon); >> @@ -52,6 +53,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); >> 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_parameter(Monitor *mon, const QDict *qdict); >> void hmp_set_password(Monitor *mon, const QDict *qdict); >> void hmp_expire_password(Monitor *mon, const QDict *qdict); >> void hmp_eject(Monitor *mon, const QDict *qdict); >> diff --git a/migration.c b/migration.c >> index 8c27347..e844290 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void) >> return max_downtime; >> } >> >> +MigrationParameters *qmp_query_migrate_parameters(Error **errp) >> +{ >> + MigrationParameters *params = g_malloc0(sizeof(*params)); >> + MigrationState *s = migrate_get_current(); >> + int i; >> + >> + params->capabilities = g_malloc0(sizeof(*params->capabilities)); >> + >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { >> + params->capabilities->value = >> + g_malloc(sizeof(*params->capabilities->value)); >> + params->capabilities->value->capability = i; >> + params->capabilities->value->state = s->enabled_capabilities[i]; >> + params->capabilities->next = NULL; >> + } >> + >> + return params; >> +} >> + >> MigrationInfo *qmp_query_migrate(Error **errp) >> { >> MigrationInfo *info = g_malloc0(sizeof(*info)); >> @@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp) >> return caps_list; >> } >> >> +void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params, >> + Error **errp) >> +{ >> + MigrationState *s = migrate_get_current(); >> + MigrationCapabilityStatusList *cap; >> + >> + if (s->state == MIG_STATE_ACTIVE) { >> + error_set(errp, QERR_MIGRATION_ACTIVE); >> + return; >> + } >> + >> + for (cap = params; cap; cap = cap->next) { >> + s->enabled_capabilities[cap->value->capability] = cap->value->state; >> + } >> +} >> + >> /* shared migration helpers */ >> >> static int migrate_fd_cleanup(MigrationState *s) >> @@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params) >> { >> MigrationState *s = migrate_get_current(); >> int64_t bandwidth_limit = s->bandwidth_limit; >> + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; >> + >> + memcpy(enabled_capabilities, s->enabled_capabilities, >> + sizeof(enabled_capabilities)); > > Is s->enabled_capabilities initialized somewhere or are we making xbzrle > disabled by default? It is automatically zeroed because it is static, which means all capabilities are disabled by default Orit > >> >> memset(s, 0, sizeof(*s)); >> s->bandwidth_limit = bandwidth_limit; >> s->params = *params; >> + memcpy(s->enabled_capabilities, enabled_capabilities, >> + sizeof(enabled_capabilities)); >> >> - s->bandwidth_limit = bandwidth_limit; > > Unrelated change. > >> s->state = MIG_STATE_SETUP; >> s->total_time = qemu_get_clock_ms(rt_clock); >> >> diff --git a/migration.h b/migration.h >> index 57572a6..713aae0 100644 >> --- a/migration.h >> +++ b/migration.h >> @@ -19,6 +19,7 @@ >> #include "notify.h" >> #include "error.h" >> #include "vmstate.h" >> +#include "qapi-types.h" >> >> struct MigrationParams { >> bool blk; >> @@ -39,6 +40,7 @@ struct MigrationState >> void *opaque; >> MigrationParams params; >> int64_t total_time; >> + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; >> }; >> >> void process_incoming_migration(QEMUFile *f); >> diff --git a/monitor.c b/monitor.c >> index fd57c5e..e07af97 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { >> .mhandler.info = hmp_info_migration_capabilities, >> }, >> { >> + .name = "migrate-parameters", >> + .args_type = "", >> + .params = "", >> + .help = "show current migration parameters", >> + .mhandler.info = hmp_info_migrate_parameters, >> + }, >> + { >> .name = "balloon", >> .args_type = "", >> .params = "", >> diff --git a/qapi-schema.json b/qapi-schema.json >> index b4d4dd6..9d6759d 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -345,6 +345,38 @@ >> { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } >> >> ## >> +# @migrate-set-parameters >> +# >> +# Enable/Disable the following migration capabilities (like xbzrle) >> +# >> +# Since: 1.2 >> +## >> +{ 'command': 'migrate-set-parameters', >> + 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } >> + >> +## >> +# @MigrationParameters >> +# >> +# @capabilities: @MigrationCapabilityStatus list contain current migration >> +# capabilities status >> +# Since: 1.2 >> +## >> +{ 'type': 'MigrationParameters', >> + 'data': {'capabilities': ['MigrationCapabilityStatus']} } >> + >> + >> +## >> +# @query-migrate-parameters >> +# >> +# Returns information about the current migration capabilities status >> +# >> +# Returns: @MigrationParameters >> +# >> +# Since: 1.2 >> +## >> +{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' } >> + >> +## >> # @MouseInfo: >> # >> # Information about a mouse device. >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index c0ed14c..7f40e2a 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -2081,7 +2081,6 @@ The main json-object contains the following: >> - "transferred": amount transferred (json-int) >> - "remaining": amount remaining (json-int) >> - "total": total (json-int) >> - >> Examples: >> >> 1. Before the first migration >> @@ -2092,7 +2091,15 @@ Examples: >> 2. Migration is done and has succeeded >> >> -> { "execute": "query-migrate" } >> -<- { "return": { "status": "completed" } } >> +<- { "return": { >> + "status": "completed", >> + "ram":{ >> + "transferred":123, >> + "remaining":123, >> + "total":246 >> + } >> + } >> + } >> >> 3. Migration is done and has failed >> >> @@ -2165,6 +2172,55 @@ EQMP >> }, >> >> SQMP >> +migrate-set-parameters >> +------- >> + >> +Enable/Disable migration capabilities >> + >> +- "xbzrle": xbzrle support >> + >> +Arguments: >> + >> +Example: >> + >> +-> { "execute": "migrate-set-parameters" , "arguments": >> + { "parameters": [ { "capability": "xbzrle", "state": true } ] } } >> + >> +EQMP >> + >> + { >> + .name = "migrate_set_parameters", >> + .args_type = "parameters:O", >> + .params = "capability:s,state:b", >> + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, >> + }, >> +SQMP >> +query-migrate-parameters >> +------- >> + >> +Query current migration parameters >> + >> +- "capabilities": migration capabilities state >> + - "xbzrle" : XBZRLE state (json-bool) >> + >> +Arguments: >> + >> +Example: >> + >> +-> { "execute": "query-migrate-parameters" } >> +<- { "return": { >> + "capabilities" : [ { "capability" : "xbzrle", "state" : false } ] >> + } >> + } >> +EQMP >> + >> + { >> + .name = "query-migrate-parameters", >> + .args_type = "", >> + .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters, >> + }, >> + >> +SQMP >> query-balloon >> ------------- >> >
On Tue, 31 Jul 2012 10:46:02 +0300 Orit Wasserman <owasserm@redhat.com> wrote: > On 07/30/2012 08:41 PM, Luiz Capitulino wrote: > > On Sun, 29 Jul 2012 12:42:54 +0300 > > Orit Wasserman <owasserm@redhat.com> wrote: > > > >> The management can enable/disable a capability for the next migration by using > >> migrate_set_parameter command. > >> The management can query the current migration capabilities using > >> query-migrate-parameters > > > > In general looks good to me, I have a question and a few nitpick comments > > below. > > > >> > >> Signed-off-by: Orit Wasserman <owasserm@redhat.com> > >> Signed-off-by: Juan Quintela <quintela@redhat.com> > >> --- > >> hmp-commands.hx | 16 ++++++++++++ > >> hmp.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > >> hmp.h | 2 + > >> migration.c | 42 +++++++++++++++++++++++++++++++- > >> migration.h | 2 + > >> monitor.c | 7 +++++ > >> qapi-schema.json | 32 ++++++++++++++++++++++++ > >> qmp-commands.hx | 60 ++++++++++++++++++++++++++++++++++++++++++++- > >> 8 files changed, 229 insertions(+), 3 deletions(-) > >> > >> diff --git a/hmp-commands.hx b/hmp-commands.hx > >> index 8786148..3e15338 100644 > >> --- a/hmp-commands.hx > >> +++ 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 = "capability state", > >> + .help = "Enable/Disable the usage of a capability for migration", > >> + .mhandler.cmd = hmp_migrate_set_parameter, > >> + }, > >> + > >> +STEXI > >> +@item migrate_set_parameter @var{capability} @var{state} > >> +@findex migrate_set_parameter > >> +Enable/Disable the usage of a capability @var{capability} 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", > >> @@ -1419,6 +1433,8 @@ show user network stack connection states > >> show migration status > >> @item info migration_capabilities > >> show migration capabilities > >> +@item info migrate_parameters > >> +show current migration parameters > >> @item info balloon > >> show balloon information > >> @item info qtree > >> diff --git a/hmp.c b/hmp.c > >> index 5c7d0be..f2f63fd 100644 > >> --- a/hmp.c > >> +++ b/hmp.c > >> @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) > >> void hmp_info_migrate(Monitor *mon) > >> { > >> MigrationInfo *info; > >> + MigrationCapabilityStatusList *cap; > >> + MigrationParameters *params; > >> > >> info = qmp_query_migrate(NULL); > >> + params = qmp_query_migrate_parameters(NULL); > >> + > >> + /* do not display parameters during setup */ > >> + if (info->has_status && params->capabilities) { > >> + monitor_printf(mon, "capabilities: "); > >> + for (cap = params->capabilities; cap; cap = cap->next) { > >> + monitor_printf(mon, "%s: %s ", > >> + MigrationCapability_lookup[cap->value->capability], > >> + cap->value->state ? "on" : "off"); > >> + } > >> + monitor_printf(mon, "\n"); > >> + } > >> > >> if (info->has_status) { > >> monitor_printf(mon, "Migration status: %s\n", info->status); > >> @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) > >> } > >> > >> qapi_free_MigrationInfo(info); > >> + qapi_free_MigrationParameters(params); > >> } > >> > >> void hmp_info_migration_capabilities(Monitor *mon) > >> @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) > >> qapi_free_MigrationCapabilityStatusList(caps_list); > >> } > >> > >> +void hmp_info_migrate_parameters(Monitor *mon) > >> +{ > >> + > >> + MigrationCapabilityStatusList *cap; > >> + MigrationParameters *params; > >> + > >> + params = qmp_query_migrate_parameters(NULL); > >> + > >> + if (params->capabilities) { > >> + monitor_printf(mon, "capabilities: "); > >> + for (cap = params->capabilities; cap; cap = cap->next) { > >> + monitor_printf(mon, "%s: %s ", > >> + MigrationCapability_lookup[cap->value->capability], > >> + cap->value->state ? "on" : "off"); > >> + } > >> + monitor_printf(mon, "\n"); > >> + } > >> + > >> + qapi_free_MigrationParameters(params); > >> +} > >> + > >> void hmp_info_cpus(Monitor *mon) > >> { > >> CpuInfoList *cpu_list, *cpu; > >> @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) > >> qmp_migrate_set_speed(value, NULL); > >> } > >> > >> +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) > >> +{ > >> + const char *cap = qdict_get_str(qdict, "capability"); > >> + bool state = qdict_get_bool(qdict, "state"); > >> + Error *err = NULL; > >> + MigrationCapabilityStatusList *params = NULL; > >> + int i; > >> + > >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > >> + if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { > >> + if (!params) { > >> + params = g_malloc0(sizeof(*params)); > >> + } > > > > This if shouldn't be necessary, right? > You are right I can always allocate it. > > > >> + params->value = g_malloc0(sizeof(*params->value)); > >> + params->value->capability = i; > >> + params->value->state = state; > >> + params->next = NULL; > > > > nitpick: I'd make the code ready for future capabilities. > Could you give me a bit more details ? This is not a proper list, it will break when we add a new capability. But that comment doesn't apply here, I meant to add that command to qmp_query_migrate_parameters()'s hunk. > > Orit > > > >> + qmp_migrate_set_parameters(params, &err); > >> + break; > >> + } > >> + } > >> + > >> + if (i == MIGRATION_CAPABILITY_MAX) { > >> + error_set(&err, QERR_INVALID_PARAMETER, cap); > >> + } > >> + > >> + qapi_free_MigrationCapabilityStatusList(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 2fb44ca..ceb2f06 100644 > >> --- a/hmp.h > >> +++ b/hmp.h > >> @@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon); > >> void hmp_info_mice(Monitor *mon); > >> void hmp_info_migrate(Monitor *mon); > >> void hmp_info_migration_capabilities(Monitor *mon); > >> +void hmp_info_migrate_parameters(Monitor *mon); > >> void hmp_info_cpus(Monitor *mon); > >> void hmp_info_block(Monitor *mon); > >> void hmp_info_blockstats(Monitor *mon); > >> @@ -52,6 +53,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); > >> 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_parameter(Monitor *mon, const QDict *qdict); > >> void hmp_set_password(Monitor *mon, const QDict *qdict); > >> void hmp_expire_password(Monitor *mon, const QDict *qdict); > >> void hmp_eject(Monitor *mon, const QDict *qdict); > >> diff --git a/migration.c b/migration.c > >> index 8c27347..e844290 100644 > >> --- a/migration.c > >> +++ b/migration.c > >> @@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void) > >> return max_downtime; > >> } > >> > >> +MigrationParameters *qmp_query_migrate_parameters(Error **errp) > >> +{ > >> + MigrationParameters *params = g_malloc0(sizeof(*params)); > >> + MigrationState *s = migrate_get_current(); > >> + int i; > >> + > >> + params->capabilities = g_malloc0(sizeof(*params->capabilities)); > >> + > >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { > >> + params->capabilities->value = > >> + g_malloc(sizeof(*params->capabilities->value)); > >> + params->capabilities->value->capability = i; > >> + params->capabilities->value->state = s->enabled_capabilities[i]; > >> + params->capabilities->next = NULL; > >> + } > >> + > >> + return params; > >> +} > >> + > >> MigrationInfo *qmp_query_migrate(Error **errp) > >> { > >> MigrationInfo *info = g_malloc0(sizeof(*info)); > >> @@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp) > >> return caps_list; > >> } > >> > >> +void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params, > >> + Error **errp) > >> +{ > >> + MigrationState *s = migrate_get_current(); > >> + MigrationCapabilityStatusList *cap; > >> + > >> + if (s->state == MIG_STATE_ACTIVE) { > >> + error_set(errp, QERR_MIGRATION_ACTIVE); > >> + return; > >> + } > >> + > >> + for (cap = params; cap; cap = cap->next) { > >> + s->enabled_capabilities[cap->value->capability] = cap->value->state; > >> + } > >> +} > >> + > >> /* shared migration helpers */ > >> > >> static int migrate_fd_cleanup(MigrationState *s) > >> @@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params) > >> { > >> MigrationState *s = migrate_get_current(); > >> int64_t bandwidth_limit = s->bandwidth_limit; > >> + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > >> + > >> + memcpy(enabled_capabilities, s->enabled_capabilities, > >> + sizeof(enabled_capabilities)); > > > > Is s->enabled_capabilities initialized somewhere or are we making xbzrle > > disabled by default? > > > >> > >> memset(s, 0, sizeof(*s)); > >> s->bandwidth_limit = bandwidth_limit; > >> s->params = *params; > >> + memcpy(s->enabled_capabilities, enabled_capabilities, > >> + sizeof(enabled_capabilities)); > >> > >> - s->bandwidth_limit = bandwidth_limit; > > > > Unrelated change. > > > >> s->state = MIG_STATE_SETUP; > >> s->total_time = qemu_get_clock_ms(rt_clock); > >> > >> diff --git a/migration.h b/migration.h > >> index 57572a6..713aae0 100644 > >> --- a/migration.h > >> +++ b/migration.h > >> @@ -19,6 +19,7 @@ > >> #include "notify.h" > >> #include "error.h" > >> #include "vmstate.h" > >> +#include "qapi-types.h" > >> > >> struct MigrationParams { > >> bool blk; > >> @@ -39,6 +40,7 @@ struct MigrationState > >> void *opaque; > >> MigrationParams params; > >> int64_t total_time; > >> + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > >> }; > >> > >> void process_incoming_migration(QEMUFile *f); > >> diff --git a/monitor.c b/monitor.c > >> index fd57c5e..e07af97 100644 > >> --- a/monitor.c > >> +++ b/monitor.c > >> @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { > >> .mhandler.info = hmp_info_migration_capabilities, > >> }, > >> { > >> + .name = "migrate-parameters", > >> + .args_type = "", > >> + .params = "", > >> + .help = "show current migration parameters", > >> + .mhandler.info = hmp_info_migrate_parameters, > >> + }, > >> + { > >> .name = "balloon", > >> .args_type = "", > >> .params = "", > >> diff --git a/qapi-schema.json b/qapi-schema.json > >> index b4d4dd6..9d6759d 100644 > >> --- a/qapi-schema.json > >> +++ b/qapi-schema.json > >> @@ -345,6 +345,38 @@ > >> { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } > >> > >> ## > >> +# @migrate-set-parameters > >> +# > >> +# Enable/Disable the following migration capabilities (like xbzrle) > >> +# > >> +# Since: 1.2 > >> +## > >> +{ 'command': 'migrate-set-parameters', > >> + 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } > >> + > >> +## > >> +# @MigrationParameters > >> +# > >> +# @capabilities: @MigrationCapabilityStatus list contain current migration > >> +# capabilities status > >> +# Since: 1.2 > >> +## > >> +{ 'type': 'MigrationParameters', > >> + 'data': {'capabilities': ['MigrationCapabilityStatus']} } > >> + > >> + > >> +## > >> +# @query-migrate-parameters > >> +# > >> +# Returns information about the current migration capabilities status > >> +# > >> +# Returns: @MigrationParameters > >> +# > >> +# Since: 1.2 > >> +## > >> +{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' } > >> + > >> +## > >> # @MouseInfo: > >> # > >> # Information about a mouse device. > >> diff --git a/qmp-commands.hx b/qmp-commands.hx > >> index c0ed14c..7f40e2a 100644 > >> --- a/qmp-commands.hx > >> +++ b/qmp-commands.hx > >> @@ -2081,7 +2081,6 @@ The main json-object contains the following: > >> - "transferred": amount transferred (json-int) > >> - "remaining": amount remaining (json-int) > >> - "total": total (json-int) > >> - > >> Examples: > >> > >> 1. Before the first migration > >> @@ -2092,7 +2091,15 @@ Examples: > >> 2. Migration is done and has succeeded > >> > >> -> { "execute": "query-migrate" } > >> -<- { "return": { "status": "completed" } } > >> +<- { "return": { > >> + "status": "completed", > >> + "ram":{ > >> + "transferred":123, > >> + "remaining":123, > >> + "total":246 > >> + } > >> + } > >> + } > >> > >> 3. Migration is done and has failed > >> > >> @@ -2165,6 +2172,55 @@ EQMP > >> }, > >> > >> SQMP > >> +migrate-set-parameters > >> +------- > >> + > >> +Enable/Disable migration capabilities > >> + > >> +- "xbzrle": xbzrle support > >> + > >> +Arguments: > >> + > >> +Example: > >> + > >> +-> { "execute": "migrate-set-parameters" , "arguments": > >> + { "parameters": [ { "capability": "xbzrle", "state": true } ] } } > >> + > >> +EQMP > >> + > >> + { > >> + .name = "migrate_set_parameters", > >> + .args_type = "parameters:O", > >> + .params = "capability:s,state:b", > >> + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, > >> + }, > >> +SQMP > >> +query-migrate-parameters > >> +------- > >> + > >> +Query current migration parameters > >> + > >> +- "capabilities": migration capabilities state > >> + - "xbzrle" : XBZRLE state (json-bool) > >> + > >> +Arguments: > >> + > >> +Example: > >> + > >> +-> { "execute": "query-migrate-parameters" } > >> +<- { "return": { > >> + "capabilities" : [ { "capability" : "xbzrle", "state" : false } ] > >> + } > >> + } > >> +EQMP > >> + > >> + { > >> + .name = "query-migrate-parameters", > >> + .args_type = "", > >> + .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters, > >> + }, > >> + > >> +SQMP > >> query-balloon > >> ------------- > >> > > >
diff --git a/hmp-commands.hx b/hmp-commands.hx index 8786148..3e15338 100644 --- a/hmp-commands.hx +++ 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 = "capability state", + .help = "Enable/Disable the usage of a capability for migration", + .mhandler.cmd = hmp_migrate_set_parameter, + }, + +STEXI +@item migrate_set_parameter @var{capability} @var{state} +@findex migrate_set_parameter +Enable/Disable the usage of a capability @var{capability} 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", @@ -1419,6 +1433,8 @@ show user network stack connection states show migration status @item info migration_capabilities show migration capabilities +@item info migrate_parameters +show current migration parameters @item info balloon show balloon information @item info qtree diff --git a/hmp.c b/hmp.c index 5c7d0be..f2f63fd 100644 --- a/hmp.c +++ b/hmp.c @@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon) void hmp_info_migrate(Monitor *mon) { MigrationInfo *info; + MigrationCapabilityStatusList *cap; + MigrationParameters *params; info = qmp_query_migrate(NULL); + params = qmp_query_migrate_parameters(NULL); + + /* do not display parameters during setup */ + if (info->has_status && params->capabilities) { + monitor_printf(mon, "capabilities: "); + for (cap = params->capabilities; cap; cap = cap->next) { + monitor_printf(mon, "%s: %s ", + MigrationCapability_lookup[cap->value->capability], + cap->value->state ? "on" : "off"); + } + monitor_printf(mon, "\n"); + } if (info->has_status) { monitor_printf(mon, "Migration status: %s\n", info->status); @@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon) } qapi_free_MigrationInfo(info); + qapi_free_MigrationParameters(params); } void hmp_info_migration_capabilities(Monitor *mon) @@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon) qapi_free_MigrationCapabilityStatusList(caps_list); } +void hmp_info_migrate_parameters(Monitor *mon) +{ + + MigrationCapabilityStatusList *cap; + MigrationParameters *params; + + params = qmp_query_migrate_parameters(NULL); + + if (params->capabilities) { + monitor_printf(mon, "capabilities: "); + for (cap = params->capabilities; cap; cap = cap->next) { + monitor_printf(mon, "%s: %s ", + MigrationCapability_lookup[cap->value->capability], + cap->value->state ? "on" : "off"); + } + monitor_printf(mon, "\n"); + } + + qapi_free_MigrationParameters(params); +} + void hmp_info_cpus(Monitor *mon) { CpuInfoList *cpu_list, *cpu; @@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict) qmp_migrate_set_speed(value, NULL); } +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) +{ + const char *cap = qdict_get_str(qdict, "capability"); + bool state = qdict_get_bool(qdict, "state"); + Error *err = NULL; + MigrationCapabilityStatusList *params = NULL; + int i; + + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { + if (strcmp(cap, MigrationCapability_lookup[i]) == 0) { + if (!params) { + params = g_malloc0(sizeof(*params)); + } + params->value = g_malloc0(sizeof(*params->value)); + params->value->capability = i; + params->value->state = state; + params->next = NULL; + qmp_migrate_set_parameters(params, &err); + break; + } + } + + if (i == MIGRATION_CAPABILITY_MAX) { + error_set(&err, QERR_INVALID_PARAMETER, cap); + } + + qapi_free_MigrationCapabilityStatusList(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 2fb44ca..ceb2f06 100644 --- a/hmp.h +++ b/hmp.h @@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon); void hmp_info_mice(Monitor *mon); void hmp_info_migrate(Monitor *mon); void hmp_info_migration_capabilities(Monitor *mon); +void hmp_info_migrate_parameters(Monitor *mon); void hmp_info_cpus(Monitor *mon); void hmp_info_block(Monitor *mon); void hmp_info_blockstats(Monitor *mon); @@ -52,6 +53,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); 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_parameter(Monitor *mon, const QDict *qdict); void hmp_set_password(Monitor *mon, const QDict *qdict); void hmp_expire_password(Monitor *mon, const QDict *qdict); void hmp_eject(Monitor *mon, const QDict *qdict); diff --git a/migration.c b/migration.c index 8c27347..e844290 100644 --- a/migration.c +++ b/migration.c @@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void) return max_downtime; } +MigrationParameters *qmp_query_migrate_parameters(Error **errp) +{ + MigrationParameters *params = g_malloc0(sizeof(*params)); + MigrationState *s = migrate_get_current(); + int i; + + params->capabilities = g_malloc0(sizeof(*params->capabilities)); + + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { + params->capabilities->value = + g_malloc(sizeof(*params->capabilities->value)); + params->capabilities->value->capability = i; + params->capabilities->value->state = s->enabled_capabilities[i]; + params->capabilities->next = NULL; + } + + return params; +} + MigrationInfo *qmp_query_migrate(Error **errp) { MigrationInfo *info = g_malloc0(sizeof(*info)); @@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp) return caps_list; } +void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params, + Error **errp) +{ + MigrationState *s = migrate_get_current(); + MigrationCapabilityStatusList *cap; + + if (s->state == MIG_STATE_ACTIVE) { + error_set(errp, QERR_MIGRATION_ACTIVE); + return; + } + + for (cap = params; cap; cap = cap->next) { + s->enabled_capabilities[cap->value->capability] = cap->value->state; + } +} + /* shared migration helpers */ static int migrate_fd_cleanup(MigrationState *s) @@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params) { MigrationState *s = migrate_get_current(); int64_t bandwidth_limit = s->bandwidth_limit; + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; + + memcpy(enabled_capabilities, s->enabled_capabilities, + sizeof(enabled_capabilities)); memset(s, 0, sizeof(*s)); s->bandwidth_limit = bandwidth_limit; s->params = *params; + memcpy(s->enabled_capabilities, enabled_capabilities, + sizeof(enabled_capabilities)); - s->bandwidth_limit = bandwidth_limit; s->state = MIG_STATE_SETUP; s->total_time = qemu_get_clock_ms(rt_clock); diff --git a/migration.h b/migration.h index 57572a6..713aae0 100644 --- a/migration.h +++ b/migration.h @@ -19,6 +19,7 @@ #include "notify.h" #include "error.h" #include "vmstate.h" +#include "qapi-types.h" struct MigrationParams { bool blk; @@ -39,6 +40,7 @@ struct MigrationState void *opaque; MigrationParams params; int64_t total_time; + bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; }; void process_incoming_migration(QEMUFile *f); diff --git a/monitor.c b/monitor.c index fd57c5e..e07af97 100644 --- a/monitor.c +++ b/monitor.c @@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = { .mhandler.info = hmp_info_migration_capabilities, }, { + .name = "migrate-parameters", + .args_type = "", + .params = "", + .help = "show current migration parameters", + .mhandler.info = hmp_info_migrate_parameters, + }, + { .name = "balloon", .args_type = "", .params = "", diff --git a/qapi-schema.json b/qapi-schema.json index b4d4dd6..9d6759d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -345,6 +345,38 @@ { 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] } ## +# @migrate-set-parameters +# +# Enable/Disable the following migration capabilities (like xbzrle) +# +# Since: 1.2 +## +{ 'command': 'migrate-set-parameters', + 'data': { 'capabilities': ['MigrationCapabilityStatus'] } } + +## +# @MigrationParameters +# +# @capabilities: @MigrationCapabilityStatus list contain current migration +# capabilities status +# Since: 1.2 +## +{ 'type': 'MigrationParameters', + 'data': {'capabilities': ['MigrationCapabilityStatus']} } + + +## +# @query-migrate-parameters +# +# Returns information about the current migration capabilities status +# +# Returns: @MigrationParameters +# +# Since: 1.2 +## +{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' } + +## # @MouseInfo: # # Information about a mouse device. diff --git a/qmp-commands.hx b/qmp-commands.hx index c0ed14c..7f40e2a 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2081,7 +2081,6 @@ The main json-object contains the following: - "transferred": amount transferred (json-int) - "remaining": amount remaining (json-int) - "total": total (json-int) - Examples: 1. Before the first migration @@ -2092,7 +2091,15 @@ Examples: 2. Migration is done and has succeeded -> { "execute": "query-migrate" } -<- { "return": { "status": "completed" } } +<- { "return": { + "status": "completed", + "ram":{ + "transferred":123, + "remaining":123, + "total":246 + } + } + } 3. Migration is done and has failed @@ -2165,6 +2172,55 @@ EQMP }, SQMP +migrate-set-parameters +------- + +Enable/Disable migration capabilities + +- "xbzrle": xbzrle support + +Arguments: + +Example: + +-> { "execute": "migrate-set-parameters" , "arguments": + { "parameters": [ { "capability": "xbzrle", "state": true } ] } } + +EQMP + + { + .name = "migrate_set_parameters", + .args_type = "parameters:O", + .params = "capability:s,state:b", + .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters, + }, +SQMP +query-migrate-parameters +------- + +Query current migration parameters + +- "capabilities": migration capabilities state + - "xbzrle" : XBZRLE state (json-bool) + +Arguments: + +Example: + +-> { "execute": "query-migrate-parameters" } +<- { "return": { + "capabilities" : [ { "capability" : "xbzrle", "state" : false } ] + } + } +EQMP + + { + .name = "query-migrate-parameters", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters, + }, + +SQMP query-balloon -------------