All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 09/35] test-bdrv-drain: Add test for node deletion
Date: Mon, 18 Jun 2018 18:44:38 +0200	[thread overview]
Message-ID: <20180618164504.24488-10-kwolf@redhat.com> (raw)
In-Reply-To: <20180618164504.24488-1-kwolf@redhat.com>

From: Max Reitz <mreitz@redhat.com>

This patch adds two bdrv-drain tests for what happens if some BDS goes
away during the drainage.

The basic idea is that you have a parent BDS with some child nodes.
Then, you drain one of the children.  Because of that, the party who
actually owns the parent decides to (A) delete it, or (B) detach all its
children from it -- both while the child is still being drained.

A real-world case where this can happen is the mirror block job, which
may exit if you drain one of its children.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/test-bdrv-drain.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index 22d31c953e..e8a5515b08 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -797,6 +797,172 @@ static void test_blockjob_drain_subtree(void)
     test_blockjob_common(BDRV_SUBTREE_DRAIN);
 }
 
+
+typedef struct BDRVTestTopState {
+    BdrvChild *wait_child;
+} BDRVTestTopState;
+
+static void bdrv_test_top_close(BlockDriverState *bs)
+{
+    BdrvChild *c, *next_c;
+    QLIST_FOREACH_SAFE(c, &bs->children, next, next_c) {
+        bdrv_unref_child(bs, c);
+    }
+}
+
+static int coroutine_fn bdrv_test_top_co_preadv(BlockDriverState *bs,
+                                                uint64_t offset, uint64_t bytes,
+                                                QEMUIOVector *qiov, int flags)
+{
+    BDRVTestTopState *tts = bs->opaque;
+    return bdrv_co_preadv(tts->wait_child, offset, bytes, qiov, flags);
+}
+
+static BlockDriver bdrv_test_top_driver = {
+    .format_name            = "test_top_driver",
+    .instance_size          = sizeof(BDRVTestTopState),
+
+    .bdrv_close             = bdrv_test_top_close,
+    .bdrv_co_preadv         = bdrv_test_top_co_preadv,
+
+    .bdrv_child_perm        = bdrv_format_default_perms,
+};
+
+typedef struct TestCoDeleteByDrainData {
+    BlockBackend *blk;
+    bool detach_instead_of_delete;
+    bool done;
+} TestCoDeleteByDrainData;
+
+static void coroutine_fn test_co_delete_by_drain(void *opaque)
+{
+    TestCoDeleteByDrainData *dbdd = opaque;
+    BlockBackend *blk = dbdd->blk;
+    BlockDriverState *bs = blk_bs(blk);
+    BDRVTestTopState *tts = bs->opaque;
+    void *buffer = g_malloc(65536);
+    QEMUIOVector qiov;
+    struct iovec iov = {
+        .iov_base = buffer,
+        .iov_len  = 65536,
+    };
+
+    qemu_iovec_init_external(&qiov, &iov, 1);
+
+    /* Pretend some internal write operation from parent to child.
+     * Important: We have to read from the child, not from the parent!
+     * Draining works by first propagating it all up the tree to the
+     * root and then waiting for drainage from root to the leaves
+     * (protocol nodes).  If we have a request waiting on the root,
+     * everything will be drained before we go back down the tree, but
+     * we do not want that.  We want to be in the middle of draining
+     * when this following requests returns. */
+    bdrv_co_preadv(tts->wait_child, 0, 65536, &qiov, 0);
+
+    g_assert_cmpint(bs->refcnt, ==, 1);
+
+    if (!dbdd->detach_instead_of_delete) {
+        blk_unref(blk);
+    } else {
+        BdrvChild *c, *next_c;
+        QLIST_FOREACH_SAFE(c, &bs->children, next, next_c) {
+            bdrv_unref_child(bs, c);
+        }
+    }
+
+    dbdd->done = true;
+}
+
+/**
+ * Test what happens when some BDS has some children, you drain one of
+ * them and this results in the BDS being deleted.
+ *
+ * If @detach_instead_of_delete is set, the BDS is not going to be
+ * deleted but will only detach all of its children.
+ */
+static void do_test_delete_by_drain(bool detach_instead_of_delete)
+{
+    BlockBackend *blk;
+    BlockDriverState *bs, *child_bs, *null_bs;
+    BDRVTestTopState *tts;
+    TestCoDeleteByDrainData dbdd;
+    Coroutine *co;
+
+    bs = bdrv_new_open_driver(&bdrv_test_top_driver, "top", BDRV_O_RDWR,
+                              &error_abort);
+    bs->total_sectors = 65536 >> BDRV_SECTOR_BITS;
+    tts = bs->opaque;
+
+    null_bs = bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+                        &error_abort);
+    bdrv_attach_child(bs, null_bs, "null-child", &child_file, &error_abort);
+
+    /* This child will be the one to pass to requests through to, and
+     * it will stall until a drain occurs */
+    child_bs = bdrv_new_open_driver(&bdrv_test, "child", BDRV_O_RDWR,
+                                    &error_abort);
+    child_bs->total_sectors = 65536 >> BDRV_SECTOR_BITS;
+    /* Takes our reference to child_bs */
+    tts->wait_child = bdrv_attach_child(bs, child_bs, "wait-child", &child_file,
+                                        &error_abort);
+
+    /* This child is just there to be deleted
+     * (for detach_instead_of_delete == true) */
+    null_bs = bdrv_open("null-co://", NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
+                        &error_abort);
+    bdrv_attach_child(bs, null_bs, "null-child", &child_file, &error_abort);
+
+    blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
+    blk_insert_bs(blk, bs, &error_abort);
+
+    /* Referenced by blk now */
+    bdrv_unref(bs);
+
+    g_assert_cmpint(bs->refcnt, ==, 1);
+    g_assert_cmpint(child_bs->refcnt, ==, 1);
+    g_assert_cmpint(null_bs->refcnt, ==, 1);
+
+
+    dbdd = (TestCoDeleteByDrainData){
+        .blk = blk,
+        .detach_instead_of_delete = detach_instead_of_delete,
+        .done = false,
+    };
+    co = qemu_coroutine_create(test_co_delete_by_drain, &dbdd);
+    qemu_coroutine_enter(co);
+
+    /* Drain the child while the read operation is still pending.
+     * This should result in the operation finishing and
+     * test_co_delete_by_drain() resuming.  Thus, @bs will be deleted
+     * and the coroutine will exit while this drain operation is still
+     * in progress. */
+    bdrv_ref(child_bs);
+    bdrv_drain(child_bs);
+    bdrv_unref(child_bs);
+
+    while (!dbdd.done) {
+        aio_poll(qemu_get_aio_context(), true);
+    }
+
+    if (detach_instead_of_delete) {
+        /* Here, the reference has not passed over to the coroutine,
+         * so we have to delete the BB ourselves */
+        blk_unref(blk);
+    }
+}
+
+
+static void test_delete_by_drain(void)
+{
+    do_test_delete_by_drain(false);
+}
+
+static void test_detach_by_drain(void)
+{
+    do_test_delete_by_drain(true);
+}
+
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -844,6 +1010,9 @@ int main(int argc, char **argv)
     g_test_add_func("/bdrv-drain/blockjob/drain_subtree",
                     test_blockjob_drain_subtree);
 
+    g_test_add_func("/bdrv-drain/deletion", test_delete_by_drain);
+    g_test_add_func("/bdrv-drain/detach", test_detach_by_drain);
+
     ret = g_test_run();
     qemu_event_destroy(&done_event);
     return ret;
-- 
2.13.6

  parent reply	other threads:[~2018-06-18 16:45 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-18 16:44 [Qemu-devel] [PULL 00/35] Block layer patches Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 01/35] test-bdrv-drain: bdrv_drain() works with cross-AioContext events Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 02/35] block: Use bdrv_do_drain_begin/end in bdrv_drain_all() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 03/35] block: Remove 'recursive' parameter from bdrv_drain_invoke() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 04/35] block: Don't manually poll in bdrv_drain_all() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 05/35] tests/test-bdrv-drain: bdrv_drain_all() works in coroutines now Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 06/35] block: Avoid unnecessary aio_poll() in AIO_WAIT_WHILE() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 07/35] block: Really pause block jobs on drain Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 08/35] block: Remove bdrv_drain_recurse() Kevin Wolf
2018-06-18 16:44 ` Kevin Wolf [this message]
2018-06-18 16:44 ` [Qemu-devel] [PULL 10/35] block: Drain recursively with a single BDRV_POLL_WHILE() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 11/35] test-bdrv-drain: Test node deletion in subtree recursion Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 12/35] block: Don't poll in parent drain callbacks Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 13/35] test-bdrv-drain: Graph change through parent callback Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 14/35] block: Defer .bdrv_drain_begin callback to polling phase Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 15/35] test-bdrv-drain: Test that bdrv_drain_invoke() doesn't poll Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 16/35] block: Allow AIO_WAIT_WHILE with NULL ctx Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 17/35] block: Move bdrv_drain_all_begin() out of coroutine context Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 18/35] block: ignore_bds_parents parameter for drain functions Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 19/35] block: Allow graph changes in bdrv_drain_all_begin/end sections Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 20/35] test-bdrv-drain: Test graph changes in drain_all section Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 21/35] block: fix QEMU crash with scsi-hd and drive_del Kevin Wolf
2018-08-06 22:04   ` Eric Blake
2018-08-07 19:57     ` Eric Blake
2018-08-08  9:33       ` Vladimir Sementsov-Ogievskiy
2018-08-08 14:32         ` Vladimir Sementsov-Ogievskiy
2018-08-08 14:53           ` Eric Blake
2018-08-08 11:40       ` Vladimir Sementsov-Ogievskiy
2018-08-08 12:53         ` Eric Blake
2018-06-18 16:44 ` [Qemu-devel] [PULL 22/35] block/mirror: Pull out mirror_perform() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 23/35] block/mirror: Convert to coroutines Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 24/35] block/mirror: Use CoQueue to wait on in-flight ops Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 25/35] block/mirror: Wait for in-flight op conflicts Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 26/35] block/mirror: Use source as a BdrvChild Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 27/35] block: Generalize should_update_child() rule Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 28/35] hbitmap: Add @advance param to hbitmap_iter_next() Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 29/35] test-hbitmap: Add non-advancing iter_next tests Kevin Wolf
2018-06-18 16:44 ` [Qemu-devel] [PULL 30/35] block/dirty-bitmap: Add bdrv_dirty_iter_next_area Kevin Wolf
2018-08-03 15:17   ` Vladimir Sementsov-Ogievskiy
2018-06-18 16:45 ` [Qemu-devel] [PULL 31/35] block/mirror: Add MirrorBDSOpaque Kevin Wolf
2018-06-18 16:45 ` [Qemu-devel] [PULL 32/35] job: Add job_progress_increase_remaining() Kevin Wolf
2018-06-18 16:45 ` [Qemu-devel] [PULL 33/35] block/mirror: Add active mirroring Kevin Wolf
2018-08-03 15:20   ` Vladimir Sementsov-Ogievskiy
2018-06-18 16:45 ` [Qemu-devel] [PULL 34/35] block/mirror: Add copy mode QAPI interface Kevin Wolf
2018-06-18 16:45 ` [Qemu-devel] [PULL 35/35] iotests: Add test for active mirroring Kevin Wolf
2018-06-18 18:50 ` [Qemu-devel] [PULL 00/35] Block layer patches no-reply
2018-06-19 15:57 ` Peter Maydell

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=20180618164504.24488-10-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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.