All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode
@ 2016-08-10 18:02 marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 01/17] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO} marcandre.lureau
                   ` (17 more replies)
  0 siblings, 18 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Although some QMP commands are still not fully qapi'fied, it is
possible to use more qapi common and generated code by dropping the
'middle' mode and use qmp_dispatch().

v4:
- export all marshaller functions (so we can keep calling them after
  middle mode is removed), remove 'export-marshal' patch
- get rid of unnecessary lambda in python code (leftover), remove
  second mcgen(), and outdated comment
- remove disabled commands at run-time to avoid any regression. It's
  now on my TODO list to fix qapi generator in 2.8 to have
  conditionals
- move qmp-commands.txt to doc/
- split the last patch, remove trailing ws
- add QEMU_VERSION_{MAJOR,MINOR,MICRO} patch, simplifying
  qmp_query_version() (could be applied outside this series)
- update commit title/messages/order

v3:
- add a reference to docs/qmp-spec.txt in qmp_capabilities doc
- remove 'props' from device_add doc, improve example
- replace a g_strcmp0 with more appropriate g_str_equal
- add 'export-marshal' command generator key patch
- call qmp_marshal_query_version() directly (also get rid of the need
  to do a make clean, since the qapi json is modified)
- add patch to check invalid arguments on no-args (the old dispatch
  code checks that), and a test
- patch reordering to fix intermediate builds
- commit messages improvements
- split some misc doc fixes in last patch
- add some r-b and rebase

v2:
- rebased on master
- add Since: 0.13 to qmp_capabilities and device_add documentation
- fix device_add doc
- add missing spaces after ',' in get_qmp_greeting()
- fix some grammar in monitor.c while touching it

Marc-André Lureau (17):
  build-sys: define QEMU_VERSION_{MAJOR,MINOR,MICRO}
  qapi-schema: use generated marshaller for 'qmp_capabilities'
  qapi-schema: add 'device_add'
  monitor: simplify invalid_qmp_mode()
  monitor: register gen:false commands manually
  monitor: unregister conditional commands
  qapi: export the marshallers
  monitor: use qmp_find_command() (using generated qapi code)
  monitor: implement 'qmp_query_commands' without qmp_cmds
  monitor: remove mhandler.cmd_new
  qapi: remove the "middle" mode
  qapi: check invalid arguments on no-args commands
  qmp: update qmp_query_spice fallback
  monitor: use qmp_dispatch()
  build-sys: remove qmp-commands-old.h
  Replace qmp-commands.hx by doc/qmp-commands.txt
  qmp-commands.txt: fix some styling

 monitor.c                                |  430 +++-------
 qmp.c                                    |   32 +-
 tests/test-qmp-commands.c                |   15 +
 vl.c                                     |    1 +
 scripts/create_config                    |    6 +
 scripts/qapi-commands.py                 |   76 +-
 .gitignore                               |    1 -
 MAINTAINERS                              |    1 -
 Makefile                                 |    8 +-
 Makefile.target                          |    7 +-
 docs/qapi-code-gen.txt                   |    6 +-
 qmp-commands.hx => docs/qmp-commands.txt | 1275 +-----------------------------
 docs/writing-qmp-commands.txt            |   46 +-
 hmp-commands-info.hx                     |  118 +--
 hmp-commands.hx                          |  208 ++---
 qapi-schema.json                         |   61 ++
 trace-events                             |    1 -
 17 files changed, 425 insertions(+), 1867 deletions(-)
 rename qmp-commands.hx => docs/qmp-commands.txt (82%)

-- 
2.9.0

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 01/17] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO}
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 02/17] qapi-schema: use generated marshaller for 'qmp_capabilities' marcandre.lureau
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

There are better chances to find what went wrong at build time than a
later assert in qmp_query_version

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qmp.c                 | 16 +++-------------
 scripts/create_config |  6 ++++++
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/qmp.c b/qmp.c
index b6d531e..ebc3ff6 100644
--- a/qmp.c
+++ b/qmp.c
@@ -51,21 +51,11 @@ NameInfo *qmp_query_name(Error **errp)
 VersionInfo *qmp_query_version(Error **errp)
 {
     VersionInfo *info = g_new0(VersionInfo, 1);
-    const char *version = QEMU_VERSION;
-    const char *tmp;
-    int err;
 
     info->qemu = g_new0(VersionTriple, 1);
-    err = qemu_strtoll(version, &tmp, 10, &info->qemu->major);
-    assert(err == 0);
-    tmp++;
-
-    err = qemu_strtoll(tmp, &tmp, 10, &info->qemu->minor);
-    assert(err == 0);
-    tmp++;
-
-    err = qemu_strtoll(tmp, &tmp, 10, &info->qemu->micro);
-    assert(err == 0);
+    info->qemu->major = QEMU_VERSION_MAJOR;
+    info->qemu->minor = QEMU_VERSION_MINOR;
+    info->qemu->micro = QEMU_VERSION_MICRO;
     info->package = g_strdup(QEMU_PKGVERSION);
 
     return info;
diff --git a/scripts/create_config b/scripts/create_config
index 1dd6a35..e6929dd 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -7,7 +7,13 @@ while read line; do
 case $line in
  VERSION=*) # configuration
     version=${line#*=}
+    major=$(echo "$version" | cut -d. -f1)
+    minor=$(echo "$version" | cut -d. -f2)
+    micro=$(echo "$version" | cut -d. -f3)
     echo "#define QEMU_VERSION \"$version\""
+    echo "#define QEMU_VERSION_MAJOR $major"
+    echo "#define QEMU_VERSION_MINOR $minor"
+    echo "#define QEMU_VERSION_MICRO $micro"
     ;;
  qemu_*dir=*) # qemu-specific directory configuration
     name=${line%=*}
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 02/17] qapi-schema: use generated marshaller for 'qmp_capabilities'
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 01/17] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO} marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 03/17] qapi-schema: add 'device_add' marcandre.lureau
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

qapi'fy the 'qmp_capabilities' command, makes the command visible in
query-qmp-schema.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c        |  4 ++--
 qapi-schema.json | 21 +++++++++++++++++++++
 qmp-commands.hx  |  2 +-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 5c00373..991eaf7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -617,7 +617,7 @@ static void monitor_qapi_event_init(void)
     qmp_event_set_func_emit(monitor_qapi_event_queue);
 }
 
-static void qmp_capabilities(QDict *params, QObject **ret_data, Error **errp)
+void qmp_qmp_capabilities(Error **errp)
 {
     cur_mon->qmp.in_command_mode = true;
 }
@@ -3656,7 +3656,7 @@ static int monitor_can_read(void *opaque)
 static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd,
                              Error **errp)
 {
-    bool is_cap = cmd->mhandler.cmd_new == qmp_capabilities;
+    bool is_cap = cmd->mhandler.cmd_new == qmp_marshal_qmp_capabilities;
 
     if (is_cap && mon->qmp.in_command_mode) {
         error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
diff --git a/qapi-schema.json b/qapi-schema.json
index 5658723..553b02e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -20,6 +20,27 @@
 # QAPI introspection
 { 'include': 'qapi/introspect.json' }
 
+##
+# @qmp_capabilities:
+#
+# Enable QMP capabilities.
+#
+# Arguments: None.
+#
+# Example:
+#
+# -> { "execute": "qmp_capabilities" }
+# <- { "return": {} }
+#
+# Notes: This command is valid exactly when first connecting: it must be
+# issued before any other command will be accepted, and will fail once the
+# monitor is accepting other commands. (see qemu docs/qmp-spec.txt)
+#
+# Since: 0.13
+#
+##
+{ 'command': 'qmp_capabilities' }
+
 ##
 # @LostTickPolicy:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c8d360a..13707ac 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2209,7 +2209,7 @@ EQMP
         .args_type  = "",
         .params     = "",
         .help       = "enable QMP capabilities",
-        .mhandler.cmd_new = qmp_capabilities,
+        .mhandler.cmd_new = qmp_marshal_qmp_capabilities,
     },
 
 SQMP
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 03/17] qapi-schema: add 'device_add'
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 01/17] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO} marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 02/17] qapi-schema: use generated marshaller for 'qmp_capabilities' marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 04/17] monitor: simplify invalid_qmp_mode() marcandre.lureau
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Even though device_add is not fully qapi'fied, we may add it to the json
schema with 'gen': false, so registration and documentation can be
generated.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qapi-schema.json | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 553b02e..6e2550e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2200,6 +2200,46 @@
 ##
 { 'command': 'xen-set-global-dirty-log', 'data': { 'enable': 'bool' } }
 
+##
+# @device_add:
+#
+# @driver: the name of the new device's driver
+#
+# @bus: #optional the device's parent bus (device tree path)
+#
+# @id: the device's ID, must be unique
+#
+# Additional arguments depend on the type.
+#
+# Add a device.
+#
+# Notes:
+# 1. For detailed information about this command, please refer to the
+#    'docs/qdev-device-use.txt' file.
+#
+# 2. It's possible to list device properties by running QEMU with the
+#    "-device DEVICE,help" command-line argument, where DEVICE is the
+#    device's name
+#
+# Example:
+#
+# -> { "execute": "device_add",
+#      "arguments": { "driver": "e1000", "id": "net1",
+#                     "bus": "pci.0",
+#                     "mac": "52:54:00:12:34:56" } }
+# <- { "return": {} }
+#
+# TODO This command effectively bypasses QAPI completely due to its
+# "additional arguments" business.  It shouldn't have been added to
+# the schema in this form.  It should be qapified properly, or
+# replaced by a properly qapified command.
+#
+# Since: 0.13
+##
+{ 'command': 'device_add',
+  'data': {'driver': 'str', 'id': 'str'},
+  'gen': false } # so we can get the additional arguments
+
 ##
 # @device_del:
 #
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 04/17] monitor: simplify invalid_qmp_mode()
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (2 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 03/17] qapi-schema: add 'device_add' marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 05/17] monitor: register gen:false commands manually marcandre.lureau
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

handle_qmp_command() will switch to use qmp_dispatch().  It won't have a
pointer to the marshaller function anymore, but only the name of the
command to invoke. Simplify invalid_qmp_mode() so it can just be called
with the command name.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/monitor.c b/monitor.c
index 991eaf7..e156026 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3653,21 +3653,21 @@ static int monitor_can_read(void *opaque)
     return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd,
+static bool invalid_qmp_mode(const Monitor *mon, const char *cmd,
                              Error **errp)
 {
-    bool is_cap = cmd->mhandler.cmd_new == qmp_marshal_qmp_capabilities;
+    bool is_cap = g_str_equal(cmd, "qmp_capabilities");
 
     if (is_cap && mon->qmp.in_command_mode) {
         error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
                   "Capabilities negotiation is already complete, command "
-                  "'%s' ignored", cmd->name);
+                  "'%s' ignored", cmd);
         return true;
     }
     if (!is_cap && !mon->qmp.in_command_mode) {
         error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
                   "Expecting capabilities negotiation with "
-                  "'qmp_capabilities' before command '%s'", cmd->name);
+                  "'qmp_capabilities' before command '%s'", cmd);
         return true;
     }
     return false;
@@ -3958,7 +3958,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
                   "The command %s has not been found", cmd_name);
         goto err_out;
     }
-    if (invalid_qmp_mode(mon, cmd, &local_err)) {
+    if (invalid_qmp_mode(mon, cmd_name, &local_err)) {
         goto err_out;
     }
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 05/17] monitor: register gen:false commands manually
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (3 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 04/17] monitor: simplify invalid_qmp_mode() marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-16 12:27   ` Markus Armbruster
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands marcandre.lureau
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since a few commands are using 'gen': false, they are not registered
automatically by the generator. Register manually instead.

This is in preparation for removal of qapi 'middle' mode generation.

Note that qmp_init_marshal() function isn't run yet, so the commands
aren't registered, until module_call_init(MODULE_INIT_QAPI) is added in
a later patch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/monitor.c b/monitor.c
index e156026..b7ae552 100644
--- a/monitor.c
+++ b/monitor.c
@@ -79,6 +79,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/qtest.h"
 #include "qemu/cutils.h"
+#include "qapi/qmp/dispatch.h"
 
 /* for hmp_info_irq/pic */
 #if defined(TARGET_SPARC)
@@ -1007,6 +1008,18 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
     *ret_data = qobject_from_json(qmp_schema_json);
 }
 
+static void qmp_init_marshal(void)
+{
+    qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
+                         QCO_NO_OPTIONS);
+    qmp_register_command("device_add", qmp_device_add,
+                         QCO_NO_OPTIONS);
+    qmp_register_command("netdev_add", qmp_netdev_add,
+                         QCO_NO_OPTIONS);
+}
+
+qapi_init(qmp_init_marshal);
+
 /* set the current CPU defined by the user */
 int monitor_set_cpu(int cpu_index)
 {
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (4 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 05/17] monitor: register gen:false commands manually marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-16 14:26   ` Markus Armbruster
  2016-08-17 13:36   ` Markus Armbruster
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 07/17] qapi: export the marshallers marcandre.lureau
                   ` (11 subsequent siblings)
  17 siblings, 2 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The current monitor dispatch codes doesn't know commands that have been
filtered out during qmp-commands.hx preprocessing. query-commands
doesn't list them either. However, qapi generator doesn't filter them
out, and they are listed in the command list.

For now, disable the commands that aren't avaible to avoid introducing a
regression there when switching to qmp_dispatch() or listing commands
from the generated qapi code.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 monitor.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/monitor.c b/monitor.c
index b7ae552..ef946ad 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
     *ret_data = qobject_from_json(qmp_schema_json);
 }
 
+/*
+ * Those commands are registered unconditionnally by generated
+ * qmp files. FIXME: Educate the QAPI schema on #ifdef commands.
+ */
+static void qmp_disable_marshal(void)
+{
+#ifndef CONFIG_SPICE
+    qmp_disable_command("query-spice");
+#endif
+#ifndef TARGET_I386
+    qmp_disable_command("rtc-reset-reinjection");
+#endif
+#ifndef TARGET_S390X
+    qmp_disable_command("dump-skeys");
+#endif
+#ifndef TARGET_ARM
+    qmp_disable_command("query-gic-capabilities");
+#endif
+}
+
 static void qmp_init_marshal(void)
 {
     qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
@@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void)
                          QCO_NO_OPTIONS);
     qmp_register_command("netdev_add", qmp_netdev_add,
                          QCO_NO_OPTIONS);
+
+    /* call it after the rest of qapi_init() */
+    register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI);
 }
 
 qapi_init(qmp_init_marshal);
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 07/17] qapi: export the marshallers
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (5 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 08/17] monitor: use qmp_find_command() (using generated qapi code) marcandre.lureau
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Make it possible to call marshallers manually, without going through
qmp_dispatch(). (this is currently only possible in middle-mode, but
it's also useful in general)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi-commands.py | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index a06a2c4..b150db9 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -84,10 +84,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in, QObject **ret_out,
 
 
 def gen_marshal_proto(name):
-    ret = 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
-    if not middle_mode:
-        ret = 'static ' + ret
-    return ret
+    return 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
 
 
 def gen_marshal_decl(name):
@@ -222,8 +219,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
         if ret_type and ret_type not in self._visited_ret_types:
             self._visited_ret_types.add(ret_type)
             self.defn += gen_marshal_output(ret_type)
-        if middle_mode:
-            self.decl += gen_marshal_decl(name)
+        self.decl += gen_marshal_decl(name)
         self.defn += gen_marshal(name, arg_type, boxed, ret_type)
         if not middle_mode:
             self._regy += gen_register_command(name, success_response)
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 08/17] monitor: use qmp_find_command() (using generated qapi code)
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (6 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 07/17] qapi: export the marshallers marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds marcandre.lureau
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Stop using the so-called 'middle' mode. Instead, use qmp_find_command()
from generated qapi commands registry. Update and fix the documentation
too.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 monitor.c                     |  10 ++-
 vl.c                          |   1 +
 Makefile                      |   2 +-
 docs/writing-qmp-commands.txt |   8 +--
 qmp-commands.hx               | 143 ------------------------------------------
 5 files changed, 11 insertions(+), 153 deletions(-)

diff --git a/monitor.c b/monitor.c
index ef946ad..2050698 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3964,6 +3964,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
     QObject *obj, *data;
     QDict *input, *args;
     const mon_cmd_t *cmd;
+    QmpCommand *qcmd;
     const char *cmd_name;
     Monitor *mon = cur_mon;
 
@@ -3989,7 +3990,8 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
     cmd_name = qdict_get_str(input, "execute");
     trace_handle_qmp_command(mon, cmd_name);
     cmd = qmp_find_cmd(cmd_name);
-    if (!cmd) {
+    qcmd = qmp_find_command(cmd_name);
+    if (!qcmd || !cmd) {
         error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND,
                   "The command %s has not been found", cmd_name);
         goto err_out;
@@ -4011,7 +4013,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
         goto err_out;
     }
 
-    cmd->mhandler.cmd_new(args, &data, &local_err);
+    qcmd->fn(args, &data, &local_err);
 
 err_out:
     monitor_protocol_emitter(mon, data, local_err);
@@ -4083,7 +4085,9 @@ static QObject *get_qmp_greeting(void)
     QObject *ver = NULL;
 
     qmp_marshal_query_version(NULL, &ver, NULL);
-    return qobject_from_jsonf("{'QMP':{'version': %p,'capabilities': []}}",ver);
+
+    return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': []}}",
+                              ver);
 }
 
 static void monitor_qmp_event(void *opaque, int event)
diff --git a/vl.c b/vl.c
index c4eeaff..a396625 100644
--- a/vl.c
+++ b/vl.c
@@ -2972,6 +2972,7 @@ int main(int argc, char **argv, char **envp)
     qemu_init_exec_dir(argv[0]);
 
     module_call_init(MODULE_INIT_QOM);
+    module_call_init(MODULE_INIT_QAPI);
 
     qemu_add_opts(&qemu_drive_opts);
     qemu_add_drive_opts(&qemu_legacy_drive_opts);
diff --git a/Makefile b/Makefile
index 50b4b3a..d8a2891 100644
--- a/Makefile
+++ b/Makefile
@@ -312,7 +312,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-event.py $(qapi-py)
 qmp-commands.h qmp-marshal.c :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
-		$(gen-out-type) -o "." -m $<, \
+		$(gen-out-type) -o "." $<, \
 		"  GEN   $@")
 qmp-introspect.h qmp-introspect.c :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 59aa77a..0a66e0e 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -127,7 +127,6 @@ following at the bottom:
     {
         .name       = "hello-world",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_hello_world,
     },
 
 You're done. Now build qemu, run it as suggested in the "Testing" section,
@@ -179,7 +178,6 @@ The last step is to update the qmp-commands.hx file:
     {
         .name       = "hello-world",
         .args_type  = "message:s?",
-        .mhandler.cmd_new = qmp_marshal_hello_world,
     },
 
 Notice that the "args_type" member got our "message" argument. The character
@@ -454,12 +452,11 @@ There are a number of things to be noticed:
 6. You have to include the "qmp-commands.h" header file in qemu-timer.c,
    otherwise qemu won't build
 
-The last step is to add the correspoding entry in the qmp-commands.hx file:
+The last step is to add the corresponding entry in the qmp-commands.hx file:
 
     {
         .name       = "query-alarm-clock",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_alarm_clock,
     },
 
 Time to test the new command. Build qemu, run it as described in the "Testing"
@@ -518,7 +515,7 @@ in the monitor.c file. The entry for the "info alarmclock" follows:
         .args_type  = "",
         .params     = "",
         .help       = "show information about the alarm clock",
-        .mhandler.info = hmp_info_alarm_clock,
+        .mhandler.cmd = hmp_info_alarm_clock,
     },
 
 To test this, run qemu and type "info alarmclock" in the user monitor.
@@ -605,7 +602,6 @@ To test this you have to add the corresponding qmp-commands.hx entry:
     {
         .name       = "query-alarm-methods",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_alarm_methods,
     },
 
 Now Build qemu, run it as explained in the "Testing" section and try our new
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 13707ac..1ad8dda 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -63,7 +63,6 @@ EQMP
     {
         .name       = "quit",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_quit,
     },
 
 SQMP
@@ -84,7 +83,6 @@ EQMP
     {
         .name       = "eject",
         .args_type  = "force:-f,device:B",
-        .mhandler.cmd_new = qmp_marshal_eject,
     },
 
 SQMP
@@ -110,7 +108,6 @@ EQMP
     {
         .name       = "change",
         .args_type  = "device:B,target:F,arg:s?",
-        .mhandler.cmd_new = qmp_marshal_change,
     },
 
 SQMP
@@ -146,7 +143,6 @@ EQMP
     {
         .name       = "screendump",
         .args_type  = "filename:F",
-        .mhandler.cmd_new = qmp_marshal_screendump,
     },
 
 SQMP
@@ -169,7 +165,6 @@ EQMP
     {
         .name       = "stop",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_stop,
     },
 
 SQMP
@@ -190,7 +185,6 @@ EQMP
     {
         .name       = "cont",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_cont,
     },
 
 SQMP
@@ -211,7 +205,6 @@ EQMP
     {
         .name       = "system_wakeup",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_system_wakeup,
     },
 
 SQMP
@@ -232,7 +225,6 @@ EQMP
     {
         .name       = "system_reset",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_system_reset,
     },
 
 SQMP
@@ -253,7 +245,6 @@ EQMP
     {
         .name       = "system_powerdown",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_system_powerdown,
     },
 
 SQMP
@@ -276,7 +267,6 @@ EQMP
         .args_type  = "device:O",
         .params     = "driver[,prop=value][,...]",
         .help       = "add device, like -device on the command line",
-        .mhandler.cmd_new = qmp_device_add,
     },
 
 SQMP
@@ -310,7 +300,6 @@ EQMP
     {
         .name       = "device_del",
         .args_type  = "id:s",
-        .mhandler.cmd_new = qmp_marshal_device_del,
     },
 
 SQMP
@@ -338,7 +327,6 @@ EQMP
     {
         .name       = "send-key",
         .args_type  = "keys:q,hold-time:i?",
-        .mhandler.cmd_new = qmp_marshal_send_key,
     },
 
 SQMP
@@ -369,7 +357,6 @@ EQMP
     {
         .name       = "cpu",
         .args_type  = "index:i",
-        .mhandler.cmd_new = qmp_marshal_cpu,
     },
 
 SQMP
@@ -394,7 +381,6 @@ EQMP
     {
         .name       = "cpu-add",
         .args_type  = "id:i",
-        .mhandler.cmd_new = qmp_marshal_cpu_add,
     },
 
 SQMP
@@ -417,7 +403,6 @@ EQMP
     {
         .name       = "memsave",
         .args_type  = "val:l,size:i,filename:s,cpu:i?",
-        .mhandler.cmd_new = qmp_marshal_memsave,
     },
 
 SQMP
@@ -446,7 +431,6 @@ EQMP
     {
         .name       = "pmemsave",
         .args_type  = "val:l,size:i,filename:s",
-        .mhandler.cmd_new = qmp_marshal_pmemsave,
     },
 
 SQMP
@@ -474,7 +458,6 @@ EQMP
     {
         .name       = "inject-nmi",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_inject_nmi,
     },
 
 SQMP
@@ -497,7 +480,6 @@ EQMP
     {
         .name       = "ringbuf-write",
         .args_type  = "device:s,data:s,format:s?",
-        .mhandler.cmd_new = qmp_marshal_ringbuf_write,
     },
 
 SQMP
@@ -526,7 +508,6 @@ EQMP
     {
         .name       = "ringbuf-read",
         .args_type  = "device:s,size:i,format:s?",
-        .mhandler.cmd_new = qmp_marshal_ringbuf_read,
     },
 
 SQMP
@@ -562,7 +543,6 @@ EQMP
     {
         .name       = "xen-save-devices-state",
         .args_type  = "filename:F",
-    .mhandler.cmd_new = qmp_marshal_xen_save_devices_state,
     },
 
 SQMP
@@ -589,7 +569,6 @@ EQMP
     {
         .name       = "xen-load-devices-state",
         .args_type  = "filename:F",
-        .mhandler.cmd_new = qmp_marshal_xen_load_devices_state,
     },
 
 SQMP
@@ -616,7 +595,6 @@ EQMP
     {
         .name       = "xen-set-global-dirty-log",
         .args_type  = "enable:b",
-        .mhandler.cmd_new = qmp_marshal_xen_set_global_dirty_log,
     },
 
 SQMP
@@ -640,7 +618,6 @@ EQMP
     {
         .name       = "migrate",
         .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
-        .mhandler.cmd_new = qmp_marshal_migrate,
     },
 
 SQMP
@@ -673,7 +650,6 @@ EQMP
     {
         .name       = "migrate_cancel",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_migrate_cancel,
     },
 
 SQMP
@@ -694,7 +670,6 @@ EQMP
     {
         .name       = "migrate-incoming",
         .args_type  = "uri:s",
-        .mhandler.cmd_new = qmp_marshal_migrate_incoming,
     },
 
 SQMP
@@ -722,7 +697,6 @@ EQMP
     {
         .name       = "migrate-set-cache-size",
         .args_type  = "value:o",
-        .mhandler.cmd_new = qmp_marshal_migrate_set_cache_size,
     },
 
 SQMP
@@ -745,7 +719,6 @@ EQMP
     {
         .name       = "migrate-start-postcopy",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_migrate_start_postcopy,
     },
 
 SQMP
@@ -764,7 +737,6 @@ EQMP
     {
         .name       = "query-migrate-cache-size",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_migrate_cache_size,
     },
 
 SQMP
@@ -786,7 +758,6 @@ EQMP
     {
         .name       = "migrate_set_speed",
         .args_type  = "value:o",
-        .mhandler.cmd_new = qmp_marshal_migrate_set_speed,
     },
 
 SQMP
@@ -809,7 +780,6 @@ EQMP
     {
         .name       = "migrate_set_downtime",
         .args_type  = "value:T",
-        .mhandler.cmd_new = qmp_marshal_migrate_set_downtime,
     },
 
 SQMP
@@ -834,7 +804,6 @@ EQMP
         .args_type  = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
         .params     = "protocol hostname port tls-port cert-subject",
         .help       = "set migration information for remote display",
-        .mhandler.cmd_new = qmp_marshal_client_migrate_info,
     },
 
 SQMP
@@ -868,7 +837,6 @@ EQMP
         .args_type  = "paging:b,protocol:s,detach:b?,begin:i?,end:i?,format:s?",
         .params     = "-p protocol [-d] [begin] [length] [format]",
         .help       = "dump guest memory to file",
-        .mhandler.cmd_new = qmp_marshal_dump_guest_memory,
     },
 
 SQMP
@@ -907,7 +875,6 @@ EQMP
     {
         .name       = "query-dump-guest-memory-capability",
         .args_type  = "",
-    .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
     },
 
 SQMP
@@ -929,7 +896,6 @@ EQMP
         .args_type  = "",
         .params     = "",
         .help       = "query background dump status",
-        .mhandler.cmd_new = qmp_marshal_query_dump,
     },
 
 SQMP
@@ -952,7 +918,6 @@ EQMP
     {
         .name       = "dump-skeys",
         .args_type  = "filename:F",
-        .mhandler.cmd_new = qmp_marshal_dump_skeys,
     },
 #endif
 
@@ -976,7 +941,6 @@ EQMP
     {
         .name       = "netdev_add",
         .args_type  = "netdev:O",
-        .mhandler.cmd_new = qmp_netdev_add,
     },
 
 SQMP
@@ -1007,7 +971,6 @@ EQMP
     {
         .name       = "netdev_del",
         .args_type  = "id:s",
-        .mhandler.cmd_new = qmp_marshal_netdev_del,
     },
 
 SQMP
@@ -1031,7 +994,6 @@ EQMP
     {
         .name       = "object-add",
         .args_type  = "qom-type:s,id:s,props:q?",
-        .mhandler.cmd_new = qmp_marshal_object_add,
     },
 
 SQMP
@@ -1057,7 +1019,6 @@ EQMP
     {
         .name       = "object-del",
         .args_type  = "id:s",
-        .mhandler.cmd_new = qmp_marshal_object_del,
     },
 
 SQMP
@@ -1082,7 +1043,6 @@ EQMP
     {
         .name       = "block_resize",
         .args_type  = "device:s?,node-name:s?,size:o",
-        .mhandler.cmd_new = qmp_marshal_block_resize,
     },
 
 SQMP
@@ -1107,7 +1067,6 @@ EQMP
     {
         .name       = "block-stream",
         .args_type  = "job-id:s?,device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",
-        .mhandler.cmd_new = qmp_marshal_block_stream,
     },
 
 SQMP
@@ -1152,7 +1111,6 @@ EQMP
     {
         .name       = "block-commit",
         .args_type  = "job-id:s?,device:B,base:s?,top:s?,backing-file:s?,speed:o?",
-        .mhandler.cmd_new = qmp_marshal_block_commit,
     },
 
 SQMP
@@ -1218,7 +1176,6 @@ EQMP
         .name       = "drive-backup",
         .args_type  = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
                       "format:s?,bitmap:s?,on-source-error:s?,on-target-error:s?",
-        .mhandler.cmd_new = qmp_marshal_drive_backup,
     },
 
 SQMP
@@ -1274,7 +1231,6 @@ EQMP
         .name       = "blockdev-backup",
         .args_type  = "job-id:s?,sync:s,device:B,target:B,speed:i?,"
                       "on-source-error:s?,on-target-error:s?",
-        .mhandler.cmd_new = qmp_marshal_blockdev_backup,
     },
 
 SQMP
@@ -1316,33 +1272,27 @@ EQMP
     {
         .name       = "block-job-set-speed",
         .args_type  = "device:B,speed:o",
-        .mhandler.cmd_new = qmp_marshal_block_job_set_speed,
     },
 
     {
         .name       = "block-job-cancel",
         .args_type  = "device:B,force:b?",
-        .mhandler.cmd_new = qmp_marshal_block_job_cancel,
     },
     {
         .name       = "block-job-pause",
         .args_type  = "device:B",
-        .mhandler.cmd_new = qmp_marshal_block_job_pause,
     },
     {
         .name       = "block-job-resume",
         .args_type  = "device:B",
-        .mhandler.cmd_new = qmp_marshal_block_job_resume,
     },
     {
         .name       = "block-job-complete",
         .args_type  = "device:B",
-        .mhandler.cmd_new = qmp_marshal_block_job_complete,
     },
     {
         .name       = "transaction",
         .args_type  = "actions:q,properties:q?",
-        .mhandler.cmd_new = qmp_marshal_transaction,
     },
 
 SQMP
@@ -1436,7 +1386,6 @@ EQMP
     {
         .name       = "block-dirty-bitmap-add",
         .args_type  = "node:B,name:s,granularity:i?",
-        .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_add,
     },
 
 SQMP
@@ -1464,7 +1413,6 @@ EQMP
     {
         .name       = "block-dirty-bitmap-remove",
         .args_type  = "node:B,name:s",
-        .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_remove,
     },
 
 SQMP
@@ -1492,7 +1440,6 @@ EQMP
     {
         .name       = "block-dirty-bitmap-clear",
         .args_type  = "node:B,name:s",
-        .mhandler.cmd_new = qmp_marshal_block_dirty_bitmap_clear,
     },
 
 SQMP
@@ -1521,7 +1468,6 @@ EQMP
     {
         .name       = "blockdev-snapshot-sync",
         .args_type  = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
-        .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_sync,
     },
 
 SQMP
@@ -1557,7 +1503,6 @@ EQMP
     {
         .name       = "blockdev-snapshot",
         .args_type  = "node:s,overlay:s",
-        .mhandler.cmd_new = qmp_marshal_blockdev_snapshot,
     },
 
 SQMP
@@ -1595,7 +1540,6 @@ EQMP
     {
         .name       = "blockdev-snapshot-internal-sync",
         .args_type  = "device:B,name:s",
-        .mhandler.cmd_new = qmp_marshal_blockdev_snapshot_internal_sync,
     },
 
 SQMP
@@ -1624,8 +1568,6 @@ EQMP
     {
         .name       = "blockdev-snapshot-delete-internal-sync",
         .args_type  = "device:B,id:s?,name:s?",
-        .mhandler.cmd_new =
-                      qmp_marshal_blockdev_snapshot_delete_internal_sync,
     },
 
 SQMP
@@ -1669,7 +1611,6 @@ EQMP
                       "on-source-error:s?,on-target-error:s?,"
                       "unmap:b?,"
                       "granularity:i?,buf-size:i?",
-        .mhandler.cmd_new = qmp_marshal_drive_mirror,
     },
 
 SQMP
@@ -1733,7 +1674,6 @@ EQMP
         .args_type  = "job-id:s?,sync:s,device:B,target:B,replaces:s?,speed:i?,"
                       "on-source-error:s?,on-target-error:s?,"
                       "granularity:i?,buf-size:i?",
-        .mhandler.cmd_new = qmp_marshal_blockdev_mirror,
     },
 
 SQMP
@@ -1781,7 +1721,6 @@ EQMP
     {
         .name       = "change-backing-file",
         .args_type  = "device:s,image-node-name:s,backing-file:s",
-        .mhandler.cmd_new = qmp_marshal_change_backing_file,
     },
 
 SQMP
@@ -1820,7 +1759,6 @@ EQMP
     {
         .name       = "balloon",
         .args_type  = "value:M",
-        .mhandler.cmd_new = qmp_marshal_balloon,
     },
 
 SQMP
@@ -1843,7 +1781,6 @@ EQMP
     {
         .name       = "set_link",
         .args_type  = "name:s,up:b",
-        .mhandler.cmd_new = qmp_marshal_set_link,
     },
 
 SQMP
@@ -1869,7 +1806,6 @@ EQMP
         .args_type  = "fdname:s",
         .params     = "getfd name",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
-        .mhandler.cmd_new = qmp_marshal_getfd,
     },
 
 SQMP
@@ -1902,7 +1838,6 @@ EQMP
         .args_type  = "fdname:s",
         .params     = "closefd name",
         .help       = "close a file descriptor previously passed via SCM rights",
-        .mhandler.cmd_new = qmp_marshal_closefd,
     },
 
 SQMP
@@ -1927,7 +1862,6 @@ EQMP
         .args_type  = "fdset-id:i?,opaque:s?",
         .params     = "add-fd fdset-id opaque",
         .help       = "Add a file descriptor, that was passed via SCM rights, to an fd set",
-        .mhandler.cmd_new = qmp_marshal_add_fd,
     },
 
 SQMP
@@ -1966,7 +1900,6 @@ EQMP
         .args_type  = "fdset-id:i,fd:i?",
         .params     = "remove-fd fdset-id fd",
         .help       = "Remove a file descriptor from an fd set",
-        .mhandler.cmd_new = qmp_marshal_remove_fd,
     },
 
 SQMP
@@ -1998,7 +1931,6 @@ EQMP
         .name       = "query-fdsets",
         .args_type  = "",
         .help       = "Return information describing all fd sets",
-        .mhandler.cmd_new = qmp_marshal_query_fdsets,
     },
 
 SQMP
@@ -2047,7 +1979,6 @@ EQMP
     {
         .name       = "block_passwd",
         .args_type  = "device:s?,node-name:s?,password:s",
-        .mhandler.cmd_new = qmp_marshal_block_passwd,
     },
 
 SQMP
@@ -2073,7 +2004,6 @@ EQMP
     {
         .name       = "block_set_io_throttle",
         .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,bps_max_length:l?,bps_rd_max_length:l?,bps_wr_max_length:l?,iops_max_length:l?,iops_rd_max_length:l?,iops_wr_max_length:l?,iops_size:l?,group:s?",
-        .mhandler.cmd_new = qmp_marshal_block_set_io_throttle,
     },
 
 SQMP
@@ -2130,7 +2060,6 @@ EQMP
     {
         .name       = "set_password",
         .args_type  = "protocol:s,password:s,connected:s?",
-        .mhandler.cmd_new = qmp_marshal_set_password,
     },
 
 SQMP
@@ -2156,7 +2085,6 @@ EQMP
     {
         .name       = "expire_password",
         .args_type  = "protocol:s,time:s",
-        .mhandler.cmd_new = qmp_marshal_expire_password,
     },
 
 SQMP
@@ -2181,7 +2109,6 @@ EQMP
     {
         .name       = "add_client",
         .args_type  = "protocol:s,fdname:s,skipauth:b?,tls:b?",
-        .mhandler.cmd_new = qmp_marshal_add_client,
     },
 
 SQMP
@@ -2209,7 +2136,6 @@ EQMP
         .args_type  = "",
         .params     = "",
         .help       = "enable QMP capabilities",
-        .mhandler.cmd_new = qmp_marshal_qmp_capabilities,
     },
 
 SQMP
@@ -2232,7 +2158,6 @@ EQMP
     {
         .name       = "human-monitor-command",
         .args_type  = "command-line:s,cpu-index:i?",
-        .mhandler.cmd_new = qmp_marshal_human_monitor_command,
     },
 
 SQMP
@@ -2311,7 +2236,6 @@ EQMP
     {
         .name       = "query-version",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_version,
     },
 
 SQMP
@@ -2348,7 +2272,6 @@ EQMP
     {
         .name       = "query-commands",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_commands,
     },
 
 SQMP
@@ -2385,7 +2308,6 @@ EQMP
     {
         .name       = "query-events",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_events,
     },
 
 SQMP
@@ -2402,7 +2324,6 @@ EQMP
     {
         .name       = "query-qmp-schema",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_query_qmp_schema,
     },
 
 SQMP
@@ -2447,7 +2368,6 @@ EQMP
     {
         .name       = "query-chardev",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_chardev,
     },
 
 SQMP
@@ -2488,7 +2408,6 @@ EQMP
     {
         .name       = "query-chardev-backends",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_chardev_backends,
     },
 
 SQMP
@@ -2672,7 +2591,6 @@ EQMP
     {
         .name       = "query-block",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_block,
     },
 
 SQMP
@@ -2869,7 +2787,6 @@ EQMP
     {
         .name       = "query-blockstats",
         .args_type  = "query-nodes:b?",
-        .mhandler.cmd_new = qmp_marshal_query_blockstats,
     },
 
 SQMP
@@ -2924,7 +2841,6 @@ EQMP
     {
         .name       = "query-cpus",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_cpus,
     },
 
 SQMP
@@ -2963,7 +2879,6 @@ EQMP
     {
         .name       = "query-iothreads",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_iothreads,
     },
 
 SQMP
@@ -3180,7 +3095,6 @@ EQMP
     {
         .name       = "query-pci",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_pci,
     },
 
 SQMP
@@ -3204,7 +3118,6 @@ EQMP
     {
         .name       = "query-kvm",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_kvm,
     },
 
 SQMP
@@ -3244,7 +3157,6 @@ EQMP
     {
         .name       = "query-status",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_status,
     },
 
 SQMP
@@ -3288,7 +3200,6 @@ EQMP
     {
         .name       = "query-mice",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_mice,
     },
 
 SQMP
@@ -3351,12 +3262,10 @@ EQMP
     {
         .name       = "query-vnc",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_vnc,
     },
     {
         .name       = "query-vnc-servers",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_vnc_servers,
     },
 
 SQMP
@@ -3433,7 +3342,6 @@ EQMP
     {
         .name       = "query-spice",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_spice,
     },
 #endif
 
@@ -3457,7 +3365,6 @@ EQMP
     {
         .name       = "query-name",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_name,
     },
 
 SQMP
@@ -3480,7 +3387,6 @@ EQMP
     {
         .name       = "query-uuid",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_uuid,
     },
 
 SQMP
@@ -3529,7 +3435,6 @@ EQMP
     {
         .name       = "query-command-line-options",
         .args_type  = "option:s?",
-        .mhandler.cmd_new = qmp_marshal_query_command_line_options,
     },
 
 SQMP
@@ -3707,7 +3612,6 @@ EQMP
     {
         .name       = "query-migrate",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_migrate,
     },
 
 SQMP
@@ -3737,7 +3641,6 @@ EQMP
         .name       = "migrate-set-capabilities",
         .args_type  = "capabilities:q",
         .params     = "capability:s,state:b",
-        .mhandler.cmd_new = qmp_marshal_migrate_set_capabilities,
     },
 SQMP
 query-migrate-capabilities
@@ -3774,7 +3677,6 @@ EQMP
     {
         .name       = "query-migrate-capabilities",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_migrate_capabilities,
     },
 
 SQMP
@@ -3804,7 +3706,6 @@ EQMP
         .name       = "migrate-set-parameters",
         .args_type  =
             "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?",
-        .mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
     },
 SQMP
 query-migrate-parameters
@@ -3841,7 +3742,6 @@ EQMP
     {
         .name       = "query-migrate-parameters",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_migrate_parameters,
     },
 
 SQMP
@@ -3869,88 +3769,73 @@ EQMP
     {
         .name       = "query-balloon",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_balloon,
     },
 
     {
         .name       = "query-block-jobs",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_block_jobs,
     },
 
     {
         .name       = "qom-list",
         .args_type  = "path:s",
-        .mhandler.cmd_new = qmp_marshal_qom_list,
     },
 
     {
         .name       = "qom-set",
 	.args_type  = "path:s,property:s,value:q",
-        .mhandler.cmd_new = qmp_marshal_qom_set,
     },
 
     {
         .name       = "qom-get",
 	.args_type  = "path:s,property:s",
-        .mhandler.cmd_new = qmp_marshal_qom_get,
     },
 
     {
         .name       = "nbd-server-start",
         .args_type  = "addr:q,tls-creds:s?",
-        .mhandler.cmd_new = qmp_marshal_nbd_server_start,
     },
     {
         .name       = "nbd-server-add",
         .args_type  = "device:B,writable:b?",
-        .mhandler.cmd_new = qmp_marshal_nbd_server_add,
     },
     {
         .name       = "nbd-server-stop",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_nbd_server_stop,
     },
 
     {
         .name       = "change-vnc-password",
         .args_type  = "password:s",
-        .mhandler.cmd_new = qmp_marshal_change_vnc_password,
     },
     {
         .name       = "qom-list-types",
         .args_type  = "implements:s?,abstract:b?",
-        .mhandler.cmd_new = qmp_marshal_qom_list_types,
     },
 
     {
         .name       = "device-list-properties",
         .args_type  = "typename:s",
-        .mhandler.cmd_new = qmp_marshal_device_list_properties,
     },
 
     {
         .name       = "query-machines",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_machines,
     },
 
     {
         .name       = "query-cpu-definitions",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_cpu_definitions,
     },
 
     {
         .name       = "query-target",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_target,
     },
 
     {
         .name       = "query-tpm",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_tpm,
     },
 
 SQMP
@@ -3984,7 +3869,6 @@ EQMP
     {
         .name       = "query-tpm-models",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_tpm_models,
     },
 
 SQMP
@@ -4005,7 +3889,6 @@ EQMP
     {
         .name       = "query-tpm-types",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_tpm_types,
     },
 
 SQMP
@@ -4026,7 +3909,6 @@ EQMP
     {
         .name       = "chardev-add",
         .args_type  = "id:s,backend:q",
-        .mhandler.cmd_new = qmp_marshal_chardev_add,
     },
 
 SQMP
@@ -4063,7 +3945,6 @@ EQMP
     {
         .name       = "chardev-remove",
         .args_type  = "id:s",
-        .mhandler.cmd_new = qmp_marshal_chardev_remove,
     },
 
 
@@ -4086,7 +3967,6 @@ EQMP
     {
         .name       = "query-rx-filter",
         .args_type  = "name:s?",
-        .mhandler.cmd_new = qmp_marshal_query_rx_filter,
     },
 
 SQMP
@@ -4152,7 +4032,6 @@ EQMP
     {
         .name       = "blockdev-add",
         .args_type  = "options:q",
-        .mhandler.cmd_new = qmp_marshal_blockdev_add,
     },
 
 SQMP
@@ -4211,7 +4090,6 @@ EQMP
     {
         .name       = "x-blockdev-del",
         .args_type  = "id:s?,node-name:s?",
-        .mhandler.cmd_new = qmp_marshal_x_blockdev_del,
     },
 
 SQMP
@@ -4268,7 +4146,6 @@ EQMP
     {
         .name       = "blockdev-open-tray",
         .args_type  = "device:s,force:b?",
-        .mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
     },
 
 SQMP
@@ -4316,7 +4193,6 @@ EQMP
     {
         .name       = "blockdev-close-tray",
         .args_type  = "device:s",
-        .mhandler.cmd_new = qmp_marshal_blockdev_close_tray,
     },
 
 SQMP
@@ -4351,7 +4227,6 @@ EQMP
     {
         .name       = "x-blockdev-remove-medium",
         .args_type  = "device:s",
-        .mhandler.cmd_new = qmp_marshal_x_blockdev_remove_medium,
     },
 
 SQMP
@@ -4399,7 +4274,6 @@ EQMP
     {
         .name       = "x-blockdev-insert-medium",
         .args_type  = "device:s,node-name:s",
-        .mhandler.cmd_new = qmp_marshal_x_blockdev_insert_medium,
     },
 
 SQMP
@@ -4439,7 +4313,6 @@ EQMP
     {
         .name       = "x-blockdev-change",
         .args_type  = "parent:B,child:B?,node:B?",
-        .mhandler.cmd_new = qmp_marshal_x_blockdev_change,
     },
 
 SQMP
@@ -4492,7 +4365,6 @@ EQMP
     {
         .name       = "query-named-block-nodes",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
     },
 
 SQMP
@@ -4554,7 +4426,6 @@ EQMP
     {
         .name       = "blockdev-change-medium",
         .args_type  = "device:B,filename:F,format:s?,read-only-mode:s?",
-        .mhandler.cmd_new = qmp_marshal_blockdev_change_medium,
     },
 
 SQMP
@@ -4607,7 +4478,6 @@ EQMP
     {
         .name       = "query-memdev",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_memdev,
     },
 
 SQMP
@@ -4645,7 +4515,6 @@ EQMP
     {
         .name       = "query-memory-devices",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_memory_devices,
     },
 
 SQMP
@@ -4672,7 +4541,6 @@ EQMP
     {
         .name       = "query-acpi-ospm-status",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_acpi_ospm_status,
     },
 
 SQMP
@@ -4695,7 +4563,6 @@ EQMP
     {
         .name       = "rtc-reset-reinjection",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_rtc_reset_reinjection,
     },
 #endif
 
@@ -4716,7 +4583,6 @@ EQMP
     {
         .name       = "trace-event-get-state",
         .args_type  = "name:s,vcpu:i?",
-        .mhandler.cmd_new = qmp_marshal_trace_event_get_state,
     },
 
 SQMP
@@ -4748,7 +4614,6 @@ EQMP
     {
         .name       = "trace-event-set-state",
         .args_type  = "name:s,enable:b,ignore-unavailable:b?,vcpu:i?",
-        .mhandler.cmd_new = qmp_marshal_trace_event_set_state,
     },
 
 SQMP
@@ -4783,7 +4648,6 @@ EQMP
     {
         .name       = "input-send-event",
         .args_type  = "console:i?,events:q",
-        .mhandler.cmd_new = qmp_marshal_input_send_event,
     },
 
 SQMP
@@ -4849,7 +4713,6 @@ EQMP
     {
         .name       = "block-set-write-threshold",
         .args_type  = "node-name:s,write-threshold:l",
-        .mhandler.cmd_new = qmp_marshal_block_set_write_threshold,
     },
 
 SQMP
@@ -4877,7 +4740,6 @@ EQMP
     {
         .name       = "query-rocker",
         .args_type  = "name:s",
-        .mhandler.cmd_new = qmp_marshal_query_rocker,
     },
 
 SQMP
@@ -4898,7 +4760,6 @@ EQMP
     {
         .name       = "query-rocker-ports",
         .args_type  = "name:s",
-        .mhandler.cmd_new = qmp_marshal_query_rocker_ports,
     },
 
 SQMP
@@ -4923,7 +4784,6 @@ EQMP
     {
         .name       = "query-rocker-of-dpa-flows",
         .args_type  = "name:s,tbl-id:i?",
-        .mhandler.cmd_new = qmp_marshal_query_rocker_of_dpa_flows,
     },
 
 SQMP
@@ -4952,7 +4812,6 @@ EQMP
     {
         .name       = "query-rocker-of-dpa-groups",
         .args_type  = "name:s,type:i?",
-        .mhandler.cmd_new = qmp_marshal_query_rocker_of_dpa_groups,
     },
 
 SQMP
@@ -4983,7 +4842,6 @@ EQMP
     {
         .name       = "query-gic-capabilities",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_gic_capabilities,
     },
 #endif
 
@@ -5007,7 +4865,6 @@ EQMP
     {
         .name       = "query-hotpluggable-cpus",
         .args_type  = "",
-        .mhandler.cmd_new = qmp_marshal_query_hotpluggable_cpus,
     },
 
 SQMP
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (7 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 08/17] monitor: use qmp_find_command() (using generated qapi code) marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-16 16:22   ` Markus Armbruster
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 10/17] monitor: remove mhandler.cmd_new marcandre.lureau
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

One step towards getting rid of the static qmp_cmds table.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/monitor.c b/monitor.c
index 2050698..cddc737 100644
--- a/monitor.c
+++ b/monitor.c
@@ -957,21 +957,28 @@ static void hmp_info_help(Monitor *mon, const QDict *qdict)
     help_cmd(mon, "info");
 }
 
-CommandInfoList *qmp_query_commands(Error **errp)
+static void query_commands_cb(QmpCommand *cmd, void *opaque)
 {
-    CommandInfoList *info, *cmd_list = NULL;
-    const mon_cmd_t *cmd;
-
-    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
-        info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
-        info->value->name = g_strdup(cmd->name);
+    CommandInfoList *info, **list = opaque;
 
-        info->next = cmd_list;
-        cmd_list = info;
+    if (!cmd->enabled) {
+        return;
     }
 
-    return cmd_list;
+    info = g_malloc0(sizeof(*info));
+    info->value = g_malloc0(sizeof(*info->value));
+    info->value->name = g_strdup(cmd->name);
+    info->next = *list;
+    *list = info;
+}
+
+CommandInfoList *qmp_query_commands(Error **errp)
+{
+    CommandInfoList *list = NULL;
+
+    qmp_for_each_command(query_commands_cb, &list);
+
+    return list;
 }
 
 EventInfoList *qmp_query_events(Error **errp)
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 10/17] monitor: remove mhandler.cmd_new
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (8 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 11/17] qapi: remove the "middle" mode marcandre.lureau
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This is no longer necessary now that we aren't using middle mode
anymore.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c                     |  13 +--
 docs/writing-qmp-commands.txt |   4 +-
 hmp-commands-info.hx          | 118 ++++++++++++------------
 hmp-commands.hx               | 208 +++++++++++++++++++++---------------------
 4 files changed, 170 insertions(+), 173 deletions(-)

diff --git a/monitor.c b/monitor.c
index cddc737..ade11a5 100644
--- a/monitor.c
+++ b/monitor.c
@@ -130,13 +130,10 @@ typedef struct mon_cmd_t {
     const char *args_type;
     const char *params;
     const char *help;
-    union {
-        void (*cmd)(Monitor *mon, const QDict *qdict);
-        void (*cmd_new)(QDict *params, QObject **ret_data, Error **errp);
-    } mhandler;
-    /* @sub_table is a list of 2nd level of commands. If it do not exist,
-     * mhandler should be used. If it exist, sub_table[?].mhandler should be
-     * used, and mhandler of 1st level plays the role of help function.
+    void (*cmd)(Monitor *mon, const QDict *qdict);
+    /* @sub_table is a list of 2nd level of commands. If it does not exist,
+     * cmd should be used. If it exists, sub_table[?].cmd should be
+     * used, and cmd of 1st level plays the role of help function.
      */
     struct mon_cmd_t *sub_table;
     void (*command_completion)(ReadLineState *rs, int nb_args, const char *str);
@@ -2997,7 +2994,7 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
         return;
     }
 
-    cmd->mhandler.cmd(mon, qdict);
+    cmd->cmd(mon, qdict);
     QDECREF(qdict);
 }
 
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 0a66e0e..c425393 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -335,7 +335,7 @@ we should add it to the hmp-commands.hx file:
         .args_type  = "message:s?",
         .params     = "hello-world [message]",
         .help       = "Print message to the standard output",
-        .mhandler.cmd = hmp_hello_world,
+        .cmd        = hmp_hello_world,
     },
 
 STEXI
@@ -515,7 +515,7 @@ in the monitor.c file. The entry for the "info alarmclock" follows:
         .args_type  = "",
         .params     = "",
         .help       = "show information about the alarm clock",
-        .mhandler.cmd = hmp_info_alarm_clock,
+        .cmd        = hmp_info_alarm_clock,
     },
 
 To test this, run qemu and type "info alarmclock" in the user monitor.
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 74446c6..19729e5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -18,7 +18,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the version of QEMU",
-        .mhandler.cmd = hmp_info_version,
+        .cmd        = hmp_info_version,
     },
 
 STEXI
@@ -32,7 +32,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the network state",
-        .mhandler.cmd = hmp_info_network,
+        .cmd        = hmp_info_network,
     },
 
 STEXI
@@ -46,7 +46,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the character devices",
-        .mhandler.cmd = hmp_info_chardev,
+        .cmd        = hmp_info_chardev,
     },
 
 STEXI
@@ -61,7 +61,7 @@ ETEXI
         .params     = "[-n] [-v] [device]",
         .help       = "show info of one block device or all block devices "
                       "(-n: show named nodes; -v: show details)",
-        .mhandler.cmd = hmp_info_block,
+        .cmd        = hmp_info_block,
     },
 
 STEXI
@@ -75,7 +75,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show block device statistics",
-        .mhandler.cmd = hmp_info_blockstats,
+        .cmd        = hmp_info_blockstats,
     },
 
 STEXI
@@ -89,7 +89,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show progress of ongoing block device operations",
-        .mhandler.cmd = hmp_info_block_jobs,
+        .cmd        = hmp_info_block_jobs,
     },
 
 STEXI
@@ -103,7 +103,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the cpu registers",
-        .mhandler.cmd = hmp_info_registers,
+        .cmd        = hmp_info_registers,
     },
 
 STEXI
@@ -118,7 +118,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show local apic state",
-        .mhandler.cmd = hmp_info_local_apic,
+        .cmd        = hmp_info_local_apic,
     },
 #endif
 
@@ -134,7 +134,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show io apic state",
-        .mhandler.cmd = hmp_info_io_apic,
+        .cmd        = hmp_info_io_apic,
     },
 #endif
 
@@ -149,7 +149,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show infos for each CPU",
-        .mhandler.cmd = hmp_info_cpus,
+        .cmd        = hmp_info_cpus,
     },
 
 STEXI
@@ -163,7 +163,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the command line history",
-        .mhandler.cmd = hmp_info_history,
+        .cmd        = hmp_info_history,
     },
 
 STEXI
@@ -180,11 +180,11 @@ ETEXI
         .params     = "",
         .help       = "show the interrupts statistics (if available)",
 #ifdef TARGET_SPARC
-        .mhandler.cmd = sun4m_hmp_info_irq,
+        .cmd        = sun4m_hmp_info_irq,
 #elif defined(TARGET_LM32)
-        .mhandler.cmd = lm32_hmp_info_irq,
+        .cmd        = lm32_hmp_info_irq,
 #else
-        .mhandler.cmd = hmp_info_irq,
+        .cmd        = hmp_info_irq,
 #endif
     },
 
@@ -200,11 +200,11 @@ ETEXI
         .params     = "",
         .help       = "show i8259 (PIC) state",
 #ifdef TARGET_SPARC
-        .mhandler.cmd = sun4m_hmp_info_pic,
+        .cmd        = sun4m_hmp_info_pic,
 #elif defined(TARGET_LM32)
-        .mhandler.cmd = lm32_hmp_info_pic,
+        .cmd        = lm32_hmp_info_pic,
 #else
-        .mhandler.cmd = hmp_info_pic,
+        .cmd        = hmp_info_pic,
 #endif
     },
 #endif
@@ -220,7 +220,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show PCI info",
-        .mhandler.cmd = hmp_info_pci,
+        .cmd        = hmp_info_pci,
     },
 
 STEXI
@@ -236,7 +236,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show virtual to physical memory mappings",
-        .mhandler.cmd = hmp_info_tlb,
+        .cmd        = hmp_info_tlb,
     },
 #endif
 
@@ -252,7 +252,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the active virtual memory mappings",
-        .mhandler.cmd = hmp_info_mem,
+        .cmd        = hmp_info_mem,
     },
 #endif
 
@@ -267,7 +267,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show memory tree",
-        .mhandler.cmd = hmp_info_mtree,
+        .cmd        = hmp_info_mtree,
     },
 
 STEXI
@@ -281,7 +281,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show dynamic compiler info",
-        .mhandler.cmd = hmp_info_jit,
+        .cmd        = hmp_info_jit,
     },
 
 STEXI
@@ -295,7 +295,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show dynamic compiler opcode counters",
-        .mhandler.cmd = hmp_info_opcount,
+        .cmd        = hmp_info_opcount,
     },
 
 STEXI
@@ -309,7 +309,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show KVM information",
-        .mhandler.cmd = hmp_info_kvm,
+        .cmd        = hmp_info_kvm,
     },
 
 STEXI
@@ -323,7 +323,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show NUMA information",
-        .mhandler.cmd = hmp_info_numa,
+        .cmd        = hmp_info_numa,
     },
 
 STEXI
@@ -337,7 +337,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show guest USB devices",
-        .mhandler.cmd = hmp_info_usb,
+        .cmd        = hmp_info_usb,
     },
 
 STEXI
@@ -351,7 +351,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show host USB devices",
-        .mhandler.cmd = hmp_info_usbhost,
+        .cmd        = hmp_info_usbhost,
     },
 
 STEXI
@@ -365,7 +365,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show profiling information",
-        .mhandler.cmd = hmp_info_profile,
+        .cmd        = hmp_info_profile,
     },
 
 STEXI
@@ -379,7 +379,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show capture information",
-        .mhandler.cmd = hmp_info_capture,
+        .cmd        = hmp_info_capture,
     },
 
 STEXI
@@ -393,7 +393,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the currently saved VM snapshots",
-        .mhandler.cmd = hmp_info_snapshots,
+        .cmd        = hmp_info_snapshots,
     },
 
 STEXI
@@ -407,7 +407,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM status (running|paused)",
-        .mhandler.cmd = hmp_info_status,
+        .cmd        = hmp_info_status,
     },
 
 STEXI
@@ -421,7 +421,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show which guest mouse is receiving events",
-        .mhandler.cmd = hmp_info_mice,
+        .cmd        = hmp_info_mice,
     },
 
 STEXI
@@ -435,7 +435,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the vnc server status",
-        .mhandler.cmd = hmp_info_vnc,
+        .cmd        = hmp_info_vnc,
     },
 
 STEXI
@@ -450,7 +450,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the spice server status",
-        .mhandler.cmd = hmp_info_spice,
+        .cmd        = hmp_info_spice,
     },
 #endif
 
@@ -465,7 +465,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM name",
-        .mhandler.cmd = hmp_info_name,
+        .cmd        = hmp_info_name,
     },
 
 STEXI
@@ -479,7 +479,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM UUID",
-        .mhandler.cmd = hmp_info_uuid,
+        .cmd        = hmp_info_uuid,
     },
 
 STEXI
@@ -493,7 +493,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show CPU statistics",
-        .mhandler.cmd = hmp_info_cpustats,
+        .cmd        = hmp_info_cpustats,
     },
 
 STEXI
@@ -508,7 +508,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show user network stack connection states",
-        .mhandler.cmd = hmp_info_usernet,
+        .cmd        = hmp_info_usernet,
     },
 #endif
 
@@ -523,7 +523,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show migration status",
-        .mhandler.cmd = hmp_info_migrate,
+        .cmd        = hmp_info_migrate,
     },
 
 STEXI
@@ -537,7 +537,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show current migration capabilities",
-        .mhandler.cmd = hmp_info_migrate_capabilities,
+        .cmd        = hmp_info_migrate_capabilities,
     },
 
 STEXI
@@ -551,7 +551,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show current migration parameters",
-        .mhandler.cmd = hmp_info_migrate_parameters,
+        .cmd        = hmp_info_migrate_parameters,
     },
 
 STEXI
@@ -565,7 +565,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show current migration xbzrle cache size",
-        .mhandler.cmd = hmp_info_migrate_cache_size,
+        .cmd        = hmp_info_migrate_cache_size,
     },
 
 STEXI
@@ -579,7 +579,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show balloon information",
-        .mhandler.cmd = hmp_info_balloon,
+        .cmd        = hmp_info_balloon,
     },
 
 STEXI
@@ -593,7 +593,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show device tree",
-        .mhandler.cmd = hmp_info_qtree,
+        .cmd        = hmp_info_qtree,
     },
 
 STEXI
@@ -607,7 +607,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show qdev device model list",
-        .mhandler.cmd = hmp_info_qdm,
+        .cmd        = hmp_info_qdm,
     },
 
 STEXI
@@ -621,7 +621,7 @@ ETEXI
         .args_type  = "path:s?",
         .params     = "[path]",
         .help       = "show QOM composition tree",
-        .mhandler.cmd = hmp_info_qom_tree,
+        .cmd        = hmp_info_qom_tree,
     },
 
 STEXI
@@ -635,7 +635,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show roms",
-        .mhandler.cmd = hmp_info_roms,
+        .cmd        = hmp_info_roms,
     },
 
 STEXI
@@ -650,7 +650,7 @@ ETEXI
         .params     = "[name] [vcpu]",
         .help       = "show available trace-events & their state "
                       "(name: event name pattern; vcpu: vCPU to query, default is any)",
-        .mhandler.cmd = hmp_info_trace_events,
+        .cmd = hmp_info_trace_events,
         .command_completion = info_trace_events_completion,
     },
 
@@ -665,7 +665,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show the TPM device",
-        .mhandler.cmd = hmp_info_tpm,
+        .cmd        = hmp_info_tpm,
     },
 
 STEXI
@@ -679,7 +679,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show memory backends",
-        .mhandler.cmd = hmp_info_memdev,
+        .cmd        = hmp_info_memdev,
     },
 
 STEXI
@@ -693,7 +693,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show memory devices",
-        .mhandler.cmd = hmp_info_memory_devices,
+        .cmd        = hmp_info_memory_devices,
     },
 
 STEXI
@@ -707,7 +707,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "show iothreads",
-        .mhandler.cmd = hmp_info_iothreads,
+        .cmd        = hmp_info_iothreads,
     },
 
 STEXI
@@ -721,7 +721,7 @@ ETEXI
         .args_type  = "name:s",
         .params     = "name",
         .help       = "Show rocker switch",
-        .mhandler.cmd = hmp_rocker,
+        .cmd        = hmp_rocker,
     },
 
 STEXI
@@ -735,7 +735,7 @@ ETEXI
         .args_type  = "name:s",
         .params     = "name",
         .help       = "Show rocker ports",
-        .mhandler.cmd = hmp_rocker_ports,
+        .cmd        = hmp_rocker_ports,
     },
 
 STEXI
@@ -749,7 +749,7 @@ ETEXI
         .args_type  = "name:s,tbl_id:i?",
         .params     = "name [tbl_id]",
         .help       = "Show rocker OF-DPA flow tables",
-        .mhandler.cmd = hmp_rocker_of_dpa_flows,
+        .cmd        = hmp_rocker_of_dpa_flows,
     },
 
 STEXI
@@ -763,7 +763,7 @@ ETEXI
         .args_type  = "name:s,type:i?",
         .params     = "name [type]",
         .help       = "Show rocker OF-DPA groups",
-        .mhandler.cmd = hmp_rocker_of_dpa_groups,
+        .cmd        = hmp_rocker_of_dpa_groups,
     },
 
 STEXI
@@ -778,7 +778,7 @@ ETEXI
         .args_type  = "addr:l",
         .params     = "address",
         .help       = "Display the value of a storage key",
-        .mhandler.cmd = hmp_info_skeys,
+        .cmd        = hmp_info_skeys,
     },
 #endif
 
@@ -793,7 +793,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "Display the latest dump status",
-        .mhandler.cmd = hmp_info_dump,
+        .cmd        = hmp_info_dump,
     },
 
 STEXI
@@ -807,7 +807,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "Show information about hotpluggable CPUs",
-        .mhandler.cmd = hmp_hotpluggable_cpus,
+        .cmd        = hmp_hotpluggable_cpus,
     },
 
 STEXI
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 848efee..6b2ae30 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -14,7 +14,7 @@ ETEXI
         .args_type  = "name:S?",
         .params     = "[cmd]",
         .help       = "show the help",
-        .mhandler.cmd = do_help_cmd,
+        .cmd        = do_help_cmd,
     },
 
 STEXI
@@ -28,7 +28,7 @@ ETEXI
         .args_type  = "device:B",
         .params     = "device|all",
         .help       = "commit changes to the disk images (if -snapshot is used) or backing files",
-        .mhandler.cmd = hmp_commit,
+        .cmd        = hmp_commit,
     },
 
 STEXI
@@ -47,7 +47,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "quit the emulator",
-        .mhandler.cmd = hmp_quit,
+        .cmd        = hmp_quit,
     },
 
 STEXI
@@ -61,7 +61,7 @@ ETEXI
         .args_type  = "device:B,size:o",
         .params     = "device size",
         .help       = "resize a block image",
-        .mhandler.cmd = hmp_block_resize,
+        .cmd        = hmp_block_resize,
     },
 
 STEXI
@@ -78,7 +78,7 @@ ETEXI
         .args_type  = "device:B,speed:o?,base:s?",
         .params     = "device [speed [base]]",
         .help       = "copy data from a backing file into a block device",
-        .mhandler.cmd = hmp_block_stream,
+        .cmd        = hmp_block_stream,
     },
 
 STEXI
@@ -92,7 +92,7 @@ ETEXI
         .args_type  = "device:B,speed:o",
         .params     = "device speed",
         .help       = "set maximum speed for a background block operation",
-        .mhandler.cmd = hmp_block_job_set_speed,
+        .cmd        = hmp_block_job_set_speed,
     },
 
 STEXI
@@ -107,7 +107,7 @@ ETEXI
         .params     = "[-f] device",
         .help       = "stop an active background block operation (use -f"
                       "\n\t\t\t if the operation is currently paused)",
-        .mhandler.cmd = hmp_block_job_cancel,
+        .cmd        = hmp_block_job_cancel,
     },
 
 STEXI
@@ -121,7 +121,7 @@ ETEXI
         .args_type  = "device:B",
         .params     = "device",
         .help       = "stop an active background block operation",
-        .mhandler.cmd = hmp_block_job_complete,
+        .cmd        = hmp_block_job_complete,
     },
 
 STEXI
@@ -136,7 +136,7 @@ ETEXI
         .args_type  = "device:B",
         .params     = "device",
         .help       = "pause an active background block operation",
-        .mhandler.cmd = hmp_block_job_pause,
+        .cmd        = hmp_block_job_pause,
     },
 
 STEXI
@@ -150,7 +150,7 @@ ETEXI
         .args_type  = "device:B",
         .params     = "device",
         .help       = "resume a paused background block operation",
-        .mhandler.cmd = hmp_block_job_resume,
+        .cmd        = hmp_block_job_resume,
     },
 
 STEXI
@@ -164,7 +164,7 @@ ETEXI
         .args_type  = "force:-f,device:B",
         .params     = "[-f] device",
         .help       = "eject a removable medium (use -f to force it)",
-        .mhandler.cmd = hmp_eject,
+        .cmd        = hmp_eject,
     },
 
 STEXI
@@ -178,7 +178,7 @@ ETEXI
         .args_type  = "id:B",
         .params     = "device",
         .help       = "remove host block device",
-        .mhandler.cmd = hmp_drive_del,
+        .cmd        = hmp_drive_del,
     },
 
 STEXI
@@ -197,7 +197,7 @@ ETEXI
         .args_type  = "device:B,target:F,arg:s?,read-only-mode:s?",
         .params     = "device filename [format [read-only-mode]]",
         .help       = "change a removable medium, optional format",
-        .mhandler.cmd = hmp_change,
+        .cmd        = hmp_change,
     },
 
 STEXI
@@ -256,7 +256,7 @@ ETEXI
         .args_type  = "filename:F",
         .params     = "filename",
         .help       = "save screen into PPM image 'filename'",
-        .mhandler.cmd = hmp_screendump,
+        .cmd        = hmp_screendump,
     },
 
 STEXI
@@ -270,7 +270,7 @@ ETEXI
         .args_type  = "filename:F",
         .params     = "filename",
         .help       = "output logs to 'filename'",
-        .mhandler.cmd = hmp_logfile,
+        .cmd        = hmp_logfile,
     },
 
 STEXI
@@ -285,7 +285,7 @@ ETEXI
         .params     = "name on|off [vcpu]",
         .help       = "changes status of a specific trace event "
                       "(vcpu: vCPU to set, default is all)",
-        .mhandler.cmd = hmp_trace_event,
+        .cmd = hmp_trace_event,
         .command_completion = trace_event_completion,
     },
 
@@ -301,7 +301,7 @@ ETEXI
         .args_type  = "op:s?,arg:F?",
         .params     = "on|off|flush|set [arg]",
         .help       = "open, close, or flush trace file, or set a new file name",
-        .mhandler.cmd = hmp_trace_file,
+        .cmd        = hmp_trace_file,
     },
 
 STEXI
@@ -316,7 +316,7 @@ ETEXI
         .args_type  = "items:s",
         .params     = "item1[,...]",
         .help       = "activate logging of the specified items",
-        .mhandler.cmd = hmp_log,
+        .cmd        = hmp_log,
     },
 
 STEXI
@@ -330,7 +330,7 @@ ETEXI
         .args_type  = "name:s?",
         .params     = "[tag|id]",
         .help       = "save a VM snapshot. If no tag or id are provided, a new snapshot is created",
-        .mhandler.cmd = hmp_savevm,
+        .cmd        = hmp_savevm,
     },
 
 STEXI
@@ -347,7 +347,7 @@ ETEXI
         .args_type  = "name:s",
         .params     = "tag|id",
         .help       = "restore a VM snapshot from its tag or id",
-        .mhandler.cmd = hmp_loadvm,
+        .cmd        = hmp_loadvm,
         .command_completion = loadvm_completion,
     },
 
@@ -363,7 +363,7 @@ ETEXI
         .args_type  = "name:s",
         .params     = "tag|id",
         .help       = "delete a VM snapshot from its tag or id",
-        .mhandler.cmd = hmp_delvm,
+        .cmd        = hmp_delvm,
         .command_completion = delvm_completion,
     },
 
@@ -378,7 +378,7 @@ ETEXI
         .args_type  = "option:s?",
         .params     = "[on|off]",
         .help       = "run emulation in singlestep mode or switch to normal mode",
-        .mhandler.cmd = hmp_singlestep,
+        .cmd        = hmp_singlestep,
     },
 
 STEXI
@@ -393,7 +393,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "stop emulation",
-        .mhandler.cmd = hmp_stop,
+        .cmd        = hmp_stop,
     },
 
 STEXI
@@ -407,7 +407,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "resume emulation",
-        .mhandler.cmd = hmp_cont,
+        .cmd        = hmp_cont,
     },
 
 STEXI
@@ -421,7 +421,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "wakeup guest from suspend",
-        .mhandler.cmd = hmp_system_wakeup,
+        .cmd        = hmp_system_wakeup,
     },
 
 STEXI
@@ -435,7 +435,7 @@ ETEXI
         .args_type  = "device:s?",
         .params     = "[device]",
         .help       = "start gdbserver on given device (default 'tcp::1234'), stop with 'none'",
-        .mhandler.cmd = hmp_gdbserver,
+        .cmd        = hmp_gdbserver,
     },
 
 STEXI
@@ -449,7 +449,7 @@ ETEXI
         .args_type  = "fmt:/,addr:l",
         .params     = "/fmt addr",
         .help       = "virtual memory dump starting at 'addr'",
-        .mhandler.cmd = hmp_memory_dump,
+        .cmd        = hmp_memory_dump,
     },
 
 STEXI
@@ -463,7 +463,7 @@ ETEXI
         .args_type  = "fmt:/,addr:l",
         .params     = "/fmt addr",
         .help       = "physical memory dump starting at 'addr'",
-        .mhandler.cmd = hmp_physical_memory_dump,
+        .cmd        = hmp_physical_memory_dump,
     },
 
 STEXI
@@ -530,7 +530,7 @@ ETEXI
         .args_type  = "fmt:/,val:l",
         .params     = "/fmt expr",
         .help       = "print expression value (use $reg for CPU register access)",
-        .mhandler.cmd = do_print,
+        .cmd        = do_print,
     },
 
 STEXI
@@ -545,7 +545,7 @@ ETEXI
         .args_type  = "fmt:/,addr:i,index:i.",
         .params     = "/fmt addr",
         .help       = "I/O port read",
-        .mhandler.cmd = hmp_ioport_read,
+        .cmd        = hmp_ioport_read,
     },
 
 STEXI
@@ -559,7 +559,7 @@ ETEXI
         .args_type  = "fmt:/,addr:i,val:i",
         .params     = "/fmt addr value",
         .help       = "I/O port write",
-        .mhandler.cmd = hmp_ioport_write,
+        .cmd        = hmp_ioport_write,
     },
 
 STEXI
@@ -573,7 +573,7 @@ ETEXI
         .args_type  = "keys:s,hold-time:i?",
         .params     = "keys [hold_ms]",
         .help       = "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)",
-        .mhandler.cmd = hmp_sendkey,
+        .cmd        = hmp_sendkey,
         .command_completion = sendkey_completion,
     },
 
@@ -596,7 +596,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "reset the system",
-        .mhandler.cmd = hmp_system_reset,
+        .cmd        = hmp_system_reset,
     },
 
 STEXI
@@ -610,7 +610,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "send system power down event",
-        .mhandler.cmd = hmp_system_powerdown,
+        .cmd        = hmp_system_powerdown,
     },
 
 STEXI
@@ -624,7 +624,7 @@ ETEXI
         .args_type  = "start:i,size:i",
         .params     = "addr size",
         .help       = "compute the checksum of a memory region",
-        .mhandler.cmd = hmp_sum,
+        .cmd        = hmp_sum,
     },
 
 STEXI
@@ -638,7 +638,7 @@ ETEXI
         .args_type  = "devname:s",
         .params     = "device",
         .help       = "add USB device (e.g. 'host:bus.addr' or 'host:vendor_id:product_id')",
-        .mhandler.cmd = hmp_usb_add,
+        .cmd        = hmp_usb_add,
     },
 
 STEXI
@@ -653,7 +653,7 @@ ETEXI
         .args_type  = "devname:s",
         .params     = "device",
         .help       = "remove USB device 'bus.addr'",
-        .mhandler.cmd = hmp_usb_del,
+        .cmd        = hmp_usb_del,
     },
 
 STEXI
@@ -669,7 +669,7 @@ ETEXI
         .args_type  = "device:O",
         .params     = "driver[,prop=value][,...]",
         .help       = "add device, like -device on the command line",
-        .mhandler.cmd = hmp_device_add,
+        .cmd        = hmp_device_add,
         .command_completion = device_add_completion,
     },
 
@@ -684,7 +684,7 @@ ETEXI
         .args_type  = "id:s",
         .params     = "device",
         .help       = "remove device",
-        .mhandler.cmd = hmp_device_del,
+        .cmd        = hmp_device_del,
         .command_completion = device_del_completion,
     },
 
@@ -700,7 +700,7 @@ ETEXI
         .args_type  = "index:i",
         .params     = "index",
         .help       = "set the default CPU",
-        .mhandler.cmd = hmp_cpu,
+        .cmd        = hmp_cpu,
     },
 
 STEXI
@@ -714,7 +714,7 @@ ETEXI
         .args_type  = "dx_str:s,dy_str:s,dz_str:s?",
         .params     = "dx dy [dz]",
         .help       = "send mouse move events",
-        .mhandler.cmd = hmp_mouse_move,
+        .cmd        = hmp_mouse_move,
     },
 
 STEXI
@@ -729,7 +729,7 @@ ETEXI
         .args_type  = "button_state:i",
         .params     = "state",
         .help       = "change mouse button state (1=L, 2=M, 4=R)",
-        .mhandler.cmd = hmp_mouse_button,
+        .cmd        = hmp_mouse_button,
     },
 
 STEXI
@@ -743,7 +743,7 @@ ETEXI
         .args_type  = "index:i",
         .params     = "index",
         .help       = "set which mouse device receives events",
-        .mhandler.cmd = hmp_mouse_set,
+        .cmd        = hmp_mouse_set,
     },
 
 STEXI
@@ -761,7 +761,7 @@ ETEXI
         .args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
         .params     = "path [frequency [bits [channels]]]",
         .help       = "capture audio to a wave file (default frequency=44100 bits=16 channels=2)",
-        .mhandler.cmd = hmp_wavcapture,
+        .cmd        = hmp_wavcapture,
     },
 STEXI
 @item wavcapture @var{filename} [@var{frequency} [@var{bits} [@var{channels}]]]
@@ -782,7 +782,7 @@ ETEXI
         .args_type  = "n:i",
         .params     = "capture index",
         .help       = "stop capture",
-        .mhandler.cmd = hmp_stopcapture,
+        .cmd        = hmp_stopcapture,
     },
 STEXI
 @item stopcapture @var{index}
@@ -798,7 +798,7 @@ ETEXI
         .args_type  = "val:l,size:i,filename:s",
         .params     = "addr size file",
         .help       = "save to disk virtual memory dump starting at 'addr' of size 'size'",
-        .mhandler.cmd = hmp_memsave,
+        .cmd        = hmp_memsave,
     },
 
 STEXI
@@ -812,7 +812,7 @@ ETEXI
         .args_type  = "val:l,size:i,filename:s",
         .params     = "addr size file",
         .help       = "save to disk physical memory dump starting at 'addr' of size 'size'",
-        .mhandler.cmd = hmp_pmemsave,
+        .cmd        = hmp_pmemsave,
     },
 
 STEXI
@@ -826,7 +826,7 @@ ETEXI
         .args_type  = "bootdevice:s",
         .params     = "bootdevice",
         .help       = "define new values for the boot device list",
-        .mhandler.cmd = hmp_boot_set,
+        .cmd        = hmp_boot_set,
     },
 
 STEXI
@@ -844,7 +844,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "inject an NMI",
-        .mhandler.cmd = hmp_nmi,
+        .cmd        = hmp_nmi,
     },
 STEXI
 @item nmi @var{cpu}
@@ -858,7 +858,7 @@ ETEXI
         .args_type  = "device:s,data:s",
         .params     = "device data",
         .help       = "Write to a ring buffer character device",
-        .mhandler.cmd = hmp_ringbuf_write,
+        .cmd        = hmp_ringbuf_write,
         .command_completion = ringbuf_write_completion,
     },
 
@@ -875,7 +875,7 @@ ETEXI
         .args_type  = "device:s,size:i",
         .params     = "device size",
         .help       = "Read from a ring buffer character device",
-        .mhandler.cmd = hmp_ringbuf_read,
+        .cmd        = hmp_ringbuf_read,
         .command_completion = ringbuf_write_completion,
     },
 
@@ -901,7 +901,7 @@ ETEXI
 		      " full copy of disk\n\t\t\t -i for migration without "
 		      "shared storage with incremental copy of disk "
 		      "(base image shared between src and destination)",
-        .mhandler.cmd = hmp_migrate,
+        .cmd        = hmp_migrate,
     },
 
 
@@ -918,7 +918,7 @@ ETEXI
         .args_type  = "",
         .params     = "",
         .help       = "cancel the current VM migration",
-        .mhandler.cmd = hmp_migrate_cancel,
+        .cmd        = hmp_migrate_cancel,
     },
 
 STEXI
@@ -933,7 +933,7 @@ ETEXI
         .args_type  = "uri:s",
         .params     = "uri",
         .help       = "Continue an incoming migration from an -incoming defer",
-        .mhandler.cmd = hmp_migrate_incoming,
+        .cmd        = hmp_migrate_incoming,
     },
 
 STEXI
@@ -954,7 +954,7 @@ ETEXI
                       "The cache size affects the number of cache misses."
                       "In case of a high cache miss ratio you need to increase"
                       " the cache size",
-        .mhandler.cmd = hmp_migrate_set_cache_size,
+        .cmd        = hmp_migrate_set_cache_size,
     },
 
 STEXI
@@ -969,7 +969,7 @@ ETEXI
         .params     = "value",
         .help       = "set maximum speed (in bytes) for migrations. "
 	"Defaults to MB if no size suffix is specified, ie. B/K/M/G/T",
-        .mhandler.cmd = hmp_migrate_set_speed,
+        .cmd        = hmp_migrate_set_speed,
     },
 
 STEXI
@@ -983,7 +983,7 @@ ETEXI
         .args_type  = "value:T",
         .params     = "value",
         .help       = "set maximum tolerated downtime (in seconds) for migrations",
-        .mhandler.cmd = hmp_migrate_set_downtime,
+        .cmd        = hmp_migrate_set_downtime,
     },
 
 STEXI
@@ -997,7 +997,7 @@ ETEXI
         .args_type  = "capability:s,state:b",
         .params     = "capability state",
         .help       = "Enable/Disable the usage of a capability for migration",
-        .mhandler.cmd = hmp_migrate_set_capability,
+        .cmd        = hmp_migrate_set_capability,
         .command_completion = migrate_set_capability_completion,
     },
 
@@ -1012,7 +1012,7 @@ ETEXI
         .args_type  = "parameter:s,value:s",
         .params     = "parameter value",
         .help       = "Set the parameter for migration",
-        .mhandler.cmd = hmp_migrate_set_parameter,
+        .cmd        = hmp_migrate_set_parameter,
         .command_completion = migrate_set_parameter_completion,
     },
 
@@ -1029,7 +1029,7 @@ ETEXI
         .help       = "Followup to a migration command to switch the migration"
                       " to postcopy mode. The postcopy-ram capability must "
                       "be set before the original migration command.",
-        .mhandler.cmd = hmp_migrate_start_postcopy,
+        .cmd        = hmp_migrate_start_postcopy,
     },
 
 STEXI
@@ -1044,7 +1044,7 @@ ETEXI
         .args_type  = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
         .params     = "protocol hostname port tls-port cert-subject",
         .help       = "set migration information for remote display",
-        .mhandler.cmd = hmp_client_migrate_info,
+        .cmd        = hmp_client_migrate_info,
     },
 
 STEXI
@@ -1067,7 +1067,7 @@ ETEXI
                       "-s: dump in kdump-compressed format, with snappy compression.\n\t\t\t"
                       "begin: the starting physical address.\n\t\t\t"
                       "length: the memory size, in bytes.",
-        .mhandler.cmd = hmp_dump_guest_memory,
+        .cmd        = hmp_dump_guest_memory,
     },
 
 
@@ -1094,7 +1094,7 @@ ETEXI
         .args_type  = "filename:F",
         .params     = "",
         .help       = "Save guest storage keys into file 'filename'.\n",
-        .mhandler.cmd = hmp_dump_skeys,
+        .cmd        = hmp_dump_skeys,
     },
 #endif
 
@@ -1116,7 +1116,7 @@ ETEXI
                       "The default format is qcow2.  The -n flag requests QEMU\n\t\t\t"
                       "to reuse the image found in new-image-file, instead of\n\t\t\t"
                       "recreating it from scratch.",
-        .mhandler.cmd = hmp_snapshot_blkdev,
+        .cmd        = hmp_snapshot_blkdev,
     },
 
 STEXI
@@ -1132,7 +1132,7 @@ ETEXI
         .help       = "take an internal snapshot of device.\n\t\t\t"
                       "The format of the image used by device must\n\t\t\t"
                       "support it, such as qcow2.\n\t\t\t",
-        .mhandler.cmd = hmp_snapshot_blkdev_internal,
+        .cmd        = hmp_snapshot_blkdev_internal,
     },
 
 STEXI
@@ -1150,7 +1150,7 @@ ETEXI
                       "the snapshot matching both id and name.\n\t\t\t"
                       "The format of the image used by device must\n\t\t\t"
                       "support it, such as qcow2.\n\t\t\t",
-        .mhandler.cmd = hmp_snapshot_delete_blkdev_internal,
+        .cmd        = hmp_snapshot_delete_blkdev_internal,
     },
 
 STEXI
@@ -1171,7 +1171,7 @@ ETEXI
                       "in new-image-file, instead of recreating it from scratch.\n\t\t\t"
                       "The -f flag requests QEMU to copy the whole disk,\n\t\t\t"
                       "so that the result does not need a backing file.\n\t\t\t",
-        .mhandler.cmd = hmp_drive_mirror,
+        .cmd        = hmp_drive_mirror,
     },
 STEXI
 @item drive_mirror
@@ -1192,7 +1192,7 @@ ETEXI
                       "in new-image-file, instead of recreating it from scratch.\n\t\t\t"
                       "The -f flag requests QEMU to copy the whole disk,\n\t\t\t"
                       "so that the result does not need a backing file.\n\t\t\t",
-        .mhandler.cmd = hmp_drive_backup,
+        .cmd        = hmp_drive_backup,
     },
 STEXI
 @item drive_backup
@@ -1210,7 +1210,7 @@ ETEXI
                       "[,snapshot=on|off][,cache=on|off]\n"
                       "[,readonly=on|off][,copy-on-read=on|off]",
         .help       = "add drive to PCI storage controller",
-        .mhandler.cmd = hmp_drive_add,
+        .cmd        = hmp_drive_add,
     },
 
 STEXI
@@ -1234,7 +1234,7 @@ ETEXI
                       "<error_status> = error string or 32bit\n\t\t\t"
                       "<tlb header> = 32bit x 4\n\t\t\t"
                       "<tlb header prefix> = 32bit x 4",
-        .mhandler.cmd = hmp_pcie_aer_inject_error,
+        .cmd        = hmp_pcie_aer_inject_error,
     },
 
 STEXI
@@ -1248,7 +1248,7 @@ ETEXI
         .args_type  = "device:s,opts:s?",
         .params     = "tap|user|socket|vde|netmap|bridge|vhost-user|dump [options]",
         .help       = "add host VLAN client",
-        .mhandler.cmd = hmp_host_net_add,
+        .cmd        = hmp_host_net_add,
         .command_completion = host_net_add_completion,
     },
 
@@ -1263,7 +1263,7 @@ ETEXI
         .args_type  = "vlan_id:i,device:s",
         .params     = "vlan_id name",
         .help       = "remove host VLAN client",
-        .mhandler.cmd = hmp_host_net_remove,
+        .cmd        = hmp_host_net_remove,
         .command_completion = host_net_remove_completion,
     },
 
@@ -1278,7 +1278,7 @@ ETEXI
         .args_type  = "netdev:O",
         .params     = "[user|tap|socket|vde|bridge|hubport|netmap|vhost-user],id=str[,prop=value][,...]",
         .help       = "add host network device",
-        .mhandler.cmd = hmp_netdev_add,
+        .cmd        = hmp_netdev_add,
         .command_completion = netdev_add_completion,
     },
 
@@ -1293,7 +1293,7 @@ ETEXI
         .args_type  = "id:s",
         .params     = "id",
         .help       = "remove host network device",
-        .mhandler.cmd = hmp_netdev_del,
+        .cmd        = hmp_netdev_del,
         .command_completion = netdev_del_completion,
     },
 
@@ -1308,7 +1308,7 @@ ETEXI
         .args_type  = "object:O",
         .params     = "[qom-type=]type,id=str[,prop=value][,...]",
         .help       = "create QOM object",
-        .mhandler.cmd = hmp_object_add,
+        .cmd        = hmp_object_add,
         .command_completion = object_add_completion,
     },
 
@@ -1323,7 +1323,7 @@ ETEXI
         .args_type  = "id:s",
         .params     = "id",
         .help       = "destroy QOM object",
-        .mhandler.cmd = hmp_object_del,
+        .cmd        = hmp_object_del,
         .command_completion = object_del_completion,
     },
 
@@ -1339,7 +1339,7 @@ ETEXI
         .args_type  = "arg1:s,arg2:s?,arg3:s?",
         .params     = "[vlan_id name] [tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
         .help       = "redirect TCP or UDP connections from host to guest (requires -net user)",
-        .mhandler.cmd = hmp_hostfwd_add,
+        .cmd        = hmp_hostfwd_add,
     },
 #endif
 STEXI
@@ -1354,7 +1354,7 @@ ETEXI
         .args_type  = "arg1:s,arg2:s?,arg3:s?",
         .params     = "[vlan_id name] [tcp|udp]:[hostaddr]:hostport",
         .help       = "remove host-to-guest TCP or UDP redirection",
-        .mhandler.cmd = hmp_hostfwd_remove,
+        .cmd        = hmp_hostfwd_remove,
     },
 
 #endif
@@ -1369,7 +1369,7 @@ ETEXI
         .args_type  = "value:M",
         .params     = "target",
         .help       = "request VM to change its memory allocation (in MB)",
-        .mhandler.cmd = hmp_balloon,
+        .cmd        = hmp_balloon,
     },
 
 STEXI
@@ -1383,7 +1383,7 @@ ETEXI
         .args_type  = "name:s,up:b",
         .params     = "name on|off",
         .help       = "change the link status of a network adapter",
-        .mhandler.cmd = hmp_set_link,
+        .cmd        = hmp_set_link,
         .command_completion = set_link_completion,
     },
 
@@ -1398,7 +1398,7 @@ ETEXI
         .args_type  = "action:s",
         .params     = "[reset|shutdown|poweroff|pause|debug|none]",
         .help       = "change watchdog action",
-        .mhandler.cmd = hmp_watchdog_action,
+        .cmd        = hmp_watchdog_action,
         .command_completion = watchdog_action_completion,
     },
 
@@ -1413,7 +1413,7 @@ ETEXI
         .args_type  = "aclname:s",
         .params     = "aclname",
         .help       = "list rules in the access control list",
-        .mhandler.cmd = hmp_acl_show,
+        .cmd        = hmp_acl_show,
     },
 
 STEXI
@@ -1430,7 +1430,7 @@ ETEXI
         .args_type  = "aclname:s,policy:s",
         .params     = "aclname allow|deny",
         .help       = "set default access control list policy",
-        .mhandler.cmd = hmp_acl_policy,
+        .cmd        = hmp_acl_policy,
     },
 
 STEXI
@@ -1446,7 +1446,7 @@ ETEXI
         .args_type  = "aclname:s,match:s,policy:s,index:i?",
         .params     = "aclname match allow|deny [index]",
         .help       = "add a match rule to the access control list",
-        .mhandler.cmd = hmp_acl_add,
+        .cmd        = hmp_acl_add,
     },
 
 STEXI
@@ -1465,7 +1465,7 @@ ETEXI
         .args_type  = "aclname:s,match:s",
         .params     = "aclname match",
         .help       = "remove a match rule from the access control list",
-        .mhandler.cmd = hmp_acl_remove,
+        .cmd        = hmp_acl_remove,
     },
 
 STEXI
@@ -1479,7 +1479,7 @@ ETEXI
         .args_type  = "aclname:s",
         .params     = "aclname",
         .help       = "reset the access control list",
-        .mhandler.cmd = hmp_acl_reset,
+        .cmd        = hmp_acl_reset,
     },
 
 STEXI
@@ -1494,7 +1494,7 @@ ETEXI
         .args_type  = "all:-a,writable:-w,uri:s",
         .params     = "nbd_server_start [-a] [-w] host:port",
         .help       = "serve block devices on the given host and port",
-        .mhandler.cmd = hmp_nbd_server_start,
+        .cmd        = hmp_nbd_server_start,
     },
 STEXI
 @item nbd_server_start @var{host}:@var{port}
@@ -1510,7 +1510,7 @@ ETEXI
         .args_type  = "writable:-w,device:B",
         .params     = "nbd_server_add [-w] device",
         .help       = "export a block device via NBD",
-        .mhandler.cmd = hmp_nbd_server_add,
+        .cmd        = hmp_nbd_server_add,
     },
 STEXI
 @item nbd_server_add @var{device}
@@ -1525,7 +1525,7 @@ ETEXI
         .args_type  = "",
         .params     = "nbd_server_stop",
         .help       = "stop serving block devices using the NBD protocol",
-        .mhandler.cmd = hmp_nbd_server_stop,
+        .cmd        = hmp_nbd_server_stop,
     },
 STEXI
 @item nbd_server_stop
@@ -1541,7 +1541,7 @@ ETEXI
         .args_type  = "broadcast:-b,cpu_index:i,bank:i,status:l,mcg_status:l,addr:l,misc:l",
         .params     = "[-b] cpu bank status mcgstatus addr misc",
         .help       = "inject a MCE on the given CPU [and broadcast to other CPUs with -b option]",
-        .mhandler.cmd = hmp_mce,
+        .cmd        = hmp_mce,
     },
 
 #endif
@@ -1556,7 +1556,7 @@ ETEXI
         .args_type  = "fdname:s",
         .params     = "getfd name",
         .help       = "receive a file descriptor via SCM rights and assign it a name",
-        .mhandler.cmd = hmp_getfd,
+        .cmd        = hmp_getfd,
     },
 
 STEXI
@@ -1572,7 +1572,7 @@ ETEXI
         .args_type  = "fdname:s",
         .params     = "closefd name",
         .help       = "close a file descriptor previously passed via SCM rights",
-        .mhandler.cmd = hmp_closefd,
+        .cmd        = hmp_closefd,
     },
 
 STEXI
@@ -1588,7 +1588,7 @@ ETEXI
         .args_type  = "device:B,password:s",
         .params     = "block_passwd device password",
         .help       = "set the password of encrypted block devices",
-        .mhandler.cmd = hmp_block_passwd,
+        .cmd        = hmp_block_passwd,
     },
 
 STEXI
@@ -1602,7 +1602,7 @@ ETEXI
         .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
         .params     = "device bps bps_rd bps_wr iops iops_rd iops_wr",
         .help       = "change I/O throttle limits for a block drive",
-        .mhandler.cmd = hmp_block_set_io_throttle,
+        .cmd        = hmp_block_set_io_throttle,
     },
 
 STEXI
@@ -1616,7 +1616,7 @@ ETEXI
         .args_type  = "protocol:s,password:s,connected:s?",
         .params     = "protocol password action-if-connected",
         .help       = "set spice/vnc password",
-        .mhandler.cmd = hmp_set_password,
+        .cmd        = hmp_set_password,
     },
 
 STEXI
@@ -1635,7 +1635,7 @@ ETEXI
         .args_type  = "protocol:s,time:s",
         .params     = "protocol time",
         .help       = "set spice/vnc password expire-time",
-        .mhandler.cmd = hmp_expire_password,
+        .cmd        = hmp_expire_password,
     },
 
 STEXI
@@ -1666,7 +1666,7 @@ ETEXI
         .args_type  = "args:s",
         .params     = "args",
         .help       = "add chardev",
-        .mhandler.cmd = hmp_chardev_add,
+        .cmd        = hmp_chardev_add,
         .command_completion = chardev_add_completion,
     },
 
@@ -1682,7 +1682,7 @@ ETEXI
         .args_type  = "id:s",
         .params     = "id",
         .help       = "remove chardev",
-        .mhandler.cmd = hmp_chardev_remove,
+        .cmd        = hmp_chardev_remove,
         .command_completion = chardev_remove_completion,
     },
 
@@ -1698,7 +1698,7 @@ ETEXI
         .args_type  = "device:B,command:s",
         .params     = "[device] \"[command]\"",
         .help       = "run a qemu-io command on a block device",
-        .mhandler.cmd = hmp_qemu_io,
+        .cmd        = hmp_qemu_io,
     },
 
 STEXI
@@ -1713,7 +1713,7 @@ ETEXI
         .args_type  = "id:i",
         .params     = "id",
         .help       = "add cpu",
-        .mhandler.cmd  = hmp_cpu_add,
+        .cmd        = hmp_cpu_add,
     },
 
 STEXI
@@ -1727,7 +1727,7 @@ ETEXI
         .args_type  = "path:s?",
         .params     = "path",
         .help       = "list QOM properties",
-        .mhandler.cmd  = hmp_qom_list,
+        .cmd        = hmp_qom_list,
     },
 
 STEXI
@@ -1740,7 +1740,7 @@ ETEXI
         .args_type  = "path:s,property:s,value:s",
         .params     = "path property value",
         .help       = "set QOM property",
-        .mhandler.cmd  = hmp_qom_set,
+        .cmd        = hmp_qom_set,
     },
 
 STEXI
@@ -1753,8 +1753,8 @@ ETEXI
         .args_type  = "item:s?",
         .params     = "[subcommand]",
         .help       = "show various information about the system state",
-        .mhandler.cmd = hmp_info_help,
-        .sub_table = info_cmds,
+        .cmd        = hmp_info_help,
+        .sub_table  = info_cmds,
     },
 
 STEXI
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 11/17] qapi: remove the "middle" mode
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (9 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 10/17] monitor: remove mhandler.cmd_new marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands marcandre.lureau
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Now that the register function is always generated, we can
remove the so-called "middle" mode from the generator script.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi-commands.py | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index b150db9..eac64ce 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -206,8 +206,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
         self._visited_ret_types = set()
 
     def visit_end(self):
-        if not middle_mode:
-            self.defn += gen_registry(self._regy)
+        self.defn += gen_registry(self._regy)
         self._regy = None
         self._visited_ret_types = None
 
@@ -221,18 +220,10 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor):
             self.defn += gen_marshal_output(ret_type)
         self.decl += gen_marshal_decl(name)
         self.defn += gen_marshal(name, arg_type, boxed, ret_type)
-        if not middle_mode:
-            self._regy += gen_register_command(name, success_response)
+        self._regy += gen_register_command(name, success_response)
 
 
-middle_mode = False
-
-(input_file, output_dir, do_c, do_h, prefix, opts) = \
-    parse_command_line("m", ["middle"])
-
-for o, a in opts:
-    if o in ("-m", "--middle"):
-        middle_mode = True
+(input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line()
 
 c_comment = '''
 /*
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (10 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 11/17] qapi: remove the "middle" mode marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-17 14:47   ` Markus Armbruster
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 13/17] qmp: update qmp_query_spice fallback marcandre.lureau
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The generated marshal functions do not visit arguments from commands
that take no arguments. Thus they fail to catch invalid
members. Visit the arguments, if provided, to throw an error in case of
invalid members.

Currently, qmp_check_client_args() checks for invalid arguments and
correctly catches this case. When switching to qmp_dispatch() we want to
keep that behaviour. The commands using 'O' may have arbitrary
arguments, and must have 'gen': false in the qapi schema to skip the
generated checks.

Old/new diff:
static void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp)
 {
+    Visitor *v = NULL;
     Error *err = NULL;
-
-    (void)args;
+    if (args) {
+        v = qmp_input_visitor_new(QOBJECT(args), true);
+        visit_start_struct(v, NULL, NULL, 0, &err);
+        if (err) {
+            goto out;
+        }
+
+        if (!err) {
+            visit_check_struct(v, &err);
+        }
+        visit_end_struct(v, NULL);
+        if (err) {
+            goto out;
+        }
+    }

     qmp_stop(&err);
+
+out:
     error_propagate(errp, err);
+    visit_free(v);
+    if (args) {
+        v = qapi_dealloc_visitor_new();
+        visit_start_struct(v, NULL, NULL, 0, NULL);
+
+        visit_end_struct(v, NULL);
+        visit_free(v);
+    }
 }

The new code closely resembles code for a command with arguments.
Differences:
- the visit of the argument and its cleanup struct don't visit any
  members (because there are none).
- the visit of the argument struct and its cleanup are conditional.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/test-qmp-commands.c | 15 ++++++++++++++
 scripts/qapi-commands.py  | 53 ++++++++++++++++++++++++++++++-----------------
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 261fd9e..81cbe54 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -106,6 +106,7 @@ static void test_dispatch_cmd(void)
 static void test_dispatch_cmd_failure(void)
 {
     QDict *req = qdict_new();
+    QDict *args = qdict_new();
     QObject *resp;
 
     qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
@@ -114,6 +115,20 @@ static void test_dispatch_cmd_failure(void)
     assert(resp != NULL);
     assert(qdict_haskey(qobject_to_qdict(resp), "error"));
 
+    qobject_decref(resp);
+    QDECREF(req);
+
+    /* check that with extra arguments it throws an error */
+    req = qdict_new();
+    qdict_put(args, "a", qint_from_int(66));
+    qdict_put(req, "arguments", args);
+
+    qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
+
+    resp = qmp_dispatch(QOBJECT(req));
+    assert(resp != NULL);
+    assert(qdict_haskey(qobject_to_qdict(resp), "error"));
+
     qobject_decref(resp);
     QDECREF(req);
 }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index eac64ce..9c79b18 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -94,11 +94,28 @@ def gen_marshal_decl(name):
                  proto=gen_marshal_proto(name))
 
 
+def if_args(arg_type, block):
+    ret = ''
+    if not arg_type or arg_type.is_empty():
+        ret += mcgen('''
+    if (args) {
+''')
+        push_indent()
+    ret += block
+    if not arg_type or arg_type.is_empty():
+        pop_indent()
+        ret += mcgen('''
+    }
+''')
+    return ret
+
+
 def gen_marshal(name, arg_type, boxed, ret_type):
     ret = mcgen('''
 
 %(proto)s
 {
+    Visitor *v = NULL;
     Error *err = NULL;
 ''',
                 proto=gen_marshal_proto(name))
@@ -109,17 +126,23 @@ def gen_marshal(name, arg_type, boxed, ret_type):
 ''',
                      c_type=ret_type.c_type())
 
+    visit_members = ''
     if arg_type and not arg_type.is_empty():
+        visit_members = 'visit_type_%s_members(v, &arg, &err);' % \
+                        arg_type.c_name()
         ret += mcgen('''
-    Visitor *v;
     %(c_name)s arg = {0};
 
+''',
+                     c_name=arg_type.c_name())
+
+    ret += if_args(arg_type, mcgen('''
     v = qmp_input_visitor_new(QOBJECT(args), true);
     visit_start_struct(v, NULL, NULL, 0, &err);
     if (err) {
         goto out;
     }
-    visit_type_%(c_name)s_members(v, &arg, &err);
+    %(visit_members)s
     if (!err) {
         visit_check_struct(v, &err);
     }
@@ -128,35 +151,27 @@ def gen_marshal(name, arg_type, boxed, ret_type):
         goto out;
     }
 ''',
-                     c_name=arg_type.c_name())
-
-    else:
-        ret += mcgen('''
-
-    (void)args;
-''')
+                                   visit_members=visit_members))
 
     ret += gen_call(name, arg_type, boxed, ret_type)
 
-    # 'goto out' produced above for arg_type, and by gen_call() for ret_type
-    if (arg_type and not arg_type.is_empty()) or ret_type:
-        ret += mcgen('''
+    if arg_type and not arg_type.is_empty():
+        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, NULL);',
+                              c_name=arg_type.c_name())
+    ret += mcgen('''
 
 out:
-''')
-    ret += mcgen('''
     error_propagate(errp, err);
-''')
-    if arg_type and not arg_type.is_empty():
-        ret += mcgen('''
     visit_free(v);
+''')
+    ret += if_args(arg_type, mcgen('''
     v = qapi_dealloc_visitor_new();
     visit_start_struct(v, NULL, NULL, 0, NULL);
-    visit_type_%(c_name)s_members(v, &arg, NULL);
+    %(visit_members)s
     visit_end_struct(v, NULL);
     visit_free(v);
 ''',
-                     c_name=arg_type.c_name())
+                                   visit_members=visit_members))
 
     ret += mcgen('''
 }
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 13/17] qmp: update qmp_query_spice fallback
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (11 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 14/17] monitor: use qmp_dispatch() marcandre.lureau
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following "use qmp_dispatch()" commit will use the generated
dispatch command table that is unaware of compile time conditionals.

There are a few commands that are under #ifdef conditions in
qmp-commands.hx. Move the qmp_query_spice fallback in the same location
as the other fallbacks, return an error instead of abort() and update
the comment.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 monitor.c | 14 ++++++++++++++
 qmp.c     | 16 ----------------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/monitor.c b/monitor.c
index ade11a5..880a4eb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4350,6 +4350,20 @@ QemuOptsList qemu_mon_opts = {
     },
 };
 
+/*
+ * the QAPI schema is blissfully unaware #ifdef FOO commands, and the
+ * QAPI code generator happily generates a qmp_marshal_foo_cmd() that
+ * calls qmp_foo_cmd(). Provide it one, or else linking fails.
+ * FIXME: Educate the QAPI schema on #ifdef commands.
+ */
+#ifndef CONFIG_SPICE
+SpiceInfo *qmp_query_spice(Error **errp)
+{
+    error_setg(errp, QERR_FEATURE_DISABLED, "spice");
+    return NULL;
+};
+#endif
+
 #ifndef TARGET_I386
 void qmp_rtc_reset_reinjection(Error **errp)
 {
diff --git a/qmp.c b/qmp.c
index ebc3ff6..42eeedf 100644
--- a/qmp.c
+++ b/qmp.c
@@ -151,22 +151,6 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
 };
 #endif
 
-#ifndef CONFIG_SPICE
-/*
- * qmp-commands.hx ensures that QMP command query-spice exists only
- * #ifdef CONFIG_SPICE.  Necessary for an accurate query-commands
- * result.  However, the QAPI schema is blissfully unaware of that,
- * and the QAPI code generator happily generates a dead
- * qmp_marshal_query_spice() that calls qmp_query_spice().  Provide it
- * one, or else linking fails.  FIXME Educate the QAPI schema on
- * CONFIG_SPICE.
- */
-SpiceInfo *qmp_query_spice(Error **errp)
-{
-    abort();
-};
-#endif
-
 void qmp_cont(Error **errp)
 {
     Error *local_err = NULL;
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 14/17] monitor: use qmp_dispatch()
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (12 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 13/17] qmp: update qmp_query_spice fallback marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 15/17] build-sys: remove qmp-commands-old.h marcandre.lureau
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Replace the old manual dispatch and validation code by the generic one
provided by qapi common code.

Note that it is now possible to call the following commands that used to
be disabled by compile-time conditionals:
- dump-skeys
- query-spice
- rtc-reset-reinjection
- query-gic-capabilities

Their fallback functions return an appropriate "feature disabled" error.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 monitor.c    | 326 +++++++----------------------------------------------------
 trace-events |   1 -
 2 files changed, 34 insertions(+), 293 deletions(-)

diff --git a/monitor.c b/monitor.c
index 880a4eb..9d5e0e8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -166,7 +166,6 @@ struct MonFdset {
 };
 
 typedef struct {
-    QObject *id;
     JSONMessageParser parser;
     /*
      * When a client connects, we're in capabilities negotiation mode.
@@ -229,8 +228,6 @@ static int mon_refcount;
 static mon_cmd_t mon_cmds[];
 static mon_cmd_t info_cmds[];
 
-static const mon_cmd_t qmp_cmds[];
-
 Monitor *cur_mon;
 
 static QEMUClockType event_clock_type = QEMU_CLOCK_REALTIME;
@@ -401,49 +398,6 @@ static void monitor_json_emitter(Monitor *mon, const QObject *data)
     QDECREF(json);
 }
 
-static QDict *build_qmp_error_dict(Error *err)
-{
-    QObject *obj;
-
-    obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }",
-                             QapiErrorClass_lookup[error_get_class(err)],
-                             error_get_pretty(err));
-
-    return qobject_to_qdict(obj);
-}
-
-static void monitor_protocol_emitter(Monitor *mon, QObject *data,
-                                     Error *err)
-{
-    QDict *qmp;
-
-    trace_monitor_protocol_emitter(mon);
-
-    if (!err) {
-        /* success response */
-        qmp = qdict_new();
-        if (data) {
-            qobject_incref(data);
-            qdict_put_obj(qmp, "return", data);
-        } else {
-            /* return an empty QDict by default */
-            qdict_put(qmp, "return", qdict_new());
-        }
-    } else {
-        /* error response */
-        qmp = build_qmp_error_dict(err);
-    }
-
-    if (mon->qmp.id) {
-        qdict_put_obj(qmp, "id", mon->qmp.id);
-        mon->qmp.id = NULL;
-    }
-
-    monitor_json_emitter(mon, QOBJECT(qmp));
-    QDECREF(qmp);
-}
-
-
 static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     /* Limit guest-triggerable events to 1 per second */
     [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
@@ -2203,11 +2157,6 @@ static mon_cmd_t mon_cmds[] = {
     { NULL, NULL, },
 };
 
-static const mon_cmd_t qmp_cmds[] = {
-#include "qmp-commands-old.h"
-    { /* NULL */ },
-};
-
 /*******************************************************************/
 
 static const char *pch;
@@ -2558,11 +2507,6 @@ static const mon_cmd_t *search_dispatch_table(const mon_cmd_t *disp_table,
     return NULL;
 }
 
-static const mon_cmd_t *qmp_find_cmd(const char *cmdname)
-{
-    return search_dispatch_table(qmp_cmds, cmdname);
-}
-
 /*
  * Parse command name from @cmdp according to command table @table.
  * If blank, return NULL.
@@ -3713,199 +3657,6 @@ static bool invalid_qmp_mode(const Monitor *mon, const char *cmd,
     return false;
 }
 
-/*
- * Argument validation rules:
- *
- * 1. The argument must exist in cmd_args qdict
- * 2. The argument type must be the expected one
- *
- * Special case: If the argument doesn't exist in cmd_args and
- *               the QMP_ACCEPT_UNKNOWNS flag is set, then the
- *               checking is skipped for it.
- */
-static void check_client_args_type(const QDict *client_args,
-                                   const QDict *cmd_args, int flags,
-                                   Error **errp)
-{
-    const QDictEntry *ent;
-
-    for (ent = qdict_first(client_args); ent;ent = qdict_next(client_args,ent)){
-        QObject *obj;
-        QString *arg_type;
-        const QObject *client_arg = qdict_entry_value(ent);
-        const char *client_arg_name = qdict_entry_key(ent);
-
-        obj = qdict_get(cmd_args, client_arg_name);
-        if (!obj) {
-            if (flags & QMP_ACCEPT_UNKNOWNS) {
-                /* handler accepts unknowns */
-                continue;
-            }
-            /* client arg doesn't exist */
-            error_setg(errp, QERR_INVALID_PARAMETER, client_arg_name);
-            return;
-        }
-
-        arg_type = qobject_to_qstring(obj);
-        assert(arg_type != NULL);
-
-        /* check if argument's type is correct */
-        switch (qstring_get_str(arg_type)[0]) {
-        case 'F':
-        case 'B':
-        case 's':
-            if (qobject_type(client_arg) != QTYPE_QSTRING) {
-                error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                           client_arg_name, "string");
-                return;
-            }
-        break;
-        case 'i':
-        case 'l':
-        case 'M':
-        case 'o':
-            if (qobject_type(client_arg) != QTYPE_QINT) {
-                error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                           client_arg_name, "int");
-                return;
-            }
-            break;
-        case 'T':
-            if (qobject_type(client_arg) != QTYPE_QINT &&
-                qobject_type(client_arg) != QTYPE_QFLOAT) {
-                error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                           client_arg_name, "number");
-                return;
-            }
-            break;
-        case 'b':
-        case '-':
-            if (qobject_type(client_arg) != QTYPE_QBOOL) {
-                error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                           client_arg_name, "bool");
-                return;
-            }
-            break;
-        case 'O':
-            assert(flags & QMP_ACCEPT_UNKNOWNS);
-            break;
-        case 'q':
-            /* Any QObject can be passed.  */
-            break;
-        case '/':
-        case '.':
-            /*
-             * These types are not supported by QMP and thus are not
-             * handled here. Fall through.
-             */
-        default:
-            abort();
-        }
-    }
-}
-
-/*
- * - Check if the client has passed all mandatory args
- * - Set special flags for argument validation
- */
-static void check_mandatory_args(const QDict *cmd_args,
-                                 const QDict *client_args, int *flags,
-                                 Error **errp)
-{
-    const QDictEntry *ent;
-
-    for (ent = qdict_first(cmd_args); ent; ent = qdict_next(cmd_args, ent)) {
-        const char *cmd_arg_name = qdict_entry_key(ent);
-        QString *type = qobject_to_qstring(qdict_entry_value(ent));
-        assert(type != NULL);
-
-        if (qstring_get_str(type)[0] == 'O') {
-            assert((*flags & QMP_ACCEPT_UNKNOWNS) == 0);
-            *flags |= QMP_ACCEPT_UNKNOWNS;
-        } else if (qstring_get_str(type)[0] != '-' &&
-                   qstring_get_str(type)[1] != '?' &&
-                   !qdict_haskey(client_args, cmd_arg_name)) {
-            error_setg(errp, QERR_MISSING_PARAMETER, cmd_arg_name);
-            return;
-        }
-    }
-}
-
-static QDict *qdict_from_args_type(const char *args_type)
-{
-    int i;
-    QDict *qdict;
-    QString *key, *type, *cur_qs;
-
-    assert(args_type != NULL);
-
-    qdict = qdict_new();
-
-    if (args_type == NULL || args_type[0] == '\0') {
-        /* no args, empty qdict */
-        goto out;
-    }
-
-    key = qstring_new();
-    type = qstring_new();
-
-    cur_qs = key;
-
-    for (i = 0;; i++) {
-        switch (args_type[i]) {
-            case ',':
-            case '\0':
-                qdict_put(qdict, qstring_get_str(key), type);
-                QDECREF(key);
-                if (args_type[i] == '\0') {
-                    goto out;
-                }
-                type = qstring_new(); /* qdict has ref */
-                cur_qs = key = qstring_new();
-                break;
-            case ':':
-                cur_qs = type;
-                break;
-            default:
-                qstring_append_chr(cur_qs, args_type[i]);
-                break;
-        }
-    }
-
-out:
-    return qdict;
-}
-
-/*
- * Client argument checking rules:
- *
- * 1. Client must provide all mandatory arguments
- * 2. Each argument provided by the client must be expected
- * 3. Each argument provided by the client must have the type expected
- *    by the command
- */
-static void qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args,
-                                  Error **errp)
-{
-    Error *err = NULL;
-    int flags;
-    QDict *cmd_args;
-
-    cmd_args = qdict_from_args_type(cmd->args_type);
-
-    flags = 0;
-    check_mandatory_args(cmd_args, client_args, &flags, &err);
-    if (err) {
-        goto out;
-    }
-
-    check_client_args_type(client_args, cmd_args, flags, &err);
-
-out:
-    error_propagate(errp, err);
-    QDECREF(cmd_args);
-}
-
 /*
  * Input object checking rules
  *
@@ -3964,67 +3715,58 @@ static QDict *qmp_check_input_obj(QObject *input_obj, Error **errp)
 
 static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
 {
-    Error *local_err = NULL;
-    QObject *obj, *data;
-    QDict *input, *args;
-    const mon_cmd_t *cmd;
-    QmpCommand *qcmd;
+    QObject *req, *rsp = NULL, *id = NULL;
+    QDict *qdict = NULL;
     const char *cmd_name;
     Monitor *mon = cur_mon;
+    Error *err = NULL;
 
-    args = input = NULL;
-    data = NULL;
-
-    obj = json_parser_parse(tokens, NULL);
-    if (!obj) {
-        // FIXME: should be triggered in json_parser_parse()
-        error_setg(&local_err, QERR_JSON_PARSING);
+    req = json_parser_parse_err(tokens, NULL, &err);
+    if (err || !req || qobject_type(req) != QTYPE_QDICT) {
+        if (!err) {
+            error_setg(&err, QERR_JSON_PARSING);
+        }
         goto err_out;
     }
 
-    input = qmp_check_input_obj(obj, &local_err);
-    if (!input) {
-        qobject_decref(obj);
+    qdict = qmp_check_input_obj(req, &err);
+    if (!qdict) {
         goto err_out;
     }
 
-    mon->qmp.id = qdict_get(input, "id");
-    qobject_incref(mon->qmp.id);
+    id = qdict_get(qdict, "id");
+    qobject_incref(id);
+    qdict_del(qdict, "id");
 
-    cmd_name = qdict_get_str(input, "execute");
+    cmd_name = qdict_get_str(qdict, "execute");
     trace_handle_qmp_command(mon, cmd_name);
-    cmd = qmp_find_cmd(cmd_name);
-    qcmd = qmp_find_command(cmd_name);
-    if (!qcmd || !cmd) {
-        error_set(&local_err, ERROR_CLASS_COMMAND_NOT_FOUND,
-                  "The command %s has not been found", cmd_name);
-        goto err_out;
-    }
-    if (invalid_qmp_mode(mon, cmd_name, &local_err)) {
+
+    if (invalid_qmp_mode(mon, cmd_name, &err)) {
         goto err_out;
     }
 
-    obj = qdict_get(input, "arguments");
-    if (!obj) {
-        args = qdict_new();
-    } else {
-        args = qobject_to_qdict(obj);
-        QINCREF(args);
-    }
+    rsp = qmp_dispatch(req);
 
-    qmp_check_client_args(cmd, args, &local_err);
-    if (local_err) {
-        goto err_out;
+err_out:
+    if (err) {
+        qdict = qdict_new();
+        qdict_put_obj(qdict, "error", qmp_build_error_object(err));
+        error_free(err);
+        rsp = QOBJECT(qdict);
     }
 
-    qcmd->fn(args, &data, &local_err);
+    if (rsp) {
+        if (id) {
+            qdict_put_obj(qobject_to_qdict(rsp), "id", id);
+            id = NULL;
+        }
 
-err_out:
-    monitor_protocol_emitter(mon, data, local_err);
-    qobject_decref(data);
-    error_free(local_err);
-    QDECREF(input);
-    QDECREF(args);
+        monitor_json_emitter(mon, rsp);
+    }
+
+    qobject_decref(id);
+    qobject_decref(rsp);
+    qobject_decref(req);
 }
 
 static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size)
diff --git a/trace-events b/trace-events
index 52c6a6c..cdc8653 100644
--- a/trace-events
+++ b/trace-events
@@ -97,7 +97,6 @@ qemu_co_mutex_unlock_return(void *mutex, void *self) "mutex %p self %p"
 
 # monitor.c
 handle_qmp_command(void *mon, const char *cmd_name) "mon %p cmd_name \"%s\""
-monitor_protocol_emitter(void *mon) "mon %p"
 monitor_protocol_event_handler(uint32_t event, void *qdict) "event=%d data=%p"
 monitor_protocol_event_emit(uint32_t event, void *data) "event=%d data=%p"
 monitor_protocol_event_queue(uint32_t event, void *qdict, uint64_t rate) "event=%d data=%p rate=%" PRId64
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 15/17] build-sys: remove qmp-commands-old.h
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (13 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 14/17] monitor: use qmp_dispatch() marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 16/17] Replace qmp-commands.hx by doc/qmp-commands.txt marcandre.lureau
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 Makefile.target | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index a440bcb..3d8572f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -156,7 +156,7 @@ else
 obj-y += hw/$(TARGET_BASE_ARCH)/
 endif
 
-GENERATED_HEADERS += hmp-commands.h hmp-commands-info.h qmp-commands-old.h
+GENERATED_HEADERS += hmp-commands.h hmp-commands-info.h
 
 endif # CONFIG_SOFTMMU
 
@@ -209,13 +209,10 @@ hmp-commands.h: $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/scripts/hxtool
 hmp-commands-info.h: $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/scripts/hxtool
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
 
-qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx $(SRC_PATH)/scripts/hxtool
-	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $(TARGET_DIR)$@")
-
 clean:
 	rm -f *.a *~ $(PROGS)
 	rm -f $(shell find . -name '*.[od]')
-	rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
+	rm -f hmp-commands.h gdbstub-xml.c
 ifdef CONFIG_TRACE_SYSTEMTAP
 	rm -f *.stp
 endif
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 16/17] Replace qmp-commands.hx by doc/qmp-commands.txt
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (14 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 15/17] build-sys: remove qmp-commands-old.h marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-17 15:01   ` Markus Armbruster
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 17/17] qmp-commands.txt: fix some styling marcandre.lureau
  2016-08-11  4:45 ` [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode no-reply
  17 siblings, 1 reply; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The only remaining function of qmp-commands.hx is to let us generate
qmp-commands.txt from it.  Replace qmp-commands.hx by qmp-commands.txt.

(a later update will move the documentation in the respective json files
and again generate the text file)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 .gitignore                               |    1 -
 MAINTAINERS                              |    1 -
 Makefile                                 |    6 +-
 docs/qapi-code-gen.txt                   |    6 +-
 qmp-commands.hx => docs/qmp-commands.txt | 1111 ------------------------------
 docs/writing-qmp-commands.txt            |   38 -
 6 files changed, 4 insertions(+), 1159 deletions(-)
 rename qmp-commands.hx => docs/qmp-commands.txt (87%)

diff --git a/.gitignore b/.gitignore
index 88ec249..0ab8263 100644
--- a/.gitignore
+++ b/.gitignore
@@ -53,7 +53,6 @@
 /qemu-bridge-helper
 /qemu-monitor.texi
 /qemu-monitor-info.texi
-/qmp-commands.txt
 /vscclient
 /fsdev/virtfs-proxy-helper
 *.[1-9]
diff --git a/MAINTAINERS b/MAINTAINERS
index b6fb84e..84bdc46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1237,7 +1237,6 @@ M: Markus Armbruster <armbru@redhat.com>
 S: Supported
 F: qmp.c
 F: monitor.c
-F: qmp-commands.hx
 F: docs/*qmp-*
 F: scripts/qmp/
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
diff --git a/Makefile b/Makefile
index d8a2891..bac69b2 100644
--- a/Makefile
+++ b/Makefile
@@ -92,7 +92,6 @@ HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
 
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
-DOCS+=qmp-commands.txt
 ifdef CONFIG_VIRTFS
 DOCS+=fsdev/virtfs-proxy-helper.1
 endif
@@ -432,7 +431,7 @@ endif
 install-doc: $(DOCS)
 	$(INSTALL_DIR) "$(DESTDIR)$(qemu_docdir)"
 	$(INSTALL_DATA) qemu-doc.html  qemu-tech.html "$(DESTDIR)$(qemu_docdir)"
-	$(INSTALL_DATA) qmp-commands.txt "$(DESTDIR)$(qemu_docdir)"
+	$(INSTALL_DATA) docs/qmp-commands.txt "$(DESTDIR)$(qemu_docdir)"
 ifdef CONFIG_POSIX
 	$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
 	$(INSTALL_DATA) qemu.1 "$(DESTDIR)$(mandir)/man1"
@@ -555,9 +554,6 @@ qemu-monitor.texi: $(SRC_PATH)/hmp-commands.hx $(SRC_PATH)/scripts/hxtool
 qemu-monitor-info.texi: $(SRC_PATH)/hmp-commands-info.hx $(SRC_PATH)/scripts/hxtool
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"  GEN   $@")
 
-qmp-commands.txt: $(SRC_PATH)/qmp-commands.hx $(SRC_PATH)/scripts/hxtool
-	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -q < $< > $@,"  GEN   $@")
-
 qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"  GEN   $@")
 
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index de298dc..5d4c2cd 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -964,9 +964,9 @@ Example:
 
 Used to generate the marshaling/dispatch functions for the commands
 defined in the schema. The generated code implements
-qmp_marshal_COMMAND() (mentioned in qmp-commands.hx, and registered
-automatically), and declares qmp_COMMAND() that the user must
-implement.  The following files are generated:
+qmp_marshal_COMMAND() (registered automatically), and declares
+qmp_COMMAND() that the user must implement.  The following files are
+generated:
 
 $(prefix)qmp-marshal.c: command marshal/dispatch functions for each
                         QMP command defined in the schema. Functions
diff --git a/qmp-commands.hx b/docs/qmp-commands.txt
similarity index 87%
rename from qmp-commands.hx
rename to docs/qmp-commands.txt
index 1ad8dda..dfa1cc1 100644
--- a/qmp-commands.hx
+++ b/docs/qmp-commands.txt
@@ -1,8 +1,3 @@
-HXCOMM QMP dispatch table and documentation
-HXCOMM Text between SQMP and EQMP is copied to the QMP documentation file and
-HXCOMM does not show up in the other formats.
-
-SQMP
                         QMP Supported Commands
                         ----------------------
 
@@ -58,14 +53,6 @@ If you're planning to adopt QMP, please observe the following:
 Server's responses in the examples below are always a success response, please
 refer to the QMP specification for more details on error responses.
 
-EQMP
-
-    {
-        .name       = "quit",
-        .args_type  = "",
-    },
-
-SQMP
 quit
 ----
 
@@ -78,14 +65,6 @@ Example:
 -> { "execute": "quit" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "eject",
-        .args_type  = "force:-f,device:B",
-    },
-
-SQMP
 eject
 -----
 
@@ -103,14 +82,6 @@ Example:
 
 Note: The "force" argument defaults to false.
 
-EQMP
-
-    {
-        .name       = "change",
-        .args_type  = "device:B,target:F,arg:s?",
-    },
-
-SQMP
 change
 ------
 
@@ -138,14 +109,6 @@ Examples:
                             "arg": "foobar1" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "screendump",
-        .args_type  = "filename:F",
-    },
-
-SQMP
 screendump
 ----------
 
@@ -160,14 +123,6 @@ Example:
 -> { "execute": "screendump", "arguments": { "filename": "/tmp/image" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "stop",
-        .args_type  = "",
-    },
-
-SQMP
 stop
 ----
 
@@ -180,14 +135,6 @@ Example:
 -> { "execute": "stop" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "cont",
-        .args_type  = "",
-    },
-
-SQMP
 cont
 ----
 
@@ -200,14 +147,6 @@ Example:
 -> { "execute": "cont" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "system_wakeup",
-        .args_type  = "",
-    },
-
-SQMP
 system_wakeup
 -------------
 
@@ -220,14 +159,6 @@ Example:
 -> { "execute": "system_wakeup" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "system_reset",
-        .args_type  = "",
-    },
-
-SQMP
 system_reset
 ------------
 
@@ -240,14 +171,6 @@ Example:
 -> { "execute": "system_reset" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "system_powerdown",
-        .args_type  = "",
-    },
-
-SQMP
 system_powerdown
 ----------------
 
@@ -260,16 +183,6 @@ Example:
 -> { "execute": "system_powerdown" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "device_add",
-        .args_type  = "device:O",
-        .params     = "driver[,prop=value][,...]",
-        .help       = "add device, like -device on the command line",
-    },
-
-SQMP
 device_add
 ----------
 
@@ -295,14 +208,6 @@ Notes:
 (2) It's possible to list device properties by running QEMU with the
     "-device DEVICE,\?" command-line argument, where DEVICE is the device's name
 
-EQMP
-
-    {
-        .name       = "device_del",
-        .args_type  = "id:s",
-    },
-
-SQMP
 device_del
 ----------
 
@@ -322,14 +227,6 @@ Example:
 -> { "execute": "device_del", "arguments": { "id": "/machine/peripheral-anon/device[0]" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "send-key",
-        .args_type  = "keys:q,hold-time:i?",
-    },
-
-SQMP
 send-key
 ----------
 
@@ -352,14 +249,6 @@ Example:
                               { "type": "qcode", "data": "delete" } ] } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "cpu",
-        .args_type  = "index:i",
-    },
-
-SQMP
 cpu
 ---
 
@@ -376,14 +265,6 @@ Example:
 
 Note: CPUs' indexes are obtained with the 'query-cpus' command.
 
-EQMP
-
-    {
-        .name       = "cpu-add",
-        .args_type  = "id:i",
-    },
-
-SQMP
 cpu-add
 -------
 
@@ -398,14 +279,6 @@ Example:
 -> { "execute": "cpu-add", "arguments": { "id": 2 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "memsave",
-        .args_type  = "val:l,size:i,filename:s,cpu:i?",
-    },
-
-SQMP
 memsave
 -------
 
@@ -426,14 +299,6 @@ Example:
                             "filename": "/tmp/virtual-mem-dump" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "pmemsave",
-        .args_type  = "val:l,size:i,filename:s",
-    },
-
-SQMP
 pmemsave
 --------
 
@@ -453,14 +318,6 @@ Example:
                             "filename": "/tmp/physical-mem-dump" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "inject-nmi",
-        .args_type  = "",
-    },
-
-SQMP
 inject-nmi
 ----------
 
@@ -475,14 +332,6 @@ Example:
 
 Note: inject-nmi fails when the guest doesn't support injecting.
 
-EQMP
-
-    {
-        .name       = "ringbuf-write",
-        .args_type  = "device:s,data:s,format:s?",
-    },
-
-SQMP
 ringbuf-write
 -------------
 
@@ -503,14 +352,6 @@ Example:
                                "format": "utf8" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "ringbuf-read",
-        .args_type  = "device:s,size:i,format:s?",
-    },
-
-SQMP
 ringbuf-read
 -------------
 
@@ -538,14 +379,6 @@ Example:
                                "format": "utf8" } }
 <- {"return": "abcdefgh"}
 
-EQMP
-
-    {
-        .name       = "xen-save-devices-state",
-        .args_type  = "filename:F",
-    },
-
-SQMP
 xen-save-devices-state
 -------
 
@@ -564,14 +397,6 @@ Example:
      "arguments": { "filename": "/tmp/save" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "xen-load-devices-state",
-        .args_type  = "filename:F",
-    },
-
-SQMP
 xen-load-devices-state
 ----------------------
 
@@ -590,14 +415,6 @@ Example:
      "arguments": { "filename": "/tmp/resume" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "xen-set-global-dirty-log",
-        .args_type  = "enable:b",
-    },
-
-SQMP
 xen-set-global-dirty-log
 -------
 
@@ -613,14 +430,6 @@ Example:
      "arguments": { "enable": true } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "migrate",
-        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
-    },
-
-SQMP
 migrate
 -------
 
@@ -645,14 +454,6 @@ Notes:
 (3) The user Monitor's "detach" argument is invalid in QMP and should not
     be used
 
-EQMP
-
-    {
-        .name       = "migrate_cancel",
-        .args_type  = "",
-    },
-
-SQMP
 migrate_cancel
 --------------
 
@@ -665,14 +466,6 @@ Example:
 -> { "execute": "migrate_cancel" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "migrate-incoming",
-        .args_type  = "uri:s",
-    },
-
-SQMP
 migrate-incoming
 ----------------
 
@@ -693,13 +486,6 @@ Notes:
     be used
 (2) The uri format is the same as for -incoming
 
-EQMP
-    {
-        .name       = "migrate-set-cache-size",
-        .args_type  = "value:o",
-    },
-
-SQMP
 migrate-set-cache-size
 ----------------------
 
@@ -715,13 +501,6 @@ Example:
 -> { "execute": "migrate-set-cache-size", "arguments": { "value": 536870912 } }
 <- { "return": {} }
 
-EQMP
-    {
-        .name       = "migrate-start-postcopy",
-        .args_type  = "",
-    },
-
-SQMP
 migrate-start-postcopy
 ----------------------
 
@@ -732,14 +511,6 @@ Example:
 -> { "execute": "migrate-start-postcopy" }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "query-migrate-cache-size",
-        .args_type  = "",
-    },
-
-SQMP
 query-migrate-cache-size
 ------------------------
 
@@ -753,14 +524,6 @@ Example:
 -> { "execute": "query-migrate-cache-size" }
 <- { "return": 67108864 }
 
-EQMP
-
-    {
-        .name       = "migrate_set_speed",
-        .args_type  = "value:o",
-    },
-
-SQMP
 migrate_set_speed
 -----------------
 
@@ -775,14 +538,6 @@ Example:
 -> { "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "migrate_set_downtime",
-        .args_type  = "value:T",
-    },
-
-SQMP
 migrate_set_downtime
 --------------------
 
@@ -797,16 +552,6 @@ Example:
 -> { "execute": "migrate_set_downtime", "arguments": { "value": 0.1 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .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",
-        .help       = "set migration information for remote display",
-    },
-
-SQMP
 client_migrate_info
 -------------------
 
@@ -830,16 +575,6 @@ Example:
                     "port": 1234 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "dump-guest-memory",
-        .args_type  = "paging:b,protocol:s,detach:b?,begin:i?,end:i?,format:s?",
-        .params     = "-p protocol [-d] [begin] [length] [format]",
-        .help       = "dump guest memory to file",
-    },
-
-SQMP
 dump
 
 
@@ -870,14 +605,6 @@ Notes:
 
 (1) All boolean arguments default to false
 
-EQMP
-
-    {
-        .name       = "query-dump-guest-memory-capability",
-        .args_type  = "",
-    },
-
-SQMP
 query-dump-guest-memory-capability
 ----------
 
@@ -889,16 +616,6 @@ Example:
 <- { "return": { "formats":
                     ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }
 
-EQMP
-
-    {
-        .name       = "query-dump",
-        .args_type  = "",
-        .params     = "",
-        .help       = "query background dump status",
-    },
-
-SQMP
 query-dump
 ----------
 
@@ -912,16 +629,6 @@ Example:
 <- { "return": { "status": "active", "completed": 1024000,
                  "total": 2048000 } }
 
-EQMP
-
-#if defined TARGET_S390X
-    {
-        .name       = "dump-skeys",
-        .args_type  = "filename:F",
-    },
-#endif
-
-SQMP
 dump-skeys
 ----------
 
@@ -936,14 +643,6 @@ Example:
 -> { "execute": "dump-skeys", "arguments": { "filename": "/tmp/skeys" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "netdev_add",
-        .args_type  = "netdev:O",
-    },
-
-SQMP
 netdev_add
 ----------
 
@@ -966,14 +665,6 @@ Note: The supported device options are the same ones supported by the '-netdev'
       command-line argument, which are listed in the '-help' output or QEMU's
       manual
 
-EQMP
-
-    {
-        .name       = "netdev_del",
-        .args_type  = "id:s",
-    },
-
-SQMP
 netdev_del
 ----------
 
@@ -989,14 +680,6 @@ Example:
 <- { "return": {} }
 
 
-EQMP
-
-    {
-        .name       = "object-add",
-        .args_type  = "qom-type:s,id:s,props:q?",
-    },
-
-SQMP
 object-add
 ----------
 
@@ -1014,14 +697,6 @@ Example:
      "props": { "filename": "/dev/hwrng" } } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "object-del",
-        .args_type  = "id:s",
-    },
-
-SQMP
 object-del
 ----------
 
@@ -1037,15 +712,6 @@ Example:
 <- { "return": {} }
 
 
-EQMP
-
-
-    {
-        .name       = "block_resize",
-        .args_type  = "device:s?,node-name:s?,size:o",
-    },
-
-SQMP
 block_resize
 ------------
 
@@ -1062,14 +728,6 @@ Example:
 -> { "execute": "block_resize", "arguments": { "device": "scratch", "size": 1073741824 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-stream",
-        .args_type  = "job-id:s?,device:B,base:s?,speed:o?,backing-file:s?,on-error:s?",
-    },
-
-SQMP
 block-stream
 ------------
 
@@ -1106,14 +764,6 @@ Example:
                                                "base": "/tmp/master.qcow2" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-commit",
-        .args_type  = "job-id:s?,device:B,base:s?,top:s?,backing-file:s?,speed:o?",
-    },
-
-SQMP
 block-commit
 ------------
 
@@ -1170,15 +820,6 @@ Example:
                                               "top": "/tmp/snap1.qcow2" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "drive-backup",
-        .args_type  = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
-                      "format:s?,bitmap:s?,on-source-error:s?,on-target-error:s?",
-    },
-
-SQMP
 drive-backup
 ------------
 
@@ -1225,15 +866,6 @@ Example:
                                                "target": "backup.img" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-backup",
-        .args_type  = "job-id:s?,sync:s,device:B,target:B,speed:i?,"
-                      "on-source-error:s?,on-target-error:s?",
-    },
-
-SQMP
 blockdev-backup
 ---------------
 
@@ -1267,35 +899,6 @@ Example:
                                                   "target": "tgt-id" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-job-set-speed",
-        .args_type  = "device:B,speed:o",
-    },
-
-    {
-        .name       = "block-job-cancel",
-        .args_type  = "device:B,force:b?",
-    },
-    {
-        .name       = "block-job-pause",
-        .args_type  = "device:B",
-    },
-    {
-        .name       = "block-job-resume",
-        .args_type  = "device:B",
-    },
-    {
-        .name       = "block-job-complete",
-        .args_type  = "device:B",
-    },
-    {
-        .name       = "transaction",
-        .args_type  = "actions:q,properties:q?",
-    },
-
-SQMP
 transaction
 -----------
 
@@ -1381,14 +984,6 @@ Example:
                                          "name": "snapshot0" } } ] } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-dirty-bitmap-add",
-        .args_type  = "node:B,name:s,granularity:i?",
-    },
-
-SQMP
 
 block-dirty-bitmap-add
 ----------------------
@@ -1408,14 +1003,6 @@ Example:
                                                    "name": "bitmap0" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-dirty-bitmap-remove",
-        .args_type  = "node:B,name:s",
-    },
-
-SQMP
 
 block-dirty-bitmap-remove
 -------------------------
@@ -1435,14 +1022,6 @@ Example:
                                                       "name": "bitmap0" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-dirty-bitmap-clear",
-        .args_type  = "node:B,name:s",
-    },
-
-SQMP
 
 block-dirty-bitmap-clear
 ------------------------
@@ -1463,14 +1042,6 @@ Example:
                                                            "name": "bitmap0" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-snapshot-sync",
-        .args_type  = "device:s?,node-name:s?,snapshot-file:s,snapshot-node-name:s?,format:s?,mode:s?",
-    },
-
-SQMP
 blockdev-snapshot-sync
 ----------------------
 
@@ -1498,14 +1069,6 @@ Example:
                                                         "format": "qcow2" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-snapshot",
-        .args_type  = "node:s,overlay:s",
-    },
-
-SQMP
 blockdev-snapshot
 -----------------
 Since 2.5
@@ -1535,14 +1098,6 @@ Example:
                                                     "overlay": "node1534" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-snapshot-internal-sync",
-        .args_type  = "device:B,name:s",
-    },
-
-SQMP
 blockdev-snapshot-internal-sync
 -------------------------------
 
@@ -1563,14 +1118,6 @@ Example:
    }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-snapshot-delete-internal-sync",
-        .args_type  = "device:B,id:s?,name:s?",
-    },
-
-SQMP
 blockdev-snapshot-delete-internal-sync
 --------------------------------------
 
@@ -1602,18 +1149,6 @@ Example:
      }
    }
 
-EQMP
-
-    {
-        .name       = "drive-mirror",
-        .args_type  = "job-id:s?,sync:s,device:B,target:s,speed:i?,mode:s?,"
-                      "format:s?,node-name:s?,replaces:s?,"
-                      "on-source-error:s?,on-target-error:s?,"
-                      "unmap:b?,"
-                      "granularity:i?,buf-size:i?",
-    },
-
-SQMP
 drive-mirror
 ------------
 
@@ -1667,16 +1202,6 @@ Example:
                                                "format": "qcow2" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-mirror",
-        .args_type  = "job-id:s?,sync:s,device:B,target:B,replaces:s?,speed:i?,"
-                      "on-source-error:s?,on-target-error:s?,"
-                      "granularity:i?,buf-size:i?",
-    },
-
-SQMP
 blockdev-mirror
 ------------
 
@@ -1717,13 +1242,6 @@ Example:
                                                   "sync": "full" } }
 <- { "return": {} }
 
-EQMP
-    {
-        .name       = "change-backing-file",
-        .args_type  = "device:s,image-node-name:s,backing-file:s",
-    },
-
-SQMP
 change-backing-file
 -------------------
 Since: 2.1
@@ -1754,14 +1272,6 @@ Arguments:
 Returns: Nothing on success
          If "device" does not exist or cannot be determined, DeviceNotFound
 
-EQMP
-
-    {
-        .name       = "balloon",
-        .args_type  = "value:M",
-    },
-
-SQMP
 balloon
 -------
 
@@ -1776,14 +1286,6 @@ Example:
 -> { "execute": "balloon", "arguments": { "value": 536870912 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "set_link",
-        .args_type  = "name:s,up:b",
-    },
-
-SQMP
 set_link
 --------
 
@@ -1799,16 +1301,6 @@ Example:
 -> { "execute": "set_link", "arguments": { "name": "e1000.0", "up": false } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "getfd",
-        .args_type  = "fdname:s",
-        .params     = "getfd name",
-        .help       = "receive a file descriptor via SCM rights and assign it a name",
-    },
-
-SQMP
 getfd
 -----
 
@@ -1831,16 +1323,6 @@ Notes:
 (2) The 'closefd' command can be used to explicitly close the file
     descriptor when it is no longer needed.
 
-EQMP
-
-    {
-        .name       = "closefd",
-        .args_type  = "fdname:s",
-        .params     = "closefd name",
-        .help       = "close a file descriptor previously passed via SCM rights",
-    },
-
-SQMP
 closefd
 -------
 
@@ -1855,16 +1337,6 @@ Example:
 -> { "execute": "closefd", "arguments": { "fdname": "fd1" } }
 <- { "return": {} }
 
-EQMP
-
-     {
-        .name       = "add-fd",
-        .args_type  = "fdset-id:i?,opaque:s?",
-        .params     = "add-fd fdset-id opaque",
-        .help       = "Add a file descriptor, that was passed via SCM rights, to an fd set",
-    },
-
-SQMP
 add-fd
 -------
 
@@ -1893,16 +1365,6 @@ Notes:
 (1) The list of fd sets is shared by all monitor connections.
 (2) If "fdset-id" is not specified, a new fd set will be created.
 
-EQMP
-
-     {
-        .name       = "remove-fd",
-        .args_type  = "fdset-id:i,fd:i?",
-        .params     = "remove-fd fdset-id fd",
-        .help       = "Remove a file descriptor from an fd set",
-    },
-
-SQMP
 remove-fd
 ---------
 
@@ -1925,15 +1387,6 @@ Notes:
 (2) If "fd" is not specified, all file descriptors in "fdset-id" will be
     removed.
 
-EQMP
-
-    {
-        .name       = "query-fdsets",
-        .args_type  = "",
-        .help       = "Return information describing all fd sets",
-    },
-
-SQMP
 query-fdsets
 -------------
 
@@ -1974,14 +1427,6 @@ Example:
 
 Note: The list of fd sets is shared by all monitor connections.
 
-EQMP
-
-    {
-        .name       = "block_passwd",
-        .args_type  = "device:s?,node-name:s?,password:s",
-    },
-
-SQMP
 block_passwd
 ------------
 
@@ -1999,14 +1444,6 @@ Example:
                                                "password": "12345" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block_set_io_throttle",
-        .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l,bps_max:l?,bps_rd_max:l?,bps_wr_max:l?,iops_max:l?,iops_rd_max:l?,iops_wr_max:l?,bps_max_length:l?,bps_rd_max_length:l?,bps_wr_max_length:l?,iops_max_length:l?,iops_rd_max_length:l?,iops_wr_max_length:l?,iops_size:l?,group:s?",
-    },
-
-SQMP
 block_set_io_throttle
 ------------
 
@@ -2055,14 +1492,6 @@ Example:
                                                "iops_size": 0 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "set_password",
-        .args_type  = "protocol:s,password:s,connected:s?",
-    },
-
-SQMP
 set_password
 ------------
 
@@ -2080,14 +1509,6 @@ Example:
                                                "password": "secret" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "expire_password",
-        .args_type  = "protocol:s,time:s",
-    },
-
-SQMP
 expire_password
 ---------------
 
@@ -2104,14 +1525,6 @@ Example:
                                                   "time": "+60" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "add_client",
-        .args_type  = "protocol:s,fdname:s,skipauth:b?,tls:b?",
-    },
-
-SQMP
 add_client
 ----------
 
@@ -2130,15 +1543,6 @@ Example:
                                              "fdname": "myclient" } }
 <- { "return": {} }
 
-EQMP
-    {
-        .name       = "qmp_capabilities",
-        .args_type  = "",
-        .params     = "",
-        .help       = "enable QMP capabilities",
-    },
-
-SQMP
 qmp_capabilities
 ----------------
 
@@ -2153,14 +1557,6 @@ Example:
 
 Note: This command must be issued before issuing any other command.
 
-EQMP
-
-    {
-        .name       = "human-monitor-command",
-        .args_type  = "command-line:s,cpu-index:i?",
-    },
-
-SQMP
 human-monitor-command
 ---------------------
 
@@ -2197,13 +1593,7 @@ Notes:
 3. Query Commands
 =================
 
-HXCOMM Each query command below is inside a SQMP/EQMP section, do NOT change
-HXCOMM this! We will possibly move query commands definitions inside those
-HXCOMM sections, just like regular commands.
-
-EQMP
 
-SQMP
 query-version
 -------------
 
@@ -2231,14 +1621,6 @@ Example:
       }
    }
 
-EQMP
-
-    {
-        .name       = "query-version",
-        .args_type  = "",
-    },
-
-SQMP
 query-commands
 --------------
 
@@ -2267,14 +1649,6 @@ Example:
 
 Note: This example has been shortened as the real response is too long.
 
-EQMP
-
-    {
-        .name       = "query-commands",
-        .args_type  = "",
-    },
-
-SQMP
 query-events
 --------------
 
@@ -2303,14 +1677,6 @@ Example:
 
 Note: This example has been shortened as the real response is too long.
 
-EQMP
-
-    {
-        .name       = "query-events",
-        .args_type  = "",
-    },
-
-SQMP
 query-qmp-schema
 ----------------
 
@@ -2319,14 +1685,6 @@ named schema entities.  Entities are commands, events and various
 types.  See docs/qapi-code-gen.txt for information on their structure
 and intended use.
 
-EQMP
-
-    {
-        .name       = "query-qmp-schema",
-        .args_type  = "",
-    },
-
-SQMP
 query-chardev
 -------------
 
@@ -2363,14 +1721,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-chardev",
-        .args_type  = "",
-    },
-
-SQMP
 query-chardev-backends
 -------------
 
@@ -2403,14 +1753,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-chardev-backends",
-        .args_type  = "",
-    },
-
-SQMP
 query-block
 -----------
 
@@ -2586,14 +1928,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-block",
-        .args_type  = "",
-    },
-
-SQMP
 query-blockstats
 ----------------
 
@@ -2782,14 +2116,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-blockstats",
-        .args_type  = "query-nodes:b?",
-    },
-
-SQMP
 query-cpus
 ----------
 
@@ -2836,14 +2162,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-cpus",
-        .args_type  = "",
-    },
-
-SQMP
 query-iothreads
 ---------------
 
@@ -2874,14 +2192,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-iothreads",
-        .args_type  = "",
-    },
-
-SQMP
 query-pci
 ---------
 
@@ -3090,14 +2400,6 @@ Example:
 
 Note: This example has been shortened as the real response is too long.
 
-EQMP
-
-    {
-        .name       = "query-pci",
-        .args_type  = "",
-    },
-
-SQMP
 query-kvm
 ---------
 
@@ -3113,14 +2415,6 @@ Example:
 -> { "execute": "query-kvm" }
 <- { "return": { "enabled": true, "present": true } }
 
-EQMP
-
-    {
-        .name       = "query-kvm",
-        .args_type  = "",
-    },
-
-SQMP
 query-status
 ------------
 
@@ -3152,14 +2446,6 @@ Example:
 -> { "execute": "query-status" }
 <- { "return": { "running": true, "singlestep": false, "status": "running" } }
 
-EQMP
-    
-    {
-        .name       = "query-status",
-        .args_type  = "",
-    },
-
-SQMP
 query-mice
 ----------
 
@@ -3195,14 +2481,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "query-mice",
-        .args_type  = "",
-    },
-
-SQMP
 query-vnc
 ---------
 
@@ -3257,18 +2535,6 @@ Example:
       }
    }
 
-EQMP
-
-    {
-        .name       = "query-vnc",
-        .args_type  = "",
-    },
-    {
-        .name       = "query-vnc-servers",
-        .args_type  = "",
-    },
-
-SQMP
 query-spice
 -----------
 
@@ -3336,16 +2602,6 @@ Example:
       }
    }
 
-EQMP
-
-#if defined(CONFIG_SPICE)
-    {
-        .name       = "query-spice",
-        .args_type  = "",
-    },
-#endif
-
-SQMP
 query-name
 ----------
 
@@ -3360,14 +2616,6 @@ Example:
 -> { "execute": "query-name" }
 <- { "return": { "name": "qemu-name" } }
 
-EQMP
-
-    {
-        .name       = "query-name",
-        .args_type  = "",
-    },
-
-SQMP
 query-uuid
 ----------
 
@@ -3382,14 +2630,6 @@ Example:
 -> { "execute": "query-uuid" }
 <- { "return": { "UUID": "550e8400-e29b-41d4-a716-446655440000" } }
 
-EQMP
-
-    {
-        .name       = "query-uuid",
-        .args_type  = "",
-    },
-
-SQMP
 query-command-line-options
 --------------------------
 
@@ -3430,14 +2670,6 @@ Example:
      ]
    }
 
-EQMP
-
-    {
-        .name       = "query-command-line-options",
-        .args_type  = "option:s?",
-    },
-
-SQMP
 query-migrate
 -------------
 
@@ -3607,14 +2839,6 @@ Examples:
       }
    }
 
-EQMP
-
-    {
-        .name       = "query-migrate",
-        .args_type  = "",
-    },
-
-SQMP
 migrate-set-capabilities
 ------------------------
 
@@ -3635,14 +2859,6 @@ Example:
 -> { "execute": "migrate-set-capabilities" , "arguments":
      { "capabilities": [ { "capability": "xbzrle", "state": true } ] } }
 
-EQMP
-
-    {
-        .name       = "migrate-set-capabilities",
-        .args_type  = "capabilities:q",
-        .params     = "capability:s,state:b",
-    },
-SQMP
 query-migrate-capabilities
 --------------------------
 
@@ -3672,14 +2888,6 @@ Example:
      {"state": false, "capability": "postcopy-ram"}
    ]}
 
-EQMP
-
-    {
-        .name       = "query-migrate-capabilities",
-        .args_type  = "",
-    },
-
-SQMP
 migrate-set-parameters
 ----------------------
 
@@ -3700,14 +2908,6 @@ Example:
 -> { "execute": "migrate-set-parameters" , "arguments":
       { "compress-level": 1 } }
 
-EQMP
-
-    {
-        .name       = "migrate-set-parameters",
-        .args_type  =
-            "compress-level:i?,compress-threads:i?,decompress-threads:i?,cpu-throttle-initial:i?,cpu-throttle-increment:i?",
-    },
-SQMP
 query-migrate-parameters
 ------------------------
 
@@ -3737,14 +2937,6 @@ Example:
       }
    }
 
-EQMP
-
-    {
-        .name       = "query-migrate-parameters",
-        .args_type  = "",
-    },
-
-SQMP
 query-balloon
 -------------
 
@@ -3764,81 +2956,6 @@ Example:
       }
    }
 
-EQMP
-
-    {
-        .name       = "query-balloon",
-        .args_type  = "",
-    },
-
-    {
-        .name       = "query-block-jobs",
-        .args_type  = "",
-    },
-
-    {
-        .name       = "qom-list",
-        .args_type  = "path:s",
-    },
-
-    {
-        .name       = "qom-set",
-	.args_type  = "path:s,property:s,value:q",
-    },
-
-    {
-        .name       = "qom-get",
-	.args_type  = "path:s,property:s",
-    },
-
-    {
-        .name       = "nbd-server-start",
-        .args_type  = "addr:q,tls-creds:s?",
-    },
-    {
-        .name       = "nbd-server-add",
-        .args_type  = "device:B,writable:b?",
-    },
-    {
-        .name       = "nbd-server-stop",
-        .args_type  = "",
-    },
-
-    {
-        .name       = "change-vnc-password",
-        .args_type  = "password:s",
-    },
-    {
-        .name       = "qom-list-types",
-        .args_type  = "implements:s?,abstract:b?",
-    },
-
-    {
-        .name       = "device-list-properties",
-        .args_type  = "typename:s",
-    },
-
-    {
-        .name       = "query-machines",
-        .args_type  = "",
-    },
-
-    {
-        .name       = "query-cpu-definitions",
-        .args_type  = "",
-    },
-
-    {
-        .name       = "query-target",
-        .args_type  = "",
-    },
-
-    {
-        .name       = "query-tpm",
-        .args_type  = "",
-    },
-
-SQMP
 query-tpm
 ---------
 
@@ -3864,14 +2981,6 @@ Example:
      ]
    }
 
-EQMP
-
-    {
-        .name       = "query-tpm-models",
-        .args_type  = "",
-    },
-
-SQMP
 query-tpm-models
 ----------------
 
@@ -3884,14 +2993,6 @@ Example:
 -> { "execute": "query-tpm-models" }
 <- { "return": [ "tpm-tis" ] }
 
-EQMP
-
-    {
-        .name       = "query-tpm-types",
-        .args_type  = "",
-    },
-
-SQMP
 query-tpm-types
 ---------------
 
@@ -3904,14 +3005,6 @@ Example:
 -> { "execute": "query-tpm-types" }
 <- { "return": [ "passthrough" ] }
 
-EQMP
-
-    {
-        .name       = "chardev-add",
-        .args_type  = "id:s,backend:q",
-    },
-
-SQMP
 chardev-add
 ----------------
 
@@ -3940,15 +3033,6 @@ Examples:
                      "backend" : { "type" : "pty", "data" : {} } } }
 <- { "return": { "pty" : "/dev/pty/42" } }
 
-EQMP
-
-    {
-        .name       = "chardev-remove",
-        .args_type  = "id:s",
-    },
-
-
-SQMP
 chardev-remove
 --------------
 
@@ -3963,13 +3047,6 @@ Example:
 -> { "execute": "chardev-remove", "arguments": { "id" : "foo" } }
 <- { "return": {} }
 
-EQMP
-    {
-        .name       = "query-rx-filter",
-        .args_type  = "name:s?",
-    },
-
-SQMP
 query-rx-filter
 ---------------
 
@@ -4027,14 +3104,6 @@ Example:
       ]
    }
 
-EQMP
-
-    {
-        .name       = "blockdev-add",
-        .args_type  = "options:q",
-    },
-
-SQMP
 blockdev-add
 ------------
 
@@ -4085,14 +3154,6 @@ Example (2):
 
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "x-blockdev-del",
-        .args_type  = "id:s?,node-name:s?",
-    },
-
-SQMP
 x-blockdev-del
 ------------
 Since 2.5
@@ -4141,14 +3202,6 @@ Example:
    }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-open-tray",
-        .args_type  = "device:s,force:b?",
-    },
-
-SQMP
 blockdev-open-tray
 ------------------
 
@@ -4188,14 +3241,6 @@ Example:
 
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "blockdev-close-tray",
-        .args_type  = "device:s",
-    },
-
-SQMP
 blockdev-close-tray
 -------------------
 
@@ -4222,14 +3267,6 @@ Example:
 
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "x-blockdev-remove-medium",
-        .args_type  = "device:s",
-    },
-
-SQMP
 x-blockdev-remove-medium
 ------------------------
 
@@ -4269,14 +3306,6 @@ Example:
 
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "x-blockdev-insert-medium",
-        .args_type  = "device:s,node-name:s",
-    },
-
-SQMP
 x-blockdev-insert-medium
 ------------------------
 
@@ -4308,14 +3337,6 @@ Example:
 
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "x-blockdev-change",
-        .args_type  = "parent:B,child:B?,node:B?",
-    },
-
-SQMP
 x-blockdev-change
 -----------------
 
@@ -4360,14 +3381,6 @@ Delete a quorum's node
                     "child": "children.1" } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "query-named-block-nodes",
-        .args_type  = "",
-    },
-
-SQMP
 @query-named-block-nodes
 ------------------------
 
@@ -4421,14 +3434,6 @@ Example:
                       }
                    } } ] }
 
-EQMP
-
-    {
-        .name       = "blockdev-change-medium",
-        .args_type  = "device:B,filename:F,format:s?,read-only-mode:s?",
-    },
-
-SQMP
 blockdev-change-medium
 ----------------------
 
@@ -4473,14 +3478,6 @@ Examples:
 
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "query-memdev",
-        .args_type  = "",
-    },
-
-SQMP
 query-memdev
 ------------
 
@@ -4510,14 +3507,6 @@ Example (1):
      ]
    }
 
-EQMP
-
-    {
-        .name       = "query-memory-devices",
-        .args_type  = "",
-    },
-
-SQMP
 @query-memory-devices
 --------------------
 
@@ -4536,14 +3525,6 @@ Example:
                         "slot": 0},
                    "type": "dimm"
                  } ] }
-EQMP
-
-    {
-        .name       = "query-acpi-ospm-status",
-        .args_type  = "",
-    },
-
-SQMP
 @query-acpi-ospm-status
 --------------------
 
@@ -4557,16 +3538,6 @@ Example:
                  { "slot": "2", "slot-type": "DIMM", "source": 0, "status": 0},
                  { "slot": "3", "slot-type": "DIMM", "source": 0, "status": 0}
    ]}
-EQMP
-
-#if defined TARGET_I386
-    {
-        .name       = "rtc-reset-reinjection",
-        .args_type  = "",
-    },
-#endif
-
-SQMP
 rtc-reset-reinjection
 ---------------------
 
@@ -4578,14 +3549,6 @@ Example:
 
 -> { "execute": "rtc-reset-reinjection" }
 <- { "return": {} }
-EQMP
-
-    {
-        .name       = "trace-event-get-state",
-        .args_type  = "name:s,vcpu:i?",
-    },
-
-SQMP
 trace-event-get-state
 ---------------------
 
@@ -4609,14 +3572,6 @@ Example:
 
 -> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
 <- { "return": [ { "name": "qemu_memalign", "state": "disabled" } ] }
-EQMP
-
-    {
-        .name       = "trace-event-set-state",
-        .args_type  = "name:s,enable:b,ignore-unavailable:b?,vcpu:i?",
-    },
-
-SQMP
 trace-event-set-state
 ---------------------
 
@@ -4643,14 +3598,6 @@ Example:
 
 -> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } }
 <- { "return": {} }
-EQMP
-
-    {
-        .name       = "input-send-event",
-        .args_type  = "console:i?,events:q",
-    },
-
-SQMP
 @input-send-event
 -----------------
 
@@ -4708,14 +3655,6 @@ Move mouse pointer to absolute coordinates (20000, 400).
                { "type": "abs", "data" : { "axis": "y", "value" : 400 } } ] } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "block-set-write-threshold",
-        .args_type  = "node-name:s,write-threshold:l",
-    },
-
-SQMP
 block-set-write-threshold
 ------------
 
@@ -4735,14 +3674,6 @@ Example:
                  "write-threshold": 17179869184 } }
 <- { "return": {} }
 
-EQMP
-
-    {
-        .name       = "query-rocker",
-        .args_type  = "name:s",
-    },
-
-SQMP
 Show rocker switch
 ------------------
 
@@ -4755,14 +3686,6 @@ Example:
 -> { "execute": "query-rocker", "arguments": { "name": "sw1" } }
 <- { "return": {"name": "sw1", "ports": 2, "id": 1327446905938}}
 
-EQMP
-
-    {
-        .name       = "query-rocker-ports",
-        .args_type  = "name:s",
-    },
-
-SQMP
 Show rocker switch ports
 ------------------------
 
@@ -4779,14 +3702,6 @@ Example:
                   "autoneg": "off", "link-up": true, "speed": 10000}
    ]}
 
-EQMP
-
-    {
-        .name       = "query-rocker-of-dpa-flows",
-        .args_type  = "name:s,tbl-id:i?",
-    },
-
-SQMP
 Show rocker switch OF-DPA flow tables
 -------------------------------------
 
@@ -4807,14 +3722,6 @@ Example:
                  {...more...},
    ]}
 
-EQMP
-
-    {
-        .name       = "query-rocker-of-dpa-groups",
-        .args_type  = "name:s,type:i?",
-    },
-
-SQMP
 Show rocker OF-DPA group tables
 -------------------------------
 
@@ -4836,16 +3743,6 @@ Example:
                   "pop-vlan": 1, "id": 251658240}
    ]}
 
-EQMP
-
-#if defined TARGET_ARM
-    {
-        .name       = "query-gic-capabilities",
-        .args_type  = "",
-    },
-#endif
-
-SQMP
 query-gic-capabilities
 ---------------
 
@@ -4860,14 +3757,6 @@ Example:
 <- { "return": [{ "version": 2, "emulated": true, "kernel": false },
                 { "version": 3, "emulated": false, "kernel": true } ] }
 
-EQMP
-
-    {
-        .name       = "query-hotpluggable-cpus",
-        .args_type  = "",
-    },
-
-SQMP
 Show existing/possible CPUs
 ---------------------------
 
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index c425393..cfa6fe7 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -119,16 +119,6 @@ There are a few things to be noticed:
 5. Printing to the terminal is discouraged for QMP commands, we do it here
    because it's the easiest way to demonstrate a QMP command
 
-Now a little hack is needed. As we're still using the old QMP server we need
-to add the new command to its internal dispatch table. This step won't be
-required in the near future. Open the qmp-commands.hx file and add the
-following at the bottom:
-
-    {
-        .name       = "hello-world",
-        .args_type  = "",
-    },
-
 You're done. Now build qemu, run it as suggested in the "Testing" section,
 and then type the following QMP command:
 
@@ -173,20 +163,6 @@ There are two important details to be noticed:
 2. The C implementation signature must follow the schema's argument ordering,
    which is defined by the "data" member
 
-The last step is to update the qmp-commands.hx file:
-
-    {
-        .name       = "hello-world",
-        .args_type  = "message:s?",
-    },
-
-Notice that the "args_type" member got our "message" argument. The character
-"s" stands for "string" and "?" means it's optional. This too must be ordered
-according to the C implementation and schema file. You can look for more
-examples in the qmp-commands.hx file if you need to define more arguments.
-
-Again, this step won't be required in the future.
-
 Time to test our new version of the "hello-world" command. Build qemu, run it as
 described in the "Testing" section and then send two commands:
 
@@ -452,13 +428,6 @@ There are a number of things to be noticed:
 6. You have to include the "qmp-commands.h" header file in qemu-timer.c,
    otherwise qemu won't build
 
-The last step is to add the corresponding entry in the qmp-commands.hx file:
-
-    {
-        .name       = "query-alarm-clock",
-        .args_type  = "",
-    },
-
 Time to test the new command. Build qemu, run it as described in the "Testing"
 section and try this:
 
@@ -597,13 +566,6 @@ iteration of the loop. That's because the alarm timer method in use is the
 first element of the alarm_timers array. Also notice that QAPI lists are handled
 by hand and we return the head of the list.
 
-To test this you have to add the corresponding qmp-commands.hx entry:
-
-    {
-        .name       = "query-alarm-methods",
-        .args_type  = "",
-    },
-
 Now Build qemu, run it as explained in the "Testing" section and try our new
 command:
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Qemu-devel] [PATCH v4 17/17] qmp-commands.txt: fix some styling
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (15 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 16/17] Replace qmp-commands.hx by doc/qmp-commands.txt marcandre.lureau
@ 2016-08-10 18:02 ` marcandre.lureau
  2016-08-11  4:45 ` [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode no-reply
  17 siblings, 0 replies; 32+ messages in thread
From: marcandre.lureau @ 2016-08-10 18:02 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, armbru, Marc-André Lureau

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add some missing lines, remove superflous @ in command name, remove
trailing spaces.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 docs/qmp-commands.txt | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt
index dfa1cc1..abd6969 100644
--- a/docs/qmp-commands.txt
+++ b/docs/qmp-commands.txt
@@ -70,7 +70,7 @@ eject
 
 Eject a removable medium.
 
-Arguments: 
+Arguments:
 
 - force: force ejection (json-bool, optional)
 - device: device name (json-string)
@@ -1562,7 +1562,7 @@ human-monitor-command
 
 Execute a Human Monitor command.
 
-Arguments: 
+Arguments:
 
 - command-line: the command name and its arguments, just like the
                 Human Monitor's shell (json-string)
@@ -2288,7 +2288,7 @@ Example:
                   },
                   "function":0,
                   "regions":[
-   
+
                   ]
                },
                {
@@ -2305,7 +2305,7 @@ Example:
                   },
                   "function":0,
                   "regions":[
-   
+
                   ]
                },
                {
@@ -2689,8 +2689,8 @@ The main json-object contains the following:
 - "setup-time" amount of setup time in milliseconds _before_ the
                iterations begin but _after_ the QMP command is issued.
                This is designed to provide an accounting of any activities
-               (such as RDMA pinning) which may be expensive, but do not 
-               actually occur during the iterative migration rounds 
+               (such as RDMA pinning) which may be expensive, but do not
+               actually occur during the iterative migration rounds
                themselves. (json-int)
 - "downtime": only present when migration has finished correctly
               total amount in ms for downtime that happened (json-int)
@@ -3381,8 +3381,8 @@ Delete a quorum's node
                     "child": "children.1" } }
 <- { "return": {} }
 
-@query-named-block-nodes
-------------------------
+query-named-block-nodes
+-----------------------
 
 Return a list of BlockDeviceInfo for all the named block driver nodes
 
@@ -3507,7 +3507,7 @@ Example (1):
      ]
    }
 
-@query-memory-devices
+query-memory-devices
 --------------------
 
 Return a list of memory devices.
@@ -3525,8 +3525,9 @@ Example:
                         "slot": 0},
                    "type": "dimm"
                  } ] }
-@query-acpi-ospm-status
---------------------
+
+query-acpi-ospm-status
+----------------------
 
 Return list of ACPIOSTInfo for devices that support status reporting
 via ACPI _OST method.
@@ -3538,6 +3539,7 @@ Example:
                  { "slot": "2", "slot-type": "DIMM", "source": 0, "status": 0},
                  { "slot": "3", "slot-type": "DIMM", "source": 0, "status": 0}
    ]}
+
 rtc-reset-reinjection
 ---------------------
 
@@ -3549,6 +3551,7 @@ Example:
 
 -> { "execute": "rtc-reset-reinjection" }
 <- { "return": {} }
+
 trace-event-get-state
 ---------------------
 
@@ -3572,6 +3575,7 @@ Example:
 
 -> { "execute": "trace-event-get-state", "arguments": { "name": "qemu_memalign" } }
 <- { "return": [ { "name": "qemu_memalign", "state": "disabled" } ] }
+
 trace-event-set-state
 ---------------------
 
@@ -3598,8 +3602,9 @@ Example:
 
 -> { "execute": "trace-event-set-state", "arguments": { "name": "qemu_memalign", "enable": "true" } }
 <- { "return": {} }
-@input-send-event
------------------
+
+input-send-event
+----------------
 
 Send input event to guest.
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode
  2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
                   ` (16 preceding siblings ...)
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 17/17] qmp-commands.txt: fix some styling marcandre.lureau
@ 2016-08-11  4:45 ` no-reply
  2016-08-17 15:03   ` Markus Armbruster
  17 siblings, 1 reply; 32+ messages in thread
From: no-reply @ 2016-08-11  4:45 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: famz, qemu-devel, armbru

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode
Message-id: 20160810180235.32469-1-marcandre.lureau@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20160810180235.32469-1-marcandre.lureau@redhat.com -> patchew/20160810180235.32469-1-marcandre.lureau@redhat.com
Switched to a new branch 'test'
11826ee qmp-commands.txt: fix some styling
efdce7f Replace qmp-commands.hx by doc/qmp-commands.txt
59685a4 build-sys: remove qmp-commands-old.h
43a8282 monitor: use qmp_dispatch()
ee6f707 qmp: update qmp_query_spice fallback
be382f6 qapi: check invalid arguments on no-args commands
0d01f13 qapi: remove the "middle" mode
d8a1541 monitor: remove mhandler.cmd_new
a489768 monitor: implement 'qmp_query_commands' without qmp_cmds
d4647e8 monitor: use qmp_find_command() (using generated qapi code)
3616d53 qapi: export the marshallers
a704864 monitor: unregister conditional commands
fae5991 monitor: register gen:false commands manually
6b689b4 monitor: simplify invalid_qmp_mode()
3952e05 qapi-schema: add 'device_add'
5481a61 qapi-schema: use generated marshaller for 'qmp_capabilities'
d3cbddf build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO}

=== OUTPUT BEGIN ===
Checking PATCH 1/17: build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO}...
Checking PATCH 2/17: qapi-schema: use generated marshaller for 'qmp_capabilities'...
Checking PATCH 3/17: qapi-schema: add 'device_add'...
Checking PATCH 4/17: monitor: simplify invalid_qmp_mode()...
Checking PATCH 5/17: monitor: register gen:false commands manually...
Checking PATCH 6/17: monitor: unregister conditional commands...
Checking PATCH 7/17: qapi: export the marshallers...
WARNING: line over 80 characters
#28: FILE: scripts/qapi-commands.py:87:
+    return 'void qmp_marshal_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)

total: 0 errors, 1 warnings, 20 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 8/17: monitor: use qmp_find_command() (using generated qapi code)...
Checking PATCH 9/17: monitor: implement 'qmp_query_commands' without qmp_cmds...
Checking PATCH 10/17: monitor: remove mhandler.cmd_new...
Checking PATCH 11/17: qapi: remove the "middle" mode...
Checking PATCH 12/17: qapi: check invalid arguments on no-args commands...
Checking PATCH 13/17: qmp: update qmp_query_spice fallback...
Checking PATCH 14/17: monitor: use qmp_dispatch()...
Checking PATCH 15/17: build-sys: remove qmp-commands-old.h...
Checking PATCH 16/17: Replace qmp-commands.hx by doc/qmp-commands.txt...
ERROR: trailing whitespace
#168: FILE: docs/qmp-commands.txt:73:
+Arguments: $

ERROR: trailing whitespace
#1660: FILE: docs/qmp-commands.txt:1565:
+Arguments: $

ERROR: trailing whitespace
#2386: FILE: docs/qmp-commands.txt:2291:
+   $

ERROR: trailing whitespace
#2403: FILE: docs/qmp-commands.txt:2308:
+   $

ERROR: trailing whitespace
#2787: FILE: docs/qmp-commands.txt:2692:
+               (such as RDMA pinning) which may be expensive, but do not $

ERROR: trailing whitespace
#2788: FILE: docs/qmp-commands.txt:2693:
+               actually occur during the iterative migration rounds $

total: 6 errors, 0 warnings, 3901 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 17/17: qmp-commands.txt: fix some styling...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 05/17] monitor: register gen:false commands manually
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 05/17] monitor: register gen:false commands manually marcandre.lureau
@ 2016-08-16 12:27   ` Markus Armbruster
  0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-08-16 12:27 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Since a few commands are using 'gen': false, they are not registered
> automatically by the generator. Register manually instead.
>
> This is in preparation for removal of qapi 'middle' mode generation.
>
> Note that qmp_init_marshal() function isn't run yet, so the commands
> aren't registered, until module_call_init(MODULE_INIT_QAPI) is added in

Suggest "aren't actually registered".  Can touch up on commit.

> a later patch.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Patch looks good.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands marcandre.lureau
@ 2016-08-16 14:26   ` Markus Armbruster
  2016-08-16 14:34     ` Marc-André Lureau
  2016-08-17 13:36   ` Markus Armbruster
  1 sibling, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-08-16 14:26 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The current monitor dispatch codes doesn't know commands that have been
> filtered out during qmp-commands.hx preprocessing. query-commands
> doesn't list them either. However, qapi generator doesn't filter them
> out, and they are listed in the command list.
>
> For now, disable the commands that aren't avaible to avoid introducing a
> regression there when switching to qmp_dispatch() or listing commands
> from the generated qapi code.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  monitor.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index b7ae552..ef946ad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
>      *ret_data = qobject_from_json(qmp_schema_json);
>  }
>  
> +/*
> + * Those commands are registered unconditionnally by generated
> + * qmp files. FIXME: Educate the QAPI schema on #ifdef commands.
> + */
> +static void qmp_disable_marshal(void)
> +{
> +#ifndef CONFIG_SPICE
> +    qmp_disable_command("query-spice");
> +#endif
> +#ifndef TARGET_I386
> +    qmp_disable_command("rtc-reset-reinjection");
> +#endif
> +#ifndef TARGET_S390X
> +    qmp_disable_command("dump-skeys");
> +#endif
> +#ifndef TARGET_ARM
> +    qmp_disable_command("query-gic-capabilities");
> +#endif
> +}
> +
>  static void qmp_init_marshal(void)
>  {
>      qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
> @@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void)
>                           QCO_NO_OPTIONS);
>      qmp_register_command("netdev_add", qmp_netdev_add,
>                           QCO_NO_OPTIONS);
> +
> +    /* call it after the rest of qapi_init() */
> +    register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI);
>  }
>  
>  qapi_init(qmp_init_marshal);

Let's see whether I understand this patch...

qmp_disable_command() sets a flag that can be tested directly or with
qmp_command_is_enabled().  do_qmp_dispatch() tests it, and fails the
command with an "The command FOO has been disabled for this instance"
error.  It's called from qmp_dispatch(), which we're not yet using for
QMP at this point of the series.

QGA uses this to implement its "freeze whitelist", i.e. to disable all
commands that aren't known to be safe while filesystems are frozen.

qmp_disable_command() does nothing when the command doesn't exist, which
I think is the case at this point of the series.

So this patch does exactly nothing by itself.  When you later flip
dispatch to use qmp_dispatch(), it becomes active, and the error for
these commands changes from

    {"error": {"class": "CommandNotFound", "desc": "The command FOO has not been found"}}

to

    {"error": {"class": "GenericError", "desc": "The command FOO has been disabled for this instance"}}

which is fine with me.

Is this basically correct?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands
  2016-08-16 14:26   ` Markus Armbruster
@ 2016-08-16 14:34     ` Marc-André Lureau
  2016-08-16 15:32       ` Markus Armbruster
  2016-09-09 13:59       ` Markus Armbruster
  0 siblings, 2 replies; 32+ messages in thread
From: Marc-André Lureau @ 2016-08-16 14:34 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: marcandre lureau, qemu-devel

Hi

----- Original Message -----
> marcandre.lureau@redhat.com writes:
> 
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The current monitor dispatch codes doesn't know commands that have been
> > filtered out during qmp-commands.hx preprocessing. query-commands
> > doesn't list them either. However, qapi generator doesn't filter them
> > out, and they are listed in the command list.
> >
> > For now, disable the commands that aren't avaible to avoid introducing a
> > regression there when switching to qmp_dispatch() or listing commands
> > from the generated qapi code.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  monitor.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/monitor.c b/monitor.c
> > index b7ae552..ef946ad 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict,
> > QObject **ret_data,
> >      *ret_data = qobject_from_json(qmp_schema_json);
> >  }
> >  
> > +/*
> > + * Those commands are registered unconditionnally by generated
> > + * qmp files. FIXME: Educate the QAPI schema on #ifdef commands.
> > + */
> > +static void qmp_disable_marshal(void)
> > +{
> > +#ifndef CONFIG_SPICE
> > +    qmp_disable_command("query-spice");
> > +#endif
> > +#ifndef TARGET_I386
> > +    qmp_disable_command("rtc-reset-reinjection");
> > +#endif
> > +#ifndef TARGET_S390X
> > +    qmp_disable_command("dump-skeys");
> > +#endif
> > +#ifndef TARGET_ARM
> > +    qmp_disable_command("query-gic-capabilities");
> > +#endif
> > +}
> > +
> >  static void qmp_init_marshal(void)
> >  {
> >      qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
> > @@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void)
> >                           QCO_NO_OPTIONS);
> >      qmp_register_command("netdev_add", qmp_netdev_add,
> >                           QCO_NO_OPTIONS);
> > +
> > +    /* call it after the rest of qapi_init() */
> > +    register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI);
> >  }
> >  
> >  qapi_init(qmp_init_marshal);
> 
> Let's see whether I understand this patch...
> 
> qmp_disable_command() sets a flag that can be tested directly or with
> qmp_command_is_enabled().  do_qmp_dispatch() tests it, and fails the
> command with an "The command FOO has been disabled for this instance"
> error.  It's called from qmp_dispatch(), which we're not yet using for
> QMP at this point of the series.
> 
> QGA uses this to implement its "freeze whitelist", i.e. to disable all
> commands that aren't known to be safe while filesystems are frozen.
> 
> qmp_disable_command() does nothing when the command doesn't exist, which
> I think is the case at this point of the series.
> 
> So this patch does exactly nothing by itself.  When you later flip
> dispatch to use qmp_dispatch(), it becomes active, and the error for
> these commands changes from
> 
>     {"error": {"class": "CommandNotFound", "desc": "The command FOO has not
>     been found"}}
> 
> to
> 
>     {"error": {"class": "GenericError", "desc": "The command FOO has been
>     disabled for this instance"}}
> 
> which is fine with me.
> 
> Is this basically correct?

I think it's correct, I haven't played much with this as I focused on the conditional build instead, which is actually not so difficult, see:

https://github.com/elmarco/qemu/commit/d0496fdabe49149e76d765d96feada0d5269c211 (qapi.py change)

https://github.com/elmarco/qemu/commit/ee3c17b7bd5ffaa0b5be258a2cfb46c6c577f653 (per target build)

https://github.com/elmarco/qemu/commit/91a692b3428878f8c26933c8a113853fb2a19970 (per target conditonal in schema)

Do you think I should introduce that conditional build earlier in this series instead of that disable workaround?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands
  2016-08-16 14:34     ` Marc-André Lureau
@ 2016-08-16 15:32       ` Markus Armbruster
  2016-09-09 13:59       ` Markus Armbruster
  1 sibling, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-08-16 15:32 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: marcandre lureau, qemu-devel

Marc-André Lureau <mlureau@redhat.com> writes:

> Hi
>
> ----- Original Message -----
>> marcandre.lureau@redhat.com writes:
>> 
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > The current monitor dispatch codes doesn't know commands that have been
>> > filtered out during qmp-commands.hx preprocessing. query-commands
>> > doesn't list them either. However, qapi generator doesn't filter them
>> > out, and they are listed in the command list.
>> >
>> > For now, disable the commands that aren't avaible to avoid introducing a
>> > regression there when switching to qmp_dispatch() or listing commands
>> > from the generated qapi code.
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  monitor.c | 23 +++++++++++++++++++++++
>> >  1 file changed, 23 insertions(+)
>> >
>> > diff --git a/monitor.c b/monitor.c
>> > index b7ae552..ef946ad 100644
>> > --- a/monitor.c
>> > +++ b/monitor.c
>> > @@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict,
>> > QObject **ret_data,
>> >      *ret_data = qobject_from_json(qmp_schema_json);
>> >  }
>> >  
>> > +/*
>> > + * Those commands are registered unconditionnally by generated
>> > + * qmp files. FIXME: Educate the QAPI schema on #ifdef commands.
>> > + */
>> > +static void qmp_disable_marshal(void)
>> > +{
>> > +#ifndef CONFIG_SPICE
>> > +    qmp_disable_command("query-spice");
>> > +#endif
>> > +#ifndef TARGET_I386
>> > +    qmp_disable_command("rtc-reset-reinjection");
>> > +#endif
>> > +#ifndef TARGET_S390X
>> > +    qmp_disable_command("dump-skeys");
>> > +#endif
>> > +#ifndef TARGET_ARM
>> > +    qmp_disable_command("query-gic-capabilities");
>> > +#endif
>> > +}
>> > +
>> >  static void qmp_init_marshal(void)
>> >  {
>> >      qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
>> > @@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void)
>> >                           QCO_NO_OPTIONS);
>> >      qmp_register_command("netdev_add", qmp_netdev_add,
>> >                           QCO_NO_OPTIONS);
>> > +
>> > +    /* call it after the rest of qapi_init() */
>> > +    register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI);
>> >  }
>> >  
>> >  qapi_init(qmp_init_marshal);
>> 
>> Let's see whether I understand this patch...
>> 
>> qmp_disable_command() sets a flag that can be tested directly or with
>> qmp_command_is_enabled().  do_qmp_dispatch() tests it, and fails the
>> command with an "The command FOO has been disabled for this instance"
>> error.  It's called from qmp_dispatch(), which we're not yet using for
>> QMP at this point of the series.
>> 
>> QGA uses this to implement its "freeze whitelist", i.e. to disable all
>> commands that aren't known to be safe while filesystems are frozen.
>> 
>> qmp_disable_command() does nothing when the command doesn't exist, which
>> I think is the case at this point of the series.
>> 
>> So this patch does exactly nothing by itself.  When you later flip
>> dispatch to use qmp_dispatch(), it becomes active, and the error for
>> these commands changes from
>> 
>>     {"error": {"class": "CommandNotFound", "desc": "The command FOO has not
>>     been found"}}
>> 
>> to
>> 
>>     {"error": {"class": "GenericError", "desc": "The command FOO has been
>>     disabled for this instance"}}
>> 
>> which is fine with me.
>> 
>> Is this basically correct?
>
> I think it's correct, I haven't played much with this as I focused on the conditional build instead, which is actually not so difficult, see:
>
> https://github.com/elmarco/qemu/commit/d0496fdabe49149e76d765d96feada0d5269c211 (qapi.py change)
>
> https://github.com/elmarco/qemu/commit/ee3c17b7bd5ffaa0b5be258a2cfb46c6c577f653 (per target build)
>
> https://github.com/elmarco/qemu/commit/91a692b3428878f8c26933c8a113853fb2a19970 (per target conditonal in schema)
>
> Do you think I should introduce that conditional build earlier in this series instead of that disable workaround?

Doing the real thing right away is less churn than workaround first,
real thing later.  On the other hand, workarounds can help you may
shorten your patch queue.  Do what you think is best.  I'll review
whatever you throw at me :)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds marcandre.lureau
@ 2016-08-16 16:22   ` Markus Armbruster
  0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-08-16 16:22 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> One step towards getting rid of the static qmp_cmds table.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  monitor.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 2050698..cddc737 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -957,21 +957,28 @@ static void hmp_info_help(Monitor *mon, const QDict *qdict)
>      help_cmd(mon, "info");
>  }
>  
> -CommandInfoList *qmp_query_commands(Error **errp)
> +static void query_commands_cb(QmpCommand *cmd, void *opaque)
>  {
> -    CommandInfoList *info, *cmd_list = NULL;
> -    const mon_cmd_t *cmd;
> -
> -    for (cmd = qmp_cmds; cmd->name != NULL; cmd++) {
> -        info = g_malloc0(sizeof(*info));
> -        info->value = g_malloc0(sizeof(*info->value));
> -        info->value->name = g_strdup(cmd->name);
> +    CommandInfoList *info, **list = opaque;
>  
> -        info->next = cmd_list;
> -        cmd_list = info;
> +    if (!cmd->enabled) {
> +        return;
>      }
>  
> -    return cmd_list;
> +    info = g_malloc0(sizeof(*info));
> +    info->value = g_malloc0(sizeof(*info->value));
> +    info->value->name = g_strdup(cmd->name);
> +    info->next = *list;
> +    *list = info;
> +}
> +
> +CommandInfoList *qmp_query_commands(Error **errp)
> +{
> +    CommandInfoList *list = NULL;
> +
> +    qmp_for_each_command(query_commands_cb, &list);
> +
> +    return list;
>  }
>  
>  EventInfoList *qmp_query_events(Error **errp)

This patch flips query-commands from qmp-commands.hx / qmp_cmds[] to
QAPI / qmp_for_each_command().  PATCH 06 "monitor: unregister
conditional commands" is needed here to keep the compile-time
conditional commands out of query-commands.

The previous patch started the transition from qmp-commands.hx /
qmp_cmds[] to QAPI / qmp_find_command().  It'll be completed in patch
14.  PATCH 06 will be needed then so we keep rejecting the compile-time
conditional commands.

The compile-time conditional commands remain in query-qmp-schema despite
PATCH 06.  No change.  Okay.

PATCH 06's commit message could perhaps spell out these connections more
clearly.  Here's my attempt:

    monitor: unregister conditional commands

    QMP commands are currently defined in two places: the QAPI schema
    and qmp-commands.hx.

    qmp-commands.hx uses the C preprocessor to define a number of
    commands only conditionally.  For instance, query-spice is #ifdef
    CONFIG_SPICE.

    The QAPI schema does no such thing.

    The current QMP command dispatch and query-commands use a command
    table generated from qmp-commands.hx.  This table contains the
    conditionally defined commands only when their conditions are met.
    For instance, query-spice is accepted by command dispatch and shown
    by query-commands only when CONFIG_SPICE is defined.

    In contrast, query-qmp-schema uses the QAPI schema, and shows
    conditional commands whether they're defined or not.

    We're going to change QMP command dispatch and query-commands to use
    QAPI, so we can define QMP commands in just one place.  We need to
    do something to avoid changing dispatch and query-spice.

    Fortunately, a suitable mechanism already exists: we can make
    commands disabled.  Do that now.  The patches that change dispatch
    and query-commands will use it to avoid undue change.

Feel free to edit to taste.  If you're happy with it as is, I can
replace the message on commit.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands marcandre.lureau
  2016-08-16 14:26   ` Markus Armbruster
@ 2016-08-17 13:36   ` Markus Armbruster
  1 sibling, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-08-17 13:36 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The current monitor dispatch codes doesn't know commands that have been
> filtered out during qmp-commands.hx preprocessing. query-commands
> doesn't list them either. However, qapi generator doesn't filter them
> out, and they are listed in the command list.
>
> For now, disable the commands that aren't avaible to avoid introducing a
> regression there when switching to qmp_dispatch() or listing commands
> from the generated qapi code.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  monitor.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index b7ae552..ef946ad 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
>      *ret_data = qobject_from_json(qmp_schema_json);
>  }
>  
> +/*
> + * Those commands are registered unconditionnally by generated

"unconditionally".  Can touch up on commit.

> + * qmp files. FIXME: Educate the QAPI schema on #ifdef commands.
> + */
> +static void qmp_disable_marshal(void)
> +{
> +#ifndef CONFIG_SPICE
> +    qmp_disable_command("query-spice");
> +#endif
> +#ifndef TARGET_I386
> +    qmp_disable_command("rtc-reset-reinjection");
> +#endif
> +#ifndef TARGET_S390X
> +    qmp_disable_command("dump-skeys");
> +#endif
> +#ifndef TARGET_ARM
> +    qmp_disable_command("query-gic-capabilities");
> +#endif
> +}
> +
>  static void qmp_init_marshal(void)
>  {
>      qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
> @@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void)
>                           QCO_NO_OPTIONS);
>      qmp_register_command("netdev_add", qmp_netdev_add,
>                           QCO_NO_OPTIONS);
> +
> +    /* call it after the rest of qapi_init() */
> +    register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI);
>  }
>  
>  qapi_init(qmp_init_marshal);

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands marcandre.lureau
@ 2016-08-17 14:47   ` Markus Armbruster
  2016-08-17 15:49     ` Marc-André Lureau
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-08-17 14:47 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The generated marshal functions do not visit arguments from commands
> that take no arguments. Thus they fail to catch invalid
> members. Visit the arguments, if provided, to throw an error in case of
> invalid members.
>
> Currently, qmp_check_client_args() checks for invalid arguments and
> correctly catches this case. When switching to qmp_dispatch() we want to
> keep that behaviour. The commands using 'O' may have arbitrary
> arguments, and must have 'gen': false in the qapi schema to skip the
> generated checks.

Explains why this isn't a bug fix for QMP.  What about QGA?

> Old/new diff:
> static void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp)
>  {
> +    Visitor *v = NULL;
>      Error *err = NULL;
> -

Please keep the blank line between declarations and statements.

> -    (void)args;
> +    if (args) {

This code...

> +        v = qmp_input_visitor_new(QOBJECT(args), true);
> +        visit_start_struct(v, NULL, NULL, 0, &err);
> +        if (err) {
> +            goto out;
> +        }
> +
> +        if (!err) {
> +            visit_check_struct(v, &err);
> +        }
> +        visit_end_struct(v, NULL);
> +        if (err) {
> +            goto out;
> +        }

... is not indented in my build.

> +    }
>
>      qmp_stop(&err);
> +
> +out:
>      error_propagate(errp, err);
> +    visit_free(v);
> +    if (args) {
> +        v = qapi_dealloc_visitor_new();
> +        visit_start_struct(v, NULL, NULL, 0, NULL);
> +
> +        visit_end_struct(v, NULL);
> +        visit_free(v);

Likewise.

> +    }
>  }
>
> The new code closely resembles code for a command with arguments.
> Differences:
> - the visit of the argument and its cleanup struct don't visit any
>   members (because there are none).
> - the visit of the argument struct and its cleanup are conditional.

Additional diff for all other qmp_marshal_FOO():

  @@ -46,9 +46,9 @@ static void qmp_marshal_output_AddfdInfo

   void qmp_marshal_add_fd(QDict *args, QObject **ret, Error **errp)
   {
  +    Visitor *v = NULL;
       Error *err = NULL;
       AddfdInfo *retval;
  -    Visitor *v;
       q_obj_add_fd_arg arg = {0};

       v = qmp_input_visitor_new(QOBJECT(args), true);

Let's avoid this churn.

>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/test-qmp-commands.c | 15 ++++++++++++++
>  scripts/qapi-commands.py  | 53 ++++++++++++++++++++++++++++++-----------------
>  2 files changed, 49 insertions(+), 19 deletions(-)
>
> diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> index 261fd9e..81cbe54 100644
> --- a/tests/test-qmp-commands.c
> +++ b/tests/test-qmp-commands.c
> @@ -106,6 +106,7 @@ static void test_dispatch_cmd(void)
>  static void test_dispatch_cmd_failure(void)
>  {
>      QDict *req = qdict_new();
> +    QDict *args = qdict_new();
>      QObject *resp;
>  
>      qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
> @@ -114,6 +115,20 @@ static void test_dispatch_cmd_failure(void)
>      assert(resp != NULL);
>      assert(qdict_haskey(qobject_to_qdict(resp), "error"));
>  
> +    qobject_decref(resp);
> +    QDECREF(req);
> +
> +    /* check that with extra arguments it throws an error */
> +    req = qdict_new();
> +    qdict_put(args, "a", qint_from_int(66));
> +    qdict_put(req, "arguments", args);
> +
> +    qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
> +
> +    resp = qmp_dispatch(QOBJECT(req));
> +    assert(resp != NULL);
> +    assert(qdict_haskey(qobject_to_qdict(resp), "error"));
> +
>      qobject_decref(resp);
>      QDECREF(req);
>  }
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index eac64ce..9c79b18 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -94,11 +94,28 @@ def gen_marshal_decl(name):
>                   proto=gen_marshal_proto(name))
>  
>  
> +def if_args(arg_type, block):
> +    ret = ''
> +    if not arg_type or arg_type.is_empty():
> +        ret += mcgen('''
> +    if (args) {
> +''')
> +        push_indent()
> +    ret += block
> +    if not arg_type or arg_type.is_empty():
> +        pop_indent()
> +        ret += mcgen('''
> +    }
> +''')

Since @block has already been formatted, the push_indent() /
pop_indent() has no effect.  I guess that's why you did it with a lambda
in v3.

> +    return ret
> +
> +
>  def gen_marshal(name, arg_type, boxed, ret_type):
>      ret = mcgen('''
>  
>  %(proto)s
>  {
> +    Visitor *v = NULL;
>      Error *err = NULL;
>  ''',
>                  proto=gen_marshal_proto(name))
> @@ -109,17 +126,23 @@ def gen_marshal(name, arg_type, boxed, ret_type):
>  ''',
>                       c_type=ret_type.c_type())
>  
> +    visit_members = ''
>      if arg_type and not arg_type.is_empty():
> +        visit_members = 'visit_type_%s_members(v, &arg, &err);' % \
> +                        arg_type.c_name()

PEP8 prefers

           visit_members = ('visit_type_%s_members(v, &arg, &err);'
                             % arg_type.c_name())

>          ret += mcgen('''
> -    Visitor *v;
>      %(c_name)s arg = {0};
>  
> +''',
> +                     c_name=arg_type.c_name())
> +
> +    ret += if_args(arg_type, mcgen('''
>      v = qmp_input_visitor_new(QOBJECT(args), true);
>      visit_start_struct(v, NULL, NULL, 0, &err);
>      if (err) {
>          goto out;
>      }
> -    visit_type_%(c_name)s_members(v, &arg, &err);
> +    %(visit_members)s
>      if (!err) {
>          visit_check_struct(v, &err);
>      }
> @@ -128,35 +151,27 @@ def gen_marshal(name, arg_type, boxed, ret_type):
>          goto out;
>      }
>  ''',
> -                     c_name=arg_type.c_name())
> -
> -    else:
> -        ret += mcgen('''
> -
> -    (void)args;
> -''')
> +                                   visit_members=visit_members))
>  
>      ret += gen_call(name, arg_type, boxed, ret_type)
>  
> -    # 'goto out' produced above for arg_type, and by gen_call() for ret_type
> -    if (arg_type and not arg_type.is_empty()) or ret_type:
> -        ret += mcgen('''
> +    if arg_type and not arg_type.is_empty():
> +        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, NULL);',
> +                              c_name=arg_type.c_name())

I'm afraid you missed this instance of "mcgen()'s output fed to
mcgen()".

> +    ret += mcgen('''
>  
>  out:
> -''')
> -    ret += mcgen('''
>      error_propagate(errp, err);
> -''')
> -    if arg_type and not arg_type.is_empty():
> -        ret += mcgen('''
>      visit_free(v);
> +''')
> +    ret += if_args(arg_type, mcgen('''
>      v = qapi_dealloc_visitor_new();
>      visit_start_struct(v, NULL, NULL, 0, NULL);
> -    visit_type_%(c_name)s_members(v, &arg, NULL);
> +    %(visit_members)s
>      visit_end_struct(v, NULL);
>      visit_free(v);
>  ''',
> -                     c_name=arg_type.c_name())
> +                                   visit_members=visit_members))
>  
>      ret += mcgen('''
>  }

I append a diff from this one to a stupider solution.  It's slightly
longer, but I find it a bit easier to understand.



diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 9c79b18..2f603b0 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -94,28 +94,13 @@ def gen_marshal_decl(name):
                  proto=gen_marshal_proto(name))
 
 
-def if_args(arg_type, block):
-    ret = ''
-    if not arg_type or arg_type.is_empty():
-        ret += mcgen('''
-    if (args) {
-''')
-        push_indent()
-    ret += block
-    if not arg_type or arg_type.is_empty():
-        pop_indent()
-        ret += mcgen('''
-    }
-''')
-    return ret
-
-
 def gen_marshal(name, arg_type, boxed, ret_type):
+    have_args = arg_type and not arg_type.is_empty()
+
     ret = mcgen('''
 
 %(proto)s
 {
-    Visitor *v = NULL;
     Error *err = NULL;
 ''',
                 proto=gen_marshal_proto(name))
@@ -126,17 +111,25 @@ def gen_marshal(name, arg_type, boxed, ret_type):
 ''',
                      c_type=ret_type.c_type())
 
-    visit_members = ''
-    if arg_type and not arg_type.is_empty():
-        visit_members = 'visit_type_%s_members(v, &arg, &err);' % \
-                        arg_type.c_name()
+    if have_args:
+        visit_members = ('visit_type_%s_members(v, &arg, &err);'
+                         % arg_type.c_name())
         ret += mcgen('''
+    Visitor *v;
     %(c_name)s arg = {0};
 
 ''',
                      c_name=arg_type.c_name())
+    else:
+        visit_members = ''
+        ret += mcgen('''
+    Visitor *v = NULL;
 
-    ret += if_args(arg_type, mcgen('''
+    if (args) {
+''')
+        push_indent()
+
+    ret += mcgen('''
     v = qmp_input_visitor_new(QOBJECT(args), true);
     visit_start_struct(v, NULL, NULL, 0, &err);
     if (err) {
@@ -151,27 +144,47 @@ def gen_marshal(name, arg_type, boxed, ret_type):
         goto out;
     }
 ''',
-                                   visit_members=visit_members))
+                 visit_members=visit_members)
+
+    if not have_args:
+        pop_indent()
+        ret += mcgen('''
+    }
+''')
 
     ret += gen_call(name, arg_type, boxed, ret_type)
 
-    if arg_type and not arg_type.is_empty():
-        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, NULL);',
-                              c_name=arg_type.c_name())
     ret += mcgen('''
 
 out:
     error_propagate(errp, err);
     visit_free(v);
 ''')
-    ret += if_args(arg_type, mcgen('''
+
+    if have_args:
+        visit_members = ('visit_type_%s_members(v, &arg, NULL);'
+                         % arg_type.c_name())
+    else:
+        visit_members = ''
+        ret += mcgen('''
+    if (args) {
+''')
+        push_indent()
+
+    ret += mcgen('''
     v = qapi_dealloc_visitor_new();
     visit_start_struct(v, NULL, NULL, 0, NULL);
     %(visit_members)s
     visit_end_struct(v, NULL);
     visit_free(v);
 ''',
-                                   visit_members=visit_members))
+                 visit_members=visit_members)
+
+    if not have_args:
+        pop_indent()
+        ret += mcgen('''
+    }
+''')
 
     ret += mcgen('''
 }

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 16/17] Replace qmp-commands.hx by doc/qmp-commands.txt
  2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 16/17] Replace qmp-commands.hx by doc/qmp-commands.txt marcandre.lureau
@ 2016-08-17 15:01   ` Markus Armbruster
  0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-08-17 15:01 UTC (permalink / raw)
  To: marcandre.lureau; +Cc: qemu-devel

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The only remaining function of qmp-commands.hx is to let us generate
> qmp-commands.txt from it.  Replace qmp-commands.hx by qmp-commands.txt.
>
> (a later update will move the documentation in the respective json files
> and again generate the text file)

Since this series doesn't move it, I'd rather say something like "We
intend to move the documentation into the QAPI schema and generate
qapi-commands.txt from it, but not right now."  Can touch up on commit.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode
  2016-08-11  4:45 ` [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode no-reply
@ 2016-08-17 15:03   ` Markus Armbruster
  0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-08-17 15:03 UTC (permalink / raw)
  To: no-reply; +Cc: marcandre.lureau, qemu-devel, famz

no-reply@ec2-52-6-146-230.compute-1.amazonaws.com writes:

> Hi,
>
> Your series seems to have some coding style problems. See output below for
> more information:
[...]
> Checking PATCH 16/17: Replace qmp-commands.hx by doc/qmp-commands.txt...
> ERROR: trailing whitespace
> #168: FILE: docs/qmp-commands.txt:73:
> +Arguments: $
>
> ERROR: trailing whitespace
> #1660: FILE: docs/qmp-commands.txt:1565:
> +Arguments: $
>
> ERROR: trailing whitespace
> #2386: FILE: docs/qmp-commands.txt:2291:
> +   $
>
> ERROR: trailing whitespace
> #2403: FILE: docs/qmp-commands.txt:2308:
> +   $
>
> ERROR: trailing whitespace
> #2787: FILE: docs/qmp-commands.txt:2692:
> +               (such as RDMA pinning) which may be expensive, but do not $
>
> ERROR: trailing whitespace
> #2788: FILE: docs/qmp-commands.txt:2693:
> +               actually occur during the iterative migration rounds $
>
> total: 6 errors, 0 warnings, 3901 lines checked

All fixed in the next commit.  You could swap the two to shut up
checkpatch, but that feels like a waste of your time to me.

[...]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands
  2016-08-17 14:47   ` Markus Armbruster
@ 2016-08-17 15:49     ` Marc-André Lureau
  2016-08-18  6:50       ` Markus Armbruster
  0 siblings, 1 reply; 32+ messages in thread
From: Marc-André Lureau @ 2016-08-17 15:49 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

Hi

On Wed, Aug 17, 2016 at 6:49 PM Markus Armbruster <armbru@redhat.com> wrote:

> marcandre.lureau@redhat.com writes:
>
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The generated marshal functions do not visit arguments from commands
> > that take no arguments. Thus they fail to catch invalid
> > members. Visit the arguments, if provided, to throw an error in case of
> > invalid members.
> >
> > Currently, qmp_check_client_args() checks for invalid arguments and
> > correctly catches this case. When switching to qmp_dispatch() we want to
> > keep that behaviour. The commands using 'O' may have arbitrary
> > arguments, and must have 'gen': false in the qapi schema to skip the
> > generated checks.
>
> Explains why this isn't a bug fix for QMP.  What about QGA?
>

Sorry, I don't understand what you ask. I thought the above paragraph that
I added described the current QMP behaviour and why we want to keep it. And
yes, it'a fix for qga too (it's qapi)


>
> > Old/new diff:
> > static void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp)
> >  {
> > +    Visitor *v = NULL;
> >      Error *err = NULL;
> > -
>
> Please keep the blank line between declarations and statements.
>



>
> > -    (void)args;
> > +    if (args) {
>
> This code...
>
> > +        v = qmp_input_visitor_new(QOBJECT(args), true);
> > +        visit_start_struct(v, NULL, NULL, 0, &err);
> > +        if (err) {
> > +            goto out;
> > +        }
> > +
> > +        if (!err) {
> > +            visit_check_struct(v, &err);
> > +        }
> > +        visit_end_struct(v, NULL);
> > +        if (err) {
> > +            goto out;
> > +        }
>
> ... is not indented in my build.
>
> > +    }
> >
> >      qmp_stop(&err);
> > +
> > +out:
> >      error_propagate(errp, err);
> > +    visit_free(v);
> > +    if (args) {
> > +        v = qapi_dealloc_visitor_new();
> > +        visit_start_struct(v, NULL, NULL, 0, NULL);
> > +
> > +        visit_end_struct(v, NULL);
> > +        visit_free(v);
>
> Likewise.
>
> > +    }
> >  }
> >
> > The new code closely resembles code for a command with arguments.
> > Differences:
> > - the visit of the argument and its cleanup struct don't visit any
> >   members (because there are none).
> > - the visit of the argument struct and its cleanup are conditional.
>
> Additional diff for all other qmp_marshal_FOO():
>
>   @@ -46,9 +46,9 @@ static void qmp_marshal_output_AddfdInfo
>
>    void qmp_marshal_add_fd(QDict *args, QObject **ret, Error **errp)
>    {
>   +    Visitor *v = NULL;
>        Error *err = NULL;
>        AddfdInfo *retval;
>   -    Visitor *v;
>        q_obj_add_fd_arg arg = {0};
>
>        v = qmp_input_visitor_new(QOBJECT(args), true);
>
> Let's avoid this churn.
>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  tests/test-qmp-commands.c | 15 ++++++++++++++
> >  scripts/qapi-commands.py  | 53
> ++++++++++++++++++++++++++++++-----------------
> >  2 files changed, 49 insertions(+), 19 deletions(-)
> >
> > diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> > index 261fd9e..81cbe54 100644
> > --- a/tests/test-qmp-commands.c
> > +++ b/tests/test-qmp-commands.c
> > @@ -106,6 +106,7 @@ static void test_dispatch_cmd(void)
> >  static void test_dispatch_cmd_failure(void)
> >  {
> >      QDict *req = qdict_new();
> > +    QDict *args = qdict_new();
> >      QObject *resp;
> >
> >      qdict_put_obj(req, "execute",
> QOBJECT(qstring_from_str("user_def_cmd2")));
> > @@ -114,6 +115,20 @@ static void test_dispatch_cmd_failure(void)
> >      assert(resp != NULL);
> >      assert(qdict_haskey(qobject_to_qdict(resp), "error"));
> >
> > +    qobject_decref(resp);
> > +    QDECREF(req);
> > +
> > +    /* check that with extra arguments it throws an error */
> > +    req = qdict_new();
> > +    qdict_put(args, "a", qint_from_int(66));
> > +    qdict_put(req, "arguments", args);
> > +
> > +    qdict_put_obj(req, "execute",
> QOBJECT(qstring_from_str("user_def_cmd")));
> > +
> > +    resp = qmp_dispatch(QOBJECT(req));
> > +    assert(resp != NULL);
> > +    assert(qdict_haskey(qobject_to_qdict(resp), "error"));
> > +
> >      qobject_decref(resp);
> >      QDECREF(req);
> >  }
> > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> > index eac64ce..9c79b18 100644
> > --- a/scripts/qapi-commands.py
> > +++ b/scripts/qapi-commands.py
> > @@ -94,11 +94,28 @@ def gen_marshal_decl(name):
> >                   proto=gen_marshal_proto(name))
> >
> >
> > +def if_args(arg_type, block):
> > +    ret = ''
> > +    if not arg_type or arg_type.is_empty():
> > +        ret += mcgen('''
> > +    if (args) {
> > +''')
> > +        push_indent()
> > +    ret += block
> > +    if not arg_type or arg_type.is_empty():
> > +        pop_indent()
> > +        ret += mcgen('''
> > +    }
> > +''')
>
> Since @block has already been formatted, the push_indent() /
> pop_indent() has no effect.  I guess that's why you did it with a lambda
> in v3.
>
> > +    return ret
> > +
> > +
> >  def gen_marshal(name, arg_type, boxed, ret_type):
> >      ret = mcgen('''
> >
> >  %(proto)s
> >  {
> > +    Visitor *v = NULL;
> >      Error *err = NULL;
> >  ''',
> >                  proto=gen_marshal_proto(name))
> > @@ -109,17 +126,23 @@ def gen_marshal(name, arg_type, boxed, ret_type):
> >  ''',
> >                       c_type=ret_type.c_type())
> >
> > +    visit_members = ''
> >      if arg_type and not arg_type.is_empty():
> > +        visit_members = 'visit_type_%s_members(v, &arg, &err);' % \
> > +                        arg_type.c_name()
>
> PEP8 prefers
>
>            visit_members = ('visit_type_%s_members(v, &arg, &err);'
>                              % arg_type.c_name())
>
> >          ret += mcgen('''
> > -    Visitor *v;
> >      %(c_name)s arg = {0};
> >
> > +''',
> > +                     c_name=arg_type.c_name())
> > +
> > +    ret += if_args(arg_type, mcgen('''
> >      v = qmp_input_visitor_new(QOBJECT(args), true);
> >      visit_start_struct(v, NULL, NULL, 0, &err);
> >      if (err) {
> >          goto out;
> >      }
> > -    visit_type_%(c_name)s_members(v, &arg, &err);
> > +    %(visit_members)s
> >      if (!err) {
> >          visit_check_struct(v, &err);
> >      }
> > @@ -128,35 +151,27 @@ def gen_marshal(name, arg_type, boxed, ret_type):
> >          goto out;
> >      }
> >  ''',
> > -                     c_name=arg_type.c_name())
> > -
> > -    else:
> > -        ret += mcgen('''
> > -
> > -    (void)args;
> > -''')
> > +                                   visit_members=visit_members))
> >
> >      ret += gen_call(name, arg_type, boxed, ret_type)
> >
> > -    # 'goto out' produced above for arg_type, and by gen_call() for
> ret_type
> > -    if (arg_type and not arg_type.is_empty()) or ret_type:
> > -        ret += mcgen('''
> > +    if arg_type and not arg_type.is_empty():
> > +        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg,
> NULL);',
> > +                              c_name=arg_type.c_name())
>
> I'm afraid you missed this instance of "mcgen()'s output fed to
> mcgen()".
>
> > +    ret += mcgen('''
> >
> >  out:
> > -''')
> > -    ret += mcgen('''
> >      error_propagate(errp, err);
> > -''')
> > -    if arg_type and not arg_type.is_empty():
> > -        ret += mcgen('''
> >      visit_free(v);
> > +''')
> > +    ret += if_args(arg_type, mcgen('''
> >      v = qapi_dealloc_visitor_new();
> >      visit_start_struct(v, NULL, NULL, 0, NULL);
> > -    visit_type_%(c_name)s_members(v, &arg, NULL);
> > +    %(visit_members)s
> >      visit_end_struct(v, NULL);
> >      visit_free(v);
> >  ''',
> > -                     c_name=arg_type.c_name())
> > +                                   visit_members=visit_members))
> >
> >      ret += mcgen('''
> >  }
>
> I append a diff from this one to a stupider solution.  It's slightly
> longer, but I find it a bit easier to understand.
>

ok, applied


>
>
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 9c79b18..2f603b0 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -94,28 +94,13 @@ def gen_marshal_decl(name):
>                   proto=gen_marshal_proto(name))
>
>
> -def if_args(arg_type, block):
> -    ret = ''
> -    if not arg_type or arg_type.is_empty():
> -        ret += mcgen('''
> -    if (args) {
> -''')
> -        push_indent()
> -    ret += block
> -    if not arg_type or arg_type.is_empty():
> -        pop_indent()
> -        ret += mcgen('''
> -    }
> -''')
> -    return ret
> -
> -
>  def gen_marshal(name, arg_type, boxed, ret_type):
> +    have_args = arg_type and not arg_type.is_empty()
> +
>      ret = mcgen('''
>
>  %(proto)s
>  {
> -    Visitor *v = NULL;
>      Error *err = NULL;
>  ''',
>                  proto=gen_marshal_proto(name))
> @@ -126,17 +111,25 @@ def gen_marshal(name, arg_type, boxed, ret_type):
>  ''',
>                       c_type=ret_type.c_type())
>
> -    visit_members = ''
> -    if arg_type and not arg_type.is_empty():
> -        visit_members = 'visit_type_%s_members(v, &arg, &err);' % \
> -                        arg_type.c_name()
> +    if have_args:
> +        visit_members = ('visit_type_%s_members(v, &arg, &err);'
> +                         % arg_type.c_name())
>          ret += mcgen('''
> +    Visitor *v;
>      %(c_name)s arg = {0};
>
>  ''',
>                       c_name=arg_type.c_name())
> +    else:
> +        visit_members = ''
> +        ret += mcgen('''
> +    Visitor *v = NULL;
>
> -    ret += if_args(arg_type, mcgen('''
> +    if (args) {
> +''')
> +        push_indent()
> +
> +    ret += mcgen('''
>      v = qmp_input_visitor_new(QOBJECT(args), true);
>      visit_start_struct(v, NULL, NULL, 0, &err);
>      if (err) {
> @@ -151,27 +144,47 @@ def gen_marshal(name, arg_type, boxed, ret_type):
>          goto out;
>      }
>  ''',
> -                                   visit_members=visit_members))
> +                 visit_members=visit_members)
> +
> +    if not have_args:
> +        pop_indent()
> +        ret += mcgen('''
> +    }
> +''')
>
>      ret += gen_call(name, arg_type, boxed, ret_type)
>
> -    if arg_type and not arg_type.is_empty():
> -        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg,
> NULL);',
> -                              c_name=arg_type.c_name())
>      ret += mcgen('''
>
>  out:
>      error_propagate(errp, err);
>      visit_free(v);
>  ''')
> -    ret += if_args(arg_type, mcgen('''
> +
> +    if have_args:
> +        visit_members = ('visit_type_%s_members(v, &arg, NULL);'
> +                         % arg_type.c_name())
> +    else:
> +        visit_members = ''
> +        ret += mcgen('''
> +    if (args) {
> +''')
> +        push_indent()
> +
> +    ret += mcgen('''
>      v = qapi_dealloc_visitor_new();
>      visit_start_struct(v, NULL, NULL, 0, NULL);
>      %(visit_members)s
>      visit_end_struct(v, NULL);
>      visit_free(v);
>  ''',
> -                                   visit_members=visit_members))
> +                 visit_members=visit_members)
> +
> +    if not have_args:
> +        pop_indent()
> +        ret += mcgen('''
> +    }
> +''')
>
>      ret += mcgen('''
>  }
>
> --
Marc-André Lureau

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands
  2016-08-17 15:49     ` Marc-André Lureau
@ 2016-08-18  6:50       ` Markus Armbruster
  2016-08-18  8:38         ` Marc-André Lureau
  0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2016-08-18  6:50 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: qemu-devel

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Wed, Aug 17, 2016 at 6:49 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> marcandre.lureau@redhat.com writes:
>>
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > The generated marshal functions do not visit arguments from commands
>> > that take no arguments. Thus they fail to catch invalid
>> > members. Visit the arguments, if provided, to throw an error in case of
>> > invalid members.
>> >
>> > Currently, qmp_check_client_args() checks for invalid arguments and
>> > correctly catches this case. When switching to qmp_dispatch() we want to
>> > keep that behaviour. The commands using 'O' may have arbitrary
>> > arguments, and must have 'gen': false in the qapi schema to skip the
>> > generated checks.
>>
>> Explains why this isn't a bug fix for QMP.  What about QGA?
>>
>
> Sorry, I don't understand what you ask. I thought the above paragraph that
> I added described the current QMP behaviour and why we want to keep it. And
> yes, it'a fix for qga too (it's qapi)

If it *is* a fix for QGA (have you tested it?), the commit message needs
to mention the fact more prominently.  I'll make a suggestion on v5 of
this patch.

[...]

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands
  2016-08-18  6:50       ` Markus Armbruster
@ 2016-08-18  8:38         ` Marc-André Lureau
  0 siblings, 0 replies; 32+ messages in thread
From: Marc-André Lureau @ 2016-08-18  8:38 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel

Hi

On Thu, Aug 18, 2016 at 10:50 AM Markus Armbruster <armbru@redhat.com>
wrote:

> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>
> > Hi
> >
> > On Wed, Aug 17, 2016 at 6:49 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> marcandre.lureau@redhat.com writes:
> >>
> >> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> >
> >> > The generated marshal functions do not visit arguments from commands
> >> > that take no arguments. Thus they fail to catch invalid
> >> > members. Visit the arguments, if provided, to throw an error in case
> of
> >> > invalid members.
> >> >
> >> > Currently, qmp_check_client_args() checks for invalid arguments and
> >> > correctly catches this case. When switching to qmp_dispatch() we want
> to
> >> > keep that behaviour. The commands using 'O' may have arbitrary
> >> > arguments, and must have 'gen': false in the qapi schema to skip the
> >> > generated checks.
> >>
> >> Explains why this isn't a bug fix for QMP.  What about QGA?
> >>
> >
> > Sorry, I don't understand what you ask. I thought the above paragraph
> that
> > I added described the current QMP behaviour and why we want to keep it.
> And
> > yes, it'a fix for qga too (it's qapi)
>
> If it *is* a fix for QGA (have you tested it?), the commit message needs
> to mention the fact more prominently.  I'll make a suggestion on v5 of
> this patch.
>

I added a test for QGA (to be added in v6):
https://github.com/elmarco/qemu/commit/a6912f43dcd06f558c0c88a5321fcbd8a507b5bc


>
> [...]
>
-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands
  2016-08-16 14:34     ` Marc-André Lureau
  2016-08-16 15:32       ` Markus Armbruster
@ 2016-09-09 13:59       ` Markus Armbruster
  1 sibling, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2016-09-09 13:59 UTC (permalink / raw)
  To: Marc-André Lureau; +Cc: marcandre lureau, qemu-devel, Eric Blake

Marc-André Lureau <mlureau@redhat.com> writes:

> Hi
>
> ----- Original Message -----
>> marcandre.lureau@redhat.com writes:
>> 
>> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >
>> > The current monitor dispatch codes doesn't know commands that have been
>> > filtered out during qmp-commands.hx preprocessing. query-commands
>> > doesn't list them either. However, qapi generator doesn't filter them
>> > out, and they are listed in the command list.
>> >
>> > For now, disable the commands that aren't avaible to avoid introducing a
>> > regression there when switching to qmp_dispatch() or listing commands
>> > from the generated qapi code.
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > ---
>> >  monitor.c | 23 +++++++++++++++++++++++
>> >  1 file changed, 23 insertions(+)
>> >
>> > diff --git a/monitor.c b/monitor.c
>> > index b7ae552..ef946ad 100644
>> > --- a/monitor.c
>> > +++ b/monitor.c
>> > @@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict,
>> > QObject **ret_data,
>> >      *ret_data = qobject_from_json(qmp_schema_json);
>> >  }
>> >  
>> > +/*
>> > + * Those commands are registered unconditionnally by generated
>> > + * qmp files. FIXME: Educate the QAPI schema on #ifdef commands.
>> > + */
>> > +static void qmp_disable_marshal(void)
>> > +{
>> > +#ifndef CONFIG_SPICE
>> > +    qmp_disable_command("query-spice");
>> > +#endif
>> > +#ifndef TARGET_I386
>> > +    qmp_disable_command("rtc-reset-reinjection");
>> > +#endif
>> > +#ifndef TARGET_S390X
>> > +    qmp_disable_command("dump-skeys");
>> > +#endif
>> > +#ifndef TARGET_ARM
>> > +    qmp_disable_command("query-gic-capabilities");
>> > +#endif
>> > +}
>> > +
>> >  static void qmp_init_marshal(void)
>> >  {
>> >      qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
>> > @@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void)
>> >                           QCO_NO_OPTIONS);
>> >      qmp_register_command("netdev_add", qmp_netdev_add,
>> >                           QCO_NO_OPTIONS);
>> > +
>> > +    /* call it after the rest of qapi_init() */
>> > +    register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI);
>> >  }
>> >  
>> >  qapi_init(qmp_init_marshal);
>> 
>> Let's see whether I understand this patch...
>> 
>> qmp_disable_command() sets a flag that can be tested directly or with
>> qmp_command_is_enabled().  do_qmp_dispatch() tests it, and fails the
>> command with an "The command FOO has been disabled for this instance"
>> error.  It's called from qmp_dispatch(), which we're not yet using for
>> QMP at this point of the series.
>> 
>> QGA uses this to implement its "freeze whitelist", i.e. to disable all
>> commands that aren't known to be safe while filesystems are frozen.
>> 
>> qmp_disable_command() does nothing when the command doesn't exist, which
>> I think is the case at this point of the series.
>> 
>> So this patch does exactly nothing by itself.  When you later flip
>> dispatch to use qmp_dispatch(), it becomes active, and the error for
>> these commands changes from
>> 
>>     {"error": {"class": "CommandNotFound", "desc": "The command FOO has not
>>     been found"}}
>> 
>> to
>> 
>>     {"error": {"class": "GenericError", "desc": "The command FOO has been
>>     disabled for this instance"}}
>> 
>> which is fine with me.

Unfortunately, it looks like it's not fine with libvirt.
qemuMonitorJSONGetGICCapabilities() in src/qemu/qemu_monitor_json.c:

    int ret = -1;
    [...]
    /* If the 'query-gic-capabilities' QMP command was not available
     * we simply successfully return zero capabilities.
     * This is the case for QEMU <2.6 and all non-ARM architectures */
    if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
        ret = 0;
        goto cleanup;
    }

    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
        goto cleanup;

Ideally, libvirt would use query-commands instead of ErrorClass
CommandNotFound.  But we need to play the hand we've been dealt.  That
means preserving ErrorClass CommandNotFound.

>> Is this basically correct?
>
> I think it's correct, I haven't played much with this as I focused on the conditional build instead, which is actually not so difficult, see:
[...]

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2016-09-09 13:59 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-10 18:02 [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 01/17] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO} marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 02/17] qapi-schema: use generated marshaller for 'qmp_capabilities' marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 03/17] qapi-schema: add 'device_add' marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 04/17] monitor: simplify invalid_qmp_mode() marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 05/17] monitor: register gen:false commands manually marcandre.lureau
2016-08-16 12:27   ` Markus Armbruster
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands marcandre.lureau
2016-08-16 14:26   ` Markus Armbruster
2016-08-16 14:34     ` Marc-André Lureau
2016-08-16 15:32       ` Markus Armbruster
2016-09-09 13:59       ` Markus Armbruster
2016-08-17 13:36   ` Markus Armbruster
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 07/17] qapi: export the marshallers marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 08/17] monitor: use qmp_find_command() (using generated qapi code) marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 09/17] monitor: implement 'qmp_query_commands' without qmp_cmds marcandre.lureau
2016-08-16 16:22   ` Markus Armbruster
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 10/17] monitor: remove mhandler.cmd_new marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 11/17] qapi: remove the "middle" mode marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 12/17] qapi: check invalid arguments on no-args commands marcandre.lureau
2016-08-17 14:47   ` Markus Armbruster
2016-08-17 15:49     ` Marc-André Lureau
2016-08-18  6:50       ` Markus Armbruster
2016-08-18  8:38         ` Marc-André Lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 13/17] qmp: update qmp_query_spice fallback marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 14/17] monitor: use qmp_dispatch() marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 15/17] build-sys: remove qmp-commands-old.h marcandre.lureau
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 16/17] Replace qmp-commands.hx by doc/qmp-commands.txt marcandre.lureau
2016-08-17 15:01   ` Markus Armbruster
2016-08-10 18:02 ` [Qemu-devel] [PATCH v4 17/17] qmp-commands.txt: fix some styling marcandre.lureau
2016-08-11  4:45 ` [Qemu-devel] [PATCH v4 00/17] qapi: remove the 'middle' mode no-reply
2016-08-17 15:03   ` Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.