Re: [PATCH v8 3/3] docs: qcow2: introduce compression type feature

[Eric Blake]

On 10/18/19 4:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> The patch add new additional field to qcow2 header: compression_type,
> which specifies compression type. If field is absent or zero, default
> compression type is set: ZLIB, which corresponds to current behavior.
>
> New compression type (ZSTD) is to be added in further commit.
>
> Suggested-by: Denis Plotnikov <address@hidden>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>   docs/interop/qcow2.txt | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index b971e59b1a..4eabd81363 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -109,6 +109,12 @@ least next five fields, up to the @header_length field.
>                                   An External Data File Name header extension 
> may
>                                   be present if this bit is set.
>   
> +                    Bit 3:      Compression type bit.  If this bit is set,
> +                                non-default compression is used for compressed
> +                                clusters. In this case, @header_length must
> +                                be at least 105 and @compression_type field
> +                                must be non-zero.

I want to insist on 8-byte alignment, so this should be at least 112.

Also, as pointed out in v7, use of the decoration '@header_length' and 
'@compression_type' is not consistent with the rest of the document. 
Drop the @.

> +
>                       Bits 3-63:  Reserved (set to 0)
>   
>            80 -  87:  compatible_features
> @@ -183,7 +189,18 @@ It's allowed for the header end to cut some field in the 
> middle (in this case
>   the field is considered as absent), but in this case the part of the field
>   which is covered by @header_length must be zeroed.
>   
> -        < ... No additional fields in the header currently ... >
> +              104:  compression_type
> +                    Defines the compression method used for compressed 
> clusters.
> +                    A single compression type is applied to all compressed 
> image
> +                    clusters.
> +                    If incompatible compression type bit is set: the field must
> +                    exist (i.e. @header_length >= 105) and must be non-zero (
> +                    which means non-zlib compression type)
> +                    If incompatible compression type bit is unset: the field
> +                    may not exist (if @header_length < 105) or it must be zero
> +                    (which means zlib).
> +                    Available compression type values:
> +                        0: zlib <https://www.zlib.net/>
>   

One byte for the field is fine, but I'd still explicitly document 7 
bytes of unused padding, since I want to insist on an 8-byte multiple.

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