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 <vsement...@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 <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org