Re: [Devel] [PATCH RHEL10 COMMIT] drivers/md/dm-qcow2: do discards during backward merge only for writable image

[Konstantin Khorenko <khorenko@virtuozzo.com>]

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git@bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.11.vz10
------>
commit 215cf7364e9f1ca179add5cc4b1a1e3a5b6b39cd
Author: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Date:   Thu Aug 27 19:06:18 2026 +0300

    drivers/md/dm-qcow2: do discards during backward merge only for writable image
    
    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>
    Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
    Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 drivers/md/dm-qcow2-cmd.c | 21 +++++++++++++--------
 drivers/md/dm-qcow2-map.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index e1a94bc4a140c..2865940cff303 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);
@@ -393,9 +396,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 c5a79a51942ae..53cb11242bf0d 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2701,6 +2701,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;
 
@@ -2744,6 +2750,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) {
 		/* Strange COW at L1, except the merge from RO image */
 		WARN_ON_ONCE(map->clu_is_cow && qio_may_modify_image(qcow2, *qio));
@@ -3654,7 +3671,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;
 	}
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel