[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v1 3/3] qcow2: add zstd cluster compression
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v1 3/3] qcow2: add zstd cluster compression |
Date: |
Wed, 3 Jul 2019 09:36:58 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 |
On 7/3/19 6:00 AM, Denis Plotnikov wrote:
> zstd significantly reduces cluster compression time.
> It provides better compression performance maintaining
> the same level of compression ratio in comparison with
> zlib, which, by the moment, has been the only compression
> method available.
>
> ---
> block/qcow2.c | 96 ++++++++++++++++++++++++++++++++++++++++++++
> configure | 32 +++++++++++++++
> qapi/block-core.json | 3 +-
> 3 files changed, 130 insertions(+), 1 deletion(-)
Where is the change to docs/interop/qcow2.txt to describe this new
compression format?
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 37a563a671..caa04b0beb 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -27,6 +27,11 @@
> #define ZLIB_CONST
> #include <zlib.h>
>
> +static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
> + const void *src, size_t src_size)
> +{
> + ssize_t ret;
> + uint32_t *c_size = dest;
> + /* steal some bytes to store compressed chunk size */
> + char *d_buf = ((char *) dest) + sizeof(*c_size);
> +
Do you always want exactly 4 bytes for the compressed size? Or is it
worth some sort of variable-length encoding, since we're already dealing
with non-cacheline-aligned data? You can represent all sizes up to 4M
using a maximum of 3 bytes (set the high bit if the integer continues,
then sizes 0-127 take 1 byte [7 bits], 128-32767 take 2 bytes [15 bits],
and 32768-4194303 take 3 bytes [22 bits]).
> + if (dest_size < sizeof(*c_size)) {
> + return -ENOMEM;
> + }
> +
> + dest_size -= sizeof(*c_size);
> +
> + ret = ZSTD_compress(d_buf, dest_size, src, src_size, 5);
The fact that you are storing the size separate from the data passed to
zstd MUST be documented in the qcow2 spec, for the next person to
produce/consume the same data.
> +++ b/qapi/block-core.json
> @@ -4215,11 +4215,12 @@
> # Compression type used in qcow2 image file
> #
> # @zlib: zlib compression, see <http://zlib.net/>
> +# @zstd: zstd compression, see <http://github.com/facebook/zstd>
> #
> # Since: 4.1
> ##
> { 'enum': 'Qcow2CompressionType',
> - 'data': [ 'zlib' ] }
> + 'data': [ 'zlib', 'zstd' ] }
Since you patched configure so that linking against zstd is optional,
this should use { 'name':'zstd', 'if':'CONDITIONAL' } so that during
introspection, the enum only advertises zstd on a build that linked
against the library.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v1 1/3] qcow2: introduce compression type feature, (continued)
Re: [Qemu-block] [Qemu-devel] [PATCH v1 1/3] qcow2: introduce compression type feature, Markus Armbruster, 2019/07/09
[Qemu-block] [PATCH v1 2/3] qcow2: rework the cluster compression routine, Denis Plotnikov, 2019/07/03
[Qemu-block] [PATCH v1 3/3] qcow2: add zstd cluster compression, Denis Plotnikov, 2019/07/03
- Re: [Qemu-block] [PATCH v1 3/3] qcow2: add zstd cluster compression,
Eric Blake <=