Re: [Qemu-devel] [PATCH] block: init bs->io_base correctly to avoid locking

[Markus Armbruster]

Paolo Bonzini <address@hidden> 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 :)