Re: [Qemu-devel] [RFC V6 03/33] qcow2: Add deduplication structures and fields.

[Stefan Hajnoczi]

On Wed, Feb 06, 2013 at 01:31:36PM +0100, Benoît Canet wrote:
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 718b52b..c7b6860 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -43,6 +43,10 @@
>  #define QCOW_OFLAG_COPIED     (1LL << 63)
>  /* indicate that the cluster is compressed (they never have the copied flag) 
> */
>  #define QCOW_OFLAG_COMPRESSED (1LL << 62)
> +/* indicate that the cluster must be processed when deduplication restart

s/restart/restarts/

> + * also indicate that the on disk dedup hash must be ignored and discarded
> + */
> +#define QCOW_OFLAG_TO_DEDUP (1LL << 61)

PENDING_DEDUP or NOT_DEDUPED_YET might be clearer.

I didn't see this defined in the spec, BTW.  Maybe I missed it.

> +/* Used to keep a single precomputed hash between the calls of the dedup
> + * function
> + */
> +typedef struct {
> +    QCowHash hash;
> +    bool reuse;                  /* The hash is precomputed reuse it */

Is this some kind of memory management "shared"/"dont_free" flag?

> +} QcowPersistantHash;

s/Persistant/Persistent/

> +
> +/* deduplication node */
> +typedef struct {
> +    QCowHash hash;
> +    uint64_t physical_sect;       /* where the cluster is stored on disk */
> +    uint64_t first_logical_sect;  /* logical sector of the first occurence of
> +                                   * this cluster
> +                                   */
> +} QCowHashNode;
> +
> +/* Undedupable hashes that must be written later to disk */
> +typedef struct QCowHashElement {
> +    QCowHash hash;
> +    QTAILQ_ENTRY(QCowHashElement) next;
> +} QCowHashElement;
> +
> +typedef struct {
> +    QcowPersistantHash phash;  /* contains a hash persisting between calls of
> +                                * qcow2_dedup()
> +                                */
> +    QTAILQ_HEAD(, QCowHashElement) undedupables;
> +    int nb_clusters_processed;
> +    int nb_undedupable_sectors;

Is int large enough for huge disk images (multi terabyte and beyond)?

> @@ -160,6 +216,17 @@ typedef struct BDRVQcowState {
>      int64_t free_cluster_index;
>      int64_t free_byte_offset;
>  
> +    bool has_dedup;
> +    DedupStatus dedup_status;
> +    QCowHashAlgo dedup_hash_algo;
> +    Coroutine *dedup_resume_co;
> +    int dedup_co_delay;
> +    uint64_t *dedup_table;
> +    uint64_t dedup_table_offset;
> +    int32_t dedup_table_size;

size_t?