Re: [Devel] [PATCH VZ10 2/6] drivers/md/dm-qcow2: keep seek parse window within limit

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

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 <andrey.zhadchenko@virtuozzo.com>
> ---
>  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.

>  	}
>  
>  	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 */

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