Re: [PATCH v5 19/31] qcow2: Add subcluster support to calculate_l2_meta()

[Alberto Garcia]

On Thu 07 May 2020 05:34:18 PM CEST, Alberto Garcia wrote:
> On Wed 06 May 2020 07:39:48 PM CEST, Eric Blake wrote:
>>   In fact, if we rely on 20/31 checking for invalid subclusters when
>> computing nb_clusters, we could probably assert that the start and end
>> cluster in this function are not invalid, instead of adding the fail:
>> label.
>
> I think you're right with that, good catch! There's no need to return an
> error code in this function.

No, wait, I got confused. 

Patch 20/31 updates qcow2_get_host_offset(), and that's used for
reading.

The caller of calculate_l2_meta() is qcow2_alloc_cluster_offset()
(indirectly), which is used for writing.

nb_clusters here is calculated with cluster_needs_new_alloc(), but that
doesn't check whether a cluster is valid or not, and certainly doesn't
check if every single subcluster is valid.

Thinking about it, there are probably faster ways to check for the
validity of extended L2 entries, for example:

- For QCOW2_CLUSTER_NORMAL, (l2_bitmap >> 32) & l2_bitmap should be 0.

- For QCOW2_CLUSTER_UNALLOCATED, l2_bitmap & QCOW_L2_BITMAP_ALL_ALLOC
  should be 0.

That's enough to cover all cases of QCOW2_SUBCLUSTER_INVALID that we
have at the moment.

Berto