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: 2706 bytes --]

On 09/01/2015 12:40 PM, Michael Roth wrote:
> Quoting Markus Armbruster (2015-08-04 10:58:14)
>> Caution, rough edges.
>>
>> qapi/introspect.json defines the introspection schema.  It should do
>> for uses other than QMP.
>> FIXME it's almost entirely devoid of comments.
>>
>> 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:
>>
>> * The built-in types are declared with their JSON type.
>>
>>   TODO Should we map all the integer types to just int?
> 
> I don't think we should. If management chooses to handle them in this
> generic fashion that's their perogative/problem, but treating all ints
> as a single generic type can lead to unexpected results when those values
> get passed on to functions expecting, for instance, uint8_t. So QEMU
> should do its part to convey this information somehow.

The argument here is that it is always more conservative to start with
less information, where we can later add more information (whether in
the form of type constraints such as uint8_t, or in a different form
such as min/max values) if they prove useful.  And QMP already
guarantees that it gives sane error messages for parameters that are out
of range, so the worst behavior a client might see is that a parameter
claiming to be 'int' in the introspection gracefully rejects an attempt
to use '256' as a value because the underlying type was uint8_t.

If we advertise full types now, then the choice of integer type becomes
ABI (switching from 'int8_t' to 'uint8_t' has observable impact in the
introspection) that we are stuck exposing in introspection forever.  And
in the past, we have successfully retyped a command with no change to
the wire API (see commit 5fba6c0).

At any rate, patch 31/32 in this series gives stronger arguments for
merging the types for at least the initial implementation; we can always
change our minds later and undo the merging, even if it is after 2.5
when we change our minds.

But the mere fact that you are questioning the idea means that patch 30
and 31 should not be merged (previously, I had argued that separating
the patches didn't make sense; I don't know if Markus was planning to
merge the two based on my recommendation), if only so that reverting the
type merging becomes an easier task if we decide down the road that we
don't need the merged 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 --]