[Devel] [PATCH hci-8.1 v12 10/12] dm-qcow2: retry R1/R2 writeback that failed with ENOSPC

[Alexander Ivanov]

>> @@ -3869,9 +3871,9 @@ static int complete_metadata_writeback(struct qcow2 *qcow2)
>>   		md = qio->ext->md;
>>   		qvec = qio->data;
>>   		ret = qio->ret;
>> -		if (unlikely(ret != PAGE_SIZE))
>> +		if (unlikely(ret >= 0 && ret != PAGE_SIZE))
>>   			ret = -EIO;
>> -		else
>> +		else if (ret == PAGE_SIZE)
>>   			ret = fsync_ret;
>>   
>>   		do_md_page_write_complete(ret, qcow2, md);
> [Severity: Medium]
> Can this change make the cluster leak repeat? With the errno now passed
> through, a failed L2 write ends up as -ENOSPC instead of -EIO:
>
> complete_metadata_writeback()
>    do_md_page_write_complete(-ENOSPC, ...)
>      wbd->ret = ret
>      complete_wbd()
>        list_for_each_entry(qio, &end_list, link) {
>            if (qio->ext)
>                qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
>        }
>    end_qios(&end_list, errno_to_blk_status(ret))
>      qrq_endio()
>        if (unlikely(bi_status == BLK_STS_NOSPC)) {
>            if (qcow2_try_delay_enospc(tgt, qrq, qio))
>                return;
>        }
>
> complete_wbd() deliberately does not free the newly allocated data cluster
> when an L1/L2 writeback fails. Before this patch the request ended once with
> -EIO and was not retried, so this leak happened at most once per request.
>
> Now qrq_endio() delays the request and resubmits it. Each resubmission
> allocates a new cluster. If the L2 write fails with -ENOSPC again, that
> cluster is leaked as well, and this repeats for as long as ENOSPC persists.
> The leaked clusters stay counted in the refcounts, and the image file keeps
> growing on an underlying filesystem that is already full.
>
> The commit message acknowledges this ("a request whose L2 write keeps failing
> with ENOSPC leaks one data cluster on every resubmission, until the space is
> back"), but the patch does not bound or fix it. Could the cluster be freed
> (or the leak otherwise limited) before the request is resubmitted on
> BLK_STS_NOSPC?
The leak only happens in a narrow window: the data write to the new cluster
succeeds, but the overwrite of the already allocated L2 page then fails with
ENOSPC. Once the filesystem is really full, the data write fails first 
and its cluster
is freed through FREE_ALLOCATED_CLU, so the leak stops by itself. 
Freeing the
cluster before the request is resubmitted is unsafe, because the failed 
L2 write
may have partially reached the disk, and two guest offsets could then 
share the
cluster. A leaked cluster leaves the image consistent and check reclaims it.
Leaking a few clusters is better than failing the guest write with EIO, 
which is
what happened before this patch.