[PATCH 00/12] Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm.

[Emanuele Giuseppe Esposito]

This serie aims to ensure necessary protection to the BlockDriverState
.children and .parents lists, modified in block.c:bdrv_replace_child_noperm().
Thanks to the assertion qemu_in_main_thread() introduced in "block layer:
split block APIs in global state and I/O", we can verify that these lists are
always modified under BQL, but are also read by I/O functions.
This means that alone the BQL is not enough, but that we need to add also
drains, to make sure that there is no I/O running in parallel.

Once we know that these fields are thread safe, we can continue doing until
the various Aiocontext lock that are taken and released to protect them can
be removed.
Currently the AioContext is used pretty much throughout the whole block layer,
and it is a little bit confusing to understand what it exactly protects, and I
am starting to think that in some places it is being taken just because of the
block API assumptions.
For example, some functions like AIO_WAIT_WHILE() release the lock with the
assumption that it is always held, so all callers must take it just to allow
the function to release it.

We can assert that some function is under drains and BQL by using
assert_bdrv_graph_writable(), introduced in the block API splitup
(patch 9 in v5), even though it is currently not checking for drains but only
for main loop. That series added the necessary asserts for .parents and
.children, but as explained we could not enable them, because drains
were missing.
This serie adds the necessary drains and patch 11 fully enables the assertion.

The main function that modifies the ->children and ->parent lists is 
bdrv_replace_child_noperm. So we need to protect that, and make sure it is
always under drain.

It is necessary to use subtree drains in this case, because it takes care of
draining the whole graph of the node, while bdrv_drained_* does not cover the
child of the given node. And bdrv_replace_child_noperm modifies also that.

Major improvements that this serie brings:
* BDRV_POLL_WHILE_UNLOCKED and bdrv_subtree_drained_begin/end_unlocked.
  This is helpful because current drains always assume that the AioContext
  lock is taken, and thus release it. We don't want to use Aiocontext at all.
* Fix/improve tests. Some tests perform what are now defined as illegal
  operations (I/O modifying a graph) or fail due to the increase of drains
  due to these patches. This brings possible bugs to the light, as shown in
  patches 2,3,4,5,6,8,9. Most of the patches try to fix all bugs that come
  out from adding such drains.
* Protect BlockDriverState's .children and .parents lists, making them thread
  safe.
* Finally fully enable assert_bdrv_graph_writable(), checking also for the
  drains, in patch 11.

This series is based on "job: replace AioContext lock with job_mutex" that in
turns is based on the block API split ("block layer: split block APIs in
global state and I/O").

Based-on: <20220105140208.365608-1-eesposit@redhat.com>

Emanuele Giuseppe Esposito (12):
  introduce BDRV_POLL_WHILE_UNLOCKED
  block/io.c: make bdrv_do_drained_begin_quiesce static and introduce
    bdrv_drained_begin_no_poll
  block.c: bdrv_replace_child_noperm: first remove the child, and then
    call ->detach()
  block.c: bdrv_replace_child_noperm: first call ->attach(), and then
    add child
  test-bdrv-drain.c: adapt test to the coming subtree drains
  test-bdrv-drain.c: remove test_detach_by_parent_cb()
  block/io.c: introduce bdrv_subtree_drained_{begin/end}_unlocked
  reopen: add a transaction to drain_end nodes picked in
    bdrv_reopen_parse_file_or_backing
  jobs: ensure sleep in job_sleep_ns is fully performed
  block.c: add subtree_drains where needed
  block/io.c: fully enable assert_bdrv_graph_writable
  block.c: additional assert qemu in main tread

 block.c                            | 95 ++++++++++++++++++++++++------
 block/block-backend.c              |  3 +
 block/io.c                         | 62 +++++++++++++------
 include/block/block-global-state.h |  5 ++
 include/block/block-io.h           | 10 +++-
 job.c                              | 28 +++++----
 tests/qemu-iotests/030             |  2 +-
 tests/qemu-iotests/151             |  4 +-
 tests/unit/test-bdrv-drain.c       | 53 ++++++-----------
 tests/unit/test-blockjob.c         |  2 +-
 10 files changed, 174 insertions(+), 90 deletions(-)

-- 
2.31.1