diff mbox

[v4,5/7] commit: support commit active layer

Message ID 1380542578-2387-6-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng Sept. 30, 2013, 12:02 p.m. UTC
If active is top, it will be mirrored to base, (with block/mirror.c
code), then the image is switched when user completes the block job.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 blockdev.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini Sept. 30, 2013, 12:16 p.m. UTC | #1
Il 30/09/2013 14:02, Fam Zheng ha scritto:
> +        /* Mirror code already swapped bs and base, we drop the bs loop chain
> +         * formed by the swap: break the loop chain, trigger the chain unref.
> +         */
> +        p = info->base->backing_hd;
> +        info->base->backing_hd = NULL;
> +        bdrv_unref(p);

Should this code be in mirror_run? Perhaps extracting it into a new
function together with the preceding lines:

        if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
            bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
        }
        bdrv_swap(s->target, s->common.bs);

Same for this:

> +        if (bdrv_reopen(base_bs, bs->open_flags, &local_err)) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +        bdrv_ref(base_bs);

Perhaps you can have an internal function mirror_start_job and two
front-ends mirror_start + commit_active_start that wrap it.  Similarly,
mirror_start_job can take the base BDS and a bool instead of
MirrorSyncMode (removing the need for MIRROR_SYNC_MODE_COMMON).

But apart from these details, the series looks very nice.

Paolo
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 032913e..f730e6f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1663,6 +1663,30 @@  void qmp_block_stream(const char *device, bool has_base,
     trace_qmp_block_stream(bs, bs->job);
 }
 
+typedef struct {
+    BlockDriverState *bs;
+    BlockDriverState *base;
+} commit_active_info_t;
+
+static void commit_active_cb(void *_info, int ret)
+{
+    commit_active_info_t *info = _info;
+    BlockDriverState *p;
+    assert(info->bs);
+    assert(info->base);
+    assert(info->base->backing_hd);
+    if (!ret) {
+        /* Mirror code already swapped bs and base, we drop the bs loop chain
+         * formed by the swap: break the loop chain, trigger the chain unref.
+         */
+        p = info->base->backing_hd;
+        info->base->backing_hd = NULL;
+        bdrv_unref(p);
+    }
+    block_job_cb(info->bs, ret);
+    g_free(info);
+}
+
 void qmp_block_commit(const char *device,
                       bool has_base, const char *base, const char *top,
                       bool has_speed, int64_t speed,
@@ -1710,8 +1734,22 @@  void qmp_block_commit(const char *device,
         return;
     }
 
-    commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
-                &local_err);
+    if (top_bs == bs) {
+        commit_active_info_t *info = g_malloc(sizeof(commit_active_info_t));
+        info->bs = bs;
+        info->base = base_bs;
+        if (bdrv_reopen(base_bs, bs->open_flags, &local_err)) {
+            error_propagate(errp, local_err);
+            return;
+        }
+        bdrv_ref(base_bs);
+        mirror_start(bs, base_bs, speed, 0, 0, MIRROR_SYNC_MODE_COMMON,
+                     on_error, on_error, true,
+                     commit_active_cb, info, &local_err);
+    } else {
+        commit_start(bs, base_bs, top_bs, speed, on_error, block_job_cb, bs,
+                    &local_err);
+    }
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         return;