From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Zhadchenko Date: Wed, 12 Aug 2026 21:56:36 +0300 Subject: [Devel] [PATCH VZ10 08/10] drivers/md/dm-qcow2: do discards during backward merge only for writable image In-Reply-To: <20260812185638.110779-1-andrey.zhadchenko@virtuozzo.com> References: <20260812185638.110779-1-andrey.zhadchenko@virtuozzo.com> Message-ID: <20260812185638.110779-9-andrey.zhadchenko@virtuozzo.com> List-Id: 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 --- drivers/md/dm-qcow2-cmd.c | 23 +++++++++++++++-------- drivers/md/dm-qcow2-map.c | 27 ++++++++++++++++++++++++++- drivers/md/dm-qcow2.h | 5 +++++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c index c15c46a0fe8b7..f2c8f51e17af6 100644 --- a/drivers/md/dm-qcow2-cmd.c +++ b/drivers/md/dm-qcow2-cmd.c @@ -264,7 +264,9 @@ 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_file_is_writable(qcow2) && qcow2->hdr.nb_snapshots) return -EACCES; if (qcow2->clu_size != lower->clu_size) return -EOPNOTSUPP; @@ -315,11 +317,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 +393,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 f80504927eaae..aa30d12f8ee04 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)) { @@ -3630,7 +3647,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