From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavel Tikhomirov Date: Fri, 21 Aug 2026 12:44:42 +0200 Subject: Re: [Devel] [PATCH VZ10 v2 09/10] drivers/md/dm-qcow2: allow shared L1 entries during merge from RO image In-Reply-To: References: <20260812202005.288994-1-andrey.zhadchenko@virtuozzo.com> <20260812202005.288994-10-andrey.zhadchenko@virtuozzo.com> Message-ID: List-Id: On 8/21/26 12:43, Pavel Tikhomirov wrote: > > > On 8/12/26 22:20, Andrey Zhadchenko wrote: >> 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 >> --- >> drivers/md/dm-qcow2-cmd.c | 2 -- >> drivers/md/dm-qcow2-map.c | 18 ++++++++++++++++-- >> 2 files changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c >> index f2c8f51e17af6..fa3762c58372b 100644 >> --- a/drivers/md/dm-qcow2-cmd.c >> +++ b/drivers/md/dm-qcow2-cmd.c >> @@ -266,8 +266,6 @@ static int qcow2_merge_backward_start(struct qcow2_target *tgt, int efd, u32 dep >> return -ENOENT; >> if (!qcow2_file_is_writable(lower)) >> return -EACCES; >> - if (!qcow2_file_is_writable(qcow2) && qcow2->hdr.nb_snapshots) >> - return -EACCES; > > Having all these add in one patch remove in next one blocks does not feel right. > Let's try to merge all fixups in final version. Sorry, maybe I'm wrong, and it's more of a separate feature here again. > >> if (qcow2->clu_size != lower->clu_size) >> return -EOPNOTSUPP; >> if (lower->hdr.size < qcow2->hdr.size) >> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c >> index ebe68e8d0d3d8..baa6778f1686c 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; > -- Best regards, Pavel Tikhomirov Senior Software Developer, Virtuozzo.