[Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm

Peter Lieven posted 4 patches 8 years, 7 months ago
[Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm
Posted by Peter Lieven 8 years, 7 months ago
this adds support for optimized zlib settings which almost
tripples the compression speed while maintaining about
the same compressed size.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/qcow2-cluster.c |  3 ++-
 block/qcow2.c         | 11 +++++++++--
 block/qcow2.h         |  1 +
 qemu-img.texi         |  1 +
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index ecb059b..d8e2378 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1532,6 +1532,7 @@ static int decompress_buffer(uint8_t *out_buf, int out_buf_size,
 
     switch (compression_algorithm_id) {
     case QCOW2_COMPRESSION_ZLIB:
+    case QCOW2_COMPRESSION_ZLIB_FAST:
         memset(strm, 0, sizeof(*strm));
 
         strm->next_in = (uint8_t *)buf;
@@ -1539,7 +1540,7 @@ static int decompress_buffer(uint8_t *out_buf, int out_buf_size,
         strm->next_out = out_buf;
         strm->avail_out = out_buf_size;
 
-        ret = inflateInit2(strm, -12);
+        ret = inflateInit2(strm, -15);
         if (ret != Z_OK) {
             return -1;
         }
diff --git a/block/qcow2.c b/block/qcow2.c
index bd65582..f07d8f0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -87,6 +87,8 @@ static uint32_t is_compression_algorithm_supported(char *algorithm)
         /* no algorithm means the old default of zlib compression
          * with 12 window bits */
         return QCOW2_COMPRESSION_ZLIB;
+    } else if (!strcmp(algorithm, "zlib-fast")) {
+        return QCOW2_COMPRESSION_ZLIB_FAST;
 #ifdef CONFIG_LZO
     } else if (!strcmp(algorithm, "lzo")) {
         return QCOW2_COMPRESSION_LZO;
@@ -2722,6 +2724,7 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
     QEMUIOVector hd_qiov;
     struct iovec iov;
     z_stream strm;
+    int z_level = Z_DEFAULT_COMPRESSION, z_windowBits = -12;
     int ret, out_len = 0;
     uint8_t *buf, *out_buf = NULL, *local_buf = NULL, *work_buf = NULL;
     uint64_t cluster_offset;
@@ -2749,13 +2752,17 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
     }
 
     switch (s->compression_algorithm_id) {
+    case QCOW2_COMPRESSION_ZLIB_FAST:
+        z_level = Z_BEST_SPEED;
+        z_windowBits = -15;
+        /* fall-through */
     case QCOW2_COMPRESSION_ZLIB:
         out_buf = g_malloc(s->cluster_size);
 
         /* best compression, small window, no zlib header */
         memset(&strm, 0, sizeof(strm));
-        ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION,
-                           Z_DEFLATED, -12,
+        ret = deflateInit2(&strm, z_level,
+                           Z_DEFLATED, z_windowBits,
                            9, Z_DEFAULT_STRATEGY);
         if (ret != 0) {
             ret = -EINVAL;
diff --git a/block/qcow2.h b/block/qcow2.h
index 716012c..a89f986 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -173,6 +173,7 @@ typedef struct Qcow2UnknownHeaderExtension {
 enum {
     QCOW2_COMPRESSION_ZLIB          = 0xC0318301,
     QCOW2_COMPRESSION_LZO           = 0xC0318302,
+    QCOW2_COMPRESSION_ZLIB_FAST     = 0xC0318303,
 };
 
 enum {
diff --git a/qemu-img.texi b/qemu-img.texi
index 043c1ba..83a5db2 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -627,6 +627,7 @@ The following options are available if support for the respective libraries
 has been enabled at compile time:
 
    zlib            Uses standard zlib compression (default)
+   zlib-fast       Uses zlib compression with optimized compression parameters
    lzo             Uses LZO1X compression
 
 The compression algorithm can only be defined at image create time and cannot
-- 
1.9.1


Re: [Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm
Posted by Eric Blake 8 years, 7 months ago
On 06/27/2017 07:34 AM, Peter Lieven wrote:
> this adds support for optimized zlib settings which almost

Start sentences with a capital.

> tripples the compression speed while maintaining about

s/tripples/triples/

> the same compressed size.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/qcow2-cluster.c |  3 ++-
>  block/qcow2.c         | 11 +++++++++--
>  block/qcow2.h         |  1 +
>  qemu-img.texi         |  1 +
>  4 files changed, 13 insertions(+), 3 deletions(-)
> 

> +++ b/block/qcow2.h
> @@ -173,6 +173,7 @@ typedef struct Qcow2UnknownHeaderExtension {
>  enum {
>      QCOW2_COMPRESSION_ZLIB          = 0xC0318301,
>      QCOW2_COMPRESSION_LZO           = 0xC0318302,
> +    QCOW2_COMPRESSION_ZLIB_FAST     = 0xC0318303,

Back to my comments on 1/4 - we MUST first get the qcow2 specification
right, rather than adding undocumented headers in the code.  And I still
think you only need one variable-length header extension for covering
all possible algorithms, rather than one header per algorithm.  Let's
get the spec right first, before worrying about the code implementing
the spec.

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

Re: [Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm
Posted by Peter Lieven 8 years, 7 months ago
Am 27.06.2017 um 14:53 schrieb Eric Blake:
> On 06/27/2017 07:34 AM, Peter Lieven wrote:
>> this adds support for optimized zlib settings which almost
> Start sentences with a capital.
>
>> tripples the compression speed while maintaining about
> s/tripples/triples/
>
>> the same compressed size.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   block/qcow2-cluster.c |  3 ++-
>>   block/qcow2.c         | 11 +++++++++--
>>   block/qcow2.h         |  1 +
>>   qemu-img.texi         |  1 +
>>   4 files changed, 13 insertions(+), 3 deletions(-)
>>
>> +++ b/block/qcow2.h
>> @@ -173,6 +173,7 @@ typedef struct Qcow2UnknownHeaderExtension {
>>   enum {
>>       QCOW2_COMPRESSION_ZLIB          = 0xC0318301,
>>       QCOW2_COMPRESSION_LZO           = 0xC0318302,
>> +    QCOW2_COMPRESSION_ZLIB_FAST     = 0xC0318303,
> Back to my comments on 1/4 - we MUST first get the qcow2 specification
> right, rather than adding undocumented headers in the code.  And I still
> think you only need one variable-length header extension for covering
> all possible algorithms, rather than one header per algorithm.  Let's
> get the spec right first, before worrying about the code implementing
> the spec.
>

Okay, I think someone came up with the idea to have an optional
header per algorithm, but you are right one header with an optional
parameter payload will also do.

I will split the spec change to a separate patch in V2 to make it easier
to respin.

Thanks,
Peter

Re: [Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm
Posted by Daniel P. Berrange 8 years, 7 months ago
On Tue, Jun 27, 2017 at 02:34:10PM +0200, Peter Lieven wrote:
> this adds support for optimized zlib settings which almost
> tripples the compression speed while maintaining about
> the same compressed size.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/qcow2-cluster.c |  3 ++-
>  block/qcow2.c         | 11 +++++++++--
>  block/qcow2.h         |  1 +
>  qemu-img.texi         |  1 +
>  4 files changed, 13 insertions(+), 3 deletions(-)
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 043c1ba..83a5db2 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -627,6 +627,7 @@ The following options are available if support for the respective libraries
>  has been enabled at compile time:
>  
>     zlib            Uses standard zlib compression (default)
> +   zlib-fast       Uses zlib compression with optimized compression parameters

This looks like a poor design from a future proofing POV. I would much
rather see us introduce a more flexible modelling of compression at the
QAPI layer which lets us have tunables for each algorith, in the same
way that the qcow2 built-in encryption now has ability to set tunables
for each algorithm.

eg your "zlib-fast" impl which just uses zlib with a window size of -15
could be expressed as

   qemu-img create -o compress.format=zlib,compress.window=-15



>     lzo             Uses LZO1X compression


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm
Posted by Peter Lieven 8 years, 7 months ago
Am 27.06.2017 um 15:16 schrieb Daniel P. Berrange:
> On Tue, Jun 27, 2017 at 02:34:10PM +0200, Peter Lieven wrote:
>> this adds support for optimized zlib settings which almost
>> tripples the compression speed while maintaining about
>> the same compressed size.
>>
>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> ---
>>   block/qcow2-cluster.c |  3 ++-
>>   block/qcow2.c         | 11 +++++++++--
>>   block/qcow2.h         |  1 +
>>   qemu-img.texi         |  1 +
>>   4 files changed, 13 insertions(+), 3 deletions(-)
>> diff --git a/qemu-img.texi b/qemu-img.texi
>> index 043c1ba..83a5db2 100644
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -627,6 +627,7 @@ The following options are available if support for the respective libraries
>>   has been enabled at compile time:
>>   
>>      zlib            Uses standard zlib compression (default)
>> +   zlib-fast       Uses zlib compression with optimized compression parameters
> This looks like a poor design from a future proofing POV. I would much
> rather see us introduce a more flexible modelling of compression at the
> QAPI layer which lets us have tunables for each algorith, in the same
> way that the qcow2 built-in encryption now has ability to set tunables
> for each algorithm.
>
> eg your "zlib-fast" impl which just uses zlib with a window size of -15
> could be expressed as
>
>     qemu-img create -o compress.format=zlib,compress.window=-15

It would also need a compress.level, but okay. This will make things
more complicated as one might not know what good values for
the algoritm are.

Wouldn't it be better to have sth like compress.level=1..9 and let
the implementation decide which parameters it choose for the algorithm?

Do you have a pointer where I can find out how to imlement that
in QAPI? Maybe the patches that introduces the encryption settings?

Thanks,
Peter


Re: [Qemu-devel] [PATCH 4/4] block/qcow2: add zlib-fast compression algorithm
Posted by Daniel P. Berrange 8 years, 7 months ago
On Tue, Jun 27, 2017 at 03:23:19PM +0200, Peter Lieven wrote:
> Am 27.06.2017 um 15:16 schrieb Daniel P. Berrange:
> > On Tue, Jun 27, 2017 at 02:34:10PM +0200, Peter Lieven wrote:
> > > this adds support for optimized zlib settings which almost
> > > tripples the compression speed while maintaining about
> > > the same compressed size.
> > > 
> > > Signed-off-by: Peter Lieven <pl@kamp.de>
> > > ---
> > >   block/qcow2-cluster.c |  3 ++-
> > >   block/qcow2.c         | 11 +++++++++--
> > >   block/qcow2.h         |  1 +
> > >   qemu-img.texi         |  1 +
> > >   4 files changed, 13 insertions(+), 3 deletions(-)
> > > diff --git a/qemu-img.texi b/qemu-img.texi
> > > index 043c1ba..83a5db2 100644
> > > --- a/qemu-img.texi
> > > +++ b/qemu-img.texi
> > > @@ -627,6 +627,7 @@ The following options are available if support for the respective libraries
> > >   has been enabled at compile time:
> > >      zlib            Uses standard zlib compression (default)
> > > +   zlib-fast       Uses zlib compression with optimized compression parameters
> > This looks like a poor design from a future proofing POV. I would much
> > rather see us introduce a more flexible modelling of compression at the
> > QAPI layer which lets us have tunables for each algorith, in the same
> > way that the qcow2 built-in encryption now has ability to set tunables
> > for each algorithm.
> > 
> > eg your "zlib-fast" impl which just uses zlib with a window size of -15
> > could be expressed as
> > 
> >     qemu-img create -o compress.format=zlib,compress.window=-15
> 
> It would also need a compress.level, but okay. This will make things
> more complicated as one might not know what good values for
> the algoritm are.
> 
> Wouldn't it be better to have sth like compress.level=1..9 and let
> the implementation decide which parameters it choose for the algorithm?

Yep, there's definitely a choice of approaches - exposing low level
details of each library as parameters, vs trying to come up with a
more abstracted, simplified notation and having the driver pick some
of the low level details implicitly from that. Both are valid, and I
don't have a particular opinion either way.

> Do you have a pointer where I can find out how to imlement that
> in QAPI? Maybe the patches that introduces the encryption settings?

It is a little messy since we don't fully use QAPI in the block create
code path. If you want to look at what I did for encryption though, see
the block/qcow2.c file. In the qcow2_create() method I grab the
encrypt.format option. Later in the qcow2_set_up_encryption() method
I then extract & parse the format specific options from the QemuOpts
array. My code is in Max's block branch, or on block-luks-qcow2-10 branch
at https://github.com/berrange/qemu

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|