[Qemu-devel] [PATCH v2 for-1.2 0/2] stream: complete early if end of backing file is reached

[Qemu-devel] [PATCH v2 for-1.2 0/2] stream: complete early if end of backing file is reached

[Stefan Hajnoczi]

Image streaming hangs if the backing image is smaller than the image file.  The
problem is that the image streaming loop makes no progress when
bdrv_co_is_allocated() returns 0 with pnum=0.  More details in the actual
patch.

I have also included a qemu-iotest to check this scenario.  It hangs when run
against qemu.git/master and passes when the patch is applied.

Stefan Hajnoczi (2):
  stream: complete early if end of backing file is reached
  qemu-iotests: add backing file smaller than image test case

 block/stream.c             |    6 ++++++
 tests/qemu-iotests/030     |   33 +++++++++++++++++++++++++++++++++
 tests/qemu-iotests/030.out |    4 ++--
 3 files changed, 41 insertions(+), 2 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH v2 for-1.2 1/2] stream: complete early if end of backing file is reached

[Stefan Hajnoczi]

It is possible to create an image that is larger than its backing file.
Reading beyond the end of the backing file produces zeroes if no writes
have been made to those sectors in the image file.

This patch finishes streaming early when the end of the backing file is
reached.  Without this patch the block job hangs and continually tries
to stream the first sectors beyond the end of the backing file.

To reproduce the hung block job bug:

  $ qemu-img create -f qcow2 backing.qcow2 128M
  $ qemu-img create -f qcow2 -o backing_file=backing.qcow2 image.qcow2 6G
  $ qemu -drive if=virtio,cache=none,file=image.qcow2
  (qemu) block_stream virtio0
  (qemu) info block-jobs

The qemu-iotests 030 streaming test still passes.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block/stream.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/block/stream.c b/block/stream.c
index 37c4652..c4f87dd 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -122,6 +122,12 @@ wait:
              * known-unallocated area [sector_num, sector_num+n).  */
             ret = bdrv_co_is_allocated_above(bs->backing_hd, base,
                                              sector_num, n, &n);
+
+            /* Finish early if end of backing file has been reached */
+            if (ret == 0 && n == 0) {
+                n = end - sector_num;
+            }
+
             copy = (ret == 1);
         }
         trace_stream_one_iteration(s, sector_num, n, ret);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 for-1.2 1/2] stream: complete early if end of backing file is reached

[Paolo Bonzini]

Il 28/08/2012 16:26, Stefan Hajnoczi ha scritto:
> It is possible to create an image that is larger than its backing file.
> Reading beyond the end of the backing file produces zeroes if no writes
> have been made to those sectors in the image file.
> 
> This patch finishes streaming early when the end of the backing file is
> reached.  Without this patch the block job hangs and continually tries
> to stream the first sectors beyond the end of the backing file.
> 
> To reproduce the hung block job bug:
> 
>   $ qemu-img create -f qcow2 backing.qcow2 128M
>   $ qemu-img create -f qcow2 -o backing_file=backing.qcow2 image.qcow2 6G
>   $ qemu -drive if=virtio,cache=none,file=image.qcow2
>   (qemu) block_stream virtio0
>   (qemu) info block-jobs
> 
> The qemu-iotests 030 streaming test still passes.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block/stream.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/stream.c b/block/stream.c
> index 37c4652..c4f87dd 100644
> --- a/block/stream.c
> +++ b/block/stream.c
> @@ -122,6 +122,12 @@ wait:
>               * known-unallocated area [sector_num, sector_num+n).  */
>              ret = bdrv_co_is_allocated_above(bs->backing_hd, base,
>                                               sector_num, n, &n);
> +
> +            /* Finish early if end of backing file has been reached */
> +            if (ret == 0 && n == 0) {
> +                n = end - sector_num;
> +            }

Reviewed-by: Paolo Bonzini <pbonzini@gnu.org>

>              copy = (ret == 1);
>          }
>          trace_stream_one_iteration(s, sector_num, n, ret);
> 

[Qemu-devel] [PATCH v2 for-1.2 2/2] qemu-iotests: add backing file smaller than image test case

[Stefan Hajnoczi]

This new test case checks that streaming completes successfully when the
backing file is smaller than the image file.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 tests/qemu-iotests/030     |   33 +++++++++++++++++++++++++++++++++
 tests/qemu-iotests/030.out |    4 ++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index f71ab8d..55b16f8 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -125,6 +125,39 @@ class TestSingleDrive(ImageStreamingTestCase):
         result = self.vm.qmp('block-stream', device='nonexistent')
         self.assert_qmp(result, 'error/class', 'DeviceNotFound')
 
+
+class TestSmallerBackingFile(ImageStreamingTestCase):
+    backing_len = 1 * 1024 * 1024 # MB
+    image_len = 2 * backing_len
+
+    def setUp(self):
+        self.create_image(backing_img, self.backing_len)
+        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img, str(self.image_len))
+        self.vm = iotests.VM().add_drive(test_img)
+        self.vm.launch()
+
+    # If this hangs, then you are missing a fix to complete streaming when the
+    # end of the backing file is reached.
+    def test_stream(self):
+        self.assert_no_active_streams()
+
+        result = self.vm.qmp('block-stream', device='drive0')
+        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', 'stream')
+                    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_streams()
+        self.vm.shutdown()
+
+
 class TestStreamStop(ImageStreamingTestCase):
     image_len = 8 * 1024 * 1024 * 1024 # GB
 
diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
index 3f8a935..2f7d390 100644
--- a/tests/qemu-iotests/030.out
+++ b/tests/qemu-iotests/030.out
@@ -1,5 +1,5 @@
-......
+.......
 ----------------------------------------------------------------------
-Ran 6 tests
+Ran 7 tests
 
 OK
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2 for-1.2 2/2] qemu-iotests: add backing file smaller than image test case

[Paolo Bonzini]

Il 28/08/2012 16:26, Stefan Hajnoczi ha scritto:
> This new test case checks that streaming completes successfully when the
> backing file is smaller than the image file.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  tests/qemu-iotests/030     |   33 +++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/030.out |    4 ++--
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index f71ab8d..55b16f8 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -125,6 +125,39 @@ class TestSingleDrive(ImageStreamingTestCase):
>          result = self.vm.qmp('block-stream', device='nonexistent')
>          self.assert_qmp(result, 'error/class', 'DeviceNotFound')
>  
> +
> +class TestSmallerBackingFile(ImageStreamingTestCase):
> +    backing_len = 1 * 1024 * 1024 # MB
> +    image_len = 2 * backing_len
> +
> +    def setUp(self):
> +        self.create_image(backing_img, self.backing_len)
> +        qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img, str(self.image_len))
> +        self.vm = iotests.VM().add_drive(test_img)
> +        self.vm.launch()
> +
> +    # If this hangs, then you are missing a fix to complete streaming when the
> +    # end of the backing file is reached.
> +    def test_stream(self):
> +        self.assert_no_active_streams()
> +
> +        result = self.vm.qmp('block-stream', device='drive0')
> +        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', 'stream')
> +                    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_streams()
> +        self.vm.shutdown()
> +
> +
>  class TestStreamStop(ImageStreamingTestCase):
>      image_len = 8 * 1024 * 1024 * 1024 # GB
>  
> diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out
> index 3f8a935..2f7d390 100644
> --- a/tests/qemu-iotests/030.out
> +++ b/tests/qemu-iotests/030.out
> @@ -1,5 +1,5 @@
> -......
> +.......
>  ----------------------------------------------------------------------
> -Ran 6 tests
> +Ran 7 tests
>  
>  OK
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 for-1.2 0/2] stream: complete early if end of backing file is reached

[Kevin Wolf]

Am 28.08.2012 16:26, schrieb Stefan Hajnoczi:
> Image streaming hangs if the backing image is smaller than the image file.  The
> problem is that the image streaming loop makes no progress when
> bdrv_co_is_allocated() returns 0 with pnum=0.  More details in the actual
> patch.
> 
> I have also included a qemu-iotest to check this scenario.  It hangs when run
> against qemu.git/master and passes when the patch is applied.
> 
> Stefan Hajnoczi (2):
>   stream: complete early if end of backing file is reached
>   qemu-iotests: add backing file smaller than image test case
> 
>  block/stream.c             |    6 ++++++
>  tests/qemu-iotests/030     |   33 +++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/030.out |    4 ++--
>  3 files changed, 41 insertions(+), 2 deletions(-)
> 

Thanks, applied all to the block branch.

Kevin