From nobody Wed Dec 17 21:47:01 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1529341366540346.8710263012101; Mon, 18 Jun 2018 10:02:46 -0700 (PDT) Received: from localhost ([::1]:36039 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fUxYD-0000pf-M3 for importer@patchew.org; Mon, 18 Jun 2018 13:02:45 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52006) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fUxHV-0004yO-Ep for qemu-devel@nongnu.org; Mon, 18 Jun 2018 12:45:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fUxHT-0006Kk-Ia for qemu-devel@nongnu.org; Mon, 18 Jun 2018 12:45:29 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58982 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fUxHP-0006H1-8P; Mon, 18 Jun 2018 12:45:23 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C2613406E1D2; Mon, 18 Jun 2018 16:45:22 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-120.ams2.redhat.com [10.36.117.120]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1CD272026D68; Mon, 18 Jun 2018 16:45:21 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Mon, 18 Jun 2018 18:44:44 +0200 Message-Id: <20180618164504.24488-16-kwolf@redhat.com> In-Reply-To: <20180618164504.24488-1-kwolf@redhat.com> References: <20180618164504.24488-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 18 Jun 2018 16:45:22 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Mon, 18 Jun 2018 16:45:22 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PULL 15/35] test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" This adds a test case that goes wrong if bdrv_drain_invoke() calls aio_poll(). Signed-off-by: Kevin Wolf --- tests/test-bdrv-drain.c | 102 +++++++++++++++++++++++++++++++++++++++++---= ---- 1 file changed, 88 insertions(+), 14 deletions(-) diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c index abb287e597..5cc179a778 100644 --- a/tests/test-bdrv-drain.c +++ b/tests/test-bdrv-drain.c @@ -34,12 +34,16 @@ static QemuEvent done_event; typedef struct BDRVTestState { int drain_count; AioContext *bh_indirection_ctx; + bool sleep_in_drain_begin; } BDRVTestState; =20 static void coroutine_fn bdrv_test_co_drain_begin(BlockDriverState *bs) { BDRVTestState *s =3D bs->opaque; s->drain_count++; + if (s->sleep_in_drain_begin) { + qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 100000); + } } =20 static void coroutine_fn bdrv_test_co_drain_end(BlockDriverState *bs) @@ -80,6 +84,22 @@ static int coroutine_fn bdrv_test_co_preadv(BlockDriverS= tate *bs, return 0; } =20 +static void bdrv_test_child_perm(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + BlockReopenQueue *reopen_queue, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + /* bdrv_format_default_perms() accepts only these two, so disguise + * detach_by_driver_cb_role as one of them. */ + if (role !=3D &child_file && role !=3D &child_backing) { + role =3D &child_file; + } + + bdrv_format_default_perms(bs, c, role, reopen_queue, perm, shared, + nperm, nshared); +} + static BlockDriver bdrv_test =3D { .format_name =3D "test", .instance_size =3D sizeof(BDRVTestState), @@ -90,7 +110,7 @@ static BlockDriver bdrv_test =3D { .bdrv_co_drain_begin =3D bdrv_test_co_drain_begin, .bdrv_co_drain_end =3D bdrv_test_co_drain_end, =20 - .bdrv_child_perm =3D bdrv_format_default_perms, + .bdrv_child_perm =3D bdrv_test_child_perm, }; =20 static void aio_ret_cb(void *opaque, int ret) @@ -987,13 +1007,14 @@ struct detach_by_parent_data { BdrvChild *child_b; BlockDriverState *c; BdrvChild *child_c; + bool by_parent_cb; }; +static struct detach_by_parent_data detach_by_parent_data; =20 -static void detach_by_parent_aio_cb(void *opaque, int ret) +static void detach_indirect_bh(void *opaque) { struct detach_by_parent_data *data =3D opaque; =20 - g_assert_cmpint(ret, =3D=3D, 0); bdrv_unref_child(data->parent_b, data->child_b); =20 bdrv_ref(data->c); @@ -1001,6 +1022,25 @@ static void detach_by_parent_aio_cb(void *opaque, in= t ret) &child_file, &error_abort); } =20 +static void detach_by_parent_aio_cb(void *opaque, int ret) +{ + struct detach_by_parent_data *data =3D &detach_by_parent_data; + + g_assert_cmpint(ret, =3D=3D, 0); + if (data->by_parent_cb) { + detach_indirect_bh(data); + } +} + +static void detach_by_driver_cb_drained_begin(BdrvChild *child) +{ + aio_bh_schedule_oneshot(qemu_get_current_aio_context(), + detach_indirect_bh, &detach_by_parent_data); + child_file.drained_begin(child); +} + +static BdrvChildRole detach_by_driver_cb_role; + /* * Initial graph: * @@ -1008,17 +1048,25 @@ static void detach_by_parent_aio_cb(void *opaque, i= nt ret) * \ / \ * A B C * - * PA has a pending write request whose callback changes the child nodes o= f PB: - * It removes B and adds C instead. The subtree of PB is drained, which wi= ll - * indirectly drain the write request, too. + * by_parent_cb =3D=3D true: Test that parent callbacks don't poll + * + * PA has a pending write request whose callback changes the child nod= es of + * PB: It removes B and adds C instead. The subtree of PB is drained, = which + * will indirectly drain the write request, too. + * + * by_parent_cb =3D=3D false: Test that bdrv_drain_invoke() doesn't poll + * + * PA's BdrvChildRole has a .drained_begin callback that schedules a BH + * that does the same graph change. If bdrv_drain_invoke() calls it, t= he + * state is messed up, but if it is only polled in the single + * BDRV_POLL_WHILE() at the end of the drain, this should work fine. */ -static void test_detach_by_parent_cb(void) +static void test_detach_indirect(bool by_parent_cb) { BlockBackend *blk; BlockDriverState *parent_a, *parent_b, *a, *b, *c; BdrvChild *child_a, *child_b; BlockAIOCB *acb; - struct detach_by_parent_data data; =20 QEMUIOVector qiov; struct iovec iov =3D { @@ -1027,6 +1075,12 @@ static void test_detach_by_parent_cb(void) }; qemu_iovec_init_external(&qiov, &iov, 1); =20 + if (!by_parent_cb) { + detach_by_driver_cb_role =3D child_file; + detach_by_driver_cb_role.drained_begin =3D + detach_by_driver_cb_drained_begin; + } + /* Create all involved nodes */ parent_a =3D bdrv_new_open_driver(&bdrv_test, "parent-a", BDRV_O_RDWR, &error_abort); @@ -1042,6 +1096,13 @@ static void test_detach_by_parent_cb(void) blk_insert_bs(blk, parent_a, &error_abort); bdrv_unref(parent_a); =20 + /* If we want to get bdrv_drain_invoke() to call aio_poll(), the driver + * callback must not return immediately. */ + if (!by_parent_cb) { + BDRVTestState *s =3D parent_a->opaque; + s->sleep_in_drain_begin =3D true; + } + /* Set child relationships */ bdrv_ref(b); bdrv_ref(a); @@ -1049,7 +1110,9 @@ static void test_detach_by_parent_cb(void) child_a =3D bdrv_attach_child(parent_b, a, "PB-A", &child_backing, &er= ror_abort); =20 bdrv_ref(a); - bdrv_attach_child(parent_a, a, "PA-A", &child_file, &error_abort); + bdrv_attach_child(parent_a, a, "PA-A", + by_parent_cb ? &child_file : &detach_by_driver_cb_ro= le, + &error_abort); =20 g_assert_cmpint(parent_a->refcnt, =3D=3D, 1); g_assert_cmpint(parent_b->refcnt, =3D=3D, 1); @@ -1062,18 +1125,19 @@ static void test_detach_by_parent_cb(void) g_assert(QLIST_NEXT(child_b, next) =3D=3D NULL); =20 /* Start the evil write request */ - data =3D (struct detach_by_parent_data) { + detach_by_parent_data =3D (struct detach_by_parent_data) { .parent_b =3D parent_b, .child_b =3D child_b, .c =3D c, + .by_parent_cb =3D by_parent_cb, }; - acb =3D blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, &dat= a); + acb =3D blk_aio_preadv(blk, 0, &qiov, 0, detach_by_parent_aio_cb, NULL= ); g_assert(acb !=3D NULL); =20 /* Drain and check the expected result */ bdrv_subtree_drained_begin(parent_b); =20 - g_assert(data.child_c !=3D NULL); + g_assert(detach_by_parent_data.child_c !=3D NULL); =20 g_assert_cmpint(parent_a->refcnt, =3D=3D, 1); g_assert_cmpint(parent_b->refcnt, =3D=3D, 1); @@ -1081,8 +1145,8 @@ static void test_detach_by_parent_cb(void) g_assert_cmpint(b->refcnt, =3D=3D, 1); g_assert_cmpint(c->refcnt, =3D=3D, 2); =20 - g_assert(QLIST_FIRST(&parent_b->children) =3D=3D data.child_c); - g_assert(QLIST_NEXT(data.child_c, next) =3D=3D child_a); + g_assert(QLIST_FIRST(&parent_b->children) =3D=3D detach_by_parent_data= .child_c); + g_assert(QLIST_NEXT(detach_by_parent_data.child_c, next) =3D=3D child_= a); g_assert(QLIST_NEXT(child_a, next) =3D=3D NULL); =20 g_assert_cmpint(parent_a->quiesce_counter, =3D=3D, 1); @@ -1110,6 +1174,15 @@ static void test_detach_by_parent_cb(void) bdrv_unref(c); } =20 +static void test_detach_by_parent_cb(void) +{ + test_detach_indirect(true); +} + +static void test_detach_by_driver_cb(void) +{ + test_detach_indirect(false); +} =20 int main(int argc, char **argv) { @@ -1162,6 +1235,7 @@ int main(int argc, char **argv) g_test_add_func("/bdrv-drain/detach/drain", test_detach_by_drain); g_test_add_func("/bdrv-drain/detach/drain_subtree", test_detach_by_dra= in_subtree); g_test_add_func("/bdrv-drain/detach/parent_cb", test_detach_by_parent_= cb); + g_test_add_func("/bdrv-drain/detach/driver_cb", test_detach_by_driver_= cb); =20 ret =3D g_test_run(); qemu_event_destroy(&done_event); --=20 2.13.6