Re: [Qemu-block] [PATCH] [RFC] qcow2: add compression type feature

[Denis Plotnikov]

On 16.05.2019 13:40, Max Reitz wrote:
> On 16.05.19 09:50, Denis Plotnikov wrote:
>>
>>
>> On 29.04.2019 1:32, Max Reitz wrote:
>>> On 05.02.19 10:08, Denis Plotnikov wrote:
>>>> The patch adds some preparation parts for incompatible compression type
>>>> feature into 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 convertion, thus the only compression
>>>> algorithm is used for the image.
>>>>
>>>> The plan is to add support for ZSTD and then may be something more 
>>>> effective
>>>> in the future.
>>>>
>>>> ZSDT compression algorithm consumes 3-5 times less CPU power with a
>>>> comparable comression ratio with zlib. It would be wise to use it for
>>>> data compression f.e. for backups.
>>>>
>>>> The default compression is ZLIB.
>>>>
>>>> Signed-off-by: Denis Plotnikov <address@hidden>
>>>> ---
>>>>    block/qcow2.c | 25 +++++++++++++++++++++++++
>>>>    block/qcow2.h | 26 ++++++++++++++++++++++----
>>>>    2 files changed, 47 insertions(+), 4 deletions(-)
>>>
>>> Are there plans to pursue this further?
>> Yes, I'm going to come up with the first version in a few weeks
> 
> OK.
> 
>>> [...]
>>>
>>>> diff --git a/block/qcow2.h b/block/qcow2.h
>>>> index 32cce9eee2..fdde5bbefd 100644
>>>> --- a/block/qcow2.h
>>>> +++ b/block/qcow2.h
>>>> @@ -112,6 +112,10 @@
>>>>    #define QCOW2_OPT_REFCOUNT_CACHE_SIZE "refcount-cache-size"
>>>>    #define QCOW2_OPT_CACHE_CLEAN_INTERVAL "cache-clean-interval"
>>>>    
>>>> +/* Compression types */
>>>> +#define QCOW2_COMPRESSION_TYPE_ZLIB    0
>>>> +#define QCOW2_COMPRESSION_TYPE_ZSTD    1
>>>
>>> We probably want QAPI types anyway (qemu-img info should report the
>>> compression type), so I think we could use them instead.
>> I'm not sure that I understood the idea. Could you please explain what
>> is meant here? We don't need those constants and should use the
>> constants defined somewhere else (where)?
> 
> qemu-img info can report format-specific information.  I think the
> compression type should be listed there as well, once there is more than
> one.
> 
> Format-specific information is a QAPI type, namely
> ImageInfoSpecificQCow2.  Therefore, everything it contains needs to be a
> QAPI object, and this includes potential compression information.  We
> thus need a QAPI enum like Qcow2CompressionType, and that would
> automatically give us these values.

Thanks for clarification. Will do that way.

Denis
> 
> Max
> 

-- 
Best,
Denis