diff mbox series

[v5,5/8] qapi: Implement deprecated-output=hide for QMP introspection

Message ID 20200914084802.4185028-6-armbru@redhat.com
State New
Headers show
Series Configurable policy for handling deprecated interfaces | expand

Commit Message

Markus Armbruster Sept. 14, 2020, 8:47 a.m. UTC
This policy suppresses deprecated bits in output, and thus permits
"testing the future".  Implement it for QMP command query-qmp-schema:
suppress information on deprecated commands, events and object type
members, i.e. anything that has the special feature flag "deprecated".

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

Comments

Eric Blake Sept. 14, 2020, 3:43 p.m. UTC | #1
On 9/14/20 3:47 AM, Markus Armbruster wrote:
> This policy suppresses deprecated bits in output, and thus permits

> "testing the future".  Implement it for QMP command query-qmp-schema:

> suppress information on deprecated commands, events and object type

> members, i.e. anything that has the special feature flag "deprecated".

> 

> Signed-off-by: Markus Armbruster <armbru@redhat.com>

> ---

>   qapi/introspect.json                 |   2 +-

>   monitor/monitor-internal.h           |   3 -

>   monitor/misc.c                       |   2 -

>   monitor/qmp-cmds-control.c           | 100 +++++++++++++++++++++++----

>   storage-daemon/qemu-storage-daemon.c |   2 -

>   5 files changed, 89 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 }


Interesting change. Dropping 'gen':false is explained below...

> @@ -153,17 +157,89 @@ 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)

> +static void *split_off_generic_list(void *list,

> +                                    bool (*splitp)(void *elt),

> +                                    void **part)


...but adding 'allow-preconfig':true, while it makes sense, seems a bit 
unrelated.  Worth a better commit message?

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Markus Armbruster Sept. 21, 2020, 2:41 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 9/14/20 3:47 AM, Markus Armbruster wrote:
>> This policy suppresses deprecated bits in output, and thus permits
>> "testing the future".  Implement it for QMP command query-qmp-schema:
>> suppress information on deprecated commands, events and object type
>> members, i.e. anything that has the special feature flag "deprecated".
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qapi/introspect.json                 |   2 +-
>>   monitor/monitor-internal.h           |   3 -
>>   monitor/misc.c                       |   2 -
>>   monitor/qmp-cmds-control.c           | 100 +++++++++++++++++++++++----
>>   storage-daemon/qemu-storage-daemon.c |   2 -
>>   5 files changed, 89 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 }
>
> Interesting change. Dropping 'gen':false is explained below...
>
>> @@ -153,17 +157,89 @@ 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)
>> +static void *split_off_generic_list(void *list,
>> +                                    bool (*splitp)(void *elt),
>> +                                    void **part)
>
> ...but adding 'allow-preconfig':true, while it makes sense, seems a
> bit unrelated.

It's not, actually: query-qmp-schema has always worked in preconfig
state.  Current master:

    $ upstream-qemu -nodefaults -S -display none -qmp stdio -preconfig
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 1, "major": 5}, "package": ""}, "capabilities": ["oob"]}}
    {"execute": "qmp_capabilities"}
    {"return": {}}
    {"execute": "query-qmp-schema"}
    {"return": [{"name": "query-status", ...}}
    {"execute": "query-block"}
    {"error": {"class": "GenericError", "desc": "The command 'query-block' isn't permitted in 'preconfig' state"}}

We better keep it working there.

>                 Worth a better commit message?

Yes.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox series

Patch

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 b39e03b744..8833afaa65 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -180,7 +180,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 e847b58a8c..3fd335e737 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -242,8 +242,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 8f04cfa6e6..5b07c7e765 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.
@@ -153,17 +157,89 @@  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)
+static void *split_off_generic_list(void *list,
+                                    bool (*splitp)(void *elt),
+                                    void **part)
 {
-    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
+    GenericList *keep = NULL, **keep_tailp = &keep;
+    GenericList *split = NULL, **split_tailp = &split;
+    GenericList *tail;
+
+    for (tail = list; tail; tail = tail->next) {
+        if (splitp(tail)) {
+            *split_tailp = tail;
+            split_tailp = &tail->next;
+        } else {
+            *keep_tailp = tail;
+            keep_tailp = &tail->next;
+        }
+    }
+
+    *keep_tailp = *split_tailp = NULL;
+    *part = split;
+    return keep;
+}
+
+static bool is_in(const char *s, strList *list)
+{
+    strList *tail;
+
+    for (tail = list; tail; tail = tail->next) {
+        if (!strcmp(tail->value, s)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static bool is_entity_deprecated(void *link)
+{
+    return is_in("deprecated", ((SchemaInfoList *)link)->value->features);
+}
+
+static bool is_member_deprecated(void *link)
+{
+    return is_in("deprecated",
+                 ((SchemaInfoObjectMemberList *)link)->value->features);
+}
+
+static SchemaInfoList *zap_deprecated(SchemaInfoList *schema)
+{
+    void *to_zap;
+    SchemaInfoList *tail;
+    SchemaInfo *ent;
+
+    schema = split_off_generic_list(schema, is_entity_deprecated, &to_zap);
+    qapi_free_SchemaInfoList(to_zap);
+
+    for (tail = schema; tail; tail = tail->next) {
+        ent = tail->value;
+        if (ent->meta_type == SCHEMA_META_TYPE_OBJECT) {
+            ent->u.object.members
+                = split_off_generic_list(ent->u.object.members,
+                                         is_member_deprecated, &to_zap);
+            qapi_free_SchemaInfoObjectMemberList(to_zap);
+        }
+    }
+
+    return schema;
+}
+
+SchemaInfoList *qmp_query_qmp_schema(Error **errp)
+{
+    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);
+
+    if (compat_policy.deprecated_output == COMPAT_POLICY_OUTPUT_HIDE) {
+        return zap_deprecated(schema);
+    }
+    return schema;
 }
diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index 7e9b0e0d3f..9490099036 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -142,8 +142,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);
 
     QTAILQ_INIT(&qmp_cap_negotiation_commands);
     qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",