[PATCH 00/14] iotests: support zstd

[PATCH 00/14] iotests: support zstd

[Vladimir Sementsov-Ogievskiy]

Hi all!

These series makes all test pass with

   IMGOPTS='compression_type=zstd'

Also, python iotests start to support IMGOPTS (they didn't before).

Also, tests works if enable compression type zstd by default. There is
no such config option currently, probably it will appear in future or
we'll go some another way (like external config file, like
/etc/qemu.conf). For now you may test with a simple patch like:

    --- a/block/qcow2.c
    +++ b/block/qcow2.c
    @@ -3539,6 +3539,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
                 goto out;
             }
         }
    +
    +    if (!qcow2_opts->has_compression_type && version >= 3) {
    +        qcow2_opts->has_compression_type = true;
    +        qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
    +    }
     
         if (qcow2_opts->has_compression_type &&
             qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {

We want to use zstd compression type by default in Virtuozzo 8. This is
the first step, which is good anyway: improve iotests.

Vladimir Sementsov-Ogievskiy (14):
  iotests.py: img_info_log(): rename imgopts argument
  iotests.py: qemu_img*("create"): support
    IMGOPTS='compression_type=zstd'
  iotest 303: explicit compression type
  iotest 065: explicit compression type
  iotests.py: filter compression type out
  iotest 302: use img_info_log() helper
  qcow2: simple case support for downgrading of qcow2 images with zstd
  iotests/common.rc: _make_test_img(): smarter compressiont_type
    handling
  iotests/common.rc: introduce _qcow2_dump_header helper
  iotests: massive use _qcow2_dump_header
  iotests: bash tests: filter compression type
  iotests 60: more accurate set dirty bit in qcow2 header
  iotest 39: use _qcow2_dump_header
  iotest 214: explicit compression type

 block/qcow2.c                    | 58 ++++++++++++++++++++++++++-
 tests/qemu-iotests/031           |  6 +--
 tests/qemu-iotests/036           |  6 +--
 tests/qemu-iotests/039           | 22 +++++------
 tests/qemu-iotests/060           | 22 +++++------
 tests/qemu-iotests/060.out       |  2 +-
 tests/qemu-iotests/061           | 36 ++++++++---------
 tests/qemu-iotests/061.out       | 12 +++---
 tests/qemu-iotests/065           | 14 +++----
 tests/qemu-iotests/082.out       | 14 +++----
 tests/qemu-iotests/137           |  2 +-
 tests/qemu-iotests/198.out       |  4 +-
 tests/qemu-iotests/206.out       | 10 ++---
 tests/qemu-iotests/210           |  8 ++--
 tests/qemu-iotests/214           |  2 +-
 tests/qemu-iotests/242.out       | 10 ++---
 tests/qemu-iotests/255.out       |  8 ++--
 tests/qemu-iotests/274.out       | 68 ++++++++++++++++----------------
 tests/qemu-iotests/280.out       |  2 +-
 tests/qemu-iotests/287           |  8 ++--
 tests/qemu-iotests/302           |  3 +-
 tests/qemu-iotests/302.out       |  7 ++--
 tests/qemu-iotests/303           | 25 +++++++-----
 tests/qemu-iotests/303.out       | 30 +++++++++++++-
 tests/qemu-iotests/common.filter |  7 ++++
 tests/qemu-iotests/common.rc     | 30 ++++++++++++++
 tests/qemu-iotests/iotests.py    | 66 +++++++++++++++++++++++++++++--
 27 files changed, 333 insertions(+), 149 deletions(-)

-- 
2.29.2

[PATCH 01/14] iotests.py: img_info_log(): rename imgopts argument

[Vladimir Sementsov-Ogievskiy]

We are going to support IMGOPTS environment variable like in bash
tests. Corresponding global variable in iotests.py should be called
imgopts. So to not interfere with function argument, rename it in
advance.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/210        | 8 ++++----
 tests/qemu-iotests/iotests.py | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/210 b/tests/qemu-iotests/210
index 5a62ed4dd1..79b4967225 100755
--- a/tests/qemu-iotests/210
+++ b/tests/qemu-iotests/210
@@ -62,7 +62,7 @@ with iotests.FilePath('t.luks') as disk_path, \
         'driver=luks,file.driver=file,file.filename=%s,key-secret=keysec0' % (disk_path),
         filter_path=disk_path,
         extra_args=['--object', 'secret,id=keysec0,data=foo'],
-        imgopts=True)
+        use_image_opts=True)
 
     #
     # Successful image creation (with non-default options)
@@ -96,7 +96,7 @@ with iotests.FilePath('t.luks') as disk_path, \
         'driver=luks,file.driver=file,file.filename=%s,key-secret=keysec0' % (disk_path),
         filter_path=disk_path,
         extra_args=['--object', 'secret,id=keysec0,data=foo'],
-        imgopts=True)
+        use_image_opts=True)
 
     #
     # Invalid BlockdevRef
@@ -132,7 +132,7 @@ with iotests.FilePath('t.luks') as disk_path, \
         'driver=luks,file.driver=file,file.filename=%s,key-secret=keysec0' % (disk_path),
         filter_path=disk_path,
         extra_args=['--object', 'secret,id=keysec0,data=foo'],
-        imgopts=True)
+        use_image_opts=True)
 
     #
     # Invalid sizes
@@ -176,4 +176,4 @@ with iotests.FilePath('t.luks') as disk_path, \
         'driver=luks,file.driver=file,file.filename=%s,key-secret=keysec0' % (disk_path),
         filter_path=disk_path,
         extra_args=['--object', 'secret,id=keysec0,data=foo'],
-        imgopts=True)
+        use_image_opts=True)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 89663dac06..0d99dd841f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -187,9 +187,10 @@ def qemu_img_log(*args):
     log(result, filters=[filter_testfiles])
     return result
 
-def img_info_log(filename, filter_path=None, imgopts=False, extra_args=()):
+def img_info_log(filename, filter_path=None, use_image_opts=False,
+                 extra_args=()):
     args = ['info']
-    if imgopts:
+    if use_image_opts:
         args.append('--image-opts')
     else:
         args += ['-f', imgfmt]
-- 
2.29.2

Re: [PATCH 01/14] iotests.py: img_info_log(): rename imgopts argument

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> We are going to support IMGOPTS environment variable like in bash
> tests. Corresponding global variable in iotests.py should be called
> imgopts. So to not interfere with function argument, rename it in
> advance.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/210        | 8 ++++----
>   tests/qemu-iotests/iotests.py | 5 +++--
>   2 files changed, 7 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Reminds me how I sent a huge series for having Python tests support 
$IMGOPTS two years ago 
(https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg00071.html). 
I guess the reason it was so big and this series is comparatively small 
is because I mostly concerned myself with `-o data_file` (and so all the 
operations that currently assume that every image is a single file need 
to be amended).

Max

[PATCH 02/14] iotests.py: qemu_img*("create"): support IMGOPTS='compression_type=zstd'

[Vladimir Sementsov-Ogievskiy]

Adding support of IMGOPTS (like in bash tests) allows user to pass a
lot of different options. Still, some may require additional logic.

Now we want compression_type option, so add some smart logic around it:
ignore compression_type=zstd in IMGOPTS, if test want qcow2 in
compatibility mode. As well, ignore compression_type for non-qcow2
formats.

Note that we may instead add support only to qemu_img_create(), but
that works bad:

1. We'll have to update a lot of tests to use qemu_img_create instead
   of qemu_img('create'). (still, we may want do it anyway, but no
   reason to create a dependancy between task of supporting IMGOPTS and
   updating a lot of tests)

2. Some tests use qemu_img_pipe('create', ..) - even more work on
   updating

3. Even if we update all tests to go through qemu_img_create, we'll
   need a way to avoid creating new tests using qemu_img*('create') -
   add assertions.. That doesn't seem good.

So, let's add support of IMGOPTS to most generic
qemu_img_pipe_and_status().

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

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 0d99dd841f..80f0cb4f42 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -16,6 +16,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import argparse
 import atexit
 import bz2
 from collections import OrderedDict
@@ -41,6 +42,19 @@
 from qemu.machine import qtest
 from qemu.qmp import QMPMessage
 
+
+def optstr2dict(opts: str) -> Dict[str, str]:
+    if not opts:
+        return {}
+
+    return {arr[0]: arr[1] for arr in
+            (opt.split('=', 1) for opt in opts.strip().split(','))}
+
+
+def dict2optstr(opts: Dict[str, str]) -> str:
+    return ','.join(f'{k}={v}' for k, v in opts.items())
+
+
 # Use this logger for logging messages directly from the iotests module
 logger = logging.getLogger('qemu.iotests')
 logger.addHandler(logging.NullHandler())
@@ -57,6 +71,8 @@
 if os.environ.get('QEMU_IMG_OPTIONS'):
     qemu_img_args += os.environ['QEMU_IMG_OPTIONS'].strip().split(' ')
 
+imgopts = optstr2dict(os.environ.get('IMGOPTS', ''))
+
 qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
 if os.environ.get('QEMU_IO_OPTIONS'):
     qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
@@ -121,11 +137,41 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
                                {-subp.returncode}: {cmd}\n')
         return (output, subp.returncode)
 
+def qemu_img_create_prepare_args(args: List[str]) -> List[str]:
+    if not args or args[0] != 'create':
+        return list(args)
+    args = args[1:]
+
+    p = argparse.ArgumentParser(allow_abbrev=False)
+    # -o option may be specified several times
+    p.add_argument('-o', action='append', default=[])
+    p.add_argument('-f')
+    parsed, remaining = p.parse_known_args(args)
+
+    opts = optstr2dict(','.join(parsed.o))
+
+    compat = 'compat' in opts and opts['compat'][0] == '0'
+    for k, v in imgopts.items():
+        if k in opts:
+            continue
+        if k == 'compression_type' and (compat or parsed.f != 'qcow2'):
+            continue
+        opts[k] = v
+
+    result = ['create']
+    if parsed.f is not None:
+        result += ['-f', parsed.f]
+    if opts:
+        result += ['-o', dict2optstr(opts)]
+    result += remaining
+
+    return result
+
 def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
     """
     Run qemu-img and return both its output and its exit code
     """
-    full_args = qemu_img_args + list(args)
+    full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
     return qemu_tool_pipe_and_status('qemu-img', full_args)
 
 def qemu_img(*args: str) -> int:
-- 
2.29.2

Re: [PATCH 02/14] iotests.py: qemu_img*("create"): support IMGOPTS='compression_type=zstd'

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> Adding support of IMGOPTS (like in bash tests) allows user to pass a
> lot of different options. Still, some may require additional logic.
>
> Now we want compression_type option, so add some smart logic around it:
> ignore compression_type=zstd in IMGOPTS, if test want qcow2 in
> compatibility mode. As well, ignore compression_type for non-qcow2
> formats.
>
> Note that we may instead add support only to qemu_img_create(), but
> that works bad:
>
> 1. We'll have to update a lot of tests to use qemu_img_create instead
>     of qemu_img('create'). (still, we may want do it anyway, but no
>     reason to create a dependancy between task of supporting IMGOPTS and
>     updating a lot of tests)
>
> 2. Some tests use qemu_img_pipe('create', ..) - even more work on
>     updating

I feel compelled to again say that we had a series that did exactly 
that.  But of course, now that so much time has passed, overhauling it 
would require quite a bit of work.

> 3. Even if we update all tests to go through qemu_img_create, we'll
>     need a way to avoid creating new tests using qemu_img*('create') -
>     add assertions.. That doesn't seem good.

That almost sounds like you remember my series, because:

https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg00135.html

;)

> So, let's add support of IMGOPTS to most generic
> qemu_img_pipe_and_status().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/iotests.py | 48 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 0d99dd841f..80f0cb4f42 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -16,6 +16,7 @@
>   # along with this program.  If not, see<http://www.gnu.org/licenses/>.
>   #
>   
> +import argparse
>   import atexit
>   import bz2
>   from collections import OrderedDict
> @@ -41,6 +42,19 @@
>   from qemu.machine import qtest
>   from qemu.qmp import QMPMessage
>   
> +
> +def optstr2dict(opts: str) -> Dict[str, str]:
> +    if not opts:
> +        return {}
> +
> +    return {arr[0]: arr[1] for arr in
> +            (opt.split('=', 1) for opt in opts.strip().split(','))}
> +
> +
> +def dict2optstr(opts: Dict[str, str]) -> str:
> +    return ','.join(f'{k}={v}' for k, v in opts.items())
> +
> +
>   # Use this logger for logging messages directly from the iotests module
>   logger = logging.getLogger('qemu.iotests')
>   logger.addHandler(logging.NullHandler())
> @@ -57,6 +71,8 @@
>   if os.environ.get('QEMU_IMG_OPTIONS'):
>       qemu_img_args += os.environ['QEMU_IMG_OPTIONS'].strip().split(' ')
>   
> +imgopts = optstr2dict(os.environ.get('IMGOPTS', ''))
> +
>   qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>   if os.environ.get('QEMU_IO_OPTIONS'):
>       qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
> @@ -121,11 +137,41 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
>                                  {-subp.returncode}: {cmd}\n')
>           return (output, subp.returncode)
>   
> +def qemu_img_create_prepare_args(args: List[str]) -> List[str]:
> +    if not args or args[0] != 'create':
> +        return list(args)
> +    args = args[1:]
> +
> +    p = argparse.ArgumentParser(allow_abbrev=False)
> +    # -o option may be specified several times
> +    p.add_argument('-o', action='append', default=[])
> +    p.add_argument('-f')
> +    parsed, remaining = p.parse_known_args(args)
> +
> +    opts = optstr2dict(','.join(parsed.o))
> +
> +    compat = 'compat' in opts and opts['compat'][0] == '0'

I suppose `opts['compat'][0] == '0'` is supposed to check for compat=0.10?

If so, then why not just check `opts['compat'] == '0.10'` to be 
clearer?  I don’t think we allow any other compat=0* values, and I have 
no reason to believe we ever will.

Also, I think compat=v2 is valid, too.  (And I think calling this 
variable “v2” would also make more sense than “compat”.)

> +    for k, v in imgopts.items():
> +        if k in opts:
> +            continue
> +        if k == 'compression_type' and (compat or parsed.f != 'qcow2'):
> +            continue
> +        opts[k] = v

Could also be done with something like

imgopts = os.environ.get('IMGOPTS')
opts = optstr2dict(','.join(([imgopts] if imgopts else []) + parsed.o))

if parsed.f != 'qcow2' or (opts.get('compat') in ['v2', '0.10']):
     opts.pop('compression_type', None)

(Never tested, of course)

Because optstr2dict() prioritizes later options over earlier ones. 
(Which is good, because that’s also qemu-img’s behavior.)

*shrug*

> +
> +    result = ['create']
> +    if parsed.f is not None:
> +        result += ['-f', parsed.f]

Can this even be None?  I hope none of our tests do this.

> +    if opts:
> +        result += ['-o', dict2optstr(opts)]
> +    result += remaining
> +
> +    return result
> +
>   def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
>       """
>       Run qemu-img and return both its output and its exit code
>       """
> -    full_args = qemu_img_args + list(args)
> +    full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
>       return qemu_tool_pipe_and_status('qemu-img', full_args)
>   
>   def qemu_img(*args: str) -> int:

There’s also qemu_img_verbose() which I think should be amended the same 
way (even if it’s currently never used for 'create').

Max

Re: [PATCH 02/14] iotests.py: qemu_img*("create"): support IMGOPTS='compression_type=zstd'

[Vladimir Sementsov-Ogievskiy]

16.07.2021 13:07, Max Reitz wrote:
> On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
>> Adding support of IMGOPTS (like in bash tests) allows user to pass a
>> lot of different options. Still, some may require additional logic.
>>
>> Now we want compression_type option, so add some smart logic around it:
>> ignore compression_type=zstd in IMGOPTS, if test want qcow2 in
>> compatibility mode. As well, ignore compression_type for non-qcow2
>> formats.
>>
>> Note that we may instead add support only to qemu_img_create(), but
>> that works bad:
>>
>> 1. We'll have to update a lot of tests to use qemu_img_create instead
>>     of qemu_img('create'). (still, we may want do it anyway, but no
>>     reason to create a dependancy between task of supporting IMGOPTS and
>>     updating a lot of tests)
>>
>> 2. Some tests use qemu_img_pipe('create', ..) - even more work on
>>     updating
> 
> I feel compelled to again say that we had a series that did exactly that.  But of course, now that so much time has passed, overhauling it would require quite a bit of work.
> 
>> 3. Even if we update all tests to go through qemu_img_create, we'll
>>     need a way to avoid creating new tests using qemu_img*('create') -
>>     add assertions.. That doesn't seem good.
> 
> That almost sounds like you remember my series, because:
> 
> https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg00135.html
> 
> ;)

:) No, I don't remember it.

> 
>> So, let's add support of IMGOPTS to most generic
>> qemu_img_pipe_and_status().
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/iotests.py | 48 ++++++++++++++++++++++++++++++++++-
>>   1 file changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 0d99dd841f..80f0cb4f42 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -16,6 +16,7 @@
>>   # along with this program.  If not, see<http://www.gnu.org/licenses/>.
>>   #
>> +import argparse
>>   import atexit
>>   import bz2
>>   from collections import OrderedDict
>> @@ -41,6 +42,19 @@
>>   from qemu.machine import qtest
>>   from qemu.qmp import QMPMessage
>> +
>> +def optstr2dict(opts: str) -> Dict[str, str]:
>> +    if not opts:
>> +        return {}
>> +
>> +    return {arr[0]: arr[1] for arr in
>> +            (opt.split('=', 1) for opt in opts.strip().split(','))}
>> +
>> +
>> +def dict2optstr(opts: Dict[str, str]) -> str:
>> +    return ','.join(f'{k}={v}' for k, v in opts.items())
>> +
>> +
>>   # Use this logger for logging messages directly from the iotests module
>>   logger = logging.getLogger('qemu.iotests')
>>   logger.addHandler(logging.NullHandler())
>> @@ -57,6 +71,8 @@
>>   if os.environ.get('QEMU_IMG_OPTIONS'):
>>       qemu_img_args += os.environ['QEMU_IMG_OPTIONS'].strip().split(' ')
>> +imgopts = optstr2dict(os.environ.get('IMGOPTS', ''))
>> +
>>   qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>>   if os.environ.get('QEMU_IO_OPTIONS'):
>>       qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>> @@ -121,11 +137,41 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
>>                                  {-subp.returncode}: {cmd}\n')
>>           return (output, subp.returncode)
>> +def qemu_img_create_prepare_args(args: List[str]) -> List[str]:
>> +    if not args or args[0] != 'create':
>> +        return list(args)
>> +    args = args[1:]
>> +
>> +    p = argparse.ArgumentParser(allow_abbrev=False)
>> +    # -o option may be specified several times
>> +    p.add_argument('-o', action='append', default=[])
>> +    p.add_argument('-f')
>> +    parsed, remaining = p.parse_known_args(args)
>> +
>> +    opts = optstr2dict(','.join(parsed.o))
>> +
>> +    compat = 'compat' in opts and opts['compat'][0] == '0'
> 
> I suppose `opts['compat'][0] == '0'` is supposed to check for compat=0.10?
> 
> If so, then why not just check `opts['compat'] == '0.10'` to be clearer?  I don’t think we allow any other compat=0* values, and I have no reason to believe we ever will.
> 
> Also, I think compat=v2 is valid, too.  (And I think calling this variable “v2” would also make more sense than “compat”.)

Good for me, will do.

> 
>> +    for k, v in imgopts.items():
>> +        if k in opts:
>> +            continue
>> +        if k == 'compression_type' and (compat or parsed.f != 'qcow2'):
>> +            continue
>> +        opts[k] = v
> 
> Could also be done with something like
> 
> imgopts = os.environ.get('IMGOPTS')

imgopts is a string after it. So you don't need to join it?

> opts = optstr2dict(','.join(([imgopts] if imgopts else []) + parsed.o))

Build a string to be than parsed looks strange IMHO..

> 
> if parsed.f != 'qcow2' or (opts.get('compat') in ['v2', '0.10']):
>      opts.pop('compression_type', None)
> 
> (Never tested, of course)
> 
> Because optstr2dict() prioritizes later options over earlier ones. (Which is good, because that’s also qemu-img’s behavior.)
> 

Ok, I'll think about this all when prepare v2, and we'll see how it goes

> 
>> +
>> +    result = ['create']
>> +    if parsed.f is not None:
>> +        result += ['-f', parsed.f]
> 
> Can this even be None?  I hope none of our tests do this.

I'm afraid they do, as I first wanted to make a default to be imgfmt, but faced tests that rely on default being raw.. May be I'm wrong. Will check it.

> 
>> +    if opts:
>> +        result += ['-o', dict2optstr(opts)]
>> +    result += remaining
>> +
>> +    return result
>> +
>>   def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
>>       """
>>       Run qemu-img and return both its output and its exit code
>>       """
>> -    full_args = qemu_img_args + list(args)
>> +    full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
>>       return qemu_tool_pipe_and_status('qemu-img', full_args)
>>   def qemu_img(*args: str) -> int:
> 
> There’s also qemu_img_verbose() which I think should be amended the same way (even if it’s currently never used for 'create').
> 

Right, thanks. I think better is rewrite qemu_img_verbose to to call qemu_img_pipe_and_status.

Another thing to do is moving handling luks from qemu_img_create to qemu_img_create_prepare_args, so all these things be in one place.


-- 
Best regards,
Vladimir

Re: [PATCH 02/14] iotests.py: qemu_img*("create"): support IMGOPTS='compression_type=zstd'

[Vladimir Sementsov-Ogievskiy]

16.07.2021 13:34, Vladimir Sementsov-Ogievskiy wrote:
> 16.07.2021 13:07, Max Reitz wrote:
>> On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
>>> Adding support of IMGOPTS (like in bash tests) allows user to pass a
>>> lot of different options. Still, some may require additional logic.
>>>
>>> Now we want compression_type option, so add some smart logic around it:
>>> ignore compression_type=zstd in IMGOPTS, if test want qcow2 in
>>> compatibility mode. As well, ignore compression_type for non-qcow2
>>> formats.
>>>
>>> Note that we may instead add support only to qemu_img_create(), but
>>> that works bad:
>>>
>>> 1. We'll have to update a lot of tests to use qemu_img_create instead
>>>     of qemu_img('create'). (still, we may want do it anyway, but no
>>>     reason to create a dependancy between task of supporting IMGOPTS and
>>>     updating a lot of tests)
>>>
>>> 2. Some tests use qemu_img_pipe('create', ..) - even more work on
>>>     updating
>>
>> I feel compelled to again say that we had a series that did exactly that.  But of course, now that so much time has passed, overhauling it would require quite a bit of work.
>>
>>> 3. Even if we update all tests to go through qemu_img_create, we'll
>>>     need a way to avoid creating new tests using qemu_img*('create') -
>>>     add assertions.. That doesn't seem good.
>>
>> That almost sounds like you remember my series, because:
>>
>> https://lists.nongnu.org/archive/html/qemu-block/2019-10/msg00135.html
>>
>> ;)
> 
> :) No, I don't remember it.
> 
>>
>>> So, let's add support of IMGOPTS to most generic
>>> qemu_img_pipe_and_status().
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/iotests.py | 48 ++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 47 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index 0d99dd841f..80f0cb4f42 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -16,6 +16,7 @@
>>>   # along with this program.  If not, see<http://www.gnu.org/licenses/>.
>>>   #
>>> +import argparse
>>>   import atexit
>>>   import bz2
>>>   from collections import OrderedDict
>>> @@ -41,6 +42,19 @@
>>>   from qemu.machine import qtest
>>>   from qemu.qmp import QMPMessage
>>> +
>>> +def optstr2dict(opts: str) -> Dict[str, str]:
>>> +    if not opts:
>>> +        return {}
>>> +
>>> +    return {arr[0]: arr[1] for arr in
>>> +            (opt.split('=', 1) for opt in opts.strip().split(','))}
>>> +
>>> +
>>> +def dict2optstr(opts: Dict[str, str]) -> str:
>>> +    return ','.join(f'{k}={v}' for k, v in opts.items())
>>> +
>>> +
>>>   # Use this logger for logging messages directly from the iotests module
>>>   logger = logging.getLogger('qemu.iotests')
>>>   logger.addHandler(logging.NullHandler())
>>> @@ -57,6 +71,8 @@
>>>   if os.environ.get('QEMU_IMG_OPTIONS'):
>>>       qemu_img_args += os.environ['QEMU_IMG_OPTIONS'].strip().split(' ')
>>> +imgopts = optstr2dict(os.environ.get('IMGOPTS', ''))
>>> +
>>>   qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>>>   if os.environ.get('QEMU_IO_OPTIONS'):
>>>       qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>>> @@ -121,11 +137,41 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
>>>                                  {-subp.returncode}: {cmd}\n')
>>>           return (output, subp.returncode)
>>> +def qemu_img_create_prepare_args(args: List[str]) -> List[str]:
>>> +    if not args or args[0] != 'create':
>>> +        return list(args)
>>> +    args = args[1:]
>>> +
>>> +    p = argparse.ArgumentParser(allow_abbrev=False)
>>> +    # -o option may be specified several times
>>> +    p.add_argument('-o', action='append', default=[])
>>> +    p.add_argument('-f')
>>> +    parsed, remaining = p.parse_known_args(args)
>>> +
>>> +    opts = optstr2dict(','.join(parsed.o))
>>> +
>>> +    compat = 'compat' in opts and opts['compat'][0] == '0'
>>
>> I suppose `opts['compat'][0] == '0'` is supposed to check for compat=0.10?
>>
>> If so, then why not just check `opts['compat'] == '0.10'` to be clearer?  I don’t think we allow any other compat=0* values, and I have no reason to believe we ever will.
>>
>> Also, I think compat=v2 is valid, too.  (And I think calling this variable “v2” would also make more sense than “compat”.)
> 
> Good for me, will do.
> 
>>
>>> +    for k, v in imgopts.items():
>>> +        if k in opts:
>>> +            continue
>>> +        if k == 'compression_type' and (compat or parsed.f != 'qcow2'):
>>> +            continue
>>> +        opts[k] = v
>>
>> Could also be done with something like
>>
>> imgopts = os.environ.get('IMGOPTS')
> 
> imgopts is a string after it. So you don't need to join it?
> 
>> opts = optstr2dict(','.join(([imgopts] if imgopts else []) + parsed.o))
> 
> Build a string to be than parsed looks strange IMHO..

Oh, but that's exactly what I should do anyway to cover several -o options. Now I see that what you write is correct.

> 
>>
>> if parsed.f != 'qcow2' or (opts.get('compat') in ['v2', '0.10']):
>>      opts.pop('compression_type', None)
>>
>> (Never tested, of course)
>>
>> Because optstr2dict() prioritizes later options over earlier ones. (Which is good, because that’s also qemu-img’s behavior.)
>>
> 
> Ok, I'll think about this all when prepare v2, and we'll see how it goes
> 
>>
>>> +
>>> +    result = ['create']
>>> +    if parsed.f is not None:
>>> +        result += ['-f', parsed.f]
>>
>> Can this even be None?  I hope none of our tests do this.
> 
> I'm afraid they do, as I first wanted to make a default to be imgfmt, but faced tests that rely on default being raw.. May be I'm wrong. Will check it.
> 
>>
>>> +    if opts:
>>> +        result += ['-o', dict2optstr(opts)]
>>> +    result += remaining
>>> +
>>> +    return result
>>> +
>>>   def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
>>>       """
>>>       Run qemu-img and return both its output and its exit code
>>>       """
>>> -    full_args = qemu_img_args + list(args)
>>> +    full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
>>>       return qemu_tool_pipe_and_status('qemu-img', full_args)
>>>   def qemu_img(*args: str) -> int:
>>
>> There’s also qemu_img_verbose() which I think should be amended the same way (even if it’s currently never used for 'create').
>>
> 
> Right, thanks. I think better is rewrite qemu_img_verbose to to call qemu_img_pipe_and_status.
> 
> Another thing to do is moving handling luks from qemu_img_create to qemu_img_create_prepare_args, so all these things be in one place.
> 
> 


-- 
Best regards,
Vladimir

Re: [PATCH 02/14] iotests.py: qemu_img*("create"): support IMGOPTS='compression_type=zstd'

[Vladimir Sementsov-Ogievskiy]

19.07.2021 15:58, Vladimir Sementsov-Ogievskiy wrote:
>>>>
>>>
>>> Could also be done with something like
>>>
>>> imgopts = os.environ.get('IMGOPTS')
>>
>> imgopts is a string after it. So you don't need to join it?
>>
>>> opts = optstr2dict(','.join(([imgopts] if imgopts else []) + parsed.o))
>>
>> Build a string to be than parsed looks strange IMHO..
> 
> Oh, but that's exactly what I should do anyway to cover several -o options. Now I see that what you write is correct.
> 
>>
>>>
>>> if parsed.f != 'qcow2' or (opts.get('compat') in ['v2', '0.10']):
>>>      opts.pop('compression_type', None)
>>>
>>> (Never tested, of course)
>>>
>>> Because optstr2dict() prioritizes later options over earlier ones. (Which is good, because that’s also qemu-img’s behavior.)
>>>
>>
>> Ok, I'll think about this all when prepare v2, and we'll see how it goes 

This way you also drop compression_type if test specify it. I think we shouldn't touch test specified options. Let it clearly fail instead.

We only want to ignore compression_type in IMGOPTS when create non-qcow2 image. I think I'll drop checking for compat: the only user for this check ic 065 and it's simpler to explicitly set compression_type in it even for compat=0.10 cases.

-- 
Best regards,
Vladimir

[PATCH 03/14] iotest 303: explicit compression type

[Vladimir Sementsov-Ogievskiy]

The test prints qcow2 header fields which depends on chosen compression
type. So, let's be explicit in what compression type we want and
independent of IMGOPTS. Test both existing compression types.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/303     | 25 ++++++++++++++++---------
 tests/qemu-iotests/303.out | 30 +++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
index 425544c064..9dee2bdfb8 100755
--- a/tests/qemu-iotests/303
+++ b/tests/qemu-iotests/303
@@ -53,12 +53,19 @@ def add_bitmap(num, begin, end, disabled):
     log('')
 
 
-qemu_img_create('-f', iotests.imgfmt, disk, '10M')
-
-add_bitmap(1, 0, 6, False)
-add_bitmap(2, 6, 8, True)
-dump = ['./qcow2.py', disk, 'dump-header']
-subprocess.run(dump)
-# Dump the metadata in JSON format
-dump.append('-j')
-subprocess.run(dump)
+def test(compression_type: str, json_output: bool) -> None:
+    qemu_img_create('-f', iotests.imgfmt,
+                    '-o', f'compression_type={compression_type}',
+                    disk, '10M')
+    add_bitmap(1, 0, 6, False)
+    add_bitmap(2, 6, 8, True)
+
+    cmd = ['./qcow2.py', disk, 'dump-header']
+    if json_output:
+        cmd.append('-j')
+
+    subprocess.run(cmd)
+
+
+test('zlib', False)
+test('zstd', True)
diff --git a/tests/qemu-iotests/303.out b/tests/qemu-iotests/303.out
index 7c16998587..b3c70827b7 100644
--- a/tests/qemu-iotests/303.out
+++ b/tests/qemu-iotests/303.out
@@ -80,6 +80,34 @@ extra_data_size           0
 Bitmap table   type            size         offset
 0              all-zeroes      0            0
 
+Add bitmap 1
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 1048576
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 2097152
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 3145728
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 4194304
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 5242880
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+
+Add bitmap 2
+wrote 1048576/1048576 bytes at offset 6291456
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 1048576/1048576 bytes at offset 7340032
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+
 {
     "magic": 1363560955,
     "version": 3,
@@ -94,7 +122,7 @@ Bitmap table   type            size         offset
     "refcount_table_clusters": 1,
     "nb_snapshots": 0,
     "snapshot_offset": 0,
-    "incompatible_features": 0,
+    "incompatible_features": 8,
     "compatible_features": 0,
     "autoclear_features": 1,
     "refcount_order": 4,
-- 
2.29.2

Re: [PATCH 03/14] iotest 303: explicit compression type

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> The test prints qcow2 header fields which depends on chosen compression
> type. So, let's be explicit in what compression type we want and
> independent of IMGOPTS. Test both existing compression types.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/303     | 25 ++++++++++++++++---------
>   tests/qemu-iotests/303.out | 30 +++++++++++++++++++++++++++++-
>   2 files changed, 45 insertions(+), 10 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[PATCH 04/14] iotest 065: explicit compression type

[Vladimir Sementsov-Ogievskiy]

The test checks different options. It of course fails if set
IMGOPTS='compression_type=zstd'. So, let's be explicit in what
compression type we want and independent of IMGOPTS. Test both existing
compression types.

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

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index 3c2ca27627..22203ed480 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -96,17 +96,17 @@ class TestQCow2(TestQemuImgInfo):
 
 class TestQCow3NotLazy(TestQemuImgInfo):
     '''Testing a qcow2 version 3 image with lazy refcounts disabled'''
-    img_options = 'compat=1.1,lazy_refcounts=off'
+    img_options = 'compat=1.1,lazy_refcounts=off,compression_type=zstd'
     json_compare = { 'compat': '1.1', 'lazy-refcounts': False,
                      'refcount-bits': 16, 'corrupt': False,
-                     'compression-type': 'zlib', 'extended-l2': False }
-    human_compare = [ 'compat: 1.1', 'compression type: zlib',
+                     'compression-type': 'zstd', 'extended-l2': False }
+    human_compare = [ 'compat: 1.1', 'compression type: zstd',
                       'lazy refcounts: false', 'refcount bits: 16',
                       'corrupt: false', 'extended l2: false' ]
 
 class TestQCow3Lazy(TestQemuImgInfo):
     '''Testing a qcow2 version 3 image with lazy refcounts enabled'''
-    img_options = 'compat=1.1,lazy_refcounts=on'
+    img_options = 'compat=1.1,lazy_refcounts=on,compression_type=zlib'
     json_compare = { 'compat': '1.1', 'lazy-refcounts': True,
                      'refcount-bits': 16, 'corrupt': False,
                      'compression-type': 'zlib', 'extended-l2': False }
@@ -117,7 +117,7 @@ class TestQCow3Lazy(TestQemuImgInfo):
 class TestQCow3NotLazyQMP(TestQMP):
     '''Testing a qcow2 version 3 image with lazy refcounts disabled, opening
        with lazy refcounts enabled'''
-    img_options = 'compat=1.1,lazy_refcounts=off'
+    img_options = 'compat=1.1,lazy_refcounts=off,compression_type=zlib'
     qemu_options = 'lazy-refcounts=on'
     compare = { 'compat': '1.1', 'lazy-refcounts': False,
                 'refcount-bits': 16, 'corrupt': False,
@@ -127,11 +127,11 @@ class TestQCow3NotLazyQMP(TestQMP):
 class TestQCow3LazyQMP(TestQMP):
     '''Testing a qcow2 version 3 image with lazy refcounts enabled, opening
        with lazy refcounts disabled'''
-    img_options = 'compat=1.1,lazy_refcounts=on'
+    img_options = 'compat=1.1,lazy_refcounts=on,compression_type=zstd'
     qemu_options = 'lazy-refcounts=off'
     compare = { 'compat': '1.1', 'lazy-refcounts': True,
                 'refcount-bits': 16, 'corrupt': False,
-                'compression-type': 'zlib', 'extended-l2': False }
+                'compression-type': 'zstd', 'extended-l2': False }
 
 TestImageInfoSpecific = None
 TestQemuImgInfo = None
-- 
2.29.2

Re: [PATCH 04/14] iotest 065: explicit compression type

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> The test checks different options. It of course fails if set
> IMGOPTS='compression_type=zstd'. So, let's be explicit in what
> compression type we want and independent of IMGOPTS. Test both existing
> compression types.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/065 | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[PATCH 05/14] iotests.py: filter compression type out

[Vladimir Sementsov-Ogievskiy]

We want iotests pass with both the default zlib compression and with
IMGOPTS='compression_type=zstd'.

Actually the only test that is interested in real compression type in
test output is 287 (test for qcow2 compression type) and it's in bash.
So for now we can safely filter out compression type in all qcow2
tests.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/206.out    | 10 +++---
 tests/qemu-iotests/242.out    | 10 +++---
 tests/qemu-iotests/255.out    |  8 ++---
 tests/qemu-iotests/274.out    | 68 +++++++++++++++++------------------
 tests/qemu-iotests/280.out    |  2 +-
 tests/qemu-iotests/iotests.py | 13 ++++++-
 6 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out
index b68c443867..253209eca9 100644
--- a/tests/qemu-iotests/206.out
+++ b/tests/qemu-iotests/206.out
@@ -18,7 +18,7 @@ virtual size: 128 MiB (134217728 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -42,7 +42,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -66,7 +66,7 @@ virtual size: 32 MiB (33554432 bytes)
 cluster_size: 2097152
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 1
     corrupt: false
@@ -92,7 +92,7 @@ backing file: TEST_IMG.base
 backing file format: IMGFMT
 Format specific information:
     compat: 0.10
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     refcount bits: 16
 
 === Successful image creation (encrypted) ===
@@ -109,7 +109,7 @@ encrypted: yes
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     encrypt:
diff --git a/tests/qemu-iotests/242.out b/tests/qemu-iotests/242.out
index 3759c99284..ce231424a7 100644
--- a/tests/qemu-iotests/242.out
+++ b/tests/qemu-iotests/242.out
@@ -12,7 +12,7 @@ virtual size: 1 MiB (1048576 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -34,7 +34,7 @@ virtual size: 1 MiB (1048576 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     bitmaps:
         [0]:
@@ -68,7 +68,7 @@ virtual size: 1 MiB (1048576 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     bitmaps:
         [0]:
@@ -110,7 +110,7 @@ virtual size: 1 MiB (1048576 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     bitmaps:
         [0]:
@@ -161,7 +161,7 @@ virtual size: 1 MiB (1048576 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     bitmaps:
         [0]:
diff --git a/tests/qemu-iotests/255.out b/tests/qemu-iotests/255.out
index 33b7f22de3..bd3b13474e 100644
--- a/tests/qemu-iotests/255.out
+++ b/tests/qemu-iotests/255.out
@@ -3,9 +3,9 @@ Finishing a commit job with background reads
 
 === Create backing chain and start VM ===
 
-Formatting 'TEST_DIR/PID-t.qcow2.mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=134217728 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-t.qcow2.mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=134217728 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=134217728 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-t.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=134217728 lazy_refcounts=off refcount_bits=16
 
 === Start background read requests ===
 
@@ -23,9 +23,9 @@ Closing the VM while a job is being cancelled
 
 === Create images and start VM ===
 
-Formatting 'TEST_DIR/PID-src.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=134217728 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-src.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=134217728 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-dst.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=134217728 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-dst.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=134217728 lazy_refcounts=off refcount_bits=16
 
 wrote 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out
index cfe17a8659..eb23acd5ed 100644
--- a/tests/qemu-iotests/274.out
+++ b/tests/qemu-iotests/274.out
@@ -1,9 +1,9 @@
 == Commit tests ==
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -53,7 +53,7 @@ backing file: TEST_DIR/PID-base
 backing file format: IMGFMT
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -66,11 +66,11 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing HMP commit (top -> mid) ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -85,7 +85,7 @@ backing file: TEST_DIR/PID-base
 backing file format: IMGFMT
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -98,11 +98,11 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing QMP active commit (top -> mid) ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -123,7 +123,7 @@ backing file: TEST_DIR/PID-base
 backing file format: IMGFMT
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -136,11 +136,11 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing qemu-img commit (top -> base) ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -153,7 +153,7 @@ virtual size: 2 MiB (2097152 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -166,11 +166,11 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing QMP active commit (top -> base) ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-mid', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=1048576 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=2097152 backing_file=TEST_DIR/PID-mid backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -191,7 +191,7 @@ backing file: TEST_DIR/PID-base
 backing file format: IMGFMT
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -205,9 +205,9 @@ read 1048576/1048576 bytes at offset 1048576
 
 == Resize tests ==
 === preallocation=off ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=6442450944 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=6442450944 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1073741824 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=1073741824 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 5368709120
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -224,9 +224,9 @@ read 65536/65536 bytes at offset 5368709120
 { "start": 1073741824, "length": 7516192768, "depth": 0, "zero": true, "data": false}]
 
 === preallocation=metadata ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=34359738368 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=34359738368 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=32212254720 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=32212254720 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 33285996544
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -248,9 +248,9 @@ read 65536/65536 bytes at offset 33285996544
 { "start": 34896609280, "length": 536870912, "depth": 0, "zero": true, "data": false, "offset": 2685075456}]
 
 === preallocation=falloc ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=10485760 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=10485760 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=5242880 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=5242880 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 9437184
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -267,9 +267,9 @@ read 65536/65536 bytes at offset 9437184
 { "start": 5242880, "length": 10485760, "depth": 0, "zero": false, "data": true, "offset": 327680}]
 
 === preallocation=full ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=16777216 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=16777216 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=8388608 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=8388608 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 11534336
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -286,9 +286,9 @@ read 65536/65536 bytes at offset 11534336
 { "start": 8388608, "length": 4194304, "depth": 0, "zero": false, "data": true, "offset": 327680}]
 
 === preallocation=off ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=393216 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=393216 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=259072 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=259072 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 259072
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -306,9 +306,9 @@ read 65536/65536 bytes at offset 259072
 { "start": 262144, "length": 262144, "depth": 0, "zero": true, "data": false}]
 
 === preallocation=off ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=409600 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=409600 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=262144 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=262144 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 344064
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -325,9 +325,9 @@ read 65536/65536 bytes at offset 344064
 { "start": 262144, "length": 262144, "depth": 0, "zero": true, "data": false}]
 
 === preallocation=off ===
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=524288 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=524288 lazy_refcounts=off refcount_bits=16
 
-Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=262144 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-top', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=262144 backing_file=TEST_DIR/PID-base backing_fmt=qcow2 lazy_refcounts=off refcount_bits=16
 
 wrote 65536/65536 bytes at offset 446464
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
diff --git a/tests/qemu-iotests/280.out b/tests/qemu-iotests/280.out
index 09a0f1a7cb..a40682d902 100644
--- a/tests/qemu-iotests/280.out
+++ b/tests/qemu-iotests/280.out
@@ -1,4 +1,4 @@
-Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=67108864 lazy_refcounts=off refcount_bits=16
+Formatting 'TEST_DIR/PID-base', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=COMPRESSION_TYPE size=67108864 lazy_refcounts=off refcount_bits=16
 
 === Launch VM ===
 Enabling migration QMP events on VM...
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 80f0cb4f42..6a8cc1bad7 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -224,9 +224,18 @@ def qemu_img_verbose(*args):
                          % (-exitcode, ' '.join(qemu_img_args + list(args))))
     return exitcode
 
+def filter_img_create(text: str) -> str:
+    return re.sub('(compression_type=)(zlib|zstd)', r'\1COMPRESSION_TYPE',
+                  text)
+
 def qemu_img_pipe(*args: str) -> str:
     '''Run qemu-img and return its output'''
-    return qemu_img_pipe_and_status(*args)[0]
+    output =  qemu_img_pipe_and_status(*args)[0]
+
+    if args[0] == 'create':
+        return filter_img_create(output)
+
+    return output
 
 def qemu_img_log(*args):
     result = qemu_img_pipe(*args)
@@ -479,6 +488,8 @@ def filter_img_info(output, filename):
                       'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX',
                       line)
         line = re.sub('cid: [0-9]+', 'cid: XXXXXXXXXX', line)
+        line = re.sub('(compression type: )(zlib|zstd)', r'\1COMPRESSION_TYPE',
+                      line)
         lines.append(line)
     return '\n'.join(lines)
 
-- 
2.29.2

Re: [PATCH 05/14] iotests.py: filter compression type out

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> We want iotests pass with both the default zlib compression and with
> IMGOPTS='compression_type=zstd'.
>
> Actually the only test that is interested in real compression type in
> test output is 287 (test for qcow2 compression type) and it's in bash.
> So for now we can safely filter out compression type in all qcow2
> tests.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/206.out    | 10 +++---
>   tests/qemu-iotests/242.out    | 10 +++---
>   tests/qemu-iotests/255.out    |  8 ++---
>   tests/qemu-iotests/274.out    | 68 +++++++++++++++++------------------
>   tests/qemu-iotests/280.out    |  2 +-
>   tests/qemu-iotests/iotests.py | 13 ++++++-
>   6 files changed, 61 insertions(+), 50 deletions(-)

Looks OK, though I wonder if it weren’t better to have a filter that 
only prints some options and explicitly filters out everything else.  
(Well, actually, I’d prefer not to have the “Formatting…” line in the 
reference output at all, because I don’t see the point, but I suppose 
that can be considered a different problem.)

[...]

> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 80f0cb4f42..6a8cc1bad7 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -224,9 +224,18 @@ def qemu_img_verbose(*args):
>                            % (-exitcode, ' '.join(qemu_img_args + list(args))))
>       return exitcode
>   
> +def filter_img_create(text: str) -> str:
> +    return re.sub('(compression_type=)(zlib|zstd)', r'\1COMPRESSION_TYPE',
> +                  text)
> +
>   def qemu_img_pipe(*args: str) -> str:
>       '''Run qemu-img and return its output'''
> -    return qemu_img_pipe_and_status(*args)[0]
> +    output =  qemu_img_pipe_and_status(*args)[0]

There’s a superfluous space after '='.

> +
> +    if args[0] == 'create':
> +        return filter_img_create(output)
> +
> +    return output

Wouldn’t it make more sense to have this filter be in 
qemu_img_pipe_and_status()?

Max

>   def qemu_img_log(*args):
>       result = qemu_img_pipe(*args)
> @@ -479,6 +488,8 @@ def filter_img_info(output, filename):
>                         'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX',
>                         line)
>           line = re.sub('cid: [0-9]+', 'cid: XXXXXXXXXX', line)
> +        line = re.sub('(compression type: )(zlib|zstd)', r'\1COMPRESSION_TYPE',
> +                      line)
>           lines.append(line)
>       return '\n'.join(lines)
>   

Re: [PATCH 05/14] iotests.py: filter compression type out

[Vladimir Sementsov-Ogievskiy]

16.07.2021 14:15, Max Reitz wrote:
> On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
>> We want iotests pass with both the default zlib compression and with
>> IMGOPTS='compression_type=zstd'.
>>
>> Actually the only test that is interested in real compression type in
>> test output is 287 (test for qcow2 compression type) and it's in bash.
>> So for now we can safely filter out compression type in all qcow2
>> tests.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/206.out    | 10 +++---
>>   tests/qemu-iotests/242.out    | 10 +++---
>>   tests/qemu-iotests/255.out    |  8 ++---
>>   tests/qemu-iotests/274.out    | 68 +++++++++++++++++------------------
>>   tests/qemu-iotests/280.out    |  2 +-
>>   tests/qemu-iotests/iotests.py | 13 ++++++-
>>   6 files changed, 61 insertions(+), 50 deletions(-)
> 
> Looks OK, though I wonder if it weren’t better to have a filter that only prints some options and explicitly filters out everything else.

That means larger work and larger audit of what actually each test wants to see in the output..

  (Well, actually, I’d prefer not to have the “Formatting…” line in the reference output at all, because I don’t see the point, but I suppose that can be considered a different problem.)

Hmm. I like the idea of dropping this line, I don't remember any bug that this line helped to catch, but we have to update it every time we add some new option. I can make a separate patch in v2 to just filter it out everywhere.

> 
> [...]
> 
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 80f0cb4f42..6a8cc1bad7 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -224,9 +224,18 @@ def qemu_img_verbose(*args):
>>                            % (-exitcode, ' '.join(qemu_img_args + list(args))))
>>       return exitcode
>> +def filter_img_create(text: str) -> str:
>> +    return re.sub('(compression_type=)(zlib|zstd)', r'\1COMPRESSION_TYPE',
>> +                  text)
>> +
>>   def qemu_img_pipe(*args: str) -> str:
>>       '''Run qemu-img and return its output'''
>> -    return qemu_img_pipe_and_status(*args)[0]
>> +    output =  qemu_img_pipe_and_status(*args)[0]
> 
> There’s a superfluous space after '='.
> 
>> +
>> +    if args[0] == 'create':
>> +        return filter_img_create(output)
>> +
>> +    return output
> 
> Wouldn’t it make more sense to have this filter be in qemu_img_pipe_and_status()?
> 

Hmm probably someone want to not filter information out, then qemu_img_pipe_and_status() will be a way to get unfiltered output..

But I tend to agree, as in 02 I do generic logic in qemu_img_pipe_and_status(), so until we have a good reason, it's better to keep all generic logic in one place. OK, will move it to qemu_img_pipe_and_status()

> 
>>   def qemu_img_log(*args):
>>       result = qemu_img_pipe(*args)
>> @@ -479,6 +488,8 @@ def filter_img_info(output, filename):
>>                         'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX',
>>                         line)
>>           line = re.sub('cid: [0-9]+', 'cid: XXXXXXXXXX', line)
>> +        line = re.sub('(compression type: )(zlib|zstd)', r'\1COMPRESSION_TYPE',
>> +                      line)
>>           lines.append(line)
>>       return '\n'.join(lines)
> 


-- 
Best regards,
Vladimir

[PATCH 06/14] iotest 302: use img_info_log() helper

[Vladimir Sementsov-Ogievskiy]

Instead of qemu_img_log("info", ..) use generic helper img_info_log().

img_info_log() has smarter logic. For example it use filter_img_info()
to filter output, which in turns filter a compression type. So it will
help us in future when we implement a possibility to use zstd
compression by default (with help of some runtime config file or maybe
build option). For now to test you should recompile qemu with a small
patch:

    --- a/block/qcow2.c
    +++ b/block/qcow2.c
    @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
             }
         }

    +    if (!qcow2_opts->has_compression_type && version >= 3) {
    +        qcow2_opts->has_compression_type = true;
    +        qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
    +    }
    +
         if (qcow2_opts->has_compression_type &&
             qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/302     | 3 ++-
 tests/qemu-iotests/302.out | 7 +++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
index 5695af4914..2180dbc896 100755
--- a/tests/qemu-iotests/302
+++ b/tests/qemu-iotests/302
@@ -34,6 +34,7 @@ from iotests import (
     qemu_img_measure,
     qemu_io,
     qemu_nbd_popen,
+    img_info_log,
 )
 
 iotests.script_initialize(supported_fmts=["qcow2"])
@@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar:
             nbd_uri)
 
         iotests.log("=== Converted image info ===")
-        qemu_img_log("info", nbd_uri)
+        img_info_log(nbd_uri)
 
         iotests.log("=== Converted image check ===")
         qemu_img_log("check", nbd_uri)
diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out
index e2f6077e83..3e7c281b91 100644
--- a/tests/qemu-iotests/302.out
+++ b/tests/qemu-iotests/302.out
@@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes)
 disk size: unavailable
 
 === Converted image info ===
-image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock
-file format: qcow2
+image: TEST_IMG
+file format: IMGFMT
 virtual size: 1 GiB (1073741824 bytes)
-disk size: unavailable
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
-- 
2.29.2

Re: [PATCH 06/14] iotest 302: use img_info_log() helper

[Vladimir Sementsov-Ogievskiy]

05.07.2021 12:15, Vladimir Sementsov-Ogievskiy wrote:
> Instead of qemu_img_log("info", ..) use generic helper img_info_log().
> 
> img_info_log() has smarter logic. For example it use filter_img_info()
> to filter output, which in turns filter a compression type. So it will
> help us in future when we implement a possibility to use zstd
> compression by default (with help of some runtime config file or maybe
> build option). For now to test you should recompile qemu with a small
> patch:
> 
>      --- a/block/qcow2.c
>      +++ b/block/qcow2.c
>      @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>               }
>           }
> 
>      +    if (!qcow2_opts->has_compression_type && version >= 3) {
>      +        qcow2_opts->has_compression_type = true;
>      +        qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
>      +    }
>      +
>           if (qcow2_opts->has_compression_type &&
>               qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com>

Wow, that was bad idea to insert patch into commit message even with indent: it breaks rpm build for me.

So, reword like this:

     build option). For now to test you should recompile qemu with a small
     addition into block/qcow2.c before
       "if (qcow2_opts->has_compression_type":
     
         if (!qcow2_opts->has_compression_type && version >= 3) {
             qcow2_opts->has_compression_type = true;
             qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
         }


-- 
Best regards,
Vladimir

Re: [PATCH 06/14] iotest 302: use img_info_log() helper

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> Instead of qemu_img_log("info", ..) use generic helper img_info_log().
>
> img_info_log() has smarter logic. For example it use filter_img_info()
> to filter output, which in turns filter a compression type. So it will
> help us in future when we implement a possibility to use zstd
> compression by default (with help of some runtime config file or maybe
> build option). For now to test you should recompile qemu with a small
> patch:
>
>      --- a/block/qcow2.c
>      +++ b/block/qcow2.c
>      @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>               }
>           }
>
>      +    if (!qcow2_opts->has_compression_type && version >= 3) {
>      +        qcow2_opts->has_compression_type = true;
>      +        qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
>      +    }
>      +
>           if (qcow2_opts->has_compression_type &&
>               qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/302     | 3 ++-
>   tests/qemu-iotests/302.out | 7 +++----
>   2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
> index 5695af4914..2180dbc896 100755
> --- a/tests/qemu-iotests/302
> +++ b/tests/qemu-iotests/302
> @@ -34,6 +34,7 @@ from iotests import (
>       qemu_img_measure,
>       qemu_io,
>       qemu_nbd_popen,
> +    img_info_log,
>   )
>   
>   iotests.script_initialize(supported_fmts=["qcow2"])
> @@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar:
>               nbd_uri)
>   
>           iotests.log("=== Converted image info ===")
> -        qemu_img_log("info", nbd_uri)
> +        img_info_log(nbd_uri)

There’s another `qemu_img_log("info", nbd_uri)` call above this place.  
We can’t use `img_info_log()` there, because in that case, the image is 
not in qcow2 format (which is the test’s image format), but 
`img_info_log()` enforces “-f {imgfmt}”.  It would have been nice to 
have a comment on that somewhere, though.

But, well.

Reviewed-by: Max Reitz <mreitz@redhat.com>

(And speaking in principle, I don’t think I like the broad 
img_info_log() very much anyway, because I feel like tests should rather 
only have the actually relevant bits in their reference outputs…)

>   
>           iotests.log("=== Converted image check ===")
>           qemu_img_log("check", nbd_uri)
> diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out
> index e2f6077e83..3e7c281b91 100644
> --- a/tests/qemu-iotests/302.out
> +++ b/tests/qemu-iotests/302.out
> @@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes)
>   disk size: unavailable
>   
>   === Converted image info ===
> -image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock
> -file format: qcow2
> +image: TEST_IMG
> +file format: IMGFMT
>   virtual size: 1 GiB (1073741824 bytes)
> -disk size: unavailable
>   cluster_size: 65536
>   Format specific information:
>       compat: 1.1
> -    compression type: zlib
> +    compression type: COMPRESSION_TYPE
>       lazy refcounts: false
>       refcount bits: 16
>       corrupt: false

Re: [PATCH 06/14] iotest 302: use img_info_log() helper

[Vladimir Sementsov-Ogievskiy]

16.07.2021 14:39, Max Reitz wrote:
> On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
>> Instead of qemu_img_log("info", ..) use generic helper img_info_log().
>>
>> img_info_log() has smarter logic. For example it use filter_img_info()
>> to filter output, which in turns filter a compression type. So it will
>> help us in future when we implement a possibility to use zstd
>> compression by default (with help of some runtime config file or maybe
>> build option). For now to test you should recompile qemu with a small
>> patch:
>>
>>      --- a/block/qcow2.c
>>      +++ b/block/qcow2.c
>>      @@ -3540,6 +3540,11 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>>               }
>>           }
>>
>>      +    if (!qcow2_opts->has_compression_type && version >= 3) {
>>      +        qcow2_opts->has_compression_type = true;
>>      +        qcow2_opts->compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
>>      +    }
>>      +
>>           if (qcow2_opts->has_compression_type &&
>>               qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/302     | 3 ++-
>>   tests/qemu-iotests/302.out | 7 +++----
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
>> index 5695af4914..2180dbc896 100755
>> --- a/tests/qemu-iotests/302
>> +++ b/tests/qemu-iotests/302
>> @@ -34,6 +34,7 @@ from iotests import (
>>       qemu_img_measure,
>>       qemu_io,
>>       qemu_nbd_popen,
>> +    img_info_log,
>>   )
>>   iotests.script_initialize(supported_fmts=["qcow2"])
>> @@ -99,7 +100,7 @@ with tarfile.open(tar_file, "w") as tar:
>>               nbd_uri)
>>           iotests.log("=== Converted image info ===")
>> -        qemu_img_log("info", nbd_uri)
>> +        img_info_log(nbd_uri)
> 
> There’s another `qemu_img_log("info", nbd_uri)` call above this place. We can’t use `img_info_log()` there, because in that case, the image is not in qcow2 format (which is the test’s image format), but `img_info_log()` enforces “-f {imgfmt}”.  It would have been nice to have a comment on that somewhere, though.

I'll add a comment. Actually, I only fixed places which breaks when I set zstd by default. That's why some things may be not covered.

> 
> But, well.
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> (And speaking in principle, I don’t think I like the broad img_info_log() very much anyway, because I feel like tests should rather only have the actually relevant bits in their reference outputs…)

I agree, extra useless information in test outputs is always a pain.. We should pay more attention to it when add new tests.

> 
>>           iotests.log("=== Converted image check ===")
>>           qemu_img_log("check", nbd_uri)
>> diff --git a/tests/qemu-iotests/302.out b/tests/qemu-iotests/302.out
>> index e2f6077e83..3e7c281b91 100644
>> --- a/tests/qemu-iotests/302.out
>> +++ b/tests/qemu-iotests/302.out
>> @@ -6,14 +6,13 @@ virtual size: 448 KiB (458752 bytes)
>>   disk size: unavailable
>>   === Converted image info ===
>> -image: nbd+unix:///exp?socket=SOCK_DIR/PID-nbd-sock
>> -file format: qcow2
>> +image: TEST_IMG
>> +file format: IMGFMT
>>   virtual size: 1 GiB (1073741824 bytes)
>> -disk size: unavailable
>>   cluster_size: 65536
>>   Format specific information:
>>       compat: 1.1
>> -    compression type: zlib
>> +    compression type: COMPRESSION_TYPE
>>       lazy refcounts: false
>>       refcount bits: 16
>>       corrupt: false
> 


-- 
Best regards,
Vladimir

[PATCH 07/14] qcow2: simple case support for downgrading of qcow2 images with zstd

[Vladimir Sementsov-Ogievskiy]

If image doesn't have any compressed cluster we can easily switch to
zlib compression, which may allow to downgrade the image.

That's mostly needed to support IMGOPTS='compression_type=zstd' in some
iotests which do qcow2 downgrade.

While being here also fix checkpatch complain against '#' in printf
formatting.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ee4530cdbd..bed3354474 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5221,6 +5221,38 @@ static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
                                         qiov->size, qiov, 0, 0);
 }
 
+static int qcow2_has_compressed_clusters(BlockDriverState *bs)
+{
+    int64_t offset = 0;
+    int64_t bytes = bdrv_getlength(bs);
+
+    if (bytes < 0) {
+        return bytes;
+    }
+
+    while (bytes != 0) {
+        int ret;
+        QCow2SubclusterType type;
+        unsigned int cur_bytes = MIN(INT_MAX, bytes);
+        uint64_t host_offset;
+
+        ret = qcow2_get_host_offset(bs, offset, &cur_bytes, &host_offset,
+                                    &type);
+        if (ret < 0) {
+            return ret;
+        }
+
+        if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
+            return 1;
+        }
+
+        offset += cur_bytes;
+        bytes -= cur_bytes;
+    }
+
+    return 0;
+}
+
 /*
  * Downgrades an image's version. To achieve this, any incompatible features
  * have to be removed.
@@ -5278,9 +5310,10 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
      * the first place; if that happens nonetheless, returning -ENOTSUP is the
      * best thing to do anyway */
 
-    if (s->incompatible_features) {
+    if (s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION) {
         error_setg(errp, "Cannot downgrade an image with incompatible features "
-                   "%#" PRIx64 " set", s->incompatible_features);
+                   "0x%" PRIx64 " set",
+                   s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION);
         return -ENOTSUP;
     }
 
@@ -5298,6 +5331,27 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
         return ret;
     }
 
+    if (s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION) {
+        ret = qcow2_has_compressed_clusters(bs);
+        if (ret < 0) {
+            error_setg(errp, "Failed to check block status");
+            return -EINVAL;
+        }
+        if (ret) {
+            error_setg(errp, "Cannot downgrade an image with zstd compression "
+                       "type and existing compressed clusters");
+            return -ENOTSUP;
+        }
+        /*
+         * No compressed clusters for now, so just chose default zlib
+         * compression.
+         */
+        s->incompatible_features = 0;
+        s->compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
+    }
+
+    assert(s->incompatible_features == 0);
+
     s->qcow_version = target_version;
     ret = qcow2_update_header(bs);
     if (ret < 0) {
-- 
2.29.2

Re: [PATCH 07/14] qcow2: simple case support for downgrading of qcow2 images with zstd

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> If image doesn't have any compressed cluster we can easily switch to
> zlib compression, which may allow to downgrade the image.
>
> That's mostly needed to support IMGOPTS='compression_type=zstd' in some
> iotests which do qcow2 downgrade.
>
> While being here also fix checkpatch complain against '#' in printf
> formatting.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/qcow2.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index ee4530cdbd..bed3354474 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -5221,6 +5221,38 @@ static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
>                                           qiov->size, qiov, 0, 0);
>   }
>   
> +static int qcow2_has_compressed_clusters(BlockDriverState *bs)
> +{
> +    int64_t offset = 0;
> +    int64_t bytes = bdrv_getlength(bs);
> +
> +    if (bytes < 0) {
> +        return bytes;
> +    }
> +
> +    while (bytes != 0) {
> +        int ret;
> +        QCow2SubclusterType type;
> +        unsigned int cur_bytes = MIN(INT_MAX, bytes);
> +        uint64_t host_offset;
> +
> +        ret = qcow2_get_host_offset(bs, offset, &cur_bytes, &host_offset,
> +                                    &type);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +
> +        if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
> +            return 1;
> +        }
> +
> +        offset += cur_bytes;
> +        bytes -= cur_bytes;
> +    }
> +
> +    return 0;
> +}
> +
>   /*
>    * Downgrades an image's version. To achieve this, any incompatible features
>    * have to be removed.
> @@ -5278,9 +5310,10 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
>        * the first place; if that happens nonetheless, returning -ENOTSUP is the
>        * best thing to do anyway */
>   
> -    if (s->incompatible_features) {
> +    if (s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION) {
>           error_setg(errp, "Cannot downgrade an image with incompatible features "
> -                   "%#" PRIx64 " set", s->incompatible_features);
> +                   "0x%" PRIx64 " set",
> +                   s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION);
>           return -ENOTSUP;
>       }
>   
> @@ -5298,6 +5331,27 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
>           return ret;
>       }
>   
> +    if (s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION) {
> +        ret = qcow2_has_compressed_clusters(bs);
> +        if (ret < 0) {
> +            error_setg(errp, "Failed to check block status");
> +            return -EINVAL;
> +        }
> +        if (ret) {
> +            error_setg(errp, "Cannot downgrade an image with zstd compression "

Perhaps s/zstd/non-zlib/?

Like, really “perhaps”.  Right now I think this is the better error 
message, it’s just that “non-zlib” is more technically correct and 
theoretically future-proof.

> +                       "type and existing compressed clusters");
> +            return -ENOTSUP;
> +        }
> +        /*
> +         * No compressed clusters for now, so just chose default zlib
> +         * compression.
> +         */
> +        s->incompatible_features = 0;

Not wrong, though I’d prefer

s->incompatible_features &= ~QCOW2_INCOMPAT_COMPRESSION;

Anyway:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +        s->compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
> +    }
> +
> +    assert(s->incompatible_features == 0);
> +
>       s->qcow_version = target_version;
>       ret = qcow2_update_header(bs);
>       if (ret < 0) {

[PATCH 08/14] iotests/common.rc: _make_test_img(): smarter compressiont_type handling

[Vladimir Sementsov-Ogievskiy]

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
+                # 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\+//')
+                optstr=$(_optstr_add "$optstr" "compression_type=zlib")
+            fi
             optstr=$(_optstr_add "$optstr" "$param")
             opts_param=false
             continue
-- 
2.29.2

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

[Max Reitz]

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

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

[Vladimir Sementsov-Ogievskiy]

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()
>>               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\+,\?//'
> 
> ?

Agree

> 
>> +                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
> 

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,
Vladimir

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

[Max Reitz]

On 16.07.21 16:24, Vladimir Sementsov-Ogievskiy wrote:
> 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()
>>>               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\+,\?//'
>>
>> ?
>
> Agree
>
>>
>>> +                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
>>
>
> Hmm.. I have better idea: deprecate v2 and drop all iotest support for 
> it now :)) What do you think?

I haven’t yet understood the appeal of deprecating v2, because basically 
all code is shared between v2 and v3. So I don’t really see the appeal 
in dropping iotest support for it either.

At least, this doesn’t appear like a better idea than to add 
_unsupported_imgopts where needed (in fact, _unsupported_imgopts should 
already be there for other v3-only options like lazy_refcounts).

> 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.

The standard procedure for this is to have a quick look whether we 
actually lose (relevant) coverage for this imgopt if we skip the test 
(usually not), and if so, split that part out into a new file. But 
again, usually nothing of value is lost, so nothing is split off.

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

So far I don’t think there is a pressing reason why bash tests would be 
harder to support than Python tests, and so the effort to port all bash 
tests to Python seems much more difficult to me than having to duplicate 
meta-work like this.

(And in fact, as an example, I found it much easier to have bash tests 
support -o data_file than the Python tests, not least because the bash 
tests at least kind of all work the same, whereas we have like three 
variants of Python test styles.  Which brings me back to “bash tests 
don’t support separately skippable test cases” – well, same for most of 
our Python tests, I guess.)

Max

[PATCH 09/14] iotests/common.rc: introduce _qcow2_dump_header helper

[Vladimir Sementsov-Ogievskiy]

We'll use it in tests instead of explicit qcow2.py. Then we are going
to add some filtering in _qcow2_dump_header.

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

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 4cae5b2d70..ee4b9d795e 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -994,5 +994,15 @@ _require_one_device_of()
     _notrun "$* not available"
 }
 
+_qcow2_dump_header()
+{
+    img="$1"
+    if [ -z "$img" ]; then
+        img="$TEST_IMG"
+    fi
+
+    $PYTHON qcow2.py "$img" dump-header
+}
+
 # make sure this script returns success
 true
-- 
2.29.2

Re: [PATCH 09/14] iotests/common.rc: introduce _qcow2_dump_header helper

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> We'll use it in tests instead of explicit qcow2.py. Then we are going
> to add some filtering in _qcow2_dump_header.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/common.rc | 10 ++++++++++
>   1 file changed, 10 insertions(+)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[PATCH 10/14] iotests: massive use _qcow2_dump_header

[Vladimir Sementsov-Ogievskiy]

We are going to add filtering in _qcow2_dump_header and want all tests
use it.

The patch is generated by commands:
  cd tests/qemu-iotests
  sed -ie 's/$PYTHON qcow2.py "$TEST_IMG" dump-header\($\| \)/_qcow2_dump_header\1/' ??? tests/*

(the difficulty is to avoid converting dump-header-exts)

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/031 |  6 +++---
 tests/qemu-iotests/036 |  6 +++---
 tests/qemu-iotests/039 | 20 ++++++++++----------
 tests/qemu-iotests/060 | 20 ++++++++++----------
 tests/qemu-iotests/061 | 36 ++++++++++++++++++------------------
 tests/qemu-iotests/137 |  2 +-
 tests/qemu-iotests/287 |  8 ++++----
 7 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031
index 58b57a0ef2..648112f796 100755
--- a/tests/qemu-iotests/031
+++ b/tests/qemu-iotests/031
@@ -58,21 +58,21 @@ for compat in "compat=0.10" "compat=1.1"; do
     echo
     _make_test_img -o $compat 64M
     $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0x12345678 "This is a test header extension"
-    $PYTHON qcow2.py "$TEST_IMG" dump-header
+    _qcow2_dump_header
     _check_test_img
 
     echo
     echo === Rewrite header with no backing file ===
     echo
     $QEMU_IMG rebase -u -b "" "$TEST_IMG"
-    $PYTHON qcow2.py "$TEST_IMG" dump-header
+    _qcow2_dump_header
     _check_test_img
 
     echo
     echo === Add a backing file and format ===
     echo
     $QEMU_IMG rebase -u -b "/some/backing/file/path" -F host_device "$TEST_IMG"
-    $PYTHON qcow2.py "$TEST_IMG" dump-header
+    _qcow2_dump_header
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index 5e567012a8..f703605e44 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -58,7 +58,7 @@ $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63
 
 # Without feature table
 $PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep features
+_qcow2_dump_header | grep features
 $PYTHON qcow2.py "$TEST_IMG" dump-header-exts
 _img_info
 
@@ -107,7 +107,7 @@ echo === Create image with unknown autoclear feature bit ===
 echo
 _make_test_img 64M
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 63
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep features
+_qcow2_dump_header | grep features
 $PYTHON qcow2.py "$TEST_IMG" dump-header-exts
 
 echo
@@ -115,7 +115,7 @@ echo === Repair image ===
 echo
 _check_test_img -r all
 
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep features
+_qcow2_dump_header | grep features
 $PYTHON qcow2.py "$TEST_IMG" dump-header-exts
 
 # success, all done
diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 12b2c7fa7b..8e783a8380 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -59,7 +59,7 @@ _make_test_img -o "compat=1.1,lazy_refcounts=on" $size
 $QEMU_IO -c "write -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io
 
 # The dirty bit must not be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 _check_test_img
 
 echo
@@ -73,7 +73,7 @@ $QEMU_IO -c "write -P 0x5a 0 512" \
     | _filter_qemu_io
 
 # The dirty bit must be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 _check_test_img
 
 echo
@@ -82,7 +82,7 @@ echo "== Read-only access must still work =="
 $QEMU_IO -r -c "read -P 0x5a 0 512" "$TEST_IMG" | _filter_qemu_io
 
 # The dirty bit must be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 echo
 echo "== Repairing the image file must succeed =="
@@ -90,7 +90,7 @@ echo "== Repairing the image file must succeed =="
 _check_test_img -r all
 
 # The dirty bit must not be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 echo
 echo "== Data should still be accessible after repair =="
@@ -108,12 +108,12 @@ $QEMU_IO -c "write -P 0x5a 0 512" \
     | _filter_qemu_io
 
 # The dirty bit must be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io
 
 # The dirty bit must not be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 echo
 echo "== Creating an image file with lazy_refcounts=off =="
@@ -126,7 +126,7 @@ $QEMU_IO -c "write -P 0x5a 0 512" \
     | _filter_qemu_io
 
 # The dirty bit must not be set since lazy_refcounts=off
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 _check_test_img
 
 echo
@@ -141,7 +141,7 @@ $QEMU_IO -c "write 0 512" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG commit "$TEST_IMG"
 
 # The dirty bit must not be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 $PYTHON qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features
 
 _check_test_img
@@ -159,7 +159,7 @@ $QEMU_IO -c "reopen -o lazy-refcounts=on" \
     | _filter_qemu_io
 
 # The dirty bit must be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 _check_test_img
 
 _make_test_img -o "compat=1.1,lazy_refcounts=on" $size
@@ -171,7 +171,7 @@ $QEMU_IO -c "reopen -o lazy-refcounts=off" \
     | _filter_qemu_io
 
 # The dirty bit must not be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 _check_test_img
 
 
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index db26c6b246..d1e3204d4e 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -80,13 +80,13 @@ poke_file "$TEST_IMG" "$l1_offset" "\x80\x00\x00\x00\x00\x03\x00\x00"
 _check_test_img
 
 # The corrupt bit should not be set anyway
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 # Try to write something, thereby forcing the corrupt bit to be set
 $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io
 
 # The corrupt bit must now be set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 # This information should be available through qemu-img info
 _img_info --format-specific
@@ -114,19 +114,19 @@ poke_file "$TEST_IMG" "$(($rb_offset+8))" "\x00\x01"
 # Redirect new data cluster onto refcount block
 poke_file "$TEST_IMG" "$l2_offset" "\x80\x00\x00\x00\x00\x02\x00\x00"
 _check_test_img
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 # Try to fix it
 _check_test_img -r all
 
 # The corrupt bit should be cleared
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 # Look if it's really really fixed
 $QEMU_IO -c "$OPEN_RW" -c "write -P 0x2a 0 512" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 echo
 echo "=== Testing cluster data reference into inactive L2 table ==="
@@ -139,13 +139,13 @@ $QEMU_IO -c "$OPEN_RW" -c "write -P 2 0 512" | _filter_qemu_io
 poke_file "$TEST_IMG" "$l2_offset_after_snapshot" \
                       "\x80\x00\x00\x00\x00\x04\x00\x00"
 _check_test_img
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 $QEMU_IO -c "$OPEN_RW" -c "write -P 3 0 512" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 _check_test_img -r all
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 $QEMU_IO -c "$OPEN_RW" -c "write -P 4 0 512" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 # Check data
 $QEMU_IO -c "$OPEN_RO" -c "read -P 4 0 512" | _filter_qemu_io
diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
index e26d94a0df..a9bfd8dc0b 100755
--- a/tests/qemu-iotests/061
+++ b/tests/qemu-iotests/061
@@ -55,9 +55,9 @@ echo "=== Testing version downgrade with zero expansion ==="
 echo
 _make_test_img -o "compat=1.1,lazy_refcounts=on" 64M
 $QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
@@ -68,10 +68,10 @@ _make_test_img -o "compat=1.1,lazy_refcounts=on" 64M
 $QEMU_IO -c "write -z 0 128k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "write -z 32M 128k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c map "$TEST_IMG" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IMG amend -o "compat=0.10" --image-opts \
           driver=qcow2,file.filename=$TEST_IMG,l2-cache-entry-size=4096
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c "read -P 0 32M 128k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IO -c map "$TEST_IMG" | _filter_qemu_io
@@ -84,9 +84,9 @@ _make_test_img -o "compat=1.1,lazy_refcounts=on" 64M
 _NO_VALGRIND \
 $QEMU_IO -c "write -P 0x2a 0 128k" -c flush \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
@@ -96,9 +96,9 @@ echo
 _make_test_img -o "compat=1.1" 64M
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit compatible 42
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit autoclear 42
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IMG amend -o "compat=0.10" "$TEST_IMG"
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 _check_test_img
 
 echo
@@ -106,9 +106,9 @@ echo "=== Testing version upgrade and resize ==="
 echo
 _make_test_img -o "compat=0.10" 64M
 $QEMU_IO -c "write -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IMG amend -o "compat=1.1,lazy_refcounts=on,size=128M" "$TEST_IMG"
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IO -c "read -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
@@ -120,29 +120,29 @@ $QEMU_IO -c "write -P 0x2a 24M 64k" "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG snapshot -c foo "$TEST_IMG"
 $QEMU_IMG resize "$TEST_IMG" 64M &&
     echo "unexpected pass"
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep '^\(version\|size\|nb_snap\)'
+_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)'
 
 $QEMU_IMG amend -o "compat=1.1,size=128M" "$TEST_IMG" ||
     echo "unexpected fail"
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep '^\(version\|size\|nb_snap\)'
+_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)'
 
 $QEMU_IMG snapshot -c bar "$TEST_IMG"
 $QEMU_IMG resize --shrink "$TEST_IMG" 64M ||
     echo "unexpected fail"
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep '^\(version\|size\|nb_snap\)'
+_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)'
 
 $QEMU_IMG amend -o "compat=0.10,size=32M" "$TEST_IMG" &&
     echo "unexpected pass"
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep '^\(version\|size\|nb_snap\)'
+_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)'
 
 $QEMU_IMG snapshot -a bar "$TEST_IMG" ||
     echo "unexpected fail"
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep '^\(version\|size\|nb_snap\)'
+_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)'
 
 $QEMU_IMG snapshot -d bar "$TEST_IMG"
 $QEMU_IMG amend -o "compat=0.10,size=32M" "$TEST_IMG" ||
     echo "unexpected fail"
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep '^\(version\|size\|nb_snap\)'
+_qcow2_dump_header | grep '^\(version\|size\|nb_snap\)'
 
 _check_test_img
 
@@ -154,9 +154,9 @@ _make_test_img -o "compat=1.1,lazy_refcounts=on" 64M
 _NO_VALGRIND \
 $QEMU_IO -c "write -P 0x2a 0 128k" -c flush \
          -c "sigraise $(kill -l KILL)" "$TEST_IMG" 2>&1 | _filter_qemu_io
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IMG amend -o "lazy_refcounts=off" "$TEST_IMG"
-$PYTHON qcow2.py "$TEST_IMG" dump-header
+_qcow2_dump_header
 $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io
 _check_test_img
 
diff --git a/tests/qemu-iotests/137 b/tests/qemu-iotests/137
index 4680d5df3d..52ee135184 100755
--- a/tests/qemu-iotests/137
+++ b/tests/qemu-iotests/137
@@ -140,7 +140,7 @@ $QEMU_IO \
 
 # The dirty bit must not be set
 # (Filter the external data file bit)
-if $PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features \
+if _qcow2_dump_header | grep incompatible_features \
     | grep -q '\<0\>'
 then
     echo 'ERROR: Dirty bit set'
diff --git a/tests/qemu-iotests/287 b/tests/qemu-iotests/287
index 22ce9ff0e4..6716419da4 100755
--- a/tests/qemu-iotests/287
+++ b/tests/qemu-iotests/287
@@ -61,13 +61,13 @@ echo
 echo "=== Testing compression type incompatible bit setting for zlib ==="
 echo
 _make_test_img -o compression_type=zlib 64M
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 echo
 echo "=== Testing compression type incompatible bit setting for zstd ==="
 echo
 _make_test_img -o compression_type=zstd 64M
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 echo
 echo "=== Testing zlib with incompatible bit set ==="
@@ -75,7 +75,7 @@ echo
 _make_test_img -o compression_type=zlib 64M
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 3
 # to make sure the bit was actually set
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 if $QEMU_IMG info "$TEST_IMG" >/dev/null 2>&1 ; then
     echo "Error: The image opened successfully. The image must not be opened."
@@ -87,7 +87,7 @@ echo
 _make_test_img -o compression_type=zstd 64M
 $PYTHON qcow2.py "$TEST_IMG" set-header incompatible_features 0
 # to make sure the bit was actually unset
-$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features
+_qcow2_dump_header | grep incompatible_features
 
 if $QEMU_IMG info "$TEST_IMG" >/dev/null 2>&1 ; then
     echo "Error: The image opened successfully. The image must not be opened."
-- 
2.29.2

Re: [PATCH 10/14] iotests: massive use _qcow2_dump_header

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> We are going to add filtering in _qcow2_dump_header and want all tests
> use it.
>
> The patch is generated by commands:
>    cd tests/qemu-iotests
>    sed -ie 's/$PYTHON qcow2.py "$TEST_IMG" dump-header\($\| \)/_qcow2_dump_header\1/' ??? tests/*
>
> (the difficulty is to avoid converting dump-header-exts)
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/031 |  6 +++---
>   tests/qemu-iotests/036 |  6 +++---
>   tests/qemu-iotests/039 | 20 ++++++++++----------
>   tests/qemu-iotests/060 | 20 ++++++++++----------
>   tests/qemu-iotests/061 | 36 ++++++++++++++++++------------------
>   tests/qemu-iotests/137 |  2 +-
>   tests/qemu-iotests/287 |  8 ++++----
>   7 files changed, 49 insertions(+), 49 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

I think I’d have merged patch 13 into this one, but if you want to keep 
it separate (so that this remains a purely auto-generated patch), then I 
think it should at least come right after this one.

Max

[PATCH 11/14] iotests: bash tests: filter compression type

[Vladimir Sementsov-Ogievskiy]

We want iotests pass with both the default zlib compression and with
IMGOPTS='compression_type=zstd'.

Actually the only test that is interested in real compression type in
test output is 287 (test for qcow2 compression type), so implement
specific option for it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/060.out       |  2 +-
 tests/qemu-iotests/061.out       | 12 ++++++------
 tests/qemu-iotests/082.out       | 14 +++++++-------
 tests/qemu-iotests/198.out       |  4 ++--
 tests/qemu-iotests/287           |  8 ++++----
 tests/qemu-iotests/common.filter |  7 +++++++
 tests/qemu-iotests/common.rc     | 14 +++++++++++++-
 7 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index b74540bafb..329977d9b9 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -17,7 +17,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: true
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index ee30da2665..11b6404186 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -524,7 +524,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     data file: TEST_DIR/t.IMGFMT.data
@@ -551,7 +551,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     data file: foo
@@ -566,7 +566,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     data file raw: false
@@ -582,7 +582,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     data file: TEST_DIR/t.IMGFMT.data
@@ -596,7 +596,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     data file: TEST_DIR/t.IMGFMT.data
@@ -611,7 +611,7 @@ virtual size: 64 MiB (67108864 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     data file: TEST_DIR/t.IMGFMT.data
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index b70c12c139..f8e2e039fc 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -17,7 +17,7 @@ virtual size: 128 MiB (134217728 bytes)
 cluster_size: 4096
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 16
     corrupt: false
@@ -31,7 +31,7 @@ virtual size: 128 MiB (134217728 bytes)
 cluster_size: 8192
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 16
     corrupt: false
@@ -329,7 +329,7 @@ virtual size: 128 MiB (134217728 bytes)
 cluster_size: 4096
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 16
     corrupt: false
@@ -342,7 +342,7 @@ virtual size: 128 MiB (134217728 bytes)
 cluster_size: 8192
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 16
     corrupt: false
@@ -639,7 +639,7 @@ virtual size: 128 MiB (134217728 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 16
     corrupt: false
@@ -652,7 +652,7 @@ virtual size: 130 MiB (136314880 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: false
     refcount bits: 16
     corrupt: false
@@ -665,7 +665,7 @@ virtual size: 132 MiB (138412032 bytes)
 cluster_size: 65536
 Format specific information:
     compat: 1.1
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     lazy refcounts: true
     refcount bits: 16
     corrupt: false
diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index 3952708444..805494916f 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -36,7 +36,7 @@ image: json:{ /* filtered */ }
 file format: IMGFMT
 virtual size: 16 MiB (16777216 bytes)
 Format specific information:
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     encrypt:
         ivgen alg: plain64
         hash alg: sha256
@@ -81,7 +81,7 @@ virtual size: 16 MiB (16777216 bytes)
 backing file: TEST_DIR/t.IMGFMT.base
 backing file format: IMGFMT
 Format specific information:
-    compression type: zlib
+    compression type: COMPRESSION_TYPE
     encrypt:
         ivgen alg: plain64
         hash alg: sha256
diff --git a/tests/qemu-iotests/287 b/tests/qemu-iotests/287
index 6716419da4..aab03fb973 100755
--- a/tests/qemu-iotests/287
+++ b/tests/qemu-iotests/287
@@ -61,13 +61,13 @@ echo
 echo "=== Testing compression type incompatible bit setting for zlib ==="
 echo
 _make_test_img -o compression_type=zlib 64M
-_qcow2_dump_header | grep incompatible_features
+_qcow2_dump_header --no-filter-compression | grep incompatible_features
 
 echo
 echo "=== Testing compression type incompatible bit setting for zstd ==="
 echo
 _make_test_img -o compression_type=zstd 64M
-_qcow2_dump_header | grep incompatible_features
+_qcow2_dump_header --no-filter-compression | grep incompatible_features
 
 echo
 echo "=== Testing zlib with incompatible bit set ==="
@@ -75,7 +75,7 @@ echo
 _make_test_img -o compression_type=zlib 64M
 $PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 3
 # to make sure the bit was actually set
-_qcow2_dump_header | grep incompatible_features
+_qcow2_dump_header --no-filter-compression | grep incompatible_features
 
 if $QEMU_IMG info "$TEST_IMG" >/dev/null 2>&1 ; then
     echo "Error: The image opened successfully. The image must not be opened."
@@ -87,7 +87,7 @@ echo
 _make_test_img -o compression_type=zstd 64M
 $PYTHON qcow2.py "$TEST_IMG" set-header incompatible_features 0
 # to make sure the bit was actually unset
-_qcow2_dump_header | grep incompatible_features
+_qcow2_dump_header --no-filter-compression | grep incompatible_features
 
 if $QEMU_IMG info "$TEST_IMG" >/dev/null 2>&1 ; then
     echo "Error: The image opened successfully. The image must not be opened."
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 268b749e2f..78efe3e4dd 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -247,6 +247,7 @@ _filter_img_info()
         -e "/block_state_zero: \\(on\\|off\\)/d" \
         -e "/log_size: [0-9]\\+/d" \
         -e "s/iters: [0-9]\\+/iters: 1024/" \
+        -e 's/\(compression type: \)\(zlib\|zstd\)/\1COMPRESSION_TYPE/' \
         -e "s/uuid: [-a-f0-9]\\+/uuid: 00000000-0000-0000-0000-000000000000/" | \
     while IFS='' read -r line; do
         if [[ $format_specific == 1 ]]; then
@@ -332,5 +333,11 @@ for fname in fnames:
 sys.stdout.write(result)'
 }
 
+_filter_qcow2_compression_type_bit()
+{
+    $SED -e 's/\(incompatible_features\s\+\)\[3\(, \)\?/\1[/' \
+         -e 's/\(incompatible_features.*\), 3\]/\1]/'
+}
+
 # make sure this script returns success
 true
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index ee4b9d795e..813b51ee03 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -697,6 +697,7 @@ _img_info()
             -e "s#$TEST_DIR#TEST_DIR#g" \
             -e "s#$SOCK_DIR/fuse-#TEST_DIR/#g" \
             -e "s#$IMGFMT#IMGFMT#g" \
+            -e 's/\(compression type: \)\(zlib\|zstd\)/\1COMPRESSION_TYPE/' \
             -e "/^disk size:/ D" \
             -e "/actual-size/ D" | \
         while IFS='' read -r line; do
@@ -996,12 +997,23 @@ _require_one_device_of()
 
 _qcow2_dump_header()
 {
+    if [[ "$1" == "--no-filter-compression" ]]; then
+        local filter_compression=0
+        shift
+    else
+        local filter_compression=1
+    fi
+
     img="$1"
     if [ -z "$img" ]; then
         img="$TEST_IMG"
     fi
 
-    $PYTHON qcow2.py "$img" dump-header
+    if [[ $filter_compression == 0 ]]; then
+        $PYTHON qcow2.py "$img" dump-header
+    else
+        $PYTHON qcow2.py "$img" dump-header | _filter_qcow2_compression_type_bit
+    fi
 }
 
 # make sure this script returns success
-- 
2.29.2

Re: [PATCH 11/14] iotests: bash tests: filter compression type

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> We want iotests pass with both the default zlib compression and with
> IMGOPTS='compression_type=zstd'.
>
> Actually the only test that is interested in real compression type in
> test output is 287 (test for qcow2 compression type), so implement
> specific option for it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/060.out       |  2 +-
>   tests/qemu-iotests/061.out       | 12 ++++++------
>   tests/qemu-iotests/082.out       | 14 +++++++-------
>   tests/qemu-iotests/198.out       |  4 ++--
>   tests/qemu-iotests/287           |  8 ++++----
>   tests/qemu-iotests/common.filter |  7 +++++++
>   tests/qemu-iotests/common.rc     | 14 +++++++++++++-
>   7 files changed, 40 insertions(+), 21 deletions(-)

[...]

> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> index 268b749e2f..78efe3e4dd 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -247,6 +247,7 @@ _filter_img_info()
>           -e "/block_state_zero: \\(on\\|off\\)/d" \
>           -e "/log_size: [0-9]\\+/d" \
>           -e "s/iters: [0-9]\\+/iters: 1024/" \
> +        -e 's/\(compression type: \)\(zlib\|zstd\)/\1COMPRESSION_TYPE/' \
>           -e "s/uuid: [-a-f0-9]\\+/uuid: 00000000-0000-0000-0000-000000000000/" | \
>       while IFS='' read -r line; do
>           if [[ $format_specific == 1 ]]; then
> @@ -332,5 +333,11 @@ for fname in fnames:
>   sys.stdout.write(result)'
>   }
>   
> +_filter_qcow2_compression_type_bit()
> +{
> +    $SED -e 's/\(incompatible_features\s\+\)\[3\(, \)\?/\1[/' \
> +         -e 's/\(incompatible_features.*\), 3\]/\1]/'

What about “incompatble_features   [2, 3, 4]”?

I’d like to propose adding some form of filtering parameter to qcow2.py 
which allows filtering a specific bit from the qcow2_format.Flags64 
representation, but that seems rather difficult, actually...

> +}
> +
>   # make sure this script returns success
>   true
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index ee4b9d795e..813b51ee03 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -697,6 +697,7 @@ _img_info()
>               -e "s#$TEST_DIR#TEST_DIR#g" \
>               -e "s#$SOCK_DIR/fuse-#TEST_DIR/#g" \
>               -e "s#$IMGFMT#IMGFMT#g" \
> +            -e 's/\(compression type: \)\(zlib\|zstd\)/\1COMPRESSION_TYPE/' \
>               -e "/^disk size:/ D" \
>               -e "/actual-size/ D" | \
>           while IFS='' read -r line; do
> @@ -996,12 +997,23 @@ _require_one_device_of()
>   
>   _qcow2_dump_header()
>   {
> +    if [[ "$1" == "--no-filter-compression" ]]; then
> +        local filter_compression=0
> +        shift
> +    else
> +        local filter_compression=1
> +    fi
> +
>       img="$1"
>       if [ -z "$img" ]; then
>           img="$TEST_IMG"
>       fi
>   
> -    $PYTHON qcow2.py "$img" dump-header
> +    if [[ $filter_compression == 0 ]]; then
> +        $PYTHON qcow2.py "$img" dump-header
> +    else
> +        $PYTHON qcow2.py "$img" dump-header | _filter_qcow2_compression_type_bit
> +    fi
>   }
>   
>   # make sure this script returns success

Could have been done more extensibly for the future (i.e. a loop over 
the parameters, and a variable to invoke all applicable filters), but, 
well.  Not much reason to think about a future that we’re not sure will 
ever happen.

Max

Re: [PATCH 11/14] iotests: bash tests: filter compression type

[Vladimir Sementsov-Ogievskiy]

16.07.2021 16:17, Max Reitz wrote:
> On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
>> We want iotests pass with both the default zlib compression and with
>> IMGOPTS='compression_type=zstd'.
>>
>> Actually the only test that is interested in real compression type in
>> test output is 287 (test for qcow2 compression type), so implement
>> specific option for it.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/060.out       |  2 +-
>>   tests/qemu-iotests/061.out       | 12 ++++++------
>>   tests/qemu-iotests/082.out       | 14 +++++++-------
>>   tests/qemu-iotests/198.out       |  4 ++--
>>   tests/qemu-iotests/287           |  8 ++++----
>>   tests/qemu-iotests/common.filter |  7 +++++++
>>   tests/qemu-iotests/common.rc     | 14 +++++++++++++-
>>   7 files changed, 40 insertions(+), 21 deletions(-)
> 
> [...]
> 
>> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
>> index 268b749e2f..78efe3e4dd 100644
>> --- a/tests/qemu-iotests/common.filter
>> +++ b/tests/qemu-iotests/common.filter
>> @@ -247,6 +247,7 @@ _filter_img_info()
>>           -e "/block_state_zero: \\(on\\|off\\)/d" \
>>           -e "/log_size: [0-9]\\+/d" \
>>           -e "s/iters: [0-9]\\+/iters: 1024/" \
>> +        -e 's/\(compression type: \)\(zlib\|zstd\)/\1COMPRESSION_TYPE/' \
>>           -e "s/uuid: [-a-f0-9]\\+/uuid: 00000000-0000-0000-0000-000000000000/" | \
>>       while IFS='' read -r line; do
>>           if [[ $format_specific == 1 ]]; then
>> @@ -332,5 +333,11 @@ for fname in fnames:
>>   sys.stdout.write(result)'
>>   }
>> +_filter_qcow2_compression_type_bit()
>> +{
>> +    $SED -e 's/\(incompatible_features\s\+\)\[3\(, \)\?/\1[/' \
>> +         -e 's/\(incompatible_features.*\), 3\]/\1]/'
> 
> What about “incompatble_features   [2, 3, 4]”?

Will add.

> 
> I’d like to propose adding some form of filtering parameter to qcow2.py which allows filtering a specific bit from the qcow2_format.Flags64 representation, but that seems rather difficult, actually...
> 
>> +}
>> +
>>   # make sure this script returns success
>>   true
>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>> index ee4b9d795e..813b51ee03 100644
>> --- a/tests/qemu-iotests/common.rc
>> +++ b/tests/qemu-iotests/common.rc
>> @@ -697,6 +697,7 @@ _img_info()
>>               -e "s#$TEST_DIR#TEST_DIR#g" \
>>               -e "s#$SOCK_DIR/fuse-#TEST_DIR/#g" \
>>               -e "s#$IMGFMT#IMGFMT#g" \
>> +            -e 's/\(compression type: \)\(zlib\|zstd\)/\1COMPRESSION_TYPE/' \
>>               -e "/^disk size:/ D" \
>>               -e "/actual-size/ D" | \
>>           while IFS='' read -r line; do
>> @@ -996,12 +997,23 @@ _require_one_device_of()
>>   _qcow2_dump_header()
>>   {
>> +    if [[ "$1" == "--no-filter-compression" ]]; then
>> +        local filter_compression=0
>> +        shift
>> +    else
>> +        local filter_compression=1
>> +    fi
>> +
>>       img="$1"
>>       if [ -z "$img" ]; then
>>           img="$TEST_IMG"
>>       fi
>> -    $PYTHON qcow2.py "$img" dump-header
>> +    if [[ $filter_compression == 0 ]]; then
>> +        $PYTHON qcow2.py "$img" dump-header
>> +    else
>> +        $PYTHON qcow2.py "$img" dump-header | _filter_qcow2_compression_type_bit
>> +    fi
>>   }
>>   # make sure this script returns success
> 
> Could have been done more extensibly for the future (i.e. a loop over the parameters, and a variable to invoke all applicable filters), but, well.  Not much reason to think about a future that we’re not sure will ever happen.
> 
> Max
> 


-- 
Best regards,
Vladimir

[PATCH 12/14] iotests 60: more accurate set dirty bit in qcow2 header

[Vladimir Sementsov-Ogievskiy]

Don't touch other incompatible bits, like compression-type. This makes
the test pass with IMGOPTS='compression_type=zstd'.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/060 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index d1e3204d4e..df87d600f7 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -326,7 +326,7 @@ _make_test_img 64M
 # Let the refblock appear unaligned
 poke_file "$TEST_IMG" "$rt_offset"        "\x00\x00\x00\x00\xff\xff\x2a\x00"
 # Mark the image dirty, thus forcing an automatic check when opening it
-poke_file "$TEST_IMG" 72 "\x00\x00\x00\x00\x00\x00\x00\x01"
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 0
 # Open the image (qemu should refuse to do so)
 $QEMU_IO -c close "$TEST_IMG" 2>&1 | _filter_testdir | _filter_imgfmt
 
-- 
2.29.2

Re: [PATCH 12/14] iotests 60: more accurate set dirty bit in qcow2 header

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> Don't touch other incompatible bits, like compression-type. This makes
> the test pass with IMGOPTS='compression_type=zstd'.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/060 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[PATCH 13/14] iotest 39: use _qcow2_dump_header

[Vladimir Sementsov-Ogievskiy]

_qcow2_dump_header has filter for compression type, so this change
makes test pass with IMGOPTS='compression_type=zstd'.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/039 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
index 8e783a8380..00d379cde2 100755
--- a/tests/qemu-iotests/039
+++ b/tests/qemu-iotests/039
@@ -142,7 +142,7 @@ $QEMU_IMG commit "$TEST_IMG"
 
 # The dirty bit must not be set
 _qcow2_dump_header | grep incompatible_features
-$PYTHON qcow2.py "$TEST_IMG".base dump-header | grep incompatible_features
+_qcow2_dump_header "$TEST_IMG".base | grep incompatible_features
 
 _check_test_img
 TEST_IMG="$TEST_IMG".base _check_test_img
-- 
2.29.2

Re: [PATCH 13/14] iotest 39: use _qcow2_dump_header

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> _qcow2_dump_header has filter for compression type, so this change
> makes test pass with IMGOPTS='compression_type=zstd'.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/039 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

But as I said, I’d prefer this to come right after patch 10.

Max

[PATCH 14/14] iotest 214: explicit compression type

[Vladimir Sementsov-Ogievskiy]

The test-case "Corrupted size field in compressed cluster descriptor"
heavily depends on zlib compression type. So, make it explicit. This
way test passes with IMGOPTS='compression_type=zstd'.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/214 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 0889089d81..c66e246ba2 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -51,7 +51,7 @@ echo
 # The L2 entries of the two compressed clusters are located at
 # 0x800000 and 0x800008, their original values are 0x4008000000a00000
 # and 0x4008000000a00802 (5 sectors for compressed data each).
-_make_test_img 8M -o cluster_size=2M
+_make_test_img 8M -o cluster_size=2M,compression_type=zlib
 $QEMU_IO -c "write -c -P 0x11 0 2M" -c "write -c -P 0x11 2M 2M" "$TEST_IMG" \
          2>&1 | _filter_qemu_io | _filter_testdir
 
-- 
2.29.2

Re: [PATCH 14/14] iotest 214: explicit compression type

[Max Reitz]

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:
> The test-case "Corrupted size field in compressed cluster descriptor"
> heavily depends on zlib compression type. So, make it explicit. This
> way test passes with IMGOPTS='compression_type=zstd'.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/214 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>