[Qemu-devel] [PATCH 3/8] block: Let bdrv_open_inherit() return the snapshot

[Max Reitz]

If bdrv_open_inherit() creates a snapshot BDS and *pbs is NULL, that
snapshot BDS should be returned instead of the BDS under it.

To this end, bdrv_append_temp_snapshot() now returns the snapshot BDS
instead of just appending it on top of the snapshotted BDS, and that
function is made static since bdrv_open_inherit() is its only user
anyway. Also, it calls bdrv_ref() before bdrv_append() (which
bdrv_open_inherit() has to undo if not returning the overlay).

Signed-off-by: Max Reitz <address@hidden>
---
 block.c               | 27 +++++++++++++++++++++++----
 include/block/block.h |  1 -
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index aae1d69..c5ea5e7 100644
--- a/block.c
+++ b/block.c
@@ -1295,7 +1295,8 @@ done:
     return c;
 }
 
-int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
+static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs,
+                                                   int flags, Error **errp)
 {
     /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
     char *tmp_filename = g_malloc0(PATH_MAX + 1);
@@ -1354,11 +1355,15 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int 
flags, Error **errp)
         goto out;
     }
 
+    bdrv_ref(bs_snapshot);
     bdrv_append(bs_snapshot, bs);
 
+    g_free(tmp_filename);
+    return bs_snapshot;
+
 out:
     g_free(tmp_filename);
-    return ret;
+    return NULL;
 }
 
 /*
@@ -1557,15 +1562,29 @@ static int bdrv_open_inherit(BlockDriverState **pbs, 
const char *filename,
     }
 
     QDECREF(options);
-    *pbs = bs;
 
     /* For snapshot=on, create a temporary qcow2 overlay. bs points to the
      * temporary snapshot afterwards. */
     if (snapshot_flags) {
-        ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
+        BlockDriverState *snapshot_bs;
+        snapshot_bs = bdrv_append_temp_snapshot(bs, snapshot_flags, 
&local_err);
         if (local_err) {
+            ret = -EINVAL;
             goto close_and_fail;
         }
+        if (!*pbs) {
+            /* The reference is now held by the overlay BDS */
+            bdrv_unref(bs);
+            bs = snapshot_bs;
+        } else {
+            /* It is still referenced in the same way that *pbs was referenced,
+             * however that may be */
+            bdrv_unref(snapshot_bs);
+        }
+    }
+
+    if (!*pbs) {
+        *pbs = bs;
     }
 
     return 0;
diff --git a/include/block/block.h b/include/block/block.h
index 1abfc70..37e49a1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -210,7 +210,6 @@ BdrvChild *bdrv_open_child(const char *filename,
                            bool allow_none, Error **errp);
 void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
-int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
 int bdrv_open(BlockDriverState **pbs, const char *filename,
               const char *reference, QDict *options, int flags, Error **errp);
 BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
-- 
2.6.2