From: Xiao Yang <yangx.jy@cn.fujitsu.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: <linux-xfs@vger.kernel.org>, <ira.weiny@intel.com>
Subject: Re: [PATCH] xfs: Add check for unsupported xflags
Date: Tue, 1 Sep 2020 14:05:53 +0800 [thread overview]
Message-ID: <5F4DE4C1.6010403@cn.fujitsu.com> (raw)
In-Reply-To: <20200831172250.GT6107@magnolia>
On 2020/9/1 1:22, Darrick J. Wong wrote:
> On Mon, Aug 31, 2020 at 09:37:45PM +0800, Xiao Yang wrote:
>> Current ioctl(FSSETXATTR) ignores unsupported xflags silently
>> so it it not clear for user to know unsupported xflags.
Hi Darrick,
Sorry for a typo(s/it it/it is/).
>> For example, use ioctl(FSSETXATTR) to set dax flag on kernel
>> v4.4 which doesn't support dax flag:
>> --------------------------------
>> # xfs_io -f -c "chattr +x" testfile;echo $?
>> 0
>> # xfs_io -c "lsattr" testfile
>> ----------------X testfile
>> --------------------------------
>>
>> Add check to report unsupported info as ioctl(SETXFLAGS) does.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>> fs/xfs/xfs_ioctl.c | 4 ++++
>> include/uapi/linux/fs.h | 8 ++++++++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>> index 6f22a66777cd..cfe7f20c94fe 100644
>> --- a/fs/xfs/xfs_ioctl.c
>> +++ b/fs/xfs/xfs_ioctl.c
>> @@ -1439,6 +1439,10 @@ xfs_ioctl_setattr(
>>
>> trace_xfs_ioctl_setattr(ip);
>>
>> + /* Check if fsx_xflags have unsupported xflags */
>> + if (fa->fsx_xflags& ~FS_XFLAG_ALL)
>> + return -EOPNOTSUPP;
> Shouldn't this be in vfs_ioc_fssetxattr_check, since we're checking
> against all the vfs defined XFLAGS?
Right, different filesystems support different XFLAGS so I think it is
hard to put this
check into vfs_ioc_fssetxattr_check(). For example,
1) ext4 defines EXT4_SUPPORTED_FS_XFLAGS and do the check before
vfs_ioc_fssetxattr_check():
-------------------------------------------------------------------------------
ext4/ioctl.c:
#define EXT4_SUPPORTED_FS_XFLAGS (FS_XFLAG_SYNC | FS_XFLAG_IMMUTABLE | \
FS_XFLAG_APPEND | FS_XFLAG_NODUMP | \
FS_XFLAG_NOATIME |
FS_XFLAG_PROJINHERIT | \
FS_XFLAG_DAX)
...
if (fa.fsx_xflags & ~EXT4_SUPPORTED_FS_XFLAGS)
return -EOPNOTSUPP;
...
-------------------------------------------------------------------------------
2) btrfs adds check_xflags() and calls it before vfs_ioc_fssetxattr_check():
-------------------------------------------------------------------------------
btrfs/ioctl.c:
static int check_xflags(unsigned int flags)
{
if (flags & ~(FS_XFLAG_APPEND | FS_XFLAG_IMMUTABLE |
FS_XFLAG_NOATIME |
FS_XFLAG_NODUMP | FS_XFLAG_SYNC))
return -EOPNOTSUPP;
return 0;
}
...
ret = check_xflags(fa.fsx_xflags);
if (ret)
return ret;
...
-------------------------------------------------------------------------------
Perhaps, I should rename FS_XFLAG_ALL to XFS_SUPPORTED_FS_XFLAGS and move
it into libxfs/xfs_fs.h.
Best Regards,
Xiao Yang
> --D
>
>> +
>> code = xfs_ioctl_setattr_check_projid(ip, fa);
>> if (code)
>> return code;
>> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> index f44eb0a04afd..31b6856f6877 100644
>> --- a/include/uapi/linux/fs.h
>> +++ b/include/uapi/linux/fs.h
>> @@ -142,6 +142,14 @@ struct fsxattr {
>> #define FS_XFLAG_COWEXTSIZE 0x00010000 /* CoW extent size allocator hint */
>> #define FS_XFLAG_HASATTR 0x80000000 /* no DIFLAG for this */
>>
>> +#define FS_XFLAG_ALL \
>> + (FS_XFLAG_REALTIME | FS_XFLAG_PREALLOC | FS_XFLAG_IMMUTABLE | \
>> + FS_XFLAG_APPEND | FS_XFLAG_SYNC | FS_XFLAG_NOATIME | FS_XFLAG_NODUMP | \
>> + FS_XFLAG_RTINHERIT | FS_XFLAG_PROJINHERIT | FS_XFLAG_NOSYMLINKS | \
>> + FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT | FS_XFLAG_NODEFRAG | \
>> + FS_XFLAG_FILESTREAM | FS_XFLAG_DAX | FS_XFLAG_COWEXTSIZE | \
>> + FS_XFLAG_HASATTR)
>> +
>> /* the read-only stuff doesn't really belong here, but any other place is
>> probably as bad and I don't want to create yet another include file. */
>>
>> --
>> 2.25.1
>>
>>
>>
>
> .
>
next prev parent reply other threads:[~2020-09-01 6:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-31 13:37 [PATCH] xfs: Add check for unsupported xflags Xiao Yang
2020-08-31 17:22 ` Darrick J. Wong
2020-09-01 6:05 ` Xiao Yang [this message]
2020-09-01 16:35 ` Darrick J. Wong
2020-09-02 2:41 ` Xiao Yang
2020-09-02 3:09 ` Darrick J. Wong
2020-09-02 3:34 ` Xiao Yang
2020-09-02 4:10 ` Darrick J. Wong
2020-09-02 5:11 ` Xiao Yang
2020-09-02 17:03 ` Darrick J. Wong
2020-09-02 17:38 ` Ira Weiny
2020-09-02 17:45 ` Darrick J. Wong
2020-09-03 2:58 ` Xiao Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5F4DE4C1.6010403@cn.fujitsu.com \
--to=yangx.jy@cn.fujitsu.com \
--cc=darrick.wong@oracle.com \
--cc=ira.weiny@intel.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).