xfs
[Top] [All Lists]

Re: [PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous FS_IOC_FIEMAPFS
From: Dhruvesh Rathore <adrscube@xxxxxxxxx>
Date: Thu, 29 Jan 2015 18:34:13 +0530
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=koU5798geejrAE1O6B0QHq3V7lJasRNrzDNMhcMkp64=; b=aBVGHM1Wvc/Z9pdK41zy6cqyLB/ZgMMBBPkTFDMzxU6qtBgcwFcjSArW5sLyiUBetw ZQ042l9MItQTGabztdpfZ5t8fRq15VMd19m8HqtqEynFNVf6huF5RqxPxiIh3K8gMO5D kO6V3lMlD94IeZUlb8rBXaxUGngUCh625TCmue/H7da7VVszXOzMNtKxciM2OY8App46 0Wr0gde16sBcs7mR+DMFp/4Xu92M9NurOE/ob0vdb13YMMQKNlDjm0TIeiIXby1GyZna NsiWHczUZkgdV+SvWvZ9048uVtGwXJLCv7RxUZN5g159pUYAEDjUM///kvABRWGM+FOa +cJA==
In-reply-to: <CALJmpHNkhDc-54LKwi7k_-V9aoZD75cZZ2gf7N6YFt0T=-GiSA@xxxxxxxxxxxxxx>
References: <54c1c11e.678c420a.37a1.13c6@xxxxxxxxxxxxx> <20150128012758.GS16552@dastard> <CALJmpHNkhDc-54LKwi7k_-V9aoZD75cZZ2gf7N6YFt0T=-GiSA@xxxxxxxxxxxxxx>
>> --- a/linux-xfs/fs/xfs/xfs_ioctl.c    2015-01-04 15:45:26.518359725 +0530
>> +++ b/linux-xfs/fs/xfs/xfs_ioctl.c    2015-01-04 15:24:06.030407817 +0530
>> @@ -49,6 +49,8 @@
>>  #include <linux/exportfs.h>
>>
>>
>> +/* So that the fiemap access checks can't overflow on 32 bit machines. */
>> +#define FIEMAP_MAX_EXTENTS   (UINT_MAX / sizeof(struct fiemap_extent))
>
> Just move to include/linux/fs.h where the FS_IOC_FIEMAP definitions
> are.

>> +static int fiemap_check_ranges(struct super_block *sb,
>> +                            u64 start, u64 len, u64 *new_len)
>> +{
>> +     u64 maxbytes = (u64) sb->s_maxbytes;
>> +
>> +     *new_len = len;
>> +
>> +     if (len == 0)
>> +             return -EINVAL;
>> +
>> +     if (start > maxbytes)
>> +             return -EFBIG;
>> +
>> +     /*
>> +      * Shrink request scope to what the fs can actually handle.
>> +      */
>> +     if (len > maxbytes || (maxbytes - len) < start)
>> +             *new_len = maxbytes - start;
>>
>> +     return 0;
>> +}
>
> Rather than copying this code, I'd suggest that you remove the "static"
> declaration, export the symbol and add a prototype to
> include/linux/fs.h similar to fiemap_check_flags().
>

>> +static int xfsctl_fiemapfs(struct super_block *sb, unsigned long arg)
>
> - xfs_ioctl_fiemapfs() follows the naming convention used in the
>   rest of the file
> - this is an XFS specific function now, so should pass the xfs_mount
>   rather than the VFS super_block.
> - need a comment saying it was mostly copied from fs/ioctl.c::ioctl_fiemap().
> - arg has already marked as __user converted in the calling
>   function, so we need to retain the __user annotations here.
> - format for XFS functions is this:
>
> /*
>  * Mostly copied from ioctl_fiemap()
>  */
> static int
> xfs_ioctl_fiemapfs(
>         struct xfs_mount        *mp,
>         void __user             *arg)
>
>

>> +     if (!sb->s_op->fiemapfs)
>> +             return -EOPNOTSUPP;
>
> No need to check this - we can call the XFS function directly.
>

>>                       free(fiemap);
>>                       exitcode = 1;
>> @@ -338,6 +338,10 @@
>>
>>       if (!init(argc, argv))
>>               return 0;
>> +
>> +     if (dumpflag)
>> +             printf("%8s %8s %8s\n", "agno", "agbno", "len");
>
> Added functionality? That should be in a separate patch ;)
>

We have assimilated the changes and divided them into three
patches one for the kernel space code, one for the user space
code  and one patch for the above added functionality.

The updated patches have been sent as a separate thread :)

Regards,
A-DRS

<Prev in Thread] Current Thread [Next in Thread>