[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 13/21] block: fix bdrv_replace_node_common
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
[PATCH 13/21] block: fix bdrv_replace_node_common |
Date: |
Mon, 23 Nov 2020 23:12:25 +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 | 42 ++++++++++++++-----------------------
tests/test-bdrv-graph-mod.c | 18 +++-------------
2 files changed, 19 insertions(+), 41 deletions(-)
diff --git a/block.c b/block.c
index 5c94f5a428..08501350b7 100644
--- a/block.c
+++ b/block.c
@@ -2178,7 +2178,6 @@ static BdrvActionDrv 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,
GSList **tran)
{
@@ -4787,8 +4786,9 @@ static int bdrv_replace_node_common(BlockDriverState
*from,
{
int ret = -EPERM;
BdrvChild *c, *next;
- GSList *list = NULL, *p;
- uint64_t perm = 0, shared = BLK_PERM_ALL;
+ GSList *tran = NULL;
+ g_autoptr(GHashTable) found = NULL;
+ g_autoptr(GSList) refresh_list = NULL;
/* Make sure that @from doesn't go away until we have successfully attached
* all of its parents to @to. */
@@ -4798,7 +4798,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)) {
@@ -4814,34 +4819,19 @@ 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;
- }
-
- /* 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;
+ bdrv_replace_child_safe(c, to, &tran);
}
- /* 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;
+ found = g_hash_table_new(NULL, NULL);
- bdrv_ref(to);
- bdrv_replace_child_noperm(c, to);
- bdrv_unref(from);
- }
+ refresh_list = bdrv_topological_dfs(refresh_list, found, to);
+ refresh_list = bdrv_topological_dfs(refresh_list, found, from);
- bdrv_set_perm(to);
+ ret = bdrv_list_refresh_perms(refresh_list, errp);
out:
- g_slist_free(list);
+ tran_finalize(tran, ret);
+
bdrv_drained_end(from);
bdrv_unref(from);
diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
index 594a1df58b..d0e0f56ec3 100644
--- a/tests/test-bdrv-graph-mod.c
+++ b/tests/test-bdrv-graph-mod.c
@@ -292,20 +292,11 @@ static void test_parallel_perm_update(void)
bdrv_child_refresh_perms(top, top->children.lh_first, &error_abort);
assert(c_fl1->perm & BLK_PERM_WRITE);
+ bdrv_unref(top);
}
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);
@@ -316,11 +307,8 @@ int main(int argc, char *argv[])
test_should_update_child);
g_test_add_func("/bdrv-graph-mod/parallel-perm-update",
test_parallel_perm_update);
-
- if (debug) {
- g_test_add_func("/bdrv-graph-mod/parallel-exclusive-write",
- test_parallel_exclusive_write);
- }
+ g_test_add_func("/bdrv-graph-mod/parallel-exclusive-write",
+ test_parallel_exclusive_write);
return g_test_run();
}
--
2.21.3
- [PATCH 04/21] block: bdrv_refresh_perms: check parents compliance, (continued)
- [PATCH 04/21] block: bdrv_refresh_perms: check parents compliance, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 06/21] block: rewrite bdrv_child_try_set_perm() using bdrv_refresh_perms(), Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 07/21] block: inline bdrv_child_*() permission functions calls, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 08/21] block: use topological sort for permission update, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 05/21] block: refactor bdrv_child* permission functions, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 09/21] block: add bdrv_drv_set_perm transaction action, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 10/21] block: add bdrv_list_* permission update functions, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 11/21] block: add bdrv_replace_child_safe() transaction action, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 12/21] block: return value from bdrv_replace_node(), Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 13/21] block: fix bdrv_replace_node_common,
Vladimir Sementsov-Ogievskiy <=
- [PATCH 15/21] block: split out bdrv_replace_node_noperm(), Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 14/21] block: add bdrv_attach_child_noperm() transaction action, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 16/21] block: bdrv_append(): don't consume reference, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 19/21] block: add bdrv_remove_backing transaction action, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 21/21] block/backup-top: drop .active, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 17/21] block: bdrv_append(): return status, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 18/21] block: adapt bdrv_append() for inserting filters, Vladimir Sementsov-Ogievskiy, 2020/11/23
- [PATCH 20/21] block: introduce bdrv_drop_filter(), Vladimir Sementsov-Ogievskiy, 2020/11/23