Re: [Qemu-devel] [PATCH V3 for-1.6 1/5] block: Repair the throttling code.

[Eric Blake]

On 07/23/2013 10:29 AM, Benoît Canet wrote:
> The throttling code was segfaulting since commit
> 02ffb504485f0920cfc75a0982a602f824a9a4f4 because some qemu_co_queue_next 
> caller
> does not run in a coroutine.
> qemu_co_queue_do_restart assume that the caller is a coroutinne.

s/assume/assumes/; s/coroutinne/coroutine/

> As suggested by Stefan fix this by entering the coroutine directly.
> Also make sure like suggested that qemu_co_queue_next() and
> qemu_co_queue_restart_all() can be called only in coroutines.
> 
> Signed-off-by: Benoit Canet <address@hidden>
> ---
>  block.c                   |    8 ++++----
>  include/block/coroutine.h |    9 +++++++--
>  qemu-coroutine-lock.c     |   20 ++++++++++++++++++--
>  3 files changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/block.c b/block.c
> index b560241..dc72643 100644
> --- a/block.c
> +++ b/block.c
> @@ -127,7 +127,8 @@ void bdrv_io_limits_disable(BlockDriverState *bs)
>  {
>      bs->io_limits_enabled = false;
>  
> -    while (qemu_co_queue_next(&bs->throttled_reqs));
> +    while (qemu_co_enter_next(&bs->throttled_reqs)) {
> +    }

On first read, I missed the s/queue/enter/ change and thought all you
were doing was the s/;/{}/ change.  Is the style change necessary to
keep checkpatch.pl happy?  If not, then keeping the old style would draw
better attention to the bug fix.

This patch is worth 1.6, even if the rest of the series is not.  I'm not
a coroutine expert, so take this with a grain of salt:
Reviewed-by: Eric Blake <address@hidden>

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

signature.asc
Description: OpenPGP digital signature