[6/6] iotests: vmdk: Enable zeroed_grained=on by default
diff mbox series

Message ID 20200430133007.170335-7-kwolf@redhat.com
State New
Headers show
Series
  • vmdk: Fix zero cluster handling
Related show

Commit Message

Kevin Wolf April 30, 2020, 1:30 p.m. UTC
In order to avoid bitrot in the zero cluster code in VMDK, enable
zero_grained=on by default for the tests.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/059   | 6 +++---
 tests/qemu-iotests/check | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

Comments

Eric Blake April 30, 2020, 2:22 p.m. UTC | #1
On 4/30/20 8:30 AM, Kevin Wolf wrote:
> In order to avoid bitrot in the zero cluster code in VMDK, enable
> zero_grained=on by default for the tests.

Here, you spell it zero_grained=on,

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/059   | 6 +++---
>   tests/qemu-iotests/check | 3 +++
>   2 files changed, 6 insertions(+), 3 deletions(-)

So you're changing the default for better coverage and speed, but 
ensuring that 59 still covers the (slower) zero_grained=off.  Seems 
reasonable.

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

> +++ b/tests/qemu-iotests/check
> @@ -546,6 +546,9 @@ fi
>   if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > /dev/null); then
>       IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
>   fi
> +if [ "$IMGFMT" == "vmdk" ] && ! (echo "$IMGOPTS" | grep "zeroed_grain=" > /dev/null); then

Here, zeroed_grain=.  Which is it?

> +    IMGOPTS=$(_optstr_add "$IMGOPTS" "zeroed_grain=on")

As a native speaker, my inclination is zero_grained; but I don't know 
the VMDK spec well enough to know if this is something in the spec, or 
just a term that qemu invented.  And since we already have existing 
usage of one spelling, switching the spelling now would require a 
deprecation period and is separate from this patch.
Kevin Wolf April 30, 2020, 2:42 p.m. UTC | #2
Am 30.04.2020 um 16:22 hat Eric Blake geschrieben:
> On 4/30/20 8:30 AM, Kevin Wolf wrote:
> > In order to avoid bitrot in the zero cluster code in VMDK, enable
> > zero_grained=on by default for the tests.
> 
> Here, you spell it zero_grained=on,

Thanks for spotting this, the typo is in the commit message.

> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   tests/qemu-iotests/059   | 6 +++---
> >   tests/qemu-iotests/check | 3 +++
> >   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> So you're changing the default for better coverage and speed, but ensuring
> that 59 still covers the (slower) zero_grained=off.  Seems reasonable.

The real reason why I'm changing 059 is that zeroed_grain=on works only
with some subformats and the test case tests many different subformats,
including those for which it doesn't work.

Kevin

Patch
diff mbox series

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 5438025285..4c90fc0363 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -41,9 +41,9 @@  trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt vmdk
 _supported_proto file
 _supported_os Linux
-_unsupported_imgopts "subformat=monolithicFlat" \
-                     "subformat=twoGbMaxExtentFlat" \
-                     "subformat=twoGbMaxExtentSparse"
+
+# We test all kinds of VMDK options here, so ignore user-specified options
+IMGOPTS=""
 
 capacity_offset=16
 granularity_offset=20
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f7a2d3d6c3..9c461cf76d 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -546,6 +546,9 @@  fi
 if [ "$IMGFMT" == "luks" ] && ! (echo "$IMGOPTS" | grep "iter-time=" > /dev/null); then
     IMGOPTS=$(_optstr_add "$IMGOPTS" "iter-time=10")
 fi
+if [ "$IMGFMT" == "vmdk" ] && ! (echo "$IMGOPTS" | grep "zeroed_grain=" > /dev/null); then
+    IMGOPTS=$(_optstr_add "$IMGOPTS" "zeroed_grain=on")
+fi
 
 if [ -z "$SAMPLE_IMG_DIR" ]; then
         SAMPLE_IMG_DIR="$source_iotests/sample_images"