diff mbox

[v14,02/13] Add migration capabilities

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

Commit Message

Orit Wasserman July 3, 2012, 1:52 p.m. UTC 
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.

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 hmp-commands.hx  |   16 ++++++++++++
 hmp.c            |   63 +++++++++++++++++++++++++++++++++++++++++++++++
 hmp.h            |    2 +
 migration.c      |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 migration.h      |    2 +
 monitor.c        |    7 +++++
 qapi-schema.json |   48 +++++++++++++++++++++++++++++++++++-
 qmp-commands.hx  |   50 +++++++++++++++++++++++++++++++++++++
 8 files changed, 257 insertions(+), 3 deletions(-)

Comments

Eric Blake July 3, 2012, 6:36 p.m. UTC | #1 
On 07/03/2012 07:52 AM, Orit Wasserman wrote:
> 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.

Couple of comment nits below.  Also, if you don't mind a bit of
bike-shedding...

[roughly translated - feel free to ignore the bulk of this email; the
patch as-is works, if you don't think my alternate design makes sense to
implement]

> +++ 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     = "",
> +        .help       = "Enable the usage of a capability for migration",
> +        .mhandler.cmd = hmp_migrate_set_parameter,

This requires the 'state' parameter to be passed.  But wouldn't it be
easier to default things to state=true if no parameter was present,
since the most common usage of this command will be to enable, rather
than disable, a migration parameter?

> -        /* no migration has happened ever */
> +        /* no migration has happened ever show enabled capabilities */

Missing some punctuation, so this was hard to read.  Maybe:

/* no migration has ever happened; show enabled capabilities */

> +++ b/qapi-schema.json

> +##
> +# @MigrationCapabilityInfo
> +#
> +# Migration capability information
> +#
> +# @capability: capability enum
> +#
> +# Since: 1.2
> +##
> +{ 'type': 'MigrationCapabilityInfo',
> +  'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }

Back to the bike-shedding - if you would mark this '*state':'bool', then
the enabled parameter becomes optional on input (it should still always
be present on query-migration-capabilities output, though).

> +migrate_set_parameters
> +-------
> +
> +Enable/Disable migration capabilities
> +
> +- "xbzrle": xbzrle support
> +
> +Arguments:
> +
> +Example:
> +
> +-> { "execute": "migrate_set_parameters" , "arguments":
> +     { "parameters":  { "capability": "xbzrle", "state": true } ] } }

Missing a '[' after "parameters":
Orit Wasserman July 5, 2012, 10:09 a.m. UTC | #2 
On 07/03/2012 09:36 PM, Eric Blake wrote:
> On 07/03/2012 07:52 AM, Orit Wasserman wrote:
>> 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.
> 
> Couple of comment nits below.  Also, if you don't mind a bit of
> bike-shedding...
> 
> [roughly translated - feel free to ignore the bulk of this email; the
> patch as-is works, if you don't think my alternate design makes sense to
> implement]
> 
>> +++ 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     = "",
>> +        .help       = "Enable the usage of a capability for migration",
>> +        .mhandler.cmd = hmp_migrate_set_parameter,
> 
> This requires the 'state' parameter to be passed.  But wouldn't it be
> easier to default things to state=true if no parameter was present,
> since the most common usage of this command will be to enable, rather
> than disable, a migration parameter?
After thinking about it , I prefer to leave to command as it is.
For me it feels strange that the enable will be different from the disable.

Orit
> 
>> -        /* no migration has happened ever */
>> +        /* no migration has happened ever show enabled capabilities */
> 
> Missing some punctuation, so this was hard to read.  Maybe:
> 
> /* no migration has ever happened; show enabled capabilities */
> 
>> +++ b/qapi-schema.json
> 
>> +##
>> +# @MigrationCapabilityInfo
>> +#
>> +# Migration capability information
>> +#
>> +# @capability: capability enum
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'type': 'MigrationCapabilityInfo',
>> +  'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }
> 
> Back to the bike-shedding - if you would mark this '*state':'bool', then
> the enabled parameter becomes optional on input (it should still always
> be present on query-migration-capabilities output, though).
> 
>> +migrate_set_parameters
>> +-------
>> +
>> +Enable/Disable migration capabilities
>> +
>> +- "xbzrle": xbzrle support
>> +
>> +Arguments:
>> +
>> +Example:
>> +
>> +-> { "execute": "migrate_set_parameters" , "arguments":
>> +     { "parameters":  { "capability": "xbzrle", "state": true } ] } }
> 
> Missing a '[' after "parameters":
>
diff mbox

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f5d9d91..a0c8df2 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     = "",
+        .help       = "Enable 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 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 b9cec1d..69711fb 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");
+    }
     if (info->has_status) {
         monitor_printf(mon, "Migration status: %s\n", info->status);
     }
@@ -159,6 +169,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;
@@ -733,6 +761,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 810727f..4b68146 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 happened ever 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");
 
@@ -141,6 +165,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;
+        }
+
         info->has_status = true;
         info->status = g_strdup("completed");
         break;
@@ -157,6 +193,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;
+
+    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)
@@ -365,12 +428,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 35207bd..1ae99f1 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 {
     bool 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 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",
+        .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 3b6e346..3eb3003 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -286,7 +286,8 @@ 
 ##
 { 'type': 'MigrationInfo',
   'data': {'*status': 'str', '*ram': 'MigrationStats',
-           '*disk': 'MigrationStats'} }
+           '*disk': 'MigrationStats',
+           '*capabilities': ['MigrationCapabilityInfo']} }
 
 ##
 # @query-migrate
@@ -300,6 +301,51 @@ 
 { 'command': 'query-migrate', 'returns': 'MigrationInfo' }
 
 ##
+# @MigrationCapability
+#
+# Migration capabilities enumeration
+#
+# @xbzrle: current migration supports xbzrle
+#
+# Since: 1.2
+##
+{ 'enum': 'MigrationCapability',
+  'data': ['xbzrle'] }
+
+##
+# @MigrationCapabilityInfo
+#
+# Migration capability information
+#
+# @capability: capability enum
+#
+# 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..7e85e0d 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2135,6 +2135,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
 -------------