| Message ID | 20210416080911.83197-16-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | block/nbd: rework client connection | expand |
On Fri, Apr 16, 2021 at 11:08:53AM +0300, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > nbd/client-connection.c | 94 ++++++++++++++++++----------------------- > 1 file changed, 42 insertions(+), 52 deletions(-) > > diff --git a/nbd/client-connection.c b/nbd/client-connection.c > index 4e39a5b1af..b45a0bd5f6 100644 > --- a/nbd/client-connection.c > +++ b/nbd/client-connection.c > @@ -87,17 +87,16 @@ static void *connect_thread_func(void *opaque) > conn->sioc = NULL; > } > > - qemu_mutex_lock(&conn->mutex); > - > - assert(conn->running); > - conn->running = false; > - if (conn->wait_co) { > - aio_co_schedule(NULL, conn->wait_co); > - conn->wait_co = NULL; > + WITH_QEMU_LOCK_GUARD(&conn->mutex) { > + assert(conn->running); > + conn->running = false; > + if (conn->wait_co) { > + aio_co_schedule(NULL, conn->wait_co); > + conn->wait_co = NULL; > + } > } > do_free = conn->detached; ->detached is now accessed outside the mutex > > - qemu_mutex_unlock(&conn->mutex); > > if (do_free) { > nbd_client_connection_do_free(conn); > @@ -136,61 +135,54 @@ void nbd_client_connection_release(NBDClientConnection *conn) > QIOChannelSocket *coroutine_fn > nbd_co_establish_connection(NBDClientConnection *conn, Error **errp) > { > - QIOChannelSocket *sioc = NULL; > QemuThread thread; > > - qemu_mutex_lock(&conn->mutex); > - > - /* > - * Don't call nbd_co_establish_connection() in several coroutines in > - * parallel. Only one call at once is supported. > - */ > - assert(!conn->wait_co); > - > - if (!conn->running) { > - if (conn->sioc) { > - /* Previous attempt finally succeeded in background */ > - sioc = g_steal_pointer(&conn->sioc); > - qemu_mutex_unlock(&conn->mutex); > - > - return sioc; > + WITH_QEMU_LOCK_GUARD(&conn->mutex) { > + /* > + * Don't call nbd_co_establish_connection() in several coroutines in > + * parallel. Only one call at once is supported. > + */ > + assert(!conn->wait_co); > + > + if (!conn->running) { > + if (conn->sioc) { > + /* Previous attempt finally succeeded in background */ > + return g_steal_pointer(&conn->sioc); > + } > + > + conn->running = true; > + error_free(conn->err); > + conn->err = NULL; > + qemu_thread_create(&thread, "nbd-connect", > + connect_thread_func, conn, QEMU_THREAD_DETACHED); > } > > - conn->running = true; > - error_free(conn->err); > - conn->err = NULL; > - qemu_thread_create(&thread, "nbd-connect", > - connect_thread_func, conn, QEMU_THREAD_DETACHED); > + conn->wait_co = qemu_coroutine_self(); > } > > - conn->wait_co = qemu_coroutine_self(); > - > - qemu_mutex_unlock(&conn->mutex); > - > /* > * We are going to wait for connect-thread finish, but > * nbd_co_establish_connection_cancel() can interrupt. > */ > qemu_coroutine_yield(); > > - qemu_mutex_lock(&conn->mutex); > - > - if (conn->running) { > - /* > - * Obviously, drained section wants to start. Report the attempt as > - * failed. Still connect thread is executing in background, and its > - * result may be used for next connection attempt. > - */ > - error_setg(errp, "Connection attempt cancelled by other operation"); > - } else { > - error_propagate(errp, conn->err); > - conn->err = NULL; > - sioc = g_steal_pointer(&conn->sioc); > + WITH_QEMU_LOCK_GUARD(&conn->mutex) { > + if (conn->running) { > + /* > + * Obviously, drained section wants to start. Report the attempt as > + * failed. Still connect thread is executing in background, and its > + * result may be used for next connection attempt. > + */ > + error_setg(errp, "Connection attempt cancelled by other operation"); > + return NULL; > + } else { > + error_propagate(errp, conn->err); > + conn->err = NULL; > + return g_steal_pointer(&conn->sioc); > + } > } > > - qemu_mutex_unlock(&conn->mutex); > - > - return sioc; > + abort(); /* unreachable */ > } > > /* > @@ -201,12 +193,10 @@ nbd_co_establish_connection(NBDClientConnection *conn, Error **errp) > */ > void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn) > { > - qemu_mutex_lock(&conn->mutex); > + QEMU_LOCK_GUARD(&conn->mutex); > > if (conn->wait_co) { > aio_co_schedule(NULL, conn->wait_co); > conn->wait_co = NULL; > } > - > - qemu_mutex_unlock(&conn->mutex); > } > -- > 2.29.2 >
28.04.2021 09:08, Roman Kagan wrote: > On Fri, Apr 16, 2021 at 11:08:53AM +0300, Vladimir Sementsov-Ogievskiy wrote: >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> nbd/client-connection.c | 94 ++++++++++++++++++----------------------- >> 1 file changed, 42 insertions(+), 52 deletions(-) >> >> diff --git a/nbd/client-connection.c b/nbd/client-connection.c >> index 4e39a5b1af..b45a0bd5f6 100644 >> --- a/nbd/client-connection.c >> +++ b/nbd/client-connection.c >> @@ -87,17 +87,16 @@ static void *connect_thread_func(void *opaque) >> conn->sioc = NULL; >> } >> >> - qemu_mutex_lock(&conn->mutex); >> - >> - assert(conn->running); >> - conn->running = false; >> - if (conn->wait_co) { >> - aio_co_schedule(NULL, conn->wait_co); >> - conn->wait_co = NULL; >> + WITH_QEMU_LOCK_GUARD(&conn->mutex) { >> + assert(conn->running); >> + conn->running = false; >> + if (conn->wait_co) { >> + aio_co_schedule(NULL, conn->wait_co); >> + conn->wait_co = NULL; >> + } >> } >> do_free = conn->detached; > > ->detached is now accessed outside the mutex Oops. Will fix. > >> >> - qemu_mutex_unlock(&conn->mutex); >> >> if (do_free) { >> nbd_client_connection_do_free(conn); >> @@ -136,61 +135,54 @@ void nbd_client_connection_release(NBDClientConnection *conn) >> QIOChannelSocket *coroutine_fn >> nbd_co_establish_connection(NBDClientConnection *conn, Error **errp) >> { >> - QIOChannelSocket *sioc = NULL; >> QemuThread thread; >> >> - qemu_mutex_lock(&conn->mutex); >> - >> - /* >> - * Don't call nbd_co_establish_connection() in several coroutines in >> - * parallel. Only one call at once is supported. >> - */ >> - assert(!conn->wait_co); >> - >> - if (!conn->running) { >> - if (conn->sioc) { >> - /* Previous attempt finally succeeded in background */ >> - sioc = g_steal_pointer(&conn->sioc); >> - qemu_mutex_unlock(&conn->mutex); >> - >> - return sioc; >> + WITH_QEMU_LOCK_GUARD(&conn->mutex) { >> + /* >> + * Don't call nbd_co_establish_connection() in several coroutines in >> + * parallel. Only one call at once is supported. >> + */ >> + assert(!conn->wait_co); >> + >> + if (!conn->running) { >> + if (conn->sioc) { >> + /* Previous attempt finally succeeded in background */ >> + return g_steal_pointer(&conn->sioc); >> + } >> + >> + conn->running = true; >> + error_free(conn->err); >> + conn->err = NULL; >> + qemu_thread_create(&thread, "nbd-connect", >> + connect_thread_func, conn, QEMU_THREAD_DETACHED); >> } >> >> - conn->running = true; >> - error_free(conn->err); >> - conn->err = NULL; >> - qemu_thread_create(&thread, "nbd-connect", >> - connect_thread_func, conn, QEMU_THREAD_DETACHED); >> + conn->wait_co = qemu_coroutine_self(); >> } >> >> - conn->wait_co = qemu_coroutine_self(); >> - >> - qemu_mutex_unlock(&conn->mutex); >> - >> /* >> * We are going to wait for connect-thread finish, but >> * nbd_co_establish_connection_cancel() can interrupt. >> */ >> qemu_coroutine_yield(); >> >> - qemu_mutex_lock(&conn->mutex); >> - >> - if (conn->running) { >> - /* >> - * Obviously, drained section wants to start. Report the attempt as >> - * failed. Still connect thread is executing in background, and its >> - * result may be used for next connection attempt. >> - */ >> - error_setg(errp, "Connection attempt cancelled by other operation"); >> - } else { >> - error_propagate(errp, conn->err); >> - conn->err = NULL; >> - sioc = g_steal_pointer(&conn->sioc); >> + WITH_QEMU_LOCK_GUARD(&conn->mutex) { >> + if (conn->running) { >> + /* >> + * Obviously, drained section wants to start. Report the attempt as >> + * failed. Still connect thread is executing in background, and its >> + * result may be used for next connection attempt. >> + */ >> + error_setg(errp, "Connection attempt cancelled by other operation"); >> + return NULL; >> + } else { >> + error_propagate(errp, conn->err); >> + conn->err = NULL; >> + return g_steal_pointer(&conn->sioc); >> + } >> } >> >> - qemu_mutex_unlock(&conn->mutex); >> - >> - return sioc; >> + abort(); /* unreachable */ >> } >> >> /* >> @@ -201,12 +193,10 @@ nbd_co_establish_connection(NBDClientConnection *conn, Error **errp) >> */ >> void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn) >> { >> - qemu_mutex_lock(&conn->mutex); >> + QEMU_LOCK_GUARD(&conn->mutex); >> >> if (conn->wait_co) { >> aio_co_schedule(NULL, conn->wait_co); >> conn->wait_co = NULL; >> } >> - >> - qemu_mutex_unlock(&conn->mutex); >> } >> -- >> 2.29.2 >>
diff --git a/nbd/client-connection.c b/nbd/client-connection.c index 4e39a5b1af..b45a0bd5f6 100644 --- a/nbd/client-connection.c +++ b/nbd/client-connection.c @@ -87,17 +87,16 @@ static void *connect_thread_func(void *opaque) conn->sioc = NULL; } - qemu_mutex_lock(&conn->mutex); - - assert(conn->running); - conn->running = false; - if (conn->wait_co) { - aio_co_schedule(NULL, conn->wait_co); - conn->wait_co = NULL; + WITH_QEMU_LOCK_GUARD(&conn->mutex) { + assert(conn->running); + conn->running = false; + if (conn->wait_co) { + aio_co_schedule(NULL, conn->wait_co); + conn->wait_co = NULL; + } } do_free = conn->detached; - qemu_mutex_unlock(&conn->mutex); if (do_free) { nbd_client_connection_do_free(conn); @@ -136,61 +135,54 @@ void nbd_client_connection_release(NBDClientConnection *conn) QIOChannelSocket *coroutine_fn nbd_co_establish_connection(NBDClientConnection *conn, Error **errp) { - QIOChannelSocket *sioc = NULL; QemuThread thread; - qemu_mutex_lock(&conn->mutex); - - /* - * Don't call nbd_co_establish_connection() in several coroutines in - * parallel. Only one call at once is supported. - */ - assert(!conn->wait_co); - - if (!conn->running) { - if (conn->sioc) { - /* Previous attempt finally succeeded in background */ - sioc = g_steal_pointer(&conn->sioc); - qemu_mutex_unlock(&conn->mutex); - - return sioc; + WITH_QEMU_LOCK_GUARD(&conn->mutex) { + /* + * Don't call nbd_co_establish_connection() in several coroutines in + * parallel. Only one call at once is supported. + */ + assert(!conn->wait_co); + + if (!conn->running) { + if (conn->sioc) { + /* Previous attempt finally succeeded in background */ + return g_steal_pointer(&conn->sioc); + } + + conn->running = true; + error_free(conn->err); + conn->err = NULL; + qemu_thread_create(&thread, "nbd-connect", + connect_thread_func, conn, QEMU_THREAD_DETACHED); } - conn->running = true; - error_free(conn->err); - conn->err = NULL; - qemu_thread_create(&thread, "nbd-connect", - connect_thread_func, conn, QEMU_THREAD_DETACHED); + conn->wait_co = qemu_coroutine_self(); } - conn->wait_co = qemu_coroutine_self(); - - qemu_mutex_unlock(&conn->mutex); - /* * We are going to wait for connect-thread finish, but * nbd_co_establish_connection_cancel() can interrupt. */ qemu_coroutine_yield(); - qemu_mutex_lock(&conn->mutex); - - if (conn->running) { - /* - * Obviously, drained section wants to start. Report the attempt as - * failed. Still connect thread is executing in background, and its - * result may be used for next connection attempt. - */ - error_setg(errp, "Connection attempt cancelled by other operation"); - } else { - error_propagate(errp, conn->err); - conn->err = NULL; - sioc = g_steal_pointer(&conn->sioc); + WITH_QEMU_LOCK_GUARD(&conn->mutex) { + if (conn->running) { + /* + * Obviously, drained section wants to start. Report the attempt as + * failed. Still connect thread is executing in background, and its + * result may be used for next connection attempt. + */ + error_setg(errp, "Connection attempt cancelled by other operation"); + return NULL; + } else { + error_propagate(errp, conn->err); + conn->err = NULL; + return g_steal_pointer(&conn->sioc); + } } - qemu_mutex_unlock(&conn->mutex); - - return sioc; + abort(); /* unreachable */ } /* @@ -201,12 +193,10 @@ nbd_co_establish_connection(NBDClientConnection *conn, Error **errp) */ void coroutine_fn nbd_co_establish_connection_cancel(NBDClientConnection *conn) { - qemu_mutex_lock(&conn->mutex); + QEMU_LOCK_GUARD(&conn->mutex); if (conn->wait_co) { aio_co_schedule(NULL, conn->wait_co); conn->wait_co = NULL; } - - qemu_mutex_unlock(&conn->mutex); }
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- nbd/client-connection.c | 94 ++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 52 deletions(-)