Re: [PATCH v5 07/31] qcow2: Document the Extended L2 Entries feature

[Eric Blake <eblake@redhat.com>]

On 5/5/20 12:38 PM, Alberto Garcia wrote:
> Subcluster allocation in qcow2 is implemented by extending the
> existing L2 table entries and adding additional information to
> indicate the allocation status of each subcluster.
> 
> This patch documents the changes to the qcow2 format and how they
> affect the calculation of the L2 cache size.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>   docs/interop/qcow2.txt | 68 ++++++++++++++++++++++++++++++++++++++++--
>   docs/qcow2-cache.txt   | 19 +++++++++++-
>   2 files changed, 83 insertions(+), 4 deletions(-)
> 

> @@ -547,7 +557,8 @@ Standard Cluster Descriptor:
>                       nor is data read from the backing file if the cluster is
>                       unallocated.
>   
> -                    With version 2, this is always 0.
> +                    With version 2 or with extended L2 entries (see the next
> +                    section), this is always 0.

In your cover letter, you said you changed things to tolerate this bit 
being set even with extended L2 entries.  Does this sentence need a tweak?

>   
>            1 -  8:    Reserved (set to 0)
>   
> @@ -584,6 +595,57 @@ file (except if bit 0 in the Standard Cluster Descriptor is set). If there is
>   no backing file or the backing file is smaller than the image, they shall read
>   zeros for all parts that are not covered by the backing file.
>   
> +== Extended L2 Entries ==
> +
> +An image uses Extended L2 Entries if bit 4 is set on the incompatible_features
> +field of the header.
> +
> +In these images standard data clusters are divided into 32 subclusters of the
> +same size. They are contiguous and start from the beginning of the cluster.
> +Subclusters can be allocated independently and the L2 entry contains information
> +indicating the status of each one of them. Compressed data clusters don't have
> +subclusters so they are treated the same as in images without this feature.
> +
> +The size of an extended L2 entry is 128 bits so the number of entries per table
> +is calculated using this formula:
> +
> +    l2_entries = (cluster_size / (2 * sizeof(uint64_t)))
> +
> +The first 64 bits have the same format as the standard L2 table entry described
> +in the previous section, with the exception of bit 0 of the standard cluster
> +descriptor.

Also this sentence.

> +
> +The last 64 bits contain a subcluster allocation bitmap with this format:
> +
> +Subcluster Allocation Bitmap (for standard clusters):
> +
> +    Bit  0 -  31:   Allocation status (one bit per subcluster)

Why two spaces after '-'?  I understand it in situations like '0 -  3' 
in the same list as '16 - 19', to make for a right-justified column, but 
here, everything in the second column is two digits, so the extra 
padding doesn't add anything useful.  Or did you mean to have '64 -  95' 
and '96 - 127', making it obvious that these are the second set of bits 
on top of the existing bits in the first 8 bytes?

> +
> +                    1: the subcluster is allocated. In this case the
> +                       host cluster offset field must contain a valid
> +                       offset.
> +                    0: the subcluster is not allocated. In this case
> +                       read requests shall go to the backing file or
> +                       return zeros if there is no backing file data.
> +
> +                    Bits are assigned starting from the least significant
> +                    one (i.e. bit x is used for subcluster x).
> +
> +        32 -  63    Subcluster reads as zeros (one bit per subcluster)
> +
> +                    1: the subcluster reads as zeros. In this case the
> +                       allocation status bit must be unset. The host
> +                       cluster offset field may or may not be set.
> +                    0: no effect.
> +
> +                    Bits are assigned starting from the least significant
> +                    one (i.e. bit x is used for subcluster x - 32).

Of course, if you change to 64-95 and 96-127, the two sentences mapping 
bit x to subcluster y need adjusting by 64 as well.

> +
> +Subcluster Allocation Bitmap (for compressed clusters):
> +
> +    Bit  0 -  63:   Reserved (set to 0)
> +                    Compressed clusters don't have subclusters,
> +                    so this field is not used.

I can live with the wording as-is (since you did call out the "second 64 
bits" or with the adjusted bit numberings.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org