Re: [RFC PATCH v3 04/27] qcow2: Add get_l2_entry() and set_l2_entry()

[Eric Blake]

On 12/22/19 5:36 AM, Alberto Garcia wrote:
> The size of an L2 entry is 64 bits, but if we want to have subclusters
> we need extended L2 entries. This means that we have to access L2
> tables and slices differently depending on whether an image has
> extended L2 entries or not.
>
> This patch replaces all l2_slice[] accesses with calls to
> get_l2_entry() and set_l2_entry().
>
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>   block/qcow2-cluster.c  | 65 ++++++++++++++++++++++--------------------
>   block/qcow2-refcount.c | 17 +++++------
>   block/qcow2.h          | 12 ++++++++
>   3 files changed, 55 insertions(+), 39 deletions(-)

> @@ -978,12 +981,12 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, 
> QCowL2Meta *m)
>            * cluster the second one has to do RMW (which is done above by
>            * perform_cow()), update l2 table with its cluster pointer and free
>            * old cluster. This is what this loop does */
> -        if (l2_slice[l2_index + i] != 0) {
> -            old_cluster[j++] = l2_slice[l2_index + i];
> +        if (get_l2_entry(s, l2_slice, l2_index + i) != 0) {
> +            old_cluster[j++] = get_l2_entry(s, l2_slice, l2_index + i);
>           }
>   
> -        l2_slice[l2_index + i] = cpu_to_be64((cluster_offset +
> -                    (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
> +        set_l2_entry(s, l2_slice, l2_index + i, QCOW_OFLAG_COPIED |
> +                     (cluster_offset + (i << s->cluster_bits)));

Cute commutative law use for line length reasons.

> +++ b/block/qcow2.h

scripts/git.orderfile can be used to hoist this part of the patch to the 
front of the message (as it is more valuable to review first).

> @@ -495,6 +495,18 @@ typedef enum QCow2MetadataOverlap {
>   
>   #define INV_OFFSET (-1ULL)
>   
> +static inline uint64_t get_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
> +                                    int idx)
> +{
> +    return be64_to_cpu(l2_slice[idx]);
> +}
> +
> +static inline void set_l2_entry(BDRVQcow2State *s, uint64_t *l2_slice,
> +                                int idx, uint64_t entry)
> +{
> +    l2_slice[idx] = cpu_to_be64(entry);
> +}
> +

Reviewed-by: Eric Blake <address@hidden>

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