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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Tue, May 14, 2019 at 08:02:49AM +0200, Markus Armbruster wrote:
> > Eric Blake <eblake@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?
> 
> 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.

Dave

> 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 :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK