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

[Denis Plotnikov]

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
> 
> [...]
> 
>> 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)?

Denis


> 
>>   typedef struct QCowHeader {
>>       uint32_t magic;
>>       uint32_t version;
>> @@ -197,10 +201,13 @@ enum {
>>   
>>   /* Incompatible feature bits */
>>   enum {
>> -    QCOW2_INCOMPAT_DIRTY_BITNR   = 0,
>> -    QCOW2_INCOMPAT_CORRUPT_BITNR = 1,
>> -    QCOW2_INCOMPAT_DIRTY         = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
>> -    QCOW2_INCOMPAT_CORRUPT       = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
>> +    QCOW2_INCOMPAT_DIRTY_BITNR            = 0,
>> +    QCOW2_INCOMPAT_CORRUPT_BITNR          = 1,
>> +    QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR = 2,
>> +    QCOW2_INCOMPAT_DIRTY                  = 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
>> +    QCOW2_INCOMPAT_CORRUPT                = 1 << 
>> QCOW2_INCOMPAT_CORRUPT_BITNR,
>> +    QCOW2_INCOMPAT_COMPRESSION_TYPE       =
>> +                                    1 << 
>> QCOW2_INCOMPAT_COMPRESSION_TYPE_BITNR,
>>   
>>       QCOW2_INCOMPAT_MASK          = QCOW2_INCOMPAT_DIRTY
>>                                    | QCOW2_INCOMPAT_CORRUPT,
> 
> This mask needs to be expanded by QCOW2_INCOMPAT_COMPRESSION_TYPE.
> 
>> @@ -256,6 +263,10 @@ typedef struct Qcow2BitmapHeaderExt {
>>       uint64_t bitmap_directory_offset;
>>   } QEMU_PACKED Qcow2BitmapHeaderExt;
>>   
>> +typedef struct Qcow2CompressionTypeExt {
>> +    uint32_t compression_type;
>> +} QEMU_PACKED Qcow2CompressionTypeExt;
>> +
>>   typedef struct BDRVQcow2State {
>>       int cluster_bits;
>>       int cluster_size;
>> @@ -340,6 +351,13 @@ typedef struct BDRVQcow2State {
>>   
>>       CoQueue compress_wait_queue;
>>       int nb_compress_threads;
>> +    /**
>> +     * Compression type used for the image. Default: 0 - ZLIB
>> +     * The image compression type is set on image creation.
>> +     * The only way to change the compression type is to convert the image
>> +     * with the desired compresion type set
> 
> *compression
> 
> And, well, ideally qemu-img amend could perform this operation, too.
> 
> Max
> 
>> +     */
>> +    uint32_t compression_type;
>>   } BDRVQcow2State;
>>   
>>   typedef struct Qcow2COWRegion {
>>
> 
> 

-- 
Best,
Denis