Patchwork [RFC,RDMA,support,v2:,3/6] install new monitor commands and setup RDMA capabilities

login
register
mail settings
Submitter mrhines@linux.vnet.ibm.com
Date Feb. 11, 2013, 10:49 p.m.
Message ID <1360622997-26904-3-git-send-email-mrhines@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/219690/
State New
Headers show

Comments

mrhines@linux.vnet.ibm.com - Feb. 11, 2013, 10:49 p.m.
From: "Michael R. Hines" <mrhines@us.ibm.com>


Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
 hmp-commands.hx               |   28 ++++++++++++++++++++++++++++
 hmp.c                         |   12 ++++++++++++
 hmp.h                         |    2 ++
 include/migration/qemu-file.h |    1 +
 include/qapi/qmp/qerror.h     |    6 ++++++
 qapi-schema.json              |   34 ++++++++++++++++++++++++++++++++--
 qemu-options.hx               |   10 ++++++++++
 7 files changed, 91 insertions(+), 2 deletions(-)
Eric Blake - Feb. 11, 2013, 10:58 p.m.
On 02/11/2013 03:49 PM, Michael R. Hines wrote:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
> 
> 
> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
> ---

> +++ b/include/qapi/qmp/qerror.h
> @@ -165,6 +165,12 @@ void assert_no_error(Error *err);
>  #define QERR_MIGRATION_ACTIVE \
>      ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
>  
> +#define QERR_MIGRATION_RDMA_NOT_CONFIGURED \
> +    ERROR_CLASS_GENERIC_ERROR, "RMDA-based migration is missing port/hostname information %s:%d"
> +
> +#define QERR_MIGRATION_RDMA_NOT_ENABLED \
> +    ERROR_CLASS_GENERIC_ERROR, "RMDA-based migration has not been compiled/enabled."
> +

You should not be creating new #defines for generic errors.  Instead,
use error_setg() at the place where you want to issue these errors.

> +++ b/qapi-schema.json

>  
>  ##
> +# @migrate_set_rdma_port

New QMP commands should favor dash, not underscore.  This should be
named 'migrate-set-rdma-port'; or perhaps we should consider a more
generic command 'migrate-set-parameter', which then takes a name/value
pair of which parameter to set, instead of having to add a new command
for every tweakable parameter.

> +#
> +# Set rdma communication port.
> +#
> +# @port: rdma communication port.
> +#
> +# Returns: nothing on success
> +#
> +# Notes: Nothing.
> +#
> +# Since: 1.3.0

1.5, at the earliest.

> +##
> +{ 'command': 'migrate_set_rdma_port', 'data': {'port': 'int'} }
> +
> +##
> +# @migrate_set_rdma_host
> +#
> +# Set rdma communication host address.
> +#
> +# @host: rdma communication host address.
> +#
> +# Returns: nothing on success
> +#
> +# Notes: Nothing.
> +#
> +# Since: 1.3.0
> +##
> +{ 'command': 'migrate_set_rdma_host', 'data': {'host': 'str'} }

Likewise: 1.5 at the earliest, and migrate-set-rdma-host
mrhines@linux.vnet.ibm.com - Feb. 12, 2013, 3:06 a.m.
None of the other migration QMP commands use dashes.
Shouldn't I stay consistent with the other commands?

If someone wants to re-work the migration command structure as a whole, 
I'll certainly adapt to the new structure.....

On 02/11/2013 05:58 PM, Eric Blake wrote:
>> +++ b/qapi-schema.json
>>   ##
>> +# @migrate_set_rdma_port
> New QMP commands should favor dash, not underscore.  This should be
> named 'migrate-set-rdma-port'; or perhaps we should consider a more
> generic command 'migrate-set-parameter', which then takes a name/value
> pair of which parameter to set, instead of having to add a new command
> for every tweakable parameter.

If someone wants to re-work the migration command structure as a whole, 
I'll certainly adapt to the new structure.....
>> +#
>> +# Set rdma communication port.
>> +#
>> +# @port: rdma communication port.
>> +#
>> +# Returns: nothing on success
>> +#
>> +# Notes: Nothing.
>> +#
>> +# Since: 1.3.0
> 1.5, at the earliest.
Acknowledged.
Eric Blake - Feb. 12, 2013, 5:05 p.m.
On 02/11/2013 08:06 PM, Michael R. Hines wrote:

[no need to top-post on a technical list]

> None of the other migration QMP commands use dashes.

Then how do you explain 'query-migrate', 'migrate-set-capabilities',
'query-migrate-capabilities', 'migrate-set-cache-size',
'query-migrate-cache-size'?  I argue that 'migrate_cancel' and
'migrate_set_downtime' are the odd ones out, and only because they were
written before our current policy of preferring dash.

> Shouldn't I stay consistent with the other commands?

Yes, which is why I'm arguing for using dash.
Michael R Hines - Feb. 12, 2013, 6:23 p.m.
Acknowledged.

/*
 * Michael R. Hines
 * http://researcher.ibm.com/person/us-mrhines
 */



From:
Eric Blake <eblake@redhat.com>
To:
"Michael R. Hines" <mrhines@linux.vnet.ibm.com>, 
Cc:
qemu-devel@nongnu.org, Anthony Liguori/Austin/IBM@IBMUS, Bulent 
Abali/Watson/IBM@IBMUS, Michael R Hines/Watson/IBM@IBMUS, Gokul B 
Kandiraju/Watson/IBM@IBMUS
Date:
02/12/2013 12:11 PM
Subject:
Re: [Qemu-devel] [RFC PATCH RDMA support v2: 3/6] install new monitor 
commands and setup RDMA capabilities



On 02/11/2013 08:06 PM, Michael R. Hines wrote:

[no need to top-post on a technical list]

> None of the other migration QMP commands use dashes.

Then how do you explain 'query-migrate', 'migrate-set-capabilities',
'query-migrate-capabilities', 'migrate-set-cache-size',
'query-migrate-cache-size'?  I argue that 'migrate_cancel' and
'migrate_set_downtime' are the odd ones out, and only because they were
written before our current policy of preferring dash.

> Shouldn't I stay consistent with the other commands?

Yes, which is why I'm arguing for using dash.
Markus Armbruster - Feb. 12, 2013, 6:53 p.m.
Eric Blake <eblake@redhat.com> writes:

> On 02/11/2013 08:06 PM, Michael R. Hines wrote:
>
> [no need to top-post on a technical list]
>
>> None of the other migration QMP commands use dashes.
>
> Then how do you explain 'query-migrate', 'migrate-set-capabilities',
> 'query-migrate-capabilities', 'migrate-set-cache-size',
> 'query-migrate-cache-size'?  I argue that 'migrate_cancel' and
> 'migrate_set_downtime' are the odd ones out, and only because they were
> written before our current policy of preferring dash.
>
>> Shouldn't I stay consistent with the other commands?
>
> Yes, which is why I'm arguing for using dash.

Correct.  Use dash instead of underscore for new commands and JSON
object member names.

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0934b9b..4466c2e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -910,6 +910,34 @@  Set maximum speed to @var{value} (in bytes) for migrations.
 ETEXI
 
     {
+        .name       = "migrate_set_rdma_port",
+        .args_type  = "port:i",
+        .params     = "port",
+        .help       = "set RDMA port for migration",
+        .mhandler.cmd = hmp_migrate_set_rdma_port,
+    },
+
+STEXI
+@item migrate_set_rdma_port @var{value}
+@findex migrate_set_rdma_port
+Set RDMA migration port to @var{value}.
+ETEXI
+
+    {
+        .name       = "migrate_set_rdma_host",
+        .args_type  = "host:s",
+        .params     = "host",
+        .help       = "set RDMA host name for migration",
+        .mhandler.cmd = hmp_migrate_set_rdma_host,
+    },
+
+STEXI
+@item migrate_set_rdma_host @var{value}
+@findex migrate_set_rdma_host
+Set RDMA migration host name to @var{value}.
+ETEXI
+
+    {
         .name       = "migrate_set_downtime",
         .args_type  = "value:T",
         .params     = "value",
diff --git a/hmp.c b/hmp.c
index c7b6ba0..67c1d4a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -848,6 +848,18 @@  void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
     }
 }
 
+void hmp_migrate_set_rdma_port(Monitor *mon, const QDict *qdict)
+{
+    int port = qdict_get_int(qdict, "port");
+    qmp_migrate_set_rdma_port(port, NULL);
+}
+
+void hmp_migrate_set_rdma_host(Monitor *mon, const QDict *qdict)
+{
+    const char *host = qdict_get_str(qdict, "host");
+    qmp_migrate_set_rdma_host(host, NULL);
+}
+
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
 {
     int64_t value = qdict_get_int(qdict, "value");
diff --git a/hmp.h b/hmp.h
index 44be683..4ee5b92 100644
--- a/hmp.h
+++ b/hmp.h
@@ -55,6 +55,8 @@  void hmp_drive_mirror(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_rdma_port(Monitor *mon, const QDict *qdict);
+void hmp_migrate_set_rdma_host(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict);
 void hmp_set_password(Monitor *mon, const QDict *qdict);
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 68deefb..7c9968e 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -112,6 +112,7 @@  int qemu_file_rate_limit(QEMUFile *f);
 int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
 int64_t qemu_file_get_rate_limit(QEMUFile *f);
 int qemu_file_get_error(QEMUFile *f);
+void qemu_file_set_error(QEMUFile *f, int ret);
 
 static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
 {
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..6ae8b8c 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -165,6 +165,12 @@  void assert_no_error(Error *err);
 #define QERR_MIGRATION_ACTIVE \
     ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
 
+#define QERR_MIGRATION_RDMA_NOT_CONFIGURED \
+    ERROR_CLASS_GENERIC_ERROR, "RMDA-based migration is missing port/hostname information %s:%d"
+
+#define QERR_MIGRATION_RDMA_NOT_ENABLED \
+    ERROR_CLASS_GENERIC_ERROR, "RMDA-based migration has not been compiled/enabled."
+
 #define QERR_MIGRATION_NOT_SUPPORTED \
     ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device '%s'"
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 6d7252b..5ce56f1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -483,7 +483,7 @@ 
 # Since: 1.2
 ##
 { 'enum': 'MigrationCapability',
-  'data': ['xbzrle'] }
+  'data': ['xbzrle', 'rdma'] }
 
 ##
 # @MigrationCapabilityStatus
@@ -502,7 +502,7 @@ 
 ##
 # @migrate-set-capabilities
 #
-# Enable/Disable the following migration capabilities (like xbzrle)
+# Enable/Disable the following migration capabilities (like xbzrle or RDMA)
 #
 # @capabilities: json array of capability modifications to make
 #
@@ -1695,6 +1695,36 @@ 
 { 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
 
 ##
+# @migrate_set_rdma_port
+#
+# Set rdma communication port.
+#
+# @port: rdma communication port.
+#
+# Returns: nothing on success
+#
+# Notes: Nothing.
+#
+# Since: 1.3.0
+##
+{ 'command': 'migrate_set_rdma_port', 'data': {'port': 'int'} }
+
+##
+# @migrate_set_rdma_host
+#
+# Set rdma communication host address.
+#
+# @host: rdma communication host address.
+#
+# Returns: nothing on success
+#
+# Notes: Nothing.
+#
+# Since: 1.3.0
+##
+{ 'command': 'migrate_set_rdma_host', 'data': {'host': 'str'} }
+
+##
 # @migrate-set-cache-size
 #
 # Set XBZRLE cache size
diff --git a/qemu-options.hx b/qemu-options.hx
index 4e2b499..dbd033b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2796,6 +2796,16 @@  CD-ROM drive and others. The @code{-nodefaults} option will disable all those
 default devices.
 ETEXI
 
+DEF("rdmaport", HAS_ARG, QEMU_OPTION_rdmaport, \
+    "-rdmaport p     port for RDMA migration\n", QEMU_ARCH_ALL)
+STEXI
+ETEXI
+
+DEF("rdmahost", HAS_ARG, QEMU_OPTION_rdmahost, \
+    "-rdmahost p     host name for RDMA migration\n", QEMU_ARCH_ALL)
+STEXI
+ETEXI
+
 #ifndef _WIN32
 DEF("chroot", HAS_ARG, QEMU_OPTION_chroot, \
     "-chroot dir     chroot to dir just before starting the VM\n",