[libvirt] [PATCH 1/2] qemu-JSON: Error out if number is out of range instead of overflowing to negative

[Peter Krempa]

On 04/03/13 15:29, Eric Blake wrote:
> On 04/03/2013 02:46 AM, Peter Krempa wrote:
>> Commit 78eb8b60d59662271c4a9a1be8c9002ee84dc8cf works around qemu's inability to
>> parse unsigned 64 bit integers by representing them as signed. This introduces a
>> bug where if the requested integer is greater than LLONG_MAX the result is
>> wrapped to negative numbers.
>
> But then qemu, on the receiving end, turns the negative number back into
> the appropriate bit pattern for an unsigned value, which lets us get
> around the fact that passing a positive unsigned value > LLONG_MAX would
> be truncated.

Hmmm, then we should at least comment that this is intentional and that 
qemu should be able to reconstruct the correct value from this as it 
looks wrong at the first look. Also qemu apparently uses this code wrong 
at least for the bandwidth limits for block-* operations.

>
>>
>> This patch adds a check to avoid the wrap for unsigned numbers and error out
>> rather than passing them along.
>> ---
>>   src/qemu/qemu_monitor_json.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> This would prevent us from sending bit patterns that qemu will interpret
> correctly, even if the JSON used in sending the bit pattern is expressed

I wasn't aware that this is supposed to work this way and I neither 
learned that from the mail thread linked in the original patch. From the 
issue in the linked BZ I inferred that we are passing negative values to 
qemu and started to looking why that was happening.

> as a negative number, where we could previously send such a pattern.
> I'm inclined to NACK this patch; unless you have a more concrete example
> of qemu no longer interpreting a negative string as the correct unsigned

I'm okay with not putting this in, but then I'll document that the 
signed value is intentional and correctly parsed (in some cases) in qemu.

> bit value.
>

Peter