On Mon, Nov 26, 2012 at 02:05:05PM +0100, Benoît Canet wrote:
> +/*
> + * Compute the hash of a given cluster
> + *
> + * @data: a buffer containing the cluster data
> + * @ret: a HASH_LENGTH long dynamically allocated array containing the hash
> + */
> +static uint8_t *qcow2_compute_cluster_hash(BlockDriverState *bs,
> + uint8_t *data)
> +{
> + return NULL;
> +}
> +
> +/* Try to find the offset of a given cluster if it's duplicated
> + * Exceptionally we cast return value to int64_t to use as error code.
I don't see an int64_t return value, this comment is probably outdated.
> + *
> + * @data: a buffer containing the cluster
> + * @skip_cluster_nr: the number of cluster to skip in the buffer
> + * @hash: if hash is provided it's used else it's computed
> + * @ret: QCowHashNode of the duplicated cluster or NULL
This function does several things:
1. Allocating and computing *hash if not given.
2. Returning existing dedup_tree_by_hash node or NULL if the node wasn't
already in the tree.
3. Inserting the node into the tree if not present.
I wonder if it can be simplified or split to do less work.
> +/*
> + * Helper used to link a deduplicated cluster in the l2
> + *
> + * @logical_cluster_offset: the cluster offset seen by the guest (in
> sectors)
> + * @physical_cluster_offset: the cluster offset in the QCOW2 file (in
> sectors)
Perhaps s/_offset/_sect/ because usually offset is in bytes.
