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

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?