From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
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>, John Snow <jsnow@redhat.com>
Subject: [PATCH v2 08/10] tests/unit/test-bdrv-drain.c: graph setup functions can't run in coroutines
Date: Mon, 14 Mar 2022 09:18:52 -0400 [thread overview]
Message-ID: <20220314131854.2202651-9-eesposit@redhat.com> (raw)
In-Reply-To: <20220314131854.2202651-1-eesposit@redhat.com>
Graph initialization functions like blk_new(), bdrv_new() and so on
should not run in a coroutine. In fact, they might invoke a drain
(for example blk_insert_bs eventually calls bdrv_replace_child_noperm)
that in turn can invoke callbacks like bdrv_do_drained_begin_quiesce(),
that asserts exactly that we are not in a coroutine.
Move the initialization phase of test_drv_cb and test_quiesce_common
outside the coroutine logic.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-Id: <20211213104014.69858-2-eesposit@redhat.com>
---
tests/unit/test-bdrv-drain.c | 118 ++++++++++++++++++++++-------------
1 file changed, 73 insertions(+), 45 deletions(-)
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index fa0d86209a..a3bc19965d 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -116,7 +116,8 @@ static void aio_ret_cb(void *opaque, int ret)
}
typedef struct CallInCoroutineData {
- void (*entry)(void);
+ void (*entry)(void *);
+ void *arg;
bool done;
} CallInCoroutineData;
@@ -124,15 +125,16 @@ static coroutine_fn void call_in_coroutine_entry(void *opaque)
{
CallInCoroutineData *data = opaque;
- data->entry();
+ data->entry(data->arg);
data->done = true;
}
-static void call_in_coroutine(void (*entry)(void))
+static void call_in_coroutine(void (*entry)(void *), void *arg)
{
Coroutine *co;
CallInCoroutineData data = {
.entry = entry,
+ .arg = arg,
.done = false,
};
@@ -192,26 +194,28 @@ static void do_drain_end_unlocked(enum drain_type drain_type, BlockDriverState *
}
}
-static void test_drv_cb_common(enum drain_type drain_type, bool recursive)
-{
+typedef struct TestDriverCBData {
+ enum drain_type drain_type;
+ bool recursive;
BlockBackend *blk;
BlockDriverState *bs, *backing;
- BDRVTestState *s, *backing_s;
+} TestDriverCBData;
+
+static void test_drv_cb_common(void *arg)
+{
+ TestDriverCBData *data = arg;
+ BlockBackend *blk = data->blk;
+ BlockDriverState *bs = data->bs;
+ BlockDriverState *backing = data->backing;
+ enum drain_type drain_type = data->drain_type;
+ bool recursive = data->recursive;
+ BDRVTestState *s = bs->opaque;
+ BDRVTestState *backing_s = backing->opaque;
BlockAIOCB *acb;
int aio_ret;
QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0);
- blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
- bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
- &error_abort);
- s = bs->opaque;
- blk_insert_bs(blk, bs, &error_abort);
-
- backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
- backing_s = backing->opaque;
- bdrv_set_backing_hd(bs, backing, &error_abort);
-
/* Simple bdrv_drain_all_begin/end pair, check that CBs are called */
g_assert_cmpint(s->drain_count, ==, 0);
g_assert_cmpint(backing_s->drain_count, ==, 0);
@@ -245,54 +249,77 @@ static void test_drv_cb_common(enum drain_type drain_type, bool recursive)
g_assert_cmpint(s->drain_count, ==, 0);
g_assert_cmpint(backing_s->drain_count, ==, 0);
+}
- bdrv_unref(backing);
- bdrv_unref(bs);
- blk_unref(blk);
+static void test_common_cb(enum drain_type drain_type, bool in_coroutine,
+ void (*cb)(void *))
+{
+ TestDriverCBData data;
+
+ data.drain_type = drain_type;
+ data.recursive = (drain_type != BDRV_DRAIN);
+
+ data.blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
+ data.bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
+ &error_abort);
+ blk_insert_bs(data.blk, data.bs, &error_abort);
+
+ data.backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
+ bdrv_set_backing_hd(data.bs, data.backing, &error_abort);
+
+ if (in_coroutine) {
+ call_in_coroutine(cb, &data);
+ } else {
+ cb(&data);
+ }
+
+ bdrv_unref(data.backing);
+ bdrv_unref(data.bs);
+ blk_unref(data.blk);
+}
+
+static void test_drv_cb(enum drain_type drain_type, bool in_coroutine)
+{
+ test_common_cb(drain_type, in_coroutine, test_drv_cb_common);
}
static void test_drv_cb_drain_all(void)
{
- test_drv_cb_common(BDRV_DRAIN_ALL, true);
+ test_drv_cb(BDRV_DRAIN_ALL, false);
}
static void test_drv_cb_drain(void)
{
- test_drv_cb_common(BDRV_DRAIN, false);
+ test_drv_cb(BDRV_DRAIN, false);
}
static void test_drv_cb_drain_subtree(void)
{
- test_drv_cb_common(BDRV_SUBTREE_DRAIN, true);
+ test_drv_cb(BDRV_SUBTREE_DRAIN, false);
}
static void test_drv_cb_co_drain_all(void)
{
- call_in_coroutine(test_drv_cb_drain_all);
+ test_drv_cb(BDRV_DRAIN_ALL, true);
}
static void test_drv_cb_co_drain(void)
{
- call_in_coroutine(test_drv_cb_drain);
+ test_drv_cb(BDRV_DRAIN, true);
}
static void test_drv_cb_co_drain_subtree(void)
{
- call_in_coroutine(test_drv_cb_drain_subtree);
+ test_drv_cb(BDRV_SUBTREE_DRAIN, true);
}
-static void test_quiesce_common(enum drain_type drain_type, bool recursive)
+static void test_quiesce_common(void *arg)
{
- BlockBackend *blk;
- BlockDriverState *bs, *backing;
-
- blk = blk_new(qemu_get_aio_context(), BLK_PERM_ALL, BLK_PERM_ALL);
- bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
- &error_abort);
- blk_insert_bs(blk, bs, &error_abort);
-
- backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
- bdrv_set_backing_hd(bs, backing, &error_abort);
+ TestDriverCBData *data = arg;
+ BlockDriverState *bs = data->bs;
+ BlockDriverState *backing = data->backing;
+ enum drain_type drain_type = data->drain_type;
+ bool recursive = data->recursive;
g_assert_cmpint(bs->quiesce_counter, ==, 0);
g_assert_cmpint(backing->quiesce_counter, ==, 0);
@@ -306,40 +333,41 @@ static void test_quiesce_common(enum drain_type drain_type, bool recursive)
g_assert_cmpint(bs->quiesce_counter, ==, 0);
g_assert_cmpint(backing->quiesce_counter, ==, 0);
+}
- bdrv_unref(backing);
- bdrv_unref(bs);
- blk_unref(blk);
+static void test_quiesce(enum drain_type drain_type, bool in_coroutine)
+{
+ test_common_cb(drain_type, in_coroutine, test_quiesce_common);
}
static void test_quiesce_drain_all(void)
{
- test_quiesce_common(BDRV_DRAIN_ALL, true);
+ test_quiesce(BDRV_DRAIN_ALL, false);
}
static void test_quiesce_drain(void)
{
- test_quiesce_common(BDRV_DRAIN, false);
+ test_quiesce(BDRV_DRAIN, false);
}
static void test_quiesce_drain_subtree(void)
{
- test_quiesce_common(BDRV_SUBTREE_DRAIN, true);
+ test_quiesce(BDRV_SUBTREE_DRAIN, false);
}
static void test_quiesce_co_drain_all(void)
{
- call_in_coroutine(test_quiesce_drain_all);
+ test_quiesce(BDRV_DRAIN_ALL, true);
}
static void test_quiesce_co_drain(void)
{
- call_in_coroutine(test_quiesce_drain);
+ test_quiesce(BDRV_DRAIN, true);
}
static void test_quiesce_co_drain_subtree(void)
{
- call_in_coroutine(test_quiesce_drain_subtree);
+ test_quiesce(BDRV_SUBTREE_DRAIN, true);
}
static void test_nested(void)
--
2.31.1
next prev parent reply other threads:[~2022-03-14 13:59 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-14 13:18 [PATCH v2 00/10] block: bug fixes in preparation of AioContext removal Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 01/10] drains: create bh only when polling Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 02/10] bdrv_parent_drained_begin_single: handle calls from coroutine context Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 03/10] block/io.c: fix bdrv_child_cb_drained_begin invocations from a coroutine Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 04/10] block.c: bdrv_replace_child_noperm: first remove the child, and then call ->detach() Emanuele Giuseppe Esposito
2022-03-16 9:13 ` Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 05/10] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child Emanuele Giuseppe Esposito
2022-03-16 9:16 ` Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 06/10] test-bdrv-drain.c: adapt test to support additional subtree drains Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 07/10] test-bdrv-drain.c: remove test_detach_by_parent_cb() Emanuele Giuseppe Esposito
2022-03-14 13:18 ` Emanuele Giuseppe Esposito [this message]
2022-03-14 13:18 ` [PATCH v2 09/10] child_job_drained_poll: override polling condition only when in home thread Emanuele Giuseppe Esposito
2022-03-14 13:18 ` [PATCH v2 10/10] tests/qemu-iotests/030: test_stream_parallel should use auto_finalize=False Emanuele Giuseppe Esposito
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=20220314131854.2202651-9-eesposit@redhat.com \
--to=eesposit@redhat.com \
--cc=fam@euphon.net \
--cc=hreitz@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@virtuozzo.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).