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',
