[Devel] [PATCH RH9 v2 2/8] dm: add llseek_hole infrastructure

[Alexander Atanasov]

On 21.08.23 15:35, Andrey Zhadchenko wrote:
> 
> 
> On 8/21/23 14:07, Alexander Atanasov wrote:
>> On 16.08.23 12:32, Andrey Zhadchenko wrote:
>>> Add new function to target_type, so any dm devices may realize it
>>> Implement intermediate llseek_hole() on device-mapper layer, do some 
>>> simple
>>> checks.
>>>
>>> Feature: dm: implement SEEK_HOLE for dm-qcow2 and dm-ploop
>>> https://jira.vzint.dev/browse/PSBM-145746
>>> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
>>>
>>> ---
>>> v2: fix return and goto inconsistency
>>>
>>>   drivers/md/dm.c               | 41 +++++++++++++++++++++++++++++++++++
>>>   include/linux/device-mapper.h |  4 ++++
>>>   2 files changed, 45 insertions(+)
>>>
>>> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
>>> index 7e5c61a75eeb..d592c023b090 100644
>>> --- a/drivers/md/dm.c
>>> +++ b/drivers/md/dm.c
>>> @@ -484,6 +484,45 @@ static int dm_blk_ioctl(struct block_device 
>>> *bdev, fmode_t mode,
>>>       return r;
>>>   }
>>> +static loff_t dm_llseek_hole(struct block_device *bdev, loff_t 
>>> offset, int whence)
>>> +{
>>> +    struct mapped_device *md = bdev->bd_disk->private_data;
>>> +    struct dm_table *table;
>>> +    struct dm_target *ti;
>>> +    int srcu_idx;
>>> +    loff_t ret;
>>> +
>>> +    table = dm_get_live_table(md, &srcu_idx);
>>> +    if (!table || !dm_table_get_size(table))
>>> +        return -ENOTTY;
>>> +
>>> +    /*
>>> +     * For now we only support devices that have a single target.
>>> +     * But probably it is not hard to break it to a few requests to 
>>> a different
>>> +     * targets
>>> +     */
>>> +    if (table->num_targets != 1)
>>> +        return -EOPNOTSUPP;
>>
>>
>> multipath, tracking and push_backup are all possible targets (that get 
>> used) so any combination can increase the count - shouldn't this be TODO?
> 
> Maybe I am mistaken, but I always thought that table->num_targets is 
> amount of targets within single device. e.g. linear dm made by two 
> devices concatenation will have two as the number of targets. And I 
> think dm-ploop and dm-qcow2 num targets is always one, at least for now

I will double check this too.

>>
>>> +    ti = dm_table_get_target(table, 0);
>>> +
>>> +    if (dm_suspended_md(md)) {
>>> +        ret = -EAGAIN;
>>> +        goto out;
>>> +    }
>>> +
>>> +    if (!ti->type->llseek_hole) {
>>> +        ret = -EINVAL;
>>
>> EOPNOTSUPP - may be swap with num_targets != 1 error - which will be 
>> EINVAL due to the argument - while here it is not supported - not an 
>> issue with the arguments ?
> 
> -EINVAL is here to preserve default SEEK_DATA or SEEK_HOLE behavior with 
> block devices. Previously fixed_size_llseek() was called in all cases 
> and returned -EINVAL when seeking holes or data. So dm devices without 
> llseek_hole implementation should return this value too.
> 
> The same comment goes for 3/8 patch.


Ok then.


-- 
Regards,
Alexander Atanasov