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

[Markus Armbruster]

Michael Roth <address@hidden> writes:

> Quoting Eric Blake (2015-09-03 15:50:16)
>> 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',

Please review docs/qapi-code-gen.txt in this patch for this aspect.

>> - 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).
>
> Agreed on all 3

Excellent.

>> 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.
>
> Given the above, I don't think I have any further objections.

Thanks!