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

[Eric Blake]

On 01/16/2013 08:47 AM, Benoît Canet wrote:
> Signed-off-by: Benoit Canet <address@hidden>
> ---
>  docs/specs/qcow2.txt |  104 
> +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 102 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
> index 36a559d..d5f8072 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.

If this bit is set, you probably want to require that the deduplication
header extension is present.

> +                                L2 tables size 64KB is different from
> +                                cluster size 4KB.

I'm still not sure what this sentence means.  Remember, cluster size of
normal disk data is configurable; are you stating that if dedup is in
effect, then the cluster size MUST be fixed at 4k (or in other words,
that header offsets 20-23 [cluster_bits] must be exactly 12)?  And my
understanding is that with dedup, there are now two L1 and L2 tables -
the normal tables to get at the actual logical data, and the dedup
tables for getting at the hashes.  Are you stating that both L2 tables
are 64k, or that just the dedup L2 is 64k?

>  
> +== Deduplication ==
> +
> +The deduplication extension contains information concerning deduplication.

Just as I suggested that the deduplication feature bit field above
should require this extension be present, here, I would probably require
that this extension not be present unless the deduplication feature bit
is set.

> +
> +    Byte   0 - 7:   Offset of the RAM deduplication table (RAM lookup)
> +
> +          8 - 11:   Size of the RAM deduplication table = number of L1 64-bit
> +                    pointers
> +
> +              12:   Hash algo enum field
> +                        0: SHA-256
> +                        1: SHA3
> +                        2: SKEIN-256
> +
> +              13:   Dedup strategies bitmap
> +                        0: RAM based hash lookup (always set to 1 for now)
> +                        1: Disk based hash lookup

Are these two bits mutually exclusive, or can they both be used at once?

> +                        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.

If so, it may be better to document in this revision of the file that
the disk-based hash lookup strategy bit must always be 0 for now.

> +
> +== Deduplication table (RAM method) ==
> +

>  == Host cluster management ==
>  
>  qcow2 manages the allocation of host clusters by maintaining a reference 
> count
> @@ -211,7 +311,7 @@ guest clusters to host clusters. They are called L1 and 
> L2 table.
>  
>  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/excepted/except/ - and again, is this for all L2 tables, or just the
dedup L2 tables?

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

signature.asc
Description: OpenPGP digital signature