From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Zhadchenko Date: Fri, 14 Aug 2026 12:26:47 +0200 Subject: Re: [Devel] [PATCH VZ10 2/6] drivers/md/dm-qcow2: keep seek parse window within limit In-Reply-To: <23cafc93-d83e-4cf9-9c7b-4f6e0e317d02@virtuozzo.com> References: <20260810123000.19834-1-andrey.zhadchenko@virtuozzo.com> <20260810123000.19834-3-andrey.zhadchenko@virtuozzo.com> <23cafc93-d83e-4cf9-9c7b-4f6e0e317d02@virtuozzo.com> Message-ID: <01f724f2-cb02-4989-9e25-9b9ecaef7915@virtuozzo.com> List-Id: On 8/14/26 11:55, Pavel Tikhomirov wrote: > > > On 8/10/26 14:29, Andrey Zhadchenko wrote: >> seek_qio_next_clu() always sets the window size to the whole >> cluster, so the window may cross the scan limit. The top level qio >> then parses metadata past the virtual disk end if the disk size is >> not cluster aligned, and SEEK_HOLE may report "sector + size" past >> the limit. >> This is especially painful for lower delta seeks, since at top >> level this is luckily alleviated by blkdev_llseek_wrapper(). >> >> Add seek_qio_set_sector(), which sets bi_sector and also clamps >> bi_size if needed. >> >> https://virtuozzo.atlassian.net/browse/VSTOR-139407 >> Feature: dm-qcow2: block device over QCOW2 files driver >> Signed-off-by: Andrey Zhadchenko >> --- >> drivers/md/dm-qcow2-map.c | 29 +++++++++++++++++------------ >> 1 file changed, 17 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c >> index 542c7949321e5..342e4af688509 100644 >> --- a/drivers/md/dm-qcow2-map.c >> +++ b/drivers/md/dm-qcow2-map.c >> @@ -4352,6 +4352,16 @@ struct qio_data_llseek_hole { >> >> #define SEEK_QIO_DATA(qio) ((struct qio_data_llseek_hole *)qio->data) >> >> +static void seek_qio_set_sector(struct qio *qio, sector_t bi_sector) >> +{ >> + loff_t pos = to_bytes(bi_sector); >> + loff_t end = round_up(pos + 1, qio->qcow2->clu_size); >> + >> + qio->bi_iter.bi_sector = bi_sector; >> + end = min_t(loff_t, end, SEEK_QIO_DATA(qio)->lim); >> + qio->bi_iter.bi_size = end > pos ? end - pos : 0; >> +} >> + >> static struct qio *alloc_seek_qio(struct qcow2 *qcow2, struct qio *parent, loff_t new_lim) >> { >> struct qio_data_llseek_hole *data; >> @@ -4376,12 +4386,7 @@ static struct qio *alloc_seek_qio(struct qcow2 *qcow2, struct qio *parent, loff_ >> >> if (parent) { >> data->higher = parent; >> - qio->bi_iter.bi_sector = parent->bi_iter.bi_sector; >> - >> - if (to_bytes(qio->bi_iter.bi_sector) + parent->bi_iter.bi_size > new_lim) >> - qio->bi_iter.bi_size = new_lim - to_bytes(qio->bi_iter.bi_sector); >> - else >> - qio->bi_iter.bi_size = parent->bi_iter.bi_size; >> + seek_qio_set_sector(qio, parent->bi_iter.bi_sector); > > Can new_lim be smaller than the end derived from qio->qcow2->clu_size? > It looks like now we ignore new_lim completely. A few lines above we set data->lim = new_lim; seek_qio_set_sector respects this data->lim > >> } >> >> return qio; >> @@ -4422,16 +4427,18 @@ static inline sector_t get_next_clu(struct qio *qio) >> >> static inline void seek_qio_next_clu(struct qio *qio, struct qcow2_map *map) >> { >> + sector_t bi_sector; >> + >> /* >> * Whole L2 table is unmapped - skip to next l2 table, >> * but only if there is no backing image >> */ >> if (map && !(map->level & L2_LEVEL) && !qio->qcow2->lower) >> - qio->bi_iter.bi_sector = get_next_l2(qio); >> + bi_sector = get_next_l2(qio); >> else >> - qio->bi_iter.bi_sector = get_next_clu(qio); >> + bi_sector = get_next_clu(qio); >> >> - qio->bi_iter.bi_size = qio->qcow2->clu_size; >> + seek_qio_set_sector(qio, bi_sector); >> } >> >> static struct qio *advance_and_spawn_lower_seek_qio(struct qio *old_qio, u32 size) >> @@ -4567,9 +4574,7 @@ loff_t qcow2_llseek_hole(struct dm_target *ti, loff_t offset, int whence) >> if (!qio) >> return -ENOMEM; >> >> - qio->bi_iter.bi_sector = to_sector(offset); >> - qio->bi_iter.bi_size = qcow2->clu_size - >> - to_bytes(qio->bi_iter.bi_sector) % qcow2->clu_size; >> + seek_qio_set_sector(qio, to_sector(offset)); >> >> ret = qcow2_llseek_hole_qio(qio, whence, &result); >> /* In case of error remap ENXIO as it have special meaning for llseek */ >