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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4471 bytes --]

On 08/04/2015 09:58 AM, Markus Armbruster wrote:
> * All type references are by name.
> 

>   Dictionary argument/result or list result is an implicit type
>   definition.
> 

> 
> * Clients should *not* look up types by name, because type names are
>   not ABI.  Look up the command or event you're interested in, then
>   follow the references.
> 
>   TODO Should we hide the type names to eliminate the temptation?

[not sure this conversation belongs better on 32 than here]

I was just experimenting with things, and noticed that our choice of
array names might not be ideal for clients.  We already discussed
earlier in the series that the name 'intList' might collide (the parser
currently rejects collisions with '*Kind', but not '*List').  But in
addition to that, look at what happens here in this patch:

    "{ \"element-type\": \"str\", \"meta-type\": \"array\", \"name\":
\"strList\" }, "

    "{ \"element-type\": \"BlockStats\", \"meta-type\": \"array\",
\"name\": \"BlockStatsList\" }, "

    "{ \"members\": [{ \"name\": \"values\", \"type\": \"strList\" } ],
\"meta-type\": \"object\", \"name\": \"SchemaInfoEnum\" }, "

    "{ \"arg-type\": \":obj-query-blockstats-arg\", \"meta-type\":
\"command\", \"name\": \"query-blockstats\", \"ret-type\":
\"BlockStatsList\" }, "


and particularly what happens to it after patch 32 is applied:

    "{ \"element-type\": \"str\", \"meta-type\": \"array\", \"name\":
\"270\" }, "

    "{ \"element-type\": \"179\", \"meta-type\": \"array\", \"name\":
\"96\" }, "

    "{ \"members\": [{ \"name\": \"values\", \"type\": \"270\" } ],
\"meta-type\": \"object\", \"name\": \"273\" }, "

    "{ \"arg-type\": \"95\", \"meta-type\": \"command\", \"name\":
\"query-blockstats\", \"ret-type\": \"96\" }, "

In order to learn the return type of query-blockstats (to see if a new
member was added to the struct), I now have to query what type 96 is,
see that it is an array, and then query what members type 179 has.  And
when dealing with type 273, I don't know whether to expect an array or
an object for the "type" member until I look up type 270 (but I _do_
know not to expect an int, number, boolean, or null).

What if we instead munged the name of array types in introspection
output to be "[int]" or "[BlockStats]"; or, with type masking in place,
"[int]" or "[179]"?  As in the following for the above examples:

 "{ \"element-type\": \"str\", \"meta-type\": \"array\", \"name\":
\"[str]\" }, "

"{ \"element-type\": \"179\", \"meta-type\": \"array\", \"name\":
\"[179]\" }, "

"{ \"members\": [{ \"name\": \"values\", \"type\": \"[str]\" } ],
\"meta-type\": \"object\", \"name\": \"273\" }, "

"{ \"arg-type\": \"95\", \"meta-type\": \"command\", \"name\":
\"query-blockstats\", \"ret-type\": \"[179]\" }, "

This way, libvirt or other clients could take shortcuts: I know that the
return of query-blockstats will be an array, and I only have to look up
the single type 179 to learn what members will be in the struct of that
array; likewise, when inspecting struct 273, I know that "type" is an
array of builtin strings and not an array of dicts, without needing to
do another type lookup.

I've always mentioned that I'm not a fan of packing multiple pieces of
information into a single JSON member (often a sign that the schema is
not specific enough), but in this particular case, the names would still
resolve without paying attention to the contents of the name.  That is,
we'd be providing extra information (whether a name starts with leading
'[') that clients can optionally use to be more efficient, but which
will not penalize clients that stick with the tried-and-true approach of
finding the array member with the matching name (that is, if I chase
"[179]", I will still learn that I need to look up "179").  And it gives
us the nice property that the first character of a type specifies its
JSON properties (letter=>builtin, [=>array, digit=>object), other than
the magic "any".

I still think we need to munge to '*List' internally (as in our
qapi_free_*() functions, for example), but what I'm trying to propose is
that we don't expose the '*List' naming convention through
introspection.  We'd also have to consider what this means for the
technical possibility of any 2D array types.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]