[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 08/14] iotests/common.rc: _make_test_img(): smarter compressi

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH 08/14] iotests/common.rc: _make_test_img(): smarter compressiont_type handling
Date: Fri, 16 Jul 2021 17:24:26 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0

16.07.2021 15:38, Max Reitz wrote:
Subject: s/compressiont_type/compression_type/

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
Like it is done in iotests.py in qemu_img_create_prepare_args(), let's
not follow compression_type=zstd of IMGOPTS if test creates image in
old format.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  tests/qemu-iotests/common.rc | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index cbbf6d7c7f..4cae5b2d70 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -438,6 +438,14 @@ _make_test_img()
          elif $opts_param; then
+            if [[ "$param" == *"compat=0"* ]]; then

Like in patch 2, probably should be 0.10, and account for “v2”.

+                # If user specified zstd compression type in IMGOPTS, this will
+                # just not work. So, let's imply forcing zlib compression when
+                # test creates image in old version of the format.
+                # Similarly works qemu_img_create_prepare_args() in iotests.py
+                optstr=$(echo "$optstr" | $SED -e 's/compression_type=\w\+//')

What about the surrounding comma, if compression_type is just one option among 
others?  Do we need something like

$SED -e 's/,compression_type=\w\+//' -e 's/compression_type=\w\+,\?//'



+                optstr=$(_optstr_add "$optstr" "compression_type=zlib")

As the comment says, this is for compression_type in $IMGOPTS and then 
compat=0.10 in the parameters.  It won’t work if you have e.g. “_make_test_img 
-o compat=0.10,compression_type=zstd”, because then this generates the optstr 
“$IMGOPTS,compression_type=zlib,compat=0.10,compression_type=zstd”. Not sure if 
we want to care about this case, but, well...

Then there’s the case where I have compat=0.10 in $IMGOPTS, and the test wants 
to use compression_type=zstd.  I think it’s correct not to replace 
compression_type=zstd then, because the test should be skipped for compat=0.10 
in $IMGOPTS.  But that’s not what happens in the iotest.py version 
(qemu_img_create_prepare_args()), so I wonder whether the latter should be made 
to match this behavior here, if in any way possible.

Now that I think about it more, I begin to wonder more...

So this code doesn’t explicitly handle compression_type only in $IMGOPTS.  If 
you have

_make_test_img -o compression_type=zstd,compat=0.10

It’ll still keep the compression_type=zstd.  However, for

_make_test_img -o compression_type=zstd -o compat=0.10

It’ll replace it by zlib.

So perhaps we should explicitly scan for compression_type only in $IMGOPTS and 
then drop it from the optstr if compat=0.10 is in the _make_test_img's -o 

But thinking further, this is not how $IMGOPTS work.  So far they aren’t 
advisory, they are mandatory.  If a test cannot work with something in 
$IMGOPTS, it has to be skipped.  Like, when you have compat=0.10 in $IMGOPTS, I 
don’t want to run tests that use v3 features and have them just create v3 
images for those tests.

So my impression is that you’re giving compression_type special treatment here, 
and I don’t know why exactly.  Tests that create v2 images should just have 
compression_type be an unsupported_imgopt.


Hmm.. I have better idea: deprecate v2 and drop all iotest support for it now 
:)) What do you think?

If not, than instead of this patch, we just should skip all tests that don't 
support compression_type=zstd due to using old version.. This means that we 
will skip some test-cases that can work with zstd just because we can't skip 
separate test cases in bash tests.

(ohh, I'd deprecate bash tests too.. But that's kind of taste)

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]