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

[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.

> +
> +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.

> +
> +                   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?

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

signature.asc
Description: OpenPGP digital signature