[Qemu-devel] [PATCH 1/5] block: Introduce BdrvChildRole.update_filename

[Kevin Wolf]

There is no good reason for bdrv_drop_intermediate() to know the active
layer above the subchain it is operating on - even more so, because
the assumption that there is a single active layer above it is not
generally true.

In order to prepare removal of the active parameter, use a BdrvChildRole
callback to update the backing file string in the overlay image instead
of directly calling bdrv_change_backing_file().

Signed-off-by: Kevin Wolf <address@hidden>
---
 include/block/block_int.h |  6 ++++++
 block.c                   | 33 ++++++++++++++++++++++++++++-----
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 99abe2ce74..e6d53cc15e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -544,6 +544,12 @@ struct BdrvChildRole {
 
     void (*attach)(BdrvChild *child);
     void (*detach)(BdrvChild *child);
+
+    /* Notifies the parent that the filename of its child has changed (e.g.
+     * because the direct child was removed from the backing chain), so that it
+     * can update its reference. */
+    int (*update_filename)(BdrvChild *child, BlockDriverState *new_base,
+                           const char *filename, Error **errp);
 };
 
 extern const BdrvChildRole child_file;
diff --git a/block.c b/block.c
index 5c65fac672..7ac8cd521b 100644
--- a/block.c
+++ b/block.c
@@ -981,6 +981,21 @@ static void bdrv_backing_options(int *child_flags, QDict 
*child_options,
     *child_flags = flags;
 }
 
+static int bdrv_backing_update_filename(BdrvChild *c, BlockDriverState *base,
+                                        const char *filename, Error **errp)
+{
+    BlockDriverState *parent = c->opaque;
+    int ret;
+
+    ret = bdrv_change_backing_file(parent, filename,
+                                   base->drv ? base->drv->format_name : "");
+    if (ret < 0) {
+        error_setg_errno(errp, ret, "Could not update backing file link");
+    }
+
+    return ret;
+}
+
 const BdrvChildRole child_backing = {
     .get_parent_desc = bdrv_child_get_parent_desc,
     .attach          = bdrv_backing_attach,
@@ -989,6 +1004,7 @@ const BdrvChildRole child_backing = {
     .drained_begin   = bdrv_child_cb_drained_begin,
     .drained_end     = bdrv_child_cb_drained_end,
     .inactivate      = bdrv_child_cb_inactivate,
+    .update_filename = bdrv_backing_update_filename,
 };
 
 static int bdrv_open_flags(BlockDriverState *bs, int flags)
@@ -3470,6 +3486,8 @@ int bdrv_drop_intermediate(BlockDriverState *active, 
BlockDriverState *top,
     Error *local_err = NULL;
     int ret = -EIO;
 
+    bdrv_ref(top);
+
     if (!top->drv || !base->drv) {
         goto exit;
     }
@@ -3494,11 +3512,15 @@ int bdrv_drop_intermediate(BlockDriverState *active, 
BlockDriverState *top,
     }
 
     /* success - we can delete the intermediate states, and link top->base */
-    backing_file_str = backing_file_str ? backing_file_str : base->filename;
-    ret = bdrv_change_backing_file(new_top_bs, backing_file_str,
-                                   base->drv ? base->drv->format_name : "");
-    if (ret) {
-        goto exit;
+    if (new_top_bs->backing->role->update_filename) {
+        backing_file_str = backing_file_str ? backing_file_str : 
base->filename;
+        ret = new_top_bs->backing->role->update_filename(new_top_bs->backing,
+                                                         base, 
backing_file_str,
+                                                         &local_err);
+        if (ret < 0) {
+            bdrv_set_backing_hd(new_top_bs, top, &error_abort);
+            goto exit;
+        }
     }
 
     bdrv_set_backing_hd(new_top_bs, base, &local_err);
@@ -3510,6 +3532,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, 
BlockDriverState *top,
 
     ret = 0;
 exit:
+    bdrv_unref(top);
     return ret;
 }
 
-- 
2.13.5