Patchwork [1.1,18/22] stream: do not copy unallocated sectors from the base

login
register
mail settings
Submitter Paolo Bonzini
Date May 8, 2012, 2:51 p.m.
Message ID <1336488722-13120-19-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/157747/
State New
Headers show

Comments

Paolo Bonzini - May 8, 2012, 2:51 p.m.
Unallocated sectors should really never be accessed by the guest,
so there's no need to copy them during the streaming process.
If they are read by the guest during streaming, guest-initiated
copy-on-read will copy them (we're in the base == NULL case, which
enables copy on read).  If they are read after we disconnect the
image from the base, they will read as zeroes anyway.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        This is a semantic change.  We could delay it to 1.2 as
        a bug fix, or do it now to avoid changing semantics in the
        next release.

 block/stream.c         |   18 ++++--------------
 tests/qemu-iotests/030 |    5 +++--
 2 files changed, 7 insertions(+), 16 deletions(-)

Patch

diff --git a/block/stream.c b/block/stream.c
index a2c8f67..608a860 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -130,14 +130,9 @@  static int coroutine_fn is_allocated_base(BlockDriverState *top,
      */
     intermediate = top->backing_hd;
 
-    while (intermediate) {
+    while (intermediate != base) {
         int pnum_inter;
 
-        /* reached base */
-        if (intermediate == base) {
-            *pnum = n;
-            return 1;
-        }
         ret = bdrv_co_is_allocated(intermediate, sector_num, nb_sectors,
                                    &pnum_inter);
         if (ret < 0) {
@@ -160,6 +155,7 @@  static int coroutine_fn is_allocated_base(BlockDriverState *top,
         intermediate = intermediate->backing_hd;
     }
 
+    *pnum = n;
     return 1;
 }
 
@@ -203,14 +199,8 @@  wait:
             break;
         }
 
-        if (base) {
-            ret = is_allocated_base(bs, base, sector_num,
-                                    STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
-        } else {
-            ret = bdrv_co_is_allocated(bs, sector_num,
-                                       STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE,
-                                       &n);
-        }
+        ret = is_allocated_base(bs, base, sector_num,
+                                STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n);
         trace_stream_one_iteration(s, sector_num, n, ret);
         if (ret == 0) {
             if (s->common.speed) {
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 0aa7a0a..61cb027 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -82,8 +82,9 @@  class TestSingleDrive(ImageStreamingTestCase):
         self.assert_no_active_streams()
         self.vm.shutdown()
 
-        self.assertFalse('sectors not allocated' in qemu_io('-c', 'map', test_img),
-                         'image file not fully populated after streaming')
+        self.assertEqual(qemu_io('-c', 'map', backing_img),
+                         qemu_io('-c', 'map', test_img),
+                         'image file map does not match backing file after streaming')
 
     def test_stream_partial(self):
         self.assert_no_active_streams()