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

[Vladimir Sementsov-Ogievskiy]

02.06.2021 22:14, Eric Blake wrote:
> 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?

Refactored together with other critical sections in patch 15

> > +
> > +            return sioc;
> >           }
> > +
> >           thr->running = true;
> >           error_free(thr->err);
> >           thr->err = NULL;
>
> Reviewed-by: Eric Blake <eblake@redhat.com>


--
Best regards,
Vladimir