[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v4 30/32] qapi: New QMP command query-schema
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC v4 30/32] qapi: New QMP command query-schema for QMP schema introspection |
Date: |
Thu, 3 Sep 2015 21:12:28 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 09/03/2015 08:03 PM, Eric Blake wrote:
> On 09/03/2015 08:30 AM, Markus Armbruster wrote:
>> qapi/introspect.json defines the introspection schema. It's designed
>> for QMP introspection, but should do for similar uses, such as QGA.
>
> [review in this sub-thread; for comments on 'int' munging or other
> followups, see other subthread]
>
> There is at least one definite bug (see multiple references below to
> [1]), and several ideas for cleanups, but in general, I think that the
> remaining changes are going to be small enough that I'd probably be okay
> if v5 started life with:
> Reviewed-by: Eric Blake <address@hidden>
> (Of course, you have every right to not add the R-b, especially if you
> want me to do another close review :)
>
>
>> +
>> + { "name": "BlockdevOptions", "meta-type": "object",
>> + "members": [
>> + { "name": "kind", "type": "BlockdevOptionsKind" } ],
>
> [1] Ouch. That should be "name": "type". I pointed out the bug in v3,
> but it looks like it still hasn't been fixed. Or rather, that our
> attempt to fix it wasn't correct.
>
>> + def _gen_variants(self, tag_name, variants):
>> + return {'tag': tag_name or 'type',
>
> [1] Ouch. Why are we still printing 'kind' as the name for simple
> unions? Oh, it's because tag_name is ALWAYS provided by the visitor, so
> the "or 'type'" clause never fires.
Not quite the right comment. 'tag' is being output correctly (tag_name
was indeed None), what was wrong was the 'members':[] array (which was
assuming the member was named 'kind').
> I have a pending patch to change
> the C code to generate 'type' as the C code name to match the wire ABI;
> but until that patch is in, I think a hack solution might be to fix the
> visitor to supply None instead of a tag_name for simple unions. I'll
> respond to the appropriate earlier patch.
Rather, 3 earlier patches. See my comments on 2, 10, and 11.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH RFC v4 29/32] qapi: Pseudo-type '**' is now unused, drop it, (continued)
[Qemu-devel] [PATCH RFC v4 19/32] qapi: Clean up after recent conversions to QAPISchemaVisitor, Markus Armbruster, 2015/09/03
Re: [Qemu-devel] [PATCH RFC v4 00/32] qapi: QMP introspection, Marc-André Lureau, 2015/09/04