Re: [Qemu-devel] [PATCH 06/11] nbd: make it thread-safe

[Eric Blake]

On 05/31/2017 04:43 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  block/nbd-client.c | 30 +++++++++---------------------
>  1 file changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 1e2952fdae..43e0292ac1 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -114,6 +114,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
>      int rc, ret, i;
>  
>      qemu_co_mutex_lock(&s->send_mutex);
> +    while (s->in_flight == MAX_NBD_REQUESTS) {
> +        qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
> +    }
> +    s->in_flight++;

Nice - if I'm not mistaken, this also solves
https://bugzilla.redhat.com/show_bug.cgi?id=1454582 - you have a while
loop here...

> -static void nbd_coroutine_start(NBDClientSession *s,
> -                                NBDRequest *request)
> -{
> -    /* Poor man semaphore.  The free_sema is locked when no other request
> -     * can be accepted, and unlocked after receiving one reply.  */
> -    if (s->in_flight == MAX_NBD_REQUESTS) {
> -        qemu_co_queue_wait(&s->free_sema, NULL);
> -        assert(s->in_flight < MAX_NBD_REQUESTS);
> -    }

...compared to the old code that only tried once, and could therefore
hit the assertion failure depending on thread load.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature