[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v3 17/36] block: fix bdrv_replace_node_common
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
[PATCH v3 17/36] block: fix bdrv_replace_node_common |
Date: |
Wed, 17 Mar 2021 17:35:10 +0300 |
inore_children thing doesn't help to track all propagated permissions
of children we want to ignore. The simplest way to correctly update
permissions is update graph first and then do permission update. In
this case we just referesh permissions for the whole subgraph (in
topological-sort defined order) and everything is correctly calculated
automatically without any ignore_children.
So, refactor bdrv_replace_node_common to first do graph update and then
refresh the permissions.
Test test_parallel_exclusive_write() now pass, so move it out of
debugging "if".
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block.c | 43 +++++++++++++-------------------
tests/unit/test-bdrv-graph-mod.c | 4 +--
2 files changed, 20 insertions(+), 27 deletions(-)
diff --git a/block.c b/block.c
index 4f9d67a6a2..98ff44dbf7 100644
--- a/block.c
+++ b/block.c
@@ -2239,7 +2239,6 @@ static TransactionActionDrv bdrv_replace_child_drv = {
*
* Note: real unref of old_bs is done only on commit.
*/
-__attribute__((unused))
static void bdrv_replace_child_safe(BdrvChild *child, BlockDriverState *new_bs,
Transaction *tran)
{
@@ -4843,8 +4842,9 @@ static int bdrv_replace_node_common(BlockDriverState
*from,
bool auto_skip, Error **errp)
{
BdrvChild *c, *next;
- GSList *list = NULL, *p;
- uint64_t perm = 0, shared = BLK_PERM_ALL;
+ Transaction *tran = tran_new();
+ g_autoptr(GHashTable) found = NULL;
+ g_autoptr(GSList) refresh_list = NULL;
int ret;
/* Make sure that @from doesn't go away until we have successfully attached
@@ -4855,7 +4855,12 @@ static int bdrv_replace_node_common(BlockDriverState
*from,
assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to));
bdrv_drained_begin(from);
- /* Put all parents into @list and calculate their cumulative permissions */
+ /*
+ * Do the replacement without permission update.
+ * Replacement may influence the permissions, we should calculate new
+ * permissions based on new graph. If we fail, we'll roll-back the
+ * replacement.
+ */
QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
assert(c->bs == from);
if (!should_update_child(c, to)) {
@@ -4873,36 +4878,24 @@ static int bdrv_replace_node_common(BlockDriverState
*from,
c->name, from->node_name);
goto out;
}
- list = g_slist_prepend(list, c);
- perm |= c->perm;
- shared &= c->shared_perm;
+ bdrv_replace_child_safe(c, to, tran);
}
- /* Check whether the required permissions can be granted on @to, ignoring
- * all BdrvChild in @list so that they can't block themselves. */
- ret = bdrv_check_update_perm(to, NULL, perm, shared, list, errp);
- if (ret < 0) {
- bdrv_abort_perm_update(to);
- goto out;
- }
+ found = g_hash_table_new(NULL, NULL);
- /* Now actually perform the change. We performed the permission check for
- * all elements of @list at once, so set the permissions all at once at the
- * very end. */
- for (p = list; p != NULL; p = p->next) {
- c = p->data;
+ refresh_list = bdrv_topological_dfs(refresh_list, found, to);
+ refresh_list = bdrv_topological_dfs(refresh_list, found, from);
- bdrv_ref(to);
- bdrv_replace_child_noperm(c, to);
- bdrv_unref(from);
+ ret = bdrv_list_refresh_perms(refresh_list, NULL, tran, errp);
+ if (ret < 0) {
+ goto out;
}
- bdrv_set_perm(to);
-
ret = 0;
out:
- g_slist_free(list);
+ tran_finalize(tran, ret);
+
bdrv_drained_end(from);
bdrv_unref(from);
diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c
index a98100ddfd..b81787487a 100644
--- a/tests/unit/test-bdrv-graph-mod.c
+++ b/tests/unit/test-bdrv-graph-mod.c
@@ -407,10 +407,10 @@ int main(int argc, char *argv[])
test_should_update_child);
g_test_add_func("/bdrv-graph-mod/parallel-perm-update",
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/parallel-exclusive-write",
- test_parallel_exclusive_write);
g_test_add_func("/bdrv-graph-mod/append-greedy-filter",
test_append_greedy_filter);
}
--
2.29.2
- [PATCH v3 03/36] tests/test-bdrv-graph-mod: add test_append_greedy_filter, (continued)
- [PATCH v3 03/36] tests/test-bdrv-graph-mod: add test_append_greedy_filter, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 06/36] block: drop ctx argument from bdrv_root_attach_child, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 04/36] block: bdrv_append(): don't consume reference, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 07/36] block: make bdrv_reopen_{prepare, commit, abort} private, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 08/36] util: add transactions.c, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 05/36] block: BdrvChildClass: add .get_parent_aio_context handler, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 09/36] block: bdrv_refresh_perms: check for parents permissions conflict, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 10/36] block: refactor bdrv_child* permission functions, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 12/36] block: inline bdrv_child_*() permission functions calls, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 11/36] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms(), Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 17/36] block: fix bdrv_replace_node_common,
Vladimir Sementsov-Ogievskiy <=
- [PATCH v3 13/36] block: use topological sort for permission update, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 19/36] block: add bdrv_attach_child_noperm() transaction action, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 20/36] block: split out bdrv_replace_node_noperm(), Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 21/36] block: adapt bdrv_append() for inserting filters, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 18/36] block: add bdrv_attach_child_common() transaction action, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 14/36] block: add bdrv_drv_set_perm transaction action, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 25/36] block: drop ignore_children for permission update functions, Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 23/36] block: introduce bdrv_drop_filter(), Vladimir Sementsov-Ogievskiy, 2021/03/17
- [PATCH v3 22/36] block: add bdrv_remove_filter_or_cow transaction action, Vladimir Sementsov-Ogievskiy, 2021/03/17