On Fri, Apr 16, 2021 at 11:08:53AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
> ---
> 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
>
