| Message ID | 20190904152915.30755-4-dplotnikov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: add zstd cluster compression | expand |
On 9/4/19 10:29 AM, Denis Plotnikov wrote: > 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 > > > Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> > --- > +static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size, > + const void *src, size_t src_size) > +{ > + ssize_t ret; > + uint32_t *c_size = dest; Potentially unaligned pointer... > + /* steal some bytes to store compressed chunk size */ > + char *d_buf = ((char *) dest) + sizeof(*c_size); > + > + /* sanity check that we can store the compressed data length */ > + if (dest_size < sizeof(*c_size)) { > + return -ENOMEM; > + } > + > + dest_size -= sizeof(*c_size); > + > + 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; > + } > + } > + > + /* store the compressed chunk size in the very beginning of the buffer */ > + *c_size = cpu_to_be32(ret); ...and you are storing into it. You are using the wrong conversion function; you want stl_be_p(dest, ret) or similar. > +static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size, > + const void *src, size_t src_size) > +{ > + ssize_t ret; > + /* > + * zstd decompress wants to know the exact length of the data > + * for that purpose, on the compression the length is stored in s/data for/data. For/ s/on the compression/on compression,/ > + * the very beginning of the compressed buffer > + */ > + uint32_t s_size; > + const char *s_buf = ((const char *) src) + sizeof(s_size); > + > + /* sanity check that we can read the content length */ > + if (src_size < sizeof(s_size)) { Should this use <=? After all, I seriously doubt you can get a 0-byte compression stream. > + return -EIO; > + } > + > + s_size = be32_to_cpu(*(const uint32_t *) src); As written, this looks like you may be dereferencing an unaligned pointer. It so happens that be32_to_cpu() applies & to your * to get back at the raw pointer, and then is careful to handle unaligned pointers, so it works; but it would look a lot nicer as merely: s_size = be32_to_cpu(src); > + > + /* sanity check that the buffer is big enough to read the content */ > + if (src_size - sizeof(s_size) < s_size) { Why < and not !=? As written, you are silently ignoring trailing garbage, instead of treating it as a client that did not write the data correctly. > +++ b/docs/interop/qcow2.txt > @@ -181,6 +181,7 @@ in the description of a field. > must be set. > Available compression type values: > 0: zlib <https://www.zlib.net/> (default) > + 1: zstd <http://github.com/facebook/zstd> At this point, this listing is almost redundant with the more-detailed header below. Maybe it is worth just forward referencing that section for a listing of valid values, and then mentioning the values 0 and 1 in that section? > > Directly after the image header, optional sections called header extensions can > be stored. Each extension has a structure like the following: > @@ -536,6 +537,9 @@ 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 > @@ -788,3 +792,19 @@ In the image file the 'enabled' state is reflected by the 'auto' flag. If this > flag is set, the software must consider the bitmap as 'enabled' and start > tracking virtual disk changes to this bitmap from the first write to the > virtual disk. If this flag is not set then the bitmap is disabled. > + > +=== Compressed cluster layout === As written, you have made this a child to '== Bitmaps ==' (and sibling to '=== Dirty tracking bitmaps ==='); that feels wrong. I would place this subsection belong under '== Cluster mapping ==' right after the 'Compressed Clusters Descriptor'. > + > +The compressed cluster data may have a different layout depending on the > +compression type used for the image, and store specific data for the particular > +compression type. > + > +Compressed data layout for the available compression types: Wordy; maybe: The compressed cluster data has a layout depending on the compression type used for the image, as follows: > +(x = data_space_length - 1) > + > + zlib <http://zlib.net/>: > + Byte 0 - x: the compressed data content > + all the space provided used for compressed data Worth a mention that this is compression type 0? > + zstd <http://github.com/facebook/zstd>: > + Byte 0 - 3: the length of compressed data in bytes > + 4 - x: the compressed data content Worth a mention that this is compression type 1? > 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: >
On 04.09.2019 19:07, Eric Blake wrote: > On 9/4/19 10:29 AM, Denis Plotnikov wrote: >> 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 >> >> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> >> --- >> +static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size, >> + const void *src, size_t src_size) >> +{ >> + ssize_t ret; >> + uint32_t *c_size = dest; > Potentially unaligned pointer... > >> + /* steal some bytes to store compressed chunk size */ >> + char *d_buf = ((char *) dest) + sizeof(*c_size); >> + >> + /* sanity check that we can store the compressed data length */ >> + if (dest_size < sizeof(*c_size)) { >> + return -ENOMEM; >> + } >> + >> + dest_size -= sizeof(*c_size); >> + >> + 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; >> + } >> + } >> + >> + /* store the compressed chunk size in the very beginning of the buffer */ >> + *c_size = cpu_to_be32(ret); > ...and you are storing into it. You are using the wrong conversion > function; you want stl_be_p(dest, ret) or similar. > >> +static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size, >> + const void *src, size_t src_size) >> +{ >> + ssize_t ret; >> + /* >> + * zstd decompress wants to know the exact length of the data >> + * for that purpose, on the compression the length is stored in > s/data for/data. For/ > s/on the compression/on compression,/ > >> + * the very beginning of the compressed buffer >> + */ >> + uint32_t s_size; >> + const char *s_buf = ((const char *) src) + sizeof(s_size); >> + >> + /* sanity check that we can read the content length */ >> + if (src_size < sizeof(s_size)) { > Should this use <=? After all, I seriously doubt you can get a 0-byte > compression stream. > >> + return -EIO; >> + } >> + >> + s_size = be32_to_cpu(*(const uint32_t *) src); > As written, this looks like you may be dereferencing an unaligned > pointer. It so happens that be32_to_cpu() applies & to your * to get > back at the raw pointer, and then is careful to handle unaligned > pointers, so it works; but it would look a lot nicer as merely: > > s_size = be32_to_cpu(src); yes, but I can't use be32_to_cpu(*src) since src is a void pointer Denis > >> + >> + /* sanity check that the buffer is big enough to read the content */ >> + if (src_size - sizeof(s_size) < s_size) { > Why < and not !=? As written, you are silently ignoring trailing > garbage, instead of treating it as a client that did not write the data > correctly. > >> +++ b/docs/interop/qcow2.txt >> @@ -181,6 +181,7 @@ in the description of a field. >> must be set. >> Available compression type values: >> 0: zlib <https://www.zlib.net/> (default) >> + 1: zstd <http://github.com/facebook/zstd> > At this point, this listing is almost redundant with the more-detailed > header below. Maybe it is worth just forward referencing that section > for a listing of valid values, and then mentioning the values 0 and 1 in > that section? > >> >> Directly after the image header, optional sections called header extensions can >> be stored. Each extension has a structure like the following: >> @@ -536,6 +537,9 @@ 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 >> @@ -788,3 +792,19 @@ In the image file the 'enabled' state is reflected by the 'auto' flag. If this >> flag is set, the software must consider the bitmap as 'enabled' and start >> tracking virtual disk changes to this bitmap from the first write to the >> virtual disk. If this flag is not set then the bitmap is disabled. >> + >> +=== Compressed cluster layout === > As written, you have made this a child to '== Bitmaps ==' (and sibling > to '=== Dirty tracking bitmaps ==='); that feels wrong. I would place > this subsection belong under '== Cluster mapping ==' right after the > 'Compressed Clusters Descriptor'. > >> + >> +The compressed cluster data may have a different layout depending on the >> +compression type used for the image, and store specific data for the particular >> +compression type. >> + >> +Compressed data layout for the available compression types: > Wordy; maybe: > > The compressed cluster data has a layout depending on the compression > type used for the image, as follows: > >> +(x = data_space_length - 1) >> + >> + zlib <http://zlib.net/>: >> + Byte 0 - x: the compressed data content >> + all the space provided used for compressed data > Worth a mention that this is compression type 0? > >> + zstd <http://github.com/facebook/zstd>: >> + Byte 0 - 3: the length of compressed data in bytes >> + 4 - x: the compressed data content > Worth a mention that this is compression type 1? > >> 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: >>
On 04.09.2019 19:07, Eric Blake wrote: > On 9/4/19 10:29 AM, Denis Plotnikov wrote: >> 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 >> >> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> >> --- >> +static ssize_t qcow2_zstd_compress(void *dest, size_t dest_size, >> + const void *src, size_t src_size) >> +{ >> + ssize_t ret; >> + uint32_t *c_size = dest; > Potentially unaligned pointer... > >> + /* steal some bytes to store compressed chunk size */ >> + char *d_buf = ((char *) dest) + sizeof(*c_size); >> + >> + /* sanity check that we can store the compressed data length */ >> + if (dest_size < sizeof(*c_size)) { >> + return -ENOMEM; >> + } >> + >> + dest_size -= sizeof(*c_size); >> + >> + 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; >> + } >> + } >> + >> + /* store the compressed chunk size in the very beginning of the buffer */ >> + *c_size = cpu_to_be32(ret); > ...and you are storing into it. You are using the wrong conversion > function; you want stl_be_p(dest, ret) or similar. > >> +static ssize_t qcow2_zstd_decompress(void *dest, size_t dest_size, >> + const void *src, size_t src_size) >> +{ >> + ssize_t ret; >> + /* >> + * zstd decompress wants to know the exact length of the data >> + * for that purpose, on the compression the length is stored in > s/data for/data. For/ > s/on the compression/on compression,/ > >> + * the very beginning of the compressed buffer >> + */ >> + uint32_t s_size; >> + const char *s_buf = ((const char *) src) + sizeof(s_size); >> + >> + /* sanity check that we can read the content length */ >> + if (src_size < sizeof(s_size)) { > Should this use <=? After all, I seriously doubt you can get a 0-byte > compression stream. > >> + return -EIO; >> + } >> + >> + s_size = be32_to_cpu(*(const uint32_t *) src); > As written, this looks like you may be dereferencing an unaligned > pointer. It so happens that be32_to_cpu() applies & to your * to get > back at the raw pointer, and then is careful to handle unaligned > pointers, so it works; but it would look a lot nicer as merely: > > s_size = be32_to_cpu(src); > >> + >> + /* sanity check that the buffer is big enough to read the content */ >> + if (src_size - sizeof(s_size) < s_size) { > Why < and not !=? As written, you are silently ignoring trailing > garbage, instead of treating it as a client that did not write the data > correctly. but src_size (512) might be bigger than the content of the compressed data, don't it? Denis > >> +++ b/docs/interop/qcow2.txt >> @@ -181,6 +181,7 @@ in the description of a field. >> must be set. >> Available compression type values: >> 0: zlib <https://www.zlib.net/> (default) >> + 1: zstd <http://github.com/facebook/zstd> > At this point, this listing is almost redundant with the more-detailed > header below. Maybe it is worth just forward referencing that section > for a listing of valid values, and then mentioning the values 0 and 1 in > that section? > >> >> Directly after the image header, optional sections called header extensions can >> be stored. Each extension has a structure like the following: >> @@ -536,6 +537,9 @@ 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 >> @@ -788,3 +792,19 @@ In the image file the 'enabled' state is reflected by the 'auto' flag. If this >> flag is set, the software must consider the bitmap as 'enabled' and start >> tracking virtual disk changes to this bitmap from the first write to the >> virtual disk. If this flag is not set then the bitmap is disabled. >> + >> +=== Compressed cluster layout === > As written, you have made this a child to '== Bitmaps ==' (and sibling > to '=== Dirty tracking bitmaps ==='); that feels wrong. I would place > this subsection belong under '== Cluster mapping ==' right after the > 'Compressed Clusters Descriptor'. > >> + >> +The compressed cluster data may have a different layout depending on the >> +compression type used for the image, and store specific data for the particular >> +compression type. >> + >> +Compressed data layout for the available compression types: > Wordy; maybe: > > The compressed cluster data has a layout depending on the compression > type used for the image, as follows: > >> +(x = data_space_length - 1) >> + >> + zlib <http://zlib.net/>: >> + Byte 0 - x: the compressed data content >> + all the space provided used for compressed data > Worth a mention that this is compression type 0? > >> + zstd <http://github.com/facebook/zstd>: >> + Byte 0 - 3: the length of compressed data in bytes >> + 4 - x: the compressed data content > Worth a mention that this is compression type 1? > >> 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: >>
On 9/5/19 2:44 AM, Denis Plotnikov wrote: >>> + >>> + s_size = be32_to_cpu(*(const uint32_t *) src); >> As written, this looks like you may be dereferencing an unaligned >> pointer. It so happens that be32_to_cpu() applies & to your * to get >> back at the raw pointer, and then is careful to handle unaligned >> pointers, so it works; but it would look a lot nicer as merely: >> >> s_size = be32_to_cpu(src); > > yes, but I can't use be32_to_cpu(*src) since src is a void pointer Then we need the correct ld*_p function; sorry for leading you down the wrong path. Looks like the right one is: s_size = ldl_be_p(src) (include/qemu/bswap.h has some good comments, but you have to know they exist...)
On 05.09.2019 17:31, Eric Blake wrote: > On 9/5/19 2:44 AM, Denis Plotnikov wrote: > > >>>> + >>>> + s_size = be32_to_cpu(*(const uint32_t *) src); >>> As written, this looks like you may be dereferencing an unaligned >>> pointer. It so happens that be32_to_cpu() applies & to your * to get >>> back at the raw pointer, and then is careful to handle unaligned >>> pointers, so it works; but it would look a lot nicer as merely: >>> >>> s_size = be32_to_cpu(src); >> yes, but I can't use be32_to_cpu(*src) since src is a void pointer > Then we need the correct ld*_p function; sorry for leading you down the > wrong path. Looks like the right one is: > > s_size = ldl_be_p(src) > > (include/qemu/bswap.h has some good comments, but you have to know they > exist...) No problem, that happens. By the way, I've already sent the series using ldl_be_p Thanks! Denis >
diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c index ebeef9e568..b5aad202bf 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,98 @@ static ssize_t qcow2_zlib_decompress(void *dest, size_t dest_size, return ret; } +#ifdef CONFIG_ZSTD +/* + * 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) +{ + ssize_t ret; + uint32_t *c_size = dest; + /* steal some bytes to store compressed chunk size */ + char *d_buf = ((char *) dest) + sizeof(*c_size); + + /* sanity check that we can store the compressed data length */ + if (dest_size < sizeof(*c_size)) { + return -ENOMEM; + } + + dest_size -= sizeof(*c_size); + + 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; + } + } + + /* store the compressed chunk size in the very beginning of the buffer */ + *c_size = cpu_to_be32(ret); + + return ret + sizeof(*c_size); +} + +/* + * 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) +{ + ssize_t ret; + /* + * zstd decompress wants to know the exact length of the data + * for that purpose, on the compression the length is stored in + * the very beginning of the compressed buffer + */ + uint32_t s_size; + const char *s_buf = ((const char *) src) + sizeof(s_size); + + /* sanity check that we can read the content length */ + if (src_size < sizeof(s_size)) { + return -EIO; + } + + s_size = be32_to_cpu(*(const uint32_t *) src); + + /* sanity check that the buffer is big enough to read the content */ + if (src_size - sizeof(s_size) < s_size) { + return -EIO; + } + + ret = ZSTD_decompress(dest, dest_size, s_buf, s_size); + + if (ZSTD_isError(ret)) { + return -EIO; + } + + return 0; +} +#endif + static int qcow2_compress_pool_func(void *opaque) { Qcow2CompressData *data = opaque; @@ -215,6 +312,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 +349,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 e1be8bd5c3..b975a34687 100644 --- a/docs/interop/qcow2.txt +++ b/docs/interop/qcow2.txt @@ -181,6 +181,7 @@ in the description of a field. must be set. Available compression type values: 0: zlib <https://www.zlib.net/> (default) + 1: zstd <http://github.com/facebook/zstd> Directly after the image header, optional sections called header extensions can be stored. Each extension has a structure like the following: @@ -536,6 +537,9 @@ 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 @@ -788,3 +792,19 @@ In the image file the 'enabled' state is reflected by the 'auto' flag. If this flag is set, the software must consider the bitmap as 'enabled' and start tracking virtual disk changes to this bitmap from the first write to the virtual disk. If this flag is not set then the bitmap is disabled. + +=== Compressed cluster layout === + +The compressed cluster data may have a different layout depending on the +compression type used for the image, and store specific data for the particular +compression type. + +Compressed data layout for the available compression types: +(x = data_space_length - 1) + + zlib <http://zlib.net/>: + Byte 0 - x: the compressed data content + all the space provided used for compressed data + zstd <http://github.com/facebook/zstd>: + Byte 0 - 3: the length of compressed data in bytes + 4 - x: the compressed data content 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:
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 | 107 +++++++++++++++++++++++++++++++++++++++++ block/qcow2.c | 7 +++ configure | 29 +++++++++++ docs/interop/qcow2.txt | 20 ++++++++ qapi/block-core.json | 3 +- 5 files changed, 165 insertions(+), 1 deletion(-)