Re: [Qemu-devel] [PATCH RFC v3 30/32] qapi: New QMP command query-schema for QMP schema introspection

[Markus Armbruster <armbru@redhat.com>]

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 08/04/2015 09:58 AM, Markus Armbruster wrote:
[...]
>>> +++ b/qapi/introspect.json
>>
>>> +{ 'struct': 'SchemaInfoObject',
>>> +  'data': { 'members': [ 'SchemaInfoObjectMember' ],
>>> +            '*tag': 'str',
>>> +            '*variants': [ 'SchemaInfoObjectVariant' ] } }
>>
>> I take it that either tag and variants will both be omitted, or both be
>> present.  No clean way to represent that in the schema, though.
>
> Needs a comment.

Alternatively, wrap both tag and variants in an object, mandatory there,
then make that object optional.  Doesn't feel wortwhile to me, but if
you like it better, I'll reconsider.

[...]
>>> +++ b/scripts/qapi-introspect.py
>>> @@ -0,0 +1,172 @@
>>> +#
>>> +# QAPI introspection generator
>>> +#
>>> +# Copyright (C) 2015 Red Hat, Inc.
>>> +#
>>> +# Authors:
>>> +#  Markus Armbruster <armbru@redhat.com>
>>> +#
>>> +# This work is licensed under the terms of the GNU GPL, version 2.
>>> +# See the COPYING file in the top-level directory.
>>> +
>>> +from qapi import *
>>> +
>>> +# Caveman's json.dumps() replacement (we're stuck at 2.4)
>>> +# TODO try to use json.dumps() once we get unstuck
>>> +def to_json(obj, level=0):
>>> +    if obj == None:
>>> +        ret = 'null'
>>> +    elif isinstance(obj, str):
>>> +        ret = '"' + obj.replace('"', r'\"') + '"'
>>> +    elif isinstance(obj, list):
>>> +        elts = [to_json(elt, level + 1)
>>> +                for elt in obj]
>>
>> No sorting here makes sense (the json-to-string converter can't know
>> whether we are using arrays as sets, vs. in the usual JSON semantics of
>> order-preserving).  Thus, if we intend to sort things like enum values
>> or object members by name, that sorting has to be done prior to feeding
>> it to this pretty-printer.
>
> Correct.  I didn't, because I ran out of time.
>
>>> +        ret = '[' + ', '.join(elts) + ' ]'
>>
>> Here's where I was wondering if you want '[ '.
>
> Yes.

I'll consistently drop the spaces, to match Python's conventions.

[...]
>>> +                 'variants': [self._gen_variant(v) for v in variants] }
>>
>> Do you want to add a sort() on variant 'case's here?
>>
>>> +
>>> +    def visit_enum_type(self, name, info, values):
>>> +        self._gen_json(name, 'enum', { 'values': values })
>>
>> Do you want to add a sort() on values here?
>
> I want to review the introspection schema systematically for sorting
> opportunities.

I'm going to leave this job for later, because I feel we should get the
basics in as soon as possible.  We then have the rest of the development
cycle to refine stuff.

[...]