[Qemu-devel] [PATCH v18 08/15] block: Support dropping active in bdrv_drop_intermediate

[Fam Zheng]

Dropping intermediate could be useful both for commit and stream, and
BDS refcnt plus bdrv_swap could do most of the job nicely. It also needs
to work with op blockers.

Signed-off-by: Fam Zheng <address@hidden>
---
 block.c        | 139 ++++++++++++++++++++++++++++-----------------------------
 block/commit.c |   2 +-
 2 files changed, 70 insertions(+), 71 deletions(-)

diff --git a/block.c b/block.c
index beacf09..b8e229f 100644
--- a/block.c
+++ b/block.c
@@ -2521,115 +2521,114 @@ BlockDriverState *bdrv_find_overlay(BlockDriverState 
*active,
     return overlay;
 }
 
-typedef struct BlkIntermediateStates {
-    BlockDriverState *bs;
-    QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
-} BlkIntermediateStates;
-
-
 /*
- * Drops images above 'base' up to and including 'top', and sets the image
- * above 'top' to have base as its backing file.
+ * Drops images above 'base' up to and including 'top', and sets new 'base' as
+ * backing_hd of top's overlay (the image orignally has 'top' as backing file).
+ * top's overlay may be NULL if 'top' is active, no such update needed.
+ * Requires that the top's overlay to 'top' is opened r/w.
+ *
+ * 1) This will convert the following chain:
+ *
+ *     ... <- base <- ... <- top <- overlay <-... <- active
  *
- * Requires that the overlay to 'top' is opened r/w, so that the backing file
- * information in 'bs' can be properly updated.
+ * to
+ *
+ *     ... <- base <- overlay <- active
+ *
+ * 2) It is allowed for bottom==base, in which case it converts:
  *
- * E.g., this will convert the following chain:
- * bottom <- base <- intermediate <- top <- active
+ *     base <- ... <- top <- overlay <- ... <- active
  *
  * to
  *
- * bottom <- base <- active
+ *     base <- overlay <- active
  *
- * It is allowed for bottom==base, in which case it converts:
+ * 2) It also allows active==top, in which case it converts:
  *
- * base <- intermediate <- top <- active
+ *     ... <- base <- ... <- top (active)
  *
  * to
  *
- * base <- active
+ *     ... <- base == active == top
+ *
+ * i.e. only base and lower remains: *top == *base when return.
+ *
+ * 3) If base==NULL, it will drop all the BDS below overlay and set its
+ * backing_hd to NULL. I.e.:
  *
- * Error conditions:
- *  if active == top, that is considered an error
+ *     base(NULL) <- ... <- overlay <- ... <- active
+ *
+ * to
+ *
+ *     overlay <- ... <- active
  *
  */
 int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
                            BlockDriverState *base)
 {
-    BlockDriverState *intermediate;
-    BlockDriverState *base_bs = NULL;
-    BlockDriverState *new_top_bs = NULL;
-    BlkIntermediateStates *intermediate_state, *next;
-    int ret = -EIO;
-
-    QSIMPLEQ_HEAD(states_to_delete, BlkIntermediateStates) states_to_delete;
-    QSIMPLEQ_INIT(&states_to_delete);
+    BlockDriverState *drop_start, *overlay, *bs;
+    int ret = -EINVAL;
 
-    if (!top->drv || !base->drv) {
+    assert(active);
+    assert(top);
+    /* Verify that top is in backing chain of active */
+    bs = active;
+    while (bs && bs != top) {
+        bs = bs->backing_hd;
+    }
+    if (!bs) {
         goto exit;
     }
+    /* Verify that base is in backing chain of top */
+    if (base) {
+        while (bs && bs != base) {
+            bs = bs->backing_hd;
+        }
+        if (bs != base) {
+            goto exit;
+        }
+    }
 
-    new_top_bs = bdrv_find_overlay(active, top);
-
-    if (new_top_bs == NULL) {
-        /* we could not find the image above 'top', this is an error */
+    if (!top->drv || (base && !base->drv)) {
         goto exit;
     }
-
-    /* special case of new_top_bs->backing_hd already pointing to base - 
nothing
-     * to do, no intermediate images */
-    if (new_top_bs->backing_hd == base) {
+    if (top == base) {
+        ret = 0;
+        goto exit;
+    } else if (top == active) {
+        assert(base);
+        drop_start = active->backing_hd;
+        bdrv_swap(base, active);
+        bdrv_set_backing_hd(base, NULL);
+        bdrv_unref(drop_start);
         ret = 0;
         goto exit;
     }
 
-    intermediate = top;
-
-    /* now we will go down through the list, and add each BDS we find
-     * into our deletion queue, until we hit the 'base'
-     */
-    while (intermediate) {
-        intermediate_state = g_malloc0(sizeof(BlkIntermediateStates));
-        intermediate_state->bs = intermediate;
-        QSIMPLEQ_INSERT_TAIL(&states_to_delete, intermediate_state, entry);
-
-        if (intermediate->backing_hd == base) {
-            base_bs = intermediate->backing_hd;
-            break;
-        }
-        intermediate = intermediate->backing_hd;
-    }
-    if (base_bs == NULL) {
-        /* something went wrong, we did not end at the base. safely
-         * unravel everything, and exit with error */
+    overlay = bdrv_find_overlay(active, top);
+    if (!overlay) {
         goto exit;
     }
-
-    /* success - we can delete the intermediate states, and link top->base */
-    ret = bdrv_change_backing_file(new_top_bs, base_bs->filename,
-                                   base_bs->drv ? base_bs->drv->format_name : 
"");
+    ret = bdrv_change_backing_file(overlay,
+                                   base ? base->filename : NULL,
+                                   base ? base->drv->format_name : NULL);
     if (ret) {
         goto exit;
     }
-    new_top_bs->backing_hd = base_bs;
 
-    bdrv_refresh_limits(new_top_bs);
-
-    QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
-        /* so that bdrv_close() does not recursively close the chain */
-        intermediate_state->bs->backing_hd = NULL;
-        bdrv_unref(intermediate_state->bs);
+    bs = overlay->backing_hd;
+    bdrv_set_backing_hd(overlay, base);
+    if (base) {
+        bdrv_ref(base);
+    }
+    if (bs) {
+        bdrv_unref(bs);
     }
     ret = 0;
-
 exit:
-    QSIMPLEQ_FOREACH_SAFE(intermediate_state, &states_to_delete, entry, next) {
-        g_free(intermediate_state);
-    }
     return ret;
 }
 
-
 static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
                                    size_t size)
 {
diff --git a/block/commit.c b/block/commit.c
index acec4ac..ac598d6 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -73,6 +73,7 @@ static void coroutine_fn commit_run(void *opaque)
     int bytes_written = 0;
     int64_t base_len;
 
+    overlay_bs = bdrv_find_overlay(active, top);
     ret = s->common.len = bdrv_getlength(top);
 
 
@@ -154,7 +155,6 @@ exit_restore_reopen:
     if (s->base_flags != bdrv_get_flags(base)) {
         bdrv_reopen(base, s->base_flags, NULL);
     }
-    overlay_bs = bdrv_find_overlay(active, top);
     if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) {
         bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
     }
-- 
1.9.1