[PATCH v6 05/10] monitor: Drop query-qmp-schema 'gen': false hack

Markus Armbruster posted 10 patches 4 years, 11 months ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Michael Roth <michael.roth@amd.com>, Eric Blake <eblake@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Markus Armbruster <armbru@redhat.com>
There is a newer version of this series
[PATCH v6 05/10] monitor: Drop query-qmp-schema 'gen': false hack
Posted by Markus Armbruster 4 years, 11 months ago
QMP commands return their response as a generated QAPI type, which the
monitor core converts to JSON via QObject.

query-qmp-schema's response is the generated introspection data.  This
is a QLitObject since commit 7d0f982bfb "qapi: generate a literal
qobject for introspection", v2.12).  Before, it was a string.  Instead
of converting QLitObject / string -> QObject -> QAPI type
SchemaInfoList -> QObject -> JSON, we take a shortcut: the command is
'gen': false, so it can return the QObject instead of the QAPI type.
Slightly simpler and more efficient.

The next commit will filter the response for output policy, and this
is easier in the SchemaInfoList representation.  Drop the shortcut.

This replaces the manual command registration by a generated one.  The
manual registration makes the commnd available before the machine is
built by passing flag QCO_ALLOW_PRECONFIG.  To keep it available
there, we need need to add 'allow-preconfig': true to its definition
in the schema.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qapi/introspect.json                 |  2 +-
 monitor/monitor-internal.h           |  3 ---
 monitor/misc.c                       |  2 --
 monitor/qmp-cmds-control.c           | 29 ++++++++++++++++------------
 storage-daemon/qemu-storage-daemon.c |  2 --
 5 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/qapi/introspect.json b/qapi/introspect.json
index 944bb87a20..39bd303778 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -49,7 +49,7 @@
 ##
 { 'command': 'query-qmp-schema',
   'returns': [ 'SchemaInfo' ],
-  'gen': false }                # just to simplify qmp_query_json()
+  'allow-preconfig': true }
 
 ##
 # @SchemaMetaType:
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 40903d6386..9c3a09cb01 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -183,7 +183,4 @@ void help_cmd(Monitor *mon, const char *name);
 void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
 int hmp_compare_cmd(const char *name, const char *list);
 
-void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
-                                 Error **errp);
-
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index a7650ed747..0b46006e42 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -231,8 +231,6 @@ static void monitor_init_qmp_commands(void)
 
     qmp_init_marshal(&qmp_commands);
 
-    qmp_register_command(&qmp_commands, "query-qmp-schema",
-                         qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
     qmp_register_command(&qmp_commands, "device_add", qmp_device_add,
                          QCO_NO_OPTIONS);
     qmp_register_command(&qmp_commands, "object-add", qmp_object_add,
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 509ae870bd..25afd0867f 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -26,10 +26,14 @@
 
 #include "monitor-internal.h"
 #include "qemu-version.h"
+#include "qapi/compat-policy.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-control.h"
+#include "qapi/qapi-commands-introspect.h"
 #include "qapi/qapi-emit-events.h"
 #include "qapi/qapi-introspect.h"
+#include "qapi/qapi-visit-introspect.h"
+#include "qapi/qobject-input-visitor.h"
 
 /*
  * Accept QMP capabilities in @list for @mon.
@@ -154,17 +158,18 @@ EventInfoList *qmp_query_events(Error **errp)
     return ev_list;
 }
 
-/*
- * Minor hack: generated marshalling suppressed for this command
- * ('gen': false in the schema) so we can parse the JSON string
- * directly into QObject instead of first parsing it with
- * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
- * to QObject with generated output marshallers, every time.  Instead,
- * we do it in test-qobject-input-visitor.c, just to make sure
- * qapi-gen.py's output actually conforms to the schema.
- */
-void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
-                                 Error **errp)
+SchemaInfoList *qmp_query_qmp_schema(Error **errp)
 {
-    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
+    QObject *obj = qobject_from_qlit(&qmp_schema_qlit);
+    Visitor *v = qobject_input_visitor_new(obj);
+    SchemaInfoList *schema = NULL;
+
+    /* test_visitor_in_qmp_introspect() ensures this can't fail */
+    visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
+    g_assert(schema);
+
+    qobject_unref(obj);
+    visit_free(v);
+
+    return schema;
 }
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 23756fc8e5..02afdbeafc 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -146,8 +146,6 @@ static QemuOptsList qemu_object_opts = {
 static void init_qmp_commands(void)
 {
     qmp_init_marshal(&qmp_commands);
-    qmp_register_command(&qmp_commands, "query-qmp-schema",
-                         qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG);
     qmp_register_command(&qmp_commands, "object-add", qmp_object_add,
                          QCO_NO_OPTIONS);
 
-- 
2.26.2


Re: [PATCH v6 05/10] monitor: Drop query-qmp-schema 'gen': false hack
Posted by Eric Blake 4 years, 11 months ago
On 3/12/21 9:32 AM, Markus Armbruster wrote:
> QMP commands return their response as a generated QAPI type, which the
> monitor core converts to JSON via QObject.
> 
> query-qmp-schema's response is the generated introspection data.  This
> is a QLitObject since commit 7d0f982bfb "qapi: generate a literal
> qobject for introspection", v2.12).  Before, it was a string.  Instead
> of converting QLitObject / string -> QObject -> QAPI type
> SchemaInfoList -> QObject -> JSON, we take a shortcut: the command is
> 'gen': false, so it can return the QObject instead of the QAPI type.
> Slightly simpler and more efficient.
> 
> The next commit will filter the response for output policy, and this
> is easier in the SchemaInfoList representation.  Drop the shortcut.
> 
> This replaces the manual command registration by a generated one.  The
> manual registration makes the commnd available before the machine is

command

> built by passing flag QCO_ALLOW_PRECONFIG.  To keep it available
> there, we need need to add 'allow-preconfig': true to its definition
> in the schema.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  qapi/introspect.json                 |  2 +-
>  monitor/monitor-internal.h           |  3 ---
>  monitor/misc.c                       |  2 --
>  monitor/qmp-cmds-control.c           | 29 ++++++++++++++++------------
>  storage-daemon/qemu-storage-daemon.c |  2 --
>  5 files changed, 18 insertions(+), 20 deletions(-)
> 

R-b still stands.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Re: [PATCH v6 05/10] monitor: Drop query-qmp-schema 'gen': false hack
Posted by Markus Armbruster 4 years, 11 months ago
Eric Blake <eblake@redhat.com> writes:

> On 3/12/21 9:32 AM, Markus Armbruster wrote:
>> QMP commands return their response as a generated QAPI type, which the
>> monitor core converts to JSON via QObject.
>> 
>> query-qmp-schema's response is the generated introspection data.  This
>> is a QLitObject since commit 7d0f982bfb "qapi: generate a literal
>> qobject for introspection", v2.12).  Before, it was a string.  Instead
>> of converting QLitObject / string -> QObject -> QAPI type
>> SchemaInfoList -> QObject -> JSON, we take a shortcut: the command is
>> 'gen': false, so it can return the QObject instead of the QAPI type.
>> Slightly simpler and more efficient.
>> 
>> The next commit will filter the response for output policy, and this
>> is easier in the SchemaInfoList representation.  Drop the shortcut.
>> 
>> This replaces the manual command registration by a generated one.  The
>> manual registration makes the commnd available before the machine is
>
> command

Fixing...

>> built by passing flag QCO_ALLOW_PRECONFIG.  To keep it available
>> there, we need need to add 'allow-preconfig': true to its definition
>> in the schema.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>  qapi/introspect.json                 |  2 +-
>>  monitor/monitor-internal.h           |  3 ---
>>  monitor/misc.c                       |  2 --
>>  monitor/qmp-cmds-control.c           | 29 ++++++++++++++++------------
>>  storage-daemon/qemu-storage-daemon.c |  2 --
>>  5 files changed, 18 insertions(+), 20 deletions(-)
>> 
>
> R-b still stands.

Thanks!