Am 10.01.2013 um 12:52 schrieb Markus Armbruster <arm...@redhat.com>:
> Paolo Bonzini <pbonz...@redhat.com> writes:
>
>> Il 10/01/2013 11:45, Markus Armbruster ha scritto:
>>>
>>>>> If io_limits are specified during runtime that exceed the number
>>>>> of operations in flight
>>>>> bs->io_base is not initialized in the else statement in
>>>>> bdrv_exceed_io_limits().
>>> I'm confused.
>>>
>>> if ((bs->slice_start < now)
>>> && (bs->slice_end > now)) {
>>> bs->slice_end = now + bs->slice_time;
>>> } else {
>>> bs->slice_time = 5 * BLOCK_IO_SLICE_TIME;
>>> bs->slice_start = now;
>>> bs->slice_end = now + bs->slice_time;
>>>
>>> bs->io_base.bytes[is_write] = bs->nr_bytes[is_write];
>>> bs->io_base.bytes[!is_write] = bs->nr_bytes[!is_write];
>>>
>>> bs->io_base.ios[is_write] = bs->nr_ops[is_write];
>>> bs->io_base.ios[!is_write] = bs->nr_ops[!is_write];
>>> }
>>
>> bdrv_io_limits_enable correctly starts a new slice (the first three
>> lines of the else), but does not set io_base correctly for that slice.
>> Here is how io_base is used:
>>
>> bytes_base = bs->nr_bytes[is_write] - bs->io_base.bytes[is_write];
>> bytes_res = (unsigned) nb_sectors * BDRV_SECTOR_SIZE;
>>
>> if (bytes_base + bytes_res <= bytes_limit) {
>> /* no wait */
>> } else {
>> /* operation needs to be throttled */
>> }
>>
>> As a result, any I/O operations that are triggered between now and
>> bs->slice_end are incorrectly limited. If 10 MB of data has been
>> written since the VM was started, QEMU thinks that 10 MB of data has
>> been written in this slice.
>
> Work that into the commit message, and I'm happy :)
Paolo, if you agree I would resubmit the patch (using your description).
I would not directly collapse the code to as its not obvious what
bdrv_exceed_io_limits(bs, 0, 0, NULL);
is doing. Maybe this could be done in a later patch.
Peter
