Re: [Qemu-devel] [PATCH 04/21] qobject: add quint type

[Marc-André Lureau]

Hi

On Tue, Mar 21, 2017 at 9:06 PM Eric Blake <address@hidden> wrote:

> On 03/21/2017 11:49 AM, Markus Armbruster wrote:
>
> >
> > QMP clients that work around the "large positive integers are rejected"
> > bugs by sending large negative ones instead may well exist.  Fixing the
> > interface would break them.  Depressing.  Eric, could you have a peek at
> > libvirt?
>
> Yes, libvirt is such a client already.  From
> util/virjson.c:virJSONValueObjectAddVArgs():
>
>
>  * Adds the key-value pairs supplied as variable argument list to @obj.
>  *
>  * Keys look like   s:name  the first letter is a type code:
> ...
>  * U: unsigned long integer value (see below for quirks)
>  * P: unsigned long integer value, omitted if zero
> ...
>         case 'P':
>         case 'U': {
>             /* qemu silently truncates numbers larger than LLONG_MAX,
>              * so passing the full range of unsigned 64 bit integers
>              * is not safe here.  Pass them as signed 64 bit integers
>              * instead.
>              */
>             long long val = va_arg(args, long long);
>
>             if (!val && type == 'P')
>                 continue;
>
>             rc = virJSONValueObjectAppendNumberLong(obj, key, val);
>         }   break;
>
> So if we "fix" QMP to reject negative values in place of large unsigned
> values, we'll need to also fix libvirt to have a way to introspect the
> difference and cope accordingly.  (I'm thinking that we'll have to keep
> the negative parsing indefinitely, and can merely improve the parser to
> also parse large positive - but that we can still reject values <
> INT64_MIN that would have a weird wraparound).
>

Currently, parsing is done without expected type information. So if the
number fits in an int64, it should be a QINT, otherwise a QFLOAT (see
parse_literal). With this series, it will try QUINT before QFLOAT. But what
I was afraid though is that the int is sometime assigned back to a uint64.
And with my series, get_uint() will fail if the number is negative int, but
for compatibility reasons, I think it should cast it instead. We may want
to have a qmp capability bit to change this silent casting behaviour.
-- 
Marc-André Lureau