qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 04/10] block: support dropping active in bdrv


From: Fam Zheng
Subject: Re: [Qemu-devel] [PATCH v7 04/10] block: support dropping active in bdrv_drop_intermediate
Date: Fri, 13 Dec 2013 11:30:30 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1

On 2013年12月12日 21:24, Kevin Wolf wrote:
Am 12.12.2013 um 09:23 hat Fam Zheng geschrieben:
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 need
some improvements in preparation for op blockers.

Signed-off-by: Fam Zheng <address@hidden>
---
  block.c        | 152 +++++++++++++++++++++++++++------------------------------
  block/commit.c |   1 +
  2 files changed, 74 insertions(+), 79 deletions(-)

diff --git a/block.c b/block.c
index 41562fd..681d3be 100644
--- a/block.c
+++ b/block.c
@@ -2163,114 +2163,108 @@ BlockDriverState *bdrv_find_overlay(BlockDriverState 
*active,
      return overlay;
  }

-typedef struct BlkIntermediateStates {
-    BlockDriverState *bs;
-    QSIMPLEQ_ENTRY(BlkIntermediateStates) entry;
-} BlkIntermediateStates;
-
+static void bdrv_set_backing_hd(BlockDriverState *bs,
+                                BlockDriverState *new_backing)
+{
+    if (bs->backing_hd) {
+        bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
+    }
+    bs->backing_hd = new_backing;
+    if (new_backing) {
+        bdrv_op_block_all(bs->backing_hd, bs->backing_blocker);

What about unblocking commit, like you did in patch 3?

Should bdrv_open_backing_file() be using this function?


Yes, will do that.

+    }
+}

  /*
- * 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_overlay (the image orignally has 'top' as backing
+ * file). top_overlay may be NULL if 'top' is active, no such update needed.
+ * Requires that the top_overlay to 'top' is opened r/w.
   *
- * Requires that the overlay to 'top' is opened r/w, so that the backing file
- * information in 'bs' can be properly updated.
+ * 1) This will convert the following chain:
+ * ... <- base <- ... <- top <- overlay <-... <- active
   *
- * E.g., this will convert the following chain:
- * bottom <- base <- intermediate <- top <- active
+ * to
+ *
+ * ... <- base <- overlay <- active
+ *
+ * 2) It is allowed for bottom==base, in which case it converts:
+ *
+ * ... <- 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);
-
-    if (!top->drv || !base->drv) {
-        goto exit;
-    }
+    BlockDriverState *drop_start, *overlay;
+    int ret = -EINVAL;

-    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;
-    }
-
-    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;
+    } else if (top == active) {
+        assert(base);
+        drop_start = active->backing_hd;
+        bdrv_swap(active, base);
+        base->backing_hd = NULL;
+        bdrv_unref(drop_start);
+        ret = 0;
+    } else {
+        /* If there's an overlay, its backing_hd points to top's BDS now,
+         * the top image is dropped but this BDS structure is kept and swapped
+         * with base, this way we keep the pointers valid after dropping top */
+        overlay = bdrv_find_overlay(active, top);
+        if (!overlay) {
+            goto exit;
+        }
+        if (base) {
+            ret = bdrv_change_backing_file(overlay, base->filename,
+                                           base->drv->format_name);
+        } else {
+            ret = bdrv_change_backing_file(overlay, "", "");

This should be NULL, NULL instead of empty strings.


OK, thanks.

Fam




reply via email to

[Prev in Thread] Current Thread [Next in Thread]