[PATCH 04/15] iotests/040: Don't check image pattern on zero-length image

John Snow posted 15 patches 3 years, 10 months ago
There is a newer version of this series
[PATCH 04/15] iotests/040: Don't check image pattern on zero-length image
Posted by John Snow 3 years, 10 months ago
qemu-io fails on read/write with zero-length raw images, so skip these
when running the zero-length image tests.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/040 | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index adf5815781..c4a90937dc 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -86,8 +86,10 @@ class TestSingleDrive(ImageCommitTestCase):
         qemu_img('create', '-f', iotests.imgfmt,
                  '-o', 'backing_file=%s' % mid_img,
                  '-F', iotests.imgfmt, test_img)
-        qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
-        qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
+        if self.image_len:
+            qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
+            qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
+                    mid_img)
         self.vm = iotests.VM().add_drive(test_img, "node-name=top,backing.node-name=mid,backing.backing.node-name=base", interface="none")
         self.vm.add_device('virtio-scsi')
         self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
@@ -101,11 +103,15 @@ class TestSingleDrive(ImageCommitTestCase):
 
     def test_commit(self):
         self.run_commit_test(mid_img, backing_img)
+        if not self.image_len:
+            return
         qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
         qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
 
     def test_commit_node(self):
         self.run_commit_test("mid", "base", node_names=True)
+        if not self.image_len:
+            return
         qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
         qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
 
@@ -192,11 +198,15 @@ class TestSingleDrive(ImageCommitTestCase):
 
     def test_top_is_active(self):
         self.run_commit_test(test_img, backing_img, need_ready=True)
+        if not self.image_len:
+            return
         qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
         qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
 
     def test_top_is_default_active(self):
         self.run_default_commit_test()
+        if not self.image_len:
+            return
         qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img)
         qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img)
 
-- 
2.34.1
Re: [PATCH 04/15] iotests/040: Don't check image pattern on zero-length image
Posted by Eric Blake 3 years, 10 months ago
On Fri, Mar 18, 2022 at 04:36:44PM -0400, John Snow wrote:
> qemu-io fails on read/write with zero-length raw images, so skip these
> when running the zero-length image tests.

On my first read, I wondered what we accomplish by rejecting
zero-length reads on a zero-length image, and whether entering the
rabbit hole of trying to make that corner case "work" differently
makes more sense...

> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/040 | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> index adf5815781..c4a90937dc 100755
> --- a/tests/qemu-iotests/040
> +++ b/tests/qemu-iotests/040
> @@ -86,8 +86,10 @@ class TestSingleDrive(ImageCommitTestCase):
>          qemu_img('create', '-f', iotests.imgfmt,
>                   '-o', 'backing_file=%s' % mid_img,
>                   '-F', iotests.imgfmt, test_img)
> -        qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
> -        qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
> +        if self.image_len:
> +            qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
> +            qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
> +                    mid_img)

...but now it is obvious - one of our test cases is attempting a
non-zero-length modification to a zero-length file, and it does make
sense for that modification attempt to fail, in which case, making the
test special case the zero-length file is the right thing to do.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Re: [PATCH 04/15] iotests/040: Don't check image pattern on zero-length image
Posted by Hanna Reitz 3 years, 10 months ago
On 18.03.22 21:36, John Snow wrote:
> qemu-io fails on read/write with zero-length raw images, so skip these
> when running the zero-length image tests.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/040 | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)

Doesn’t look specific to zero-length images, but the fact that we do I/O 
beyond the image size, i.e. any image below 1 MB would be affected.

Anyway, the zero-length image is the only one tested with a size of less 
than 1 MB, so this works.

Reviewed-by: Hanna Reitz <hreitz@redhat.com>


Re: [PATCH 04/15] iotests/040: Don't check image pattern on zero-length image
Posted by John Snow 3 years, 10 months ago
On Tue, Mar 22, 2022, 10:22 AM Hanna Reitz <hreitz@redhat.com> wrote:

> On 18.03.22 21:36, John Snow wrote:
> > qemu-io fails on read/write with zero-length raw images, so skip these
> > when running the zero-length image tests.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   tests/qemu-iotests/040 | 14 ++++++++++++--
> >   1 file changed, 12 insertions(+), 2 deletions(-)
>
> Doesn’t look specific to zero-length images, but the fact that we do I/O
> beyond the image size, i.e. any image below 1 MB would be affected.
>
> Anyway, the zero-length image is the only one tested with a size of less
> than 1 MB, so this works.
>
> Reviewed-by: Hanna Reitz <hreitz@redhat.com>
>

(Check the cover letter, too - this patch is still good, but iotest 040
still fails and I'm not 100% clear as to why.)

>
Re: [PATCH 04/15] iotests/040: Don't check image pattern on zero-length image
Posted by Hanna Reitz 3 years, 10 months ago
On 22.03.22 17:19, John Snow wrote:
>
>
> On Tue, Mar 22, 2022, 10:22 AM Hanna Reitz <hreitz@redhat.com> wrote:
>
>     On 18.03.22 21:36, John Snow wrote:
>     > qemu-io fails on read/write with zero-length raw images, so skip
>     these
>     > when running the zero-length image tests.
>     >
>     > Signed-off-by: John Snow <jsnow@redhat.com>
>     > ---
>     >   tests/qemu-iotests/040 | 14 ++++++++++++--
>     >   1 file changed, 12 insertions(+), 2 deletions(-)
>
>     Doesn’t look specific to zero-length images, but the fact that we
>     do I/O
>     beyond the image size, i.e. any image below 1 MB would be affected.
>
>     Anyway, the zero-length image is the only one tested with a size
>     of less
>     than 1 MB, so this works.
>
>     Reviewed-by: Hanna Reitz <hreitz@redhat.com>
>
>
> (Check the cover letter, too - this patch is still good, but iotest 
> 040 still fails and I'm not 100% clear as to why.)

Sure, but I didn’t see anything wrong with the patch, so...

 From a glance, I believe the problem to be that the commit job changed 
one image’s backing file string to contain a JSON description of a block 
graph including a throttle node.  The accompanying throttle group of 
course doesn’t exist outside of qemu, so qemu-io complains.

We never noticed because we just checked for “pattern verification 
failed”, which isn’t in the error message, so this was a pass.  I’ll 
take a closer look.

Hanna