[v3,10/33] block/nbd: make nbd_co_establish_connection_cancel() bs-independent

Commit Message

Vladimir Sementsov-Ogievskiy April 16, 2021, 8:08 a.m. UTC

nbd_co_establish_connection_cancel() actually needs only pointer to
NBDConnectThread. So, make it clean.

Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
Reviewed-by: Roman Kagan <[e-mail]>
---
 block/nbd.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

Comments

Eric Blake June 2, 2021, 9:18 p.m. UTC | #1

On Fri, Apr 16, 2021 at 11:08:48AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> nbd_co_establish_connection_cancel() actually needs only pointer to
> NBDConnectThread. So, make it clean.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
> Reviewed-by: Roman Kagan <[e-mail]>
> ---
>  block/nbd.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index dd97ea0916..dab73bdf3b 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c

>  /*
>   * nbd_co_establish_connection_cancel
> - * Cancel nbd_co_establish_connection asynchronously: it will finish soon, to
> - * allow drained section to begin.
> + * Cancel nbd_co_establish_connection() asynchronously. Note, that it doesn't
> + * stop the thread itself neither close the socket. It just safely wakes
> + * nbd_co_establish_connection() sleeping in the yield().

Grammar suggestion:

Note that this function neither directly stops the thread nor closes
the socket, but rather safely wakes nbd_co_establish_connection()
which is sleeping in yield(), triggering subsequent cleanup there.

Reviewed-by: Eric Blake <[e-mail]>

Patch

diff --git a/block/nbd.c b/block/nbd.c
index dd97ea0916..dab73bdf3b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -123,7 +123,7 @@  static int nbd_establish_connection(BlockDriverState *bs, SocketAddress *saddr,
                                     Error **errp);
 static coroutine_fn QIOChannelSocket *
 nbd_co_establish_connection(NBDConnectThread *thr, Error **errp);
-static void nbd_co_establish_connection_cancel(BlockDriverState *bs);
+static void nbd_co_establish_connection_cancel(NBDConnectThread *thr);
 static int nbd_client_handshake(BlockDriverState *bs, Error **errp);
 static void nbd_yank(void *opaque);
 
@@ -286,7 +286,7 @@  static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs)
         qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
     }
 
-    nbd_co_establish_connection_cancel(bs);
+    nbd_co_establish_connection_cancel(s->connect_thread);
 
     reconnect_delay_timer_del(s);
 
@@ -326,7 +326,7 @@  static void nbd_teardown_connection(BlockDriverState *bs)
         if (s->connection_co_sleep_ns_state) {
             qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
         }
-        nbd_co_establish_connection_cancel(bs);
+        nbd_co_establish_connection_cancel(s->connect_thread);
     }
     if (qemu_in_coroutine()) {
         s->teardown_co = qemu_coroutine_self();
@@ -477,14 +477,12 @@  nbd_co_establish_connection(NBDConnectThread *thr, Error **errp)
 
 /*
  * nbd_co_establish_connection_cancel
- * Cancel nbd_co_establish_connection asynchronously: it will finish soon, to
- * allow drained section to begin.
+ * Cancel nbd_co_establish_connection() asynchronously. Note, that it doesn't
+ * stop the thread itself neither close the socket. It just safely wakes
+ * nbd_co_establish_connection() sleeping in the yield().
  */
-static void nbd_co_establish_connection_cancel(BlockDriverState *bs)
+static void nbd_co_establish_connection_cancel(NBDConnectThread *thr)
 {
-    BDRVNBDState *s = bs->opaque;
-    NBDConnectThread *thr = s->connect_thread;
-
     qemu_mutex_lock(&thr->mutex);
 
     if (thr->wait_co) {