Re: [Qemu-devel] [RFC 1/1] qcow2: add ZSTD compression feature

[Kevin Wolf]

Am 23.03.2017 um 15:17 hat Eric Blake geschrieben:
> On 03/23/2017 08:28 AM, Denis V. Lunev wrote:
> > ZSDT compression algorithm consumes 3-5 times less CPU power with a
> 
> s/ZSDT/ZSTD/
> 
> > comparable comression ratio with zlib. It would be wise to use it for
> 
> s/comression/compression/
> 
> > data compression f.e. for backups.

Note that we don't really care that much about fast compression because
that's an one time offline operation. Maybe a better compression ratio
while maintaining decent decompression performance would be the more
important feature?

Or are you planning to extend the qcow2 driver so that compressed
clusters are used even for writes after the initial conversion? I think
it would be doable, and then I can see that better compression speed
becomes important, too.

> > The patch adds incompatible ZSDT feature into QCOW2 header that indicates
> > that compressed clusters must be decoded using ZSTD.
> > 
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Kevin Wolf <address@hidden>
> > CC: Max Reitz <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Fam Zheng <address@hidden>
> > ---
> > Actually this is very straightforward. May be we should implement 2 stage
> > scheme, i.e. add bit that indicates presence of the "compression
> > extension", which will actually define the compression algorithm. Though
> > at my opinion we will not have too many compression algorithms and proposed
> > one tier scheme is good enough.
> 
> I wouldn't bet on NEVER changing compression algorithms again, and while
> I suspect that we won't necessarily run out of bits, it's safer to not
> require burning another bit every time we change our minds.  Having a
> two-level scheme means we only have to burn 1 bit for the use of a
> compression extension header, where we can then flip algorithms in the
> extension header without having to burn a top-level incompatible feature
> bit every time.

Header extensions make sense for compatible features or for variable
size data. In this specific case I would simply increase the header size
if we want another field to store the compression algorithm. And I think
having such a field is a good idea.

> > 
> >  docs/specs/qcow2.txt | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> > index 80cdfd0..eb5c41b 100644
> > --- a/docs/specs/qcow2.txt
> > +++ b/docs/specs/qcow2.txt
> > @@ -85,7 +85,10 @@ in the description of a field.
> >                                  be written to (unless for regaining
> >                                  consistency).
> >  
> > -                    Bits 2-63:  Reserved (set to 0)
> > +                    Bits 2:     ZSDT compression bit. ZSDT algorithm is 
> > used
> 
> s/ZSDT/ZSTD/
> 
> Another reason I think you should add a compression extension header:
> compression algorithms are probably best treated as mutually-exclusive
> (the entire image should be compressed with exactly one compressor).
> Even if we only ever add one more type (say 'xz') in addition to the
> existing gzip and your proposed zstd, then we do NOT want someone
> specifying both xz and zstd at the same time.  Having a single
> incompatible feature bit that states that a compression header must be
> present and honored to understand the image, where the compression
> header then chooses exactly one compression algorithm, seems safer than
> having two separate incompatible feature bits for two opposing algorithms

Actually, if we used compression after the initial convert, having
mixed-format images would make a lot of sense because after an update
you could then start using a new compression format on an image that
already has some compressed clusters.

But we have neither L2 table bits left for this nor do we use
compression for later writes, so I agree that we'll have to make them
mututally exclusive in this reality.

Kevin