[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH V4 08/10] block/qcow2: start using the compress
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH V4 08/10] block/qcow2: start using the compress format extension |
Date: |
Thu, 20 Jul 2017 11:00:01 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/20/2017 09:20 AM, Peter Lieven wrote:
> we now pass the parameters to the zlib compressor if the
> extension is present and use the old default values if
> the extension is absent.
>
> Signed-off-by: Peter Lieven <address@hidden>
> ---
> block/qcow2-cluster.c | 58
> ++++++++++++++++++++++++++++++---------------------
> block/qcow2.c | 57 +++++++++++++++++++++++++++-----------------------
> 2 files changed, 65 insertions(+), 50 deletions(-)
> static int decompress_buffer(uint8_t *out_buf, int out_buf_size,
> - const uint8_t *buf, int buf_size)
> + const uint8_t *buf, int buf_size,
> + int compress_format)
> {
> -
> - ret = inflateInit2(strm, -12);
The old code initializes with 12,
> + switch (compress_format) {
> + case QCOW2_COMPRESS_FORMAT_ZLIB:
> + case -1: {
What is case -1 supposed to be? When the compression format was not
specified? Again, I'd really like there a way to encode the default
(when no format is encoded in the qcow2 file) to be representable
alongside the new code, and then we just special-case writing the
compression format to the file to omit the extension header if the user
requested parameters that match the old default.
> + z_stream z_strm = {};
> +
> + z_strm.next_in = (uint8_t *)buf;
> + z_strm.avail_in = buf_size;
> + z_strm.next_out = out_buf;
> + z_strm.avail_out = out_buf_size;
> +
> + ret = inflateInit2(&z_strm, -15);
The new code is now unconditionally initializing with -15 instead of
-12. Does that matter, or does decompression work regardless of window
size used at creation, as long as the initialized size at decompression
is at least as large? On the other hand, I guess that means if someone
compresses with a large window, and then I initialize the decompressor
with a small window, my decompression will fail? That's why knowing the
minimum window size should be part of the spec, whether or not we make
it a tunable.
> +++ b/block/qcow2.c
> @@ -3391,9 +3391,10 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs,
> uint64_t offset,
> BDRVQcow2State *s = bs->opaque;
> QEMUIOVector hd_qiov;
> struct iovec iov;
> - z_stream strm;
> - int ret, out_len;
> - uint8_t *buf, *out_buf, *local_buf = NULL;
> + z_stream z_strm = {};
> + int z_windowBits = -15, z_level = Z_DEFAULT_COMPRESSION;
> + int ret, out_len = 0;
> + uint8_t *buf, *out_buf = NULL, *local_buf = NULL;
> uint64_t cluster_offset;
>
> if (bytes == 0) {
> @@ -3418,34 +3419,38 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs,
> uint64_t offset,
> buf = qiov->iov[0].iov_base;
> }
>
> - out_buf = g_malloc(s->cluster_size);
> + switch (s->compress_format) {
> + case -1:
> + z_windowBits = -12;
> + case QCOW2_COMPRESS_FORMAT_ZLIB:
> + out_buf = g_malloc(s->cluster_size);
> + if (s->compress_level > 0) {
> + z_level = s->compress_level;
> + }
And this is where I wonder if z_windowBits should be explicitly encoded
in the qcow2 format, rather than just magic numbers we use under the
hood, so that someone else trying to reimplement things will create
compatible files.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH V4 01/10] specs/qcow2: add compress format extension, (continued)
[Qemu-block] [PATCH V4 10/10] block/qcow2: add compress info to image specific info, Peter Lieven, 2017/07/20
[Qemu-block] [PATCH V4 02/10] qapi/block-core: add Qcow2Compress parameters, Peter Lieven, 2017/07/20
[Qemu-block] [PATCH V4 07/10] block/qcow2: optimize qcow2_co_pwritev_compressed, Peter Lieven, 2017/07/20
[Qemu-block] [PATCH V4 08/10] block/qcow2: start using the compress format extension, Peter Lieven, 2017/07/20
- Re: [Qemu-block] [PATCH V4 08/10] block/qcow2: start using the compress format extension,
Eric Blake <=
[Qemu-block] [PATCH V4 03/10] block/qcow2: parse compress create options, Peter Lieven, 2017/07/20
[Qemu-block] [PATCH V4 09/10] block/qcow2: add lzo compress format, Peter Lieven, 2017/07/20
[Qemu-block] [PATCH V4 06/10] block/qcow2: simplify ret usage in qcow2_create, Peter Lieven, 2017/07/20
[Qemu-block] [PATCH V4 05/10] block/qcow2: read and write the compress format extension, Peter Lieven, 2017/07/20