[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V4 01/10] specs/qcow2: add compress format exten
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH V4 01/10] specs/qcow2: add compress format extension |
Date: |
Thu, 20 Jul 2017 20:59:32 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
Am 20.07.2017 um 20:31 schrieb Eric Blake:
> On 07/20/2017 11:26 AM, Peter Lieven wrote:
>>> I haven't yet checked your code implementation to see where you are
>>> setting window sizes, to know if window size is something that should be
>>> a tunable in the file format.
>>>
>> You don't have to know the window size that was used. The docs of zlib
>>
>> just say, that the windowSize specified at inflate must be greater or equal
>>
>> to the one used at deflate. So if we change the code to use 15 we can safely
>>
>> decompress old clusters that where compressed with 12.
> Then you DO have to know the window size. If I write my own qcow2
> parser, and don't know what size you picked on compression, then how do
> I know that the size I pick is >= your size during my decompression?
>
> Even if you DON'T want size to be a tunable (and I'm on the fence on
> that one), you STILL need the spec to state that for maximum
> compatibility, when the incompatible feature is off, implementations
> MUST use window size 12 for compression; and that when the compression
> extension is used, implementations MUST use window size 15.
>
> Or, by having window size be a tunable in the extension header allows
> implementations to record window size, then you allow implemenations to
> explicitly KNOW what window size they must meet or exceed.
>
But they still have to now what values to use if the header is absent.
Bottom line, you are right. Its cleaner to have that value in the header and
default it to zlib, level 0, windowBits 12 if the header is absent and on the
other
side don't write the header if these values are used.
I checked the spec again expect for the windowBits nothing else is passed as
a parameter to inflateinit2.
Here is what is written about the windowBits provided to inflateInit2:
" The windowBits parameter is the base two logarithm of the maximum window size
(the size of the history buffer). It should be in the range 8..15 for this
version of the library. The default value is 15 if inflateInit is used instead.
windowBits must be greater than or equal to the windowBitsvalue provided to
deflateInit2() while compressing, or it must be equal to 15 if deflateInit2()
was not used. If a compressed stream with a larger window size is given as
input, inflate() will return with the error code Z_DATA_ERROR instead of trying
to allocate a larger window."
I checked through the parameters of some compression algorithms. Most obviously
have a positive interger for a compression level. And at least some have not a
window as zlib, but a dictionary size which is also a power of two. So it might
be reasonable to have that second
uint8_t also in the default header for a window/dictionary size exponent.
So it would propose to update the header like this:
Byte 0 - 13: compress_format_name
14: compress_level (uint8_t)
15: compress_window_exp (uint8_t)
Peter
[Qemu-devel] [PATCH V4 10/10] block/qcow2: add compress info to image specific info, Peter Lieven, 2017/07/20
[Qemu-devel] [PATCH V4 02/10] qapi/block-core: add Qcow2Compress parameters, Peter Lieven, 2017/07/20
[Qemu-devel] [PATCH V4 07/10] block/qcow2: optimize qcow2_co_pwritev_compressed, Peter Lieven, 2017/07/20
[Qemu-devel] [PATCH V4 06/10] block/qcow2: simplify ret usage in qcow2_create, Peter Lieven, 2017/07/20
[Qemu-devel] [PATCH V4 05/10] block/qcow2: read and write the compress format extension, Peter Lieven, 2017/07/20
[Qemu-devel] [PATCH V4 03/10] block/qcow2: parse compress create options, Peter Lieven, 2017/07/20
[Qemu-devel] [PATCH V4 09/10] block/qcow2: add lzo compress format, Peter Lieven, 2017/07/20