Eric Blake <eblake(a)redhat.com> writes:
On 5/13/19 8:53 AM, Markus Armbruster wrote:
>> We have a few options
>>
>> 1. Use string format for values > 2^53-1, int format below that
>> 2. Use string format for all fields which are 64-bit ints whether
>> signed or unsigned
>> 3. Use string format for all fields which are integers, even 32-bit
>> ones
>>
>> I would probably suggest option 2. It would make the QEMU impl quite
>> easy IIUC, we we'd just change the QAPI visitor's impl for the int64
>> and uint64 fields to use string format (when the right capability is
>> negotiated by QMP).
>>
>> I include 3 only for completeness - I don't think there's a hugely
>> compelling reason to mess with 32-bit ints.
>
> Agree.
Other than if we ever change the type of a QMP integer. Right now, if we
widen from 'int32' to 'int' (aka 'int64'), it is invisible to
clients;
but once we start stringizing 64-bit numbers (at client request) but NOT
32-bit numbers, then changing a type from 32 to 64 bits (or the
converse) becomes an API change to clients. Introspection will at least
let a client know which form to expect, but it does mean we have to be
more aware of typing issues going forward.
Thank you so much for helping my old synapses finally fire! Option 2 is
not what we thought it is. Let me explain.
Introspection reports *all* QAPI integer types as "int". This is
deliberate.
So, when the client that negotiated the interoperability capability sees
"int", it has to accept *both* integer encodings: JSON number and JSON
string.
The difference between option 1 and option 2 for the client is that
option 2 will use only one encoding. But the client must not rely on
that! Another QEMU version may well use the other encoding (because we
narrowed or widened the QAPI integer type in the QAPI schema).
Elsewhere in this thread, David pointed out that option 1 complicates
testing QEMU: full coverage requires passing both a small number (to
cover JSON number encoding) and a large number (to cover JSON string
encoding), to which I replied that there are very few places to test.
Option 2 complicates testing clients: full coverage requires testing
with both a version of QEMU (or a mock-up) that uses wide integers
(encoded as JSON string) and narrow integers (encoded as JSON number).
Impractical.
>> Option 1 is the bare minimum needed to ensure precision, but
to me
>> it feels a bit dirty to say a given field will have different encoding
>> depending on the value. If apps need to deal with string encoding, they
>> might as well just use it for all values in a given field.
>
> I guess that depends on what this interoperability capability does for
> QMP *input*.
"Be liberal in what you accept, strict in what you produce" - that
argues we should accept both forms on input (it's easy enough to ALWAYS
permit a string in place of an integer, and to take an in-range integer
even when we would in turn output it as a string).
With option 2, QEMU *has* to be liberal in what it accepts, because the
client cannot deduce from introspection whether the integer is wide or
narrow.
[...]
Daniel, you wrote you'd probably suggest option 2. Would you like to
reconsider?