On 10.04.20 14:47, Alberto Garcia wrote:
> 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.Hm, right, this is the only way to have non-preallocated zero clusters after all. I suppose I read the spec wrong and assumed somehow that unallocated clusters don’t use “standard cluster descriptors”, so their bitmap usage would be undefined. Don’t know how that happened. > 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. Yep, seems right. Reviewed-by: Max Reitz <mre...@redhat.com>
signature.asc
Description: OpenPGP digital signature
