diff mbox

[02/11] Add migrate_set_parameter and query-migrate-parameters

Message ID 1343554983-4195-3-git-send-email-owasserm@redhat.com
State New
Headers show

Commit Message

Orit Wasserman July 29, 2012, 9:42 a.m. UTC 
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>
---
 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(-)

Comments

Luiz Capitulino July 30, 2012, 5:41 p.m. UTC | #1 
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
>  -------------
>
Eric Blake July 30, 2012, 6:11 p.m. UTC | #2 
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.
Luiz Capitulino July 30, 2012, 6:15 p.m. UTC | #3 
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.
>
Juan Quintela July 30, 2012, 7:12 p.m. UTC | #4 
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.
Eric Blake July 30, 2012, 7:24 p.m. UTC | #5 
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.
Anthony Liguori July 30, 2012, 7:37 p.m. UTC | #6 
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.
Anthony Liguori July 30, 2012, 7:45 p.m. UTC | #7 
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
Luiz Capitulino July 30, 2012, 7:58 p.m. UTC | #8 
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.
Anthony Liguori July 30, 2012, 8:04 p.m. UTC | #9 
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
Luiz Capitulino July 30, 2012, 8:20 p.m. UTC | #10 
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 July 30, 2012, 8:21 p.m. UTC | #11 
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.
Orit Wasserman July 31, 2012, 7:46 a.m. UTC | #12 
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
>>  -------------
>>  
>
Orit Wasserman July 31, 2012, 8:03 a.m. UTC | #13 
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
>>  -------------
>>  
>
Luiz Capitulino July 31, 2012, 1:09 p.m. UTC | #14 
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 mbox

Patch

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