[Qemu-devel] [PATCH v7 0/7] block: allow commit active as top

[Qemu-devel] [PATCH v7 0/7] block: allow commit active as top

[Fam Zheng]

Previously live commit of active block device is not supported, this series
implements it and updates corresponding qemu-iotests cases.

This series is based on BlockJobType enum QAPI series.

v7: Fix "Since 1.8" to "Since 2.0".
    Rebase patch 05 to master.

v6: Address comments from Stefan:

    [04/06] commit: support commit active layer
            Fix wording.
    [05/06] qemu-iotests: update test cases for commit active
            Drop is_active.

    Experimental for reviewers: the side by side diff against previous series:

        http://goo.gl/vgN6mc

v5: Address comments from Eric and Paolo:
    Add mirror_start_job and front end wrapper. [Paolo]
    Base on BlockJobType enum in QAPI. [Eric]
    Drop "common" sync mode. [Eric]

v4: Rewrite to reuse block/mirror.c.
    When committing the active layer, the job is internally a mirror job with
    type name faked to "commit".
    When the job completes, the BDSes are swapped, so the base image become
    active and [top, base) dropped.


Fam Zheng (7):
  blkdebug: Use QLIST_FOREACH_SAFE to resume IO
  mirror: Don't close target
  mirror: Move base to MirrorBlockJob
  block: Add commit_active_start()
  commit: Support commit active layer
  qemu-iotests: Update test cases for commit active
  commit: Remove unused check

 block/blkdebug.c          |  8 ++---
 block/commit.c            |  8 +----
 block/mirror.c            | 78 +++++++++++++++++++++++++++++++++++++++--------
 blockdev.c                |  9 ++++--
 include/block/block_int.h | 22 +++++++++++--
 qapi-schema.json          |  5 +--
 tests/qemu-iotests/040    | 74 +++++++++++++++++++-------------------------
 7 files changed, 131 insertions(+), 73 deletions(-)

-- 
1.8.5.1

[Qemu-devel] [PATCH v7 1/7] blkdebug: Use QLIST_FOREACH_SAFE to resume IO

[Fam Zheng]

Qemu-iotest 030 was broken.

When the coroutine runs and finishes, it will remove itself from the req
list, so let's use safe version of foreach to avoid use after free.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/blkdebug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 37cf028..957be2c 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -594,9 +594,9 @@ static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event,
 static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugSuspendedReq *r;
+    BlkdebugSuspendedReq *r, *next;
 
-    QLIST_FOREACH(r, &s->suspended_reqs, next) {
+    QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, next) {
         if (!strcmp(r->tag, tag)) {
             qemu_coroutine_enter(r->co, NULL);
             return 0;
@@ -609,7 +609,7 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
                                             const char *tag)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugSuspendedReq *r;
+    BlkdebugSuspendedReq *r, *r_next;
     BlkdebugRule *rule, *next;
     int i, ret = -ENOENT;
 
@@ -622,7 +622,7 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
             }
         }
     }
-    QLIST_FOREACH(r, &s->suspended_reqs, next) {
+    QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, r_next) {
         if (!strcmp(r->tag, tag)) {
             qemu_coroutine_enter(r->co, NULL);
             ret = 0;
-- 
1.8.5.1

[Qemu-devel] [PATCH v7 2/7] mirror: Don't close target

[Fam Zheng]

Let reference count manage target and don't call bdrv_close here.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/mirror.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 6dc27ad..5b2c119 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -482,7 +482,6 @@ immediate_exit:
         }
         bdrv_swap(s->target, s->common.bs);
     }
-    bdrv_close(s->target);
     bdrv_unref(s->target);
     block_job_completed(&s->common, ret);
 }
-- 
1.8.5.1

[Qemu-devel] [PATCH v7 3/7] mirror: Move base to MirrorBlockJob

[Fam Zheng]

This allows setting the base before entering mirror_run, commit will
make use of it.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/mirror.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 5b2c119..605dda6 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -31,6 +31,7 @@ typedef struct MirrorBlockJob {
     BlockJob common;
     RateLimit limit;
     BlockDriverState *target;
+    BlockDriverState *base;
     MirrorSyncMode mode;
     BlockdevOnError on_source_error, on_target_error;
     bool synced;
@@ -337,8 +338,7 @@ static void coroutine_fn mirror_run(void *opaque)
 
     if (s->mode != MIRROR_SYNC_MODE_NONE) {
         /* First part, loop on the sectors and initialize the dirty bitmap.  */
-        BlockDriverState *base;
-        base = s->mode == MIRROR_SYNC_MODE_FULL ? NULL : bs->backing_hd;
+        BlockDriverState *base = s->base;
         for (sector_num = 0; sector_num < end; ) {
             int64_t next = (sector_num | (sectors_per_chunk - 1)) + 1;
             ret = bdrv_is_allocated_above(bs, base,
@@ -543,6 +543,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
                   void *opaque, Error **errp)
 {
     MirrorBlockJob *s;
+    BlockDriverState *base = NULL;
 
     if (granularity == 0) {
         /* Choose the default granularity based on the target file's cluster
@@ -565,6 +566,12 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    if (mode == MIRROR_SYNC_MODE_TOP) {
+        base = bs->backing_hd;
+    } else {
+        base = NULL;
+    }
+
     s = block_job_create(&mirror_job_driver, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
@@ -574,6 +581,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     s->on_target_error = on_target_error;
     s->target = target;
     s->mode = mode;
+    s->base = base;
     s->granularity = granularity;
     s->buf_size = MAX(buf_size, granularity);
 
-- 
1.8.5.1

[Qemu-devel] [PATCH v7 4/7] block: Add commit_active_start()

[Fam Zheng]

commit_active_start is implemented in block/mirror.c, It will create a
job with "commit" type and designated base in block-commit command. This
will be used for committing active layer of device.

Sync mode is removed from MirrorBlockJob because there's no proper type
for commit. The used information is is_none_mode.

The common part of mirror_start and commit_active_start is moved to
mirror_start_job().

Fix the comment wording for commit_start.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/mirror.c            | 66 +++++++++++++++++++++++++++++++++++------------
 include/block/block_int.h | 22 +++++++++++++---
 2 files changed, 69 insertions(+), 19 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 605dda6..04af341 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -32,7 +32,7 @@ typedef struct MirrorBlockJob {
     RateLimit limit;
     BlockDriverState *target;
     BlockDriverState *base;
-    MirrorSyncMode mode;
+    bool is_none_mode;
     BlockdevOnError on_source_error, on_target_error;
     bool synced;
     bool should_complete;
@@ -336,7 +336,7 @@ static void coroutine_fn mirror_run(void *opaque)
     sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
     mirror_free_init(s);
 
-    if (s->mode != MIRROR_SYNC_MODE_NONE) {
+    if (!s->is_none_mode) {
         /* First part, loop on the sectors and initialize the dirty bitmap.  */
         BlockDriverState *base = s->base;
         for (sector_num = 0; sector_num < end; ) {
@@ -535,15 +535,26 @@ static const BlockJobDriver mirror_job_driver = {
     .complete      = mirror_complete,
 };
 
-void mirror_start(BlockDriverState *bs, BlockDriverState *target,
-                  int64_t speed, int64_t granularity, int64_t buf_size,
-                  MirrorSyncMode mode, BlockdevOnError on_source_error,
-                  BlockdevOnError on_target_error,
-                  BlockDriverCompletionFunc *cb,
-                  void *opaque, Error **errp)
+static const BlockJobDriver commit_active_job_driver = {
+    .instance_size = sizeof(MirrorBlockJob),
+    .job_type      = BLOCK_JOB_TYPE_COMMIT,
+    .set_speed     = mirror_set_speed,
+    .iostatus_reset
+                   = mirror_iostatus_reset,
+    .complete      = mirror_complete,
+};
+
+static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
+                            int64_t speed, int64_t granularity,
+                            int64_t buf_size,
+                            BlockdevOnError on_source_error,
+                            BlockdevOnError on_target_error,
+                            BlockDriverCompletionFunc *cb,
+                            void *opaque, Error **errp,
+                            const BlockJobDriver *driver,
+                            bool is_none_mode, BlockDriverState *base)
 {
     MirrorBlockJob *s;
-    BlockDriverState *base = NULL;
 
     if (granularity == 0) {
         /* Choose the default granularity based on the target file's cluster
@@ -566,13 +577,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
-    if (mode == MIRROR_SYNC_MODE_TOP) {
-        base = bs->backing_hd;
-    } else {
-        base = NULL;
-    }
 
-    s = block_job_create(&mirror_job_driver, bs, speed, cb, opaque, errp);
+    s = block_job_create(driver, bs, speed, cb, opaque, errp);
     if (!s) {
         return;
     }
@@ -580,7 +586,7 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     s->on_source_error = on_source_error;
     s->on_target_error = on_target_error;
     s->target = target;
-    s->mode = mode;
+    s->is_none_mode = is_none_mode;
     s->base = base;
     s->granularity = granularity;
     s->buf_size = MAX(buf_size, granularity);
@@ -593,3 +599,31 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     trace_mirror_start(bs, s, s->common.co, opaque);
     qemu_coroutine_enter(s->common.co, s);
 }
+
+void mirror_start(BlockDriverState *bs, BlockDriverState *target,
+                  int64_t speed, int64_t granularity, int64_t buf_size,
+                  MirrorSyncMode mode, BlockdevOnError on_source_error,
+                  BlockdevOnError on_target_error,
+                  BlockDriverCompletionFunc *cb,
+                  void *opaque, Error **errp)
+{
+    bool is_none_mode;
+    BlockDriverState *base;
+
+    is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
+    base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
+    mirror_start_job(bs, target, speed, granularity, buf_size,
+                     on_source_error, on_target_error, cb, opaque, errp,
+                     &mirror_job_driver, is_none_mode, base);
+}
+
+void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
+                         int64_t speed,
+                         BlockdevOnError on_error,
+                         BlockDriverCompletionFunc *cb,
+                         void *opaque, Error **errp)
+{
+    mirror_start_job(bs, base, speed, 0, 0,
+                     on_error, on_error, cb, opaque, errp,
+                     &commit_active_job_driver, false, base);
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8b132d7..2772f2f 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -394,8 +394,9 @@ void stream_start(BlockDriverState *bs, BlockDriverState *base,
 
 /**
  * commit_start:
- * @bs: Top Block device
- * @base: Block device that will be written into, and become the new top
+ * @bs: Active block device.
+ * @top: Top block device to be committed.
+ * @base: Block device that will be written into, and become the new top.
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
  * @on_error: The action to take upon error.
  * @cb: Completion function for the job.
@@ -407,7 +408,22 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
                  BlockDriverState *top, int64_t speed,
                  BlockdevOnError on_error, BlockDriverCompletionFunc *cb,
                  void *opaque, Error **errp);
-
+/**
+ * commit_active_start:
+ * @bs: Active block device to be committed.
+ * @base: Block device that will be written into, and become the new top.
+ * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
+ * @on_error: The action to take upon error.
+ * @cb: Completion function for the job.
+ * @opaque: Opaque pointer value passed to @cb.
+ * @errp: Error object.
+ *
+ */
+void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
+                         int64_t speed,
+                         BlockdevOnError on_error,
+                         BlockDriverCompletionFunc *cb,
+                         void *opaque, Error **errp);
 /*
  * mirror_start:
  * @bs: Block device to operate on.
-- 
1.8.5.1

[Qemu-devel] [PATCH v7 5/7] commit: Support commit active layer

[Fam Zheng]

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.

QMP documentation is updated.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/mirror.c   | 11 +++++++++++
 blockdev.c       |  9 +++++++--
 qapi-schema.json |  5 +++--
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 04af341..2932bab 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -481,6 +481,13 @@ immediate_exit:
             bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
         }
         bdrv_swap(s->target, s->common.bs);
+        if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
+            /* drop the bs loop chain formed by the swap: break the loop then
+             * trigger the unref from the top one */
+            BlockDriverState *p = s->base->backing_hd;
+            s->base->backing_hd = NULL;
+            bdrv_unref(p);
+        }
     }
     bdrv_unref(s->target);
     block_job_completed(&s->common, ret);
@@ -623,6 +630,10 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
                          BlockDriverCompletionFunc *cb,
                          void *opaque, Error **errp)
 {
+    if (bdrv_reopen(base, bs->open_flags, errp)) {
+        return;
+    }
+    bdrv_ref(base);
     mirror_start_job(bs, base, speed, 0, 0,
                      on_error, on_error, cb, opaque, errp,
                      &commit_active_job_driver, false, base);
diff --git a/blockdev.c b/blockdev.c
index 44755e1..db8c86e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1820,8 +1820,13 @@ 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_start(bs, base_bs, speed, on_error, block_job_cb,
+                            bs, &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;
diff --git a/qapi-schema.json b/qapi-schema.json
index d6f8615..4342abc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1967,9 +1967,11 @@
 #
 # @top:              The file name of the backing image within the image chain,
 #                    which contains the topmost data to be committed down.
-#                    Note, the active layer as 'top' is currently unsupported.
 #
 #                    If top == base, that is an error.
+#                    If top == active, the job will not be completed by itself,
+#                    user needs to complete the job with the block-job-complete
+#                    command after getting the ready event. (Since 2.0)
 #
 #
 # @speed:  #optional the maximum speed, in bytes per second
@@ -1979,7 +1981,6 @@
 #          If @device does not exist, DeviceNotFound
 #          If image commit is not supported by this device, NotSupported
 #          If @base or @top is invalid, a generic error is returned
-#          If @top is the active layer, or omitted, a generic error is returned
 #          If @speed is invalid, InvalidParameter
 #
 # Since: 1.3
-- 
1.8.5.1

[Qemu-devel] [PATCH v7 6/7] qemu-iotests: Update test cases for commit active

[Fam Zheng]

Factor out commit test common logic into super class, and update test
of committing the active image.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/040 | 74 ++++++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 42 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 18dcd61..72eaad5 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -39,6 +39,29 @@ class ImageCommitTestCase(iotests.QMPTestCase):
         result = self.vm.qmp('query-block-jobs')
         self.assert_qmp(result, 'return', [])
 
+    def run_commit_test(self, top, base):
+        self.assert_no_active_commit()
+        result = self.vm.qmp('block-commit', device='drive0', top=top, base=base)
+        self.assert_qmp(result, 'return', {})
+
+        completed = False
+        while not completed:
+            for event in self.vm.get_qmp_events(wait=True):
+                if event['event'] == 'BLOCK_JOB_COMPLETED':
+                    self.assert_qmp(event, 'data/type', 'commit')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/offset', self.image_len)
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    completed = True
+                elif event['event'] == 'BLOCK_JOB_READY':
+                    self.assert_qmp(event, 'data/type', 'commit')
+                    self.assert_qmp(event, 'data/device', 'drive0')
+                    self.assert_qmp(event, 'data/len', self.image_len)
+                    self.vm.qmp('block-job-complete', device='drive0')
+
+        self.assert_no_active_commit()
+        self.vm.shutdown()
+
 class TestSingleDrive(ImageCommitTestCase):
     image_len = 1 * 1024 * 1024
     test_len = 1 * 1024 * 256
@@ -59,23 +82,7 @@ class TestSingleDrive(ImageCommitTestCase):
         os.remove(backing_img)
 
     def test_commit(self):
-        self.assert_no_active_commit()
-        result = self.vm.qmp('block-commit', device='drive0', top='%s' % mid_img)
-        self.assert_qmp(result, 'return', {})
-
-        completed = False
-        while not completed:
-            for event in self.vm.get_qmp_events(wait=True):
-                if event['event'] == 'BLOCK_JOB_COMPLETED':
-                    self.assert_qmp(event, 'data/type', 'commit')
-                    self.assert_qmp(event, 'data/device', 'drive0')
-                    self.assert_qmp(event, 'data/offset', self.image_len)
-                    self.assert_qmp(event, 'data/len', self.image_len)
-                    completed = True
-
-        self.assert_no_active_commit()
-        self.vm.shutdown()
-
+        self.run_commit_test(mid_img, backing_img)
         self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
         self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
 
@@ -102,10 +109,9 @@ class TestSingleDrive(ImageCommitTestCase):
         self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
 
     def test_top_is_active(self):
-        self.assert_no_active_commit()
-        result = self.vm.qmp('block-commit', device='drive0', top='%s' % test_img, base='%s' % backing_img)
-        self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Top image as the active layer is currently unsupported')
+        self.run_commit_test(test_img, backing_img)
+        self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
+        self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
 
     def test_top_and_base_reversed(self):
         self.assert_no_active_commit()
@@ -166,23 +172,7 @@ class TestRelativePaths(ImageCommitTestCase):
                 raise
 
     def test_commit(self):
-        self.assert_no_active_commit()
-        result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.mid_img)
-        self.assert_qmp(result, 'return', {})
-
-        completed = False
-        while not completed:
-            for event in self.vm.get_qmp_events(wait=True):
-                if event['event'] == 'BLOCK_JOB_COMPLETED':
-                    self.assert_qmp(event, 'data/type', 'commit')
-                    self.assert_qmp(event, 'data/device', 'drive0')
-                    self.assert_qmp(event, 'data/offset', self.image_len)
-                    self.assert_qmp(event, 'data/len', self.image_len)
-                    completed = True
-
-        self.assert_no_active_commit()
-        self.vm.shutdown()
-
+        self.run_commit_test(self.mid_img, self.backing_img)
         self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
         self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
 
@@ -209,10 +199,9 @@ class TestRelativePaths(ImageCommitTestCase):
         self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
 
     def test_top_is_active(self):
-        self.assert_no_active_commit()
-        result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.test_img, base='%s' % self.backing_img)
-        self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Top image as the active layer is currently unsupported')
+        self.run_commit_test(self.test_img, self.backing_img)
+        self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
+        self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
 
     def test_top_and_base_reversed(self):
         self.assert_no_active_commit()
@@ -229,6 +218,7 @@ class TestSetSpeed(ImageCommitTestCase):
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
         qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
         qemu_io('-c', 'write -P 0x1 0 512', test_img)
+        qemu_io('-c', 'write -P 0xef 524288 524288', mid_img)
         self.vm = iotests.VM().add_drive(test_img)
         self.vm.launch()
 
-- 
1.8.5.1

[Qemu-devel] [PATCH v7 7/7] commit: Remove unused check

[Fam Zheng]

We support top == active for commit now, remove the check and add an
assertion here.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 block/commit.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index d4090cb..acec4ac 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -198,13 +198,7 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base,
         return;
     }
 
-    /* Once we support top == active layer, remove this check */
-    if (top == bs) {
-        error_setg(errp,
-                   "Top image as the active layer is currently unsupported");
-        return;
-    }
-
+    assert(top != bs);
     if (top == base) {
         error_setg(errp, "Invalid files for merge: top and base are the same");
         return;
-- 
1.8.5.1

Re: [Qemu-devel] [PATCH v7 1/7] blkdebug: Use QLIST_FOREACH_SAFE to resume IO

[Fam Zheng]

On 2013年12月16日 14:45, Fam Zheng wrote:
> Qemu-iotest 030 was broken.
>
> When the coroutine runs and finishes, it will remove itself from the req
> list, so let's use safe version of foreach to avoid use after free.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---

This one is already applied to Kevin's tree, please skip it.

Fam

Re: [Qemu-devel] [PATCH v7 0/7] block: allow commit active as top

[Stefan Hajnoczi]

On Mon, Dec 16, 2013 at 02:45:26PM +0800, Fam Zheng wrote:
> Previously live commit of active block device is not supported, this series
> implements it and updates corresponding qemu-iotests cases.
> 
> This series is based on BlockJobType enum QAPI series.
> 
> v7: Fix "Since 1.8" to "Since 2.0".
>     Rebase patch 05 to master.
> 
> v6: Address comments from Stefan:
> 
>     [04/06] commit: support commit active layer
>             Fix wording.
>     [05/06] qemu-iotests: update test cases for commit active
>             Drop is_active.
> 
>     Experimental for reviewers: the side by side diff against previous series:
> 
>         http://goo.gl/vgN6mc
> 
> v5: Address comments from Eric and Paolo:
>     Add mirror_start_job and front end wrapper. [Paolo]
>     Base on BlockJobType enum in QAPI. [Eric]
>     Drop "common" sync mode. [Eric]
> 
> v4: Rewrite to reuse block/mirror.c.
>     When committing the active layer, the job is internally a mirror job with
>     type name faked to "commit".
>     When the job completes, the BDSes are swapped, so the base image become
>     active and [top, base) dropped.
> 
> 
> Fam Zheng (7):
>   blkdebug: Use QLIST_FOREACH_SAFE to resume IO
>   mirror: Don't close target
>   mirror: Move base to MirrorBlockJob
>   block: Add commit_active_start()
>   commit: Support commit active layer
>   qemu-iotests: Update test cases for commit active
>   commit: Remove unused check
> 
>  block/blkdebug.c          |  8 ++---
>  block/commit.c            |  8 +----
>  block/mirror.c            | 78 +++++++++++++++++++++++++++++++++++++++--------
>  blockdev.c                |  9 ++++--
>  include/block/block_int.h | 22 +++++++++++--
>  qapi-schema.json          |  5 +--
>  tests/qemu-iotests/040    | 74 +++++++++++++++++++-------------------------
>  7 files changed, 131 insertions(+), 73 deletions(-)
> 
> -- 
> 1.8.5.1
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan