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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2693 bytes --]

Am 08.08.2019 um 14:50 hat Max Reitz geschrieben:
> On 08.08.19 02:18, Eric Blake wrote:
> > 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.
> 
> Just because an image doesn’t adhere to the specification doesn’t mean
> you have to reject it, if the intention is clear.
> 
> > I'd drop this sentence.
> 
> I wouldn’t, I like it (in essence).  Though maybe the “must” is indeed
> too strong and it should be a “should” instead.

I'd agree that "should" is the best way for the spec.

In the code, I'd insist that QEMU doesn't add any extra code to verify
that this is the case (which previous versions of the series did).

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]