[v3,27/33] block/nbd: split nbd_co_do_establish_connection out of nbd_reconnect_attempt

Commit Message

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

Split out part, that we want to reuse for nbd_open().

Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
---
 block/nbd.c | 79 +++++++++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 38 deletions(-)

Comments

Eric Blake June 3, 2021, 8:04 p.m. UTC | #1

On Fri, Apr 16, 2021 at 11:09:05AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Split out part, that we want to reuse for nbd_open().

Split out the part that we want to reuse for nbd_open().

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
> ---
>  block/nbd.c | 79 +++++++++++++++++++++++++++--------------------------
>  1 file changed, 41 insertions(+), 38 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 15b5921725..59971bfba8 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -361,11 +361,49 @@ static int nbd_handle_updated_info(BlockDriverState *bs, Error **errp)
>      return 0;
>  }
>  
> -static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
> +static int nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp)

Given the _co_ in the name, don't you need a coroutine_fn marker?

Otherwise looks sane.

Vladimir Sementsov-Ogievskiy June 4, 2021, 5:30 a.m. UTC | #2

03.06.2021 23:04, Eric Blake wrote:
> On Fri, Apr 16, 2021 at 11:09:05AM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Split out part, that we want to reuse for nbd_open().
> 
> Split out the part that we want to reuse for nbd_open().
> 
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <[e-mail]>
>> ---
>>   block/nbd.c | 79 +++++++++++++++++++++++++++--------------------------
>>   1 file changed, 41 insertions(+), 38 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 15b5921725..59971bfba8 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -361,11 +361,49 @@ static int nbd_handle_updated_info(BlockDriverState *bs, Error **errp)
>>       return 0;
>>   }
>>   
>> -static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
>> +static int nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp)
> 
> Given the _co_ in the name, don't you need a coroutine_fn marker?

Yes, strange that I've dropped it

> 
> Otherwise looks sane.
>

Patch

diff --git a/block/nbd.c b/block/nbd.c
index 15b5921725..59971bfba8 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -361,11 +361,49 @@  static int nbd_handle_updated_info(BlockDriverState *bs, Error **errp)
     return 0;
 }
 
-static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
+static int nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp)
 {
+    BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
     int ret;
-    AioContext *aio_context = bdrv_get_aio_context(s->bs);
 
+    assert(!s->ioc);
+
+    s->ioc = nbd_co_establish_connection(s->conn, &s->info, errp);
+    if (!s->ioc) {
+        return -ECONNREFUSED;
+    }
+
+    ret = nbd_handle_updated_info(s->bs, NULL);
+    if (ret < 0) {
+        /*
+         * We have connected, but must fail for other reasons.
+         * Send NBD_CMD_DISC as a courtesy to the server.
+         */
+        NBDRequest request = { .type = NBD_CMD_DISC };
+
+        nbd_send_request(s->ioc, &request);
+
+        object_unref(OBJECT(s->ioc));
+        s->ioc = NULL;
+
+        return ret;
+    }
+
+    qio_channel_set_blocking(s->ioc, false, NULL);
+    qio_channel_attach_aio_context(s->ioc, bdrv_get_aio_context(bs));
+
+    yank_register_function(BLOCKDEV_YANK_INSTANCE(s->bs->node_name), nbd_yank,
+                           bs);
+
+    /* successfully connected */
+    s->state = NBD_CLIENT_CONNECTED;
+    qemu_co_queue_restart_all(&s->free_sema);
+
+    return 0;
+}
+
+static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
+{
     if (!nbd_client_connecting(s)) {
         return;
     }
@@ -403,42 +441,7 @@  static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
         s->ioc = NULL;
     }
 
-    s->ioc = nbd_co_establish_connection(s->conn, &s->info, NULL);
-    if (!s->ioc) {
-        ret = -ECONNREFUSED;
-        goto out;
-    }
-
-    qio_channel_set_blocking(QIO_CHANNEL(s->ioc), false, NULL);
-    qio_channel_attach_aio_context(QIO_CHANNEL(s->ioc), aio_context);
-
-    yank_register_function(BLOCKDEV_YANK_INSTANCE(s->bs->node_name), nbd_yank,
-                           s->bs);
-
-    ret = nbd_handle_updated_info(s->bs, NULL);
-    if (ret < 0) {
-        /*
-         * We have connected, but must fail for other reasons.
-         * Send NBD_CMD_DISC as a courtesy to the server.
-         */
-        NBDRequest request = { .type = NBD_CMD_DISC };
-
-        nbd_send_request(s->ioc, &request);
-
-        yank_unregister_function(BLOCKDEV_YANK_INSTANCE(s->bs->node_name),
-                                 nbd_yank, s->bs);
-        object_unref(OBJECT(s->ioc));
-        s->ioc = NULL;
-
-        return;
-    }
-
-out:
-    if (ret >= 0) {
-        /* successfully connected */
-        s->state = NBD_CLIENT_CONNECTED;
-        qemu_co_queue_restart_all(&s->free_sema);
-    }
+    nbd_co_do_establish_connection(s->bs, NULL);
 }
 
 static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s)