[PATCH v3 18/33] nbd/client-connection: shutdown connection on release
Now, when thread can do negotiation and retry, it may run relatively
long. We need a mechanism to stop it, when user is not interested in
result anymore. So, on nbd_client_connection_release() let's shutdown
the socked, and do not retry connection if thread is detached.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
nbd/client-connection.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index 002bd91f42..54f73c6c24 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -158,9 +158,13 @@ static void *connect_thread_func(void *opaque)
uint64_t timeout = 1;
uint64_t max_timeout = 16;
-while (true) {
+qemu_mutex_lock(&conn->mutex);
+while (!conn->detached) {
+assert(!conn->sioc);
conn->sioc = qio_channel_socket_new();
+qemu_mutex_unlock(&conn->mutex);
+
error_free(conn->err);
conn->err = NULL;
conn->updated_info = conn->initial_info;
@@ -171,14 +175,20 @@ static void *connect_thread_func(void *opaque)
conn->updated_info.x_dirty_bitmap = NULL;
conn->updated_info.name = NULL;
+qemu_mutex_lock(&conn->mutex);
+
if (ret < 0) {
object_unref(OBJECT(conn->sioc));
conn->sioc = NULL;
-if (conn->do_retry) {
+if (conn->do_retry && !conn->detached) {
+qemu_mutex_unlock(&conn->mutex);
+
sleep(timeout);
if (timeout < max_timeout) {
timeout *= 2;
}
+
+qemu_mutex_lock(&conn->mutex);
continue;
}
}
@@ -186,15 +196,17 @@ static void *connect_thread_func(void *opaque)
break;
}
-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;
+/* mutex is locked */
+
+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);
@@ -215,6 +227,10 @@ void nbd_client_connection_release(NBDClientConnection
*conn)
if (conn->running) {
conn->detached = true;
}
+if (conn->sioc) {
+qio_channel_shutdown(QIO_CHANNEL(conn->sioc),
+ QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
do_free = !conn->running && !conn->detached;
qemu_mutex_unlock(&conn->mutex);
--
2.29.2
Re: [PATCH v3 18/33] nbd/client-connection: shutdown connection on release
On Fri, Apr 16, 2021 at 11:08:56AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Now, when thread can do negotiation and retry, it may run relatively
> long. We need a mechanism to stop it, when user is not interested in
> result anymore. So, on nbd_client_connection_release() let's shutdown
> the socked, and do not retry connection if thread is detached.
This kinda answers my question to the previous patch about reconnect
cancellation.
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
> nbd/client-connection.c | 36 ++--
> 1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/nbd/client-connection.c b/nbd/client-connection.c
> index 002bd91f42..54f73c6c24 100644
> --- a/nbd/client-connection.c
> +++ b/nbd/client-connection.c
> @@ -158,9 +158,13 @@ static void *connect_thread_func(void *opaque)
> uint64_t timeout = 1;
> uint64_t max_timeout = 16;
>
> -while (true) {
> +qemu_mutex_lock(&conn->mutex);
> +while (!conn->detached) {
> +assert(!conn->sioc);
> conn->sioc = qio_channel_socket_new();
>
> +qemu_mutex_unlock(&conn->mutex);
> +
> error_free(conn->err);
> conn->err = NULL;
> conn->updated_info = conn->initial_info;
> @@ -171,14 +175,20 @@ static void *connect_thread_func(void *opaque)
> conn->updated_info.x_dirty_bitmap = NULL;
> conn->updated_info.name = NULL;
>
> +qemu_mutex_lock(&conn->mutex);
> +
> if (ret < 0) {
> object_unref(OBJECT(conn->sioc));
> conn->sioc = NULL;
> -if (conn->do_retry) {
> +if (conn->do_retry && !conn->detached) {
> +qemu_mutex_unlock(&conn->mutex);
> +
> sleep(timeout);
> if (timeout < max_timeout) {
> timeout *= 2;
> }
> +
> +qemu_mutex_lock(&conn->mutex);
> continue;
> }
> }
> @@ -186,15 +196,17 @@ static void *connect_thread_func(void *opaque)
> break;
> }
>
> -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;
> +/* mutex is locked */
> +
> +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);
This basically reverts some hunks from patch 15 "nbd/client-connection:
use QEMU_LOCK_GUARD". How about dropping them there (they weren't an
obvious improvement even then).
Roman.
>
> if (do_free) {
> nbd_client_connection_do_free(conn);
> @@ -215,6 +227,10 @@ void nbd_client_connection_release(NBDClientConnection
> *conn)
> if (conn->running) {
> conn->detached = true;
> }
> +if (conn->sioc) {
> +qio_channel_shutdown(QIO_CHANNEL(conn->sioc),
> + QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
> +}
> do_free = !conn->running && !conn->detached;
> qemu_mutex_unlock(&conn->mutex);
>
> --
> 2.29.2
>
Re: [PATCH v3 18/33] nbd/client-connection: shutdown connection on release
12.05.2021 00:08, Roman Kagan wrote:
On Fri, Apr 16, 2021 at 11:08:56AM +0300, Vladimir Sementsov-Ogievskiy wrote:
Now, when thread can do negotiation and retry, it may run relatively
long. We need a mechanism to stop it, when user is not interested in
result anymore. So, on nbd_client_connection_release() let's shutdown
the socked, and do not retry connection if thread is detached.
This kinda answers my question to the previous patch about reconnect
cancellation.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
nbd/client-connection.c | 36 ++--
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/nbd/client-connection.c b/nbd/client-connection.c
index 002bd91f42..54f73c6c24 100644
--- a/nbd/client-connection.c
+++ b/nbd/client-connection.c
@@ -158,9 +158,13 @@ static void *connect_thread_func(void *opaque)
uint64_t timeout = 1;
uint64_t max_timeout = 16;
-while (true) {
+qemu_mutex_lock(&conn->mutex);
+while (!conn->detached) {
+assert(!conn->sioc);
conn->sioc = qio_channel_socket_new();
+qemu_mutex_unlock(&conn->mutex);
+
error_free(conn->err);
conn->err = NULL;
conn->updated_info = conn->initial_info;
@@ -171,14 +175,20 @@ static void *connect_thread_func(void *opaque)
conn->updated_info.x_dirty_bitmap = NULL;
conn->updated_info.name = NULL;
+qemu_mutex_lock(&conn->mutex);
+
if (ret < 0) {
object_unref(OBJECT(conn->sioc));
conn->sioc = NULL;
-if (conn->do_retry) {
+if (conn->do_retry && !conn->detached) {
+qemu_mutex_unlock(&conn->mutex);
+
sleep(timeout);
if (timeout < max_timeout) {
timeout *= 2;
}
+
+qemu_mutex_lock(&conn->mutex);
continue;
}
}
@@ -186,15 +196,17 @@ static void *connect_thread_func(void *opaque)
break;
}
-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;
+/* mutex is locked */
+
+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);
This basically reverts some hunks from patch 15 "nbd/client-connection:
use QEMU_LOCK_GUARD". How about dropping them there (they weren't an
obvious improvement even then).
OK, will do
if (do_free) {
nbd_client_connection_do_free(conn);
@@ -215,6 +227,10 @@ void nbd_client_connection_release(NBDClientConnection
*conn)
if (conn->running) {
conn->detached = true;
}
+if (conn->sioc) {
+qio_channel_shutdown(QIO_CHANNEL(conn->sioc),
+ QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
do_free = !conn->running && !conn->detached;
qemu_mutex_unlock(&conn->mutex);
--
2.29.2
--
Best regards,
Vladimir
Re: [PATCH v3 18/33] nbd/client-connection: shutdown connection on release
On Fri, Apr 16, 2021 at 11:08:56AM +0300, Vladimir Sementsov-Ogievskiy wrote: > Now, when thread can do negotiation and retry, it may run relatively when a thread > long. We need a mechanism to stop it, when user is not interested in when the user > result anymore. So, on nbd_client_connection_release() let's shutdown in a result any more. > the socked, and do not retry connection if thread is detached. socket > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > nbd/client-connection.c | 36 ++-- > 1 file changed, 26 insertions(+), 10 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
