[Devel] [PATCH hci-8.1 v11 08/10] dm-qcow2: leak clusters allocated for a failed L1/L2 writeback

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

A failed L1/L2 writeback reverts the page in memory and frees each
cluster allocated for it: revert_l_entries_update() drops the refcount
of a newly allocated cluster, and a COW qio waiting on the page frees
its cluster through FREE_ALLOCATED_CLU when it ends with the error. The
write may still have partially reached the disk, so L1/L2 on disk may
point to such a cluster. Until the page is rewritten, the allocator may
give the cluster to another offset, and two guest offsets then share
it. If the page is not rewritten before suspend, the image is left with
a reference to a free cluster.

Keep the refcount of these clusters instead. If L1/L2 on disk points to
such a cluster, the image stays consistent; otherwise the cluster is
leaked, which check repairs by freeing it. So the image needs no check
and resume needs no refusal.

https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 222b6f1291c1c ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c    | 51 +++++++-----------------------------
 drivers/md/dm-qcow2-target.c |  4 ---
 2 files changed, 9 insertions(+), 46 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 995f611ae5bb..4ab8051f08c4 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1331,50 +1331,15 @@ static void do_md_page_read_complete(int ret, struct qcow2 *qcow2,
 /* Be careful with dirty_or_writeback()/etc! Check races. */
 static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
 {
-	struct qcow2_map_item r1, r2;
-	struct page *pe_page;
-	bool skip_odd;
-	u64 pos, old;
-	bool cleared;
-	int i, ret;
-
-	skip_odd = qcow2->ext_l2 && wbd->lx_level == L2_LEVEL;
+	struct page *pe_page = wbd->pe_page;
+	u64 old;
+	int i;
 
 	lockdep_assert_held(&qcow2->md_pages_lock);
 	for_each_set_bit(i, wbd->changed_indexes, LX_INDEXES_PER_PAGE) {
-		pos = get_u64_from_be_page(wbd->md->page, i);
-
-		cleared = !(pos & ~(u64)L2_READS_ALL_ZEROES);
-
 		/* Here we restore prealloced and compressed clu mappings */
-		pe_page = wbd->pe_page;
-		if (pe_page) { /* Only L2 has this. */
-			old = get_u64_from_be_page(pe_page, i);
-			if (old != 0) {
-				set_u64_to_be_page(wbd->md->page, i, old);
-				continue; /* Avoid mark_cluster_unused() */
-			}
-		}
-
-		set_u64_to_be_page(wbd->md->page, i, 0);
-		if (skip_odd && (i & 1))
-			continue; /* pos contains ext_l2 part of L2 entry */
-		if (cleared)
-			continue; /* no cluster was allocated */
-
-		spin_unlock(&qcow2->md_pages_lock);
-		pos &= ~LX_REFCOUNT_EXACTLY_ONE;
-
-		/*
-		 * R1/R2 should be cached, since we was able
-		 * to submit cluster allocation.
-		 */
-		ret = qcow2_handle_r1r2_maps(qcow2, pos, NULL, &r1, &r2, false);
-		if (WARN_ON_ONCE(ret <= 0))
-			continue;
-
-		mark_cluster_unused(qcow2, r2.md, r2.index_in_page, pos);
-		spin_lock(&qcow2->md_pages_lock);
+		old = pe_page ? get_u64_from_be_page(pe_page, i) : 0;
+		set_u64_to_be_page(wbd->md->page, i, old);
 	}
 }
 
@@ -1408,6 +1373,7 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
 		LIST_HEAD(wait_list);
 		LIST_HEAD(end_list);
 		unsigned long flags;
+		struct qio *qio;
 
 		spin_lock_irqsave(&qcow2->md_pages_lock, flags);
 		revert_l_entries_update(qcow2, wbd);
@@ -1415,6 +1381,8 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
 				       &wait_list, &end_list);
 		spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
 
+		list_for_each_entry(qio, &end_list, link)
+			qio->ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
 		qcow2_dispatch_qios(qcow2, NULL, &wait_list);
 		end_qios(&end_list, errno_to_blk_status(wbd->ret));
 	}
@@ -1445,8 +1413,7 @@ static void do_md_page_write_complete(int ret, struct qcow2 *qcow2,
 		/*
 		 * We can finish wb only in case of success.
 		 * Otherwise this is done in finalize_wbd()
-		 * after data qios stopped use wbd clusters
-		 * and clusters allocations reverted.
+		 * after data qios stopped use wbd clusters.
 		 */
 		if (likely(ret == 0)) {
 			clear_writeback_status(qcow2, md, ret,
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 6824d95193ae..99fb68a5535e 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -173,10 +173,6 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
 	struct md_page *md;
 	int i;
 
-	/*
-	 * We need second iteration, since revert_l_entries_update()
-	 * may start timer again after failed wb.
-	 */
 	for (i = 0; i < 2; i++) {
 		if (timer_delete_sync_try(&qcow2->slow_wb_timer) != -1)
 			qcow2_slow_wb_timer_fn(&qcow2->slow_wb_timer);
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel