diff mbox

[v11,2/9] Add migration capabilites

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

Commit Message

Orit Wasserman May 22, 2012, 12:56 p.m. UTC 
Add migration capabiltes that can be queried by the management.
The managment can query the source QEMU and the destination QEMU in order to
verify both support some  migration capability (currently only XBZRLE).
The managment can enable a capabilty for the next migration by using
migrate_set_parameter command.

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 hmp-commands.hx  |   16 ++++++++++++++++
 hmp.c            |   41 +++++++++++++++++++++++++++++++++++++++++
 hmp.h            |    2 ++
 migration.c      |   53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 migration.h      |    2 ++
 monitor.c        |    7 +++++++
 qapi-schema.json |   46 +++++++++++++++++++++++++++++++++++++++++++++-
 qmp-commands.hx  |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 savevm.c         |    2 +-
 9 files changed, 212 insertions(+), 4 deletions(-)

Comments

Juan Quintela June 1, 2012, 10:57 a.m. UTC | #1 
Orit Wasserman <owasserm@redhat.com> wrote:
> Add migration capabiltes that can be queried by the management.
> The managment can query the source QEMU and the destination QEMU in order to
> verify both support some  migration capability (currently only XBZRLE).
> The managment can enable a capabilty for the next migration by using
> migrate_set_parameter command.
>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> +void qmp_migrate_set_parameter(const char *parameter, Error **errp)
> +{
> +    MigrationState *s = migrate_get_current();
> +    int i;
> +
> +    if (s->state == MIG_STATE_ACTIVE) {
> +        error_set(errp, QERR_MIGRATION_ACTIVE);
> +        return;
> +    }
> +
> +    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
> +        if (strcmp(parameter, MigrationCapability_lookup[i]) == 0) {
> +            s->enabled_capabilities[i] = true;
> +            return;
> +        }
> +    }
> +
> +    error_set(errp, QERR_INVALID_PARAMETER, parameter);
> +}

Two things here:
- Is there a way to disable capabilities?  it seems no.
- Would we want in the future capabilities that are not "bool"?  Just
  asking loud, I haven't thought a lot about this.  Fixing it as a
  paramenter, it would make trivial to fix previous comment: cap:true vs
  cap:false, or whatever syntax we want.

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

Nice catch/cleanup.


> diff --git a/savevm.c b/savevm.c
> index dd66f2c..42937a0 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1711,7 +1711,7 @@ static int qemu_savevm_state(QEMUFile *f)
>      int ret;
>      MigrationParams params = {
>          .blk = 0,
> -        .shared = 0
> +        .shared = 0,
>      };
>  
>      if (qemu_savevm_state_blocked(NULL)) {

This belongs to previous patch?

Later, Juan.
Orit Wasserman June 6, 2012, 1:48 a.m. UTC | #2 
On 06/01/2012 01:57 PM, Juan Quintela wrote:
> Orit Wasserman <owasserm@redhat.com> wrote:
>> Add migration capabiltes that can be queried by the management.
>> The managment can query the source QEMU and the destination QEMU in order to
>> verify both support some  migration capability (currently only XBZRLE).
>> The managment can enable a capabilty for the next migration by using
>> migrate_set_parameter command.
>>
>> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
>> +void qmp_migrate_set_parameter(const char *parameter, Error **errp)
>> +{
>> +    MigrationState *s = migrate_get_current();
>> +    int i;
>> +
>> +    if (s->state == MIG_STATE_ACTIVE) {
>> +        error_set(errp, QERR_MIGRATION_ACTIVE);
>> +        return;
>> +    }
>> +
>> +    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
>> +        if (strcmp(parameter, MigrationCapability_lookup[i]) == 0) {
>> +            s->enabled_capabilities[i] = true;
>> +            return;
>> +        }
>> +    }
>> +
>> +    error_set(errp, QERR_INVALID_PARAMETER, parameter);
>> +}
> 
> Two things here:
> - Is there a way to disable capabilities?  it seems no.

In this implementation we can't disable a capability , do you see a need to add it ?

> - Would we want in the future capabilities that are not "bool"?  Just
>   asking loud, I haven't thought a lot about this.  Fixing it as a
>   paramenter, it would make trivial to fix previous comment: cap:true vs
>   cap:false, or whatever syntax we want.
That is a good idea I will change it in next patch set.

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;
>>      s->state = MIG_STATE_SETUP;
> 
> Nice catch/cleanup.
> 
> 
>> diff --git a/savevm.c b/savevm.c
>> index dd66f2c..42937a0 100644
>> --- a/savevm.c
>> +++ b/savevm.c
>> @@ -1711,7 +1711,7 @@ static int qemu_savevm_state(QEMUFile *f)
>>      int ret;
>>      MigrationParams params = {
>>          .blk = 0,
>> -        .shared = 0
>> +        .shared = 0,
>>      };
>>  
>>      if (qemu_savevm_state_blocked(NULL)) {
> 
> This belongs to previous patch?
> 
> Later, Juan.
Juan Quintela June 7, 2012, 10:41 a.m. UTC | #3 
Orit Wasserman <owasserm@redhat.com> wrote:
> On 06/01/2012 01:57 PM, Juan Quintela wrote:
>> Orit Wasserman <owasserm@redhat.com> wrote:
>>> Add migration capabiltes that can be queried by the management.
>>> The managment can query the source QEMU and the destination QEMU in order to
>>> verify both support some  migration capability (currently only XBZRLE).
>>> The managment can enable a capabilty for the next migration by using
>>> migrate_set_parameter command.
>>>
>>> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
>>> +void qmp_migrate_set_parameter(const char *parameter, Error **errp)
>>> +{
>>> +    MigrationState *s = migrate_get_current();
>>> +    int i;
>>> +
>>> +    if (s->state == MIG_STATE_ACTIVE) {
>>> +        error_set(errp, QERR_MIGRATION_ACTIVE);
>>> +        return;
>>> +    }
>>> +
>>> +    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
>>> +        if (strcmp(parameter, MigrationCapability_lookup[i]) == 0) {
>>> +            s->enabled_capabilities[i] = true;
>>> +            return;
>>> +        }
>>> +    }
>>> +
>>> +    error_set(errp, QERR_INVALID_PARAMETER, parameter);
>>> +}
>> 
>> Two things here:
>> - Is there a way to disable capabilities?  it seems no.
>
> In this implementation we can't disable a capability , do you see a
> need to add it ?

As we continue adding capabilities, I guess that at least for
testing. it is going to be needed.  Specially if we decide the path that
Anthony suggested:

set_capababilities(interesction(caps_source, caps_target))

if we do something like that, and we _know_ that we don't want a
capabilitie because we know it dont' work for our load, it sounds like a
good idea to have a good way, and the other reason is the next comment.
If we could have a capability that is _not_ a bool, we need to be able
to assign a value anyways.  Notice that I still don't know if we are
going to need it.  But can see one reason one way or another.

>
>> - Would we want in the future capabilities that are not "bool"?  Just
>>   asking loud, I haven't thought a lot about this.  Fixing it as a
>>   paramenter, it would make trivial to fix previous comment: cap:true vs
>>   cap:false, or whatever syntax we want.
> That is a good idea I will change it in next patch set.

Thanks, Juan.
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 18cb415..e14e7be 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  = "parameter:s",
+        .params     = "parameter",
+        .help       = "Enable the usage of a capability for migration",
+        .mhandler.cmd = hmp_migrate_set_parameter,
+    },
+
+STEXI
+@item migrate_set_parameter @var{parameter}
+@findex migrate_set_parameter
+Enable the usage of a capability @var{parameter} for migration.
+ETEXI
+
+    {
         .name       = "client_migrate_info",
         .args_type  = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
         .params     = "protocol hostname port tls-port cert-subject",
@@ -1393,6 +1407,8 @@  show CPU statistics
 show user network stack connection states
 @item info migrate
 show migration status
+@item info migration_capabilities
+show migration capabilities
 @item info balloon
 show balloon information
 @item info qtree
diff --git a/hmp.c b/hmp.c
index bb0952e..9582400 100644
--- a/hmp.c
+++ b/hmp.c
@@ -128,9 +128,18 @@  void hmp_info_mice(Monitor *mon)
 void hmp_info_migrate(Monitor *mon)
 {
     MigrationInfo *info;
+    MigrationCapabilityInfoList *cap;
 
     info = qmp_query_migrate(NULL);
 
+    if (info->has_params && info->params) {
+        monitor_printf(mon, "params: ");
+        for (cap = info->params; cap; cap = cap->next) {
+            monitor_printf(mon, "%s",
+                           MigrationCapability_lookup[cap->value->capability]);
+        }
+        monitor_printf(mon, "\n");
+    }
     if (info->has_status) {
         monitor_printf(mon, "Migration status: %s\n", info->status);
     }
@@ -156,6 +165,24 @@  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 ",
+                       MigrationCapability_lookup[cap->value->capability]);
+    }
+
+    qapi_free_MigrationCapabilityInfoList(caps_list);
+}
+
 void hmp_info_cpus(Monitor *mon)
 {
     CpuInfoList *cpu_list, *cpu;
@@ -730,6 +757,20 @@  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 *value = qdict_get_str(qdict, "parameter");
+    Error *err = NULL;
+
+    qmp_migrate_set_parameter(value, &err);
+
+    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 443b812..5f9d842 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 810727f..952f542 100644
--- a/migration.c
+++ b/migration.c
@@ -117,10 +117,22 @@  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 happened ever show enabled capabilities */
+        for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
+            if (s->enabled_capabilities[i]) {
+                if (!info->has_params) {
+                    info->params = g_malloc0(sizeof(*info->params));
+                    info->has_params = true;
+                }
+                info->params->value = g_malloc(sizeof(*info->params->value));
+                info->params->value->capability = i;
+                info->params->next = NULL;
+            }
+        }
         break;
     case MIG_STATE_ACTIVE:
         info->has_status = true;
@@ -157,6 +169,38 @@  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;
+
+    return caps_list;
+}
+
+
+void qmp_migrate_set_parameter(const char *parameter, Error **errp)
+{
+    MigrationState *s = migrate_get_current();
+    int i;
+
+    if (s->state == MIG_STATE_ACTIVE) {
+        error_set(errp, QERR_MIGRATION_ACTIVE);
+        return;
+    }
+
+    for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
+        if (strcmp(parameter, MigrationCapability_lookup[i]) == 0) {
+            s->enabled_capabilities[i] = true;
+            return;
+        }
+    }
+
+    error_set(errp, QERR_INVALID_PARAMETER, parameter);
+}
+
 /* shared migration helpers */
 
 static int migrate_fd_cleanup(MigrationState *s)
@@ -365,12 +409,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;
 
     return s;
diff --git a/migration.h b/migration.h
index 4168883..00d1992 100644
--- a/migration.h
+++ b/migration.h
@@ -18,6 +18,7 @@ 
 #include "qemu-common.h"
 #include "notify.h"
 #include "error.h"
+#include "qapi-types.h"
 
 struct MigrationParams {
     int blk;
@@ -37,6 +38,7 @@  struct MigrationState
     int (*write)(MigrationState *s, const void *buff, size_t size);
     void *opaque;
     MigrationParams params;
+    bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
 };
 
 void process_incoming_migration(QEMUFile *f);
diff --git a/monitor.c b/monitor.c
index 12a6fe2..0233bc3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2558,6 +2558,13 @@  static mon_cmd_t info_cmds[] = {
         .mhandler.info = hmp_info_migrate,
     },
     {
+        .name       = "migration_capabilities",
+        .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 2ca7195..2887c51 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -264,7 +264,7 @@ 
 ##
 { 'type': 'MigrationInfo',
   'data': {'*status': 'str', '*ram': 'MigrationStats',
-           '*disk': 'MigrationStats'} }
+           '*disk': 'MigrationStats', '*params': ['MigrationCapabilityInfo']} }
 
 ##
 # @query-migrate
@@ -278,6 +278,50 @@ 
 { 'command': 'query-migrate', 'returns': 'MigrationInfo' }
 
 ##
+# @MigrationCapability
+#
+# Migration capabilities enumaration
+#
+# @xbzrle: current migration supports xbzrle
+#
+# Since: 1.1
+##
+{ 'enum': 'MigrationCapability',
+  'data': ['xbzrle'] }
+
+##
+# @MigrationCapabilityInfo
+#
+# Migration capability information
+#
+# @capability: capability enum
+#
+# Since: 1.2
+##
+{ 'type': 'MigrationCapabilityInfo',
+  'data': { 'capability' : 'MigrationCapability'} }
+
+##
+# @query-migration-capabilities
+#
+# Returns information about current migration process capabilties.
+#
+# Returns: @MigrationCapabilityInfo list
+#
+# Since: 1.2
+##
+{ 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityInfo'] }
+
+##
+# @migrate_set_parameter
+#
+# Set the following migration parameters (like xbzrle )
+##
+# Since: 1.2
+##
+{ 'command': 'migrate-set-parameter', 'data': { 'parameter': 'str' } }
+
+##
 # @MouseInfo:
 #
 # Information about a mouse device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db980fa..7750f2f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2068,6 +2068,53 @@  EQMP
     },
 
 SQMP
+query-migration-capabilities
+-------
+
+Query migration capabilities
+
+- "xbzrle": xbzrle support
+
+Arguments:
+
+Example:
+
+-> { "execute": "query-migration-capabilities"}
+<- { "return": { "xbzrle" }
+
+EQMP
+
+    {
+        .name       = "query-migration-capabilities",
+        .args_type  = "",
+	.mhandler.cmd_new = qmp_marshal_input_query_migration_capabilities,
+    },
+
+SQMP
+migrate_set_parameter
+-------
+
+Enable migration parameter
+
+- "xbzrle": xbzrle support
+
+Arguments:
+
+Example:
+
+-> { "execute": "migrate_set_parameter" , "arguments": { "parameter": xbzrle"} }
+
+EQMP
+
+    {
+        .name       = "migrate_set_parameter",
+        .args_type  = "parameter:s",
+	.mhandler.cmd_new = qmp_marshal_input_migrate_set_parameter,
+    },
+
+
+
+SQMP
 query-balloon
 -------------
 
diff --git a/savevm.c b/savevm.c
index dd66f2c..42937a0 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1711,7 +1711,7 @@  static int qemu_savevm_state(QEMUFile *f)
     int ret;
     MigrationParams params = {
         .blk = 0,
-        .shared = 0
+        .shared = 0,
     };
 
     if (qemu_savevm_state_blocked(NULL)) {