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

[Orit Wasserman]

On 07/25/2012 04:11 PM, Luiz Capitulino wrote:
> 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.
ok
> 
>>>
>>>> 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).
you are right .
Orit
> 
>>
>>>
>>> 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
>>>>>>>>  -------------
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>