Re: [Devel] [PATCH VZ10 4/6] drivers/md/dm-qcow2: fix seek skip to the next L2 table

[Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>]

On 8/14/26 12:25, Pavel Tikhomirov wrote:
> 
> 
> On 8/10/26 14:29, Andrey Zhadchenko wrote:
>> get_next_l2() adds the number of remaining clusters to the current
>> sector, so for a position which is not cluster aligned (e.g. the
>> llseek offset itself) the result overshoots the L2 table boundary by
>> the in-cluster offset.
>>
>> Compute the boundary from the cluster index instead of adding to the
>> current position.
>>
>> 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 | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
>> index d6d15b04215c6..776e89fc4eb1b 100644
>> --- a/drivers/md/dm-qcow2-map.c
>> +++ b/drivers/md/dm-qcow2-map.c
>> @@ -4405,12 +4405,12 @@ static struct qio *free_seek_qio_ret_higher(struct qio *qio)
>>   static inline sector_t get_next_l2(struct qio *qio)
>>   {
>>   	struct qcow2 *qcow2 = qio->qcow2;
>> -	loff_t start, add;
>> +	u64 base_l2, add;
>>   
>> -	start = to_bytes(qio->bi_iter.bi_sector);
>> -	add = qcow2->l2_entries - (start / qcow2->clu_size) % qcow2->l2_entries;
>> +	base_l2 = to_bytes(qio->bi_iter.bi_sector) / qcow2->clu_size;
>> +	add = qcow2->l2_entries - base_l2 % qcow2->l2_entries;
>>   
>> -	return qio->bi_iter.bi_sector + (qcow2->clu_size / to_bytes(1)) * add;
>> +	return to_sector((base_l2 + add) * qcow2->clu_size);
> 
> The above code seems too complex to what it actually tries to accomplish,
> it's just an alignment, what about:
> 
> static inline sector_t get_next_l2(struct qio *qio)
> {
>        struct qcow2 *qcow2 = qio->qcow2;
>        u64 l2_covered = (u64)qcow2->clu_size * qcow2->l2_entries;
> 
>        return to_sector(round_up(to_bytes(qio->bi_iter.bi_sector) + 1,
>                                  l2_covered));
> }

Yeah, that looks much better. But probably no sense to convert sector to 
bytes and then back to sector (as cluster_size for qcow2 is always a 
power of 2 no less then 512). I will update this part for v2

> 
>>   }
>>   
>>   static inline sector_t get_next_clu(struct qio *qio)
>