Re: [Qemu-devel] [PATCH v0 1/3] qcow2: introduce compression type feature

[Kevin Wolf]

Am 28.05.2019 um 16:37 hat Denis Plotnikov geschrieben:
> 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 x2 faster than ZLIB providing a comparable compression ratio
> and therefore provide a performance advantage in backup scenarios.
> 
> The default compression is ZLIB. Images created with ZLIB compression type
> is backward compatible with older qemu versions.
> 
> Signed-off-by: Denis Plotnikov <address@hidden>

> @@ -3119,6 +3123,10 @@
>  #                         an image, the data file name is loaded from the 
> image
>  #                         file. (since 4.0)
>  #
> +# @compression-type:      compression method to use for image clusters 
> compression
> +#                         The comression method is set on image creation and 
> can
> +#                         be changed via image converting only. (since 4.1)
> +#
>  # Since: 2.9
>  ##
>  { 'struct': 'BlockdevOptionsQcow2',
> @@ -3134,7 +3142,8 @@
>              '*refcount-cache-size': 'int',
>              '*cache-clean-interval': 'int',
>              '*encrypt': 'BlockdevQcow2Encryption',
> -            '*data-file': 'BlockdevRef' } }
> +            '*data-file': 'BlockdevRef',
> +            '*compression-type': 'Qcow2CompressionType' } }

qcow2_open() doesn't actually parse this option (and it couldn't do
anything useful with it because the image is fixed to a single
compression type), so this shouldn't be added.

>  ##
>  # @SshHostKeyCheckMode:
> @@ -4206,6 +4215,19 @@
>    'data': [ 'v2', 'v3' ] }
>  
>  
> +##
> +# @Qcow2CompressionType:
> +#
> +# Compression type used in qcow2 image file
> +#
> +# @zlib     : gzip compressor
> +# @zstd     : zstd compression
> +#
> +# Since: 4.1
> +##
> +{ 'enum': 'Qcow2CompressionType',
> +  'data': [ 'zlib', 'zstd' ] }

I think it would be cleaner to start with only 'zlib' here, like your C
code that doesn't implement anything for non-zlib compression types yet.

'zstd' can be added to the enum when it's actually implemented. This
will also make schema introspection provide the right information with a
build that includes this patch, but not the zstd compression type
patch.

Kevin