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

[Eric Blake]

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 to follow in separate message; I'm using this message to focus
on one point for easier tracking of the sub-thread]

> 
> The introspection schema does not reflect all the rules and
> restrictions that apply to QAPI schemata.  A valid QAPI schema has an
> introspection value conforming to the introspection schema, but the
> converse is not true.
> 
> Introspection lowers away a number of schema details, and makes
> implicit things explicit:
> 
> * The built-in types are declared with their JSON type.
> 
>   TODO Should we map all the integer types to just int?

So, given the discussion on v3, I think we will have consensus if v5
does the following:

- Merge 30 and 31 into a single patch, and drop this TODO statement (I
think we have agreement that exposing QMP [that is, JSON number] wire
form, with no additional constraints, as a single 'int', rather than the
underlying generated C types, is the way to go), provided that...
- Document in the qapi side that the schema intentionally lacks details
on ranges, and that consulting the qapi and/or C code may be required to
see what actual values are valid anywhere introspection merely says 'int',
- leave 32 as a separate patch, as it is complex enough, and still may
have debate on whether the types '123' and 'int' should have
corresponding array types '[123]' and '[int]' rather than the v3 patch
munging of '456' and '789' (I haven't yet reviewed whether v4 changed that).

Any further arguments on whether exposing just 'int' in the
introspection for all integral types, and/or whether patches 30 and 31
should be merged, are best made in response to this mail.

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

signature.asc
Description: OpenPGP digital signature