Re: [Qemu-devel] [PATCH RFC v2 47/47] qapi-introspect: Hide type names

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 07/28/2015 12:24 PM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>>>> To eliminate the temptation for clients to look up types by name
>>>> (which are not ABI), replace all type names by meaningless strings.
>>>>
>>>> Reduces output of query-schema by 9 out of 80KiB.
>>>
>>> I'm not sure whether I like this or not.  It does make sense from the
>>> perspective of forcing clients to stick to ABI queries, but makes it a
>>> bit harder to navigate things except by automated scripts.
>> 
>> Yes.  I'm not sure it's a good idea.  If we decide to hide types this
>> way, then I'd find an option to generate without type hiding useful.
>
> As in, an optional boolean flag to the QMP command that requests whether
> to get compact output with hidden names vs. full output with qapi names
> exposed (but then, are we storing TWO copies of the introspection
> strings)?  Or merely as in the generated qmp-introspect.c file having
> strategic comments so that reading _that_ file lets you see the type
> names, even if they don't get passed on to the end user?

I was thinking of optionally unhidden type names, for when you messed up
the output, and the stupid hidden type names make it hard to see what
exactly's wrong.  I.e. the option is merely a development aid.

>> What the patch adds:
>> 
>> Move the introspection information for the non-types out of the way
>> before the loop, append the information on types afterwards.  The result
>> is now in jsons rather than self.jsons (see the next patch hunk).
>
> I did notice that; pre-patch interleaved commands and types all
> according to a global namespace, while post-patch sank all types to the
> bottom.  Interestingly enough, if libvirt is going to query what
> features a command has, it will first find the command (in the first
> half of the returned array), then resolve the types used by that command
> until it learns if the member is present (whether new enum value or
> added dictionary member) - so there is that slight optimization that if
> we guarantee that types are always output last, then libvirt doesn't
> have to start generating its own hash table lookup of types until the
> first type is seen, after already learning everything it needed from the
> earlier command listings.

I'm reluctant to complicate the contract with such a promise, though.

I guess I'd simply slurp in the JSON, then do a quick pass over the
resulting data structure to build an index.

>> Why it does that:
>> 
>> With the funny typenames, sorting everything by name results in a mess.
>> Keeping non-types and types separate is less of a mess.
>
> You do have a point there - as soon as we introduce name aliases, the
> aliases are unlikely to sort in the same manner as the original types,

s/are unlikely to/won't/ :)

> particularly if we generate the aliases based solely on what order we
> detected that those particular types were in use.

That's the easy order.  Other orders are possible, but they're extra
work.  Best to keep things simple.