[PATCH v5 0/5] qcow2: Implement zstd cluster compression method

Denis Plotnikov posted 5 patches 5 years, 8 months ago
Test docker-quick@centos7 failed
Test FreeBSD failed
Test docker-mingw@fedora failed
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200304133538.9159-1-dplotnikov@virtuozzo.com
Maintainers: Markus Armbruster <armbru@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>, Eric Blake <eblake@redhat.com>
There is a newer version of this series
docs/interop/qcow2.txt           |  20 +++
configure                        |   2 +-
qapi/block-core.json             |  23 +++-
block/qcow2.h                    |  18 ++-
include/block/block_int.h        |   1 +
block/qcow2-threads.c            | 206 ++++++++++++++++++++++++++++---
block/qcow2.c                    | 108 ++++++++++++++++
tests/qemu-iotests/031.out       |  14 +--
tests/qemu-iotests/036.out       |   4 +-
tests/qemu-iotests/049.out       | 102 +++++++--------
tests/qemu-iotests/060.out       |   1 +
tests/qemu-iotests/061.out       |  34 ++---
tests/qemu-iotests/065           |  28 +++--
tests/qemu-iotests/080           |   2 +-
tests/qemu-iotests/144.out       |   4 +-
tests/qemu-iotests/182.out       |   2 +-
tests/qemu-iotests/242.out       |   5 +
tests/qemu-iotests/255.out       |   8 +-
tests/qemu-iotests/287           | 128 +++++++++++++++++++
tests/qemu-iotests/287.out       |  43 +++++++
tests/qemu-iotests/common.filter |   3 +-
tests/qemu-iotests/group         |   1 +
22 files changed, 644 insertions(+), 113 deletions(-)
create mode 100755 tests/qemu-iotests/287
create mode 100644 tests/qemu-iotests/287.out
[PATCH v5 0/5] qcow2: Implement zstd cluster compression method
Posted by Denis Plotnikov 5 years, 8 months ago
v5:
   * replace -ENOTSUP with abort in qcow2_co_decompress [Vladimir]
   * set cluster size for all test cases in the beginning of the 287 test

v4:
   * the series is rebased on top of 01 "block/qcow2-threads: fix qcow2_decompress"
   * 01 is just a no-change resend to avoid extra dependencies. Still, it may be merged in separate

v3:
   * remove redundant max compression type value check [Vladimir, Eric]
     (the switch below checks everything)
   * prevent compression type changing on "qemu-img amend" [Vladimir]
   * remove zstd config setting, since it has been added already by
     "migration" patches [Vladimir]
   * change the compression type error message [Vladimir] 
   * fix alignment and 80-chars exceeding [Vladimir]

v2:
   * rework compression type setting [Vladimir]
   * squash iotest changes to the compression type introduction patch [Vladimir, Eric]
   * fix zstd availability checking in zstd iotest [Vladimir]
   * remove unnecessry casting [Eric]
   * remove rudundant checks [Eric]
   * fix compressed cluster layout in qcow2 spec [Vladimir]
   * fix wording [Eric, Vladimir]
   * fix compression type filtering in iotests [Eric]

v1:
   the initial series

Denis Plotnikov (4):
  qcow2: introduce compression type feature
  qcow2: rework the cluster compression routine
  qcow2: add zstd cluster compression
  iotests: 287: add qcow2 compression type test

Vladimir Sementsov-Ogievskiy (1):
  block/qcow2-threads: fix qcow2_decompress

 docs/interop/qcow2.txt           |  20 +++
 configure                        |   2 +-
 qapi/block-core.json             |  23 +++-
 block/qcow2.h                    |  18 ++-
 include/block/block_int.h        |   1 +
 block/qcow2-threads.c            | 206 ++++++++++++++++++++++++++++---
 block/qcow2.c                    | 108 ++++++++++++++++
 tests/qemu-iotests/031.out       |  14 +--
 tests/qemu-iotests/036.out       |   4 +-
 tests/qemu-iotests/049.out       | 102 +++++++--------
 tests/qemu-iotests/060.out       |   1 +
 tests/qemu-iotests/061.out       |  34 ++---
 tests/qemu-iotests/065           |  28 +++--
 tests/qemu-iotests/080           |   2 +-
 tests/qemu-iotests/144.out       |   4 +-
 tests/qemu-iotests/182.out       |   2 +-
 tests/qemu-iotests/242.out       |   5 +
 tests/qemu-iotests/255.out       |   8 +-
 tests/qemu-iotests/287           | 128 +++++++++++++++++++
 tests/qemu-iotests/287.out       |  43 +++++++
 tests/qemu-iotests/common.filter |   3 +-
 tests/qemu-iotests/group         |   1 +
 22 files changed, 644 insertions(+), 113 deletions(-)
 create mode 100755 tests/qemu-iotests/287
 create mode 100644 tests/qemu-iotests/287.out

-- 
2.17.0


Re: [PATCH v5 0/5] qcow2: Implement zstd cluster compression method
Posted by Denis Plotnikov 5 years, 8 months ago
ping!

Is there any other comments/concerns/objections/suggestions according to 
the series except the minor ones from Alberto and Vladimir?
If not, please, let me know, so I can resend the series with the minor 
changes for applying to the corresponding branch.

Thanks!

Denis

On 04.03.2020 16:35, Denis Plotnikov wrote:
> v5:
>     * replace -ENOTSUP with abort in qcow2_co_decompress [Vladimir]
>     * set cluster size for all test cases in the beginning of the 287 test
>
> v4:
>     * the series is rebased on top of 01 "block/qcow2-threads: fix qcow2_decompress"
>     * 01 is just a no-change resend to avoid extra dependencies. Still, it may be merged in separate
>
> v3:
>     * remove redundant max compression type value check [Vladimir, Eric]
>       (the switch below checks everything)
>     * prevent compression type changing on "qemu-img amend" [Vladimir]
>     * remove zstd config setting, since it has been added already by
>       "migration" patches [Vladimir]
>     * change the compression type error message [Vladimir]
>     * fix alignment and 80-chars exceeding [Vladimir]
>
> v2:
>     * rework compression type setting [Vladimir]
>     * squash iotest changes to the compression type introduction patch [Vladimir, Eric]
>     * fix zstd availability checking in zstd iotest [Vladimir]
>     * remove unnecessry casting [Eric]
>     * remove rudundant checks [Eric]
>     * fix compressed cluster layout in qcow2 spec [Vladimir]
>     * fix wording [Eric, Vladimir]
>     * fix compression type filtering in iotests [Eric]
>
> v1:
>     the initial series
>
> Denis Plotnikov (4):
>    qcow2: introduce compression type feature
>    qcow2: rework the cluster compression routine
>    qcow2: add zstd cluster compression
>    iotests: 287: add qcow2 compression type test
>
> Vladimir Sementsov-Ogievskiy (1):
>    block/qcow2-threads: fix qcow2_decompress
>
>   docs/interop/qcow2.txt           |  20 +++
>   configure                        |   2 +-
>   qapi/block-core.json             |  23 +++-
>   block/qcow2.h                    |  18 ++-
>   include/block/block_int.h        |   1 +
>   block/qcow2-threads.c            | 206 ++++++++++++++++++++++++++++---
>   block/qcow2.c                    | 108 ++++++++++++++++
>   tests/qemu-iotests/031.out       |  14 +--
>   tests/qemu-iotests/036.out       |   4 +-
>   tests/qemu-iotests/049.out       | 102 +++++++--------
>   tests/qemu-iotests/060.out       |   1 +
>   tests/qemu-iotests/061.out       |  34 ++---
>   tests/qemu-iotests/065           |  28 +++--
>   tests/qemu-iotests/080           |   2 +-
>   tests/qemu-iotests/144.out       |   4 +-
>   tests/qemu-iotests/182.out       |   2 +-
>   tests/qemu-iotests/242.out       |   5 +
>   tests/qemu-iotests/255.out       |   8 +-
>   tests/qemu-iotests/287           | 128 +++++++++++++++++++
>   tests/qemu-iotests/287.out       |  43 +++++++
>   tests/qemu-iotests/common.filter |   3 +-
>   tests/qemu-iotests/group         |   1 +
>   22 files changed, 644 insertions(+), 113 deletions(-)
>   create mode 100755 tests/qemu-iotests/287
>   create mode 100644 tests/qemu-iotests/287.out
>


Re: [PATCH v5 0/5] qcow2: Implement zstd cluster compression method
Posted by Max Reitz 5 years, 8 months ago
On 11.03.20 08:31, Denis Plotnikov wrote:
> ping!
> 
> Is there any other comments/concerns/objections/suggestions according to
> the series except the minor ones from Alberto and Vladimir?
> If not, please, let me know, so I can resend the series with the minor
> changes for applying to the corresponding branch.

Sounds good to me.

I’d like to note that most iotests that seem to do something with
compression (i.e., where grep finds a 'compress' somewhere; 013, 014,
023, 042, 046, 053, 055, ...) pass with -o compression_type=zstd, too.
060 hangs somewhere.  112 complains about v2 incompatibility; and 214
relies on intricacies of zlib, I think.  So that looks good, too.

Well, one thing I did have to fix for this to work is to quote
everything in common.pattern that tries to echo something with brackets
(e.g. “Clusters to be compressed [1]”).  I don’t quite know why the
brackets suddenly disappear when I run the tests with -o
compression_type, who knows.  But putting quotes around the echo
arguments fixes it, so...

Max

Re: [PATCH v5 0/5] qcow2: Implement zstd cluster compression method
Posted by Eric Blake 5 years, 8 months ago
On 3/11/20 11:28 AM, Max Reitz wrote:
> On 11.03.20 08:31, Denis Plotnikov wrote:
>> ping!
>>
>> Is there any other comments/concerns/objections/suggestions according to
>> the series except the minor ones from Alberto and Vladimir?
>> If not, please, let me know, so I can resend the series with the minor
>> changes for applying to the corresponding branch.
> 
> Sounds good to me.
> 
> I’d like to note that most iotests that seem to do something with
> compression (i.e., where grep finds a 'compress' somewhere; 013, 014,
> 023, 042, 046, 053, 055, ...) pass with -o compression_type=zstd, too.
> 060 hangs somewhere.  112 complains about v2 incompatibility; and 214
> relies on intricacies of zlib, I think.  So that looks good, too.
> 
> Well, one thing I did have to fix for this to work is to quote
> everything in common.pattern that tries to echo something with brackets
> (e.g. “Clusters to be compressed [1]”).  I don’t quite know why the
> brackets suddenly disappear when I run the tests with -o
> compression_type, who knows.

That sounds like you have a file named '1' in the directory where the 
echo was run.  Unquoted [1] is a shell glob that expands to '1' if that 
file exists, otherwise remains unexpanded as '[1]'.

>  But putting quotes around the echo
> arguments fixes it, so...

Yes.  Figuring out why a file named '1' is being created is also a 
useful exercise, but quoting any time you have [ that you want output is 
already a good idea.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org