Re: [PATCH v3 09/33] block/nbd: bs-independent interface for nbd_co_establish_connection()

[Eric Blake]

On Fri, Apr 16, 2021 at 11:08:47AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We are going to split connection code to separate file. Now we are

to a separate

> ready to give nbd_co_establish_connection() clean and bs-independent
> interface.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru>
> ---
>  block/nbd.c | 49 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 18 deletions(-)
> 

> -static int coroutine_fn
> -nbd_co_establish_connection(BlockDriverState *bs, Error **errp)
> +/*
> + * Get a new connection in context of @thr:
> + *   if thread is running, wait for completion

if the thread is running,...

> + *   if thread is already succeeded in background, and user didn't get the

if the thread already succeeded in the background,...

> + *     result, just return it now
> + *   otherwise if thread is not running, start a thread and wait for 
> completion

otherwise, the thread is not running, so start...

> + */
> +static coroutine_fn QIOChannelSocket *
> +nbd_co_establish_connection(NBDConnectThread *thr, Error **errp)
>  {
> +    QIOChannelSocket *sioc = NULL;
>      QemuThread thread;
> -    BDRVNBDState *s = bs->opaque;
> -    NBDConnectThread *thr = s->connect_thread;
> -
> -    assert(!s->sioc);
>  
>      qemu_mutex_lock(&thr->mutex);
>  
> +    /*
> +     * Don't call nbd_co_establish_connection() in several coroutines in
> +     * parallel. Only one call at once is supported.
> +     */
> +    assert(!thr->wait_co);
> +
>      if (!thr->running) {
>          if (thr->sioc) {
>              /* Previous attempt finally succeeded in background */
> -            goto out;
> +            sioc = g_steal_pointer(&thr->sioc);
> +            qemu_mutex_unlock(&thr->mutex);

Worth using QEMU_LOCK_GUARD() here?

> +
> +            return sioc;
>          }
> +
>          thr->running = true;
>          error_free(thr->err);
>          thr->err = NULL;

Reviewed-by: Eric Blake <eblake@redhat.com>

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