From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
Emanuele Giuseppe Esposito <eesposit@redhat.com>,
qemu-devel@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>
Subject: [RFC PATCH 5/6] test-bdrv-drain.c: adapt test to the new subtree drains
Date: Mon, 13 Dec 2021 05:40:13 -0500 [thread overview]
Message-ID: <20211213104014.69858-6-eesposit@redhat.com> (raw)
In-Reply-To: <20211213104014.69858-1-eesposit@redhat.com>
There are a couple of problems in this test when we add
subtree drains in bdrv_replace_child_noperm:
- First of all, inconsistency between block_job_create under
aiocontext lock that internally calls blk_insert_bs and therefore
bdrv_replace_child_noperm, and blk_insert_bs that is called two lines
above in the same test without aiocontext. Since we use the
unlocked subtree_drain, we want to move the aiocontext further
down.
- test_detach_by_parent_cb: this test uses a callback of an I/O
function (blk_aio_preadv) to modify the grah, using bdrv_attach_child.
This is simply not allowed anymore. I/O cannot change the graph.
- test_detach_indirect: here it is simply a matter of wrong callbacks
used. In the original test, there was only a subtree drain, so
overriding .drained_begin was not a problem. Now that we have
additional subtree drains, we risk to call the test callback
to early, or multiple times. We do not want that, so override
the callback only when we actually want to use it.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
tests/unit/test-bdrv-drain.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index a62e6451a1..9414fc68b7 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -939,10 +939,10 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
blk_insert_bs(blk_target, target, &error_abort);
blk_set_allow_aio_context_change(blk_target, true);
- aio_context_acquire(ctx);
tjob = block_job_create("job0", &test_job_driver, NULL, src,
0, BLK_PERM_ALL,
0, 0, NULL, NULL, &error_abort);
+ aio_context_acquire(ctx);
job = &tjob->common;
block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, &error_abort);
@@ -1388,8 +1388,6 @@ static void test_detach_indirect(bool by_parent_cb)
if (!by_parent_cb) {
detach_by_driver_cb_class = child_of_bds;
- detach_by_driver_cb_class.drained_begin =
- detach_by_driver_cb_drained_begin;
}
/* Create all involved nodes */
@@ -1447,6 +1445,12 @@ static void test_detach_indirect(bool by_parent_cb)
acb = blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL);
g_assert(acb != NULL);
+ if (!by_parent_cb) {
+ /* set .drained_begin cb to run only in the following drain. */
+ detach_by_driver_cb_class.drained_begin =
+ detach_by_driver_cb_drained_begin;
+ }
+
/* Drain and check the expected result */
bdrv_subtree_drained_begin(parent_b);
@@ -1470,6 +1474,12 @@ static void test_detach_indirect(bool by_parent_cb)
bdrv_subtree_drained_end(parent_b);
+ if (!by_parent_cb) {
+ /* restore .drained_begin cb, we don't need it anymore. */
+ detach_by_driver_cb_class.drained_begin =
+ child_of_bds.drained_begin;
+ }
+
bdrv_unref(parent_b);
blk_unref(blk);
@@ -1483,7 +1493,7 @@ static void test_detach_indirect(bool by_parent_cb)
static void test_detach_by_parent_cb(void)
{
- test_detach_indirect(true);
+ /* test_detach_indirect(true); */
}
static void test_detach_by_driver_cb(void)
--
2.31.1
next prev parent reply other threads:[~2021-12-13 10:44 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-13 10:40 [RFC PATCH 0/6] Removal of Aiocontext lock and usage of subtree drains in aborted transactions Emanuele Giuseppe Esposito
2021-12-13 10:40 ` [RFC PATCH 1/6] tests/unit/test-bdrv-drain.c: graph setup functions can't run in coroutines Emanuele Giuseppe Esposito
2021-12-13 10:40 ` [RFC PATCH 2/6] introduce BDRV_POLL_WHILE_UNLOCKED Emanuele Giuseppe Esposito
2021-12-13 10:40 ` [RFC PATCH 3/6] block/io.c: introduce bdrv_subtree_drained_{begin/end}_unlocked Emanuele Giuseppe Esposito
2021-12-13 10:40 ` [RFC PATCH 4/6] block.c: add subtree_drains where needed Emanuele Giuseppe Esposito
2021-12-13 10:40 ` Emanuele Giuseppe Esposito [this message]
2021-12-13 10:40 ` [RFC PATCH 6/6] block/io.c: enable assert_bdrv_graph_writable Emanuele Giuseppe Esposito
2021-12-13 14:52 ` [RFC PATCH 0/6] Removal of Aiocontext lock and usage of subtree drains in aborted transactions Stefan Hajnoczi
2021-12-14 18:10 ` Emanuele Giuseppe Esposito
2021-12-15 12:34 ` Hanna Reitz
2021-12-16 10:37 ` Kevin Wolf
2021-12-14 16:47 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211213104014.69858-6-eesposit@redhat.com \
--to=eesposit@redhat.com \
--cc=fam@euphon.net \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.