Re: [PATCH v2 00/10] block/nbd: move connection code to separate file

[Roman Kagan]

On Thu, Apr 08, 2021 at 05:08:17PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> This substitutes "[PATCH 00/14] nbd: move reconnect-thread to separate file"
> Supersedes: <20210407104637.36033-1-vsementsov@virtuozzo.com>
> 
> I want to simplify block/nbd.c which is overcomplicated now. First step
> is splitting out what could be split.
> 
> These series creates new file nbd/client-connection.c and part of
> block/nbd.c is refactored and moved.
> 
> v2 is mostly rewritten. I decided move larger part, otherwise it doesn't
> make real sense.
> 
> Note also that v2 is based on master. Patch 01 actually solves same
> problem as
> "[PATCH for-6.0] block/nbd: fix possible use after free of s->connect_thread" 
> [*]
> in a smarter way. So, if [*] goes first, this will be rebased to undo
> [*].
> 
> Vladimir Sementsov-Ogievskiy (10):
>   block/nbd: introduce NBDConnectThread reference counter
>   block/nbd: BDRVNBDState: drop unused connect_err and connect_status
>   util/async: aio_co_enter(): do aio_co_schedule in general case
>   block/nbd: simplify waking of nbd_co_establish_connection()
>   block/nbd: drop thr->state
>   block/nbd: bs-independent interface for nbd_co_establish_connection()
>   block/nbd: make nbd_co_establish_connection_cancel() bs-independent
>   block/nbd: rename NBDConnectThread to NBDClientConnection
>   block/nbd: introduce nbd_client_connection_new()
>   nbd: move connection code from block/nbd to nbd/client-connection
> 
>  include/block/nbd.h     |  11 ++
>  block/nbd.c             | 288 ++--------------------------------------
>  nbd/client-connection.c | 192 +++++++++++++++++++++++++++
>  util/async.c            |  11 +-
>  nbd/meson.build         |   1 +
>  5 files changed, 218 insertions(+), 285 deletions(-)
>  create mode 100644 nbd/client-connection.c

I think this is a nice cleanup overall, and makes the logic in
block/nbd.c much easier to reason about.

I guess it's 6.1 material though, as it looks somewhat too big for 6.0,
and the only serious bug it actually fixes can be addressed with a
band-aid mentioned above.

The problem I originally came across with, that of the requests being
canceled on drain despite reconnect, still remains, but I think the fix
for it should build up on this series (and thus probably wait till after
6.0).

Thanks,
Roman.