[Devel] [PATCH VZ10 v3 00/11] dm-qcow2: improve discard and read-only merge handling

[Devel] [PATCH VZ10 v3 00/11] dm-qcow2: improve discard and read-only merge handling

[Andrey Zhadchenko]

This series makes discard update qcow2 metadata and permits backward
merge from read-only source images.

First patch improves revert_cluster_alloc: this function didn't expect
L2 entries to contain subcluster descriptions (when ext2_l2 is set).

The second patch renames COW machinery to a more general 'replace entry'.

Next five patches improve discard support in general: now it clears
metadata and have a subcluster granularity.

The next two patches support backward merge when the image we merge
from is read-only. Such merges now leave the source mappings and
metadata unchanged. Previously the module ignored file mode and
submitted writes anyway.

Last patch makes merge respect all metadata-zeroed blocks. For now it is
writing zeroes as data, but we will improve this to metadata when we
support REQ_OP_WRITE_ZEROES.


v2:
 - patch "update metadata on subclusters discard": expand
qio_discard_updates_metadata() condition to return true when backing
file is present and fully covered sublcu zero bit isn't already set.
prepare_cluster_discard() already handled this case but I forgot
about it here.

v3:
 - add new "drivers/md/dm-qcow2: do not attach a bvec to discard qios"
patch which ensures discard qios do not have garbage bvecs. Otherwise
COW operations before discard may use these them. Following patches
move discard above COW and alleviate this problem, but let's be
explicit.
 - various commit message and code formatting fixes, but nothing
serious

Andrey Zhadchenko (11):
  drivers/md/dm-qcow2: do not attach a bvec to discard qios
  drivers/md/dm-qcow2: fix revert_cluster_alloc() for ext_l2 case
  drivers/md/dm-qcow2: generalize COW index update machinery
  drivers/md/dm-qcow2: update metadata on whole cluster discard
  drivers/md/dm-qcow2: never trigger COW or allocation on discard
  drivers/md/dm-qcow2: set L2_READS_ALL_ZEROES after some discards
  drivers/md/dm-qcow2: update metadata on subclusters discard
  drivers/md/dm-qcow2: unmap cluster when discard clears last allocated
    subclusters
  drivers/md/dm-qcow2: do discards during backward merge only for
    writable image
  drivers/md/dm-qcow2: allow shared L1 entries during merge from RO
    image
  drivers/md/dm-qcow2: respect zeroes during merge

 drivers/md/dm-qcow2-cmd.c    |  21 +-
 drivers/md/dm-qcow2-map.c    | 415 +++++++++++++++++++++++++++--------
 drivers/md/dm-qcow2-target.c |   2 +-
 drivers/md/dm-qcow2.h        |  19 +-
 4 files changed, 351 insertions(+), 106 deletions(-)

-- 
2.43.5

[Devel] [PATCH VZ10 v3 01/11] drivers/md/dm-qcow2: do not attach a bvec to discard qios

[Andrey Zhadchenko]

prepare_one_embedded_qio() skipped bvec setup only for multi-bio
discard requests. A single-bio discard still took the else branch:
payload-less bios have bi_io_vec == NULL and a non-zero bi_size, so
qio->bi_io_vec was set to bio->bi_inline_vecs. That flexible array
is never NULL, but it is not a valid vec list.
Explicitly initialize the qio iterator from the request for every
discard and leave bi_io_vec NULL.

Feature: dm-qcow2: block device over QCOW2 files driver
Fixes: 1118b9c7875a ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index db21efb45e17a..506458f04888e 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -3515,6 +3515,14 @@ static struct bio_vec *create_bvec_from_rq(struct request *rq)
 	return bvec;
 }
 
+static void qio_init_iter_from_rq(struct qio *qio, struct request *rq)
+{
+	qio->bi_iter.bi_sector = blk_rq_pos(rq);
+	qio->bi_iter.bi_size = blk_rq_bytes(rq);
+	qio->bi_iter.bi_idx = 0;
+	qio->bi_iter.bi_bvec_done = 0;
+}
+
 static void prepare_one_embedded_qio(struct qcow2 *qcow2, struct qio *qio,
 				     struct list_head *deferred_qios)
 {
@@ -3524,9 +3532,9 @@ static void prepare_one_embedded_qio(struct qcow2 *qcow2, struct qio *qio,
 	LIST_HEAD(list);
 	int ret;
 
-	if (rq->bio != rq->biotail) {
-		if (req_op(rq) == REQ_OP_DISCARD)
-			goto skip_bvec;
+	if (req_op(rq) == REQ_OP_DISCARD) {
+		qio_init_iter_from_rq(qio, rq);
+	} else if (rq->bio != rq->biotail) {
 		/*
 		 * Transform a set of bvec arrays related to bios
 		 * into a single bvec array (which we can iterate).
@@ -3535,11 +3543,7 @@ static void prepare_one_embedded_qio(struct qcow2 *qcow2, struct qio *qio,
 		if (unlikely(!bvec))
 			goto err;
 		qrq->bvec = bvec;
-skip_bvec:
-		qio->bi_iter.bi_sector = blk_rq_pos(rq);
-		qio->bi_iter.bi_size = blk_rq_bytes(rq);
-		qio->bi_iter.bi_idx = 0;
-		qio->bi_iter.bi_bvec_done = 0;
+		qio_init_iter_from_rq(qio, rq);
 	} else {
 		/* Single bio already provides bvec array */
 		bvec = rq->bio->bi_io_vec;
-- 
2.43.5

[Devel] [PATCH VZ10 v3 02/11] drivers/md/dm-qcow2: fix revert_cluster_alloc() for ext_l2 case

[Andrey Zhadchenko]

This function walks over all changed u64 values in md. With ext_l2
half of them holds subcluster description. Firstly it reverts these
values to saved pe_page, which is fine, but then it tries to revert
r1r2 changes. It makes no sense when the value is a subcluster
description.
Teach the function to skip subcluster descriptions: do it based on
a new lx_level in struct wb_desc. Add new argument to
prepare_l_entry_update() and set it there.
The warning also could have tripped for ext_l2 entries, so drop
it entirely.
The function effectively reverts not only cluster alloc, so rename
it to revert_l_entries_update() to mimic prepare_l_entry_update().

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c    | 28 ++++++++++++++++++----------
 drivers/md/dm-qcow2-target.c |  2 +-
 drivers/md/dm-qcow2.h        |  1 +
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 506458f04888e..651730bd15901 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1297,18 +1297,19 @@ static void do_md_page_read_complete(int ret, struct qcow2 *qcow2,
 }
 
 /* Be careful with dirty_or_writeback()/etc! Check races. */
-static void revert_clusters_alloc(struct qcow2 *qcow2, struct wb_desc *wbd)
+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;
 	int i, ret;
 
+	skip_odd = qcow2->ext_l2 && wbd->lx_level == L2_LEVEL;
+
 	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);
-		WARN_ON_ONCE(!(pos & ~LX_REFCOUNT_EXACTLY_ONE) ||
-			     !(pos & LX_REFCOUNT_EXACTLY_ONE));
 
 		/* Here we restore prealloced and compressed clu mappings */
 		pe_page = wbd->pe_page;
@@ -1321,6 +1322,9 @@ static void revert_clusters_alloc(struct qcow2 *qcow2, struct wb_desc *wbd)
 		}
 
 		set_u64_to_be_page(wbd->md->page, i, 0);
+		if (skip_odd && (i & 1))
+			continue; /* pos contains ext_l2 part of L2 entry */
+
 		spin_unlock(&qcow2->md_pages_lock);
 		pos &= ~LX_REFCOUNT_EXACTLY_ONE;
 
@@ -1369,7 +1373,7 @@ static void complete_wbd(struct qcow2 *qcow2, struct wb_desc *wbd)
 		unsigned long flags;
 
 		spin_lock_irqsave(&qcow2->md_pages_lock, flags);
-		revert_clusters_alloc(qcow2, wbd);
+		revert_l_entries_update(qcow2, wbd);
 		clear_writeback_status(qcow2, wbd->md, wbd->ret,
 				       &wait_list, &end_list);
 		spin_unlock_irqrestore(&qcow2->md_pages_lock, flags);
@@ -2433,7 +2437,7 @@ static loff_t allocate_cluster(struct qcow2 *qcow2, struct qio *qio,
 #define LU_IGN_CHANGED_IND	(1 << 3)
 static int prepare_l_entry_update(struct qcow2 *qcow2, struct qio *qio,
 				  struct md_page *md, u32 index_in_page,
-				  u64 *pval, u32 arg_mask)
+				  u64 *pval, u32 arg_mask, u8 lx_level)
 {
 	bool wants_pe_page = (arg_mask & LU_WANTS_PE_PAGE);
 	struct wb_desc *new_wbd = NULL;
@@ -2453,6 +2457,7 @@ static int prepare_l_entry_update(struct qcow2 *qcow2, struct qio *qio,
 		if (!new_wbd)
 			return -ENOMEM;
 		new_wbd->md = md;
+		new_wbd->lx_level = lx_level;
 	} else if (wants_pe_page && !md->wbd->pe_page) {
 		pe_page = alloc_page(GFP_NOIO|__GFP_ZERO);
 		if (!pe_page)
@@ -2515,7 +2520,8 @@ static int prepare_l1l2_allocation(struct qcow2 *qcow2, struct qio *qio,
 		/* Allocate cluster for L2 entries, and prepare L1 update */
 		ret = prepare_l_entry_update(qcow2, qio, map->l1.md,
 					     map->l1.index_in_page, &val,
-					     LU_SET_ONE_MASK|LU_WANTS_ALLOC);
+					     LU_SET_ONE_MASK | LU_WANTS_ALLOC,
+					     L1_LEVEL);
 		if (ret <= 0)
 			return ret;
 
@@ -2541,7 +2547,8 @@ static int prepare_l1l2_allocation(struct qcow2 *qcow2, struct qio *qio,
 
 		ret = prepare_l_entry_update(qcow2, qio, map->l2.md,
 					     map->l2.index_in_page,
-					     &map->data_clu_pos, arg_mask);
+					     &map->data_clu_pos, arg_mask,
+					     L2_LEVEL);
 		if (ret <= 0)
 			return ret;
 
@@ -2562,7 +2569,7 @@ static int prepare_l1l2_allocation(struct qcow2 *qcow2, struct qio *qio,
 
 	return prepare_l_entry_update(qcow2, qio, map->l2.md,
 				      map->l2.index_in_page + 1,
-				      &val, arg_mask);
+				      &val, arg_mask, L2_LEVEL);
 }
 
 /*
@@ -3979,7 +3986,7 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 		ret = prepare_l_entry_update(qcow2, qio, lx_md,
 					     ext->lx_index_in_page,
 					     &ext->allocated_clu_pos,
-					     arg_mask);
+					     arg_mask, ext->cow_level);
 		if (ret < 0) {
 			qio->bi_status = errno_to_blk_status(ret);
 			qio_endio(qio);
@@ -3990,7 +3997,8 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 			arg_mask &= ~LU_SET_ONE_MASK;
 			ret = prepare_l_entry_update(qcow2, qio, lx_md,
 					     ext->lx_index_in_page + 1,
-					   &ext->new_ext_l2, arg_mask);
+					     &ext->new_ext_l2, arg_mask,
+					     L2_LEVEL);
 			WARN_ON_ONCE(ret < 0);
 		}
 
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 3f65897ce9da2..0f1d3e3c5258e 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -174,7 +174,7 @@ void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2
 	int i;
 
 	/*
-	 * We need second iteration, since revert_clusters_alloc()
+	 * We need second iteration, since revert_l_entries_update()
 	 * may start timer again after failed wb.
 	 */
 	for (i = 0; i < 2; i++) {
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 86f0688e7345a..aa3487007523f 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -118,6 +118,7 @@ struct wb_desc {
 	struct list_head dependent_list;
 	int nr_submitted;
 	bool completed;
+	u8 lx_level;
 	int ret;
 };
 
-- 
2.43.5

[Devel] [PATCH VZ10 v3 03/11] drivers/md/dm-qcow2: generalize COW index update machinery

[Andrey Zhadchenko]

The two-stage L1/L2 entry update pipeline is not COW-specific:
backward merge already uses it to zero L2 entries, and upcoming
discard support will be the third user. Rename it to reflect
what it does.

No functional changes.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 104 +++++++++++++++++++-------------------
 drivers/md/dm-qcow2.h     |  10 ++--
 2 files changed, 56 insertions(+), 58 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 651730bd15901..83b540ccefee6 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2579,10 +2579,10 @@ static int prepare_l1l2_allocation(struct qcow2 *qcow2, struct qio *qio,
  * we have to wait all previous READs. We do that around
  * index wb. See md->wpc_noread_count update details.
  */
-static int prepare_l_entry_cow(struct qcow2 *qcow2, struct qcow2_map *map,
-			       struct qio *qio, struct md_page *md,
-			       u32 index_in_page, loff_t cow_clu_pos,
-			       loff_t cow_clu_end, u8 cow_level)
+static int prepare_l_entry_replace(struct qcow2 *qcow2, struct qcow2_map *map,
+				   struct qio *qio, struct md_page *md,
+				   u32 index_in_page, loff_t unuse_clu_pos,
+				   loff_t unuse_clu_end, u8 lx_level)
 {
 	struct lock_desc *lockd = NULL;
 	struct qio_ext *ext;
@@ -2591,9 +2591,9 @@ static int prepare_l_entry_cow(struct qcow2 *qcow2, struct qcow2_map *map,
 		return -ENOMEM;
 
 	ext = qio->ext;
-	ext->cow_clu_pos = cow_clu_pos;
-	ext->cow_clu_end = cow_clu_end;
-	ext->cow_level = cow_level;
+	ext->unuse_clu_pos = unuse_clu_pos;
+	ext->unuse_clu_end = unuse_clu_end;
+	ext->lx_level = lx_level;
 
 	spin_lock_irq(&qcow2->md_pages_lock);
 	if (!md->lockd) {
@@ -2614,23 +2614,23 @@ static int prepare_l_entry_cow(struct qcow2 *qcow2, struct qcow2_map *map,
 	return 1;
 }
 
-static int prepare_l1l2_cow(struct qcow2 *qcow2, struct qio *qio,
-			    struct qcow2_map *map)
+static int prepare_l1l2_replace(struct qcow2 *qcow2, struct qio *qio,
+				struct qcow2_map *map)
 {
 	if (WARN_ON_ONCE(!(map->level & L1_LEVEL)))
 		return -EIO; /* Sanity check: L1 must be cached */
 
 	if (!(map->level & L2_LEVEL)) {
-		return prepare_l_entry_cow(qcow2, map, qio, map->l1.md,
-					   map->l1.index_in_page,
-					   map->cow_clu_pos,
-					   map->cow_clu_end, L1_LEVEL);
+		return prepare_l_entry_replace(qcow2, map, qio, map->l1.md,
+					       map->l1.index_in_page,
+					       map->cow_clu_pos,
+					       map->cow_clu_end, L1_LEVEL);
 	}
 
-	return prepare_l_entry_cow(qcow2, map, qio, map->l2.md,
-				  map->l2.index_in_page,
-				  map->cow_clu_pos,
-				  map->cow_clu_end, L2_LEVEL);
+	return prepare_l_entry_replace(qcow2, map, qio, map->l2.md,
+				       map->l2.index_in_page,
+				       map->cow_clu_pos,
+				       map->cow_clu_end, L2_LEVEL);
 }
 
 static void backward_merge_write_complete(struct qcow2_target *tgt, struct qio *unused,
@@ -2648,7 +2648,7 @@ static void backward_merge_write_complete(struct qcow2_target *tgt, struct qio *
 	WARN_ON_ONCE(qio->flags & QIO_IS_DISCARD_FL);
 	qio->flags |= QIO_IS_DISCARD_FL;
 
-	qio->queue_list_id = QLIST_COW_INDEXES;
+	qio->queue_list_id = QLIST_INDEXES_WRITE;
 	qcow2_dispatch_qios(qcow2, qio, NULL);
 }
 
@@ -2704,7 +2704,7 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 	if (!op_is_write((*qio)->bi_op)) {
 		/*
 		 * READ qio may data may be contained in several deltas.
-		 * We can't read lower delta after prepare_l1l2_cow()
+		 * We can't read lower delta after prepare_l1l2_replace()
 		 * prepares us.
 		 */
 		aux_qio = qcow2_alloc_qio(qcow2->tgt->qio_pool, true);
@@ -2725,10 +2725,10 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 	}
 
 	/*
-	 * Mark as COW, as this completely defers any parallel qios.
-	 * @qio is COW status holder.
+	 * Lock the entry, as this completely defers any parallel qios.
+	 * @qio is the lock holder.
 	 */
-	ret = prepare_l1l2_cow(qcow2, *qio, map);
+	ret = prepare_l1l2_replace(qcow2, *qio, map);
 	if (ret < 0) {
 		(*qio)->bi_status = errno_to_blk_status(ret);
 		goto endio;
@@ -2736,13 +2736,13 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 
 	if (!map->clu_is_cow) {
 		/* Forced set these to unuse them after discard */
-		(*qio)->ext->cow_clu_pos = map->data_clu_pos;
-		(*qio)->ext->cow_clu_end = map->data_clu_pos + qcow2->clu_size;
+		(*qio)->ext->unuse_clu_pos = map->data_clu_pos;
+		(*qio)->ext->unuse_clu_end = map->data_clu_pos + qcow2->clu_size;
 	}
 
 	return 1;
 endio:
-	qio_endio(*qio); /* Breaks COW set in prepare_l1l2_cow() */
+	qio_endio(*qio); /* Releases the lock set in prepare_l1l2_replace() */
 	return 0;
 }
 
@@ -3348,7 +3348,7 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 		   (!qio_is_fully_alloced(qcow2, *qio, map) || map->clu_is_cow)) {
 		if (map->clu_is_cow) {
 			/* COW to compressed or shared with snapshot cluster */
-			ret = prepare_l1l2_cow(qcow2, *qio, map);
+			ret = prepare_l1l2_replace(qcow2, *qio, map);
 		} else if ((map->level & L2_LEVEL) &&
 		    qio_border_is_inside_unmapped_unit(qcow2, *qio, map) &&
 		    maybe_mapped_in_lower_delta(qcow2, *qio)) {
@@ -3358,7 +3358,7 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 			 * snapshots). Here is data COW on L2_LEVEL.
 			 */
 			map->backing_file_cow = true;
-			ret = prepare_l1l2_cow(qcow2, *qio, map);
+			ret = prepare_l1l2_replace(qcow2, *qio, map);
 		} else if (unlikely(op_is_discard((*qio)->bi_op) &&
 				    (map->level & L2_LEVEL))) {
 			if (!map->data_clu_alloced) {
@@ -3841,7 +3841,7 @@ static void cow_data_write_endio(struct qcow2_target *tgt, struct qio *unused,
 		qio->bi_status = bi_status;
 		qio_endio(qio);
 	} else {
-		qio->queue_list_id = QLIST_COW_INDEXES;
+		qio->queue_list_id = QLIST_INDEXES_WRITE;
 		qcow2_dispatch_qios(qcow2, qio, NULL);
 	}
 }
@@ -3889,7 +3889,7 @@ static void sliced_cow_data_write_complete(struct qcow2_target *tgt, struct qio
 		qio->bi_status = bi_status;
 		qio_endio(qio);
 	} else {
-		qio->queue_list_id = QLIST_COW_INDEXES;
+		qio->queue_list_id = QLIST_INDEXES_WRITE;
 		qcow2_dispatch_qios(qcow2, qio, NULL);
 	}
 }
@@ -3923,7 +3923,7 @@ static void process_cow_data_write(struct qcow2 *qcow2, struct list_head *cow_li
 		ext = qio->ext;
 
 		if (ext->only_set_ext_l2) {
-			WARN_ON_ONCE(ext->cow_level != L2_LEVEL);
+			WARN_ON_ONCE(ext->lx_level != L2_LEVEL);
 			pos = ext->allocated_clu_pos;
 			goto submit;
 		}
@@ -3942,17 +3942,16 @@ static void process_cow_data_write(struct qcow2 *qcow2, struct list_head *cow_li
 		ext->allocated_clu_pos = pos;
 		ext->cleanup_mask |= FREE_ALLOCATED_CLU;
 submit:
-		if (ext->cow_level == L2_LEVEL)
+		if (ext->lx_level == L2_LEVEL)
 			submit_sliced_cow_data_write(qcow2, qio, pos);
 		else
 			submit_cow_data_write(qcow2, qio, pos);
 	}
 }
 
-static void process_cow_indexes_write(struct qcow2 *qcow2,
-				      struct list_head *qio_list)
+static void process_indexes_write(struct qcow2 *qcow2,
+				  struct list_head *qio_list)
 {
-	struct qcow2_bvec *qvec;
 	struct md_page *lx_md;
 	struct qio_ext *ext;
 	struct qio *qio;
@@ -3965,11 +3964,10 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 		if (!qio)
 			break;
 		ext = qio->ext;
-		qvec = qio->data;
 		lx_md = ext->lx_md;
 
 		/* Return back to the same stage in case of writeback */
-		qio->queue_list_id = QLIST_COW_INDEXES;
+		qio->queue_list_id = QLIST_INDEXES_WRITE;
 		if (delay_if_writeback(qcow2, lx_md, -1, &qio, true))
 			continue;
 
@@ -3978,7 +3976,7 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 
 		arg_mask = (discard ? 0 : LU_SET_ONE_MASK) | LU_WANTS_PE_PAGE;
 		if (ext->only_set_ext_l2) {
-			WARN_ON_ONCE(ext->cow_level != L2_LEVEL);
+			WARN_ON_ONCE(ext->lx_level != L2_LEVEL);
 			goto set_ext_l2;
 		}
 
@@ -3986,14 +3984,14 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 		ret = prepare_l_entry_update(qcow2, qio, lx_md,
 					     ext->lx_index_in_page,
 					     &ext->allocated_clu_pos,
-					     arg_mask, ext->cow_level);
+					     arg_mask, ext->lx_level);
 		if (ret < 0) {
 			qio->bi_status = errno_to_blk_status(ret);
 			qio_endio(qio);
 			continue;
 		}
 set_ext_l2:
-		if (qcow2->ext_l2 && ext->cow_level == L2_LEVEL) {
+		if (qcow2->ext_l2 && ext->lx_level == L2_LEVEL) {
 			arg_mask &= ~LU_SET_ONE_MASK;
 			ret = prepare_l_entry_update(qcow2, qio, lx_md,
 					     ext->lx_index_in_page + 1,
@@ -4003,7 +4001,7 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 		}
 
 		/* Next stage */
-		qio->queue_list_id = QLIST_COW_END;
+		qio->queue_list_id = QLIST_INDEXES_END;
 
 		spin_lock_irq(&qcow2->md_pages_lock);
 		/*
@@ -4019,8 +4017,8 @@ static void process_cow_indexes_write(struct qcow2 *qcow2,
 	}
 }
 
-/* Finalize successful COW */
-static void process_cow_end(struct qcow2 *qcow2, struct list_head *qio_list)
+/* Finalize successful L1/L2 entry replace */
+static void process_indexes_end(struct qcow2 *qcow2, struct list_head *qio_list)
 {
 	struct dm_target *ti = qcow2->tgt->ti;
 	u32 mask, clu_size = qcow2->clu_size;
@@ -4040,7 +4038,7 @@ next:		qio = qio_list_pop(qio_list);
 			ext->cleanup_mask &= ~FREE_ALLOCATED_CLU;
 
 		/* Should be already set... */
-		qio->queue_list_id = QLIST_COW_END;
+		qio->queue_list_id = QLIST_INDEXES_END;
 		/*
 		 * Wait last user before we (possible) mark clusters
 		 * unused. In real only compressed COW requires this.
@@ -4048,8 +4046,8 @@ next:		qio = qio_list_pop(qio_list);
 		if (delay_if_has_wpc_readers(qcow2, ext->lx_md, &qio))
 			goto next;
 
-		pos = ext->cow_clu_pos;
-		for (; pos < ext->cow_clu_end; pos += clu_size) {
+		pos = ext->unuse_clu_pos;
+		for (; pos < ext->unuse_clu_end; pos += clu_size) {
 			ret = __handle_r1r2_maps(qcow2, pos, &qio, &r1, &r2);
 			if (ret == 0) /* We never shrink md pages, impossible */
 				goto next;
@@ -4057,7 +4055,7 @@ next:		qio = qio_list_pop(qio_list);
 				QC_ERR(ti, "clu at %lld leaked", pos);
 			else
 				dec_cluster_usage(qcow2, r2.md, r2.index_in_page, pos);
-			ext->cow_clu_pos += clu_size;
+			ext->unuse_clu_pos += clu_size;
 		}
 
 		mask = MD_INDEX_SET_UNLOCKED|DEC_WPC_NOREAD_COUNT;
@@ -4065,7 +4063,7 @@ next:		qio = qio_list_pop(qio_list);
 			mask |= FREE_QIO_DATA_QVEC;
 		WARN_ON_ONCE(ext->cleanup_mask != mask); /* Sanity check */
 
-		if (ext->cow_level == L1_LEVEL) {
+		if (ext->lx_level == L1_LEVEL) {
 			finalize_qio_ext(qio);
 			/* COW on L1 completed, it's time for COW on L2 */
 			qio->queue_list_id = QLIST_DEFERRED;
@@ -4105,8 +4103,8 @@ void do_qcow2_work(struct work_struct *ws)
 	LIST_HEAD(zread_qios);
 	LIST_HEAD(bwrite_qios);
 	LIST_HEAD(cow_data_qios);
-	LIST_HEAD(cow_indexes_qios);
-	LIST_HEAD(cow_end_qios);
+	LIST_HEAD(indexes_write_qios);
+	LIST_HEAD(indexes_end_qios);
 	LIST_HEAD(resubmit_qios);
 	LIST_HEAD(seek_qios);
 	unsigned int pflags = current->flags;
@@ -4118,8 +4116,8 @@ void do_qcow2_work(struct work_struct *ws)
 	list_splice_init(&qcow2->qios[QLIST_ZREAD], &zread_qios);
 	list_splice_init(&qcow2->qios[QLIST_BMERGE_WRITE], &bwrite_qios);
 	list_splice_init(&qcow2->qios[QLIST_COW_DATA], &cow_data_qios);
-	list_splice_init(&qcow2->qios[QLIST_COW_INDEXES], &cow_indexes_qios);
-	list_splice_init(&qcow2->qios[QLIST_COW_END], &cow_end_qios);
+	list_splice_init(&qcow2->qios[QLIST_INDEXES_WRITE], &indexes_write_qios);
+	list_splice_init(&qcow2->qios[QLIST_INDEXES_END], &indexes_end_qios);
 	list_splice_init(&qcow2->resubmit_qios, &resubmit_qios);
 	list_splice_init(&qcow2->qios[QLIST_SEEK], &seek_qios);
 	spin_unlock_irq(&qcow2->deferred_lock);
@@ -4129,8 +4127,8 @@ void do_qcow2_work(struct work_struct *ws)
 	process_compressed_read(&zread_qios, &cow_data_qios);
 	process_backward_merge_write(qcow2, &bwrite_qios);
 	process_cow_data_write(qcow2, &cow_data_qios);
-	process_cow_indexes_write(qcow2, &cow_indexes_qios);
-	process_cow_end(qcow2, &cow_end_qios);
+	process_indexes_write(qcow2, &indexes_write_qios);
+	process_indexes_end(qcow2, &indexes_end_qios);
 	process_resubmit_qios(qcow2, &resubmit_qios);
 	process_seek_qios(qcow2, &seek_qios);
 
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index aa3487007523f..230e7a4a34e76 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -228,8 +228,8 @@ enum {
 	QLIST_ZREAD,
 	QLIST_BMERGE_WRITE,
 	QLIST_COW_DATA,
-	QLIST_COW_INDEXES,
-	QLIST_COW_END,
+	QLIST_INDEXES_WRITE,
+	QLIST_INDEXES_END,
 	QLIST_SEEK,
 
 	QLIST_COUNT,
@@ -307,13 +307,13 @@ struct qio_ext {
 	u32 lx_index_in_page, r2_index_in_page;
 	u64 allocated_clu_pos;
 
-	loff_t cow_clu_pos;
-	loff_t cow_clu_end;
+	loff_t unuse_clu_pos;
+	loff_t unuse_clu_end;
 	u64 new_ext_l2;
 	u32 cow_mask;
 	bool only_set_ext_l2:1;
 
-	u8 cow_level;
+	u8 lx_level;
 
 #define MD_INDEX_SET_UNLOCKED	(1ULL << 0)
 #define DEC_WPC_NOREAD_COUNT	(1ULL << 1)
-- 
2.43.5

[Devel] [PATCH VZ10 v3 04/11] drivers/md/dm-qcow2: update metadata on whole cluster discard

[Andrey Zhadchenko]

Discard of a mapped cluster used to only punch a hole in the image
file: the L2 entry and refcounts were left untouched, so the cluster
remained allocated in qcow2 metadata forever.

Handle discards covering a whole cluster via the L1/L2 entry replace
machinery: prepare_cluster_discard() locks the L2 entry, then
issue_discard() punches the data cluster out of the image file and
queues the qio to write the zeroed L2 entry (and extended L2 bitmap)
in process_indexes_write(). Refcounts of the discarded cluster are
decremented in process_indexes_end() after the L2 writeback, like COW
does with its source clusters. Clusters shared with internal
snapshots or holding compressed data are not punched: only their
usage count is decremented.

Also teach revert_l_entries_update() that a changed index may contain
a discard-cleared entry, which has no allocation to revert.

Partial cluster discards keep the previous behavior: punch a hole
without touching metadata.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 110 +++++++++++++++++++++++++++++++++++---
 drivers/md/dm-qcow2.h     |   2 +-
 2 files changed, 104 insertions(+), 8 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 83b540ccefee6..a1bd36a112170 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -101,6 +101,12 @@ static loff_t bio_sector_to_file_pos(struct qcow2 *qcow2, struct qio *qio,
 	return map->data_clu_pos + bytes_off_in_cluster(qcow2, qio);
 }
 
+static bool qio_covers_full_clu(struct qcow2 *qcow2, struct qio *qio)
+{
+	return bytes_off_in_cluster(qcow2, qio) == 0 &&
+	       qio->bi_iter.bi_size == qcow2->clu_size;
+}
+
 static loff_t compressed_clu_end_pos(loff_t start, sector_t compressed_sectors)
 {
 	if (start % SECTOR_SIZE == 0)
@@ -1324,6 +1330,8 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
 		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 (!pos)
+			continue; /* no cluster was allocated */
 
 		spin_unlock(&qcow2->md_pages_lock);
 		pos &= ~LX_REFCOUNT_EXACTLY_ONE;
@@ -2015,7 +2023,12 @@ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio,
 		return ret;
 
 	map->data_clu_pos = pos;
-	if (!write || !map->clu_is_cow)
+	if (!write)
+		return 0;
+
+	/* discards also need to update r1r2 */
+	if (!map->clu_is_cow &&
+	    !(op_is_discard((*qio)->bi_op) && qio_covers_full_clu(qcow2, *qio)))
 		return 0;
 
 	/* Now refcounters table/block */
@@ -3317,12 +3330,86 @@ static void issue_discard(struct qcow2_map *map, struct qio *qio)
 	int ret;
 
 	WARN_ON_ONCE(!(map->level & L2_LEVEL));
-	pos = bio_sector_to_file_pos(qcow2, qio, map);
-	ret = qcow2_punch_hole(qcow2->file, pos, qio->bi_iter.bi_size);
 
-	if (ret)
-		qio->bi_status = errno_to_blk_status(ret);
-	qio_endio(qio);
+	if (!map->clu_is_cow) {
+		pos = bio_sector_to_file_pos(qcow2, qio, map);
+		ret = qcow2_punch_hole(qcow2->file, pos, qio->bi_iter.bi_size);
+
+		if (ret) {
+			qio->bi_status = errno_to_blk_status(ret);
+			qio_endio(qio);
+			return;
+		}
+	}
+
+	/* Clear metadata if needed */
+	if (qio->flags & QIO_IS_DISCARD_FL) {
+		qio->queue_list_id = QLIST_INDEXES_WRITE;
+		qcow2_dispatch_qios(qcow2, qio, NULL);
+	} else {
+		qio_endio(qio);
+	}
+}
+
+static bool qio_discard_updates_metadata(struct qcow2 *qcow2, struct qio *qio,
+					 struct qcow2_map *map)
+{
+	if (!(map->level & L2_LEVEL) || !map->data_clu_alloced)
+		return false;
+	if (qio_covers_full_clu(qcow2, qio))
+		return true;
+	return false;
+}
+
+/*
+ * Discard changes metadata: prepare L2 entry update. It gets locked
+ * here, new value is written in process_indexes_write(), and
+ * refcounts are handled after the L2 writeback in process_indexes_end().
+ *
+ * Discard covering the whole cluster replaces the entry and unuses
+ * the discarded (or COW source) cluster.
+ */
+static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
+				   struct qcow2_map *map)
+{
+	u32 index_in_page = map->l2.index_in_page;
+	struct md_page *md = map->l2.md;
+	loff_t unuse_pos, unuse_end;
+	struct qio_ext *ext;
+	int ret;
+
+	WARN_ON_ONCE(!(map->level & L2_LEVEL) || !map->data_clu_alloced);
+
+	spin_lock_irq(&qcow2->md_pages_lock);
+	if (delay_if_dirty(qcow2, md, index_in_page, qio) ||
+	    __delay_if_writeback(qcow2, md, index_in_page, qio, true) ||
+	    (qcow2->ext_l2 &&
+	     delay_if_dirty(qcow2, md, index_in_page + 1, qio))) {
+		spin_unlock_irq(&qcow2->md_pages_lock);
+		return 0;
+	}
+	spin_unlock_irq(&qcow2->md_pages_lock);
+
+	if (map->clu_is_cow) {
+		/* Cluster is shared or compressed. Decrement refcount. */
+		unuse_pos = map->cow_clu_pos;
+		unuse_end = map->cow_clu_end;
+	} else {
+		/* Nobody else refers the cluster: unuse it after discard */
+		unuse_pos = map->data_clu_pos;
+		unuse_end = map->data_clu_pos + qcow2->clu_size;
+	}
+
+	ret = prepare_l_entry_replace(qcow2, map, *qio, md, index_in_page,
+				      unuse_pos, unuse_end, L2_LEVEL);
+	if (ret <= 0)
+		return ret;
+
+	ext = (*qio)->ext;
+	ext->lx_md = md;
+
+	(*qio)->flags |= QIO_IS_DISCARD_FL;
+	return 1;
 }
 
 static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
@@ -3344,6 +3431,9 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 		/* Nothing to COW or L1 is mapped exactly once */
 		qio_endio(*qio);
 		ret = 0;
+	} else if (unlikely(op_is_discard((*qio)->bi_op)) &&
+		   qio_discard_updates_metadata(qcow2, *qio, map)) {
+		ret = prepare_cluster_discard(qcow2, qio, map);
 	} else if (write &&
 		   (!qio_is_fully_alloced(qcow2, *qio, map) || map->clu_is_cow)) {
 		if (map->clu_is_cow) {
@@ -3365,7 +3455,7 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 				qio_endio(*qio);
 				ret = 0;
 			}
-			/* Otherwise issue_discard(). TODO: update L2 */
+			/* Otherwise issue_discard(). */
 		} else {
 			/* Wants L1 or L2 entry allocation */
 			ret = prepare_l1l2_allocation(qcow2, *qio, map);
@@ -3479,6 +3569,12 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
 
 	write = op_is_write(qio->bi_op);
 
+	/* Discard with prepared metadata update, see prepare_cluster_discard() */
+	if (unlikely(qio->flags & QIO_IS_DISCARD_FL)) {
+		issue_discard(&map, qio);
+		return;
+	}
+
 	if (unlikely(map.compressed)) {
 		/* Compressed qio never uses sub-clus */
 		submit_read_compressed(&map, qio, write);
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 230e7a4a34e76..8a24e04130e4d 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -340,7 +340,7 @@ struct qio {
 	blk_status_t bi_status;
 #define QIO_FREE_ON_ENDIO_FL	(1 << 0) /* Free this qio memory from qio_endio() */
 #define QIO_IS_MERGE_FL		(1 << 3) /* This is service merge qio */
-#define QIO_IS_DISCARD_FL	(1 << 4) /* This zeroes index on backward merge */
+#define QIO_IS_DISCARD_FL	(1 << 4) /* This zeroes index (discard or backward merge) */
 #define QIO_IS_L1COW_FL		(1 << 5) /* This qio only wants COW at L1 */
 #define QIO_SPLIT_INHERITED_FLAGS (QIO_IS_DISCARD_FL)
 	u8 flags;
-- 
2.43.5

[Devel] [PATCH VZ10 v3 05/11] drivers/md/dm-qcow2: never trigger COW or allocation on discard

[Andrey Zhadchenko]

In handle_metadata() the partial cluster discard fallback is checked
only after the COW branches. As a result a discard, whose range
touches a shared (snapshot or compressed) cluster or crosses an
unmapped unit border over a backing file, is routed into the data COW
machinery. COW then tries to submit the discard payload as a data
write, but discard qios carry no bvec array: the iov_iter is built
from garbage, which triggers the WARN_ON() in __submit_rw_mapped()
and writes random kernel memory into the COW'ed cluster.

Check for discard before the COW branches: discard is advisory and
carries no data, so it must never COW or allocate. End discard
early while handling metadata if possible. Otherwise do discard
operations before COWs.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index a1bd36a112170..a2e5c37adb7b9 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -3431,9 +3431,18 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 		/* Nothing to COW or L1 is mapped exactly once */
 		qio_endio(*qio);
 		ret = 0;
-	} else if (unlikely(op_is_discard((*qio)->bi_op)) &&
-		   qio_discard_updates_metadata(qcow2, *qio, map)) {
-		ret = prepare_cluster_discard(qcow2, qio, map);
+	} else if (unlikely(op_is_discard((*qio)->bi_op))) {
+		/*
+		 * Discard carries no data and is advisory. If it does not
+		 * trigger metadata changes or to-be-discarded cluster
+		 * is not present, end early.
+		 */
+		if (qio_discard_updates_metadata(qcow2, *qio, map)) {
+			ret = prepare_cluster_discard(qcow2, qio, map);
+		} else if (!map->data_clu_alloced || map->clu_is_cow) {
+			qio_endio(*qio);
+			ret = 0;
+		}
 	} else if (write &&
 		   (!qio_is_fully_alloced(qcow2, *qio, map) || map->clu_is_cow)) {
 		if (map->clu_is_cow) {
@@ -3449,13 +3458,6 @@ static int handle_metadata(struct qcow2 *qcow2, struct qio **qio,
 			 */
 			map->backing_file_cow = true;
 			ret = prepare_l1l2_replace(qcow2, *qio, map);
-		} else if (unlikely(op_is_discard((*qio)->bi_op) &&
-				    (map->level & L2_LEVEL))) {
-			if (!map->data_clu_alloced) {
-				qio_endio(*qio);
-				ret = 0;
-			}
-			/* Otherwise issue_discard(). */
 		} else {
 			/* Wants L1 or L2 entry allocation */
 			ret = prepare_l1l2_allocation(qcow2, *qio, map);
@@ -3569,8 +3571,8 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
 
 	write = op_is_write(qio->bi_op);
 
-	/* Discard with prepared metadata update, see prepare_cluster_discard() */
-	if (unlikely(qio->flags & QIO_IS_DISCARD_FL)) {
+	/* Process discard early. Don't do COW just to discard */
+	if (unlikely(op_is_discard(qio->bi_op))) {
 		issue_discard(&map, qio);
 		return;
 	}
@@ -3588,8 +3590,6 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
 			submit_read_whole_cow_clu(&map, qio);
 		else if (unlikely(map.clu_is_cow || map.backing_file_cow))
 			submit_read_sliced_cow_clu(&map, qio);
-		else if (unlikely(op_is_discard(qio->bi_op)))
-			issue_discard(&map, qio);
 		else
 			perform_rw_mapped(&map, qio);
 	}
-- 
2.43.5

[Devel] [PATCH VZ10 v3 06/11] drivers/md/dm-qcow2: set L2_READS_ALL_ZEROES after some discards

[Andrey Zhadchenko]

Discard does not guarantee zero data. Therefore, to safely erase
data, users may write zeroes and then discard. Imagine we have
a backing file. If we write zeroes and do the discard, next read
will give the stale data from backing image.
Probably this is rather an edge case, but to be sure let's just
set L2 entry to L2_READS_ALL_ZEROES so the device looks more
consistent to users.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 24 +++++++++++++++++++++---
 drivers/md/dm-qcow2.h     |  1 +
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index a2e5c37adb7b9..390331554b7ac 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1309,6 +1309,7 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
 	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;
@@ -1317,6 +1318,8 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
 	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. */
@@ -1330,7 +1333,7 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
 		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 (!pos)
+		if (cleared)
 			continue; /* no cluster was allocated */
 
 		spin_unlock(&qcow2->md_pages_lock);
@@ -3368,12 +3371,16 @@ static bool qio_discard_updates_metadata(struct qcow2 *qcow2, struct qio *qio,
  *
  * Discard covering the whole cluster replaces the entry and unuses
  * the discarded (or COW source) cluster.
+ * If the backing is present, set 'reads as zeroes' to avoid exposing
+ * stale data.
  */
 static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 				   struct qcow2_map *map)
 {
+	bool zeroes = maybe_mapped_in_lower_delta(qcow2, *qio);
 	u32 index_in_page = map->l2.index_in_page;
 	struct md_page *md = map->l2.md;
+	u64 new_ext_l2 = map->ext_l2;
 	loff_t unuse_pos, unuse_end;
 	struct qio_ext *ext;
 	int ret;
@@ -3399,6 +3406,8 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 		unuse_pos = map->data_clu_pos;
 		unuse_end = map->data_clu_pos + qcow2->clu_size;
 	}
+	if (zeroes && qcow2->ext_l2)
+		new_ext_l2 = (u64)U32_MAX << 32;
 
 	ret = prepare_l_entry_replace(qcow2, map, *qio, md, index_in_page,
 				      unuse_pos, unuse_end, L2_LEVEL);
@@ -3408,6 +3417,10 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	ext = (*qio)->ext;
 	ext->lx_md = md;
 
+	ext->new_ext_l2 = new_ext_l2;
+	if (zeroes && !qcow2->ext_l2)
+		ext->set_all_zeroes = true;
+
 	(*qio)->flags |= QIO_IS_DISCARD_FL;
 	return 1;
 }
@@ -4053,6 +4066,7 @@ static void process_indexes_write(struct qcow2 *qcow2,
 	struct qio *qio;
 	bool discard;
 	u32 arg_mask;
+	u64 entry;
 	int ret;
 
 	while (1) {
@@ -4076,11 +4090,15 @@ static void process_indexes_write(struct qcow2 *qcow2,
 			goto set_ext_l2;
 		}
 
+		entry = ext->allocated_clu_pos;
+		if (unlikely(ext->set_all_zeroes))
+			entry = L2_READS_ALL_ZEROES;
+
 		/* XXX: check prealloced_pos ==> revert */
 		ret = prepare_l_entry_update(qcow2, qio, lx_md,
 					     ext->lx_index_in_page,
-					     &ext->allocated_clu_pos,
-					     arg_mask, ext->lx_level);
+					     &entry, arg_mask,
+					     ext->lx_level);
 		if (ret < 0) {
 			qio->bi_status = errno_to_blk_status(ret);
 			qio_endio(qio);
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 8a24e04130e4d..0f006f1ae48cc 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -312,6 +312,7 @@ struct qio_ext {
 	u64 new_ext_l2;
 	u32 cow_mask;
 	bool only_set_ext_l2:1;
+	bool set_all_zeroes:1;
 
 	u8 lx_level;
 
-- 
2.43.5

[Devel] [PATCH VZ10 v3 07/11] drivers/md/dm-qcow2: update metadata on subclusters discard

[Andrey Zhadchenko]

On images with extended L2 entries a discard smaller than the whole
cluster only punches a hole in the image file, while the subcluster
allocation bitmap still reports the range as allocated.

Extend prepare_cluster_discard() to also handle discards covering
whole subclusters of a mapped non-compressed cluster: clear the
"allocated" bits of the covered subclusters in the extended L2 entry
via the existing only_set_ext_l2 machinery. If the range may be
mapped in lower delta, set the "reads as zeroes" bits instead, so the
discarded range doesn't expose stale lower data. The cluster itself
remains allocated, so refcounts are not touched and the unuse stage
has nothing to do (empty unuse range). The covered range is hole
punched unless the cluster is shared with a snapshot.

ext_l2 and unuse range handling are in different if blocks for a while:
next patch will cover the case when we need uncoupling.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 71 ++++++++++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 12 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 390331554b7ac..2e83bd995684d 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -107,6 +107,21 @@ static bool qio_covers_full_clu(struct qcow2 *qcow2, struct qio *qio)
 	       qio->bi_iter.bi_size == qcow2->clu_size;
 }
 
+/* Mask of subclusters fully covered by qio (qio is trimmed inward) */
+static u32 qio_full_subclus_mask(struct qcow2 *qcow2, struct qio *qio)
+{
+	u32 off = bytes_off_in_cluster(qcow2, qio);
+	u32 first_bit = DIV_ROUND_UP(off, qcow2->subclu_size);
+	u32 end_bit = (off + qio->bi_iter.bi_size) / qcow2->subclu_size;
+
+	WARN_ON_ONCE(!qcow2->ext_l2);
+
+	if (end_bit <= first_bit)
+		return 0;
+
+	return GENMASK(end_bit - 1, first_bit);
+}
+
 static loff_t compressed_clu_end_pos(loff_t start, sector_t compressed_sectors)
 {
 	if (start % SECTOR_SIZE == 0)
@@ -3357,11 +3372,21 @@ static void issue_discard(struct qcow2_map *map, struct qio *qio)
 static bool qio_discard_updates_metadata(struct qcow2 *qcow2, struct qio *qio,
 					 struct qcow2_map *map)
 {
+	u32 mask;
+
 	if (!(map->level & L2_LEVEL) || !map->data_clu_alloced)
 		return false;
 	if (qio_covers_full_clu(qcow2, qio))
 		return true;
-	return false;
+	if (!qcow2->ext_l2 || map->compressed)
+		return false;
+
+	mask = qio_full_subclus_mask(qcow2, qio);
+	if ((u32)map->ext_l2 & mask)
+		return true;
+
+	return maybe_mapped_in_lower_delta(qcow2, qio) &&
+	       (mask & ~(u32)(map->ext_l2 >> 32));
 }
 
 /*
@@ -3371,6 +3396,9 @@ static bool qio_discard_updates_metadata(struct qcow2 *qcow2, struct qio *qio,
  *
  * Discard covering the whole cluster replaces the entry and unuses
  * the discarded (or COW source) cluster.
+ * Subclusters fully covered by the discard are marked unallocated in
+ * ext_l2 bitmap: the cluster itself remains allocated, so refcounts
+ * are not touched (hence empty unuse range).
  * If the backing is present, set 'reads as zeroes' to avoid exposing
  * stale data.
  */
@@ -3380,8 +3408,9 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	bool zeroes = maybe_mapped_in_lower_delta(qcow2, *qio);
 	u32 index_in_page = map->l2.index_in_page;
 	struct md_page *md = map->l2.md;
-	u64 new_ext_l2 = map->ext_l2;
 	loff_t unuse_pos, unuse_end;
+	bool whole_clu;
+	u64 new_ext_l2 = 0;
 	struct qio_ext *ext;
 	int ret;
 
@@ -3397,17 +3426,33 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	}
 	spin_unlock_irq(&qcow2->md_pages_lock);
 
-	if (map->clu_is_cow) {
-		/* Cluster is shared or compressed. Decrement refcount. */
-		unuse_pos = map->cow_clu_pos;
-		unuse_end = map->cow_clu_end;
+	whole_clu = qio_covers_full_clu(qcow2, *qio);
+	if (whole_clu) {
+		if (zeroes && qcow2->ext_l2)
+			new_ext_l2 = (u64)U32_MAX << 32;
+	} else {
+		u64 mask = qio_full_subclus_mask(qcow2, *qio);
+
+		new_ext_l2 = map->ext_l2 & ~(mask << 32 | mask);
+		if (zeroes)
+			new_ext_l2 |= mask << 32;
+	}
+
+	if (whole_clu) {
+		if (map->clu_is_cow) {
+			/* Cluster is shared or compressed. Decrement refcount. */
+			unuse_pos = map->cow_clu_pos;
+			unuse_end = map->cow_clu_end;
+		} else {
+			/* Nobody else refers the cluster: unuse it */
+			unuse_pos = map->data_clu_pos;
+			unuse_end = map->data_clu_pos + qcow2->clu_size;
+		}
 	} else {
-		/* Nobody else refers the cluster: unuse it after discard */
-		unuse_pos = map->data_clu_pos;
-		unuse_end = map->data_clu_pos + qcow2->clu_size;
+		/* Subclusters become unallocated, cluster remains */
+		unuse_pos = 0;
+		unuse_end = 0;
 	}
-	if (zeroes && qcow2->ext_l2)
-		new_ext_l2 = (u64)U32_MAX << 32;
 
 	ret = prepare_l_entry_replace(qcow2, map, *qio, md, index_in_page,
 				      unuse_pos, unuse_end, L2_LEVEL);
@@ -3418,7 +3463,9 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	ext->lx_md = md;
 
 	ext->new_ext_l2 = new_ext_l2;
-	if (zeroes && !qcow2->ext_l2)
+	if (!whole_clu)
+		ext->only_set_ext_l2 = true;
+	else if (zeroes && !qcow2->ext_l2)
 		ext->set_all_zeroes = true;
 
 	(*qio)->flags |= QIO_IS_DISCARD_FL;
-- 
2.43.5

[Devel] [PATCH VZ10 v3 08/11] drivers/md/dm-qcow2: unmap cluster when discard clears last allocated subclusters

[Andrey Zhadchenko]

A subclusters discard only clears bits in the extended L2 bitmap and
keeps the cluster mapped. Teach dm-qcow2 to clear L2 entry if every
subcluster is discarded.

Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-139406
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 2e83bd995684d..aa11956ac1d8c 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -122,6 +122,17 @@ static u32 qio_full_subclus_mask(struct qcow2 *qcow2, struct qio *qio)
 	return GENMASK(end_bit - 1, first_bit);
 }
 
+static bool qio_discard_unmaps_cluster(struct qcow2 *qcow2, struct qio *qio,
+				       struct qcow2_map *map)
+{
+	if (qio_covers_full_clu(qcow2, qio))
+		return true;
+	if (!qcow2->ext_l2 || map->compressed)
+		return false;
+	return (u32)map->ext_l2 &&
+	       !((u32)map->ext_l2 & ~qio_full_subclus_mask(qcow2, qio));
+}
+
 static loff_t compressed_clu_end_pos(loff_t start, sector_t compressed_sectors)
 {
 	if (start % SECTOR_SIZE == 0)
@@ -2044,9 +2055,10 @@ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio,
 	if (!write)
 		return 0;
 
-	/* discards also need to update r1r2 */
+	/* cluster unmapping discards also need to update r1r2 */
 	if (!map->clu_is_cow &&
-	    !(op_is_discard((*qio)->bi_op) && qio_covers_full_clu(qcow2, *qio)))
+	    !(op_is_discard((*qio)->bi_op) &&
+	      qio_discard_unmaps_cluster(qcow2, *qio, map)))
 		return 0;
 
 	/* Now refcounters table/block */
@@ -3399,6 +3411,8 @@ static bool qio_discard_updates_metadata(struct qcow2 *qcow2, struct qio *qio,
  * Subclusters fully covered by the discard are marked unallocated in
  * ext_l2 bitmap: the cluster itself remains allocated, so refcounts
  * are not touched (hence empty unuse range).
+ * Discarding last subclusters in a cluster clears it from L2 like a
+ * whole cluster discard.
  * If the backing is present, set 'reads as zeroes' to avoid exposing
  * stale data.
  */
@@ -3409,7 +3423,7 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	u32 index_in_page = map->l2.index_in_page;
 	struct md_page *md = map->l2.md;
 	loff_t unuse_pos, unuse_end;
-	bool whole_clu;
+	bool whole_clu, unmap;
 	u64 new_ext_l2 = 0;
 	struct qio_ext *ext;
 	int ret;
@@ -3427,6 +3441,8 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	spin_unlock_irq(&qcow2->md_pages_lock);
 
 	whole_clu = qio_covers_full_clu(qcow2, *qio);
+	unmap = qio_discard_unmaps_cluster(qcow2, *qio, map);
+
 	if (whole_clu) {
 		if (zeroes && qcow2->ext_l2)
 			new_ext_l2 = (u64)U32_MAX << 32;
@@ -3438,7 +3454,7 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 			new_ext_l2 |= mask << 32;
 	}
 
-	if (whole_clu) {
+	if (unmap) {
 		if (map->clu_is_cow) {
 			/* Cluster is shared or compressed. Decrement refcount. */
 			unuse_pos = map->cow_clu_pos;
@@ -3463,7 +3479,7 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
 	ext->lx_md = md;
 
 	ext->new_ext_l2 = new_ext_l2;
-	if (!whole_clu)
+	if (!unmap)
 		ext->only_set_ext_l2 = true;
 	else if (zeroes && !qcow2->ext_l2)
 		ext->set_all_zeroes = true;
-- 
2.43.5

[Devel] [PATCH VZ10 v3 09/11] drivers/md/dm-qcow2: do discards during backward merge only for writable image

[Andrey Zhadchenko]

Backward merge discards every merged cluster from the image we merge
from: the L2 entry is zeroed and refcounts are decremented, which
dirties the image metadata. This requires the image file to be opened
for write, while merge-in-the-middle images usually opened read-only.

Skip the discard step when the merged image is read-only:
 - complete the merge qio right after its data is written to the lower
delta, without the L1/L2 entry update;
 - process READs on such image in the regular way. Previously reads would
cause out-of-order merge for present cluster and then requeue. Without
discard it will loop.
 - do not break COW at L1. Note that due to how merge machinery works,
we can't merge without unuse and internal snaphots (to be addressed
in the next patches).
 - do not clear the dirty bit on merge completion: the image was never
modified.

Just in case add warning and end qio if we somehow encounter non-service
write qio for read-only images during the merge.

https://virtuozzo.atlassian.net/browse/VSTOR-138288
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-cmd.c | 21 +++++++++++++--------
 drivers/md/dm-qcow2-map.c | 27 ++++++++++++++++++++++++++-
 drivers/md/dm-qcow2.h     |  5 +++++
 3 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index c15c46a0fe8b7..fa3762c58372b 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -264,7 +264,7 @@ static int qcow2_merge_backward_start(struct qcow2_target *tgt, int efd, u32 dep
 	lower = qcow2->lower;
 	if (!lower)
 		return -ENOENT;
-	if (!(lower->file->f_mode & FMODE_WRITE))
+	if (!qcow2_file_is_writable(lower))
 		return -EACCES;
 	if (qcow2->clu_size != lower->clu_size)
 		return -EOPNOTSUPP;
@@ -315,11 +315,14 @@ void qcow2_merge_backward_work(struct work_struct *work)
 	 * there would be problems with unusing them:
 	 * we'd have to freeze IO going to all data clusters
 	 * under every L1 entry related to several snapshots.
+	 * Readonly images skip this stage.
 	 */
-	ret = qcow2_break_l1cow(tgt, qcow2);
-	if (ret) {
-		QC_ERR(tgt->ti, "Can't break L1 COW");
-		goto out_err;
+	if (qcow2_file_is_writable(qcow2)) {
+		ret = qcow2_break_l1cow(tgt, qcow2);
+		if (ret) {
+			QC_ERR(tgt->ti, "Can't break L1 COW");
+			goto out_err;
+		}
 	}
 
 	backward_merge_update_stage(tgt, BACKWARD_MERGE_STAGE_SET_DIRTY);
@@ -388,9 +391,11 @@ static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
 	qcow2_flush_deferred_activity(tgt, qcow2); /* Delayed md pages */
 	qcow2->lower = NULL;
 
-	ret = qcow2_set_image_file_features(qcow2, false);
-	if (ret < 0)
-		QC_ERR(tgt->ti, "Can't unuse merged img (%d)", ret);
+	if (qcow2_file_is_writable(qcow2)) {
+		ret = qcow2_set_image_file_features(qcow2, false);
+		if (ret < 0)
+			QC_ERR(tgt->ti, "Can't unuse merged img (%d)", ret);
+	}
 	qcow2_destroy(qcow2);
 
 	tgt->backward_merge.state = BACKWARD_MERGE_STOPPED;
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index aa11956ac1d8c..c790b2ad3787e 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2688,6 +2688,12 @@ static void backward_merge_write_complete(struct qcow2_target *tgt, struct qio *
 		return;
 	}
 
+	/* Skip discard for read-only source images */
+	if (!qcow2_file_is_writable(qcow2)) {
+		qio_endio(qio);
+		return;
+	}
+
 	WARN_ON_ONCE(qio->flags & QIO_IS_DISCARD_FL);
 	qio->flags |= QIO_IS_DISCARD_FL;
 
@@ -2731,6 +2737,17 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 	struct qio *aux_qio;
 	int ret;
 
+	/* Readonly image mappings remain stable, so reads just go through */
+	if (!qcow2_file_is_writable(qcow2)) {
+		if (!op_is_write((*qio)->bi_op))
+			return 1;
+		if (WARN_ON_ONCE(!fake_merge_qio(*qio))) {
+			(*qio)->bi_status = BLK_STS_IOERR;
+			qio_endio(*qio);
+			return 0;
+		}
+	}
+
 	if (!map->data_clu_alloced) {
 		WARN_ON_ONCE(map->clu_is_cow); /* Strange COW at L1 */
 		if (fake_merge_qio(*qio)) {
@@ -3640,7 +3657,15 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
 	if (!handle_metadata(qcow2, &qio, &map))
 		return;
 
-	if (unlikely(qcow2->backward_merge_in_process)) {
+	/*
+	 * Merge machinery makes out of order merges for present
+	 * clusters when it sees the reads. But if the merge does
+	 * not discard the cluser mapping, it will spin endlessly.
+	 * So process only actual merge qios or reads from writable
+	 * images.
+	 */
+	if (unlikely(qcow2->backward_merge_in_process) &&
+	    (fake_merge_qio(qio) || qcow2_file_is_writable(qcow2))) {
 		submit_top_delta_read(&map, qio);
 		return;
 	}
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index 0f006f1ae48cc..d9b8c38e093f5 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -460,6 +460,11 @@ static inline bool qcow2_wants_check(struct qcow2_target *tgt)
 	return !!(tgt->md_writeback_error|tgt->truncate_error);
 }
 
+static inline bool qcow2_file_is_writable(struct qcow2 *qcow2)
+{
+	return qcow2->file->f_mode & FMODE_WRITE;
+}
+
 static inline void remap_to_clu(struct qcow2 *qcow2, struct qio *qio, loff_t clu_pos)
 {
 	qio->bi_iter.bi_sector &= (to_sector(qcow2->clu_size) - 1);
-- 
2.43.5

[Devel] [PATCH VZ10 v3 10/11] drivers/md/dm-qcow2: allow shared L1 entries during merge from RO image

[Andrey Zhadchenko]

For RO images with internal snapshots L1 entries are shared.
Write-mode metadata parsing stops at such entries to avoid
modifying a shared L2 table, which made prepare_backward_merge()
see the cluster as unallocated and skip it with
"nothing to merge": the merged result would silently lose all
data under shared L1 entries.
Allow such scenario in parse_l1() and ease WARN in
prepare_backward_merge().

https://virtuozzo.atlassian.net/browse/VSTOR-138288
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index c790b2ad3787e..41a504c65b722 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1726,6 +1726,14 @@ static bool qio_is_fully_alloced(struct qcow2 *qcow2, struct qio *qio,
 	return !(subclus_mask & ~alloced_mask);
 }
 
+static bool qio_may_modify_image(struct qcow2 *qcow2, struct qio *qio)
+{
+	if (qcow2_file_is_writable(qcow2))
+		return true;
+
+	return !fake_merge_qio(qio);
+}
+
 static loff_t parse_l1(struct qcow2 *qcow2, struct qcow2_map *map,
 		       struct qio **qio, bool write)
 
@@ -1762,7 +1770,8 @@ static loff_t parse_l1(struct qcow2 *qcow2, struct qcow2_map *map,
 		goto out;
 	if (delay_if_dirty(qcow2, l1->md, l1->index_in_page, qio))
 		goto out;
-	if (write && map->clu_is_cow)
+	/* Don't refuse L1 parse for merge qios with readonly disks */
+	if (write && map->clu_is_cow && qio_may_modify_image(qcow2, *qio))
 		goto out; /* Avoid to return pos */
 
 	ret = pos;
@@ -2061,6 +2070,10 @@ static int parse_metadata(struct qcow2 *qcow2, struct qio **qio,
 	      qio_discard_unmaps_cluster(qcow2, *qio, map)))
 		return 0;
 
+	/* Don't need refcount table if we don't modify the image */
+	if (!qio_may_modify_image(qcow2, *qio))
+		return 0;
+
 	/* Now refcounters table/block */
 	ret = qcow2_handle_r1r2_maps(qcow2, pos, qio, &map->r1,
 			       &map->r2, map->compressed);
@@ -2749,7 +2762,8 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 	}
 
 	if (!map->data_clu_alloced) {
-		WARN_ON_ONCE(map->clu_is_cow); /* Strange COW at L1 */
+		/* Strange COW at L1, except the merge from RO image */
+		WARN_ON_ONCE(map->clu_is_cow && qio_may_modify_image(qcow2, *qio));
 		if (fake_merge_qio(*qio)) {
 			/* Nothing is to merge */
 			goto endio;
-- 
2.43.5

[Devel] [PATCH VZ10 v3 11/11] drivers/md/dm-qcow2: respect zeroes during merge

[Andrey Zhadchenko]

The merge machinery used to skip unallocated clusters, therefore
breaking all clusters with zero bits and non-zero backing.
Technically this bug was present even before discard changes, as
qcow2 image could have 'reads as zero' bits before being inserted
into dm-qcow2.
Properly commit all zero clusters (albeit with data for now).

https://virtuozzo.atlassian.net/browse/VSTOR-138288
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 41a504c65b722..e909548530d27 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2744,6 +2744,13 @@ static void requeue_if_ok(struct qcow2_target *tgt, struct qio *unused,
 	qcow2_dispatch_qios(qio->qcow2, qio, NULL);
 }
 
+static bool clu_zeroes_present(struct qcow2 *qcow2, struct qcow2_map *map)
+{
+	if (qcow2->ext_l2)
+		return (map->ext_l2 >> 32) != 0;
+	return map->all_zeroes;
+}
+
 static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 				  struct qcow2_map *map, bool write)
 {
@@ -2761,7 +2768,7 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 		}
 	}
 
-	if (!map->data_clu_alloced) {
+	if (!map->data_clu_alloced && !clu_zeroes_present(qcow2, map)) {
 		/* Strange COW at L1, except the merge from RO image */
 		WARN_ON_ONCE(map->clu_is_cow && qio_may_modify_image(qcow2, *qio));
 		if (fake_merge_qio(*qio)) {
@@ -2808,7 +2815,7 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
 		goto endio;
 	}
 
-	if (!map->clu_is_cow) {
+	if (!map->clu_is_cow && map->data_clu_alloced) {
 		/* Forced set these to unuse them after discard */
 		(*qio)->ext->unuse_clu_pos = map->data_clu_pos;
 		(*qio)->ext->unuse_clu_end = map->data_clu_pos + qcow2->clu_size;
-- 
2.43.5

Re: [Devel] [PATCH VZ10 v3 00/11] dm-qcow2: improve discard and read-only merge handling

[Pavel Tikhomirov]

Looks good.

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

On 8/25/26 14:24, Andrey Zhadchenko wrote:
> This series makes discard update qcow2 metadata and permits backward
> merge from read-only source images.
> 
> First patch improves revert_cluster_alloc: this function didn't expect
> L2 entries to contain subcluster descriptions (when ext2_l2 is set).
> 
> The second patch renames COW machinery to a more general 'replace entry'.
> 
> Next five patches improve discard support in general: now it clears
> metadata and have a subcluster granularity.
> 
> The next two patches support backward merge when the image we merge
> from is read-only. Such merges now leave the source mappings and
> metadata unchanged. Previously the module ignored file mode and
> submitted writes anyway.
> 
> Last patch makes merge respect all metadata-zeroed blocks. For now it is
> writing zeroes as data, but we will improve this to metadata when we
> support REQ_OP_WRITE_ZEROES.
> 
> 
> v2:
>  - patch "update metadata on subclusters discard": expand
> qio_discard_updates_metadata() condition to return true when backing
> file is present and fully covered sublcu zero bit isn't already set.
> prepare_cluster_discard() already handled this case but I forgot
> about it here.
> 
> v3:
>  - add new "drivers/md/dm-qcow2: do not attach a bvec to discard qios"
> patch which ensures discard qios do not have garbage bvecs. Otherwise
> COW operations before discard may use these them. Following patches
> move discard above COW and alleviate this problem, but let's be
> explicit.
>  - various commit message and code formatting fixes, but nothing
> serious
> 
> Andrey Zhadchenko (11):
>   drivers/md/dm-qcow2: do not attach a bvec to discard qios
>   drivers/md/dm-qcow2: fix revert_cluster_alloc() for ext_l2 case
>   drivers/md/dm-qcow2: generalize COW index update machinery
>   drivers/md/dm-qcow2: update metadata on whole cluster discard
>   drivers/md/dm-qcow2: never trigger COW or allocation on discard
>   drivers/md/dm-qcow2: set L2_READS_ALL_ZEROES after some discards
>   drivers/md/dm-qcow2: update metadata on subclusters discard
>   drivers/md/dm-qcow2: unmap cluster when discard clears last allocated
>     subclusters
>   drivers/md/dm-qcow2: do discards during backward merge only for
>     writable image
>   drivers/md/dm-qcow2: allow shared L1 entries during merge from RO
>     image
>   drivers/md/dm-qcow2: respect zeroes during merge
> 
>  drivers/md/dm-qcow2-cmd.c    |  21 +-
>  drivers/md/dm-qcow2-map.c    | 415 +++++++++++++++++++++++++++--------
>  drivers/md/dm-qcow2-target.c |   2 +-
>  drivers/md/dm-qcow2.h        |  19 +-
>  4 files changed, 351 insertions(+), 106 deletions(-)
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.

Re: [Devel] [PATCH VZ10 v3 06/11] drivers/md/dm-qcow2: set L2_READS_ALL_ZEROES after some discards

[Konstantin Khorenko]

prepare_cluster_discard() initializes new_ext_l2 = map->ext_l2 and
only overwrites it when a backing file is present.

On an ext_l2 image without backing, a whole-cluster discard therefore writes L2 entry = 0
while keeping the old subcluster allocation bits.

Readers treat alloc bits as "mapped" regardless of the offset,
so the next read of that cluster goes through perform_rw_mapped() with data_clu_pos == 0 and
returns the qcow2 header/L1 area to the guest;
a write corrupts the image header at file offset 0.

Patch 7 changes the initializer to 0. Suggest fixing patch 6 in place or squashing 6+7.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 8/25/26 14:24, Andrey Zhadchenko wrote:
> Discard does not guarantee zero data. Therefore, to safely erase
> data, users may write zeroes and then discard. Imagine we have
> a backing file. If we write zeroes and do the discard, next read
> will give the stale data from backing image.
> Probably this is rather an edge case, but to be sure let's just
> set L2 entry to L2_READS_ALL_ZEROES so the device looks more
> consistent to users.
> 
> Feature: dm-qcow2: block device over QCOW2 files driver
> https://virtuozzo.atlassian.net/browse/VSTOR-139406
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> ---
>  drivers/md/dm-qcow2-map.c | 24 +++++++++++++++++++++---
>  drivers/md/dm-qcow2.h     |  1 +
>  2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> index a2e5c37adb7b9..390331554b7ac 100644
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
> @@ -1309,6 +1309,7 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
>  	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;
> @@ -1317,6 +1318,8 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
>  	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. */
> @@ -1330,7 +1333,7 @@ static void revert_l_entries_update(struct qcow2 *qcow2, struct wb_desc *wbd)
>  		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 (!pos)
> +		if (cleared)
>  			continue; /* no cluster was allocated */
>  
>  		spin_unlock(&qcow2->md_pages_lock);
> @@ -3368,12 +3371,16 @@ static bool qio_discard_updates_metadata(struct qcow2 *qcow2, struct qio *qio,
>   *
>   * Discard covering the whole cluster replaces the entry and unuses
>   * the discarded (or COW source) cluster.
> + * If the backing is present, set 'reads as zeroes' to avoid exposing
> + * stale data.
>   */
>  static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
>  				   struct qcow2_map *map)
>  {
> +	bool zeroes = maybe_mapped_in_lower_delta(qcow2, *qio);
>  	u32 index_in_page = map->l2.index_in_page;
>  	struct md_page *md = map->l2.md;
> +	u64 new_ext_l2 = map->ext_l2;
>  	loff_t unuse_pos, unuse_end;
>  	struct qio_ext *ext;
>  	int ret;
> @@ -3399,6 +3406,8 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
>  		unuse_pos = map->data_clu_pos;
>  		unuse_end = map->data_clu_pos + qcow2->clu_size;
>  	}
> +	if (zeroes && qcow2->ext_l2)
> +		new_ext_l2 = (u64)U32_MAX << 32;
>  
>  	ret = prepare_l_entry_replace(qcow2, map, *qio, md, index_in_page,
>  				      unuse_pos, unuse_end, L2_LEVEL);
> @@ -3408,6 +3417,10 @@ static int prepare_cluster_discard(struct qcow2 *qcow2, struct qio **qio,
>  	ext = (*qio)->ext;
>  	ext->lx_md = md;
>  
> +	ext->new_ext_l2 = new_ext_l2;
> +	if (zeroes && !qcow2->ext_l2)
> +		ext->set_all_zeroes = true;
> +
>  	(*qio)->flags |= QIO_IS_DISCARD_FL;
>  	return 1;
>  }
> @@ -4053,6 +4066,7 @@ static void process_indexes_write(struct qcow2 *qcow2,
>  	struct qio *qio;
>  	bool discard;
>  	u32 arg_mask;
> +	u64 entry;
>  	int ret;
>  
>  	while (1) {
> @@ -4076,11 +4090,15 @@ static void process_indexes_write(struct qcow2 *qcow2,
>  			goto set_ext_l2;
>  		}
>  
> +		entry = ext->allocated_clu_pos;
> +		if (unlikely(ext->set_all_zeroes))
> +			entry = L2_READS_ALL_ZEROES;
> +
>  		/* XXX: check prealloced_pos ==> revert */
>  		ret = prepare_l_entry_update(qcow2, qio, lx_md,
>  					     ext->lx_index_in_page,
> -					     &ext->allocated_clu_pos,
> -					     arg_mask, ext->lx_level);
> +					     &entry, arg_mask,
> +					     ext->lx_level);
>  		if (ret < 0) {
>  			qio->bi_status = errno_to_blk_status(ret);
>  			qio_endio(qio);
> diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
> index 8a24e04130e4d..0f006f1ae48cc 100644
> --- a/drivers/md/dm-qcow2.h
> +++ b/drivers/md/dm-qcow2.h
> @@ -312,6 +312,7 @@ struct qio_ext {
>  	u64 new_ext_l2;
>  	u32 cow_mask;
>  	bool only_set_ext_l2:1;
> +	bool set_all_zeroes:1;
>  
>  	u8 lx_level;
>  

Re: [Devel] [PATCH VZ10 v3 09/11] drivers/md/dm-qcow2: do discards during backward merge only for writable image

[Konstantin Khorenko]

as committed, a read-only backward merge of an image with internal snapshots
completes "successfully" while silently skipping every cluster under a shared L1 entry
(parse_l1() still refuses write-mode parsing of shared entries, so the merge qio ends with "nothing to merge") -

data loss once the source is dropped from the chain.
Patch 10 fixes exactly this and its own commit message confirms the data loss.
Suggest squashing/reordering, or rejecting RO sources with nb_snapshots != 0 in patch 9.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 8/25/26 14:24, Andrey Zhadchenko wrote:
> Backward merge discards every merged cluster from the image we merge
> from: the L2 entry is zeroed and refcounts are decremented, which
> dirties the image metadata. This requires the image file to be opened
> for write, while merge-in-the-middle images usually opened read-only.
> 
> Skip the discard step when the merged image is read-only:
>  - complete the merge qio right after its data is written to the lower
> delta, without the L1/L2 entry update;
>  - process READs on such image in the regular way. Previously reads would
> cause out-of-order merge for present cluster and then requeue. Without
> discard it will loop.
>  - do not break COW at L1. Note that due to how merge machinery works,
> we can't merge without unuse and internal snaphots (to be addressed
> in the next patches).
>  - do not clear the dirty bit on merge completion: the image was never
> modified.
> 
> Just in case add warning and end qio if we somehow encounter non-service
> write qio for read-only images during the merge.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-138288
> Feature: dm-qcow2: block device over QCOW2 files driver
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
> ---
>  drivers/md/dm-qcow2-cmd.c | 21 +++++++++++++--------
>  drivers/md/dm-qcow2-map.c | 27 ++++++++++++++++++++++++++-
>  drivers/md/dm-qcow2.h     |  5 +++++
>  3 files changed, 44 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
> index c15c46a0fe8b7..fa3762c58372b 100644
> --- a/drivers/md/dm-qcow2-cmd.c
> +++ b/drivers/md/dm-qcow2-cmd.c
> @@ -264,7 +264,7 @@ static int qcow2_merge_backward_start(struct qcow2_target *tgt, int efd, u32 dep
>  	lower = qcow2->lower;
>  	if (!lower)
>  		return -ENOENT;
> -	if (!(lower->file->f_mode & FMODE_WRITE))
> +	if (!qcow2_file_is_writable(lower))
>  		return -EACCES;
>  	if (qcow2->clu_size != lower->clu_size)
>  		return -EOPNOTSUPP;
> @@ -315,11 +315,14 @@ void qcow2_merge_backward_work(struct work_struct *work)
>  	 * there would be problems with unusing them:
>  	 * we'd have to freeze IO going to all data clusters
>  	 * under every L1 entry related to several snapshots.
> +	 * Readonly images skip this stage.
>  	 */
> -	ret = qcow2_break_l1cow(tgt, qcow2);
> -	if (ret) {
> -		QC_ERR(tgt->ti, "Can't break L1 COW");
> -		goto out_err;
> +	if (qcow2_file_is_writable(qcow2)) {
> +		ret = qcow2_break_l1cow(tgt, qcow2);
> +		if (ret) {
> +			QC_ERR(tgt->ti, "Can't break L1 COW");
> +			goto out_err;
> +		}
>  	}
>  
>  	backward_merge_update_stage(tgt, BACKWARD_MERGE_STAGE_SET_DIRTY);
> @@ -388,9 +391,11 @@ static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
>  	qcow2_flush_deferred_activity(tgt, qcow2); /* Delayed md pages */
>  	qcow2->lower = NULL;
>  
> -	ret = qcow2_set_image_file_features(qcow2, false);
> -	if (ret < 0)
> -		QC_ERR(tgt->ti, "Can't unuse merged img (%d)", ret);
> +	if (qcow2_file_is_writable(qcow2)) {
> +		ret = qcow2_set_image_file_features(qcow2, false);
> +		if (ret < 0)
> +			QC_ERR(tgt->ti, "Can't unuse merged img (%d)", ret);
> +	}
>  	qcow2_destroy(qcow2);
>  
>  	tgt->backward_merge.state = BACKWARD_MERGE_STOPPED;
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> index aa11956ac1d8c..c790b2ad3787e 100644
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
> @@ -2688,6 +2688,12 @@ static void backward_merge_write_complete(struct qcow2_target *tgt, struct qio *
>  		return;
>  	}
>  
> +	/* Skip discard for read-only source images */
> +	if (!qcow2_file_is_writable(qcow2)) {
> +		qio_endio(qio);
> +		return;
> +	}
> +
>  	WARN_ON_ONCE(qio->flags & QIO_IS_DISCARD_FL);
>  	qio->flags |= QIO_IS_DISCARD_FL;
>  
> @@ -2731,6 +2737,17 @@ static int prepare_backward_merge(struct qcow2 *qcow2, struct qio **qio,
>  	struct qio *aux_qio;
>  	int ret;
>  
> +	/* Readonly image mappings remain stable, so reads just go through */
> +	if (!qcow2_file_is_writable(qcow2)) {
> +		if (!op_is_write((*qio)->bi_op))
> +			return 1;
> +		if (WARN_ON_ONCE(!fake_merge_qio(*qio))) {
> +			(*qio)->bi_status = BLK_STS_IOERR;
> +			qio_endio(*qio);
> +			return 0;
> +		}
> +	}
> +
>  	if (!map->data_clu_alloced) {
>  		WARN_ON_ONCE(map->clu_is_cow); /* Strange COW at L1 */
>  		if (fake_merge_qio(*qio)) {
> @@ -3640,7 +3657,15 @@ static void process_one_qio(struct qcow2 *qcow2, struct qio *qio)
>  	if (!handle_metadata(qcow2, &qio, &map))
>  		return;
>  
> -	if (unlikely(qcow2->backward_merge_in_process)) {
> +	/*
> +	 * Merge machinery makes out of order merges for present
> +	 * clusters when it sees the reads. But if the merge does
> +	 * not discard the cluser mapping, it will spin endlessly.
> +	 * So process only actual merge qios or reads from writable
> +	 * images.
> +	 */
> +	if (unlikely(qcow2->backward_merge_in_process) &&
> +	    (fake_merge_qio(qio) || qcow2_file_is_writable(qcow2))) {
>  		submit_top_delta_read(&map, qio);
>  		return;
>  	}
> diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
> index 0f006f1ae48cc..d9b8c38e093f5 100644
> --- a/drivers/md/dm-qcow2.h
> +++ b/drivers/md/dm-qcow2.h
> @@ -460,6 +460,11 @@ static inline bool qcow2_wants_check(struct qcow2_target *tgt)
>  	return !!(tgt->md_writeback_error|tgt->truncate_error);
>  }
>  
> +static inline bool qcow2_file_is_writable(struct qcow2 *qcow2)
> +{
> +	return qcow2->file->f_mode & FMODE_WRITE;
> +}
> +
>  static inline void remap_to_clu(struct qcow2 *qcow2, struct qio *qio, loff_t clu_pos)
>  {
>  	qio->bi_iter.bi_sector &= (to_sector(qcow2->clu_size) - 1);

Re: [Devel] [PATCH VZ10 v3 00/11] dm-qcow2: improve discard and read-only merge handling

[Vasileios Almpanis]

On Tue, 25 Aug 2026 15:24:19 +0300, Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com> wrote:
> dm-qcow2: improve discard and read-only merge handling
> 
> This series makes discard update qcow2 metadata and permits backward
> merge from read-only source images.
> 
> First patch improves revert_cluster_alloc: this function didn't expect
> L2 entries to contain subcluster descriptions (when ext2_l2 is set).
> 
> [...]

Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel