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

[no-reply at sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dm-qcow2: ENOSPC retry of failed L1/L2 writeback leaks a data cluster per retry
--

commit 647503fd94be077d5a1fad60c25b0ae7127ab8cf
Author: Alexander Ivanov <alexander.ivanov at virtuozzo.com>

dm-qcow2: retry R1/R2 writeback that failed with ENOSPC

This commit redirties an R1/R2 metadata page whose writeback failed with
-ENOSPC, so the slow writeback timer retries it. It also passes the errno
of a failed metadata write through complete_metadata_writeback() instead of
turning it into -EIO. As a result, a data request that depends on a failed
L1/L2 write can now end with BLK_STS_NOSPC and be delayed and resubmitted.

> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c

[ ... ]

> @@ -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?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261110075140.1475607-1-alexander.ivanov@virtuozzo.com?part=10