This is the third and hopefully for now last part of my work to fix
drain. The main goal of this series is to make drain robust against
graph changes that happen in any callbacks of in-flight requests while
we drain a block node.

The individual patches describe the details, but the rough plan is to
change all three drain types (single node, subtree and all) to work like
this:

1. First call all the necessary callbacks to quiesce external sources
   for new requests. This includes the block driver callbacks, the child
   node callbacks and disabling external AioContext events. This is done
   recursively.

   Much of the trouble we had with drain resulted from the fact that the
   graph changed while we were traversing the graph recursively. None of
   the callbacks called in this phase may change the graph.

2. Then do a single AIO_WAIT_WHILE() to drain the requests of all
   affected nodes. The aio_poll() called by it is where graph changes
   can happen and we need to be careful.

   However, while evaluating the loop condition, the graph can't change,
   so we can safely call all necessary callbacks, if needed recursively,
   to determine whether there are still pending requests in any affected
   nodes. We just need to make sure that we don't rely on the set of
   nodes being the same between any two evaluation of the condition.

There are a few more smaller, mostly self-contained changes needed
before we're actually safe, but this is the main mechanism that will
help you understand what we're working towards during the series.

Kevin Wolf (18):
  test-bdrv-drain: bdrv_drain() works with cross-AioContext events
  block: Use bdrv_do_drain_begin/end in bdrv_drain_all()
  block: Remove 'recursive' parameter from bdrv_drain_invoke()
  block: Don't manually poll in bdrv_drain_all()
  tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now
  block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE()
  block: Really pause block jobs on drain
  block: Remove bdrv_drain_recurse()
  block: Drain recursively with a single BDRV_POLL_WHILE()
  test-bdrv-drain: Test node deletion in subtree recursion
  block: Don't poll in parent drain callbacks
  test-bdrv-drain: Graph change through parent callback
  block: Defer .bdrv_drain_begin callback to polling phase
  test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll
  block: Allow AIO_WAIT_WHILE with NULL ctx
  block: Move bdrv_drain_all_begin() out of coroutine context
  block: Allow graph changes in bdrv_drain_all_begin/end sections
  test-bdrv-drain: Test graph changes in drain_all section

Max Reitz (1):
  test-bdrv-drain: Add test for node deletion

 include/block/aio-wait.h     |  25 +-
 include/block/block.h        |  17 +
 include/block/block_int.h    |   8 +
 include/block/blockjob_int.h |   8 +
 block.c                      |  31 +-
 block/io.c                   | 280 +++++++++-------
 block/mirror.c               |   8 +
 blockjob.c                   |  21 ++
 tests/test-bdrv-drain.c      | 738 ++++++++++++++++++++++++++++++++++++++++---
 9 files changed, 974 insertions(+), 162 deletions(-)

-- 
2.13.6

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180411163940.2523-1-kwolf@redhat.com
Subject: [Qemu-devel] [PATCH 00/19] Drain fixes and cleanups, part 3

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   675608cb84..6523eaca37  master     -> master
 t [tag update]            patchew/1523089594-1422-1-git-send-email-lidongchen@tencent.com -> patchew/1523089594-1422-1-git-send-email-lidongchen@tencent.com
 t [tag update]            patchew/1523377186-32578-1-git-send-email-cota@braap.org -> patchew/1523377186-32578-1-git-send-email-cota@braap.org
 t [tag update]            patchew/20180410134203.17552-1-peter.maydell@linaro.org -> patchew/20180410134203.17552-1-peter.maydell@linaro.org
 t [tag update]            patchew/20180410193919.28026-1-alex.bennee@linaro.org -> patchew/20180410193919.28026-1-alex.bennee@linaro.org
 t [tag update]            patchew/20180411122606.367301-1-vsementsov@virtuozzo.com -> patchew/20180411122606.367301-1-vsementsov@virtuozzo.com
 * [new tag]               patchew/20180411163940.2523-1-kwolf@redhat.com -> patchew/20180411163940.2523-1-kwolf@redhat.com
Switched to a new branch 'test'
4b7690f2d7 test-bdrv-drain: Test graph changes in drain_all section
c3b712e854 block: Allow graph changes in bdrv_drain_all_begin/end sections
b42a7e0a7d block: Move bdrv_drain_all_begin() out of coroutine context
74bb69f37c block: Allow AIO_WAIT_WHILE with NULL ctx
a6e790e0bc test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll
26fc9f7a2f block: Defer .bdrv_drain_begin callback to polling phase
5de06df1ac test-bdrv-drain: Graph change through parent callback
13fb2f568b block: Don't poll in parent drain callbacks
48cfd9a68a test-bdrv-drain: Test node deletion in subtree recursion
81174751a0 block: Drain recursively with a single BDRV_POLL_WHILE()
1f4daf1742 test-bdrv-drain: Add test for node deletion
df4213f29a block: Remove bdrv_drain_recurse()
5bddf60629 block: Really pause block jobs on drain
aed8d29900 block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE()
6479633b40 tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now
b02f2e5912 block: Don't manually poll in bdrv_drain_all()
c3fc61add1 block: Remove 'recursive' parameter from bdrv_drain_invoke()
f33873949d block: Use bdrv_do_drain_begin/end in bdrv_drain_all()
9edb04df89 test-bdrv-drain: bdrv_drain() works with cross-AioContext events

=== OUTPUT BEGIN ===
Checking PATCH 1/19: test-bdrv-drain: bdrv_drain() works with cross-AioContext events...
Checking PATCH 2/19: block: Use bdrv_do_drain_begin/end in bdrv_drain_all()...
Checking PATCH 3/19: block: Remove 'recursive' parameter from bdrv_drain_invoke()...
Checking PATCH 4/19: block: Don't manually poll in bdrv_drain_all()...
Checking PATCH 5/19: tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now...
Checking PATCH 6/19: block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE()...
ERROR: trailing statements should be on next line
#37: FILE: block/io.c:189:
+    while (aio_poll(bs->aio_context, false));

ERROR: braces {} are necessary for all arms of this statement
#37: FILE: block/io.c:189:
+    while (aio_poll(bs->aio_context, false));
[...]

total: 2 errors, 0 warnings, 60 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 7/19: block: Really pause block jobs on drain...
ERROR: trailing statements should be on next line
#98: FILE: block/io.c:204:
+        while (aio_poll(bs->aio_context, false));

ERROR: braces {} are necessary for all arms of this statement
#98: FILE: block/io.c:204:
+        while (aio_poll(bs->aio_context, false));
[...]

total: 2 errors, 0 warnings, 234 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 8/19: block: Remove bdrv_drain_recurse()...
Checking PATCH 9/19: test-bdrv-drain: Add test for node deletion...
Checking PATCH 10/19: block: Drain recursively with a single BDRV_POLL_WHILE()...
Checking PATCH 11/19: test-bdrv-drain: Test node deletion in subtree recursion...
WARNING: line over 80 characters
#85: FILE: tests/test-bdrv-drain.c:1029:
+    g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_drain_subtree);

total: 0 errors, 1 warnings, 68 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 12/19: block: Don't poll in parent drain callbacks...
Checking PATCH 13/19: test-bdrv-drain: Graph change through parent callback...
WARNING: line over 80 characters
#81: FILE: tests/test-bdrv-drain.c:1044:
+    child_a = bdrv_attach_child(parent_b, a, "PB-A", &child_backing, &error_abort);

total: 0 errors, 1 warnings, 142 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 14/19: block: Defer .bdrv_drain_begin callback to polling phase...
Checking PATCH 15/19: test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll...
Checking PATCH 16/19: block: Allow AIO_WAIT_WHILE with NULL ctx...
Checking PATCH 17/19: block: Move bdrv_drain_all_begin() out of coroutine context...
WARNING: line over 80 characters
#27: FILE: block/io.c:261:
+            bdrv_do_drained_begin(bs, data->recursive, data->parent, data->poll);

total: 0 errors, 1 warnings, 41 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 18/19: block: Allow graph changes in bdrv_drain_all_begin/end sections...
ERROR: do not initialise globals to 0 or NULL
#105: FILE: block/io.c:450:
+unsigned int bdrv_drain_all_count = 0;

ERROR: trailing statements should be on next line
#114: FILE: block/io.c:459:
+    while (aio_poll(qemu_get_aio_context(), false));

ERROR: braces {} are necessary for all arms of this statement
#114: FILE: block/io.c:459:
+    while (aio_poll(qemu_get_aio_context(), false));
[...]

total: 3 errors, 0 warnings, 274 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 19/19: test-bdrv-drain: Test graph changes in drain_all section...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On Wed, Apr 11, 2018 at 06:39:21PM +0200, Kevin Wolf wrote:
> This is the third and hopefully for now last part of my work to fix
> drain. The main goal of this series is to make drain robust against
> graph changes that happen in any callbacks of in-flight requests while
> we drain a block node.

I have reviewed the first half of this series and will review the rest
when v2 is posted since I want to see how the changes suggested by Paolo
play out first.

Stefan