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
signature.asc
Description: OpenPGP digital signature