Re: [Qemu-devel] [PATCH 18/31] qcow2: Update qcow2_get_cluster_offset() to support L2 slices

[Alberto Garcia]

On Thu, Oct 12, 2017 at 04:05:32PM +0300, Alberto Garcia wrote:
> @@ -522,8 +522,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>  {
>      BDRVQcow2State *s = bs->opaque;
>      unsigned int l2_index;
> -    uint64_t l1_index, l2_offset, *l2_table;
> -    int l1_bits, c;
> +    uint64_t l1_index, l2_offset, *l2_slice;
> +    int c;
>      unsigned int offset_in_cluster;
>      uint64_t bytes_available, bytes_needed, nb_clusters;
>      QCow2ClusterType type;
> @@ -532,12 +532,11 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>      offset_in_cluster = offset_into_cluster(s, offset);
>      bytes_needed = (uint64_t) *bytes + offset_in_cluster;
>  
> -    l1_bits = s->l2_bits + s->cluster_bits;
> -
>      /* compute how many bytes there are between the start of the cluster
> -     * containing offset and the end of the l1 entry */
> -    bytes_available = (1ULL << l1_bits) - (offset & ((1ULL << l1_bits) - 1))
> -                    + offset_in_cluster;
> +     * containing offset and the end of the l2 slice that contains
> +     * the entry pointing to it */
> +    bytes_available = (s->l2_slice_size - offset_to_l2_slice_index(s, 
> offset))
> +        << s->cluster_bits;

There's a mistake here, this should be cast to uint64_t before doing
the shift, else it overflows if the cluster size is large:

    bytes_available =
        ((uint64_t) (s->l2_slice_size - offset_to_l2_slice_index(s, offset)))
        << s->cluster_bits;

The next revision will have this correction, but I'm not sending it
now, so if you want to test/review this one please go ahead.

Berto