* [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
@ 2021-06-17 19:51 Vladimir Sementsov-Ogievskiy
2021-06-19 14:13 ` Vladimir Sementsov-Ogievskiy
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-06-17 19:51 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, den, vsementsov
Recently we added support of zstd to qcow2 format, as zstd seems to be
better than zlib in general, and which is important (as qcow2
compression used mostly for backups) compressed writes are faster with
zstd.
Let's add a build option to use zstd by default.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
Hi all! We want to use zstd as a default compression type for newly
created qcow2 images in upcoming Virtuozzo 8.
I am not sure, how much the community interested in such option,
probably I should just keep a downstream-only patch changing the
default.
But if you like it, I'd be happy to only set new config option in our
qemu build instead of maintaining extra downstream-only patch :)
So, it's an RFC. I also can split the patch so that refactoring of
qcow2_co_create() go in a separate preparation patch.
Another RFC question, shouldn't we move to zstd by default in upstream
too?
configure | 10 +++++++++-
meson.build | 4 ++++
block/qcow2.c | 32 +++++++++++++++++++++++++-------
meson_options.txt | 2 ++
4 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/configure b/configure
index debd50c085..b19af43525 100755
--- a/configure
+++ b/configure
@@ -385,6 +385,7 @@ snappy="auto"
bzip2="auto"
lzfse="auto"
zstd="auto"
+qcow2_zstd_default="no"
guest_agent="$default_feature"
guest_agent_with_vss="no"
guest_agent_ntddscsi="no"
@@ -1318,6 +1319,10 @@ for opt do
;;
--enable-zstd) zstd="enabled"
;;
+ --disable-qcow2-zstd-default) qcow2_zstd_default="disabled"
+ ;;
+ --enable-qcow2-zstd-default) qcow2_zstd_default="enabled"
+ ;;
--enable-guest-agent) guest_agent="yes"
;;
--disable-guest-agent) guest_agent="no"
@@ -1903,6 +1908,8 @@ disabled with --disable-FEATURE, default is enabled if available
(for reading lzfse-compressed dmg images)
zstd support for zstd compression library
(for migration compression and qcow2 cluster compression)
+ qcow2-zstd-default Use zstd by default for qcow2 image creation
+ (requires zstd enabled)
seccomp seccomp support
coroutine-pool coroutine freelist (better performance)
glusterfs GlusterFS backend
@@ -6424,7 +6431,8 @@ if test "$skip_meson" = no; then
-Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
-Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
-Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse \
- -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
+ -Dzstd=$zstd -Dqcow2_zstd_default=$qcow2_zstd_default \
+ -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
-Dattr=$attr -Ddefault_devices=$default_devices \
-Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
-Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
diff --git a/meson.build b/meson.build
index d8a92666fb..3d65b6c46b 100644
--- a/meson.build
+++ b/meson.build
@@ -484,6 +484,9 @@ if not get_option('zstd').auto() or have_block
required: get_option('zstd'),
method: 'pkg-config', kwargs: static_kwargs)
endif
+if not zstd.found() and get_option('qcow2_zstd_default').enabled()
+ error('--enable-qcow2-zstd-default: Cannot use zstd by default without enabling zstd')
+endif
gbm = not_found
if 'CONFIG_GBM' in config_host
gbm = declare_dependency(compile_args: config_host['GBM_CFLAGS'].split(),
@@ -1168,6 +1171,7 @@ config_host_data.set('CONFIG_GETTID', has_gettid)
config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
config_host_data.set('CONFIG_STATX', has_statx)
config_host_data.set('CONFIG_ZSTD', zstd.found())
+config_host_data.set('CONFIG_QCOW2_ZSTD_DEFAULT', get_option('qcow2_zstd_default').enabled())
config_host_data.set('CONFIG_FUSE', fuse.found())
config_host_data.set('CONFIG_FUSE_LSEEK', fuse_lseek.found())
config_host_data.set('CONFIG_X11', x11.found())
diff --git a/block/qcow2.c b/block/qcow2.c
index ee4530cdbd..06bfbbf7b8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3540,17 +3540,36 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
}
}
- if (qcow2_opts->has_compression_type &&
- qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
-
- ret = -EINVAL;
-
- if (version < 3) {
+ if (version < 3) {
+ if (qcow2_opts->has_compression_type &&
+ qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB)
+ {
+ ret = -EINVAL;
error_setg(errp, "Non-zlib compression type is only supported with "
"compatibility level 1.1 and above (use version=v3 or "
"greater)");
goto out;
}
+ } else {
+ if (qcow2_opts->has_compression_type) {
+ compression_type = qcow2_opts->compression_type;
+#ifdef CONFIG_QCOW2_ZSTD_DEFAULT
+ } else {
+ compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
+#endif
+ }
+
+#ifndef CONFIG_ZSTD
+ assert(compression_type == QCOW2_COMPRESSION_TYPE_ZLIB);
+#endif
+ }
+
+ if (qcow2_opts->has_compression_type &&
+ qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
+
+ ret = -EINVAL;
+
+ compression_type = qcow2_opts->compression_type;
switch (qcow2_opts->compression_type) {
#ifdef CONFIG_ZSTD
@@ -3562,7 +3581,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
goto out;
}
- compression_type = qcow2_opts->compression_type;
}
/* Create BlockBackend to write to the image */
diff --git a/meson_options.txt b/meson_options.txt
index 3d304cac96..8af9bb97f5 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -108,6 +108,8 @@ option('xkbcommon', type : 'feature', value : 'auto',
description: 'xkbcommon support')
option('zstd', type : 'feature', value : 'auto',
description: 'zstd compression support')
+option('qcow2_zstd_default', type : 'feature', value : 'disabled',
+ description: 'Use zstd compression type for qcow2 image creation by default')
option('fuse', type: 'feature', value: 'auto',
description: 'FUSE block device export')
option('fuse_lseek', type : 'feature', value : 'auto',
--
2.29.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
2021-06-17 19:51 [PATCH RFC] meson: add option to use zstd for qcow2 compression by default Vladimir Sementsov-Ogievskiy
@ 2021-06-19 14:13 ` Vladimir Sementsov-Ogievskiy
2021-06-21 8:22 ` Paolo Bonzini
2021-07-01 9:54 ` Vladimir Sementsov-Ogievskiy
2 siblings, 0 replies; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-06-19 14:13 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, den
17.06.2021 22:51, Vladimir Sementsov-Ogievskiy wrote:
> Recently we added support of zstd to qcow2 format, as zstd seems to be
> better than zlib in general, and which is important (as qcow2
> compression used mostly for backups) compressed writes are faster with
> zstd.
>
> Let's add a build option to use zstd by default.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>
> Hi all! We want to use zstd as a default compression type for newly
> created qcow2 images in upcoming Virtuozzo 8.
>
> I am not sure, how much the community interested in such option,
> probably I should just keep a downstream-only patch changing the
> default.
>
> But if you like it, I'd be happy to only set new config option in our
> qemu build instead of maintaining extra downstream-only patch :)
>
> So, it's an RFC. I also can split the patch so that refactoring of
> qcow2_co_create() go in a separate preparation patch.
>
> Another RFC question, shouldn't we move to zstd by default in upstream
> too?
>
> configure | 10 +++++++++-
> meson.build | 4 ++++
> block/qcow2.c | 32 +++++++++++++++++++++++++-------
> meson_options.txt | 2 ++
> 4 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/configure b/configure
> index debd50c085..b19af43525 100755
> --- a/configure
> +++ b/configure
> @@ -385,6 +385,7 @@ snappy="auto"
> bzip2="auto"
> lzfse="auto"
> zstd="auto"
> +qcow2_zstd_default="no"
s/no/disabled/
> guest_agent="$default_feature"
> guest_agent_with_vss="no"
> guest_agent_ntddscsi="no"
> @@ -1318,6 +1319,10 @@ for opt do
> ;;
> --enable-zstd) zstd="enabled"
> ;;
> + --disable-qcow2-zstd-default) qcow2_zstd_default="disabled"
> + ;;
> + --enable-qcow2-zstd-default) qcow2_zstd_default="enabled"
> + ;;
> --enable-guest-agent) guest_agent="yes"
> ;;
> --disable-guest-agent) guest_agent="no"
> @@ -1903,6 +1908,8 @@ disabled with --disable-FEATURE, default is enabled if available
> (for reading lzfse-compressed dmg images)
> zstd support for zstd compression library
> (for migration compression and qcow2 cluster compression)
> + qcow2-zstd-default Use zstd by default for qcow2 image creation
> + (requires zstd enabled)
> seccomp seccomp support
> coroutine-pool coroutine freelist (better performance)
> glusterfs GlusterFS backend
> @@ -6424,7 +6431,8 @@ if test "$skip_meson" = no; then
> -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
> -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
> -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse \
> - -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
> + -Dzstd=$zstd -Dqcow2_zstd_default=$qcow2_zstd_default \
> + -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
> -Dattr=$attr -Ddefault_devices=$default_devices \
> -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
> -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
> diff --git a/meson.build b/meson.build
> index d8a92666fb..3d65b6c46b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -484,6 +484,9 @@ if not get_option('zstd').auto() or have_block
> required: get_option('zstd'),
> method: 'pkg-config', kwargs: static_kwargs)
> endif
> +if not zstd.found() and get_option('qcow2_zstd_default').enabled()
> + error('--enable-qcow2-zstd-default: Cannot use zstd by default without enabling zstd')
> +endif
> gbm = not_found
> if 'CONFIG_GBM' in config_host
> gbm = declare_dependency(compile_args: config_host['GBM_CFLAGS'].split(),
> @@ -1168,6 +1171,7 @@ config_host_data.set('CONFIG_GETTID', has_gettid)
> config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
> config_host_data.set('CONFIG_STATX', has_statx)
> config_host_data.set('CONFIG_ZSTD', zstd.found())
> +config_host_data.set('CONFIG_QCOW2_ZSTD_DEFAULT', get_option('qcow2_zstd_default').enabled())
> config_host_data.set('CONFIG_FUSE', fuse.found())
> config_host_data.set('CONFIG_FUSE_LSEEK', fuse_lseek.found())
> config_host_data.set('CONFIG_X11', x11.found())
> diff --git a/block/qcow2.c b/block/qcow2.c
> index ee4530cdbd..06bfbbf7b8 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3540,17 +3540,36 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> }
> }
>
> - if (qcow2_opts->has_compression_type &&
> - qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
> -
> - ret = -EINVAL;
> -
> - if (version < 3) {
> + if (version < 3) {
> + if (qcow2_opts->has_compression_type &&
> + qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB)
> + {
> + ret = -EINVAL;
> error_setg(errp, "Non-zlib compression type is only supported with "
> "compatibility level 1.1 and above (use version=v3 or "
> "greater)");
> goto out;
> }
> + } else {
> + if (qcow2_opts->has_compression_type) {
> + compression_type = qcow2_opts->compression_type;
> +#ifdef CONFIG_QCOW2_ZSTD_DEFAULT
> + } else {
> + compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
> +#endif
> + }
> +
> +#ifndef CONFIG_ZSTD
> + assert(compression_type == QCOW2_COMPRESSION_TYPE_ZLIB);
> +#endif
> + }
> +
> + if (qcow2_opts->has_compression_type &&
> + qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
> +
> + ret = -EINVAL;
> +
> + compression_type = qcow2_opts->compression_type;
>
> switch (qcow2_opts->compression_type) {
> #ifdef CONFIG_ZSTD
> @@ -3562,7 +3581,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> goto out;
> }
>
> - compression_type = qcow2_opts->compression_type;
> }
>
> /* Create BlockBackend to write to the image */
> diff --git a/meson_options.txt b/meson_options.txt
> index 3d304cac96..8af9bb97f5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -108,6 +108,8 @@ option('xkbcommon', type : 'feature', value : 'auto',
> description: 'xkbcommon support')
> option('zstd', type : 'feature', value : 'auto',
> description: 'zstd compression support')
> +option('qcow2_zstd_default', type : 'feature', value : 'disabled',
> + description: 'Use zstd compression type for qcow2 image creation by default')
> option('fuse', type: 'feature', value: 'auto',
> description: 'FUSE block device export')
> option('fuse_lseek', type : 'feature', value : 'auto',
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
2021-06-17 19:51 [PATCH RFC] meson: add option to use zstd for qcow2 compression by default Vladimir Sementsov-Ogievskiy
2021-06-19 14:13 ` Vladimir Sementsov-Ogievskiy
@ 2021-06-21 8:22 ` Paolo Bonzini
2021-06-21 15:59 ` Kevin Wolf
2021-07-01 9:54 ` Vladimir Sementsov-Ogievskiy
2 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2021-06-21 8:22 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, den, qemu-devel, mreitz
On 17/06/21 21:51, Vladimir Sementsov-Ogievskiy wrote:
> So, it's an RFC. I also can split the patch so that refactoring of
> qcow2_co_create() go in a separate preparation patch.
>
> Another RFC question, shouldn't we move to zstd by default in upstream
> too?
I think backwards-incompatible changes in the past were not handled with
build options, but that can be changed.
However I would prefer to have an option like
--with-qcow2-compression={zstd,zlib}. Meson supports multiple-choice
options, they don't have to use enabled/disabled or (if boolean) true/false.
Regarding changing the default, that would make images unreadable to
QEMU 5.0 and earlier versions. Does that apply to images that have no
compressed clusters?
Paolo
> configure | 10 +++++++++-
> meson.build | 4 ++++
> block/qcow2.c | 32 +++++++++++++++++++++++++-------
> meson_options.txt | 2 ++
> 4 files changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/configure b/configure
> index debd50c085..b19af43525 100755
> --- a/configure
> +++ b/configure
> @@ -385,6 +385,7 @@ snappy="auto"
> bzip2="auto"
> lzfse="auto"
> zstd="auto"
> +qcow2_zstd_default="no"
> guest_agent="$default_feature"
> guest_agent_with_vss="no"
> guest_agent_ntddscsi="no"
> @@ -1318,6 +1319,10 @@ for opt do
> ;;
> --enable-zstd) zstd="enabled"
> ;;
> + --disable-qcow2-zstd-default) qcow2_zstd_default="disabled"
> + ;;
> + --enable-qcow2-zstd-default) qcow2_zstd_default="enabled"
> + ;;
> --enable-guest-agent) guest_agent="yes"
> ;;
> --disable-guest-agent) guest_agent="no"
> @@ -1903,6 +1908,8 @@ disabled with --disable-FEATURE, default is enabled if available
> (for reading lzfse-compressed dmg images)
> zstd support for zstd compression library
> (for migration compression and qcow2 cluster compression)
> + qcow2-zstd-default Use zstd by default for qcow2 image creation
> + (requires zstd enabled)
> seccomp seccomp support
> coroutine-pool coroutine freelist (better performance)
> glusterfs GlusterFS backend
> @@ -6424,7 +6431,8 @@ if test "$skip_meson" = no; then
> -Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2 -Dlibiscsi=$libiscsi \
> -Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
> -Drbd=$rbd -Dlzo=$lzo -Dsnappy=$snappy -Dlzfse=$lzfse \
> - -Dzstd=$zstd -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
> + -Dzstd=$zstd -Dqcow2_zstd_default=$qcow2_zstd_default \
> + -Dseccomp=$seccomp -Dvirtfs=$virtfs -Dcap_ng=$cap_ng \
> -Dattr=$attr -Ddefault_devices=$default_devices \
> -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
> -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
> diff --git a/meson.build b/meson.build
> index d8a92666fb..3d65b6c46b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -484,6 +484,9 @@ if not get_option('zstd').auto() or have_block
> required: get_option('zstd'),
> method: 'pkg-config', kwargs: static_kwargs)
> endif
> +if not zstd.found() and get_option('qcow2_zstd_default').enabled()
> + error('--enable-qcow2-zstd-default: Cannot use zstd by default without enabling zstd')
> +endif
> gbm = not_found
> if 'CONFIG_GBM' in config_host
> gbm = declare_dependency(compile_args: config_host['GBM_CFLAGS'].split(),
> @@ -1168,6 +1171,7 @@ config_host_data.set('CONFIG_GETTID', has_gettid)
> config_host_data.set('CONFIG_MALLOC_TRIM', has_malloc_trim)
> config_host_data.set('CONFIG_STATX', has_statx)
> config_host_data.set('CONFIG_ZSTD', zstd.found())
> +config_host_data.set('CONFIG_QCOW2_ZSTD_DEFAULT', get_option('qcow2_zstd_default').enabled())
> config_host_data.set('CONFIG_FUSE', fuse.found())
> config_host_data.set('CONFIG_FUSE_LSEEK', fuse_lseek.found())
> config_host_data.set('CONFIG_X11', x11.found())
> diff --git a/block/qcow2.c b/block/qcow2.c
> index ee4530cdbd..06bfbbf7b8 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3540,17 +3540,36 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> }
> }
>
> - if (qcow2_opts->has_compression_type &&
> - qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
> -
> - ret = -EINVAL;
> -
> - if (version < 3) {
> + if (version < 3) {
> + if (qcow2_opts->has_compression_type &&
> + qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB)
> + {
> + ret = -EINVAL;
> error_setg(errp, "Non-zlib compression type is only supported with "
> "compatibility level 1.1 and above (use version=v3 or "
> "greater)");
> goto out;
> }
> + } else {
> + if (qcow2_opts->has_compression_type) {
> + compression_type = qcow2_opts->compression_type;
> +#ifdef CONFIG_QCOW2_ZSTD_DEFAULT
> + } else {
> + compression_type = QCOW2_COMPRESSION_TYPE_ZSTD;
> +#endif
> + }
> +
> +#ifndef CONFIG_ZSTD
> + assert(compression_type == QCOW2_COMPRESSION_TYPE_ZLIB);
> +#endif
> + }
> +
> + if (qcow2_opts->has_compression_type &&
> + qcow2_opts->compression_type != QCOW2_COMPRESSION_TYPE_ZLIB) {
> +
> + ret = -EINVAL;
> +
> + compression_type = qcow2_opts->compression_type;
>
> switch (qcow2_opts->compression_type) {
> #ifdef CONFIG_ZSTD
> @@ -3562,7 +3581,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> goto out;
> }
>
> - compression_type = qcow2_opts->compression_type;
> }
>
> /* Create BlockBackend to write to the image */
> diff --git a/meson_options.txt b/meson_options.txt
> index 3d304cac96..8af9bb97f5 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -108,6 +108,8 @@ option('xkbcommon', type : 'feature', value : 'auto',
> description: 'xkbcommon support')
> option('zstd', type : 'feature', value : 'auto',
> description: 'zstd compression support')
> +option('qcow2_zstd_default', type : 'feature', value : 'disabled',
> + description: 'Use zstd compression type for qcow2 image creation by default')
> option('fuse', type: 'feature', value: 'auto',
> description: 'FUSE block device export')
> option('fuse_lseek', type : 'feature', value : 'auto',
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
2021-06-21 8:22 ` Paolo Bonzini
@ 2021-06-21 15:59 ` Kevin Wolf
2021-06-22 10:16 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2021-06-21 15:59 UTC (permalink / raw)
To: Paolo Bonzini
Cc: den, Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block, mreitz
Am 21.06.2021 um 10:22 hat Paolo Bonzini geschrieben:
> On 17/06/21 21:51, Vladimir Sementsov-Ogievskiy wrote:
> > So, it's an RFC. I also can split the patch so that refactoring of
> > qcow2_co_create() go in a separate preparation patch.
> >
> > Another RFC question, shouldn't we move to zstd by default in upstream
> > too?
>
> I think backwards-incompatible changes in the past were not handled with
> build options, but that can be changed.
>
> However I would prefer to have an option like
> --with-qcow2-compression={zstd,zlib}. Meson supports multiple-choice
> options, they don't have to use enabled/disabled or (if boolean) true/false.
Yes, this is more extensible.
> Regarding changing the default, that would make images unreadable to QEMU
> 5.0 and earlier versions. Does that apply to images that have no compressed
> clusters?
I think it does because you could be writing compressed clusters to it
later. Originally, we had only 'qemu-img convert -c' that could write
compressed clusters, but today the backup job can write them, too, and
it doesn't create the image file itself.
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
2021-06-21 15:59 ` Kevin Wolf
@ 2021-06-22 10:16 ` Vladimir Sementsov-Ogievskiy
2021-06-22 10:20 ` Paolo Bonzini
0 siblings, 1 reply; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-06-22 10:16 UTC (permalink / raw)
To: Kevin Wolf, Paolo Bonzini; +Cc: qemu-block, qemu-devel, mreitz, den
21.06.2021 18:59, Kevin Wolf wrote:
> Am 21.06.2021 um 10:22 hat Paolo Bonzini geschrieben:
>> On 17/06/21 21:51, Vladimir Sementsov-Ogievskiy wrote:
>>> So, it's an RFC. I also can split the patch so that refactoring of
>>> qcow2_co_create() go in a separate preparation patch.
>>>
>>> Another RFC question, shouldn't we move to zstd by default in upstream
>>> too?
>>
>> I think backwards-incompatible changes in the past were not handled with
>> build options, but that can be changed.
>>
>> However I would prefer to have an option like
>> --with-qcow2-compression={zstd,zlib}. Meson supports multiple-choice
>> options, they don't have to use enabled/disabled or (if boolean) true/false.
>
> Yes, this is more extensible.
Looks good, I'll try. I'm just not familiar with it and followed the pattern of zstd option.
>
>> Regarding changing the default, that would make images unreadable to QEMU
>> 5.0 and earlier versions. Does that apply to images that have no compressed
>> clusters?
>
> I think it does because you could be writing compressed clusters to it
> later.
I'm afraid it does anyway: incompatible feature bit is in use for the feature. Older Qemu will refuse opening image with incompatible bit
> Originally, we had only 'qemu-img convert -c' that could write
> compressed clusters, but today the backup job can write them, too, and
> it doesn't create the image file itself.
>
More details:
- compression type is specified once, when image is created by qemu-img create Or blockdev-create. At that time incompatible bit is set if zstd is chosen.
- compressed writes are done by
- qemu-img convert -c
- backup jobs (blockdev-backup adn drive-backup) when option compress is set to true
- compress filter (which is available to insert where user wants)
- drive-backup may create target image automatically or use existing image depending on options.
I have a plan to deprecate drive-backup (patches are somewhere in the list and waits for possibility to deprecate union fields in QAPI).
I think preferred way of compression would be compress-filter, and we may want to deprecate compress option of blockdev-backup at some moment.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
2021-06-22 10:16 ` Vladimir Sementsov-Ogievskiy
@ 2021-06-22 10:20 ` Paolo Bonzini
0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2021-06-22 10:20 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, Kevin Wolf
Cc: den, qemu-devel, qemu-block, mreitz
On 22/06/21 12:16, Vladimir Sementsov-Ogievskiy wrote:
>>
>> Yes, this is more extensible.
>
> Looks good, I'll try. I'm just not familiar with it and followed the
> pattern of zstd option.
You can look at -Dmalloc for an example.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC] meson: add option to use zstd for qcow2 compression by default
2021-06-17 19:51 [PATCH RFC] meson: add option to use zstd for qcow2 compression by default Vladimir Sementsov-Ogievskiy
2021-06-19 14:13 ` Vladimir Sementsov-Ogievskiy
2021-06-21 8:22 ` Paolo Bonzini
@ 2021-07-01 9:54 ` Vladimir Sementsov-Ogievskiy
2 siblings, 0 replies; 7+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-07-01 9:54 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz, kwolf, den
17.06.2021 22:51, Vladimir Sementsov-Ogievskiy wrote:
> Recently we added support of zstd to qcow2 format, as zstd seems to be
> better than zlib in general, and which is important (as qcow2
> compression used mostly for backups) compressed writes are faster with
> zstd.
>
> Let's add a build option to use zstd by default.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>
> Hi all! We want to use zstd as a default compression type for newly
> created qcow2 images in upcoming Virtuozzo 8.
>
> I am not sure, how much the community interested in such option,
> probably I should just keep a downstream-only patch changing the
> default.
>
> But if you like it, I'd be happy to only set new config option in our
> qemu build instead of maintaining extra downstream-only patch :)
>
> So, it's an RFC. I also can split the patch so that refactoring of
> qcow2_co_create() go in a separate preparation patch.
>
> Another RFC question, shouldn't we move to zstd by default in upstream
> too?
>
First: the biggest difficulty with zstd as default is updating all iotests to work in such conditions. I'm working on this now and hope to post series soon, which just improves iotests to support IMGOPTS='compression_type=zstd'. That's a first step which is good anyway and don't change any default.
Next: I have a doubt now. Maybe, better way of having alternative default in downstream project is introcuducing a kind of configuration file for Qemu? Qemu will parse it if exist, and we can ship such a file in rpm package. Something like /etc/qemu.conf. Any thoughts? Did something like this ever considered?
Benefit of configuration file vs build options is obvious: you can change a default without recompiling.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-07-01 9:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 19:51 [PATCH RFC] meson: add option to use zstd for qcow2 compression by default Vladimir Sementsov-Ogievskiy
2021-06-19 14:13 ` Vladimir Sementsov-Ogievskiy
2021-06-21 8:22 ` Paolo Bonzini
2021-06-21 15:59 ` Kevin Wolf
2021-06-22 10:16 ` Vladimir Sementsov-Ogievskiy
2021-06-22 10:20 ` Paolo Bonzini
2021-07-01 9:54 ` Vladimir Sementsov-Ogievskiy
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.