All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression
@ 2019-09-05  9:31 Denis Plotnikov
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature Denis Plotnikov
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-05  9:31 UTC (permalink / raw)
  To: armbru, qemu-devel; +Cc: kwolf, vsementsov, den, qemu-block, mreitz

v6:
* fixed zstd compressed length storing/loading [Eric]
* fixed wording, spec section placement [Eric]

v5:
* type changed for compression_type at BDRVQcow2State [Kevin]
* fixed typos, grammar [Kevin]
* fixed default config zstd setting [Kevin]

v4:
* remove not feasible switch case [Vladimir]
* add sanity checks to zstd decompresssion [Vladimir]
* store zstd compressed length in big endian [Max, Kevin]

v3:
* relax the compression type setting requirement when
  the compression type is not zlib [Eric, Kevin]
* add compression type values to the spec [Eric]
* fix wording in the spec and descriptions [Eric]
* fix functions descriptions [Max]
* fix zstd (de)compression functions flaws [Max]
* fix zstd related parts of configure file [Max]
* rebased to v4.1.0-rc5 and chenged the series version aiming to 4.2

v2:
* relax the compression type setting restriction in the spec
* fix qcow2 header size checking
* fix error processing and messaging
* fix qcow2 image specific info reporting
* set Qcow2CompressionType zstd config dependant
* add zstd compressed cluster format description to the spec

v1:
* extend qcow2 header instead of adding a new incompatible extension header
specification re-written accordingly
* enable zstd compression via config
* fix zstd (de)compression functions
* fix comments/description
* fix function naming

---
The goal of the patch-set is to enable qcow2 to use zstd compression for
clusters. ZSTD provides better (de)compression performance than currently
used ZLIB. Using it will improve perforamnce (reduce compression time)
when the compressed clusters is used, e.g backup scenarios.

Also, the patch-set extends qcow2 specification by adding compression_type
feature. The feature enables adding ZSTD and another compression algorithms
in the future.

Here is some measurements ZSTD vs ZLIB:

The test:
Test compresses and decompresses qemu qcow2 image with just
installed rhel-7.6 guest.
Image cluster size: 64K. Image on disk size: 2.2G

The test was conducted with brd disk to reduce the influence
of disk subsystem to the test results.
The results is given in seconds.

compress cmd:
time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
src.img [zlib|zstd]_compressed.img
decompress cmd
time ./qemu-img convert -O qcow2
[zlib|zstd]_compressed.img uncompressed.img


The results:
compression decompression
zlib zstd zlib zstd
------------------------------------------------------------
real 65.5 16.3 (-75 %) 1.9 1.6 (-16 %)
user 65.0 15.8 5.3 2.5
sys 3.3 0.2 2.0 2.0

Both ZLIB and ZSTD gave the same compression ratio: ~1.5
compressed image size in both cases: ~1.4G

Denis Plotnikov (3):
  qcow2: introduce compression type feature
  qcow2: rework the cluster compression routine
  qcow2: add zstd cluster compression

 block/qcow2-threads.c     | 199 +++++++++++++++++++++++++++++++++++---
 block/qcow2.c             |  98 +++++++++++++++++++
 block/qcow2.h             |  26 +++--
 configure                 |  29 ++++++
 docs/interop/qcow2.txt    |  35 ++++++-
 include/block/block_int.h |   1 +
 qapi/block-core.json      |  23 ++++-
 7 files changed, 387 insertions(+), 24 deletions(-)

-- 
2.17.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature
  2019-09-05  9:31 [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression Denis Plotnikov
@ 2019-09-05  9:31 ` Denis Plotnikov
  2019-09-05 13:44   ` Vladimir Sementsov-Ogievskiy
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 2/3] qcow2: rework the cluster compression routine Denis Plotnikov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-05  9:31 UTC (permalink / raw)
  To: armbru, qemu-devel; +Cc: kwolf, vsementsov, den, qemu-block, mreitz

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 provides a performance advantage in backup scenarios.

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>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2.c             | 91 +++++++++++++++++++++++++++++++++++++++
 block/qcow2.h             | 26 ++++++++---
 docs/interop/qcow2.txt    | 18 +++++++-
 include/block/block_int.h |  1 +
 qapi/block-core.json      | 22 +++++++++-
 5 files changed, 148 insertions(+), 10 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 039bdc2f7e..2884b9d9f2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1197,6 +1197,32 @@ static int qcow2_update_options(BlockDriverState *bs, QDict *options,
     return ret;
 }
 
+static int check_compression_type(BDRVQcow2State *s, Error **errp)
+{
+    switch (s->compression_type) {
+    case QCOW2_COMPRESSION_TYPE_ZLIB:
+        break;
+
+    default:
+        error_setg(errp, "qcow2: unknown compression type: %u",
+                   s->compression_type);
+        return -ENOTSUP;
+    }
+
+    /*
+     * if the compression type differs from QCOW2_COMPRESSION_TYPE_ZLIB
+     * the incompatible feature flag must be set
+     */
+
+    if (s->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB &&
+        !(s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION_TYPE)) {
+            error_setg(errp, "qcow2: Invalid compression type setting");
+            return -EINVAL;
+    }
+
+    return 0;
+}
+
 /* Called with s->lock held.  */
 static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
                                       int flags, Error **errp)
@@ -1312,6 +1338,35 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
     s->compatible_features      = header.compatible_features;
     s->autoclear_features       = header.autoclear_features;
 
+    /*
+     * Handle compression type
+     * Older qcow2 images don't contain the compression type header.
+     * Distinguish them by the header length and use
+     * the only valid (default) compression type in that case
+     */
+    if (header.header_length > offsetof(QCowHeader, compression_type)) {
+        /* sanity check that we can read a compression type */
+        size_t min_len = offsetof(QCowHeader, compression_type) +
+                         sizeof(header.compression_type);
+        if (header.header_length < min_len) {
+            error_setg(errp,
+                       "Could not read compression type, "
+                       "qcow2 header is too short");
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        header.compression_type = be32_to_cpu(header.compression_type);
+        s->compression_type = header.compression_type;
+    } else {
+        s->compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
+    }
+
+    ret = check_compression_type(s, errp);
+    if (ret) {
+        goto fail;
+    }
+
     if (s->incompatible_features & ~QCOW2_INCOMPAT_MASK) {
         void *feature_table = NULL;
         qcow2_read_extensions(bs, header.header_length, ext_end,
@@ -2516,6 +2571,12 @@ int qcow2_update_header(BlockDriverState *bs)
     total_size = bs->total_sectors * BDRV_SECTOR_SIZE;
     refcount_table_clusters = s->refcount_table_size >> (s->cluster_bits - 3);
 
+    ret = check_compression_type(s, NULL);
+
+    if (ret) {
+        goto fail;
+    }
+
     *header = (QCowHeader) {
         /* Version 2 fields */
         .magic                  = cpu_to_be32(QCOW_MAGIC),
@@ -2538,6 +2599,7 @@ int qcow2_update_header(BlockDriverState *bs)
         .autoclear_features     = cpu_to_be64(s->autoclear_features),
         .refcount_order         = cpu_to_be32(s->refcount_order),
         .header_length          = cpu_to_be32(header_length),
+        .compression_type       = cpu_to_be32(s->compression_type),
     };
 
     /* For older versions, write a shorter header */
@@ -2635,6 +2697,11 @@ int qcow2_update_header(BlockDriverState *bs)
                 .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
                 .name = "lazy refcounts",
             },
+            {
+                .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
+                .bit  = QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR,
+                .name = "compression type",
+            },
         };
 
         ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE,
@@ -3202,6 +3269,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
         .refcount_table_offset      = cpu_to_be64(cluster_size),
         .refcount_table_clusters    = cpu_to_be32(1),
         .refcount_order             = cpu_to_be32(refcount_order),
+        .compression_type           = cpu_to_be32(QCOW2_COMPRESSION_TYPE_ZLIB),
         .header_length              = cpu_to_be32(sizeof(*header)),
     };
 
@@ -3221,6 +3289,21 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
             cpu_to_be64(QCOW2_AUTOCLEAR_DATA_FILE_RAW);
     }
 
+    if (qcow2_opts->has_compression_type &&
+        qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
+
+        switch (qcow2_opts->compression_type) {
+        default:
+            error_setg_errno(errp, -EINVAL, "Unknown compression type");
+            goto out;
+        }
+
+        header->compression_type = cpu_to_be32(qcow2_opts->compression_type);
+
+        header->incompatible_features |=
+            cpu_to_be64(QCOW2_INCOMPAT_COMPRESSION_TYPE);
+    }
+
     ret = blk_pwrite(blk, 0, header, cluster_size, 0);
     g_free(header);
     if (ret < 0) {
@@ -3402,6 +3485,7 @@ static int coroutine_fn qcow2_co_create_opts(const char *filename, QemuOpts *opt
         { BLOCK_OPT_ENCRYPT,            BLOCK_OPT_ENCRYPT_FORMAT },
         { BLOCK_OPT_COMPAT_LEVEL,       "version" },
         { BLOCK_OPT_DATA_FILE_RAW,      "data-file-raw" },
+        { BLOCK_OPT_COMPRESSION_TYPE,   "compression-type" },
         { NULL, NULL },
     };
 
@@ -4598,6 +4682,7 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
             .data_file          = g_strdup(s->image_data_file),
             .has_data_file_raw  = has_data_file(bs),
             .data_file_raw      = data_file_is_raw(bs),
+            .compression_type   = s->compression_type,
         };
     } else {
         /* if this assertion fails, this probably means a new version was
@@ -5163,6 +5248,12 @@ static QemuOptsList qcow2_create_opts = {
             .help = "Width of a reference count entry in bits",
             .def_value_str = "16"
         },
+        {
+            .name = BLOCK_OPT_COMPRESSION_TYPE,
+            .type = QEMU_OPT_STRING,
+            .help = "Compression method used for image clusters compression",
+            .def_value_str = "zlib"
+        },
         { /* end of list */ }
     }
 };
diff --git a/block/qcow2.h b/block/qcow2.h
index fc1b0d3c1e..659e4f852a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -140,6 +140,7 @@ typedef struct QCowHeader {
 
     uint32_t refcount_order;
     uint32_t header_length;
+    uint32_t compression_type;
 } QEMU_PACKED QCowHeader;
 
 typedef struct QEMU_PACKED QCowSnapshotHeader {
@@ -203,16 +204,20 @@ enum {
 
 /* Incompatible feature bits */
 enum {
-    QCOW2_INCOMPAT_DIRTY_BITNR      = 0,
-    QCOW2_INCOMPAT_CORRUPT_BITNR    = 1,
-    QCOW2_INCOMPAT_DATA_FILE_BITNR  = 2,
-    QCOW2_INCOMPAT_DIRTY            = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
-    QCOW2_INCOMPAT_CORRUPT          = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
-    QCOW2_INCOMPAT_DATA_FILE        = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
+    QCOW2_INCOMPAT_DIRTY_BITNR            = 0,
+    QCOW2_INCOMPAT_CORRUPT_BITNR          = 1,
+    QCOW2_INCOMPAT_DATA_FILE_BITNR        = 2,
+    QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR = 3,
+    QCOW2_INCOMPAT_DIRTY                  = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
+    QCOW2_INCOMPAT_CORRUPT                = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
+    QCOW2_INCOMPAT_DATA_FILE              = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
+    QCOW2_INCOMPAT_COMPRESSION_TYPE       =
+        1 << QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR,
 
     QCOW2_INCOMPAT_MASK             = QCOW2_INCOMPAT_DIRTY
                                     | QCOW2_INCOMPAT_CORRUPT
-                                    | QCOW2_INCOMPAT_DATA_FILE,
+                                    | QCOW2_INCOMPAT_DATA_FILE
+                                    | QCOW2_INCOMPAT_COMPRESSION_TYPE,
 };
 
 /* Compatible feature bits */
@@ -359,6 +364,13 @@ typedef struct BDRVQcow2State {
 
     bool metadata_preallocation_checked;
     bool metadata_preallocation;
+    /*
+     * Compression type used for the image. Default: 0 - ZLIB
+     * The image compression type is set on image creation.
+     * The only way to change the compression type is to convert the image
+     * with the desired compression type set
+     */
+    Qcow2CompressionType compression_type;
 } BDRVQcow2State;
 
 typedef struct Qcow2COWRegion {
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index af5711e533..59e09caacf 100644
--- a/docs/interop/qcow2.txt
+++ 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 of zlib.
+                                If the compression type is default the bit should
+                                be unset.
+
+                    Bits 4-63:  Reserved (set to 0)
 
          80 -  87:  compatible_features
                     Bitmask of compatible features. An implementation can
@@ -165,6 +170,17 @@ 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 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.
+                    Available compression type values:
+                        0: zlib <https://www.zlib.net/> (default)
+
 Directly after the image header, optional sections called header extensions can
 be stored. Each extension has a structure like the following:
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3aa1e832a8..4b254802e5 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -58,6 +58,7 @@
 #define BLOCK_OPT_REFCOUNT_BITS     "refcount_bits"
 #define BLOCK_OPT_DATA_FILE         "data_file"
 #define BLOCK_OPT_DATA_FILE_RAW     "data_file_raw"
+#define BLOCK_OPT_COMPRESSION_TYPE  "compression_type"
 
 #define BLOCK_PROBE_BUF_SIZE        512
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0d43d4f37c..2c002ca6a9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -78,6 +78,8 @@
 #
 # @bitmaps: A list of qcow2 bitmap details (since 4.0)
 #
+# @compression-type: the image cluster compression method (since 4.2)
+#
 # Since: 1.7
 ##
 { 'struct': 'ImageInfoSpecificQCow2',
@@ -89,7 +91,8 @@
       '*corrupt': 'bool',
       'refcount-bits': 'int',
       '*encrypt': 'ImageInfoSpecificQCow2Encryption',
-      '*bitmaps': ['Qcow2BitmapInfo']
+      '*bitmaps': ['Qcow2BitmapInfo'],
+      'compression-type': 'Qcow2CompressionType'
   } }
 
 ##
@@ -4274,6 +4277,18 @@
   'data': [ 'v2', 'v3' ] }
 
 
+##
+# @Qcow2CompressionType:
+#
+# Compression type used in qcow2 image file
+#
+# @zlib:  zlib compression, see <http://zlib.net/>
+#
+# Since: 4.2
+##
+{ 'enum': 'Qcow2CompressionType',
+  'data': [ 'zlib' ] }
+
 ##
 # @BlockdevCreateOptionsQcow2:
 #
@@ -4297,6 +4312,8 @@
 #                   allowed values: off, falloc, full, metadata)
 # @lazy-refcounts   True if refcounts may be updated lazily (default: off)
 # @refcount-bits    Width of reference counts in bits (default: 16)
+# @compression-type The image cluster compression method
+#                   (default: zlib, since 4.2)
 #
 # Since: 2.12
 ##
@@ -4312,7 +4329,8 @@
             '*cluster-size':    'size',
             '*preallocation':   'PreallocMode',
             '*lazy-refcounts':  'bool',
-            '*refcount-bits':   'int' } }
+            '*refcount-bits':   'int',
+            '*compression-type': 'Qcow2CompressionType' } }
 
 ##
 # @BlockdevCreateOptionsQed:
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 2/3] qcow2: rework the cluster compression routine
  2019-09-05  9:31 [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression Denis Plotnikov
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature Denis Plotnikov
@ 2019-09-05  9:31 ` Denis Plotnikov
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 3/3] qcow2: add zstd cluster compression Denis Plotnikov
  2019-09-12  8:46 ` [Qemu-devel] [PATCH v6 0/3] " Denis Plotnikov
  3 siblings, 0 replies; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-05  9:31 UTC (permalink / raw)
  To: armbru, qemu-devel; +Cc: kwolf, vsementsov, den, qemu-block, mreitz

The patch allows processing the image compression type defined
in the image header and chooses an appropriate method for
image clusters (de)compression.

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/qcow2-threads.c | 77 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 14 deletions(-)

diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
index 3b1e63fe41..ebeef9e568 100644
--- a/block/qcow2-threads.c
+++ b/block/qcow2-threads.c
@@ -74,7 +74,9 @@ typedef struct Qcow2CompressData {
 } Qcow2CompressData;
 
 /*
- * qcow2_compress()
+ * qcow2_zlib_compress()
+ *
+ * Compress @src_size bytes of data using zlib compression method
  *
  * @dest - destination buffer, @dest_size bytes
  * @src - source buffer, @src_size bytes
@@ -83,8 +85,8 @@ typedef struct Qcow2CompressData {
  *          -ENOMEM destination buffer is not enough to store compressed data
  *          -EIO    on any other error
  */
-static ssize_t qcow2_compress(void *dest, size_t dest_size,
-                              const void *src, size_t src_size)
+static ssize_t qcow2_zlib_compress(void *dest, size_t dest_size,
+                                   const void *src, size_t src_size)
 {
     ssize_t ret;
     z_stream strm;
@@ -119,19 +121,19 @@ static ssize_t qcow2_compress(void *dest, size_t dest_size,
 }
 
 /*
- * qcow2_decompress()
+ * qcow2_zlib_decompress()
  *
  * Decompress some data (not more than @src_size bytes) to produce exactly
- * @dest_size bytes.
+ * @dest_size bytes using zlib compression method
  *
  * @dest - destination buffer, @dest_size bytes
  * @src - source buffer, @src_size bytes
  *
  * Returns: 0 on success
- *          -1 on fail
+ *          -EIO on failure
  */
-static ssize_t qcow2_decompress(void *dest, size_t dest_size,
-                                const void *src, size_t src_size)
+static ssize_t qcow2_zlib_decompress(void *dest, size_t dest_size,
+                                     const void *src, size_t src_size)
 {
     int ret = 0;
     z_stream strm;
@@ -144,7 +146,7 @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size,
 
     ret = inflateInit2(&strm, -12);
     if (ret != Z_OK) {
-        return -1;
+        return -EIO;
     }
 
     ret = inflate(&strm, Z_FINISH);
@@ -154,7 +156,7 @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size,
          * @src buffer may be processed partly (because in qcow2 we know size of
          * compressed data with precision of one sector)
          */
-        ret = -1;
+        ret = -EIO;
     }
 
     inflateEnd(&strm);
@@ -189,20 +191,67 @@ qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size,
     return arg.ret;
 }
 
+/*
+ * qcow2_co_compress()
+ *
+ * Compress @src_size bytes of data using the compression
+ * method defined by the image compression type
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *          a negative error code on failure
+ */
 ssize_t coroutine_fn
 qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
                   const void *src, size_t src_size)
 {
-    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-                                qcow2_compress);
+    BDRVQcow2State *s = bs->opaque;
+    Qcow2CompressFunc fn;
+
+    switch (s->compression_type) {
+    case QCOW2_COMPRESSION_TYPE_ZLIB:
+        fn = qcow2_zlib_compress;
+        break;
+
+    default:
+        return -ENOTSUP;
+    }
+
+    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size, fn);
 }
 
+/*
+ * qcow2_co_decompress()
+ *
+ * Decompress some data (not more than @src_size bytes) to produce exactly
+ * @dest_size bytes using the compression method defined by the image
+ * compression type
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *          a negative error code on failure
+ */
 ssize_t coroutine_fn
 qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
                     const void *src, size_t src_size)
 {
-    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size,
-                                qcow2_decompress);
+    BDRVQcow2State *s = bs->opaque;
+    Qcow2CompressFunc fn;
+
+    switch (s->compression_type) {
+    case QCOW2_COMPRESSION_TYPE_ZLIB:
+        fn = qcow2_zlib_decompress;
+        break;
+
+    default:
+        return -ENOTSUP;
+    }
+
+    return qcow2_co_do_compress(bs, dest, dest_size, src, src_size, fn);
 }
 
 
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v6 3/3] qcow2: add zstd cluster compression
  2019-09-05  9:31 [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression Denis Plotnikov
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature Denis Plotnikov
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 2/3] qcow2: rework the cluster compression routine Denis Plotnikov
@ 2019-09-05  9:31 ` Denis Plotnikov
  2019-09-12  8:46 ` [Qemu-devel] [PATCH v6 0/3] " Denis Plotnikov
  3 siblings, 0 replies; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-05  9:31 UTC (permalink / raw)
  To: armbru, qemu-devel; +Cc: kwolf, vsementsov, den, qemu-block, mreitz

zstd significantly reduces cluster compression time.
It provides better compression performance maintaining
the same level of compression ratio in comparison with
zlib, which, at the moment, has been the only compression
method available.

The performance test results:
Test compresses and decompresses qemu qcow2 image with just
installed rhel-7.6 guest.
Image cluster size: 64K. Image on disk size: 2.2G

The test was conducted with brd disk to reduce the influence
of disk subsystem to the test results.
The results is given in seconds.

compress cmd:
  time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
                  src.img [zlib|zstd]_compressed.img
decompress cmd
  time ./qemu-img convert -O qcow2
                  [zlib|zstd]_compressed.img uncompressed.img

           compression               decompression
         zlib       zstd           zlib         zstd
------------------------------------------------------------
real     65.5       16.3 (-75 %)    1.9          1.6 (-16 %)
user     65.0       15.8            5.3          2.5
sys       3.3        0.2            2.0          2.0

Both ZLIB and ZSTD gave the same compression ratio: 1.57
compressed image size in both cases: 1.4G

Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
---
 block/qcow2-threads.c  | 122 +++++++++++++++++++++++++++++++++++++++++
 block/qcow2.c          |   7 +++
 configure              |  29 ++++++++++
 docs/interop/qcow2.txt |  21 ++++++-
 qapi/block-core.json   |   3 +-
 5 files changed, 179 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
index ebeef9e568..4770bcb646 100644
--- a/block/qcow2-threads.c
+++ b/block/qcow2-threads.c
@@ -28,6 +28,11 @@
 #define ZLIB_CONST
 #include <zlib.h>
 
+#ifdef CONFIG_ZSTD
+#include <zstd.h>
+#include <zstd_errors.h>
+#endif
+
 #include "qcow2.h"
 #include "block/thread-pool.h"
 #include "crypto.h"
@@ -164,6 +169,113 @@ static ssize_t qcow2_zlib_decompress(void *dest, size_t dest_size,
     return ret;
 }
 
+#ifdef CONFIG_ZSTD
+
+#define ZSTD_LEN_BUF 4
+
+/*
+ * qcow2_zstd_compress()
+ *
+ * Compress @src_size bytes of data using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: compressed size on success
+ *          -ENOMEM destination buffer is not enough to store compressed data
+ *          -EIO    on any other error
+ */
+
+static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size,
+                                   const void *src, size_t src_size)
+{
+    size_t ret;
+
+    /*
+     * steal ZSTD_LEN_BUF bytes in the very beginng of the buffer
+     * to store compressed chunk size
+     */
+    char *d_buf = ((char *) dest) + ZSTD_LEN_BUF;
+
+    /*
+     * sanity check that we can store the compressed data length,
+     * and there is some space left for the compressor buffer
+     */
+    if (dest_size <= ZSTD_LEN_BUF) {
+        return -ENOMEM;
+    }
+
+    dest_size -= ZSTD_LEN_BUF;
+
+    ret = ZSTD_compress(d_buf, dest_size, src, src_size, 5);
+
+    if (ZSTD_isError(ret)) {
+        if (ZSTD_getErrorCode(ret) == ZSTD_error_dstSize_tooSmall) {
+            return -ENOMEM;
+        } else {
+            return -EIO;
+        }
+    }
+
+    /* paraniod sanity check that we can store the commpressed size */
+    if (ret > UINT_MAX) {
+        return -ENOMEM;
+    }
+
+    /* store the compressed chunk size in the very beginning of the buffer */
+    stl_be_p(dest, ret);
+
+    return ret + ZSTD_LEN_BUF;
+}
+
+/*
+ * qcow2_zstd_decompress()
+ *
+ * Decompress some data (not more than @src_size bytes) to produce exactly
+ * @dest_size bytes using zstd compression method
+ *
+ * @dest - destination buffer, @dest_size bytes
+ * @src - source buffer, @src_size bytes
+ *
+ * Returns: 0 on success
+ *          -EIO on any error
+ */
+
+static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size,
+                                     const void *src, size_t src_size)
+{
+    /*
+     * zstd decompress wants to know the exact length of the data.
+     * For that purpose, on compression, the length is stored in
+     * the very beginning of the compressed buffer
+     */
+    size_t s_size;
+    const char *s_buf = ((const char *) src) + ZSTD_LEN_BUF;
+
+    /*
+     * sanity check that we can read 4 byte the content length and
+     * and there is some content to decompress
+     */
+    if (src_size <= ZSTD_LEN_BUF) {
+        return -EIO;
+    }
+
+    s_size = ldl_be_p(src);
+
+    /* sanity check that the buffer is big enough to read the content from */
+    if (src_size - ZSTD_LEN_BUF < s_size) {
+        return -EIO;
+    }
+
+    if (ZSTD_isError(
+            ZSTD_decompress(dest, dest_size, s_buf, s_size))) {
+        return -EIO;
+    }
+
+    return 0;
+}
+#endif
+
 static int qcow2_compress_pool_func(void *opaque)
 {
     Qcow2CompressData *data = opaque;
@@ -215,6 +327,11 @@ qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size,
         fn = qcow2_zlib_compress;
         break;
 
+#ifdef CONFIG_ZSTD
+    case QCOW2_COMPRESSION_TYPE_ZSTD:
+        fn = qcow2_zstd_compress;
+        break;
+#endif
     default:
         return -ENOTSUP;
     }
@@ -247,6 +364,11 @@ qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
         fn = qcow2_zlib_decompress;
         break;
 
+#ifdef CONFIG_ZSTD
+    case QCOW2_COMPRESSION_TYPE_ZSTD:
+        fn = qcow2_zstd_decompress;
+        break;
+#endif
     default:
         return -ENOTSUP;
     }
diff --git a/block/qcow2.c b/block/qcow2.c
index 2884b9d9f2..06f346e8cc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1201,6 +1201,9 @@ static int check_compression_type(BDRVQcow2State *s, Error **errp)
 {
     switch (s->compression_type) {
     case QCOW2_COMPRESSION_TYPE_ZLIB:
+#ifdef CONFIG_ZSTD
+    case QCOW2_COMPRESSION_TYPE_ZSTD:
+#endif
         break;
 
     default:
@@ -3293,6 +3296,10 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
         qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
 
         switch (qcow2_opts->compression_type) {
+#ifdef CONFIG_ZSTD
+        case QCOW2_COMPRESSION_TYPE_ZSTD:
+            break;
+#endif
         default:
             error_setg_errno(errp, -EINVAL, "Unknown compression type");
             goto out;
diff --git a/configure b/configure
index 714e7fb6a1..964126c569 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,7 @@ opengl_dmabuf="no"
 cpuid_h="no"
 avx2_opt=""
 zlib="yes"
+zstd=""
 capstone=""
 lzo=""
 snappy=""
@@ -1358,6 +1359,10 @@ for opt do
   ;;
   --disable-lzfse) lzfse="no"
   ;;
+  --enable-zstd) zstd="yes"
+  ;;
+  --disable-zstd) zstd="no"
+  ;;
   --enable-guest-agent) guest_agent="yes"
   ;;
   --disable-guest-agent) guest_agent="no"
@@ -1812,6 +1817,7 @@ disabled with --disable-FEATURE, default is enabled if available:
                   (for reading bzip2-compressed dmg images)
   lzfse           support of lzfse compression library
                   (for reading lzfse-compressed dmg images)
+  zstd            support of zstd compression library
   seccomp         seccomp support
   coroutine-pool  coroutine freelist (better performance)
   glusterfs       GlusterFS backend
@@ -2407,6 +2413,25 @@ EOF
     fi
 fi
 
+#########################################
+# zstd check
+
+if test "$zstd" != "no" ; then
+    cat > $TMPC << EOF
+#include <zstd.h>
+int main(void) { ZSTD_versionNumber(); return 0; }
+EOF
+    if compile_prog "" "-lzstd" ; then
+        LIBS="$LIBS -lzstd"
+        zstd="yes"
+    else
+        if test "$zstd" = "yes"; then
+            feature_not_found "zstd" "Install libzstd-devel"
+        fi
+        zstd="no"
+    fi
+fi
+
 ##########################################
 # libseccomp check
 
@@ -6460,6 +6485,7 @@ echo "lzo support       $lzo"
 echo "snappy support    $snappy"
 echo "bzip2 support     $bzip2"
 echo "lzfse support     $lzfse"
+echo "zstd support      $zstd"
 echo "NUMA host support $numa"
 echo "libxml2           $libxml2"
 echo "tcmalloc support  $tcmalloc"
@@ -7306,6 +7332,9 @@ fi
 if test "$sheepdog" = "yes" ; then
   echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
 fi
+if test "$zstd" = "yes" ; then
+  echo "CONFIG_ZSTD=y" >> $config_host_mak
+fi
 
 if test "$tcg_interpreter" = "yes"; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 59e09caacf..18a2369d34 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -178,8 +178,8 @@ in the description of a field.
                     When the compression type differs from the default
                     the compression type bit (incompatible feature bit 3)
                     must be set.
-                    Available compression type values:
-                        0: zlib <https://www.zlib.net/> (default)
+                    For available compression types see "Compressed Cluster
+                    Layout" section.
 
 Directly after the image header, optional sections called header extensions can
 be stored. Each extension has a structure like the following:
@@ -535,11 +535,28 @@ Compressed Clusters Descriptor (x = 62 - (cluster_bits - 8)):
                     Another compressed cluster may map to the tail of the final
                     sector used by this compressed cluster.
 
+                    The layout of the compressed data depends on the compression
+                    type used for the image (see compressed cluster layout).
+
 If a cluster is unallocated, read requests shall read the data from the backing
 file (except if bit 0 in the Standard Cluster Descriptor is set). If there is
 no backing file or the backing file is smaller than the image, they shall read
 zeros for all parts that are not covered by the backing file.
 
+=== Compressed Cluster Layout ===
+
+The compressed cluster data has a layout depending on the compression
+type used for the image, as follows:
+
+Compressed data layout for the available compression types:
+(x = data_space_length - 1)
+
+    0:  (default)  zlib <http://zlib.net/>:
+            Byte  0 -  x:     the compressed data content
+                              all the space provided used for compressed data
+    1:  zstd <http://github.com/facebook/zstd>:
+            Byte  0 -  3:     the length of compressed data in bytes
+                  4 -  x:     the compressed data content
 
 == Snapshots ==
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2c002ca6a9..9e458d5b40 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4283,11 +4283,12 @@
 # Compression type used in qcow2 image file
 #
 # @zlib:  zlib compression, see <http://zlib.net/>
+# @zstd:  zstd compression, see <http://github.com/facebook/zstd>
 #
 # Since: 4.2
 ##
 { 'enum': 'Qcow2CompressionType',
-  'data': [ 'zlib' ] }
+  'data': [ 'zlib', { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
 
 ##
 # @BlockdevCreateOptionsQcow2:
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature Denis Plotnikov
@ 2019-09-05 13:44   ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-05 13:44 UTC (permalink / raw)
  To: Denis Plotnikov, armbru, qemu-devel
  Cc: kwolf, Denis Lunev, qemu-block, mreitz

05.09.2019 12:31, 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 provides a performance advantage in backup scenarios.
> 
> 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>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>   block/qcow2.c             | 91 +++++++++++++++++++++++++++++++++++++++
>   block/qcow2.h             | 26 ++++++++---
>   docs/interop/qcow2.txt    | 18 +++++++-
>   include/block/block_int.h |  1 +
>   qapi/block-core.json      | 22 +++++++++-
>   5 files changed, 148 insertions(+), 10 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 039bdc2f7e..2884b9d9f2 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1197,6 +1197,32 @@ static int qcow2_update_options(BlockDriverState *bs, QDict *options,
>       return ret;
>   }
>   
> +static int check_compression_type(BDRVQcow2State *s, Error **errp)
> +{
> +    switch (s->compression_type) {
> +    case QCOW2_COMPRESSION_TYPE_ZLIB:
> +        break;
> +
> +    default:
> +        error_setg(errp, "qcow2: unknown compression type: %u",
> +                   s->compression_type);
> +        return -ENOTSUP;
> +    }
> +
> +    /*
> +     * if the compression type differs from QCOW2_COMPRESSION_TYPE_ZLIB
> +     * the incompatible feature flag must be set
> +     */
> +
> +    if (s->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB &&
> +        !(s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION_TYPE)) {
> +            error_setg(errp, "qcow2: Invalid compression type setting");
> +            return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
>   /* Called with s->lock held.  */
>   static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
>                                         int flags, Error **errp)
> @@ -1312,6 +1338,35 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
>       s->compatible_features      = header.compatible_features;
>       s->autoclear_features       = header.autoclear_features;
>   
> +    /*
> +     * Handle compression type
> +     * Older qcow2 images don't contain the compression type header.
> +     * Distinguish them by the header length and use
> +     * the only valid (default) compression type in that case
> +     */
> +    if (header.header_length > offsetof(QCowHeader, compression_type)) {
> +        /* sanity check that we can read a compression type */
> +        size_t min_len = offsetof(QCowHeader, compression_type) +
> +                         sizeof(header.compression_type);
> +        if (header.header_length < min_len) {
> +            error_setg(errp,
> +                       "Could not read compression type, "
> +                       "qcow2 header is too short");
> +            ret = -EINVAL;
> +            goto fail;
> +        }
> +
> +        header.compression_type = be32_to_cpu(header.compression_type);
> +        s->compression_type = header.compression_type;
> +    } else {
> +        s->compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
> +    }
> +
> +    ret = check_compression_type(s, errp);
> +    if (ret) {
> +        goto fail;
> +    }
> +
>       if (s->incompatible_features & ~QCOW2_INCOMPAT_MASK) {
>           void *feature_table = NULL;
>           qcow2_read_extensions(bs, header.header_length, ext_end,
> @@ -2516,6 +2571,12 @@ int qcow2_update_header(BlockDriverState *bs)
>       total_size = bs->total_sectors * BDRV_SECTOR_SIZE;
>       refcount_table_clusters = s->refcount_table_size >> (s->cluster_bits - 3);
>   
> +    ret = check_compression_type(s, NULL);
> +
> +    if (ret) {
> +        goto fail;
> +    }
> +
>       *header = (QCowHeader) {
>           /* Version 2 fields */
>           .magic                  = cpu_to_be32(QCOW_MAGIC),
> @@ -2538,6 +2599,7 @@ int qcow2_update_header(BlockDriverState *bs)
>           .autoclear_features     = cpu_to_be64(s->autoclear_features),
>           .refcount_order         = cpu_to_be32(s->refcount_order),
>           .header_length          = cpu_to_be32(header_length),
> +        .compression_type       = cpu_to_be32(s->compression_type),
>       };
>   
>       /* For older versions, write a shorter header */
> @@ -2635,6 +2697,11 @@ int qcow2_update_header(BlockDriverState *bs)
>                   .bit  = QCOW2_COMPAT_LAZY_REFCOUNTS_BITNR,
>                   .name = "lazy refcounts",
>               },
> +            {
> +                .type = QCOW2_FEAT_TYPE_INCOMPATIBLE,
> +                .bit  = QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR,
> +                .name = "compression type",
> +            },
>           };
>   
>           ret = header_ext_add(buf, QCOW2_EXT_MAGIC_FEATURE_TABLE,
> @@ -3202,6 +3269,7 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>           .refcount_table_offset      = cpu_to_be64(cluster_size),
>           .refcount_table_clusters    = cpu_to_be32(1),
>           .refcount_order             = cpu_to_be32(refcount_order),
> +        .compression_type           = cpu_to_be32(QCOW2_COMPRESSION_TYPE_ZLIB),
>           .header_length              = cpu_to_be32(sizeof(*header)),
>       };
>   
> @@ -3221,6 +3289,21 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
>               cpu_to_be64(QCOW2_AUTOCLEAR_DATA_FILE_RAW);
>       }
>   
> +    if (qcow2_opts->has_compression_type &&
> +        qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
> +
> +        switch (qcow2_opts->compression_type) {
> +        default:
> +            error_setg_errno(errp, -EINVAL, "Unknown compression type");
> +            goto out;
> +        }
> +
> +        header->compression_type = cpu_to_be32(qcow2_opts->compression_type);
> +
> +        header->incompatible_features |=
> +            cpu_to_be64(QCOW2_INCOMPAT_COMPRESSION_TYPE);
> +    }
> +
>       ret = blk_pwrite(blk, 0, header, cluster_size, 0);
>       g_free(header);
>       if (ret < 0) {
> @@ -3402,6 +3485,7 @@ static int coroutine_fn qcow2_co_create_opts(const char *filename, QemuOpts *opt
>           { BLOCK_OPT_ENCRYPT,            BLOCK_OPT_ENCRYPT_FORMAT },
>           { BLOCK_OPT_COMPAT_LEVEL,       "version" },
>           { BLOCK_OPT_DATA_FILE_RAW,      "data-file-raw" },
> +        { BLOCK_OPT_COMPRESSION_TYPE,   "compression-type" },
>           { NULL, NULL },
>       };
>   
> @@ -4598,6 +4682,7 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs,
>               .data_file          = g_strdup(s->image_data_file),
>               .has_data_file_raw  = has_data_file(bs),
>               .data_file_raw      = data_file_is_raw(bs),
> +            .compression_type   = s->compression_type,
>           };
>       } else {
>           /* if this assertion fails, this probably means a new version was
> @@ -5163,6 +5248,12 @@ static QemuOptsList qcow2_create_opts = {
>               .help = "Width of a reference count entry in bits",
>               .def_value_str = "16"
>           },
> +        {
> +            .name = BLOCK_OPT_COMPRESSION_TYPE,
> +            .type = QEMU_OPT_STRING,
> +            .help = "Compression method used for image clusters compression",
> +            .def_value_str = "zlib"
> +        },
>           { /* end of list */ }
>       }
>   };
> diff --git a/block/qcow2.h b/block/qcow2.h
> index fc1b0d3c1e..659e4f852a 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -140,6 +140,7 @@ typedef struct QCowHeader {
>   
>       uint32_t refcount_order;
>       uint32_t header_length;
> +    uint32_t compression_type;
>   } QEMU_PACKED QCowHeader;
>   
>   typedef struct QEMU_PACKED QCowSnapshotHeader {
> @@ -203,16 +204,20 @@ enum {
>   
>   /* Incompatible feature bits */
>   enum {
> -    QCOW2_INCOMPAT_DIRTY_BITNR      = 0,
> -    QCOW2_INCOMPAT_CORRUPT_BITNR    = 1,
> -    QCOW2_INCOMPAT_DATA_FILE_BITNR  = 2,
> -    QCOW2_INCOMPAT_DIRTY            = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
> -    QCOW2_INCOMPAT_CORRUPT          = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
> -    QCOW2_INCOMPAT_DATA_FILE        = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
> +    QCOW2_INCOMPAT_DIRTY_BITNR            = 0,
> +    QCOW2_INCOMPAT_CORRUPT_BITNR          = 1,
> +    QCOW2_INCOMPAT_DATA_FILE_BITNR        = 2,
> +    QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR = 3,
> +    QCOW2_INCOMPAT_DIRTY                  = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
> +    QCOW2_INCOMPAT_CORRUPT                = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
> +    QCOW2_INCOMPAT_DATA_FILE              = 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
> +    QCOW2_INCOMPAT_COMPRESSION_TYPE       =
> +        1 << QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR,
>   
>       QCOW2_INCOMPAT_MASK             = QCOW2_INCOMPAT_DIRTY
>                                       | QCOW2_INCOMPAT_CORRUPT
> -                                    | QCOW2_INCOMPAT_DATA_FILE,
> +                                    | QCOW2_INCOMPAT_DATA_FILE
> +                                    | QCOW2_INCOMPAT_COMPRESSION_TYPE,
>   };
>   
>   /* Compatible feature bits */
> @@ -359,6 +364,13 @@ typedef struct BDRVQcow2State {
>   
>       bool metadata_preallocation_checked;
>       bool metadata_preallocation;
> +    /*
> +     * Compression type used for the image. Default: 0 - ZLIB
> +     * The image compression type is set on image creation.
> +     * The only way to change the compression type is to convert the image
> +     * with the desired compression type set
> +     */
> +    Qcow2CompressionType compression_type;
>   } BDRVQcow2State;
>   
>   typedef struct Qcow2COWRegion {
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index af5711e533..59e09caacf 100644
> --- a/docs/interop/qcow2.txt
> +++ 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 of zlib.
> +                                If the compression type is default the bit should
> +                                be unset.
> +
> +                    Bits 4-63:  Reserved (set to 0)
>   
>            80 -  87:  compatible_features
>                       Bitmask of compatible features. An implementation can
> @@ -165,6 +170,17 @@ 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 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.
> +                    Available compression type values:
> +                        0: zlib <https://www.zlib.net/> (default)
> +
>   Directly after the image header, optional sections called header extensions can
>   be stored. Each extension has a structure like the following:
>   

Hmmm

We have this  above  "incompatible_features" field:

If the version is 3 or higher, the header has the following additional fields.
For version 2, the values are assumed to be zero, unless specified otherwise
in the description of a field.

So, now we actually documented that all v3 images has compression_type field.. But it's not so.
At least it should be reworded, like:

If the version is 3 or higher, the header has the following additional fields, up to the header_length field (inclusive).
For version 2, the values are assumed to be zero, unless specified otherwise
in the description of a field.


And then, after header_length field, we should add something like:

The following fields may present, if header_length is large enough. However, header_length must be aligned to some field
(or should? I think "should" is better, as it was not documented before)

Also, we need to write somewhere, that software which don't know about new fields may use header_length and it is safe. It definitely
means that all new fields we are going to add are compatible by default (old software may ignore it and may rewrite header to be smaller,
and it is safe).

Actually, in this patch, we add not only incompatible extension, we also add compatible header expansion (with incompatible bit unset, but header is larger)...

Then, let's look at possible combinations of header_length and compression type feature

incompat_flag  length          type
0              old             <absent>    # old image. It is should be still valid, so we need to fix spec like I proposed above
0              old < l < new   <?>         # Should we consider it invalid, or old one?
0              new <= l        0           # new (or from future) image with default zlib, compatible. Old software knows where to find qcow2 extensions, as we have valid header len
0              new <= l        t != 0      # broken, invalid image. Software which don't know about compression type will misread it, software which knows will not open
1              old             <absent>    # Hmmm, is it correct? I will be opened after this patch as valid, but it does _not_ correspond to specification..
1              old < l < new   <?>         # Considered as invalid by this patch, but this is not documented
1              new <= l        0           # new (or from future) default zlib, incomaptible for no reason, but still valid
1              new <= l        t != 0      # new (or from future) not zlib, incompatible, valid


> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 3aa1e832a8..4b254802e5 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -58,6 +58,7 @@
>   #define BLOCK_OPT_REFCOUNT_BITS     "refcount_bits"
>   #define BLOCK_OPT_DATA_FILE         "data_file"
>   #define BLOCK_OPT_DATA_FILE_RAW     "data_file_raw"
> +#define BLOCK_OPT_COMPRESSION_TYPE  "compression_type"
>   
>   #define BLOCK_PROBE_BUF_SIZE        512
>   
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 0d43d4f37c..2c002ca6a9 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -78,6 +78,8 @@
>   #
>   # @bitmaps: A list of qcow2 bitmap details (since 4.0)
>   #
> +# @compression-type: the image cluster compression method (since 4.2)
> +#
>   # Since: 1.7
>   ##
>   { 'struct': 'ImageInfoSpecificQCow2',
> @@ -89,7 +91,8 @@
>         '*corrupt': 'bool',
>         'refcount-bits': 'int',
>         '*encrypt': 'ImageInfoSpecificQCow2Encryption',
> -      '*bitmaps': ['Qcow2BitmapInfo']
> +      '*bitmaps': ['Qcow2BitmapInfo'],
> +      'compression-type': 'Qcow2CompressionType'
>     } }
>   
>   ##
> @@ -4274,6 +4277,18 @@
>     'data': [ 'v2', 'v3' ] }
>   
>   
> +##
> +# @Qcow2CompressionType:
> +#
> +# Compression type used in qcow2 image file
> +#
> +# @zlib:  zlib compression, see <http://zlib.net/>
> +#
> +# Since: 4.2
> +##
> +{ 'enum': 'Qcow2CompressionType',
> +  'data': [ 'zlib' ] }
> +
>   ##
>   # @BlockdevCreateOptionsQcow2:
>   #
> @@ -4297,6 +4312,8 @@
>   #                   allowed values: off, falloc, full, metadata)
>   # @lazy-refcounts   True if refcounts may be updated lazily (default: off)
>   # @refcount-bits    Width of reference counts in bits (default: 16)
> +# @compression-type The image cluster compression method
> +#                   (default: zlib, since 4.2)
>   #
>   # Since: 2.12
>   ##
> @@ -4312,7 +4329,8 @@
>               '*cluster-size':    'size',
>               '*preallocation':   'PreallocMode',
>               '*lazy-refcounts':  'bool',
> -            '*refcount-bits':   'int' } }
> +            '*refcount-bits':   'int',
> +            '*compression-type': 'Qcow2CompressionType' } }
>   
>   ##
>   # @BlockdevCreateOptionsQed:
> 


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression
  2019-09-05  9:31 [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression Denis Plotnikov
                   ` (2 preceding siblings ...)
  2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 3/3] qcow2: add zstd cluster compression Denis Plotnikov
@ 2019-09-12  8:46 ` Denis Plotnikov
  2019-09-18 16:29   ` [Qemu-devel] [Ping] " Denis Plotnikov
  3 siblings, 1 reply; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-12  8:46 UTC (permalink / raw)
  To: armbru, qemu-devel
  Cc: kwolf, Vladimir Sementsov-Ogievskiy, Denis Lunev, qemu-block, mreitz

ping!

On 05.09.2019 12:31, Denis Plotnikov wrote:
> v6:
> * fixed zstd compressed length storing/loading [Eric]
> * fixed wording, spec section placement [Eric]
>
> v5:
> * type changed for compression_type at BDRVQcow2State [Kevin]
> * fixed typos, grammar [Kevin]
> * fixed default config zstd setting [Kevin]
>
> v4:
> * remove not feasible switch case [Vladimir]
> * add sanity checks to zstd decompresssion [Vladimir]
> * store zstd compressed length in big endian [Max, Kevin]
>
> v3:
> * relax the compression type setting requirement when
>    the compression type is not zlib [Eric, Kevin]
> * add compression type values to the spec [Eric]
> * fix wording in the spec and descriptions [Eric]
> * fix functions descriptions [Max]
> * fix zstd (de)compression functions flaws [Max]
> * fix zstd related parts of configure file [Max]
> * rebased to v4.1.0-rc5 and chenged the series version aiming to 4.2
>
> v2:
> * relax the compression type setting restriction in the spec
> * fix qcow2 header size checking
> * fix error processing and messaging
> * fix qcow2 image specific info reporting
> * set Qcow2CompressionType zstd config dependant
> * add zstd compressed cluster format description to the spec
>
> v1:
> * extend qcow2 header instead of adding a new incompatible extension header
> specification re-written accordingly
> * enable zstd compression via config
> * fix zstd (de)compression functions
> * fix comments/description
> * fix function naming
>
> ---
> The goal of the patch-set is to enable qcow2 to use zstd compression for
> clusters. ZSTD provides better (de)compression performance than currently
> used ZLIB. Using it will improve perforamnce (reduce compression time)
> when the compressed clusters is used, e.g backup scenarios.
>
> Also, the patch-set extends qcow2 specification by adding compression_type
> feature. The feature enables adding ZSTD and another compression algorithms
> in the future.
>
> Here is some measurements ZSTD vs ZLIB:
>
> The test:
> Test compresses and decompresses qemu qcow2 image with just
> installed rhel-7.6 guest.
> Image cluster size: 64K. Image on disk size: 2.2G
>
> The test was conducted with brd disk to reduce the influence
> of disk subsystem to the test results.
> The results is given in seconds.
>
> compress cmd:
> time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
> src.img [zlib|zstd]_compressed.img
> decompress cmd
> time ./qemu-img convert -O qcow2
> [zlib|zstd]_compressed.img uncompressed.img
>
>
> The results:
> compression decompression
> zlib zstd zlib zstd
> ------------------------------------------------------------
> real 65.5 16.3 (-75 %) 1.9 1.6 (-16 %)
> user 65.0 15.8 5.3 2.5
> sys 3.3 0.2 2.0 2.0
>
> Both ZLIB and ZSTD gave the same compression ratio: ~1.5
> compressed image size in both cases: ~1.4G
>
> Denis Plotnikov (3):
>    qcow2: introduce compression type feature
>    qcow2: rework the cluster compression routine
>    qcow2: add zstd cluster compression
>
>   block/qcow2-threads.c     | 199 +++++++++++++++++++++++++++++++++++---
>   block/qcow2.c             |  98 +++++++++++++++++++
>   block/qcow2.h             |  26 +++--
>   configure                 |  29 ++++++
>   docs/interop/qcow2.txt    |  35 ++++++-
>   include/block/block_int.h |   1 +
>   qapi/block-core.json      |  23 ++++-
>   7 files changed, 387 insertions(+), 24 deletions(-)
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [Ping] [PATCH v6 0/3] qcow2: add zstd cluster compression
  2019-09-12  8:46 ` [Qemu-devel] [PATCH v6 0/3] " Denis Plotnikov
@ 2019-09-18 16:29   ` Denis Plotnikov
  2019-09-20 17:55     ` [Qemu-block] " John Snow
  0 siblings, 1 reply; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-18 16:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, Vladimir Sementsov-Ogievskiy, Denis Lunev, qemu-block,
	armbru, mreitz

On 12.09.2019 11:46, Denis Plotnikov wrote:
> ping!
>
> On 05.09.2019 12:31, Denis Plotnikov wrote:
>> v6:
>> * fixed zstd compressed length storing/loading [Eric]
>> * fixed wording, spec section placement [Eric]
>>
>> v5:
>> * type changed for compression_type at BDRVQcow2State [Kevin]
>> * fixed typos, grammar [Kevin]
>> * fixed default config zstd setting [Kevin]
>>
>> v4:
>> * remove not feasible switch case [Vladimir]
>> * add sanity checks to zstd decompresssion [Vladimir]
>> * store zstd compressed length in big endian [Max, Kevin]
>>
>> v3:
>> * relax the compression type setting requirement when
>>     the compression type is not zlib [Eric, Kevin]
>> * add compression type values to the spec [Eric]
>> * fix wording in the spec and descriptions [Eric]
>> * fix functions descriptions [Max]
>> * fix zstd (de)compression functions flaws [Max]
>> * fix zstd related parts of configure file [Max]
>> * rebased to v4.1.0-rc5 and chenged the series version aiming to 4.2
>>
>> v2:
>> * relax the compression type setting restriction in the spec
>> * fix qcow2 header size checking
>> * fix error processing and messaging
>> * fix qcow2 image specific info reporting
>> * set Qcow2CompressionType zstd config dependant
>> * add zstd compressed cluster format description to the spec
>>
>> v1:
>> * extend qcow2 header instead of adding a new incompatible extension header
>> specification re-written accordingly
>> * enable zstd compression via config
>> * fix zstd (de)compression functions
>> * fix comments/description
>> * fix function naming
>>
>> ---
>> The goal of the patch-set is to enable qcow2 to use zstd compression for
>> clusters. ZSTD provides better (de)compression performance than currently
>> used ZLIB. Using it will improve perforamnce (reduce compression time)
>> when the compressed clusters is used, e.g backup scenarios.
>>
>> Also, the patch-set extends qcow2 specification by adding compression_type
>> feature. The feature enables adding ZSTD and another compression algorithms
>> in the future.
>>
>> Here is some measurements ZSTD vs ZLIB:
>>
>> The test:
>> Test compresses and decompresses qemu qcow2 image with just
>> installed rhel-7.6 guest.
>> Image cluster size: 64K. Image on disk size: 2.2G
>>
>> The test was conducted with brd disk to reduce the influence
>> of disk subsystem to the test results.
>> The results is given in seconds.
>>
>> compress cmd:
>> time ./qemu-img convert -O qcow2 -c -o compression_type=[zlib|zstd]
>> src.img [zlib|zstd]_compressed.img
>> decompress cmd
>> time ./qemu-img convert -O qcow2
>> [zlib|zstd]_compressed.img uncompressed.img
>>
>>
>> The results:
>> compression decompression
>> zlib zstd zlib zstd
>> ------------------------------------------------------------
>> real 65.5 16.3 (-75 %) 1.9 1.6 (-16 %)
>> user 65.0 15.8 5.3 2.5
>> sys 3.3 0.2 2.0 2.0
>>
>> Both ZLIB and ZSTD gave the same compression ratio: ~1.5
>> compressed image size in both cases: ~1.4G
>>
>> Denis Plotnikov (3):
>>     qcow2: introduce compression type feature
>>     qcow2: rework the cluster compression routine
>>     qcow2: add zstd cluster compression
>>
>>    block/qcow2-threads.c     | 199 +++++++++++++++++++++++++++++++++++---
>>    block/qcow2.c             |  98 +++++++++++++++++++
>>    block/qcow2.h             |  26 +++--
>>    configure                 |  29 ++++++
>>    docs/interop/qcow2.txt    |  35 ++++++-
>>    include/block/block_int.h |   1 +
>>    qapi/block-core.json      |  23 ++++-
>>    7 files changed, 387 insertions(+), 24 deletions(-)
>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-block] [Ping] [PATCH v6 0/3] qcow2: add zstd cluster compression
  2019-09-18 16:29   ` [Qemu-devel] [Ping] " Denis Plotnikov
@ 2019-09-20 17:55     ` John Snow
  2019-09-22 16:17       ` Denis Plotnikov
  0 siblings, 1 reply; 9+ messages in thread
From: John Snow @ 2019-09-20 17:55 UTC (permalink / raw)
  To: Denis Plotnikov, qemu-devel
  Cc: kwolf, Vladimir Sementsov-Ogievskiy, Denis Lunev, qemu-block,
	armbru, mreitz



On 9/18/19 12:29 PM, Denis Plotnikov wrote:
> On 12.09.2019 11:46, Denis Plotnikov wrote:
>> ping!
>>

https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00943.html

I was under the impression you were addressing feedback from Vladimir.

--js


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-block] [Ping] [PATCH v6 0/3] qcow2: add zstd cluster compression
  2019-09-20 17:55     ` [Qemu-block] " John Snow
@ 2019-09-22 16:17       ` Denis Plotnikov
  0 siblings, 0 replies; 9+ messages in thread
From: Denis Plotnikov @ 2019-09-22 16:17 UTC (permalink / raw)
  To: John Snow, qemu-devel
  Cc: kwolf, Vladimir Sementsov-Ogievskiy, Denis Lunev, qemu-block,
	armbru, mreitz

On 20.09.2019 20:55, John Snow wrote:
>
> On 9/18/19 12:29 PM, Denis Plotnikov wrote:
>> On 12.09.2019 11:46, Denis Plotnikov wrote:
>>> ping!
>>>
> https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg00943.html
>
> I was under the impression you were addressing feedback from Vladimir.
>
> --js

I don't really know what to do with what Vladimir pointed out so I would 
like to get a feedback from Kevin and Eric about this.

Denis


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-09-22 16:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05  9:31 [Qemu-devel] [PATCH v6 0/3] qcow2: add zstd cluster compression Denis Plotnikov
2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 1/3] qcow2: introduce compression type feature Denis Plotnikov
2019-09-05 13:44   ` Vladimir Sementsov-Ogievskiy
2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 2/3] qcow2: rework the cluster compression routine Denis Plotnikov
2019-09-05  9:31 ` [Qemu-devel] [PATCH v6 3/3] qcow2: add zstd cluster compression Denis Plotnikov
2019-09-12  8:46 ` [Qemu-devel] [PATCH v6 0/3] " Denis Plotnikov
2019-09-18 16:29   ` [Qemu-devel] [Ping] " Denis Plotnikov
2019-09-20 17:55     ` [Qemu-block] " John Snow
2019-09-22 16:17       ` Denis Plotnikov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.