Re: [PATCH v3 30/33] block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()

[Eric Blake]

On Fri, Apr 16, 2021 at 11:09:08AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> The only last step we need to reuse the function is coroutine-wrapper.
> nbd_open() may be called from non-coroutine context. So, generate the
> wrapper and use it.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/coroutines.h |   6 +++
>  block/nbd.c        | 101 ++-------------------------------------------
>  2 files changed, 10 insertions(+), 97 deletions(-)
> 
> diff --git a/block/coroutines.h b/block/coroutines.h
> index 4cfb4946e6..514d169d23 100644
> --- a/block/coroutines.h
> +++ b/block/coroutines.h
> @@ -66,4 +66,10 @@ int coroutine_fn bdrv_co_readv_vmstate(BlockDriverState 
> *bs,
>  int coroutine_fn bdrv_co_writev_vmstate(BlockDriverState *bs,
>                                          QEMUIOVector *qiov, int64_t pos);
>  
> +int generated_co_wrapper
> +nbd_do_establish_connection(BlockDriverState *bs, Error **errp);
> +int coroutine_fn
> +nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp);

Tagged coroutine_fn here,...

> +++ b/block/nbd.c
>  
> -static int nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp)
> +int nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp)

...but not here.  Is it worth being consistent?

> @@ -2056,22 +1974,11 @@ static int nbd_open(BlockDriverState *bs, QDict 
> *options, int flags,
>                                          s->x_dirty_bitmap, s->tlscreds,
>                                          monitor_cur());
>  
> -    /*
> -     * establish TCP connection, return error if it fails
> -     * TODO: Configurable retry-until-timeout behaviour.
> -     */
> -    sioc = nbd_establish_connection(bs, s->saddr, errp);
> -    if (!sioc) {
> -        ret = -ECONNREFUSED;
> -        goto fail;
> -    }
> -
> -    ret = nbd_client_handshake(bs, sioc, errp);
> +    /* TODO: Configurable retry-until-timeout behaviour.*/

Space before */

Nice diffstat, proving the refactoring worked.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org