The main point of this series is patch 7, which removes the dubious and
probably wrong use of atomics in block/nbd.c.  This in turn is enabled
mostly by the cleanups in patches 3-5.  Together, they introduce a
QemuMutex that synchronizes the NBD client coroutines, the reconnect_delay
timer and nbd_cancel_in_flight() as well.

The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
by s->send_mutex.

The rest is bugfixes, simplifying the code a bit, and extra documentation.

v1->v2:
- cleaner patch 1
- fix grammar in patch 4
- split out patch 6

Paolo Bonzini (9):
  nbd: safeguard against waking up invalid coroutine
  nbd: mark more coroutine_fns
  nbd: remove peppering of nbd_client_connected
  nbd: keep send_mutex/free_sema handling outside
    nbd_co_do_establish_connection
  nbd: use a QemuMutex to synchronize yanking, reconnection and
    coroutines
  nbd: code motion and function renaming
  nbd: move s->state under requests_lock
  nbd: take receive_mutex when reading requests[].receiving
  nbd: document what is protected by the CoMutexes

 block/coroutines.h |   4 +-
 block/nbd.c        | 298 +++++++++++++++++++++++----------------------
 2 files changed, 154 insertions(+), 148 deletions(-)

-- 
2.35.1

On Thu, 14 Apr 2022 19:57:47 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The main point of this series is patch 7, which removes the dubious and
> probably wrong use of atomics in block/nbd.c.  This in turn is enabled
> mostly by the cleanups in patches 3-5.  Together, they introduce a
> QemuMutex that synchronizes the NBD client coroutines, the reconnect_delay
> timer and nbd_cancel_in_flight() as well.
> 
> The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
> and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
> by s->send_mutex.
> 
> The rest is bugfixes, simplifying the code a bit, and extra documentation.
> 
> v1->v2:
> - cleaner patch 1
> - fix grammar in patch 4
> - split out patch 6
> 
> Paolo Bonzini (9):
>   nbd: safeguard against waking up invalid coroutine
>   nbd: mark more coroutine_fns
>   nbd: remove peppering of nbd_client_connected
>   nbd: keep send_mutex/free_sema handling outside
>     nbd_co_do_establish_connection
>   nbd: use a QemuMutex to synchronize yanking, reconnection and
>     coroutines
>   nbd: code motion and function renaming
>   nbd: move s->state under requests_lock
>   nbd: take receive_mutex when reading requests[].receiving
>   nbd: document what is protected by the CoMutexes
> 
>  block/coroutines.h |   4 +-
>  block/nbd.c        | 298 +++++++++++++++++++++++----------------------
>  2 files changed, 154 insertions(+), 148 deletions(-)
> 

For the whole series:

Reviewed-by: Lukas Straub <lukasstraub2@web.de>

Regards,
Lukas

-- 

On Sat, Apr 16, 2022 at 07:03:57PM +0000, Lukas Straub wrote:
> On Thu, 14 Apr 2022 19:57:47 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > The main point of this series is patch 7, which removes the dubious and
> > probably wrong use of atomics in block/nbd.c.  This in turn is enabled
> > mostly by the cleanups in patches 3-5.  Together, they introduce a
> > QemuMutex that synchronizes the NBD client coroutines, the reconnect_delay
> > timer and nbd_cancel_in_flight() as well.
> > 
> > The fixes happen to remove an incorrect use of qemu_co_queue_restart_all
> > and qemu_co_enter_next on the s->free_sema CoQueue, which was not guarded
> > by s->send_mutex.
> > 
> > The rest is bugfixes, simplifying the code a bit, and extra documentation.

> For the whole series:
> 
> Reviewed-by: Lukas Straub <lukasstraub2@web.de>

I've queued the series through my NBD tree for a pull request in the next week.

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