Re: [Qemu-devel] [RFC V6 01/33] qcow2: Add deduplication to the qcow2 specification.

[Eric Blake]

On 02/06/2013 05:31 AM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet <address@hidden>
> ---
>  docs/specs/qcow2.txt |  105 
> +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 36a559d..8e52de1 100644
> --- a/docs/specs/qcow2.txt
> +++ b/docs/specs/qcow2.txt
> @@ -80,7 +80,12 @@ in the description of a field.
>                                  tables to repair refcounts before accessing 
> the
>                                  image.
>  
> -                    Bits 1-63:  Reserved (set to 0)
> +                    Bit 1:      Deduplication bit.  If this bit is set then
> +                                deduplication is used on this image.
> +                                L2 tables size 64KB is different from
> +                                cluster size 4KB.

If you are going to enforce other aspects of the file when deduplication
is enabled, such as forcing a cluster size of 4kb and forcing the use of
a 16-bit refcount on each cluster, then spell it out here.

> +
> +              13:   Dedup strategies bitmap
> +                        0: RAM based hash lookup (always set to 1 for now)
> +                        1: Disk based hash lookup
> +                        2: Deduplication running if set to 1
> +
> +        14 - 69:    Set to zero and reserved for future use
> +
> +Disk based lookup structure will be described in a future QCOW2 
> specification.

It looks weird to define bit 0 and 2 as the only valid bits, while
reserving bit 1 but stating it cannot be used.  Also, in the future, can
you use both RAM and disk lookup simultaneously, or are you forced to
choose only one or the other?  Depending on how you answer that, I'd
recommend one of these two approaches:

Mutually exclusive:

Now:
13: Dedup flags
    0: Deduplication is running if set to 1
Future patch:
13: Dedup flags
    0: Deduplication is running if set to 1
    1: hash lookup mode: 0 for RAM, 1 for disk

Both supported at once:

Now:
13: Dedup flags
    0: Deduplication is running if set to 1
    1: use RAM lookup (always set to 1 for now)
Future patch:
13: Dedup flags
    0: Deduplication is running if set to 1
    1: use RAM lookup if set to 1
    2: use disk lookup if set to 1
    Bits 1 and 2 cannot both be 0.

> +== Deduplication table arithmetics (RAM method) ==
> +
> +cluster_size = 4096
> +dedup_block_size = 65536 * 5
> +l2_size = 65536 * 16 (16 factor is from the smaller cluster_size)
> +refcount_order must be >= 4

Make restrictions on cluster_size and refcount_order more explicit at
the point where you introduce the deduplication feature bit.

> +
> +The 16 remaining bytes in each l2 deduplication blocks are set to zero and

s/blocks/block/

>  
>  The L1 table has a variable size (stored in the header) and may use multiple
>  clusters, however it must be contiguous in the image file. L2 tables are
> -exactly one cluster in size.
> +exactly one cluster in size excepted for the deduplication case.

s/size excepted/size; except/
s/case./case, where clusters are fixed at 4k and the L2 table is fixed
at 64k./

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature