On 05/05/21 10:05, Vladimir Sementsov-Ogievskiy wrote:
diff --git a/block.c b/block.c
index 874c22c43e..3ca27bd2d9 100644
--- a/block.c
+++ b/block.c
@@ -4851,7 +4851,7 @@ static int
bdrv_replace_node_common(BlockDriverState *from,
Transaction *tran = tran_new();
g_autoptr(GHashTable) found = NULL;
g_autoptr(GSList) refresh_list = NULL;
- BlockDriverState *to_cow_parent;
+ BlockDriverState *to_cow_parent = NULL;
May be
+ BlockDriverState *to_cow_parent = NULL; /* Silence compiler warning */
We can also do something like this where the only caller with
to_detach==true takes care of passing the right CoW-parent, and the
for loop goes away completely if I'm not mistaken:
diff --git a/block.c b/block.c
index ae1a7e25aa..3f6fa8475c 100644
--- a/block.c
+++ b/block.c
@@ -4839,31 +4839,19 @@ static int bdrv_replace_node_noperm(BlockDriverState
*from,
* With auto_skip=false the error is returned if from has a parent which should
* not be updated.
*
- * With @detach_subchain=true @to must be in a backing chain of @from. In this
- * case backing link of the cow-parent of @to is removed.
+ * With @to_detach is not #NULL its link to @to is removed.
*/
static int bdrv_replace_node_common(BlockDriverState *from,
BlockDriverState *to,
- bool auto_skip, bool detach_subchain,
+ bool auto_skip, BlockDriverState
*to_detach,
Error **errp)
{
Transaction *tran = tran_new();
g_autoptr(GHashTable) found = NULL;
g_autoptr(GSList) refresh_list = NULL;
- BlockDriverState *to_cow_parent;
+ BlockDriverState *to_detach;
int ret;
- if (detach_subchain) {
- assert(bdrv_chain_contains(from, to));
- assert(from != to);
- for (to_cow_parent = from;
- bdrv_filter_or_cow_bs(to_cow_parent) != to;
- to_cow_parent = bdrv_filter_or_cow_bs(to_cow_parent))
- {
- ;
- }
- }
-
/* Make sure that @from doesn't go away until we have successfully attached
* all of its parents to @to. */
bdrv_ref(from);
@@ -4883,8 +4871,8 @@ static int bdrv_replace_node_common(BlockDriverState
*from,
goto out;
}
- if (detach_subchain) {
- bdrv_remove_filter_or_cow_child(to_cow_parent, tran);
+ if (to_detach) {
+ bdrv_remove_filter_or_cow_child(to_detach, tran);
}
found = g_hash_table_new(NULL, NULL);
@@ -4911,13 +4899,21 @@ out:
int bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
Error **errp)
{
- return bdrv_replace_node_common(from, to, true, false, errp);
+ return bdrv_replace_node_common(from, to, true, NULL, errp);
}
int bdrv_drop_filter(BlockDriverState *bs, Error **errp)
{
- return bdrv_replace_node_common(bs, bdrv_filter_or_cow_bs(bs), true, true,
- errp);
+ BlockDriverState *to = bdrv_filter_or_cow_bs(bs);
+
+ assert(bdrv_chain_contains(bs, to));
+ assert(bs != to);
+ return bdrv_replace_node_common(bs, to, true, bs, errp);
}
/*
@@ -5262,7 +5262,7 @@ int bdrv_drop_intermediate(BlockDriverState *top,
BlockDriverState *base,
* test-bdrv-drain. test_drop_intermediate_poll() test-case will crash.
* That's a FIXME.
*/
- bdrv_replace_node_common(top, base, false, false, &local_err);
+ bdrv_replace_node_common(top, base, false, NULL, &local_err);
if (local_err) {
error_report_err(local_err);
goto exit;
Even nicer would be to move the bdrv_remove_filter_or_cow_child() call to
bdrv_drop_filter, and pass in a Transaction to bdrv_replace_node_common, but
I'm not sure if bdrv_replace_node_noperm and bdrv_remove_filter_or_cow_child
can commute.
Paolo