On Tue, Apr 25, 2023 at 07:31:43PM +0200, Kevin Wolf wrote: > test-bdrv-drain contains a few test cases that are run both in coroutine > and non-coroutine context. Running the entire code including the setup > and shutdown in coroutines is incorrect because graph modifications can > generally not happen in coroutines. > > Change the test so that creating and destroying the test nodes and > BlockBackends always happens outside of coroutine context. > > Signed-off-by: Kevin Wolf <[email protected]> > --- > tests/unit/test-bdrv-drain.c | 112 +++++++++++++++++++++++------------ > 1 file changed, 75 insertions(+), 37 deletions(-) > > diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c > index d9d3807062..765ae382da 100644 > --- a/tests/unit/test-bdrv-drain.c > +++ b/tests/unit/test-bdrv-drain.c > @@ -188,6 +188,25 @@ static void do_drain_begin_unlocked(enum drain_type > drain_type, BlockDriverState > } > } > > +static BlockBackend * no_coroutine_fn test_setup(void) > +{ > + 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); > + > + bdrv_unref(backing); > + bdrv_unref(bs); > + > + return blk; > +} > + > static void do_drain_end_unlocked(enum drain_type drain_type, > BlockDriverState *bs) > { > if (drain_type != BDRV_DRAIN_ALL) { > @@ -199,25 +218,19 @@ 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) > +static void test_drv_cb_common(BlockBackend *blk, enum drain_type drain_type, > + bool recursive) > { > - BlockBackend *blk; > - BlockDriverState *bs, *backing; > + BlockDriverState *bs = blk_bs(blk); > + BlockDriverState *backing = bs->backing->bs; > BDRVTestState *s, *backing_s; > 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); > @@ -252,44 +265,53 @@ 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_drv_cb_drain_all(void) > { > - test_drv_cb_common(BDRV_DRAIN_ALL, true); > + BlockBackend *blk = test_setup(); > + test_drv_cb_common(blk, BDRV_DRAIN_ALL, true); > + blk_unref(blk); > } > > static void test_drv_cb_drain(void) > { > - test_drv_cb_common(BDRV_DRAIN, false); > + BlockBackend *blk = test_setup(); > + test_drv_cb_common(blk, BDRV_DRAIN, false); > + blk_unref(blk); > +} > + > +static void test_drv_cb_co_drain_all_entry(void)
Missing coroutine_fn.
> +{
> + BlockBackend *blk = blk_all_next(NULL);
> + test_drv_cb_common(blk, BDRV_DRAIN_ALL, true);
> }
>
> static void test_drv_cb_co_drain_all(void)
> {
> - call_in_coroutine(test_drv_cb_drain_all);
> + BlockBackend *blk = test_setup();
> + call_in_coroutine(test_drv_cb_co_drain_all_entry);
> + blk_unref(blk);
> }
>
> -static void test_drv_cb_co_drain(void)
> +static void test_drv_cb_co_drain_entry(void)
Missing coroutine_fn.
> {
> - call_in_coroutine(test_drv_cb_drain);
> + BlockBackend *blk = blk_all_next(NULL);
> + test_drv_cb_common(blk, BDRV_DRAIN, false);
> }
>
> -static void test_quiesce_common(enum drain_type drain_type, bool recursive)
> +static void test_drv_cb_co_drain(void)
> {
> - 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);
> + BlockBackend *blk = test_setup();
> + call_in_coroutine(test_drv_cb_co_drain_entry);
> + blk_unref(blk);
> +}
>
> - backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
> - bdrv_set_backing_hd(bs, backing, &error_abort);
> +static void test_quiesce_common(BlockBackend *blk, enum drain_type
> drain_type,
> + bool recursive)
> +{
> + BlockDriverState *bs = blk_bs(blk);
> + BlockDriverState *backing = bs->backing->bs;
>
> g_assert_cmpint(bs->quiesce_counter, ==, 0);
> g_assert_cmpint(backing->quiesce_counter, ==, 0);
> @@ -307,30 +329,46 @@ 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_drain_all(void)
> {
> - test_quiesce_common(BDRV_DRAIN_ALL, true);
> + BlockBackend *blk = test_setup();
> + test_quiesce_common(blk, BDRV_DRAIN_ALL, true);
> + blk_unref(blk);
> }
>
> static void test_quiesce_drain(void)
> {
> - test_quiesce_common(BDRV_DRAIN, false);
> + BlockBackend *blk = test_setup();
> + test_quiesce_common(blk, BDRV_DRAIN, false);
> + blk_unref(blk);
> +}
> +
> +static void test_quiesce_co_drain_all_entry(void)
Missing coroutine_fn.
> +{
> + BlockBackend *blk = blk_all_next(NULL);
> + test_quiesce_common(blk, BDRV_DRAIN_ALL, true);
> }
>
> static void test_quiesce_co_drain_all(void)
> {
> - call_in_coroutine(test_quiesce_drain_all);
> + BlockBackend *blk = test_setup();
> + call_in_coroutine(test_quiesce_co_drain_all_entry);
> + blk_unref(blk);
> +}
> +
> +static void test_quiesce_co_drain_entry(void)
Missing coroutine_fn.
> +{
> + BlockBackend *blk = blk_all_next(NULL);
> + test_quiesce_common(blk, BDRV_DRAIN, false);
> }
>
> static void test_quiesce_co_drain(void)
> {
> - call_in_coroutine(test_quiesce_drain);
> + BlockBackend *blk = test_setup();
> + call_in_coroutine(test_quiesce_co_drain_entry);
> + blk_unref(blk);
> }
>
> static void test_nested(void)
> --
> 2.40.0
>
signature.asc
Description: PGP signature
