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 <kw...@redhat.com>
> ---
> 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
