Re: [PATCH v4 20/30] qcow2: Add subcluster support to discard_in_l2_slice()

[Alberto Garcia]

On Thu 09 Apr 2020 12:05:12 PM CEST, Max Reitz wrote:
>>          switch (qcow2_get_cluster_type(bs, old_l2_entry)) {
>>          case QCOW2_CLUSTER_UNALLOCATED:
>> -            if (full_discard || !bs->backing) {
>> +            if (full_discard) {
>> +                /* If the image has extended L2 entries we can only
>> +                 * skip this operation if the L2 bitmap is zero. */
>> +                uint64_t bitmap = has_subclusters(s) ?
>> +                    get_l2_bitmap(s, l2_slice, l2_index + i) : 0;
>
> Isn’t this bitmap only valid for standard clusters?  In this case, the
> whole cluster is unallocated, so the bitmap shouldn’t be relevant,
> AFAIU.

I'm not sure if I follow you.

An unallocated cluster can still have QCOW_OFLAG_SUB_ZERO set in some of
its subclusters. Those read as zeroes and the rest go to the backing
file.

After a full discard all subclusters should be completely deallocated so
those bits should be cleared.

If the bitmap is already 0 (the whole cluster is already unallocated) or
if the image does not have extended L2 entries (which also means that
the whole cluster is already unallocated) then we can skip the discard.

Berto