Re: [Qemu-block] [Qemu-devel] [RFC PATCH 04/56] char: Make ringbuf-read size unsigned in QAPI/QMP

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Hi
>
> On Tue, Aug 22, 2017 at 3:00 PM, Markus Armbruster <address@hidden> wrote:
>> Marc-André Lureau <address@hidden> writes:
>>
>>> Hi
>>>
>>> On Mon, Aug 7, 2017 at 4:45 PM, Markus Armbruster <address@hidden> wrote:
>>>> Sizes should use QAPI type 'size' (uint64_t).  ringbuf-read parameter
>>>> @size is 'int' (int64_t).  qmp_ringbuf_read() rejects negative values,
>>>> then implicitly converts to size_t.
>>>>
>>>> Change the parameter to 'size' and drop the check for negative values.
>>>>
>>>> ringbuf-read now accepts size values between 2^63 and 2^64-1.  It
>>>> accepts negative values as before, because that's how the QObject
>>>> input visitor works for backward compatibility.
>>>>
>>>
>>> Negative values over json will be implicitly converted to positive
>>> values with this change, right? Or are they rejected earlier?
>>
>> Yes.  For details, see my reply to Juan's review of PATCH 15.
>>
>>> If so that is a change of behaviour that I am not sure is worth doing
>>> now (without explicit protocol break), but I don't mind.
>
> So before this change:
>
> (QEMU) ringbuf-read device=foo size=-1
> {"error": {"class": "GenericError", "desc": "size must be greater than zero"}}
>
> after:
>
> (QEMU) ringbuf-read device=foo size=-1
> {"return": ""}
>
> Is this really what we want?

Yes, because it's what all the other commands do with byte counts, and
because it's the price of admission for "size": 18446744073709551615 to
work.

We could split QAPI type size into legacy-size (accepts negative for
backward compatibility, do not use in new code) and size (rejects
negative, do use in new code and for cases that previously rejected
negative manually).  Trades consistency for tightness.

This series picked consistency.