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

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

[Daniel P. Berrangé]

The QEMU  QMP service is based on JSON which is nice because that is a
widely supported "standard" data format.....

....except QEMU's implementation (and indeed most impls) are not strictly
standards compliant.

Specifically the problem is around representing 64-bit integers, whether
signed or unsigned.

The JSON standard declares that largest integer is 2^53-1 and the
likewise the smallest is -(2^53-1):

  http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer

A crazy limit inherited from its javascript origins IIUC.

QEMU, and indeed many applications, want to handle 64-bit integers.
The C JSON library impls have traditionally mapped integers to the
data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.

QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.

Libvirt has historically used the YAJL library which uses 'long long int'
and thus can't officially go beyond 2^63-1 values. Fortunately it lets
libvirt get at the raw json string, so libvirt can re-parse the value
to get an 'unsigned long long'.

We recently tried to switch to Jansson because YAJL has a dead upstream
for many years and countless unanswered bugs & patches. Unfortunately we
forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
also provides no backdoor for libvirt todo its own integer parsing. Thus
we had to abort our switch to jansson as it broke parsing QEMU's JSON:

  https://bugzilla.redhat.com/show_bug.cgi?id=1614569

Other JSON libraries we've investigated have similar problems. I imagine
the same may well be true of non-C based JOSN impls, though I've not
investigated in any detail.

Essentially libvirt is stuck with either using the dead YAJL library
forever, or writing its own JSON parser (most likely copying QEMU's
JSON code into libvirt's git).

This feels like a very unappealing situation to be in as not being
able to use a JSON library of our choice is loosing one of the key
benefits of using a standard data format.

Thus I'd like to see a solution to this to allow QMP to be reliably
consumed by any JSON library that exists.

I can think of some options:

  1. Encode unsigned 64-bit integers as signed 64-bit integers.

     This follows the example that most C libraries map JSON ints
     to 'long long int'. This is still relying on undefined
     behaviour as apps don't need to support > 2^53-1.

     Apps would need to cast back to 'unsigned long long' for
     those QMP fields they know are supposed to be unsigned.


  2. Encode all 64-bit integers as a pair of 32-bit integers.
    
     This is fully compliant with the JSON spec as each half
     is fully within the declared limits. App has to split or
     assemble the 2 pieces from/to a signed/unsigned 64-bit
     int as needed.


  3. Encode all 64-bit integers as strings

     The application has todo all parsing/formatting client
     side.


None of these changes are backwards compatible, so I doubt we could make
the change transparently in QMP.  Instead we would have to have a
QMP greeting message capability where the client can request enablement
of the enhanced integer handling.

Any of the three options above would likely work for libvirt, but I
would have a slight preference for either 2 or 3, so that we become
100% standards compliant.

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

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

[Daniel P. Berrangé]

The QEMU  QMP service is based on JSON which is nice because that is a
widely supported "standard" data format.....

....except QEMU's implementation (and indeed most impls) are not strictly
standards compliant.

Specifically the problem is around representing 64-bit integers, whether
signed or unsigned.

The JSON standard declares that largest integer is 2^53-1 and the
likewise the smallest is -(2^53-1):

  http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer

A crazy limit inherited from its javascript origins IIUC.

QEMU, and indeed many applications, want to handle 64-bit integers.
The C JSON library impls have traditionally mapped integers to the
data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.

QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.

Libvirt has historically used the YAJL library which uses 'long long int'
and thus can't officially go beyond 2^63-1 values. Fortunately it lets
libvirt get at the raw json string, so libvirt can re-parse the value
to get an 'unsigned long long'.

We recently tried to switch to Jansson because YAJL has a dead upstream
for many years and countless unanswered bugs & patches. Unfortunately we
forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
also provides no backdoor for libvirt todo its own integer parsing. Thus
we had to abort our switch to jansson as it broke parsing QEMU's JSON:

  https://bugzilla.redhat.com/show_bug.cgi?id=1614569

Other JSON libraries we've investigated have similar problems. I imagine
the same may well be true of non-C based JOSN impls, though I've not
investigated in any detail.

Essentially libvirt is stuck with either using the dead YAJL library
forever, or writing its own JSON parser (most likely copying QEMU's
JSON code into libvirt's git).

This feels like a very unappealing situation to be in as not being
able to use a JSON library of our choice is loosing one of the key
benefits of using a standard data format.

Thus I'd like to see a solution to this to allow QMP to be reliably
consumed by any JSON library that exists.

I can think of some options:

  1. Encode unsigned 64-bit integers as signed 64-bit integers.

     This follows the example that most C libraries map JSON ints
     to 'long long int'. This is still relying on undefined
     behaviour as apps don't need to support > 2^53-1.

     Apps would need to cast back to 'unsigned long long' for
     those QMP fields they know are supposed to be unsigned.


  2. Encode all 64-bit integers as a pair of 32-bit integers.
    
     This is fully compliant with the JSON spec as each half
     is fully within the declared limits. App has to split or
     assemble the 2 pieces from/to a signed/unsigned 64-bit
     int as needed.


  3. Encode all 64-bit integers as strings

     The application has todo all parsing/formatting client
     side.


None of these changes are backwards compatible, so I doubt we could make
the change transparently in QMP.  Instead we would have to have a
QMP greeting message capability where the client can request enablement
of the enhanced integer handling.

Any of the three options above would likely work for libvirt, but I
would have a slight preference for either 2 or 3, so that we become
100% standards compliant.

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

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

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The QEMU  QMP service is based on JSON which is nice because that is a
> widely supported "standard" data format.....
> 
> ....except QEMU's implementation (and indeed most impls) are not strictly
> standards compliant.
> 
> Specifically the problem is around representing 64-bit integers, whether
> signed or unsigned.
> 
> The JSON standard declares that largest integer is 2^53-1 and the
> likewise the smallest is -(2^53-1):
> 
>   http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer
> 
> A crazy limit inherited from its javascript origins IIUC.

Ewwww.

> QEMU, and indeed many applications, want to handle 64-bit integers.
> The C JSON library impls have traditionally mapped integers to the
> data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
> 
> QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
> 
> Libvirt has historically used the YAJL library which uses 'long long int'
> and thus can't officially go beyond 2^63-1 values. Fortunately it lets
> libvirt get at the raw json string, so libvirt can re-parse the value
> to get an 'unsigned long long'.
> 
> We recently tried to switch to Jansson because YAJL has a dead upstream
> for many years and countless unanswered bugs & patches. Unfortunately we
> forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
> and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
> also provides no backdoor for libvirt todo its own integer parsing. Thus
> we had to abort our switch to jansson as it broke parsing QEMU's JSON:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
> 
> Other JSON libraries we've investigated have similar problems. I imagine
> the same may well be true of non-C based JOSN impls, though I've not
> investigated in any detail.
> 
> Essentially libvirt is stuck with either using the dead YAJL library
> forever, or writing its own JSON parser (most likely copying QEMU's
> JSON code into libvirt's git).
> 
> This feels like a very unappealing situation to be in as not being
> able to use a JSON library of our choice is loosing one of the key
> benefits of using a standard data format.
> 
> Thus I'd like to see a solution to this to allow QMP to be reliably
> consumed by any JSON library that exists.
> 
> I can think of some options:
> 
>   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> 
>      This follows the example that most C libraries map JSON ints
>      to 'long long int'. This is still relying on undefined
>      behaviour as apps don't need to support > 2^53-1.
> 
>      Apps would need to cast back to 'unsigned long long' for
>      those QMP fields they know are supposed to be unsigned.
> 
> 
>   2. Encode all 64-bit integers as a pair of 32-bit integers.
>     
>      This is fully compliant with the JSON spec as each half
>      is fully within the declared limits. App has to split or
>      assemble the 2 pieces from/to a signed/unsigned 64-bit
>      int as needed.
> 
> 
>   3. Encode all 64-bit integers as strings
> 
>      The application has todo all parsing/formatting client
>      side.
> 
> 
> None of these changes are backwards compatible, so I doubt we could make
> the change transparently in QMP.  Instead we would have to have a
> QMP greeting message capability where the client can request enablement
> of the enhanced integer handling.
> 
> Any of the three options above would likely work for libvirt, but I
> would have a slight preference for either 2 or 3, so that we become
> 100% standards compliant.

My preference would be 3 with the strings defined as being
%x lower case hex formated with a 0x prefix and no longer than 18 characters
("0x" + 16 nybbles). Zero padding allowed but not required.
It's readable and unambiguous when dealing with addresses; I don't want
to have to start decoding (2) by hand when debugging.


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

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

[Dr. David Alan Gilbert]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 4411 bytes --]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The QEMU  QMP service is based on JSON which is nice because that is a
> widely supported "standard" data format.....
> 
> ....except QEMU's implementation (and indeed most impls) are not strictly
> standards compliant.
> 
> Specifically the problem is around representing 64-bit integers, whether
> signed or unsigned.
> 
> The JSON standard declares that largest integer is 2^53-1 and the
> likewise the smallest is -(2^53-1):
> 
>   http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer
> 
> A crazy limit inherited from its javascript origins IIUC.

Ewwww.

> QEMU, and indeed many applications, want to handle 64-bit integers.
> The C JSON library impls have traditionally mapped integers to the
> data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
> 
> QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
> 
> Libvirt has historically used the YAJL library which uses 'long long int'
> and thus can't officially go beyond 2^63-1 values. Fortunately it lets
> libvirt get at the raw json string, so libvirt can re-parse the value
> to get an 'unsigned long long'.
> 
> We recently tried to switch to Jansson because YAJL has a dead upstream
> for many years and countless unanswered bugs & patches. Unfortunately we
> forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
> and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
> also provides no backdoor for libvirt todo its own integer parsing. Thus
> we had to abort our switch to jansson as it broke parsing QEMU's JSON:
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
> 
> Other JSON libraries we've investigated have similar problems. I imagine
> the same may well be true of non-C based JOSN impls, though I've not
> investigated in any detail.
> 
> Essentially libvirt is stuck with either using the dead YAJL library
> forever, or writing its own JSON parser (most likely copying QEMU's
> JSON code into libvirt's git).
> 
> This feels like a very unappealing situation to be in as not being
> able to use a JSON library of our choice is loosing one of the key
> benefits of using a standard data format.
> 
> Thus I'd like to see a solution to this to allow QMP to be reliably
> consumed by any JSON library that exists.
> 
> I can think of some options:
> 
>   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> 
>      This follows the example that most C libraries map JSON ints
>      to 'long long int'. This is still relying on undefined
>      behaviour as apps don't need to support > 2^53-1.
> 
>      Apps would need to cast back to 'unsigned long long' for
>      those QMP fields they know are supposed to be unsigned.
> 
> 
>   2. Encode all 64-bit integers as a pair of 32-bit integers.
>     
>      This is fully compliant with the JSON spec as each half
>      is fully within the declared limits. App has to split or
>      assemble the 2 pieces from/to a signed/unsigned 64-bit
>      int as needed.
> 
> 
>   3. Encode all 64-bit integers as strings
> 
>      The application has todo all parsing/formatting client
>      side.
> 
> 
> None of these changes are backwards compatible, so I doubt we could make
> the change transparently in QMP.  Instead we would have to have a
> QMP greeting message capability where the client can request enablement
> of the enhanced integer handling.
> 
> Any of the three options above would likely work for libvirt, but I
> would have a slight preference for either 2 or 3, so that we become
> 100% standards compliant.

My preference would be 3 with the strings defined as being
%x lower case hex formated with a 0x prefix and no longer than 18 characters
("0x" + 16 nybbles). Zero padding allowed but not required.
It's readable and unambiguous when dealing with addresses; I don't want
to have to start decoding (2) by hand when debugging.


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

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

[Daniel P. Berrangé]

On Tue, Apr 30, 2019 at 03:45:46PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > The QEMU  QMP service is based on JSON which is nice because that is a
> > widely supported "standard" data format.....
> > 
> > ....except QEMU's implementation (and indeed most impls) are not strictly
> > standards compliant.
> > 
> > Specifically the problem is around representing 64-bit integers, whether
> > signed or unsigned.
> > 
> > The JSON standard declares that largest integer is 2^53-1 and the
> > likewise the smallest is -(2^53-1):
> > 
> >   http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer
> > 
> > A crazy limit inherited from its javascript origins IIUC.
> 
> Ewwww.

Looking a bit deeper it seems this limit comes from the use of double
precision floating point for storing integers. 2^53-1 is the largest
integer value that can be stored in a 64-bit float without loss of
precision.

The Golang JSON parser decodes JSON numbers to float64 by default so
will have this precision limitation too, though at least they provide
a backdoor for custom parsing from the original serialized representation.

> > QEMU, and indeed many applications, want to handle 64-bit integers.
> > The C JSON library impls have traditionally mapped integers to the
> > data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
> > 
> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
> > 
> > Libvirt has historically used the YAJL library which uses 'long long int'
> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets
> > libvirt get at the raw json string, so libvirt can re-parse the value
> > to get an 'unsigned long long'.
> > 
> > We recently tried to switch to Jansson because YAJL has a dead upstream
> > for many years and countless unanswered bugs & patches. Unfortunately we
> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
> > and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
> > also provides no backdoor for libvirt todo its own integer parsing. Thus
> > we had to abort our switch to jansson as it broke parsing QEMU's JSON:
> > 
> >   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
> > 
> > Other JSON libraries we've investigated have similar problems. I imagine
> > the same may well be true of non-C based JOSN impls, though I've not
> > investigated in any detail.
> > 
> > Essentially libvirt is stuck with either using the dead YAJL library
> > forever, or writing its own JSON parser (most likely copying QEMU's
> > JSON code into libvirt's git).
> > 
> > This feels like a very unappealing situation to be in as not being
> > able to use a JSON library of our choice is loosing one of the key
> > benefits of using a standard data format.
> > 
> > Thus I'd like to see a solution to this to allow QMP to be reliably
> > consumed by any JSON library that exists.
> > 
> > I can think of some options:
> > 
> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> > 
> >      This follows the example that most C libraries map JSON ints
> >      to 'long long int'. This is still relying on undefined
> >      behaviour as apps don't need to support > 2^53-1.
> > 
> >      Apps would need to cast back to 'unsigned long long' for
> >      those QMP fields they know are supposed to be unsigned.
> > 
> > 
> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
> >     
> >      This is fully compliant with the JSON spec as each half
> >      is fully within the declared limits. App has to split or
> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
> >      int as needed.
> > 
> > 
> >   3. Encode all 64-bit integers as strings
> > 
> >      The application has todo all parsing/formatting client
> >      side.
> > 
> > 
> > None of these changes are backwards compatible, so I doubt we could make
> > the change transparently in QMP.  Instead we would have to have a
> > QMP greeting message capability where the client can request enablement
> > of the enhanced integer handling.
> > 
> > Any of the three options above would likely work for libvirt, but I
> > would have a slight preference for either 2 or 3, so that we become
> > 100% standards compliant.
> 
> My preference would be 3 with the strings defined as being
> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> ("0x" + 16 nybbles). Zero padding allowed but not required.
> It's readable and unambiguous when dealing with addresses; I don't want
> to have to start decoding (2) by hand when debugging.

Yep, that's a good point about readability.

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

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

[Daniel P. Berrangé]

On Tue, Apr 30, 2019 at 03:45:46PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > The QEMU  QMP service is based on JSON which is nice because that is a
> > widely supported "standard" data format.....
> > 
> > ....except QEMU's implementation (and indeed most impls) are not strictly
> > standards compliant.
> > 
> > Specifically the problem is around representing 64-bit integers, whether
> > signed or unsigned.
> > 
> > The JSON standard declares that largest integer is 2^53-1 and the
> > likewise the smallest is -(2^53-1):
> > 
> >   http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer
> > 
> > A crazy limit inherited from its javascript origins IIUC.
> 
> Ewwww.

Looking a bit deeper it seems this limit comes from the use of double
precision floating point for storing integers. 2^53-1 is the largest
integer value that can be stored in a 64-bit float without loss of
precision.

The Golang JSON parser decodes JSON numbers to float64 by default so
will have this precision limitation too, though at least they provide
a backdoor for custom parsing from the original serialized representation.

> > QEMU, and indeed many applications, want to handle 64-bit integers.
> > The C JSON library impls have traditionally mapped integers to the
> > data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
> > 
> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
> > 
> > Libvirt has historically used the YAJL library which uses 'long long int'
> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets
> > libvirt get at the raw json string, so libvirt can re-parse the value
> > to get an 'unsigned long long'.
> > 
> > We recently tried to switch to Jansson because YAJL has a dead upstream
> > for many years and countless unanswered bugs & patches. Unfortunately we
> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
> > and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
> > also provides no backdoor for libvirt todo its own integer parsing. Thus
> > we had to abort our switch to jansson as it broke parsing QEMU's JSON:
> > 
> >   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
> > 
> > Other JSON libraries we've investigated have similar problems. I imagine
> > the same may well be true of non-C based JOSN impls, though I've not
> > investigated in any detail.
> > 
> > Essentially libvirt is stuck with either using the dead YAJL library
> > forever, or writing its own JSON parser (most likely copying QEMU's
> > JSON code into libvirt's git).
> > 
> > This feels like a very unappealing situation to be in as not being
> > able to use a JSON library of our choice is loosing one of the key
> > benefits of using a standard data format.
> > 
> > Thus I'd like to see a solution to this to allow QMP to be reliably
> > consumed by any JSON library that exists.
> > 
> > I can think of some options:
> > 
> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> > 
> >      This follows the example that most C libraries map JSON ints
> >      to 'long long int'. This is still relying on undefined
> >      behaviour as apps don't need to support > 2^53-1.
> > 
> >      Apps would need to cast back to 'unsigned long long' for
> >      those QMP fields they know are supposed to be unsigned.
> > 
> > 
> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
> >     
> >      This is fully compliant with the JSON spec as each half
> >      is fully within the declared limits. App has to split or
> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
> >      int as needed.
> > 
> > 
> >   3. Encode all 64-bit integers as strings
> > 
> >      The application has todo all parsing/formatting client
> >      side.
> > 
> > 
> > None of these changes are backwards compatible, so I doubt we could make
> > the change transparently in QMP.  Instead we would have to have a
> > QMP greeting message capability where the client can request enablement
> > of the enhanced integer handling.
> > 
> > Any of the three options above would likely work for libvirt, but I
> > would have a slight preference for either 2 or 3, so that we become
> > 100% standards compliant.
> 
> My preference would be 3 with the strings defined as being
> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> ("0x" + 16 nybbles). Zero padding allowed but not required.
> It's readable and unambiguous when dealing with addresses; I don't want
> to have to start decoding (2) by hand when debugging.

Yep, that's a good point about readability.

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

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

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Apr 30, 2019 at 03:45:46PM +0100, Dr. David Alan Gilbert wrote:
>> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>> > The QEMU  QMP service is based on JSON which is nice because that is a
>> > widely supported "standard" data format.....
>> > 
>> > ....except QEMU's implementation (and indeed most impls) are not strictly
>> > standards compliant.

As we'll see below, speaking of "strict standard compliance" is
meaningless, because the standard is too weak.  What you mean is "lowest
common denominator of relevant implementations".

>> > Specifically the problem is around representing 64-bit integers, whether
>> > signed or unsigned.
>> > 
>> > The JSON standard declares that largest integer is 2^53-1 and the
>> > likewise the smallest is -(2^53-1):
>> > 
>> >   http://www.ecma-international.org/ecma-262/6.0/index.html#sec-number.max_safe_integer

This is the ECMAScript standard, not the JSON standard.  For JSON, the
situation is more confused.  See below.

>> > A crazy limit inherited from its javascript origins IIUC.
>> 
>> Ewwww.
>
> Looking a bit deeper it seems this limit comes from the use of double
> precision floating point for storing integers. 2^53-1 is the largest
> integer value that can be stored in a 64-bit float without loss of
> precision.

Correct.  JavaScript has only one type of number, and they're all
represented as double precision floating-point.  Perfectly sensible for
a prototype done in ten days.  Move fast and leave things broken
forever.

JSON is derived from JavaScript.  RFC 4672 is notoriously useless on
precision and range.  RFC 7159 attempts to contain the damage by
spelling out how useless it:

    This specification allows implementations to set limits on the range
    and precision of numbers accepted.  Since software that implements
    IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is
    generally available and widely used, good interoperability can be
    achieved by implementations that expect no more precision or range
    than these provide, in the sense that implementations will
    approximate JSON numbers within the expected precision.  A JSON
    number such as 1E400 or 3.141592653589793238462643383279 may indicate
    potential interoperability problems, since it suggests that the
    software that created it expects receiving software to have greater
    capabilities for numeric magnitude and precision than is widely
    available.

    Note that when such software is used, numbers that are integers and
    are in the range [-(2**53)+1, (2**53)-1] are interoperable in the
    sense that implementations will agree exactly on their numeric
    values.

RFC 8259 keeps this unchanged.

QEMU does conform to RFC 8259.  It sets limits on range and precision of
numbers accepted: double precision \union the integers between INT64_MIN
and UINT64_MAX inclusive.  The trouble is that conformance doesn't imply
interoperability.

Many other implementations follow JavaScript and limit numbers to
double.

> The Golang JSON parser decodes JSON numbers to float64 by default so
> will have this precision limitation too, though at least they provide
> a backdoor for custom parsing from the original serialized representation.
>
>> > QEMU, and indeed many applications, want to handle 64-bit integers.
>> > The C JSON library impls have traditionally mapped integers to the
>> > data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
>> > 
>> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.

Correct.

Support for integers 2^63..2^64-1 is relatively recent: commit
2bc7cfea095 (v2.10, 2017).

Since we really needed these, the QObject input visitor silently casts
negative integers to uint64_t.  It still does for backward
compatibility.  Commit 5923f85fb82 (right after 2bc7cfea095) explains

    The input visitor will cast i64 input to u64 for compatibility
    reasons (existing json QMP client already use negative i64 for large
    u64, and expect an implicit cast in qemu).
    
    Note: before the patch, uint64_t values above INT64_MAX are sent over
    json QMP as negative values, e.g. UINT64_MAX is sent as -1. After the
    patch, they are sent unmodified.  Clearly a bug fix, but we have to
    consider compatibility issues anyway.  libvirt should cope fine,
    because its parsing of unsigned integers accepts negative values
    modulo 2^64.  There's hope that other clients will, too.

>> > Libvirt has historically used the YAJL library which uses 'long long int'
>> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets
>> > libvirt get at the raw json string, so libvirt can re-parse the value
>> > to get an 'unsigned long long'.
>> > 
>> > We recently tried to switch to Jansson because YAJL has a dead upstream
>> > for many years and countless unanswered bugs & patches. Unfortunately we
>> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
>> > and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
>> > also provides no backdoor for libvirt todo its own integer parsing. Thus
>> > we had to abort our switch to jansson as it broke parsing QEMU's JSON:
>> > 
>> >   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
>> > 
>> > Other JSON libraries we've investigated have similar problems. I imagine
>> > the same may well be true of non-C based JOSN impls, though I've not
>> > investigated in any detail.
>> > 
>> > Essentially libvirt is stuck with either using the dead YAJL library
>> > forever, or writing its own JSON parser (most likely copying QEMU's
>> > JSON code into libvirt's git).
>> > 
>> > This feels like a very unappealing situation to be in as not being
>> > able to use a JSON library of our choice is loosing one of the key
>> > benefits of using a standard data format.
>> > 
>> > Thus I'd like to see a solution to this to allow QMP to be reliably
>> > consumed by any JSON library that exists.

JSON is terrible at interoperability, so good luck with that.

If you reduce your order to "the commonly used JSON libraries we know",
we can talk.

>> > I can think of some options:
>> > 
>> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
>> > 
>> >      This follows the example that most C libraries map JSON ints
>> >      to 'long long int'. This is still relying on undefined
>> >      behaviour as apps don't need to support > 2^53-1.
>> > 
>> >      Apps would need to cast back to 'unsigned long long' for
>> >      those QMP fields they know are supposed to be unsigned.

Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
direction still does it, for backward compatibility.

>> > 
>> > 
>> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
>> >     
>> >      This is fully compliant with the JSON spec as each half
>> >      is fully within the declared limits. App has to split or
>> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
>> >      int as needed.

Differently ugly.

>> > 
>> > 
>> >   3. Encode all 64-bit integers as strings
>> > 
>> >      The application has todo all parsing/formatting client
>> >      side.

Yet another ugly.

>> > 
>> > 
>> > None of these changes are backwards compatible, so I doubt we could make
>> > the change transparently in QMP.  Instead we would have to have a
>> > QMP greeting message capability where the client can request enablement
>> > of the enhanced integer handling.

We might be able to do option 1 without capability negotiation.  v2.10's
change from option 1 to what we have now produced zero complaints.

On the other hand, we made that change for a reason, so we may want a
"send large integers as negative integers" capability regardless.

>> > 
>> > Any of the three options above would likely work for libvirt, but I
>> > would have a slight preference for either 2 or 3, so that we become
>> > 100% standards compliant.

There's no such thing.  You mean "we maximize interoperability with
common implementations of JSON".

Let's talk implementation for a bit.

Encoding and decoding integers in funny ways should be fairly easy in
the QObject visitors.  The generated QMP marshallers all use them.
Trouble is a few commands still bypass the generated marshallers, and
mess with the QObject themselves:

* query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
  comment.  Should be harmless.

* netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
  because they reject some abuses like passing numbers and bools as
  strings.

* device_add: not QAPIfied.  We're not sure QAPIfication is feasible.

netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
could hack that to mirror what the QObject visitor do.

Else, we might have to do it in the JSON parser.  Should be possible,
but I'd rather not.

>> My preference would be 3 with the strings defined as being
>> %x lower case hex formated with a 0x prefix and no longer than 18 characters
>> ("0x" + 16 nybbles). Zero padding allowed but not required.
>> It's readable and unambiguous when dealing with addresses; I don't want
>> to have to start decoding (2) by hand when debugging.
>
> Yep, that's a good point about readability.

QMP sending all integers in decimal is inconvenient for some values,
such as addresses.  QMP sending all (large) integers in hexadecimal
would be inconvenient for other values.

Let's keep it simple & stupid.  If you want sophistication, JSON is the
wrong choice.


Option 1 feels simplest.

Option 2 feels ugliest.  Less simple, more interoperable than option 1.

Option 3 is like option 2, just not quite as ugly.

Can we agree to eliminate option 2 from the race?

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

[Daniel P. Berrangé]

On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote:

> > The Golang JSON parser decodes JSON numbers to float64 by default so
> > will have this precision limitation too, though at least they provide
> > a backdoor for custom parsing from the original serialized representation.
> >
> >> > QEMU, and indeed many applications, want to handle 64-bit integers.
> >> > The C JSON library impls have traditionally mapped integers to the
> >> > data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
> >> > 
> >> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
> 
> Correct.
> 
> Support for integers 2^63..2^64-1 is relatively recent: commit
> 2bc7cfea095 (v2.10, 2017).
> 
> Since we really needed these, the QObject input visitor silently casts
> negative integers to uint64_t.  It still does for backward
> compatibility.  Commit 5923f85fb82 (right after 2bc7cfea095) explains
> 
>     The input visitor will cast i64 input to u64 for compatibility
>     reasons (existing json QMP client already use negative i64 for large
>     u64, and expect an implicit cast in qemu).
>     
>     Note: before the patch, uint64_t values above INT64_MAX are sent over
>     json QMP as negative values, e.g. UINT64_MAX is sent as -1. After the
>     patch, they are sent unmodified.  Clearly a bug fix, but we have to
>     consider compatibility issues anyway.  libvirt should cope fine,
>     because its parsing of unsigned integers accepts negative values
>     modulo 2^64.  There's hope that other clients will, too.

So QEMU reading stuff sent by libvirt in a back compatible manner is
ok. The problem was specifically when a QEMU reply sent back UINT64_MAX
value as a positive number.


> >> > Libvirt has historically used the YAJL library which uses 'long long int'
> >> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets
> >> > libvirt get at the raw json string, so libvirt can re-parse the value
> >> > to get an 'unsigned long long'.
> >> > 
> >> > We recently tried to switch to Jansson because YAJL has a dead upstream
> >> > for many years and countless unanswered bugs & patches. Unfortunately we
> >> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
> >> > and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
> >> > also provides no backdoor for libvirt todo its own integer parsing. Thus
> >> > we had to abort our switch to jansson as it broke parsing QEMU's JSON:
> >> > 
> >> >   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
> >> > 
> >> > Other JSON libraries we've investigated have similar problems. I imagine
> >> > the same may well be true of non-C based JOSN impls, though I've not
> >> > investigated in any detail.
> >> > 
> >> > Essentially libvirt is stuck with either using the dead YAJL library
> >> > forever, or writing its own JSON parser (most likely copying QEMU's
> >> > JSON code into libvirt's git).
> >> > 
> >> > This feels like a very unappealing situation to be in as not being
> >> > able to use a JSON library of our choice is loosing one of the key
> >> > benefits of using a standard data format.
> >> > 
> >> > Thus I'd like to see a solution to this to allow QMP to be reliably
> >> > consumed by any JSON library that exists.
> 
> JSON is terrible at interoperability, so good luck with that.
> 
> If you reduce your order to "the commonly used JSON libraries we know",
> we can talk.

I don't particularly want us to rely on semantics of small known set
of JSON libs. I really do want us to do something that is capable of
working with any JSON impl that exists in any programming language.

My suggested option 2 & 3 at least would manage that I believe, as
any credible JSON impl will be able to represent 32-bit integers
or strings without loosing data.

Option 1 would not cope as some impls can't even cope with
signed 64-bit ints.

> >> > I can think of some options:
> >> > 
> >> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> >> > 
> >> >      This follows the example that most C libraries map JSON ints
> >> >      to 'long long int'. This is still relying on undefined
> >> >      behaviour as apps don't need to support > 2^53-1.
> >> > 
> >> >      Apps would need to cast back to 'unsigned long long' for
> >> >      those QMP fields they know are supposed to be unsigned.
> 
> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
> direction still does it, for backward compatibility.
> 
> >> > 
> >> > 
> >> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
> >> >     
> >> >      This is fully compliant with the JSON spec as each half
> >> >      is fully within the declared limits. App has to split or
> >> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
> >> >      int as needed.
> 
> Differently ugly.
> 
> >> > 
> >> > 
> >> >   3. Encode all 64-bit integers as strings
> >> > 
> >> >      The application has todo all parsing/formatting client
> >> >      side.
> 
> Yet another ugly.
> 
> >> > 
> >> > 
> >> > None of these changes are backwards compatible, so I doubt we could make
> >> > the change transparently in QMP.  Instead we would have to have a
> >> > QMP greeting message capability where the client can request enablement
> >> > of the enhanced integer handling.
> 
> We might be able to do option 1 without capability negotiation.  v2.10's
> change from option 1 to what we have now produced zero complaints.
> 
> On the other hand, we made that change for a reason, so we may want a
> "send large integers as negative integers" capability regardless.
> 
> >> > 
> >> > Any of the three options above would likely work for libvirt, but I
> >> > would have a slight preference for either 2 or 3, so that we become
> >> > 100% standards compliant.
> 
> There's no such thing.  You mean "we maximize interoperability with
> common implementations of JSON".

s/common/any/

> Let's talk implementation for a bit.
> 
> Encoding and decoding integers in funny ways should be fairly easy in
> the QObject visitors.  The generated QMP marshallers all use them.
> Trouble is a few commands still bypass the generated marshallers, and
> mess with the QObject themselves:
> 
> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
>   comment.  Should be harmless.
> 
> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
>   because they reject some abuses like passing numbers and bools as
>   strings.
> 
> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
> 
> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
> could hack that to mirror what the QObject visitor do.
> 
> Else, we might have to do it in the JSON parser.  Should be possible,
> but I'd rather not.
> 
> >> My preference would be 3 with the strings defined as being
> >> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> >> ("0x" + 16 nybbles). Zero padding allowed but not required.
> >> It's readable and unambiguous when dealing with addresses; I don't want
> >> to have to start decoding (2) by hand when debugging.
> >
> > Yep, that's a good point about readability.
> 
> QMP sending all integers in decimal is inconvenient for some values,
> such as addresses.  QMP sending all (large) integers in hexadecimal
> would be inconvenient for other values.
> 
> Let's keep it simple & stupid.  If you want sophistication, JSON is the
> wrong choice.
> 
> 
> Option 1 feels simplest.

But will still fail with any JSON impl that uses double precision floating
point for integers as it will loose precision.

> Option 2 feels ugliest.  Less simple, more interoperable than option 1.

If we assume any JSON impl can do 32-bit integers without loss of
precision, then I think we can say it is guaranteed portable, but
it is certainly horrible / ugly.

> Option 3 is like option 2, just not quite as ugly.

I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
that exists, since you're delegating all string -> int conversion to
the application code taking the JSON parser/formatter out of the equation.

This is close to the approach libvirt takes with YAJL parser today. YAJL
parses as a int64 and we then ignore its result, and re-parse the string
again in libvirt as uint64. When generating json we format as uint64
in libvirt and ignore YAJLs formatting for int64.

> Can we agree to eliminate option 2 from the race?

I'm fine with eliminating option 2.

I guess I'd have a preference for option 3 given that it has better
interoperability

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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 7790 bytes --]

On 5/7/19 4:39 AM, Daniel P. Berrangé wrote:

>> JSON is terrible at interoperability, so good luck with that.
>>
>> If you reduce your order to "the commonly used JSON libraries we know",
>> we can talk.
> 
> I don't particularly want us to rely on semantics of small known set
> of JSON libs. I really do want us to do something that is capable of
> working with any JSON impl that exists in any programming language.
> 
> My suggested option 2 & 3 at least would manage that I believe, as
> any credible JSON impl will be able to represent 32-bit integers
> or strings without loosing data.
> 
> Option 1 would not cope as some impls can't even cope with
> signed 64-bit ints.
> 
>>>>> I can think of some options:
>>>>>
>>>>>   1. Encode unsigned 64-bit integers as signed 64-bit integers.
>>>>>
>>>>>      This follows the example that most C libraries map JSON ints
>>>>>      to 'long long int'. This is still relying on undefined
>>>>>      behaviour as apps don't need to support > 2^53-1.
>>>>>
>>>>>      Apps would need to cast back to 'unsigned long long' for
>>>>>      those QMP fields they know are supposed to be unsigned.
>>
>> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
>> direction still does it, for backward compatibility.

Having qemu accept signed ints in place of large unsigned values is easy
enough. But you are right that it loses precision when doubles are
involved on the receiving end, and we cross the 2^53 barrier.

>>
>>>>>
>>>>>
>>>>>   2. Encode all 64-bit integers as a pair of 32-bit integers.
>>>>>     
>>>>>      This is fully compliant with the JSON spec as each half
>>>>>      is fully within the declared limits. App has to split or
>>>>>      assemble the 2 pieces from/to a signed/unsigned 64-bit
>>>>>      int as needed.
>>
>> Differently ugly.

Particularly ugly as we turn 1<<55 from:

"value":36028797018963968

into

"value":[8388608,0]

and now both qemu and the client end have to agree that an array of two
integers is a valid replacement for any larger 64-bit quantity
(presumably, we'd always accept the array form even for small integer
values, but only produce the array form for large values).  And while it
manages just fine for uint64_t values, what rules would you place on
int64_t values? That the resulting 2-integer array is combined with the
first number as a 2's-complement signed value, and the second being a
32-bit unsigned value?

>>
>>>>>
>>>>>
>>>>>   3. Encode all 64-bit integers as strings
>>>>>
>>>>>      The application has todo all parsing/formatting client
>>>>>      side.
>>
>> Yet another ugly.

But less so than option 2.

"value":36028797018963968

vs.

"value":"36028797018963968"

is at least tolerable.

>>
>>>>>
>>>>>
>>>>> None of these changes are backwards compatible, so I doubt we could make
>>>>> the change transparently in QMP.  Instead we would have to have a
>>>>> QMP greeting message capability where the client can request enablement
>>>>> of the enhanced integer handling.
>>
>> We might be able to do option 1 without capability negotiation.  v2.10's
>> change from option 1 to what we have now produced zero complaints.
>>
>> On the other hand, we made that change for a reason, so we may want a
>> "send large integers as negative integers" capability regardless.
>>
>>>>>
>>>>> Any of the three options above would likely work for libvirt, but I
>>>>> would have a slight preference for either 2 or 3, so that we become
>>>>> 100% standards compliant.

If we're going to negotiate something, I'd lean towards option 3
(anywhere the introspection states that we accept 'int64' or similar, it
is also appropriate to send a string value in its place). We'd also have
to decide if we want to allow "0xabcd", or strictly insist on 43981,
when stringizing an integer.  And while qemu should accept a string or a
number on input, we'd still have to decide/document whether it's
response to the client capability negotiation is to output a string
always, or only for values larger than the 2^53 threshold.

>>
>> There's no such thing.  You mean "we maximize interoperability with
>> common implementations of JSON".
> 
> s/common/any/
> 
>> Let's talk implementation for a bit.
>>
>> Encoding and decoding integers in funny ways should be fairly easy in
>> the QObject visitors.  The generated QMP marshallers all use them.
>> Trouble is a few commands still bypass the generated marshallers, and
>> mess with the QObject themselves:
>>
>> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
>>   comment.  Should be harmless.
>>
>> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
>>   because they reject some abuses like passing numbers and bools as
>>   strings.
>>
>> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
>>
>> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
>> could hack that to mirror what the QObject visitor do.
>>
>> Else, we might have to do it in the JSON parser.  Should be possible,
>> but I'd rather not.
>>
>>>> My preference would be 3 with the strings defined as being
>>>> %x lower case hex formated with a 0x prefix and no longer than 18 characters
>>>> ("0x" + 16 nybbles). Zero padding allowed but not required.
>>>> It's readable and unambiguous when dealing with addresses; I don't want
>>>> to have to start decoding (2) by hand when debugging.
>>>
>>> Yep, that's a good point about readability.
>>
>> QMP sending all integers in decimal is inconvenient for some values,
>> such as addresses.  QMP sending all (large) integers in hexadecimal
>> would be inconvenient for other values.
>>
>> Let's keep it simple & stupid.  If you want sophistication, JSON is the
>> wrong choice.

JSON requires decimal-only, but I'm okay if we state that when
negotiating the alternative representation, that we output hex-only.
(JSON5 adds hex support among other things, but it is not an RFC
standard, and even fewer libraries exist that parse JSON5 in addition to
straight JSON).

>>
>>
>> Option 1 feels simplest.
> 
> But will still fail with any JSON impl that uses double precision floating
> point for integers as it will loose precision.
> 
>> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
> 
> If we assume any JSON impl can do 32-bit integers without loss of
> precision, then I think we can say it is guaranteed portable, but
> it is certainly horrible / ugly.
> 
>> Option 3 is like option 2, just not quite as ugly.
> 
> I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
> that exists, since you're delegating all string -> int conversion to
> the application code taking the JSON parser/formatter out of the equation.
> 
> This is close to the approach libvirt takes with YAJL parser today. YAJL
> parses as a int64 and we then ignore its result, and re-parse the string
> again in libvirt as uint64. When generating json we format as uint64
> in libvirt and ignore YAJLs formatting for int64.
> 
>> Can we agree to eliminate option 2 from the race?
> 
> I'm fine with eliminating option 2.

Same here.

> 
> I guess I'd have a preference for option 3 given that it has better
> interoperability

Likewise - if we're going to bother with a capability that changes
output and allows the input validators to accept more forms, I'd prefer
a string form with correct sign over a negative integer that depends on
64-bit 2's-complement arithmetic to intepret correctly.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 5/7/19 4:39 AM, Daniel P. Berrangé wrote:
>
>>> JSON is terrible at interoperability, so good luck with that.
>>>
>>> If you reduce your order to "the commonly used JSON libraries we know",
>>> we can talk.
>> 
>> I don't particularly want us to rely on semantics of small known set
>> of JSON libs. I really do want us to do something that is capable of
>> working with any JSON impl that exists in any programming language.
>> 
>> My suggested option 2 & 3 at least would manage that I believe, as
>> any credible JSON impl will be able to represent 32-bit integers
>> or strings without loosing data.
>> 
>> Option 1 would not cope as some impls can't even cope with
>> signed 64-bit ints.
>> 
>>>>>> I can think of some options:
>>>>>>
>>>>>>   1. Encode unsigned 64-bit integers as signed 64-bit integers.
>>>>>>
>>>>>>      This follows the example that most C libraries map JSON ints
>>>>>>      to 'long long int'. This is still relying on undefined
>>>>>>      behaviour as apps don't need to support > 2^53-1.
>>>>>>
>>>>>>      Apps would need to cast back to 'unsigned long long' for
>>>>>>      those QMP fields they know are supposed to be unsigned.
>>>
>>> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
>>> direction still does it, for backward compatibility.
>
> Having qemu accept signed ints in place of large unsigned values is easy
> enough. But you are right that it loses precision when doubles are
> involved on the receiving end, and we cross the 2^53 barrier.
>
>>>
>>>>>>
>>>>>>
>>>>>>   2. Encode all 64-bit integers as a pair of 32-bit integers.
>>>>>>     
>>>>>>      This is fully compliant with the JSON spec as each half
>>>>>>      is fully within the declared limits. App has to split or
>>>>>>      assemble the 2 pieces from/to a signed/unsigned 64-bit
>>>>>>      int as needed.
>>>
>>> Differently ugly.
>
> Particularly ugly as we turn 1<<55 from:
>
> "value":36028797018963968
>
> into
>
> "value":[8388608,0]
>
> and now both qemu and the client end have to agree that an array of two
> integers is a valid replacement for any larger 64-bit quantity
> (presumably, we'd always accept the array form even for small integer
> values, but only produce the array form for large values).  And while it
> manages just fine for uint64_t values, what rules would you place on
> int64_t values? That the resulting 2-integer array is combined with the
> first number as a 2's-complement signed value, and the second being a
> 32-bit unsigned value?

There's more than one way to encode integers as a list of 53 bit signed
integers.  Any of them will do, we just have to specify one.

>>>>>>
>>>>>>
>>>>>>   3. Encode all 64-bit integers as strings
>>>>>>
>>>>>>      The application has todo all parsing/formatting client
>>>>>>      side.
>>>
>>> Yet another ugly.
>
> But less so than option 2.
>
> "value":36028797018963968
>
> vs.
>
> "value":"36028797018963968"
>
> is at least tolerable.

Yes.

>>>>>> None of these changes are backwards compatible, so I doubt we could make
>>>>>> the change transparently in QMP.  Instead we would have to have a
>>>>>> QMP greeting message capability where the client can request enablement
>>>>>> of the enhanced integer handling.
>>>
>>> We might be able to do option 1 without capability negotiation.  v2.10's
>>> change from option 1 to what we have now produced zero complaints.
>>>
>>> On the other hand, we made that change for a reason, so we may want a
>>> "send large integers as negative integers" capability regardless.
>>>
>>>>>>
>>>>>> Any of the three options above would likely work for libvirt, but I
>>>>>> would have a slight preference for either 2 or 3, so that we become
>>>>>> 100% standards compliant.
>
> If we're going to negotiate something, I'd lean towards option 3
> (anywhere the introspection states that we accept 'int64' or similar, it
> is also appropriate to send a string value in its place). We'd also have
> to decide if we want to allow "0xabcd", or strictly insist on 43981,
> when stringizing an integer.  And while qemu should accept a string or a
> number on input, we'd still have to decide/document whether it's
> response to the client capability negotiation is to output a string
> always, or only for values larger than the 2^53 threshold.

Picking option 3 is no excuse for complicating matters further.  QMP is
primarily for machines.  So my first choice would be to keep everything
decimal.  I could be persuaded to have QEMU parse integers from strings
with base 0, i.e. leading 0x gets you hex, leading 0 gets you octal.

>>>
>>> There's no such thing.  You mean "we maximize interoperability with
>>> common implementations of JSON".
>> 
>> s/common/any/
>> 
>>> Let's talk implementation for a bit.
>>>
>>> Encoding and decoding integers in funny ways should be fairly easy in
>>> the QObject visitors.  The generated QMP marshallers all use them.
>>> Trouble is a few commands still bypass the generated marshallers, and
>>> mess with the QObject themselves:
>>>
>>> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
>>>   comment.  Should be harmless.
>>>
>>> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
>>>   because they reject some abuses like passing numbers and bools as
>>>   strings.
>>>
>>> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
>>>
>>> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
>>> could hack that to mirror what the QObject visitor do.
>>>
>>> Else, we might have to do it in the JSON parser.  Should be possible,
>>> but I'd rather not.
>>>
>>>>> My preference would be 3 with the strings defined as being
>>>>> %x lower case hex formated with a 0x prefix and no longer than 18 characters
>>>>> ("0x" + 16 nybbles). Zero padding allowed but not required.
>>>>> It's readable and unambiguous when dealing with addresses; I don't want
>>>>> to have to start decoding (2) by hand when debugging.
>>>>
>>>> Yep, that's a good point about readability.
>>>
>>> QMP sending all integers in decimal is inconvenient for some values,
>>> such as addresses.  QMP sending all (large) integers in hexadecimal
>>> would be inconvenient for other values.
>>>
>>> Let's keep it simple & stupid.  If you want sophistication, JSON is the
>>> wrong choice.
>
> JSON requires decimal-only, but I'm okay if we state that when
> negotiating the alternative representation, that we output hex-only.
> (JSON5 adds hex support among other things, but it is not an RFC
> standard, and even fewer libraries exist that parse JSON5 in addition to
> straight JSON).
>
>>>
>>>
>>> Option 1 feels simplest.
>> 
>> But will still fail with any JSON impl that uses double precision floating
>> point for integers as it will loose precision.
>> 
>>> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
>> 
>> If we assume any JSON impl can do 32-bit integers without loss of
>> precision, then I think we can say it is guaranteed portable, but
>> it is certainly horrible / ugly.
>> 
>>> Option 3 is like option 2, just not quite as ugly.
>> 
>> I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
>> that exists, since you're delegating all string -> int conversion to
>> the application code taking the JSON parser/formatter out of the equation.
>> 
>> This is close to the approach libvirt takes with YAJL parser today. YAJL
>> parses as a int64 and we then ignore its result, and re-parse the string
>> again in libvirt as uint64. When generating json we format as uint64
>> in libvirt and ignore YAJLs formatting for int64.
>> 
>>> Can we agree to eliminate option 2 from the race?
>> 
>> I'm fine with eliminating option 2.
>
> Same here.

Noted.

>> I guess I'd have a preference for option 3 given that it has better
>> interoperability
>
> Likewise - if we're going to bother with a capability that changes
> output and allows the input validators to accept more forms, I'd prefer
> a string form with correct sign over a negative integer that depends on
> 64-bit 2's-complement arithmetic to intepret correctly.

Noted.

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

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 5/7/19 4:39 AM, Daniel P. Berrangé wrote:
> >
> >>> JSON is terrible at interoperability, so good luck with that.
> >>>
> >>> If you reduce your order to "the commonly used JSON libraries we know",
> >>> we can talk.
> >> 
> >> I don't particularly want us to rely on semantics of small known set
> >> of JSON libs. I really do want us to do something that is capable of
> >> working with any JSON impl that exists in any programming language.
> >> 
> >> My suggested option 2 & 3 at least would manage that I believe, as
> >> any credible JSON impl will be able to represent 32-bit integers
> >> or strings without loosing data.
> >> 
> >> Option 1 would not cope as some impls can't even cope with
> >> signed 64-bit ints.
> >> 
> >>>>>> I can think of some options:
> >>>>>>
> >>>>>>   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> >>>>>>
> >>>>>>      This follows the example that most C libraries map JSON ints
> >>>>>>      to 'long long int'. This is still relying on undefined
> >>>>>>      behaviour as apps don't need to support > 2^53-1.
> >>>>>>
> >>>>>>      Apps would need to cast back to 'unsigned long long' for
> >>>>>>      those QMP fields they know are supposed to be unsigned.
> >>>
> >>> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
> >>> direction still does it, for backward compatibility.
> >
> > Having qemu accept signed ints in place of large unsigned values is easy
> > enough. But you are right that it loses precision when doubles are
> > involved on the receiving end, and we cross the 2^53 barrier.
> >
> >>>
> >>>>>>
> >>>>>>
> >>>>>>   2. Encode all 64-bit integers as a pair of 32-bit integers.
> >>>>>>     
> >>>>>>      This is fully compliant with the JSON spec as each half
> >>>>>>      is fully within the declared limits. App has to split or
> >>>>>>      assemble the 2 pieces from/to a signed/unsigned 64-bit
> >>>>>>      int as needed.
> >>>
> >>> Differently ugly.
> >
> > Particularly ugly as we turn 1<<55 from:
> >
> > "value":36028797018963968
> >
> > into
> >
> > "value":[8388608,0]
> >
> > and now both qemu and the client end have to agree that an array of two
> > integers is a valid replacement for any larger 64-bit quantity
> > (presumably, we'd always accept the array form even for small integer
> > values, but only produce the array form for large values).  And while it
> > manages just fine for uint64_t values, what rules would you place on
> > int64_t values? That the resulting 2-integer array is combined with the
> > first number as a 2's-complement signed value, and the second being a
> > 32-bit unsigned value?
> 
> There's more than one way to encode integers as a list of 53 bit signed
> integers.  Any of them will do, we just have to specify one.
> 
> >>>>>>
> >>>>>>
> >>>>>>   3. Encode all 64-bit integers as strings
> >>>>>>
> >>>>>>      The application has todo all parsing/formatting client
> >>>>>>      side.
> >>>
> >>> Yet another ugly.
> >
> > But less so than option 2.
> >
> > "value":36028797018963968
> >
> > vs.
> >
> > "value":"36028797018963968"
> >
> > is at least tolerable.
> 
> Yes.
> 
> >>>>>> None of these changes are backwards compatible, so I doubt we could make
> >>>>>> the change transparently in QMP.  Instead we would have to have a
> >>>>>> QMP greeting message capability where the client can request enablement
> >>>>>> of the enhanced integer handling.
> >>>
> >>> We might be able to do option 1 without capability negotiation.  v2.10's
> >>> change from option 1 to what we have now produced zero complaints.
> >>>
> >>> On the other hand, we made that change for a reason, so we may want a
> >>> "send large integers as negative integers" capability regardless.
> >>>
> >>>>>>
> >>>>>> Any of the three options above would likely work for libvirt, but I
> >>>>>> would have a slight preference for either 2 or 3, so that we become
> >>>>>> 100% standards compliant.
> >
> > If we're going to negotiate something, I'd lean towards option 3
> > (anywhere the introspection states that we accept 'int64' or similar, it
> > is also appropriate to send a string value in its place). We'd also have
> > to decide if we want to allow "0xabcd", or strictly insist on 43981,
> > when stringizing an integer.  And while qemu should accept a string or a
> > number on input, we'd still have to decide/document whether it's
> > response to the client capability negotiation is to output a string
> > always, or only for values larger than the 2^53 threshold.
> 
> Picking option 3 is no excuse for complicating matters further.  QMP is
> primarily for machines.  So my first choice would be to keep everything
> decimal.  I could be persuaded to have QEMU parse integers from strings
> with base 0, i.e. leading 0x gets you hex, leading 0 gets you octal.

as I said in an earlier reply, my preference would also be to keep it
very strict and simple; although my preference is to stick to hex.
My suggestion was 0x%x format, always with the leading 0x, always lower
case, allowing but not requiring 0 padding, and never more than 18
characters (including the 0x). Always using the string format.

Dave

> >>>
> >>> There's no such thing.  You mean "we maximize interoperability with
> >>> common implementations of JSON".
> >> 
> >> s/common/any/
> >> 
> >>> Let's talk implementation for a bit.
> >>>
> >>> Encoding and decoding integers in funny ways should be fairly easy in
> >>> the QObject visitors.  The generated QMP marshallers all use them.
> >>> Trouble is a few commands still bypass the generated marshallers, and
> >>> mess with the QObject themselves:
> >>>
> >>> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
> >>>   comment.  Should be harmless.
> >>>
> >>> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
> >>>   because they reject some abuses like passing numbers and bools as
> >>>   strings.
> >>>
> >>> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
> >>>
> >>> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
> >>> could hack that to mirror what the QObject visitor do.
> >>>
> >>> Else, we might have to do it in the JSON parser.  Should be possible,
> >>> but I'd rather not.
> >>>
> >>>>> My preference would be 3 with the strings defined as being
> >>>>> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> >>>>> ("0x" + 16 nybbles). Zero padding allowed but not required.
> >>>>> It's readable and unambiguous when dealing with addresses; I don't want
> >>>>> to have to start decoding (2) by hand when debugging.
> >>>>
> >>>> Yep, that's a good point about readability.
> >>>
> >>> QMP sending all integers in decimal is inconvenient for some values,
> >>> such as addresses.  QMP sending all (large) integers in hexadecimal
> >>> would be inconvenient for other values.
> >>>
> >>> Let's keep it simple & stupid.  If you want sophistication, JSON is the
> >>> wrong choice.
> >
> > JSON requires decimal-only, but I'm okay if we state that when
> > negotiating the alternative representation, that we output hex-only.
> > (JSON5 adds hex support among other things, but it is not an RFC
> > standard, and even fewer libraries exist that parse JSON5 in addition to
> > straight JSON).
> >
> >>>
> >>>
> >>> Option 1 feels simplest.
> >> 
> >> But will still fail with any JSON impl that uses double precision floating
> >> point for integers as it will loose precision.
> >> 
> >>> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
> >> 
> >> If we assume any JSON impl can do 32-bit integers without loss of
> >> precision, then I think we can say it is guaranteed portable, but
> >> it is certainly horrible / ugly.
> >> 
> >>> Option 3 is like option 2, just not quite as ugly.
> >> 
> >> I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
> >> that exists, since you're delegating all string -> int conversion to
> >> the application code taking the JSON parser/formatter out of the equation.
> >> 
> >> This is close to the approach libvirt takes with YAJL parser today. YAJL
> >> parses as a int64 and we then ignore its result, and re-parse the string
> >> again in libvirt as uint64. When generating json we format as uint64
> >> in libvirt and ignore YAJLs formatting for int64.
> >> 
> >>> Can we agree to eliminate option 2 from the race?
> >> 
> >> I'm fine with eliminating option 2.
> >
> > Same here.
> 
> Noted.
> 
> >> I guess I'd have a preference for option 3 given that it has better
> >> interoperability
> >
> > Likewise - if we're going to bother with a capability that changes
> > output and allows the input validators to accept more forms, I'd prefer
> > a string form with correct sign over a negative integer that depends on
> > 64-bit 2's-complement arithmetic to intepret correctly.
> 
> Noted.
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote:
>
>> > The Golang JSON parser decodes JSON numbers to float64 by default so
>> > will have this precision limitation too, though at least they provide
>> > a backdoor for custom parsing from the original serialized representation.
>> >
>> >> > QEMU, and indeed many applications, want to handle 64-bit integers.
>> >> > The C JSON library impls have traditionally mapped integers to the
>> >> > data type 'long long int' which gives a min/max of  -(2^63) / 2^63-1.
>> >> > 
>> >> > QEMU however /really/ needs 64-bit unsigned integers, ie a max 2^64-1.
>> 
>> Correct.
>> 
>> Support for integers 2^63..2^64-1 is relatively recent: commit
>> 2bc7cfea095 (v2.10, 2017).
>> 
>> Since we really needed these, the QObject input visitor silently casts
>> negative integers to uint64_t.  It still does for backward
>> compatibility.  Commit 5923f85fb82 (right after 2bc7cfea095) explains
>> 
>>     The input visitor will cast i64 input to u64 for compatibility
>>     reasons (existing json QMP client already use negative i64 for large
>>     u64, and expect an implicit cast in qemu).
>>     
>>     Note: before the patch, uint64_t values above INT64_MAX are sent over
>>     json QMP as negative values, e.g. UINT64_MAX is sent as -1. After the
>>     patch, they are sent unmodified.  Clearly a bug fix, but we have to
>>     consider compatibility issues anyway.  libvirt should cope fine,
>>     because its parsing of unsigned integers accepts negative values
>>     modulo 2^64.  There's hope that other clients will, too.
>
> So QEMU reading stuff sent by libvirt in a back compatible manner is
> ok. The problem was specifically when a QEMU reply sent back UINT64_MAX
> value as a positive number.

Yes.  The commit message (tries to) explain exactly that.

>> >> > Libvirt has historically used the YAJL library which uses 'long long int'
>> >> > and thus can't officially go beyond 2^63-1 values. Fortunately it lets
>> >> > libvirt get at the raw json string, so libvirt can re-parse the value
>> >> > to get an 'unsigned long long'.
>> >> > 
>> >> > We recently tried to switch to Jansson because YAJL has a dead upstream
>> >> > for many years and countless unanswered bugs & patches. Unfortunately we
>> >> > forgot about this need for 2^64-1 max, and Jansson also uses 'long long int'
>> >> > and raises a fatal parse error for unsigned 64-bit values above 2^63-1. It
>> >> > also provides no backdoor for libvirt todo its own integer parsing. Thus
>> >> > we had to abort our switch to jansson as it broke parsing QEMU's JSON:
>> >> > 
>> >> >   https://bugzilla.redhat.com/show_bug.cgi?id=1614569
>> >> > 
>> >> > Other JSON libraries we've investigated have similar problems. I imagine
>> >> > the same may well be true of non-C based JOSN impls, though I've not
>> >> > investigated in any detail.
>> >> > 
>> >> > Essentially libvirt is stuck with either using the dead YAJL library
>> >> > forever, or writing its own JSON parser (most likely copying QEMU's
>> >> > JSON code into libvirt's git).
>> >> > 
>> >> > This feels like a very unappealing situation to be in as not being
>> >> > able to use a JSON library of our choice is loosing one of the key
>> >> > benefits of using a standard data format.
>> >> > 
>> >> > Thus I'd like to see a solution to this to allow QMP to be reliably
>> >> > consumed by any JSON library that exists.
>> 
>> JSON is terrible at interoperability, so good luck with that.
>> 
>> If you reduce your order to "the commonly used JSON libraries we know",
>> we can talk.
>
> I don't particularly want us to rely on semantics of small known set
> of JSON libs. I really do want us to do something that is capable of
> working with any JSON impl that exists in any programming language.

I think we're disagreeing mostly on the meaning of terms, and in ways
that don't matter all that much.

When you say "any JSON impl that exists in any programming language",
then I understand "any conceivable implementation that conforms to the
specification".

Trouble is the specification underspecifies numbers to a degree that you
cannot know what conforming implementations will accept.  An
implementation that accepts just 0 would still be conforming.  It would
also be useless.  An implementation that accepts just 64 bit signed
integers would still be conforming.  Useless?  Depends.  My point is: we
cannot know what *all* implementations accept.

One of "know" and "all" will have to give.

My choice is to relax in accordance with the guidance from RFC 7159 and
8259 I quoted (and you snipped):

    This specification allows implementations to set limits on the range
    and precision of numbers accepted.  Since software that implements
    IEEE 754-2008 binary64 (double precision) numbers [IEEE754] is
    generally available and widely used, good interoperability can be
    achieved by implementations that expect no more precision or range
    than these provide, in the sense that implementations will
    approximate JSON numbers within the expected precision.  A JSON
    number such as 1E400 or 3.141592653589793238462643383279 may indicate
    potential interoperability problems, since it suggests that the
    software that created it expects receiving software to have greater
    capabilities for numeric magnitude and precision than is widely
    available.

    Note that when such software is used, numbers that are integers and
    are in the range [-(2**53)+1, (2**53)-1] are interoperable in the
    sense that implementations will agree exactly on their numeric
    values.

Bottom line: limiting integer use to 53 bits signed is likely to
maximize interoperability with common implementations.  It might well
achieve interoperability with all implementations that matter to us.  I
think that's close enough to "all" to stop arguing about the difference.
If you insist to call it "all" going forward, I'll silently
error-correct it to "enough".

> My suggested option 2 & 3 at least would manage that I believe, as
> any credible JSON impl will be able to represent 32-bit integers
> or strings without loosing data.

You just relaxed your categorical "all" to just "credible" :)

> Option 1 would not cope as some impls can't even cope with
> signed 64-bit ints.
>
>> >> > I can think of some options:
>> >> > 
>> >> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
>> >> > 
>> >> >      This follows the example that most C libraries map JSON ints
>> >> >      to 'long long int'. This is still relying on undefined
>> >> >      behaviour as apps don't need to support > 2^53-1.
>> >> > 
>> >> >      Apps would need to cast back to 'unsigned long long' for
>> >> >      those QMP fields they know are supposed to be unsigned.
>> 
>> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
>> direction still does it, for backward compatibility.
>> 
>> >> > 
>> >> > 
>> >> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
>> >> >     
>> >> >      This is fully compliant with the JSON spec as each half
>> >> >      is fully within the declared limits. App has to split or
>> >> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
>> >> >      int as needed.
>> 
>> Differently ugly.
>> 
>> >> > 
>> >> > 
>> >> >   3. Encode all 64-bit integers as strings
>> >> > 
>> >> >      The application has todo all parsing/formatting client
>> >> >      side.
>> 
>> Yet another ugly.
>> 
>> >> > 
>> >> > 
>> >> > None of these changes are backwards compatible, so I doubt we could make
>> >> > the change transparently in QMP.  Instead we would have to have a
>> >> > QMP greeting message capability where the client can request enablement
>> >> > of the enhanced integer handling.
>> 
>> We might be able to do option 1 without capability negotiation.  v2.10's
>> change from option 1 to what we have now produced zero complaints.
>> 
>> On the other hand, we made that change for a reason, so we may want a
>> "send large integers as negative integers" capability regardless.
>> 
>> >> > 
>> >> > Any of the three options above would likely work for libvirt, but I
>> >> > would have a slight preference for either 2 or 3, so that we become
>> >> > 100% standards compliant.
>> 
>> There's no such thing.  You mean "we maximize interoperability with
>> common implementations of JSON".
>
> s/common/any/

info: error correction applied, future applications will be silent ;-P

>> Let's talk implementation for a bit.
>> 
>> Encoding and decoding integers in funny ways should be fairly easy in
>> the QObject visitors.  The generated QMP marshallers all use them.
>> Trouble is a few commands still bypass the generated marshallers, and
>> mess with the QObject themselves:
>> 
>> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
>>   comment.  Should be harmless.
>> 
>> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
>>   because they reject some abuses like passing numbers and bools as
>>   strings.
>> 
>> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
>> 
>> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
>> could hack that to mirror what the QObject visitor do.
>> 
>> Else, we might have to do it in the JSON parser.  Should be possible,
>> but I'd rather not.
>> 
>> >> My preference would be 3 with the strings defined as being
>> >> %x lower case hex formated with a 0x prefix and no longer than 18 characters
>> >> ("0x" + 16 nybbles). Zero padding allowed but not required.
>> >> It's readable and unambiguous when dealing with addresses; I don't want
>> >> to have to start decoding (2) by hand when debugging.
>> >
>> > Yep, that's a good point about readability.
>> 
>> QMP sending all integers in decimal is inconvenient for some values,
>> such as addresses.  QMP sending all (large) integers in hexadecimal
>> would be inconvenient for other values.
>> 
>> Let's keep it simple & stupid.  If you want sophistication, JSON is the
>> wrong choice.
>> 
>> 
>> Option 1 feels simplest.
>
> But will still fail with any JSON impl that uses double precision floating
> point for integers as it will loose precision.
>
>> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
>
> If we assume any JSON impl can do 32-bit integers without loss of
> precision, then I think we can say it is guaranteed portable, but
> it is certainly horrible / ugly.
>
>> Option 3 is like option 2, just not quite as ugly.
>
> I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
> that exists, since you're delegating all string -> int conversion to
> the application code taking the JSON parser/formatter out of the equation.

Double-checking: do you propose to encode *all* numbers as strings, or
just certain "problematic" numbers?

If the latter, I guess your idea of "problematic" is "not representable
exactly as double precision floating-point".

> This is close to the approach libvirt takes with YAJL parser today. YAJL
> parses as a int64 and we then ignore its result, and re-parse the string
> again in libvirt as uint64. When generating json we format as uint64
> in libvirt and ignore YAJLs formatting for int64.
>
>> Can we agree to eliminate option 2 from the race?
>
> I'm fine with eliminating option 2.

Eric, too.  Consider it eliminated.

> I guess I'd have a preference for option 3 given that it has better
> interoperability

If we add a QMP capability for interoperability with JSON
implementations that set limits on range and precision that are
incompatible with the ones QMP sets, one could argue we effectively pay
the price for option 3, so we should take it for its benefits.

Option 1 without a QMP capability merely reverts the change we made in
2.10.  We can do that if we think it's sufficient.

You expressed a strong preference for maximizing interoperability (via
option 3).  Acknowledged.  However, I care a lot more about issues we
know we have than about issues somebody might have.

You mentioned the libvirt's switch to Jansson you had to abort due to
QMP sending numbers Jansson refuses to parse.  That's the kind of
non-hypothetical issue that can make me mess with the QMP language.

You wrote Jansson "raises a fatal parse error for unsigned 64-bit values
above 2^63-1".  Does that mean it rejects 9223372036854775808, but
accepts 9223372036854775808.0 (with loss of precision)?

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

[Daniel P. Berrangé]

On Wed, May 08, 2019 at 02:44:07PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote:
> >
> >> >> > I can think of some options:
> >> >> > 
> >> >> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> >> >> > 
> >> >> >      This follows the example that most C libraries map JSON ints
> >> >> >      to 'long long int'. This is still relying on undefined
> >> >> >      behaviour as apps don't need to support > 2^53-1.
> >> >> > 
> >> >> >      Apps would need to cast back to 'unsigned long long' for
> >> >> >      those QMP fields they know are supposed to be unsigned.
> >> 
> >> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
> >> direction still does it, for backward compatibility.
> >> 
> >> >> > 
> >> >> > 
> >> >> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
> >> >> >     
> >> >> >      This is fully compliant with the JSON spec as each half
> >> >> >      is fully within the declared limits. App has to split or
> >> >> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
> >> >> >      int as needed.
> >> 
> >> Differently ugly.
> >> 
> >> >> > 
> >> >> > 
> >> >> >   3. Encode all 64-bit integers as strings
> >> >> > 
> >> >> >      The application has todo all parsing/formatting client
> >> >> >      side.
> >> 
> >> Yet another ugly.
> >> 
> >> >> > 
> >> >> > 
> >> >> > None of these changes are backwards compatible, so I doubt we could make
> >> >> > the change transparently in QMP.  Instead we would have to have a
> >> >> > QMP greeting message capability where the client can request enablement
> >> >> > of the enhanced integer handling.
> >> 
> >> We might be able to do option 1 without capability negotiation.  v2.10's
> >> change from option 1 to what we have now produced zero complaints.
> >> 
> >> On the other hand, we made that change for a reason, so we may want a
> >> "send large integers as negative integers" capability regardless.
> >> 
> >> >> > 
> >> >> > Any of the three options above would likely work for libvirt, but I
> >> >> > would have a slight preference for either 2 or 3, so that we become
> >> >> > 100% standards compliant.
> >> 
> >> There's no such thing.  You mean "we maximize interoperability with
> >> common implementations of JSON".
> >
> > s/common/any/
> 
> info: error correction applied, future applications will be silent ;-P
> 
> >> Let's talk implementation for a bit.
> >> 
> >> Encoding and decoding integers in funny ways should be fairly easy in
> >> the QObject visitors.  The generated QMP marshallers all use them.
> >> Trouble is a few commands still bypass the generated marshallers, and
> >> mess with the QObject themselves:
> >> 
> >> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
> >>   comment.  Should be harmless.
> >> 
> >> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
> >>   because they reject some abuses like passing numbers and bools as
> >>   strings.
> >> 
> >> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
> >> 
> >> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
> >> could hack that to mirror what the QObject visitor do.
> >> 
> >> Else, we might have to do it in the JSON parser.  Should be possible,
> >> but I'd rather not.
> >> 
> >> >> My preference would be 3 with the strings defined as being
> >> >> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> >> >> ("0x" + 16 nybbles). Zero padding allowed but not required.
> >> >> It's readable and unambiguous when dealing with addresses; I don't want
> >> >> to have to start decoding (2) by hand when debugging.
> >> >
> >> > Yep, that's a good point about readability.
> >> 
> >> QMP sending all integers in decimal is inconvenient for some values,
> >> such as addresses.  QMP sending all (large) integers in hexadecimal
> >> would be inconvenient for other values.
> >> 
> >> Let's keep it simple & stupid.  If you want sophistication, JSON is the
> >> wrong choice.
> >> 
> >> 
> >> Option 1 feels simplest.
> >
> > But will still fail with any JSON impl that uses double precision floating
> > point for integers as it will loose precision.
> >
> >> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
> >
> > If we assume any JSON impl can do 32-bit integers without loss of
> > precision, then I think we can say it is guaranteed portable, but
> > it is certainly horrible / ugly.
> >
> >> Option 3 is like option 2, just not quite as ugly.
> >
> > I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
> > that exists, since you're delegating all string -> int conversion to
> > the application code taking the JSON parser/formatter out of the equation.
> 
> Double-checking: do you propose to encode *all* numbers as strings, or
> just certain "problematic" numbers?
> 
> If the latter, I guess your idea of "problematic" is "not representable
> exactly as double precision floating-point".

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.

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 I'd have a preference for option 3 given that it has better
> > interoperability
> 
> If we add a QMP capability for interoperability with JSON
> implementations that set limits on range and precision that are
> incompatible with the ones QMP sets, one could argue we effectively pay
> the price for option 3, so we should take it for its benefits.
> 
> Option 1 without a QMP capability merely reverts the change we made in
> 2.10.  We can do that if we think it's sufficient.
> 
> You expressed a strong preference for maximizing interoperability (via
> option 3).  Acknowledged.  However, I care a lot more about issues we
> know we have than about issues somebody might have.
> 
> You mentioned the libvirt's switch to Jansson you had to abort due to
> QMP sending numbers Jansson refuses to parse.  That's the kind of
> non-hypothetical issue that can make me mess with the QMP language.
> 
> You wrote Jansson "raises a fatal parse error for unsigned 64-bit values
> above 2^63-1".  Does that mean it rejects 9223372036854775808, but
> accepts 9223372036854775808.0 (with loss of precision)?

If it sees a '.' in the number, then it call strtod() and checks for
the overflow conditions.

If it doesn't see a '.' in the number then it calls strtoll and checks
for the overflow conditions.

So to answer you question, yes, it looks like it will reject
9223372036854775808 and accept 9223372036854775808.0 with loss of
precision.

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

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

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Wed, May 08, 2019 at 02:44:07PM +0200, Markus Armbruster wrote:
> > Daniel P. Berrangé <berrange@redhat.com> writes:
> > 
> > > On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote:
> > >
> > >> >> > I can think of some options:
> > >> >> > 
> > >> >> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> > >> >> > 
> > >> >> >      This follows the example that most C libraries map JSON ints
> > >> >> >      to 'long long int'. This is still relying on undefined
> > >> >> >      behaviour as apps don't need to support > 2^53-1.
> > >> >> > 
> > >> >> >      Apps would need to cast back to 'unsigned long long' for
> > >> >> >      those QMP fields they know are supposed to be unsigned.
> > >> 
> > >> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
> > >> direction still does it, for backward compatibility.
> > >> 
> > >> >> > 
> > >> >> > 
> > >> >> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
> > >> >> >     
> > >> >> >      This is fully compliant with the JSON spec as each half
> > >> >> >      is fully within the declared limits. App has to split or
> > >> >> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
> > >> >> >      int as needed.
> > >> 
> > >> Differently ugly.
> > >> 
> > >> >> > 
> > >> >> > 
> > >> >> >   3. Encode all 64-bit integers as strings
> > >> >> > 
> > >> >> >      The application has todo all parsing/formatting client
> > >> >> >      side.
> > >> 
> > >> Yet another ugly.
> > >> 
> > >> >> > 
> > >> >> > 
> > >> >> > None of these changes are backwards compatible, so I doubt we could make
> > >> >> > the change transparently in QMP.  Instead we would have to have a
> > >> >> > QMP greeting message capability where the client can request enablement
> > >> >> > of the enhanced integer handling.
> > >> 
> > >> We might be able to do option 1 without capability negotiation.  v2.10's
> > >> change from option 1 to what we have now produced zero complaints.
> > >> 
> > >> On the other hand, we made that change for a reason, so we may want a
> > >> "send large integers as negative integers" capability regardless.
> > >> 
> > >> >> > 
> > >> >> > Any of the three options above would likely work for libvirt, but I
> > >> >> > would have a slight preference for either 2 or 3, so that we become
> > >> >> > 100% standards compliant.
> > >> 
> > >> There's no such thing.  You mean "we maximize interoperability with
> > >> common implementations of JSON".
> > >
> > > s/common/any/
> > 
> > info: error correction applied, future applications will be silent ;-P
> > 
> > >> Let's talk implementation for a bit.
> > >> 
> > >> Encoding and decoding integers in funny ways should be fairly easy in
> > >> the QObject visitors.  The generated QMP marshallers all use them.
> > >> Trouble is a few commands still bypass the generated marshallers, and
> > >> mess with the QObject themselves:
> > >> 
> > >> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
> > >>   comment.  Should be harmless.
> > >> 
> > >> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
> > >>   because they reject some abuses like passing numbers and bools as
> > >>   strings.
> > >> 
> > >> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
> > >> 
> > >> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
> > >> could hack that to mirror what the QObject visitor do.
> > >> 
> > >> Else, we might have to do it in the JSON parser.  Should be possible,
> > >> but I'd rather not.
> > >> 
> > >> >> My preference would be 3 with the strings defined as being
> > >> >> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> > >> >> ("0x" + 16 nybbles). Zero padding allowed but not required.
> > >> >> It's readable and unambiguous when dealing with addresses; I don't want
> > >> >> to have to start decoding (2) by hand when debugging.
> > >> >
> > >> > Yep, that's a good point about readability.
> > >> 
> > >> QMP sending all integers in decimal is inconvenient for some values,
> > >> such as addresses.  QMP sending all (large) integers in hexadecimal
> > >> would be inconvenient for other values.
> > >> 
> > >> Let's keep it simple & stupid.  If you want sophistication, JSON is the
> > >> wrong choice.
> > >> 
> > >> 
> > >> Option 1 feels simplest.
> > >
> > > But will still fail with any JSON impl that uses double precision floating
> > > point for integers as it will loose precision.
> > >
> > >> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
> > >
> > > If we assume any JSON impl can do 32-bit integers without loss of
> > > precision, then I think we can say it is guaranteed portable, but
> > > it is certainly horrible / ugly.
> > >
> > >> Option 3 is like option 2, just not quite as ugly.
> > >
> > > I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
> > > that exists, since you're delegating all string -> int conversion to
> > > the application code taking the JSON parser/formatter out of the equation.
> > 
> > Double-checking: do you propose to encode *all* numbers as strings, or
> > just certain "problematic" numbers?
> > 
> > If the latter, I guess your idea of "problematic" is "not representable
> > exactly as double precision floating-point".
> 
> 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.

What about when the size is architecture dependent?

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

Yeh, 1 is horrid; it's too easy to miss a case which forgot to handle
 the 2^53-1 because we hadn't forced a large value down that check.

Dave

> > > I guess I'd have a preference for option 3 given that it has better
> > > interoperability
> > 
> > If we add a QMP capability for interoperability with JSON
> > implementations that set limits on range and precision that are
> > incompatible with the ones QMP sets, one could argue we effectively pay
> > the price for option 3, so we should take it for its benefits.
> > 
> > Option 1 without a QMP capability merely reverts the change we made in
> > 2.10.  We can do that if we think it's sufficient.
> > 
> > You expressed a strong preference for maximizing interoperability (via
> > option 3).  Acknowledged.  However, I care a lot more about issues we
> > know we have than about issues somebody might have.
> > 
> > You mentioned the libvirt's switch to Jansson you had to abort due to
> > QMP sending numbers Jansson refuses to parse.  That's the kind of
> > non-hypothetical issue that can make me mess with the QMP language.
> > 
> > You wrote Jansson "raises a fatal parse error for unsigned 64-bit values
> > above 2^63-1".  Does that mean it rejects 9223372036854775808, but
> > accepts 9223372036854775808.0 (with loss of precision)?
> 
> If it sees a '.' in the number, then it call strtod() and checks for
> the overflow conditions.
> 
> If it doesn't see a '.' in the number then it calls strtoll and checks
> for the overflow conditions.
> 
> So to answer you question, yes, it looks like it will reject
> 9223372036854775808 and accept 9223372036854775808.0 with loss of
> precision.
> 
> 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

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

[Daniel P. Berrangé]

On Mon, May 13, 2019 at 01:29:34PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
> > On Wed, May 08, 2019 at 02:44:07PM +0200, Markus Armbruster wrote:
> > > Daniel P. Berrangé <berrange@redhat.com> writes:
> > > 
> > > > On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote:
> > > >
> > > >> >> > I can think of some options:
> > > >> >> > 
> > > >> >> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
> > > >> >> > 
> > > >> >> >      This follows the example that most C libraries map JSON ints
> > > >> >> >      to 'long long int'. This is still relying on undefined
> > > >> >> >      behaviour as apps don't need to support > 2^53-1.
> > > >> >> > 
> > > >> >> >      Apps would need to cast back to 'unsigned long long' for
> > > >> >> >      those QMP fields they know are supposed to be unsigned.
> > > >> 
> > > >> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
> > > >> direction still does it, for backward compatibility.
> > > >> 
> > > >> >> > 
> > > >> >> > 
> > > >> >> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
> > > >> >> >     
> > > >> >> >      This is fully compliant with the JSON spec as each half
> > > >> >> >      is fully within the declared limits. App has to split or
> > > >> >> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
> > > >> >> >      int as needed.
> > > >> 
> > > >> Differently ugly.
> > > >> 
> > > >> >> > 
> > > >> >> > 
> > > >> >> >   3. Encode all 64-bit integers as strings
> > > >> >> > 
> > > >> >> >      The application has todo all parsing/formatting client
> > > >> >> >      side.
> > > >> 
> > > >> Yet another ugly.
> > > >> 
> > > >> >> > 
> > > >> >> > 
> > > >> >> > None of these changes are backwards compatible, so I doubt we could make
> > > >> >> > the change transparently in QMP.  Instead we would have to have a
> > > >> >> > QMP greeting message capability where the client can request enablement
> > > >> >> > of the enhanced integer handling.
> > > >> 
> > > >> We might be able to do option 1 without capability negotiation.  v2.10's
> > > >> change from option 1 to what we have now produced zero complaints.
> > > >> 
> > > >> On the other hand, we made that change for a reason, so we may want a
> > > >> "send large integers as negative integers" capability regardless.
> > > >> 
> > > >> >> > 
> > > >> >> > Any of the three options above would likely work for libvirt, but I
> > > >> >> > would have a slight preference for either 2 or 3, so that we become
> > > >> >> > 100% standards compliant.
> > > >> 
> > > >> There's no such thing.  You mean "we maximize interoperability with
> > > >> common implementations of JSON".
> > > >
> > > > s/common/any/
> > > 
> > > info: error correction applied, future applications will be silent ;-P
> > > 
> > > >> Let's talk implementation for a bit.
> > > >> 
> > > >> Encoding and decoding integers in funny ways should be fairly easy in
> > > >> the QObject visitors.  The generated QMP marshallers all use them.
> > > >> Trouble is a few commands still bypass the generated marshallers, and
> > > >> mess with the QObject themselves:
> > > >> 
> > > >> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
> > > >>   comment.  Should be harmless.
> > > >> 
> > > >> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
> > > >>   because they reject some abuses like passing numbers and bools as
> > > >>   strings.
> > > >> 
> > > >> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
> > > >> 
> > > >> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
> > > >> could hack that to mirror what the QObject visitor do.
> > > >> 
> > > >> Else, we might have to do it in the JSON parser.  Should be possible,
> > > >> but I'd rather not.
> > > >> 
> > > >> >> My preference would be 3 with the strings defined as being
> > > >> >> %x lower case hex formated with a 0x prefix and no longer than 18 characters
> > > >> >> ("0x" + 16 nybbles). Zero padding allowed but not required.
> > > >> >> It's readable and unambiguous when dealing with addresses; I don't want
> > > >> >> to have to start decoding (2) by hand when debugging.
> > > >> >
> > > >> > Yep, that's a good point about readability.
> > > >> 
> > > >> QMP sending all integers in decimal is inconvenient for some values,
> > > >> such as addresses.  QMP sending all (large) integers in hexadecimal
> > > >> would be inconvenient for other values.
> > > >> 
> > > >> Let's keep it simple & stupid.  If you want sophistication, JSON is the
> > > >> wrong choice.
> > > >> 
> > > >> 
> > > >> Option 1 feels simplest.
> > > >
> > > > But will still fail with any JSON impl that uses double precision floating
> > > > point for integers as it will loose precision.
> > > >
> > > >> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
> > > >
> > > > If we assume any JSON impl can do 32-bit integers without loss of
> > > > precision, then I think we can say it is guaranteed portable, but
> > > > it is certainly horrible / ugly.
> > > >
> > > >> Option 3 is like option 2, just not quite as ugly.
> > > >
> > > > I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
> > > > that exists, since you're delegating all string -> int conversion to
> > > > the application code taking the JSON parser/formatter out of the equation.
> > > 
> > > Double-checking: do you propose to encode *all* numbers as strings, or
> > > just certain "problematic" numbers?
> > > 
> > > If the latter, I guess your idea of "problematic" is "not representable
> > > exactly as double precision floating-point".
> > 
> > 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.
> 
> What about when the size is architecture dependent?

The QAPI visitor for 'int' uses an 'int64_t' parameters, so I think
that will want to be string encoded, as if it was a 64-bit int, even
if built on a 32-bit platform.

> 
> > 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.
> 
> Yeh, 1 is horrid; it's too easy to miss a case which forgot to handle
>  the 2^53-1 because we hadn't forced a large value down that check.

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

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

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, May 13, 2019 at 01:29:34PM +0100, Dr. David Alan Gilbert wrote:
>> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>> > On Wed, May 08, 2019 at 02:44:07PM +0200, Markus Armbruster wrote:
>> > > Daniel P. Berrangé <berrange@redhat.com> writes:
>> > > 
>> > > > On Tue, May 07, 2019 at 10:47:06AM +0200, Markus Armbruster wrote:
>> > > >
>> > > >> >> > I can think of some options:
>> > > >> >> > 
>> > > >> >> >   1. Encode unsigned 64-bit integers as signed 64-bit integers.
>> > > >> >> > 
>> > > >> >> >      This follows the example that most C libraries map JSON ints
>> > > >> >> >      to 'long long int'. This is still relying on undefined
>> > > >> >> >      behaviour as apps don't need to support > 2^53-1.
>> > > >> >> > 
>> > > >> >> >      Apps would need to cast back to 'unsigned long long' for
>> > > >> >> >      those QMP fields they know are supposed to be unsigned.
>> > > >> 
>> > > >> Ugly.  It's also what we did until v2.10, August 2017.  QMP's input
>> > > >> direction still does it, for backward compatibility.
>> > > >> 
>> > > >> >> > 
>> > > >> >> > 
>> > > >> >> >   2. Encode all 64-bit integers as a pair of 32-bit integers.
>> > > >> >> >     
>> > > >> >> >      This is fully compliant with the JSON spec as each half
>> > > >> >> >      is fully within the declared limits. App has to split or
>> > > >> >> >      assemble the 2 pieces from/to a signed/unsigned 64-bit
>> > > >> >> >      int as needed.
>> > > >> 
>> > > >> Differently ugly.
>> > > >> 
>> > > >> >> > 
>> > > >> >> > 
>> > > >> >> >   3. Encode all 64-bit integers as strings
>> > > >> >> > 
>> > > >> >> >      The application has todo all parsing/formatting client
>> > > >> >> >      side.
>> > > >> 
>> > > >> Yet another ugly.
>> > > >> 
>> > > >> >> > 
>> > > >> >> > 
>> > > >> >> > None of these changes are backwards compatible, so I doubt we could make
>> > > >> >> > the change transparently in QMP.  Instead we would have to have a
>> > > >> >> > QMP greeting message capability where the client can request enablement
>> > > >> >> > of the enhanced integer handling.
>> > > >> 
>> > > >> We might be able to do option 1 without capability negotiation.  v2.10's
>> > > >> change from option 1 to what we have now produced zero complaints.
>> > > >> 
>> > > >> On the other hand, we made that change for a reason, so we may want a
>> > > >> "send large integers as negative integers" capability regardless.
>> > > >> 
>> > > >> >> > 
>> > > >> >> > Any of the three options above would likely work for libvirt, but I
>> > > >> >> > would have a slight preference for either 2 or 3, so that we become
>> > > >> >> > 100% standards compliant.
>> > > >> 
>> > > >> There's no such thing.  You mean "we maximize interoperability with
>> > > >> common implementations of JSON".
>> > > >
>> > > > s/common/any/
>> > > 
>> > > info: error correction applied, future applications will be silent ;-P
>> > > 
>> > > >> Let's talk implementation for a bit.
>> > > >> 
>> > > >> Encoding and decoding integers in funny ways should be fairly easy in
>> > > >> the QObject visitors.  The generated QMP marshallers all use them.
>> > > >> Trouble is a few commands still bypass the generated marshallers, and
>> > > >> mess with the QObject themselves:
>> > > >> 
>> > > >> * query-qmp-schema: minor hack explained in qmp_query_qmp_schema()'s
>> > > >>   comment.  Should be harmless.
>> > > >> 
>> > > >> * netdev_add: not QAPIfied.  Eric's patches to QAPIfy it got stuck
>> > > >>   because they reject some abuses like passing numbers and bools as
>> > > >>   strings.
>> > > >> 
>> > > >> * device_add: not QAPIfied.  We're not sure QAPIfication is feasible.
>> > > >> 
>> > > >> netdev_add and device_add both use qemu_opts_from_qdict().  Perhaps we
>> > > >> could hack that to mirror what the QObject visitor do.
>> > > >> 
>> > > >> Else, we might have to do it in the JSON parser.  Should be possible,
>> > > >> but I'd rather not.
>> > > >> 
>> > > >> >> My preference would be 3 with the strings defined as being
>> > > >> >> %x lower case hex formated with a 0x prefix and no longer than 18 characters
>> > > >> >> ("0x" + 16 nybbles). Zero padding allowed but not required.
>> > > >> >> It's readable and unambiguous when dealing with addresses; I don't want
>> > > >> >> to have to start decoding (2) by hand when debugging.
>> > > >> >
>> > > >> > Yep, that's a good point about readability.
>> > > >> 
>> > > >> QMP sending all integers in decimal is inconvenient for some values,
>> > > >> such as addresses.  QMP sending all (large) integers in hexadecimal
>> > > >> would be inconvenient for other values.
>> > > >> 
>> > > >> Let's keep it simple & stupid.  If you want sophistication, JSON is the
>> > > >> wrong choice.
>> > > >> 
>> > > >> 
>> > > >> Option 1 feels simplest.
>> > > >
>> > > > But will still fail with any JSON impl that uses double precision floating
>> > > > point for integers as it will loose precision.
>> > > >
>> > > >> Option 2 feels ugliest.  Less simple, more interoperable than option 1.
>> > > >
>> > > > If we assume any JSON impl can do 32-bit integers without loss of
>> > > > precision, then I think we can say it is guaranteed portable, but
>> > > > it is certainly horrible / ugly.
>> > > >
>> > > >> Option 3 is like option 2, just not quite as ugly.
>> > > >
>> > > > I think option 3 can be guaranteed to be loss-less with /any/ JSON impl
>> > > > that exists, since you're delegating all string -> int conversion to
>> > > > the application code taking the JSON parser/formatter out of the equation.
>> > > 
>> > > Double-checking: do you propose to encode *all* numbers as strings, or
>> > > just certain "problematic" numbers?
>> > > 
>> > > If the latter, I guess your idea of "problematic" is "not representable
>> > > exactly as double precision floating-point".
>> > 
>> > 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.
>> 
>> What about when the size is architecture dependent?
>
> The QAPI visitor for 'int' uses an 'int64_t' parameters, so I think
> that will want to be string encoded, as if it was a 64-bit int, even
> if built on a 32-bit platform.

Yes, QAPI type 'int' is int64_t.  All of QAPI's integer types map to
exact-width C integer types.

>> 
>> > 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.
>> 
>> Yeh, 1 is horrid; it's too easy to miss a case which forgot to handle
>>  the 2^53-1 because we hadn't forced a large value down that check.

If we had many places where we could forget to handle the
interoperability capability, I'd shoot down the idea of having it :)

Fortunately, we don't: the qobject visitors take care of it, and they
come with unit tests.

I listed the few exceptions above under "Let's talk implementation for a
bit."

* query-qmp-schema

  No input.

  Output contains no numbers.  Should it acquire numbers, we'll probably
  want to get rid of the hack, so the output goes through the visitor.

* netdev_add

  Input gets stringified anyway (misfeature).

  No output.

* device_add

  Input gets stringified anyway (misfeature).

  No output.

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

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Wed, May 08, 2019 at 02:44:07PM +0200, Markus Armbruster wrote:
[...]
>> Double-checking: do you propose to encode *all* numbers as strings, or
>> just certain "problematic" numbers?
>> 
>> If the latter, I guess your idea of "problematic" is "not representable
>> exactly as double precision floating-point".
>
> 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.

> 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*.

For *output*, QEMU has to encode a number either as JSON number or as
JSON string

For *input*, QEMU could accept either.  Or it could accept only the
encoding it produces on output.

Got a preference?

[...]
>> You wrote Jansson "raises a fatal parse error for unsigned 64-bit values
>> above 2^63-1".  Does that mean it rejects 9223372036854775808, but
>> accepts 9223372036854775808.0 (with loss of precision)?
>
> If it sees a '.' in the number, then it call strtod() and checks for
> the overflow conditions.
>
> If it doesn't see a '.' in the number then it calls strtoll and checks
> for the overflow conditions.
>
> So to answer you question, yes, it looks like it will reject
> 9223372036854775808 and accept 9223372036854775808.0 with loss of
> precision.

Sucks :)

But then JSON does.

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

[Daniel P. Berrangé]

On Mon, May 13, 2019 at 03:53:19PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Wed, May 08, 2019 at 02:44:07PM +0200, Markus Armbruster wrote:
> [...]
> >> Double-checking: do you propose to encode *all* numbers as strings, or
> >> just certain "problematic" numbers?
> >> 
> >> If the latter, I guess your idea of "problematic" is "not representable
> >> exactly as double precision floating-point".
> >
> > 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.
> 
> > 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*.
> 
> For *output*, QEMU has to encode a number either as JSON number or as
> JSON string
> 
> For *input*, QEMU could accept either.  Or it could accept only the
> encoding it produces on output.
> 
> Got a preference?

IMHO if a mgmt app enables the (hypothetically named) "int64-as-string"
capability, then we should be strict and require string format on both
input & output. If QEMU accepted either, it would silently hide bugs
where the app has mistakenly not used string formatting.


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

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2771 bytes --]

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.

> 
>> 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).

> 
> For *output*, QEMU has to encode a number either as JSON number or as
> JSON string

For output, we should document that whatever capability the client
passes in at initial connect controls the behavior of all future integer
outputs (and if no capability is negotiated, we output all integers as
integers, even if it risks overflowing the client's JSON parser).

> 
> For *input*, QEMU could accept either.  Or it could accept only the
> encoding it produces on output.
> 
> Got a preference?
> 

My ramblings above argue to teach the parser to always allow both
integer and string forms regardless of width, to output 32-bit integers
as integers, and to output 64-bit integers as string-only (if capability
is negotiated) or as integer-only (if in back-compat mode).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Markus Armbruster]

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?

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

[Daniel P. Berrangé]

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.

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

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

[Dr. David Alan Gilbert]

* 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

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é (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.

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

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

[Peter Krempa]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

On Tue, May 14, 2019 at 10:43:31 +0100, Daniel Berrange wrote:
> On Tue, May 14, 2019 at 10:37:55AM +0100, Dr. David Alan Gilbert wrote:
> > * 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:

[...]

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

I think this would make the most sense. If you are going to switch to
the "string" mode, why bother doing any compat?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Markus Armbruster]

We've discussed possible solutions.  Is anyone working or intending to
work on patches?

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

[Daniel P. Berrangé]

On Tue, Jun 04, 2019 at 08:38:24AM +0200, Markus Armbruster wrote:
> We've discussed possible solutions.  Is anyone working or intending to
> work on patches?

I'm not actively working on it now, nor any plans in the near future.

I would like to see it fixed sooner rather than later though.

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