[libvirt] [PATCH v3 05/18] blockjob: hoist bandwidth scaling out of monitor code

[Eric Blake]

On 09/04/2014 09:54 AM, Peter Krempa wrote:
> On 08/31/14 06:02, Eric Blake wrote:
>> qemu treats blockjob bandwidth as a 64-bit number, in the units
>> of bytes/second.  But we stupidly modeled block job bandwidth
>> after migration bandwidth, which in turn was an 'unsigned long'
>> and therefore subject to 32-bit vs. 64-bit interpretations, and
>> with a scale of MiB/s.  Our code already has to convert between
>> the two scales, and report overflow as appropriate; although
>> this conversion currently lives in the monitor code.
>>


>>
>> -    /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
>> -     * limited to LLONG_MAX also for unsigned values */
>> -    speed = bandwidth;
>> -    if (speed > LLONG_MAX >> 20) {
>> -        virReportError(VIR_ERR_OVERFLOW,
>> -                       _("bandwidth must be less than %llu"),
>> -                       LLONG_MAX >> 20);
> 
> I'd keep the check for if (speed > LLONG_MAX) here to be sure that we
> don't pass something between LLONG_MAX and ULLONG_MAX to qemu as it
> would be converted to signed by the monitor code.

The callers in qemu_driver.c are making the same check, so by this
point, it is just a redundant safety check.  I'm not sure it adds
anything, since it is not arbitrary user input so much as protection
against developer botches.


> 
> Possibly we could add a new conversion option to
> qemuMonitorJSONMakeCommand that would check and reject numbers between
> LLONG_MAX and ULLONG_MAX rather than converting them to signed silently...

That actually sounds interesting - a new code that requires a positive
value within a signed value.  I'll see what it looks like for v4.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140904/4f47a626/attachment-0001.sig>