Re: [Qemu-devel] [PATCH 13/22] Add migration capabilities

[Luiz Capitulino]

On Wed, 25 Jul 2012 16:05:10 +0300
Orit Wasserman <address@hidden> wrote:

> On 07/24/2012 09:17 PM, Luiz Capitulino wrote:
> > On Tue, 24 Jul 2012 20:06:06 +0300
> > Orit Wasserman <address@hidden> wrote:
> > 
> >> On 07/24/2012 03:50 PM, Luiz Capitulino wrote:
> >>> On Tue, 24 Jul 2012 09:25:11 +0300
> >>> Orit Wasserman <address@hidden> wrote:
> >>>
> >>>> On 07/23/2012 09:23 PM, Luiz Capitulino wrote:
> >>>>> On Fri, 13 Jul 2012 09:23:35 +0200
> >>>>> Juan Quintela <address@hidden> wrote:
> >>>>>
> >>>>>> From: Orit Wasserman <address@hidden>
> >>>>>>
> >>>>>> Add migration capabilities that can be queried by the management.
> >>>>>> The management can query the source QEMU and the destination QEMU in 
> >>>>>> order to
> >>>>>> verify both support some migration capability (currently only XBZRLE).
> >>>>>> The management can enable a capability for the next migration by using
> >>>>>> migrate_set_parameter command.
> >>>>>
> >>>>> Please, split this into one command per-patch. Otherwise it's difficult 
> >>>>> to
> >>>>> review.
> >>>>>
> >>>> Sure.
> >>>>> Have libvirt folks acked this approach btw? It looks fine to me, but we 
> >>>>> need
> >>>>> their ack too.
> >>>>>
> >>>>> More comments below.
> >>>>>
> >>>>>>
> >>>>>> Signed-off-by: Orit Wasserman <address@hidden>
> >>>>>> Signed-off-by: Juan Quintela <address@hidden>
> >>>>>> ---
> >>>>>>  hmp-commands.hx  |   16 ++++++++++++
> >>>>>>  hmp.c            |   64 
> >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>>  hmp.h            |    2 ++
> >>>>>>  migration.c      |   72 
> >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >>>>>>  migration.h      |    2 ++
> >>>>>>  monitor.c        |    7 ++++++
> >>>>>>  qapi-schema.json |   53 ++++++++++++++++++++++++++++++++++++++--
> >>>>>>  qmp-commands.hx  |   71 
> >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++---
> >>>>>>  8 files changed, 280 insertions(+), 7 deletions(-)
> >>>>>>
> >>>>>> diff --git a/hmp-commands.hx b/hmp-commands.hx
> >>>>>> index f5d9d91..9245bef 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     = "",
> >>>>>
> >>>>> Please, fill in params.
> >>>> ok
> >>>>>
> >>>>>> +        .help       = "Enable/Disable the usage of a capability for 
> >>>>>> migration",
> >>>>>> +        .mhandler.cmd = hmp_migrate_set_parameter,
> >>>>>> +    },
> >>>>>> +
> >>>>>> +STEXI
> >>>>>> address@hidden migrate_set_parameter @var{capability} @var{state}
> >>>>>> address@hidden 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 CPU statistics
> >>>>>>  show user network stack connection states
> >>>>>>  @item info migrate
> >>>>>>  show migration status
> >>>>>> address@hidden info migration_capabilities
> >>>>>> +show migration capabilities
> >>>>>>  @item info balloon
> >>>>>>  show balloon information
> >>>>>>  @item info qtree
> >>>>>> diff --git a/hmp.c b/hmp.c
> >>>>>> index 4c6d4ae..b0440e6 100644
> >>>>>> --- a/hmp.c
> >>>>>> +++ b/hmp.c
> >>>>>> @@ -131,9 +131,19 @@ void hmp_info_mice(Monitor *mon)
> >>>>>>  void hmp_info_migrate(Monitor *mon)
> >>>>>>  {
> >>>>>>      MigrationInfo *info;
> >>>>>> +    MigrationCapabilityInfoList *cap;
> >>>>>>
> >>>>>>      info = qmp_query_migrate(NULL);
> >>>>>>
> >>>>>> +    if (info->has_capabilities && info->capabilities) {
> >>>>>> +        monitor_printf(mon, "capabilities: ");
> >>>>>> +        for (cap = info->capabilities; cap; cap = cap->next) {
> >>>>>> +            monitor_printf(mon, "%s: %s ",
> >>>>>> +                           
> >>>>>> MigrationCapability_lookup[cap->value->capability],
> >>>>>> +                           cap->value->state ? "on" : "off");
> >>>>>> +        }
> >>>>>> +        monitor_printf(mon, "\n");
> >>>>>
> >>>>> Why is this is needed? Isn't info migration-capabilities good enough?
> >>>>> Besides, info migrate should only contain information about current 
> >>>>> migration
> >>>>> process...
> >>>> The reason we introduced capabilities is that xbzrle needs for both 
> >>>> source and destination QEMU
> >>>> to be able to handle it. Even if both side support xbzrle the user may 
> >>>> decide not to use it.
> >>>> User that wants to use xbzrle needs to check that both sides have 
> >>>> support for it (using info capabilities) , than 
> >>>> enable it in both sides (using migrate-set-parameter/s commands). This 
> >>>> is a parameter for the current migration.
> >>>> So the user needs to know if xbzrle was enabled or disabled for the 
> >>>> current migration, this code displays it.
> >>>
> >>> But this is being returned even when there is no migration taking place. 
> >>> I'm
> >>> answering this here in the 'info migrate' hunk, but my main concern is
> >>> query-migrate.
> >> I think migration configuration is part of migration information and 
> >> should be available to the user in 'info migration'. Especially before he 
> >> starts running it to see if he need to change the configuration. Also it 
> >> is very helpful for the user during the migration and also after it's 
> >> completes.
> > 
> > I don't mind having this in 'info migrate' because we can change it later 
> > case
> > we regret, so it's fine with me having that info there.
> > 
> > But query-migrate is different, once we add anything there it's there 
> > forever,
> > so we have to be careful.
> ok I will remove them from query-migrate.

Note that I'm asking to remove it from MIG_SETUP, the others states are fine.

> > 
> >> There is no way today to query  the downtime and migration speed at all . 
> >> you can always set them again
> >> but there is no way to know how it is configured.
> > 
> > Good point, but I think it's better to concentrate on the needs of xbzrle
> > for now.
> > 
> >> I was actually thinking to add them to info migration (also for SETUP).
> > 
> > But we're in SETUP state only once, what happens if the settings are changed
> > after a migration completed? How is mngt going to be able to query it?
> After migration completes you can't change those setting , you should get an 
> error.

Really, why? I thought it would be fine to change them at any time
(except during migration, of course).

> 
> > 
> > My advice to not delay the inclusion of this series is to drop the
> > information from SETUP state and rely on query-migration-capabilities in
> > 'info migrate'. We can extend later if needed (by means of new commands or
> > adding info to query-migrate).
> It needs to be another command, I can add query-migration-parameters.
> You can look at this as 64bit support - you have 64bit support in the 
> processor (this is what query-capabilities) but the OS can decide not to work 
> in 64bit mode (do nothing) or using 64 bit (migrate-set-parameter).
> 
> Orit
> > 
> >>>
> >>> query-migrate shouldn't return this info in MIG_STATE_SETUP, and for
> >>> MIG_STATE_COMPLETED it should only be returned if there was a migration.
> >> I think is case of MIG_SETUP we should return the configuration and params 
> >> for the next migration. 
> >> MIG_STATE_COMPLETED happens only if there was a successful migration other 
> >> wise there will be another state.
> >>>
> >>>> some day when there will be better migration protocol, feature 
> >>>> negotiations can be part of it ...
> >>>>>
> >>>>>> +    }
> >>>>>>      if (info->has_status) {
> >>>>>>          monitor_printf(mon, "Migration status: %s\n", info->status);
> >>>>>>      }
> >>>>>> @@ -161,6 +171,25 @@ void hmp_info_migrate(Monitor *mon)
> >>>>>>      qapi_free_MigrationInfo(info);
> >>>>>>  }
> >>>>>>
> >>>>>> +void hmp_info_migration_capabilities(Monitor *mon)
> >>>>>> +{
> >>>>>> +    MigrationCapabilityInfoList *caps_list, *cap;
> >>>>>> +
> >>>>>> +    caps_list = qmp_query_migration_capabilities(NULL);
> >>>>>> +    if (!caps_list) {
> >>>>>> +        monitor_printf(mon, "No migration capabilities found\n");
> >>>>>> +        return;
> >>>>>> +    }
> >>>>>> +
> >>>>>> +    for (cap = caps_list; cap; cap = cap->next) {
> >>>>>> +        monitor_printf(mon, "%s: %s ",
> >>>>>> +                       
> >>>>>> MigrationCapability_lookup[cap->value->capability],
> >>>>>> +                       cap->value->state ? "on" : "off");
> >>>>>> +    }
> >>>>>> +
> >>>>>> +    qapi_free_MigrationCapabilityInfoList(caps_list);
> >>>>>> +}
> >>>>>> +
> >>>>>>  void hmp_info_cpus(Monitor *mon)
> >>>>>>  {
> >>>>>>      CpuInfoList *cpu_list, *cpu;
> >>>>>> @@ -735,6 +764,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;
> >>>>>> +    MigrationCapabilityInfoList *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_MigrationCapabilityInfoList(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 79d138d..09ba198 100644
> >>>>>> --- a/hmp.h
> >>>>>> +++ b/hmp.h
> >>>>>> @@ -25,6 +25,7 @@ void hmp_info_uuid(Monitor *mon);
> >>>>>>  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_cpus(Monitor *mon);
> >>>>>>  void hmp_info_block(Monitor *mon);
> >>>>>>  void hmp_info_blockstats(Monitor *mon);
> >>>>>> @@ -51,6 +52,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 8db1b43..fd004d7 100644
> >>>>>> --- a/migration.c
> >>>>>> +++ b/migration.c
> >>>>>> @@ -117,12 +117,36 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>>>>>  {
> >>>>>>      MigrationInfo *info = g_malloc0(sizeof(*info));
> >>>>>>      MigrationState *s = migrate_get_current();
> >>>>>> +    int i;
> >>>>>>
> >>>>>>      switch (s->state) {
> >>>>>>      case MIG_STATE_SETUP:
> >>>>>> -        /* no migration has happened ever */
> >>>>>> +        /* no migration has ever happened; show enabled capabilities 
> >>>>>> */
> >>>>>> +        for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> >>>>>> +            if (!info->has_capabilities) {
> >>>>>> +                info->capabilities = 
> >>>>>> g_malloc0(sizeof(*info->capabilities));
> >>>>>> +                info->has_capabilities = true;
> >>>>>> +            }
> >>>>>> +            info->capabilities->value =
> >>>>>> +                g_malloc(sizeof(*info->capabilities->value));
> >>>>>> +            info->capabilities->value->capability = i;
> >>>>>> +            info->capabilities->value->state = 
> >>>>>> s->enabled_capabilities[i];
> >>>>>> +            info->capabilities->next = NULL;
> >>>>>> +        }
> >>>>>>          break;
> >>>>>>      case MIG_STATE_ACTIVE:
> >>>>>> +        for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> >>>>>> +            if (!info->has_capabilities) {
> >>>>>> +                info->capabilities = 
> >>>>>> g_malloc0(sizeof(*info->capabilities));
> >>>>>> +                info->has_capabilities = true;
> >>>>>> +            }
> >>>>>> +            info->capabilities->value =
> >>>>>> +                g_malloc(sizeof(*info->capabilities->value));
> >>>>>> +            info->capabilities->value->capability = i;
> >>>>>> +            info->capabilities->value->state = 
> >>>>>> s->enabled_capabilities[i];
> >>>>>> +            info->capabilities->next = NULL;
> >>>>>> +        }
> >>>>>> +
> >>>>>>          info->has_status = true;
> >>>>>>          info->status = g_strdup("active");
> >>>>>>
> >>>>>> @@ -143,6 +167,18 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>>>>>          }
> >>>>>>          break;
> >>>>>>      case MIG_STATE_COMPLETED:
> >>>>>> +        for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> >>>>>> +            if (!info->has_capabilities) {
> >>>>>> +                info->capabilities = 
> >>>>>> g_malloc0(sizeof(*info->capabilities));
> >>>>>> +                info->has_capabilities = true;
> >>>>>> +            }
> >>>>>> +            info->capabilities->value =
> >>>>>> +                g_malloc(sizeof(*info->capabilities->value));
> >>>>>> +            info->capabilities->value->capability = i;
> >>>>>> +            info->capabilities->value->state = 
> >>>>>> s->enabled_capabilities[i];
> >>>>>> +            info->capabilities->next = NULL;
> >>>>>> +        }
> >>>>>
> >>>>> Code triplication :)
> >>>>>
> >>>>> Why is this is needed? Isn't query-migration-capabilities good enough?
> >>>>> Besides, query-migrate should only contain information about current 
> >>>>> migration
> >>>>> process...
> >>>>>
> >>>> see above
> >>>>>> +
> >>>>>>          info->has_status = true;
> >>>>>>          info->status = g_strdup("completed");
> >>>>>>
> >>>>>> @@ -166,6 +202,33 @@ MigrationInfo *qmp_query_migrate(Error **errp)
> >>>>>>      return info;
> >>>>>>  }
> >>>>>>
> >>>>>> +MigrationCapabilityInfoList *qmp_query_migration_capabilities(Error 
> >>>>>> **errp)
> >>>>>> +{
> >>>>>> +    MigrationCapabilityInfoList *caps_list = 
> >>>>>> g_malloc0(sizeof(*caps_list));
> >>>>>> +
> >>>>>> +    caps_list->value = g_malloc(sizeof(*caps_list->value));
> >>>>>> +    caps_list->value->capability = MIGRATION_CAPABILITY_XBZRLE;
> >>>>>> +    caps_list->next = NULL;
> >>>>>
> >>>>> Shouldn't this get the capabilities array from migrate_get_current()?
> >>>>>
> >>>>> I mean, this makes query-migration-capabilities always return true for
> >>>>> xbzrle, even if we set it to off.
> >>>> Those are the general capabilities , if I want to use xbzrle ,does this 
> >>>> QEMU support it ? 
> >>>> this is required because we need both the destination and the source to 
> >>>> be able to handle it.
> >>>> If one QEMU doesn't (older version of qemu, maybe it will be disbaled 
> >>>> for certain architectures) we can't use the feature.
> >>>> This is for management and the user to check what migration capabilities 
> >>>> this QEMU supports.
> >>>
> >>> My point was that query-migration-capabilities will always return 
> >>> state=true
> >>> for all existing capabilities. Don't we want users/mngt app to know which
> >>> states have been turned off?
> >> You need to separate the ability to use a capability and actually using it 
> >> for migration. Also I don't want the user to be able to set some 
> >> capability on, it depends on the code not user configuration.
> > 
> > Not sure I can follow, a user can set xbzrle capability to off, no?
> > 
> >> And for xbzrle we always return true for some version on QEMU , let say we 
> >> decide we want to remove the code that implement it for some reason in 
> >> some future version than it will return false.
> > 
> > That's fine (I mean, I'm not sure we will ever do it, but shouldn't be a 
> > problem
> > if we do).
> > 
> >> There is a reason the command that enables a capability for a migration 
> >> was called 'migrate-set-parameter' and not set-capability to distinguish 
> >> between them one is per QEMU code base and one is per migration.
> >>
> >> Regards,
> >> Orit
> >>>
> >>>>
> >>>>>
> >>>>>> +
> >>>>>> +    return caps_list;
> >>>>>> +}
> >>>>>> +
> >>>>>> +void qmp_migrate_set_parameters(MigrationCapabilityInfoList *params,
> >>>>>> +                                Error **errp)
> >>>>>> +{
> >>>>>> +    MigrationState *s = migrate_get_current();
> >>>>>> +    MigrationCapabilityInfoList *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)
> >>>>>> @@ -375,12 +438,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 f6107ba..e2be6cd 100644
> >>>>>> --- a/monitor.c
> >>>>>> +++ b/monitor.c
> >>>>>> @@ -2687,6 +2687,13 @@ static mon_cmd_t info_cmds[] = {
> >>>>>>          .mhandler.info = hmp_info_migrate,
> >>>>>>      },
> >>>>>>      {
> >>>>>> +        .name       = "migration_capabilities",
> >>>>>
> >>>>> migration-capabilities is better.
> >>>> ok
> >>>>>
> >>>>>> +        .args_type  = "",
> >>>>>> +        .params     = "",
> >>>>>> +        .help       = "show migration capabilities",
> >>>>>> +        .mhandler.info = hmp_info_migration_capabilities,
> >>>>>> +    },
> >>>>>> +    {
> >>>>>>          .name       = "balloon",
> >>>>>>          .args_type  = "",
> >>>>>>          .params     = "",
> >>>>>> diff --git a/qapi-schema.json b/qapi-schema.json
> >>>>>> index 1ab5dbd..a8408fd 100644
> >>>>>> --- a/qapi-schema.json
> >>>>>> +++ b/qapi-schema.json
> >>>>>> @@ -288,11 +288,15 @@
> >>>>>>  #        status, only returned if status is 'active' and it is a block
> >>>>>>  #        migration
> >>>>>>  #
> >>>>>> -# Since: 0.14.0
> >>>>>> +# @capabilities: #optional a list describing all the migration 
> >>>>>> capabilities
> >>>>>> +#                state
> >>>>>
> >>>>> I don't think this is needed, as I've said above.
> >>>> see above
> >>>>>
> >>>>>> +#
> >>>>>> +# Since: 0.14.0, 'capabilities' since 1.2
> >>>>>>  ##
> >>>>>>  { 'type': 'MigrationInfo',
> >>>>>>    'data': {'*status': 'str', '*ram': 'MigrationStats',
> >>>>>> -           '*disk': 'MigrationStats'} }
> >>>>>> +           '*disk': 'MigrationStats',
> >>>>>> +           '*capabilities': ['MigrationCapabilityInfo']} }
> >>>>>>
> >>>>>>  ##
> >>>>>>  # @query-migrate
> >>>>>> @@ -306,6 +310,51 @@
> >>>>>>  { 'command': 'query-migrate', 'returns': 'MigrationInfo' }
> >>>>>>
> >>>>>>  ##
> >>>>>> +# @MigrationCapability
> >>>>>> +#
> >>>>>> +# Migration capabilities enumeration
> >>>>>> +#
> >>>>>> +# @xbzrle: current migration supports xbzrle
> >>>>>
> >>>>> You should explain what xbzrle is.
> >>>> ok
> >>>>>
> >>>>>> +#
> >>>>>> +# Since: 1.2
> >>>>>> +##
> >>>>>> +{ 'enum': 'MigrationCapability',
> >>>>>> +  'data': ['xbzrle'] }
> >>>>>> +
> >>>>>> +##
> >>>>>> +# @MigrationCapabilityInfo
> >>>>>
> >>>>> MigrationCapabilityStatus?
> >>>>>
> >>>>>> +#
> >>>>>> +# Migration capability information
> >>>>>> +#
> >>>>>> +# @capability: capability enum
> >>>>>
> >>>>> Please, document state too.
> >>>> ok
> >>>>
> >>>> Orit
> >>>>>
> >>>>>> +#
> >>>>>> +# Since: 1.2
> >>>>>> +##
> >>>>>> +{ 'type': 'MigrationCapabilityInfo',
> >>>>>> +  'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }
> >>>>>> +
> >>>>>> +##
> >>>>>> +# @query-migration-capabilities
> >>>>>> +#
> >>>>>> +# Returns information about current migration process capabilties.
> >>>>>> +#
> >>>>>> +# Returns: @MigrationCapabilityInfo list
> >>>>>> +#
> >>>>>> +# Since: 1.2
> >>>>>> +##
> >>>>>> +{ 'command': 'query-migration-capabilities', 'returns': 
> >>>>>> ['MigrationCapabilityInfo'] }
> >>>>>> +
> >>>>>> +##
> >>>>>> +# @migrate_set_parameters
> >>>>>> +#
> >>>>>> +# Enable/Disable the following migration capabilities (like xbzrle)
> >>>>>> +#
> >>>>>> +# Since: 1.2
> >>>>>> +##
> >>>>>> +{ 'command': 'migrate-set-parameters',
> >>>>>> +  'data': { 'capabilities': ['MigrationCapabilityInfo'] } }
> >>>>>> +
> >>>>>> +##
> >>>>>>  # @MouseInfo:
> >>>>>>  #
> >>>>>>  # Information about a mouse device.
> >>>>>> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >>>>>> index 2e1a38e..3ee6e00 100644
> >>>>>> --- a/qmp-commands.hx
> >>>>>> +++ b/qmp-commands.hx
> >>>>>> @@ -2075,18 +2075,31 @@ The main json-object contains the following:
> >>>>>>           - "transferred": amount transferred (json-int)
> >>>>>>           - "remaining": amount remaining (json-int)
> >>>>>>           - "total": total (json-int)
> >>>>>> -
> >>>>>> +- "capabilities": migration capabilities state
> >>>>>> +         - "xbzrle" : XBZRLE state (json-bool)
> >>>>>>  Examples:
> >>>>>>
> >>>>>>  1. Before the first migration
> >>>>>>
> >>>>>>  -> { "execute": "query-migrate" }
> >>>>>> -<- { "return": {} }
> >>>>>> +<- { "return": {
> >>>>>> +        "capabilities" :  [ { "capability" : "xbzrle", "state" : 
> >>>>>> false } ]
> >>>>>> +     }
> >>>>>> +   }
> >>>>>>
> >>>>>>  2. Migration is done and has succeeded
> >>>>>>
> >>>>>>  -> { "execute": "query-migrate" }
> >>>>>> -<- { "return": { "status": "completed" } }
> >>>>>> +<- { "return": {
> >>>>>> +        "status": "completed",
> >>>>>> +        "capabilities":  [ { "capability" : "xbzrle", "state" : false 
> >>>>>> } ],
> >>>>>> +        "ram":{
> >>>>>> +          "transferred":123,
> >>>>>> +          "remaining":123,
> >>>>>> +          "total":246
> >>>>>> +        }
> >>>>>> +     }
> >>>>>> +   }
> >>>>>>
> >>>>>>  3. Migration is done and has failed
> >>>>>>
> >>>>>> @@ -2099,6 +2112,7 @@ Examples:
> >>>>>>  <- {
> >>>>>>        "return":{
> >>>>>>           "status":"active",
> >>>>>> +         "capabilities":  [ { "capability" : "xbzrle", "state" : 
> >>>>>> false } ],
> >>>>>>           "ram":{
> >>>>>>              "transferred":123,
> >>>>>>              "remaining":123,
> >>>>>> @@ -2113,6 +2127,7 @@ Examples:
> >>>>>>  <- {
> >>>>>>        "return":{
> >>>>>>           "status":"active",
> >>>>>> +         "capabilities":  [ { "capability" : "xbzrle", "state" : 
> >>>>>> false } ],
> >>>>>>           "ram":{
> >>>>>>              "total":1057024,
> >>>>>>              "remaining":1053304,
> >>>>>> @@ -2135,6 +2150,56 @@ EQMP
> >>>>>>      },
> >>>>>>
> >>>>>>  SQMP
> >>>>>> +query-migration-capabilities
> >>>>>> +-------
> >>>>>> +
> >>>>>> +Query migration capabilities
> >>>>>> +
> >>>>>> +- "xbzrle": xbzrle support
> >>>>>> +
> >>>>>> +Arguments:
> >>>>>> +
> >>>>>> +Example:
> >>>>>> +
> >>>>>> +-> { "execute": "query-migration-capabilities"}
> >>>>>> +<- { "return": [ { "capability": "xbzrle", "state": true },
> >>>>>> +                 { "capability": "foobar", "state": false } ] }
> >>>>>> +
> >>>>>> +EQMP
> >>>>>> +
> >>>>>> +    {
> >>>>>> +        .name       = "query-migration-capabilities",
> >>>>>> +        .args_type  = "",
> >>>>>> +      .mhandler.cmd_new = 
> >>>>>> qmp_marshal_input_query_migration_capabilities,
> >>>>>> +    },
> >>>>>> +
> >>>>>> +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-balloon
> >>>>>>  -------------
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>
> >>
> > 
>