@@ -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.
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to