block: Fix error handling in bdrv_replace_in_backing_chain()
When adding an Error parameter, bdrv_replace_in_backing_chain() would
become nothing more than a wrapper around change_parent_backing_link().
So make the latter public, renamed as bdrv_replace_node(), and remove
bdrv_replace_in_backing_chain().
Most of the callers just remove a node from the graph that they just
inserted, so they can use &error_abort, but completion of a mirror job
with 'replaces' set can actually fail.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/block.c b/block.c
index a310132..dd9ded8 100644
--- a/block.c
+++ b/block.c
@@ -2932,8 +2932,8 @@
return true;
}
-static void change_parent_backing_link(BlockDriverState *from,
- BlockDriverState *to, Error **errp)
+void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
+ Error **errp)
{
BdrvChild *c, *next;
GSList *list = NULL, *p;
@@ -2941,6 +2941,9 @@
uint64_t perm = 0, shared = BLK_PERM_ALL;
int ret;
+ assert(!atomic_read(&from->in_flight));
+ assert(!atomic_read(&to->in_flight));
+
/* Make sure that @from doesn't go away until we have successfully attached
* all of its parents to @to. */
bdrv_ref(from);
@@ -3003,16 +3006,13 @@
{
Error *local_err = NULL;
- assert(!atomic_read(&bs_top->in_flight));
- assert(!atomic_read(&bs_new->in_flight));
-
bdrv_set_backing_hd(bs_new, bs_top, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
}
- change_parent_backing_link(bs_top, bs_new, &local_err);
+ bdrv_replace_node(bs_top, bs_new, &local_err);
if (local_err) {
error_propagate(errp, local_err);
bdrv_set_backing_hd(bs_new, NULL, &error_abort);
@@ -3025,19 +3025,6 @@
bdrv_unref(bs_new);
}
-void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new)
-{
- assert(!bdrv_requests_pending(old));
- assert(!bdrv_requests_pending(new));
-
- bdrv_ref(old);
-
- /* FIXME Proper error handling */
- change_parent_backing_link(old, new, &error_abort);
-
- bdrv_unref(old);
-}
-
static void bdrv_delete(BlockDriverState *bs)
{
assert(!bs->job);
diff --git a/block/mirror.c b/block/mirror.c
index f24dc51..a5d30ee 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -550,8 +550,12 @@
/* The mirror job has no requests in flight any more, but we need to
* drain potential other users of the BDS before changing the graph. */
bdrv_drained_begin(target_bs);
- bdrv_replace_in_backing_chain(to_replace, target_bs);
+ bdrv_replace_node(to_replace, target_bs, &local_err);
bdrv_drained_end(target_bs);
+ if (local_err) {
+ error_report_err(local_err);
+ data->ret = -EPERM;
+ }
}
if (s->to_replace) {
bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
@@ -570,12 +574,11 @@
* block the removal. */
block_job_remove_all_bdrv(job);
bdrv_child_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL);
- bdrv_replace_in_backing_chain(mirror_top_bs, backing_bs(mirror_top_bs));
+ bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
/* We just changed the BDS the job BB refers to (with either or both of the
- * bdrv_replace_in_backing_chain() calls), so switch the BB back so the
- * cleanup does the right thing. We don't need any permissions any more
- * now. */
+ * bdrv_replace_node() calls), so switch the BB back so the cleanup does
+ * the right thing. We don't need any permissions any more now. */
blk_remove_bs(job->blk);
blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort);
blk_insert_bs(job->blk, mirror_top_bs, &error_abort);
@@ -1234,7 +1237,7 @@
}
bdrv_child_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL);
- bdrv_replace_in_backing_chain(mirror_top_bs, backing_bs(mirror_top_bs));
+ bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
}
void mirror_start(const char *job_id, BlockDriverState *bs,
diff --git a/blockdev.c b/blockdev.c
index af67ce4..f1f49bd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1806,7 +1806,7 @@
DO_UPCAST(ExternalSnapshotState, common, common);
if (state->new_bs) {
if (state->overlay_appended) {
- bdrv_replace_in_backing_chain(state->new_bs, state->old_bs);
+ bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
}
}
}
diff --git a/include/block/block.h b/include/block/block.h
index c7c4a3a..5149260 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -238,8 +238,8 @@
BlockDriverState *bdrv_new(void);
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
Error **errp);
-void bdrv_replace_in_backing_chain(BlockDriverState *old,
- BlockDriverState *new);
+void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,
+ Error **errp);
int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);
int bdrv_parse_discard_flags(const char *mode, int *flags);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index fc83f7f..6c699ac 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -441,8 +441,8 @@
} BdrvAioNotifier;
struct BdrvChildRole {
- /* If true, bdrv_replace_in_backing_chain() doesn't change the node this
- * BdrvChild points to. */
+ /* If true, bdrv_replace_node() doesn't change the node this BdrvChild
+ * points to. */
bool stay_at_node;
void (*inherit_options)(int *child_flags, QDict *child_options,
