From: Max Reitz <mreitz@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
qemu-block@nongnu.org
Cc: kwolf@redhat.com, den@openvz.org, jsnow@redhat.com,
qemu-devel@nongnu.org
Subject: Re: [PATCH 08/14] iotests/common.rc: _make_test_img(): smarter compressiont_type handling
Date: Fri, 16 Jul 2021 14:38:20 +0200 [thread overview]
Message-ID: <d983ae4a-dce5-35d3-1c63-7285651eae56@redhat.com> (raw)
In-Reply-To: <20210705091549.178335-9-vsementsov@virtuozzo.com>
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()
> backing_file=$param
> continue
> 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 options.
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.
Max
> + fi
> optstr=$(_optstr_add "$optstr" "$param")
> opts_param=false
> continue
next prev parent reply other threads:[~2021-07-16 12:39 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-05 9:15 [PATCH 00/14] iotests: support zstd Vladimir Sementsov-Ogievskiy
2021-07-05 9:15 ` [PATCH 01/14] iotests.py: img_info_log(): rename imgopts argument Vladimir Sementsov-Ogievskiy
2021-07-16 10:07 ` Max Reitz
2021-07-05 9:15 ` [PATCH 02/14] iotests.py: qemu_img*("create"): support IMGOPTS='compression_type=zstd' Vladimir Sementsov-Ogievskiy
2021-07-16 10:07 ` Max Reitz
2021-07-16 10:34 ` Vladimir Sementsov-Ogievskiy
2021-07-19 12:58 ` Vladimir Sementsov-Ogievskiy
2021-07-19 13:51 ` Vladimir Sementsov-Ogievskiy
2021-07-05 9:15 ` [PATCH 03/14] iotest 303: explicit compression type Vladimir Sementsov-Ogievskiy
2021-07-16 10:38 ` Max Reitz
2021-07-05 9:15 ` [PATCH 04/14] iotest 065: " Vladimir Sementsov-Ogievskiy
2021-07-16 11:01 ` Max Reitz
2021-07-05 9:15 ` [PATCH 05/14] iotests.py: filter compression type out Vladimir Sementsov-Ogievskiy
2021-07-16 11:15 ` Max Reitz
2021-07-16 11:32 ` Vladimir Sementsov-Ogievskiy
2021-07-05 9:15 ` [PATCH 06/14] iotest 302: use img_info_log() helper Vladimir Sementsov-Ogievskiy
2021-07-05 11:02 ` Vladimir Sementsov-Ogievskiy
2021-07-16 11:39 ` Max Reitz
2021-07-16 12:32 ` Vladimir Sementsov-Ogievskiy
2021-07-05 9:15 ` [PATCH 07/14] qcow2: simple case support for downgrading of qcow2 images with zstd Vladimir Sementsov-Ogievskiy
2021-07-16 12:08 ` Max Reitz
2021-07-05 9:15 ` [PATCH 08/14] iotests/common.rc: _make_test_img(): smarter compressiont_type handling Vladimir Sementsov-Ogievskiy
2021-07-16 12:38 ` Max Reitz [this message]
2021-07-16 14:24 ` Vladimir Sementsov-Ogievskiy
2021-07-16 14:46 ` Max Reitz
2021-07-05 9:15 ` [PATCH 09/14] iotests/common.rc: introduce _qcow2_dump_header helper Vladimir Sementsov-Ogievskiy
2021-07-16 12:56 ` Max Reitz
2021-07-05 9:15 ` [PATCH 10/14] iotests: massive use _qcow2_dump_header Vladimir Sementsov-Ogievskiy
2021-07-16 13:04 ` Max Reitz
2021-07-05 9:15 ` [PATCH 11/14] iotests: bash tests: filter compression type Vladimir Sementsov-Ogievskiy
2021-07-16 13:17 ` Max Reitz
2021-07-16 14:35 ` Vladimir Sementsov-Ogievskiy
2021-07-05 9:15 ` [PATCH 12/14] iotests 60: more accurate set dirty bit in qcow2 header Vladimir Sementsov-Ogievskiy
2021-07-16 13:20 ` Max Reitz
2021-07-05 9:15 ` [PATCH 13/14] iotest 39: use _qcow2_dump_header Vladimir Sementsov-Ogievskiy
2021-07-16 13:31 ` Max Reitz
2021-07-05 9:15 ` [PATCH 14/14] iotest 214: explicit compression type Vladimir Sementsov-Ogievskiy
2021-07-16 13:35 ` Max Reitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d983ae4a-dce5-35d3-1c63-7285651eae56@redhat.com \
--to=mreitz@redhat.com \
--cc=den@openvz.org \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.