[PATCH v3 00/33] block/nbd: rework client connection

Vladimir Sementsov-Ogievskiy posted 33 patches 3 years, 1 month ago
Test checkpatch passed
Failed in applying to current master (apply log)
There is a newer version of this series
block/coroutines.h                 |   6 +
include/block/aio.h                |   2 +-
include/block/nbd.h                |  19 +
include/io/channel-socket.h        |  20 +
include/qemu/sockets.h             |   2 +-
block/nbd.c                        | 908 +++++++----------------------
io/channel-socket.c                |  17 +-
nbd/client-connection.c            | 364 ++++++++++++
nbd/client.c                       |   2 -
tests/unit/test-util-sockets.c     |  16 +-
util/async.c                       |   8 +
util/qemu-sockets.c                |  10 +-
nbd/meson.build                    |   1 +
scripts/block-coroutine-wrapper.py |   7 +-
14 files changed, 666 insertions(+), 716 deletions(-)
create mode 100644 nbd/client-connection.c
[PATCH v3 00/33] block/nbd: rework client connection
Posted by Vladimir Sementsov-Ogievskiy 3 years, 1 month ago
The series substitutes "[PATCH v2 00/10] block/nbd: move connection code to separate file"
Supersedes: <20210408140827.332915-1-vsementsov@virtuozzo.com>
so it's called v3

block/nbd.c is overcomplicated. These series is a big refactoring, which
finally drops all the complications around drained sections and context
switching, including abuse of bs->in_flight counter.

Also, at the end of the series we don't cancel reconnect on drained
sections (and don't cancel requests waiting for reconnect on drained
section begin), which fixes a problem reported by Roman.

The series is also available at tag up-nbd-client-connection-v3 in
git https://src.openvz.org/scm/~vsementsov/qemu.git 

v3:
Changes in first part of the series (main thing is not using refcnt, but instead (modified) Roman's patch):

01-04: new
05: add Roman's r-b
06: new
07: now, new aio_co_schedule(NULL, thr->wait_co) is used
08: reworked, we now need also bool detached, as we don't have refcnt
09,10: add Roman's r-b
11: rebased, don't modify nbd_free_connect_thread() name at this point
12: add Roman's r-b
13: new
14: rebased

Other patches are new.

Roman Kagan (2):
  block/nbd: fix channel object leak
  block/nbd: ensure ->connection_thread is always valid

Vladimir Sementsov-Ogievskiy (31):
  block/nbd: fix how state is cleared on nbd_open() failure paths
  block/nbd: nbd_client_handshake(): fix leak of s->ioc
  block/nbd: BDRVNBDState: drop unused connect_err and connect_status
  util/async: aio_co_schedule(): support reschedule in same ctx
  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()
  block/nbd: introduce nbd_client_connection_release()
  nbd: move connection code from block/nbd to nbd/client-connection
  nbd/client-connection: use QEMU_LOCK_GUARD
  nbd/client-connection: add possibility of negotiation
  nbd/client-connection: implement connection retry
  nbd/client-connection: shutdown connection on release
  block/nbd: split nbd_handle_updated_info out of nbd_client_handshake()
  block/nbd: use negotiation of NBDClientConnection
  qemu-socket: pass monitor link to socket_get_fd directly
  block/nbd: pass monitor directly to connection thread
  block/nbd: nbd_teardown_connection() don't touch s->sioc
  block/nbd: drop BDRVNBDState::sioc
  nbd/client-connection: return only one io channel
  block-coroutine-wrapper: allow non bdrv_ prefix
  block/nbd: split nbd_co_do_establish_connection out of
    nbd_reconnect_attempt
  nbd/client-connection: do qio_channel_set_delay(false)
  nbd/client-connection: add option for non-blocking connection attempt
  block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()
  block/nbd: add nbd_clinent_connected() helper
  block/nbd: safer transition to receiving request
  block/nbd: drop connection_co

 block/coroutines.h                 |   6 +
 include/block/aio.h                |   2 +-
 include/block/nbd.h                |  19 +
 include/io/channel-socket.h        |  20 +
 include/qemu/sockets.h             |   2 +-
 block/nbd.c                        | 908 +++++++----------------------
 io/channel-socket.c                |  17 +-
 nbd/client-connection.c            | 364 ++++++++++++
 nbd/client.c                       |   2 -
 tests/unit/test-util-sockets.c     |  16 +-
 util/async.c                       |   8 +
 util/qemu-sockets.c                |  10 +-
 nbd/meson.build                    |   1 +
 scripts/block-coroutine-wrapper.py |   7 +-
 14 files changed, 666 insertions(+), 716 deletions(-)
 create mode 100644 nbd/client-connection.c

-- 
2.29.2


Re: [PATCH v3 00/33] block/nbd: rework client connection
Posted by Paolo Bonzini 3 years ago
On 16/04/21 10:08, Vladimir Sementsov-Ogievskiy wrote:
> The series substitutes "[PATCH v2 00/10] block/nbd: move connection code to separate file"
> Supersedes: <20210408140827.332915-1-vsementsov@virtuozzo.com>
> so it's called v3
> 
> block/nbd.c is overcomplicated. These series is a big refactoring, which
> finally drops all the complications around drained sections and context
> switching, including abuse of bs->in_flight counter.
> 
> Also, at the end of the series we don't cancel reconnect on drained
> sections (and don't cancel requests waiting for reconnect on drained
> section begin), which fixes a problem reported by Roman.
> 
> The series is also available at tag up-nbd-client-connection-v3 in
> git https://src.openvz.org/scm/~vsementsov/qemu.git

I have independently done some rework of the connection state machine, 
mostly in order to use the QemuCoSleep API instead of aio_co_wake.  In 
general it seems to be independent of this work.  I'll review this series.

Paolo

> v3:
> Changes in first part of the series (main thing is not using refcnt, but instead (modified) Roman's patch):
> 
> 01-04: new
> 05: add Roman's r-b
> 06: new
> 07: now, new aio_co_schedule(NULL, thr->wait_co) is used
> 08: reworked, we now need also bool detached, as we don't have refcnt
> 09,10: add Roman's r-b
> 11: rebased, don't modify nbd_free_connect_thread() name at this point
> 12: add Roman's r-b
> 13: new
> 14: rebased
> 
> Other patches are new.
> 
> Roman Kagan (2):
>    block/nbd: fix channel object leak
>    block/nbd: ensure ->connection_thread is always valid
> 
> Vladimir Sementsov-Ogievskiy (31):
>    block/nbd: fix how state is cleared on nbd_open() failure paths
>    block/nbd: nbd_client_handshake(): fix leak of s->ioc
>    block/nbd: BDRVNBDState: drop unused connect_err and connect_status
>    util/async: aio_co_schedule(): support reschedule in same ctx
>    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()
>    block/nbd: introduce nbd_client_connection_release()
>    nbd: move connection code from block/nbd to nbd/client-connection
>    nbd/client-connection: use QEMU_LOCK_GUARD
>    nbd/client-connection: add possibility of negotiation
>    nbd/client-connection: implement connection retry
>    nbd/client-connection: shutdown connection on release
>    block/nbd: split nbd_handle_updated_info out of nbd_client_handshake()
>    block/nbd: use negotiation of NBDClientConnection
>    qemu-socket: pass monitor link to socket_get_fd directly
>    block/nbd: pass monitor directly to connection thread
>    block/nbd: nbd_teardown_connection() don't touch s->sioc
>    block/nbd: drop BDRVNBDState::sioc
>    nbd/client-connection: return only one io channel
>    block-coroutine-wrapper: allow non bdrv_ prefix
>    block/nbd: split nbd_co_do_establish_connection out of
>      nbd_reconnect_attempt
>    nbd/client-connection: do qio_channel_set_delay(false)
>    nbd/client-connection: add option for non-blocking connection attempt
>    block/nbd: reuse nbd_co_do_establish_connection() in nbd_open()
>    block/nbd: add nbd_clinent_connected() helper
>    block/nbd: safer transition to receiving request
>    block/nbd: drop connection_co
> 
>   block/coroutines.h                 |   6 +
>   include/block/aio.h                |   2 +-
>   include/block/nbd.h                |  19 +
>   include/io/channel-socket.h        |  20 +
>   include/qemu/sockets.h             |   2 +-
>   block/nbd.c                        | 908 +++++++----------------------
>   io/channel-socket.c                |  17 +-
>   nbd/client-connection.c            | 364 ++++++++++++
>   nbd/client.c                       |   2 -
>   tests/unit/test-util-sockets.c     |  16 +-
>   util/async.c                       |   8 +
>   util/qemu-sockets.c                |  10 +-
>   nbd/meson.build                    |   1 +
>   scripts/block-coroutine-wrapper.py |   7 +-
>   14 files changed, 666 insertions(+), 716 deletions(-)
>   create mode 100644 nbd/client-connection.c
>