block: Update BlockDriverState.inherits_from on bdrv_set_backing_hd()

When a BlockDriverState's child is opened (be it a backing file, the
protocol layer, or any other) inherits_from is set to point to the
parent node. Children opened separately and then attached to a parent
don't have this pointer set.

bdrv_reopen_queue_child() uses this to determine whether a node's
children must also be reopened inheriting the options from the parent
or not. If inherits_from points to the parent then the child is
reopened and its options can be changed, like in this example:

   $ qemu-img create -f qcow2 hd0.qcow2 1M
   $ qemu-img create -f qcow2 hd1.qcow2 1M
   $ $QEMU -drive if=none,node-name=hd0,file=hd0.qcow2,\
                  backing.driver=qcow2,backing.file.filename=hd1.qcow2
   (qemu) qemu-io hd0 "reopen -o backing.l2-cache-size=2M"

If the child does not inherit from the parent then it does not get
reopened and its options cannot be changed:

   $ $QEMU -drive if=none,node-name=hd1,file=hd1.qcow2
           -drive if=none,node-name=hd0,file=hd0.qcow2,backing=hd1
   (qemu) qemu-io hd0 "reopen -o backing.l2-cache-size=2M"
   Cannot change the option 'backing.l2-cache-size'

If a disk image has a chain of backing files then all of them are also
connected through their inherits_from pointers (i.e. it's possible to
walk the chain in reverse order from base to top).

However this is broken if the intermediate nodes are removed using
e.g. block-stream because the inherits_from pointer from the base node
becomes NULL:

   $ qemu-img create -f qcow2 hd0.qcow2 1M
   $ qemu-img create -f qcow2 -b hd0.qcow2 hd1.qcow2
   $ qemu-img create -f qcow2 -b hd1.qcow2 hd2.qcow2
   $ $QEMU -drive if=none,file=hd2.qcow2
   (qemu) qemu-io none0 "reopen -o backing.l2-cache-size=2M"
   (qemu) block_stream none0 0 hd0.qcow2
   (qemu) qemu-io none0 "reopen -o backing.l2-cache-size=2M"
   Cannot change the option 'backing.l2-cache-size'

This patch updates the inherits_from pointer if the intermediate nodes
of a backing chain are removed using bdrv_set_backing_hd(), and adds a
test case for this scenario.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>

1 files changed, 21 insertions, 0 deletions

diff --git a/block.c b/block.c
index 9cd6f4a50d..fe0cdc7d99 100644
--- a/block.c
+++ b/block.c
@@ -2260,6 +2260,18 @@ static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
     }
 }
 
+/* Return true if you can reach parent going through child->inherits_from
+ * recursively. If parent or child are NULL, return false */
+static bool bdrv_inherits_from_recursive(BlockDriverState *child,
+                                         BlockDriverState *parent)
+{
+    while (child && child != parent) {
+        child = child->inherits_from;
+    }
+
+    return child != NULL;
+}
+
 /*
  * Sets the backing file link of a BDS. A new reference is created; callers
  * which don't need their own reference any more must call bdrv_unref().
@@ -2267,6 +2279,9 @@ static void bdrv_parent_cb_change_media(BlockDriverState *bs, bool load)
 void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
                          Error **errp)
 {
+    bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
+        bdrv_inherits_from_recursive(backing_hd, bs);
+
     if (backing_hd) {
         bdrv_ref(backing_hd);
     }
@@ -2282,6 +2297,12 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
 
     bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing,
                                     errp);
+    /* If backing_hd was already part of bs's backing chain, and
+     * inherits_from pointed recursively to bs then let's update it to
+     * point directly to bs (else it will become NULL). */
+    if (update_inherits_from) {
+        backing_hd->inherits_from = bs;
+    }
     if (!bs->backing) {
         bdrv_unref(backing_hd);
     }