diff --git a/block.c b/block.c
index b4d2313a9b..7c66d3ec26 100644
--- a/block.c
+++ b/block.c
@@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
return child;
}
-void bdrv_detach_child(BdrvChild *child)
+static void bdrv_detach_child(BdrvChild *child)
{
QLIST_REMOVE(child, next);
QLIST_REMOVE(child, next_parent);
@@ -2232,6 +2232,36 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top)
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);
+
+ if (old->blk) {
+ /* As long as these fields aren't in BlockBackend, but in the top-level
+ * BlockDriverState, it's not possible for a BDS to have two BBs.
+ *
+ * We really want to copy the fields from old to new, but we go for a
+ * swap instead so that pointers aren't duplicated and cause trouble.
+ * (Also, bdrv_swap() used to do the same.) */
+ assert(!new->blk);
+ swap_feature_fields(old, new);
+ }
+ change_parent_backing_link(old, new);
+
+ /* Change backing files if a previously independent node is added to the
+ * chain. For active commit, we replace top by its own (indirect) backing
+ * file and don't do anything here so we don't build a loop. */
+ if (new->backing == NULL && !bdrv_chain_contains(backing_bs(old), new)) {
+ bdrv_set_backing_hd(new, backing_bs(old));
+ bdrv_set_backing_hd(old, NULL);
+ }
+
+ bdrv_unref(old);
+}
+
static void bdrv_delete(BlockDriverState *bs)
{
assert(!bs->job);
diff --git a/block/mirror.c b/block/mirror.c
index c277691e4d..7e43511832 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -353,6 +353,11 @@ static void mirror_exit(BlockJob *job, void *opaque)
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
MirrorExitData *data = opaque;
AioContext *replace_aio_context = NULL;
+ BlockDriverState *src = s->common.bs;
+
+ /* Make sure that the source BDS doesn't go away before we called
+ * block_job_completed(). */
+ bdrv_ref(src);
if (s->to_replace) {
replace_aio_context = bdrv_get_aio_context(s->to_replace);
@@ -367,22 +372,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
}
- bdrv_swap(s->target, to_replace);
- if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
- /* drop the bs loop chain formed by the swap: break the loop then
- * trigger the unref */
- /* FIXME This duplicates bdrv_set_backing_hd(), except for the
- * actual detach/unref so that the loop can be broken. When
- * bdrv_swap() gets replaced, this will become sane again. */
- BlockDriverState *backing = s->base->backing->bs;
- assert(s->base->backing_blocker);
- bdrv_op_unblock_all(backing, s->base->backing_blocker);
- error_free(s->base->backing_blocker);
- s->base->backing_blocker = NULL;
- bdrv_detach_child(s->base->backing);
- s->base->backing = NULL;
- bdrv_unref(backing);
- }
+ bdrv_replace_in_backing_chain(to_replace, s->target);
}
if (s->to_replace) {
bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
@@ -396,6 +386,7 @@ static void mirror_exit(BlockJob *job, void *opaque)
bdrv_unref(s->target);
block_job_completed(&s->common, data->ret);
g_free(data);
+ bdrv_unref(src);
}
static void coroutine_fn mirror_run(void *opaque)
diff --git a/include/block/block.h b/include/block/block.h
index da94bdfa9d..6d70eb42fe 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -203,6 +203,9 @@ BlockDriverState *bdrv_new(void);
void bdrv_make_anon(BlockDriverState *bs);
void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
+void bdrv_replace_in_backing_chain(BlockDriverState *old,
+ BlockDriverState *new);
+
int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_parse_discard_flags(const char *mode, int *flags);
BdrvChild *bdrv_open_child(const char *filename,
@@ -513,7 +516,6 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
void bdrv_ref(BlockDriverState *bs);
void bdrv_unref(BlockDriverState *bs);
void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child);
-void bdrv_detach_child(BdrvChild *child);
bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
diff --git a/qemu-img.c b/qemu-img.c
index e678fdc403..3025776e14 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -766,12 +766,12 @@ static int img_commit(int argc, char **argv)
goto done;
}
- /* The block job will swap base_bs and bs (which is not what we really want
- * here, but okay) and unref base_bs (after the swap, i.e., the old top
- * image). In order to still be able to empty that top image afterwards,
- * increment the reference counter here preemptively. */
+ /* When the block job completes, the BlockBackend reference will point to
+ * the old backing file. In order to avoid that the top image is already
+ * deleted, so we can still empty it afterwards, increment the reference
+ * counter here preemptively. */
if (!drop) {
- bdrv_ref(base_bs);
+ bdrv_ref(bs);
}
run_block_job(bs->job, &local_err);
@@ -779,8 +779,8 @@ static int img_commit(int argc, char **argv)
goto unref_backing;
}
- if (!drop && base_bs->drv->bdrv_make_empty) {
- ret = base_bs->drv->bdrv_make_empty(base_bs);
+ if (!drop && bs->drv->bdrv_make_empty) {
+ ret = bs->drv->bdrv_make_empty(bs);
if (ret) {
error_setg_errno(&local_err, -ret, "Could not empty %s",
filename);
@@ -790,7 +790,7 @@ static int img_commit(int argc, char **argv)
unref_backing:
if (!drop) {
- bdrv_unref(base_bs);
+ bdrv_unref(bs);
}
done: