Re: [libvirt] [Qemu-devel] QMP; unsigned 64-bit ints; JSON standards compliance

On Tue, May 14, 2019 at 08:02:49AM +0200, Markus Armbruster wrote: > Eric Blake <eblake(a)redhat.com&gt; 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? Based on the above, let me try & summarize what we need behaviour to be: - Integer mode (current default): - QEMU & clients MUST format integer fields as numbers regardless of size - QEMU & clients MUST parse number format for any integer fields - String mode: - QEMU & clients MUST format integer fields as strings if their value can not fit in a 32-bit integer. - QEMU & clients MAY format integer fields as strings even if their value can fit in 32-bit integer - QEMU & client MUST parse both string and number format for any integer fields. Unless I'm missing something, this should ensure we don't loose precision, can always parse large numbers, and can internally change QEMU precision from int8/16/32 upto full int64 without breaking clients.

Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|