[Qemu-block] [PATCH 1/1] block: update BlockDriverState's children in bdrv_set_backing_hd()

[Alberto Garcia]

When a backing image is opened using bdrv_open_inherit(), it is added
to the parent image's list of children. However there's no way to
remove it from there.

In particular, changing a BlockDriverState's backing image does not
add the new one to the list nor removes the old one. If the latter is
closed then the pointer in the list becomes invalid. This can be
reproduced easily using the block-stream command.

Signed-off-by: Alberto Garcia <address@hidden>
Cc: Kevin Wolf <address@hidden>
---
 block.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index 7e130cc..eaf3ad0 100644
--- a/block.c
+++ b/block.c
@@ -88,6 +88,13 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const 
char *filename,
                              const BdrvChildRole *child_role,
                              BlockDriver *drv, Error **errp);
 
+static void bdrv_attach_child(BlockDriverState *parent_bs,
+                              BlockDriverState *child_bs,
+                              const BdrvChildRole *child_role);
+
+static void bdrv_detach_child(BlockDriverState *parent_bs,
+                              BlockDriverState *child_bs);
+
 static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
@@ -1108,6 +1115,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
     if (bs->backing_hd) {
         assert(bs->backing_blocker);
         bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
+        bdrv_detach_child(bs, bs->backing_hd);
     } else if (backing_hd) {
         error_setg(&bs->backing_blocker,
                    "node is used as backing hd of '%s'",
@@ -1120,6 +1128,11 @@ void bdrv_set_backing_hd(BlockDriverState *bs, 
BlockDriverState *backing_hd)
         bs->backing_blocker = NULL;
         goto out;
     }
+
+    bdrv_attach_child(bs, backing_hd, &child_backing);
+    backing_hd->inherits_from = bs;
+    backing_hd->open_flags = child_backing.inherit_flags(bs->open_flags);
+
     bs->open_flags &= ~BDRV_O_NO_BACKING;
     pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename);
     pstrcpy(bs->backing_format, sizeof(bs->backing_format),
@@ -1332,7 +1345,16 @@ static void bdrv_attach_child(BlockDriverState 
*parent_bs,
                               BlockDriverState *child_bs,
                               const BdrvChildRole *child_role)
 {
-    BdrvChild *child = g_new(BdrvChild, 1);
+    BdrvChild *child;
+
+    /* Don't attach the child if it's already attached */
+    QLIST_FOREACH(child, &parent_bs->children, next) {
+        if (child->bs == child_bs) {
+            return;
+        }
+    }
+
+    child = g_new(BdrvChild, 1);
     *child = (BdrvChild) {
         .bs     = child_bs,
         .role   = child_role,
@@ -1341,6 +1363,21 @@ static void bdrv_attach_child(BlockDriverState 
*parent_bs,
     QLIST_INSERT_HEAD(&parent_bs->children, child, next);
 }
 
+static void bdrv_detach_child(BlockDriverState *parent_bs,
+                              BlockDriverState *child_bs)
+{
+    BdrvChild *child, *next_child;
+    QLIST_FOREACH_SAFE(child, &parent_bs->children, next, next_child) {
+        if (child->bs == child_bs) {
+            if (child->bs->inherits_from == parent_bs) {
+                child->bs->inherits_from = NULL;
+            }
+            QLIST_REMOVE(child, next);
+            g_free(child);
+        }
+    }
+}
+
 /*
  * Opens a disk image (raw, qcow2, vmdk, ...)
  *
@@ -2116,7 +2153,6 @@ void bdrv_append(BlockDriverState *bs_new, 
BlockDriverState *bs_top)
     /* The contents of 'tmp' will become bs_top, as we are
      * swapping bs_new and bs_top contents. */
     bdrv_set_backing_hd(bs_top, bs_new);
-    bdrv_attach_child(bs_top, bs_new, &child_backing);
 }
 
 static void bdrv_delete(BlockDriverState *bs)
-- 
2.1.4