Skip to contentSkip to conversation

Various block-stream fixes

PATCH Applied Alberto Garcia 2 participants 5 messages Mar 21, 2016 – Mar 23, 2016 Block layer

lore
Share & tools
lore

Alberto Garcia

author
#0 lore raw

v2 cover Various block-stream fixes

This series is a follow-up to the patch I sent last week[1]. It contains a few fixes from my intermediate block streaming branch, but are independent from that feature, so they can be applied now without problems.

Berto

[1] https://lists.gnu.org/archive/html/qemu-block/2016-03/msg00551.html

Alberto Garcia (3):
block: never cancel a streaming job without running stream_complete()
qemu-iotests: fix test_stream_partial()
qemu-iotests: add no-op streaming test

 block/stream.c             | 11 ++++++-----
 tests/qemu-iotests/030     | 21 ++++++++++++++++++++-
 tests/qemu-iotests/030.out |  4 ++--
 3 files changed, 28 insertions(+), 8 deletions(-)
Signature · 1 line
2.8.0.rc3
author
#3 lore raw

PATCH v2 3/3 qemu-iotests: add no-op streaming test

This patch tests that in a partial block-stream operation, no data is ever copied from the base image.

Signed-off-by Alberto Garcia <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 tests/qemu-iotests/030     | 18 ++++++++++++++++++
 tests/qemu-iotests/030.out |  4 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)
tests/qemu-iotests/030 +18 -0
index 48a924c..3ac2443 100755
@@ -91,6 +91,24 @@ class TestSingleDrive(iotests.QMPTestCase):
                          qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
                          'image file map does not match backing file after streaming')
 
+    def test_stream_no_op(self):
+        self.assert_no_active_block_jobs()
+
+        # The image map is empty before the operation
+        empty_map = qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img)
+
+        # This is a no-op: no data should ever be copied from the base image
+        result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
+        self.assert_qmp(result, 'return', {})
+
+        self.wait_until_completed()
+
+        self.assert_no_active_block_jobs()
+        self.vm.shutdown()
+
+        self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
+                         empty_map, 'image file map changed after a no-op')
+
     def test_stream_partial(self):
         self.assert_no_active_block_jobs()
tests/qemu-iotests/030.out +2 -2
index fa16b5c..6323079 100644
@@ -1,5 +1,5 @@
-.............
+..............
 ----------------------------------------------------------------------
-Ran 13 tests
+Ran 14 tests
 
 OK
Signature · 1 line
2.8.0.rc3
author
#2 lore raw

PATCH v2 2/3 qemu-iotests: fix test_stream_partial()

In reply to Alberto Garcia

This test is streaming to the top layer using the intermediate image as the base. This is a mistake since block-stream never copies data from the base image and its backing chain, so this is effectively a no-op.

In addition to fixing the base parameter, this patch also writes some data to the intermediate image before the test, so there's something to copy and the test is meaningful.

Signed-off-by Alberto Garcia <[email protected]>
Reviewed-by Max Reitz <[email protected]>
---
 tests/qemu-iotests/030 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
tests/qemu-iotests/030 +2 -1
index 32469ef..48a924c 100755
@@ -35,6 +35,7 @@ class TestSingleDrive(iotests.QMPTestCase):
         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('-f', 'raw', '-c', 'write -P 0x1 0 512', backing_img)
+        qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 524288 512', mid_img)
         self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
         self.vm.launch()
 

@@ -93,7 +94,7 @@ class TestSingleDrive(iotests.QMPTestCase):
     def test_stream_partial(self):
         self.assert_no_active_block_jobs()
 
-        result = self.vm.qmp('block-stream', device='drive0', base=mid_img)
+        result = self.vm.qmp('block-stream', device='drive0', base=backing_img)
         self.assert_qmp(result, 'return', {})
 
         self.wait_until_completed()
Signature · 1 line
2.8.0.rc3
author
#1 lore raw

PATCH v2 1/3 block: never cancel a streaming job without running stream_complete()

In reply to Alberto Garcia

We need to call stream_complete() in order to do all the necessary clean-ups, even if there's an early failure. At the moment it's only useful to make sure that s->backing_file_str is not leaked, but it will become more important if we introduce support for streaming to any intermediate node.

Signed-off-by Alberto Garcia <[email protected]>
Reviewed-by Max Reitz <[email protected]>
Reviewed-by Fam Zheng <[email protected]>
Reviewed-by Stefan Hajnoczi <[email protected]>
---
 block/stream.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)
block/stream.c +6 -5
index cafaa07..eea3938 100644
@@ -89,21 +89,21 @@ static void coroutine_fn stream_run(void *opaque)
     StreamCompleteData *data;
     BlockDriverState *bs = s->common.bs;
     BlockDriverState *base = s->base;
-    int64_t sector_num, end;
+    int64_t sector_num = 0;
+    int64_t end = -1;
     int error = 0;
     int ret = 0;
     int n = 0;
     void *buf;
 
     if (!bs->backing) {
-        block_job_completed(&s->common, 0);
-        return;
+        goto out;
     }
 
     s->common.len = bdrv_getlength(bs);
     if (s->common.len < 0) {
-        block_job_completed(&s->common, s->common.len);
-        return;
+        ret = s->common.len;
+        goto out;
     }
 
     end = s->common.len >> BDRV_SECTOR_BITS;

@@ -190,6 +190,7 @@ wait:
 
     qemu_vfree(buf);
 
+out:
     /* Modify backing chain and close BDSes in main loop */
     data = g_malloc(sizeof(*data));
     data->ret = ret;
Signature · 1 line
2.8.0.rc3
Applied
#4 lore raw
In reply to Alberto Garcia
Quoted 14 lines · Alberto Garcia

This series is a follow-up to the patch I sent last week[1]. It contains a few fixes from my intermediate block streaming branch, but are independent from that feature, so they can be applied now without problems.

Berto

[1] https://lists.gnu.org/archive/html/qemu-block/2016-03/msg00551.html

Alberto Garcia (3):
block: never cancel a streaming job without running stream_complete()
qemu-iotests: fix test_stream_partial()
qemu-iotests: add no-op streaming test

Quoted 9 lines
 block/stream.c             | 11 ++++++-----
 tests/qemu-iotests/030     | 21 ++++++++++++++++++++-
 tests/qemu-iotests/030.out |  4 ++--
 3 files changed, 28 insertions(+), 8 deletions(-)
Signature · 1 line
2.8.0.rc3

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc.git block

-Jeff