Re: [Qemu-devel] [PATCH V3 1/9] specs/qcow2: add compress format extension

[Peter Lieven]

Am 14.07.2017 um 16:52 schrieb Eric Blake:
> On 07/14/2017 04:56 AM, Peter Lieven wrote:
>> Signed-off-by: Peter Lieven <address@hidden>
>> ---
>>  docs/interop/qcow2.txt | 35 ++++++++++++++++++++++++++++++++++-
>>  1 file changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
>> index d7fdb1f..c2d3dab 100644
>> --- a/docs/interop/qcow2.txt
>> +++ b/docs/interop/qcow2.txt
>> @@ -86,7 +86,12 @@ in the description of a field.
>>                                  be written to (unless for regaining
>>                                  consistency).
>>  
>> -                    Bits 2-63:  Reserved (set to 0)
>> +                    Bit 2:      Compress format bit.  If and only if this 
>> bit
>> +                                is set then the compress format extension
>> +                                MUST be present and MUST be parsed and 
>> checked
>> +                                for compatibility.
>> +
>> +                    Bits 3-63:  Reserved (set to 0)
>>  
>>           80 -  87:  compatible_features
>>                      Bitmask of compatible features. An implementation can
>> @@ -137,6 +142,7 @@ be stored. Each extension has a structure like the 
>> following:
>>                          0x6803f857 - Feature name table
>>                          0x23852875 - Bitmaps extension
>>                          0x0537be77 - Full disk encryption header pointer
>> +                        0xC03183A3 - Compress format extension
>>                          other      - Unknown header extension, can be safely
>>                                       ignored
>>  
>> @@ -311,6 +317,33 @@ The algorithms used for encryption vary depending on 
>> the method
>>     in the LUKS header, with the physical disk sector as the
>>     input tweak.
>>  
>> +
>> +== Compress format extension ==
>> +
>> +The compress format extension is an optional header extension. It provides
>> +the ability to specify the compress algorithm and compress parameters
>> +that are used for compressed clusters. This new header MUST be present if
>> +the incompatible-feature bit "compress format bit" is set and MUST be absent
>> +otherwise.
> Probably worth mentioning that omitting the incompatible "Compress
> format bit" is the same as setting it and populating the Compress format
> extension with "zlib"/0.  Don't know if that fits better here, or up
> above at the incompatible feature bit section.

Its not exactly the same. Its zlib with default compression, but the old
default was to use 12 windowBits whereas windowBits are 15 when
the header is present. The reason behind this is that the inflate call
needs the maximum alllowed windowBits as parameter. In theory
it should fail if it has a 12 as parameter and receives an input stream
with 15. My zlib doesn't, but the docs say something else.
The change of the windowBits to 15 yields in about 15% better compression
time and about 5% better compression ratio.

>
>> +
>> +The fields of the compress format extension are:
>> +
>> +    Byte  0 - 15:  compress_format_name (padded with zeros, but not
>> +                   necessarily null terminated if it has full length).
>> +                   Valid compression format names currently are:
>> +
>> +                   zlib: Standard zlib deflate compression
> 16 bytes,
>
>> +
>> +              16:  compress_level (uint8_t)
> + 1, means you now have to pad the struct. I'd make the
> compress_format_name be only 15 bits (bytes 0-14, compress_level at 15),
> so that you have nice alignment without wasting padding.

Good point, I will change that.

>
>> +
>> +                   0 = default compress level (valid for all formats, 
>> default)
>> +
>> +                   Additional valid compression levels for zlib compression:
>> +
>> +                   All values between 1 and 9. 1 gives best speed, 9 gives 
>> best
>> +                   compression.
> Is there a particular value between 1 and 9 that matches the output you
> get when using 0?
>
Thats depending on what the macro Z_DEFAULT_COMPRESSION points to.

Its provided by libz. The zlib docs say it currently is set to level 6.


Peter