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, peter.maydell@linaro.org, qemu-devel@nongnu.org
Subject: [PULL 21/39] block: adapt bdrv_append() for inserting filters
Date: Fri, 30 Apr 2021 12:51:29 +0200	[thread overview]
Message-ID: <20210430105147.125840-22-kwolf@redhat.com> (raw)
In-Reply-To: <20210430105147.125840-1-kwolf@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

bdrv_append is not very good for inserting filters: it does extra
permission update as part of bdrv_set_backing_hd(). During this update
filter may conflict with other parents of top_bs.

Instead, let's first do all graph modifications and after it update
permissions.

append-greedy-filter test-case in test-bdrv-graph-mod is now works, so
move it out of debug option.

Note: bdrv_append() is still only works for backing-child based
filters. It's something to improve later.

Note2: we use the fact that bdrv_append() is used to append new nodes,
without backing child, so we don't need frozen check and inherits_from
logic from bdrv_set_backing_hd().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20210428151804.439460-22-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                          | 27 ++++++++++++++++++++-------
 tests/unit/test-bdrv-graph-mod.c | 17 ++---------------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/block.c b/block.c
index 9283c8d97b..5bb6a2bef9 100644
--- a/block.c
+++ b/block.c
@@ -5088,25 +5088,38 @@ int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
  * This will modify the BlockDriverState fields, and swap contents
  * between bs_new and bs_top. Both bs_new and bs_top are modified.
  *
- * bs_new must not be attached to a BlockBackend.
+ * bs_new must not be attached to a BlockBackend and must not have backing
+ * child.
  *
  * This function does not create any image files.
  */
 int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
                 Error **errp)
 {
-    int ret = bdrv_set_backing_hd(bs_new, bs_top, errp);
+    int ret;
+    Transaction *tran = tran_new();
+
+    assert(!bs_new->backing);
+
+    ret = bdrv_attach_child_noperm(bs_new, bs_top, "backing",
+                                   &child_of_bds, bdrv_backing_role(bs_new),
+                                   &bs_new->backing, tran, errp);
     if (ret < 0) {
-        return ret;
+        goto out;
     }
 
-    ret = bdrv_replace_node(bs_top, bs_new, errp);
+    ret = bdrv_replace_node_noperm(bs_top, bs_new, true, tran, errp);
     if (ret < 0) {
-        bdrv_set_backing_hd(bs_new, NULL, &error_abort);
-        return ret;
+        goto out;
     }
 
-    return 0;
+    ret = bdrv_refresh_perms(bs_new, errp);
+out:
+    tran_finalize(tran, ret);
+
+    bdrv_refresh_limits(bs_top, NULL);
+
+    return ret;
 }
 
 static void bdrv_delete(BlockDriverState *bs)
diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c
index 7b3c8b437a..88f25c0cdb 100644
--- a/tests/unit/test-bdrv-graph-mod.c
+++ b/tests/unit/test-bdrv-graph-mod.c
@@ -388,16 +388,6 @@ static void test_append_greedy_filter(void)
 
 int main(int argc, char *argv[])
 {
-    int i;
-    bool debug = false;
-
-    for (i = 1; i < argc; i++) {
-        if (!strcmp(argv[i], "-d")) {
-            debug = true;
-            break;
-        }
-    }
-
     bdrv_init();
     qemu_init_main_loop(&error_abort);
 
@@ -410,11 +400,8 @@ int main(int argc, char *argv[])
                     test_parallel_perm_update);
     g_test_add_func("/bdrv-graph-mod/parallel-exclusive-write",
                     test_parallel_exclusive_write);
-
-    if (debug) {
-        g_test_add_func("/bdrv-graph-mod/append-greedy-filter",
-                        test_append_greedy_filter);
-    }
+    g_test_add_func("/bdrv-graph-mod/append-greedy-filter",
+                    test_append_greedy_filter);
 
     return g_test_run();
 }
-- 
2.30.2



  parent reply	other threads:[~2021-04-30 11:30 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30 10:51 [PULL 00/39] Block layer patches Kevin Wolf
2021-04-30 10:51 ` [PULL 01/39] tests/test-bdrv-graph-mod: add test_parallel_exclusive_write Kevin Wolf
2021-04-30 10:51 ` [PULL 02/39] tests/test-bdrv-graph-mod: add test_parallel_perm_update Kevin Wolf
2021-04-30 10:51 ` [PULL 03/39] tests/test-bdrv-graph-mod: add test_append_greedy_filter Kevin Wolf
2021-04-30 10:51 ` [PULL 04/39] block: bdrv_append(): don't consume reference Kevin Wolf
2021-04-30 10:51 ` [PULL 05/39] block: BdrvChildClass: add .get_parent_aio_context handler Kevin Wolf
2021-04-30 10:51 ` [PULL 06/39] block: drop ctx argument from bdrv_root_attach_child Kevin Wolf
2021-04-30 10:51 ` [PULL 07/39] block: make bdrv_reopen_{prepare,commit,abort} private Kevin Wolf
2021-04-30 10:51 ` [PULL 08/39] util: add transactions.c Kevin Wolf
2021-04-30 10:51 ` [PULL 09/39] block: bdrv_refresh_perms: check for parents permissions conflict Kevin Wolf
2021-04-30 10:51 ` [PULL 10/39] block: refactor bdrv_child* permission functions Kevin Wolf
2021-04-30 10:51 ` [PULL 11/39] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms() Kevin Wolf
2021-04-30 10:51 ` [PULL 12/39] block: inline bdrv_child_*() permission functions calls Kevin Wolf
2021-04-30 10:51 ` [PULL 13/39] block: use topological sort for permission update Kevin Wolf
2021-04-30 10:51 ` [PULL 14/39] block: add bdrv_drv_set_perm transaction action Kevin Wolf
2021-04-30 10:51 ` [PULL 15/39] block: add bdrv_list_* permission update functions Kevin Wolf
2021-04-30 10:51 ` [PULL 16/39] block: add bdrv_replace_child_safe() transaction action Kevin Wolf
2021-04-30 10:51 ` [PULL 17/39] block: fix bdrv_replace_node_common Kevin Wolf
2021-04-30 10:51 ` [PULL 18/39] block: add bdrv_attach_child_common() transaction action Kevin Wolf
2021-04-30 22:33   ` Peter Maydell
2021-04-30 10:51 ` [PULL 19/39] block: add bdrv_attach_child_noperm() " Kevin Wolf
2021-04-30 10:51 ` [PULL 20/39] block: split out bdrv_replace_node_noperm() Kevin Wolf
2021-04-30 10:51 ` Kevin Wolf [this message]
2021-04-30 10:51 ` [PULL 22/39] block: add bdrv_remove_filter_or_cow transaction action Kevin Wolf
2021-04-30 10:51 ` [PULL 23/39] block: introduce bdrv_drop_filter() Kevin Wolf
2021-04-30 10:51 ` [PULL 24/39] block/backup-top: drop .active Kevin Wolf
2021-04-30 10:51 ` [PULL 25/39] block: drop ignore_children for permission update functions Kevin Wolf
2021-04-30 10:51 ` [PULL 26/39] block: make bdrv_unset_inherits_from to be a transaction action Kevin Wolf
2021-04-30 10:51 ` [PULL 27/39] block: make bdrv_refresh_limits() " Kevin Wolf
2021-04-30 10:51 ` [PULL 28/39] block: add bdrv_set_backing_noperm() " Kevin Wolf
2021-04-30 10:51 ` [PULL 29/39] block: bdrv_reopen_multiple(): move bdrv_flush to separate pre-prepare Kevin Wolf
2021-04-30 10:51 ` [PULL 30/39] block: bdrv_reopen_multiple: refresh permissions on updated graph Kevin Wolf
2021-04-30 22:38   ` Peter Maydell
2021-04-30 10:51 ` [PULL 31/39] block: drop unused permission update functions Kevin Wolf
2021-04-30 10:51 ` [PULL 32/39] block: inline bdrv_check_perm_common() Kevin Wolf
2021-04-30 10:51 ` [PULL 33/39] block: inline bdrv_replace_child() Kevin Wolf
2021-04-30 10:51 ` [PULL 34/39] block: refactor bdrv_child_set_perm_safe() transaction action Kevin Wolf
2021-04-30 10:51 ` [PULL 35/39] block: rename bdrv_replace_child_safe() to bdrv_replace_child() Kevin Wolf
2021-04-30 10:51 ` [PULL 36/39] block: refactor bdrv_node_check_perm() Kevin Wolf
2021-04-30 10:51 ` [PULL 37/39] block: Add BDRV_O_NO_SHARE for blk_new_open() Kevin Wolf
2021-04-30 10:51 ` [PULL 38/39] qemu-img convert: Unshare write permission for source Kevin Wolf
2021-04-30 10:51 ` [PULL 39/39] vhost-user-blk: Fail gracefully on too large queue size Kevin Wolf
2021-04-30 15:00 ` [PULL 00/39] Block layer patches 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=20210430105147.125840-22-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=peter.maydell@linaro.org \
    --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.