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

[Markus Armbruster]

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!