qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top
@ 2013-10-09  5:19 Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 1/6] mirror: don't close target Fam Zheng
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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.

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 (6):
  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/commit.c            |  8 +----
 block/mirror.c            | 77 +++++++++++++++++++++++++++++++++++++++--------
 blockdev.c                |  9 ++++--
 include/block/block_int.h | 22 ++++++++++++--
 qapi-schema.json          |  5 +--
 tests/qemu-iotests/040    | 73 +++++++++++++++++++-------------------------
 6 files changed, 125 insertions(+), 69 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 1/6] mirror: don't close target
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
@ 2013-10-09  5:19 ` Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 2/6] mirror: move base to MirrorBlockJob Fam Zheng
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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

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

diff --git a/block/mirror.c b/block/mirror.c
index 7b95acf..01a795a 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -479,7 +479,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.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 2/6] mirror: move base to MirrorBlockJob
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 1/6] mirror: don't close target Fam Zheng
@ 2013-10-09  5:19 ` Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 3/6] block: add commit_active_start() Fam Zheng
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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

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

diff --git a/block/mirror.c b/block/mirror.c
index 01a795a..cbdcc21 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;
@@ -334,8 +335,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,
@@ -540,6 +540,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
@@ -562,6 +563,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;
@@ -571,6 +578,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.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 3/6] block: add commit_active_start()
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 1/6] mirror: don't close target Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 2/6] mirror: move base to MirrorBlockJob Fam Zheng
@ 2013-10-09  5:19 ` Fam Zheng
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 4/6] commit: support commit active layer Fam Zheng
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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>
---
 block/mirror.c            | 65 +++++++++++++++++++++++++++++++++++------------
 include/block/block_int.h | 22 +++++++++++++---
 2 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index cbdcc21..9be741a 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;
@@ -333,7 +333,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; ) {
@@ -532,15 +532,25 @@ 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
@@ -563,13 +573,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;
     }
@@ -577,7 +582,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);
@@ -590,3 +595,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 211087a..ba0cc8b 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -359,8 +359,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.
@@ -372,7 +373,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.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 4/6] commit: support commit active layer
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
                   ` (2 preceding siblings ...)
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 3/6] block: add commit_active_start() Fam Zheng
@ 2013-10-09  5:19 ` Fam Zheng
  2013-11-25 10:28   ` Stefan Hajnoczi
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active Fam Zheng
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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 9be741a..86ffac8 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -478,6 +478,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);
@@ -619,6 +626,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 8aa66a9..77cbd1d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1710,8 +1710,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 381ffbf..eb13707 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1910,9 +1910,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 need to complete the job with block-job-complete
+#                    command after getting the ready event. (Since 1.7)
 #
 #
 # @speed:  #optional the maximum speed, in bytes per second
@@ -1922,7 +1924,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.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
                   ` (3 preceding siblings ...)
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 4/6] commit: support commit active layer Fam Zheng
@ 2013-10-09  5:19 ` Fam Zheng
  2013-11-25 10:35   ` Stefan Hajnoczi
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 6/6] commit: remove unused check Fam Zheng
  2013-11-08  3:12 ` [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
  6 siblings, 1 reply; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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 | 73 +++++++++++++++++++++-----------------------------
 1 file changed, 31 insertions(+), 42 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index aad535a..902df0d 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -63,6 +63,28 @@ class ImageCommitTestCase(iotests.QMPTestCase):
             i = i + 512
         file.close()
 
+    def run_commit_test(self, top, base, is_active=False):
+        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
@@ -84,23 +106,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"))
 
@@ -127,10 +133,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, True)
+        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()
@@ -191,23 +196,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"))
 
@@ -234,10 +223,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, True)
+        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()
@@ -253,6 +241,7 @@ class TestSetSpeed(ImageCommitTestCase):
         qemu_img('create', backing_img, str(TestSetSpeed.image_len))
         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 0xef 524288 524288', mid_img)
         self.vm = iotests.VM().add_drive(test_img)
         self.vm.launch()
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PATCH v5 6/6] commit: remove unused check
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
                   ` (4 preceding siblings ...)
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active Fam Zheng
@ 2013-10-09  5:19 ` Fam Zheng
  2013-11-08  3:12 ` [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
  6 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-10-09  5:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

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

Signed-off-by: Fam Zheng <famz@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.3.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top
  2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
                   ` (5 preceding siblings ...)
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 6/6] commit: remove unused check Fam Zheng
@ 2013-11-08  3:12 ` Fam Zheng
  6 siblings, 0 replies; 10+ messages in thread
From: Fam Zheng @ 2013-11-08  3:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, jcody, stefanha

On 10/09/2013 01:19 PM, 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.
>
> 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 (6):
>    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/commit.c            |  8 +----
>   block/mirror.c            | 77 +++++++++++++++++++++++++++++++++++++++--------
>   blockdev.c                |  9 ++++--
>   include/block/block_int.h | 22 ++++++++++++--
>   qapi-schema.json          |  5 +--
>   tests/qemu-iotests/040    | 73 +++++++++++++++++++-------------------------
>   6 files changed, 125 insertions(+), 69 deletions(-)
>

Ping?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v5 4/6] commit: support commit active layer
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 4/6] commit: support commit active layer Fam Zheng
@ 2013-11-25 10:28   ` Stefan Hajnoczi
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-11-25 10:28 UTC (permalink / raw)
  To: Fam Zheng; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

On Wed, Oct 09, 2013 at 01:19:41PM +0800, Fam Zheng wrote:
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 381ffbf..eb13707 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1910,9 +1910,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 need to complete the job with block-job-complete

s/need/needs/
s/with block-job-complete/with the block-job-complete/

> +#                    command after getting the ready event. (Since 1.7)

s/1.7/1.8/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active
  2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active Fam Zheng
@ 2013-11-25 10:35   ` Stefan Hajnoczi
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2013-11-25 10:35 UTC (permalink / raw)
  To: Fam Zheng; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

On Wed, Oct 09, 2013 at 01:19:42PM +0800, Fam Zheng wrote:
> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> index aad535a..902df0d 100755
> --- a/tests/qemu-iotests/040
> +++ b/tests/qemu-iotests/040
> @@ -63,6 +63,28 @@ class ImageCommitTestCase(iotests.QMPTestCase):
>              i = i + 512
>          file.close()
>  
> +    def run_commit_test(self, top, base, is_active=False):

is_active is unused and can be dropped.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-11-25 10:35 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-09  5:19 [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng
2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 1/6] mirror: don't close target Fam Zheng
2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 2/6] mirror: move base to MirrorBlockJob Fam Zheng
2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 3/6] block: add commit_active_start() Fam Zheng
2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 4/6] commit: support commit active layer Fam Zheng
2013-11-25 10:28   ` Stefan Hajnoczi
2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 5/6] qemu-iotests: update test cases for commit active Fam Zheng
2013-11-25 10:35   ` Stefan Hajnoczi
2013-10-09  5:19 ` [Qemu-devel] [PATCH v5 6/6] commit: remove unused check Fam Zheng
2013-11-08  3:12 ` [Qemu-devel] [PATCH v5 0/6] block: allow commit active as top Fam Zheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).