[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 27/35] block: Generalize should_update_child() rule
From: |
Kevin Wolf |
Subject: |
[Qemu-devel] [PULL 27/35] block: Generalize should_update_child() rule |
Date: |
Mon, 18 Jun 2018 18:44:56 +0200 |
From: Max Reitz <address@hidden>
Currently, bdrv_replace_node() refuses to create loops from one BDS to
itself if the BDS to be replaced is the backing node of the BDS to
replace it: Say there is a node A and a node B. Replacing B by A means
making all references to B point to A. If B is a child of A (i.e. A has
a reference to B), that would mean we would have to make this reference
point to A itself -- so we'd create a loop.
bdrv_replace_node() (through should_update_child()) refuses to do so if
B is the backing node of A. There is no reason why we should create
loops if B is not the backing node of A, though. The BDS graph should
never contain loops, so we should always refuse to create them.
If B is a child of A and B is to be replaced by A, we should simply
leave B in place there because it is the most sensible choice.
A more specific argument would be: Putting filter drivers into the BDS
graph is basically the same as appending an overlay to a backing chain.
But the main child BDS of a filter driver is not "backing" but "file",
so restricting the no-loop rule to backing nodes would fail here.
Signed-off-by: Max Reitz <address@hidden>
Reviewed-by: Fam Zheng <address@hidden>
Reviewed-by: Alberto Garcia <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>
---
block.c | 44 ++++++++++++++++++++++++++++++++++----------
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index 6c128007fd..1b8147c1b3 100644
--- a/block.c
+++ b/block.c
@@ -3427,16 +3427,39 @@ static bool should_update_child(BdrvChild *c,
BlockDriverState *to)
return false;
}
- if (c->role == &child_backing) {
- /* If @from is a backing file of @to, ignore the child to avoid
- * creating a loop. We only want to change the pointer of other
- * parents. */
- QLIST_FOREACH(to_c, &to->children, next) {
- if (to_c == c) {
- break;
- }
- }
- if (to_c) {
+ /* If the child @c belongs to the BDS @to, replacing the current
+ * c->bs by @to would mean to create a loop.
+ *
+ * Such a case occurs when appending a BDS to a backing chain.
+ * For instance, imagine the following chain:
+ *
+ * guest device -> node A -> further backing chain...
+ *
+ * Now we create a new BDS B which we want to put on top of this
+ * chain, so we first attach A as its backing node:
+ *
+ * node B
+ * |
+ * v
+ * guest device -> node A -> further backing chain...
+ *
+ * Finally we want to replace A by B. When doing that, we want to
+ * replace all pointers to A by pointers to B -- except for the
+ * pointer from B because (1) that would create a loop, and (2)
+ * that pointer should simply stay intact:
+ *
+ * guest device -> node B
+ * |
+ * v
+ * node A -> further backing chain...
+ *
+ * In general, when replacing a node A (c->bs) by a node B (@to),
+ * if A is a child of B, that means we cannot replace A by B there
+ * because that would create a loop. Silently detaching A from B
+ * is also not really an option. So overall just leaving A in
+ * place there is the most sensible choice. */
+ QLIST_FOREACH(to_c, &to->children, next) {
+ if (to_c == c) {
return false;
}
}
@@ -3462,6 +3485,7 @@ void bdrv_replace_node(BlockDriverState *from,
BlockDriverState *to,
/* Put all parents into @list and calculate their cumulative permissions */
QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
+ assert(c->bs == from);
if (!should_update_child(c, to)) {
continue;
}
--
2.13.6
- [Qemu-devel] [PULL 16/35] block: Allow AIO_WAIT_WHILE with NULL ctx, (continued)
- [Qemu-devel] [PULL 16/35] block: Allow AIO_WAIT_WHILE with NULL ctx, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 20/35] test-bdrv-drain: Test graph changes in drain_all section, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 21/35] block: fix QEMU crash with scsi-hd and drive_del, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 18/35] block: ignore_bds_parents parameter for drain functions, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 19/35] block: Allow graph changes in bdrv_drain_all_begin/end sections, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 22/35] block/mirror: Pull out mirror_perform(), Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 25/35] block/mirror: Wait for in-flight op conflicts, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 24/35] block/mirror: Use CoQueue to wait on in-flight ops, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 23/35] block/mirror: Convert to coroutines, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 26/35] block/mirror: Use source as a BdrvChild, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 27/35] block: Generalize should_update_child() rule,
Kevin Wolf <=
- [Qemu-devel] [PULL 28/35] hbitmap: Add @advance param to hbitmap_iter_next(), Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 29/35] test-hbitmap: Add non-advancing iter_next tests, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 32/35] job: Add job_progress_increase_remaining(), Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 31/35] block/mirror: Add MirrorBDSOpaque, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 30/35] block/dirty-bitmap: Add bdrv_dirty_iter_next_area, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 34/35] block/mirror: Add copy mode QAPI interface, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 33/35] block/mirror: Add active mirroring, Kevin Wolf, 2018/06/18
- [Qemu-devel] [PULL 35/35] iotests: Add test for active mirroring, Kevin Wolf, 2018/06/18
- Re: [Qemu-devel] [PULL 00/35] Block layer patches, no-reply, 2018/06/18
- Re: [Qemu-devel] [PULL 00/35] Block layer patches, Peter Maydell, 2018/06/19