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

[Daniel P . Berrangé]

On Tue, May 14, 2019 at 10:37:55AM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (address@hidden) wrote:
> > On Tue, May 14, 2019 at 08:02:49AM +0200, Markus Armbruster wrote:
> > > Eric Blake <address@hidden> 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.
> 
> But we could be stricter and simpler in string mode:
> 
>   - QEMU & clients MUST format integer fields as strings, always
>   - QEMU & clients MUST parse only strings for integer fields.
> 
> That's (3) above, but also meets your requirements.

Yep, given that we don't actually expose the int8/int16/int32/int64
distinction via the QMP introspection data, that would be fine too.

Its basically saying we'll never use JSON's number format.

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 :|