Re: [PATCH v3 14/33] nbd: move connection code from block/nbd to nbd/client-connection

[Eric Blake <eblake@redhat.com>]

On Fri, Apr 16, 2021 at 11:08:52AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We now have bs-independent connection API, which consists of four
> functions:
> 
>   nbd_client_connection_new()
>   nbd_client_connection_unref()
>   nbd_co_establish_connection()
>   nbd_co_establish_connection_cancel()
> 
> Move them to a separate file together with NBDClientConnection
> structure which becomes private to the new API.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/block/nbd.h     |  11 +++
>  block/nbd.c             | 187 -----------------------------------
>  nbd/client-connection.c | 212 ++++++++++++++++++++++++++++++++++++++++
>  nbd/meson.build         |   1 +

> +++ b/include/block/nbd.h
> @@ -406,4 +406,15 @@ const char *nbd_info_lookup(uint16_t info);
>  const char *nbd_cmd_lookup(uint16_t info);
>  const char *nbd_err_lookup(int err);
>  
> +/* nbd/client-connection.c */
> +typedef struct NBDClientConnection NBDClientConnection;
> +
> +NBDClientConnection *nbd_client_connection_new(const SocketAddress *saddr);
> +void nbd_client_connection_release(NBDClientConnection *conn);
> +
> +QIOChannelSocket *coroutine_fn
> +nbd_co_establish_connection(NBDClientConnection *conn, Error **errp);

To me, the placement of coroutine_fn looks a bit odd here, like it is
the name of a pointer declaration.  But I see that we have precedence
for doing it that way (such as block.c:bdrv_co_enter()); the
difference being that none of the other locations split the return
type on one line and the function name on another.  I don't really
have any changes to suggest, though, so I'm fine keeping it the way
you wrote.

> +++ b/nbd/client-connection.c

There may be fallout in v4 based on what you tweak in the code that
got moved here, but the split to a new file looks sane to me.

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

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