Re: [Qemu-devel] [PATCH v2 1/3] qcow2: introduce compression type feature

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3632 bytes --]

On 7/4/19 8:09 AM, Denis Plotnikov wrote:
> The patch adds some preparation parts for incompatible compression type
> feature to QCOW2 header that indicates that *all* compressed clusters
> must be (de)compressed using a certain compression type.
> 
> It is implied that the compression type is set on the image creation and
> can be changed only later by image conversion, thus compression type
> defines the only compression algorithm used for the image.
> 
> The goal of the feature is to add support of other compression algorithms
> to qcow2. For example, ZSTD which is more effective on compression than ZLIB.
> It works roughly 2x faster than ZLIB providing a comparable compression ratio
> and therefore provide a performance advantage in backup scenarios.

provides

> 
> The default compression is ZLIB. Images created with ZLIB compression type
> are backward compatible with older qemu versions.
> 
> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>

> +++ b/docs/interop/qcow2.txt
> @@ -109,7 +109,12 @@ in the description of a field.
>                                  An External Data File Name header extension may
>                                  be present if this bit is set.
>  
> -                    Bits 3-63:  Reserved (set to 0)
> +                    Bit 3:      Compression type bit. The bit must be set if
> +                                the compression type differs from default: zlib.

I'd word this 'from the default of zlib.'

> +                                If the compression type is default the bit must
> +                                be unset.

Why? I see no reason to forbid a qcow2 image that has the incompatible
bit set but still uses zlib compression.  True, such an image is not as
friendly to older clients, but it is not technically wrong, and newer
clients would still be able to use the image if not for this sentence
telling them they must not.  I'd drop this sentence.

> +
> +                    Bits 4-63:  Reserved (set to 0)
>  
>           80 -  87:  compatible_features
>                      Bitmask of compatible features. An implementation can
> @@ -165,6 +170,20 @@ in the description of a field.
>                      Length of the header structure in bytes. For version 2
>                      images, the length is always assumed to be 72 bytes.
>  
> +        104 - 107:  compression_type
> +                    Defines the compression method used for compressed clusters.
> +                    A single compression type is applied to all compressed image
> +                    clusters.
> +                    The compression type is set on image creation only.
> +                    The default compression type is zlib (value: 0).
> +                    When the compression type differs from the default
> +                    the compression type bit (incompatible feature bit 3)
> +                    must be set.

So far, so good.

> +                    Qemu versions older than 4.1 can use images created with
> +                    the default compression type without any additional
> +                    preparations and cannot use images created with any other
> +                    compression type.

I'm wondering whether we need to spell this out in the spec.  Yes, I
know we spell out other qemu limitations elsewhere, but with a version
number.  But the spec would not be any less correct if you omitted this
sentence.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]