The first few patches in this series fix coroutine correctness problems
that have existed for a while, but only actually start to make things
fail in practice with stricter checks that we're going to introduce with
the graph locking work.

The rest of the series makes sure that the graph lock is held in more
block driver functions that access the children or parents list of a
block node. This is incremental work and more patches are yet to come.

v2:
- Rebased on current git master
- Improved some commit messages
- Patch 5: Added missing coroutine_fn annotations in test
- Patch 7: Updated comments, too

I didn't remove the assertion in patch 13 yet which Stefan was
questioning. I can remove it while applying if we decide that we really
don't want it.

Emanuele Giuseppe Esposito (5):
  nbd: Mark nbd_co_do_establish_connection() and callers GRAPH_RDLOCK
  block: Mark bdrv_co_get_allocated_file_size() and callers GRAPH_RDLOCK
  block: Mark bdrv_co_get_info() and callers GRAPH_RDLOCK
  block: Mark bdrv_co_debug_event() GRAPH_RDLOCK
  block: Mark BlockDriver callbacks for amend job GRAPH_RDLOCK

Kevin Wolf (15):
  qcow2: Don't call bdrv_getlength() in coroutine_fns
  block: Consistently call bdrv_activate() outside coroutine
  block: bdrv/blk_co_unref() for calls in coroutine context
  block: Don't call no_coroutine_fns in qmp_block_resize()
  test-bdrv-drain: Don't modify the graph in coroutines
  graph-lock: Add GRAPH_UNLOCKED(_PTR)
  graph-lock: Fix GRAPH_RDLOCK_GUARD*() to be reader lock
  block: .bdrv_open is non-coroutine and unlocked
  nbd: Remove nbd_co_flush() wrapper function
  vhdx: Require GRAPH_RDLOCK for accessing a node's parent list
  mirror: Require GRAPH_RDLOCK for accessing a node's parent list
  block: Mark bdrv_query_bds_stats() and callers GRAPH_RDLOCK
  block: Mark bdrv_query_block_graph_info() and callers GRAPH_RDLOCK
  block: Mark bdrv_recurse_can_replace() and callers GRAPH_RDLOCK
  block: Mark bdrv_refresh_limits() and callers GRAPH_RDLOCK

 block/coroutines.h                          |   5 +-
 block/qcow2.h                               |   4 +-
 include/block/block-global-state.h          |  19 +++-
 include/block/block-io.h                    |  23 ++--
 include/block/block_int-common.h            |  37 +++----
 include/block/block_int-global-state.h      |   4 +-
 include/block/graph-lock.h                  |  18 ++--
 include/block/qapi.h                        |   7 +-
 include/sysemu/block-backend-global-state.h |   5 +-
 block.c                                     |  25 ++++-
 block/amend.c                               |   8 +-
 block/blkverify.c                           |   5 +-
 block/block-backend.c                       |  10 +-
 block/crypto.c                              |   8 +-
 block/io.c                                  |  12 +--
 block/mirror.c                              |  14 ++-
 block/nbd.c                                 |  50 +++++----
 block/parallels.c                           |   6 +-
 block/qapi.c                                |   6 +-
 block/qcow.c                                |   6 +-
 block/qcow2-refcount.c                      |   2 +-
 block/qcow2.c                               |  48 ++++-----
 block/qed.c                                 |  24 ++---
 block/quorum.c                              |   4 +-
 block/raw-format.c                          |   2 +-
 block/vdi.c                                 |   6 +-
 block/vhdx.c                                |  15 +--
 block/vmdk.c                                |  20 ++--
 block/vpc.c                                 |   6 +-
 blockdev.c                                  |   7 +-
 qemu-img.c                                  |   2 +
 tests/unit/test-bdrv-drain.c                | 112 +++++++++++++-------
 32 files changed, 310 insertions(+), 210 deletions(-)

-- 
2.40.1

Am 04.05.2023 um 13:57 hat Kevin Wolf geschrieben:
> The first few patches in this series fix coroutine correctness problems
> that have existed for a while, but only actually start to make things
> fail in practice with stricter checks that we're going to introduce with
> the graph locking work.
> 
> The rest of the series makes sure that the graph lock is held in more
> block driver functions that access the children or parents list of a
> block node. This is incremental work and more patches are yet to come.
> 
> v2:
> - Rebased on current git master
> - Improved some commit messages
> - Patch 5: Added missing coroutine_fn annotations in test
> - Patch 7: Updated comments, too

Thanks for the review, applied to the block branch.

Kevin

On Thu, May 04, 2023 at 01:57:30PM +0200, Kevin Wolf wrote:
> The first few patches in this series fix coroutine correctness problems
> that have existed for a while, but only actually start to make things
> fail in practice with stricter checks that we're going to introduce with
> the graph locking work.
> 
> The rest of the series makes sure that the graph lock is held in more
> block driver functions that access the children or parents list of a
> block node. This is incremental work and more patches are yet to come.
> 
> v2:
> - Rebased on current git master
> - Improved some commit messages
> - Patch 5: Added missing coroutine_fn annotations in test
> - Patch 7: Updated comments, too
> 
> I didn't remove the assertion in patch 13 yet which Stefan was
> questioning. I can remove it while applying if we decide that we really
> don't want it.

That's fine, we can leave them for now.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>