On Tue, Feb 25, 2020 at 03:10:09PM -0800, Christoph Hellwig wrote:
> The ATTR_* flags have a long IRIX history, where they a userspace
> interface, the on-disk format and an internal interface. We've split
> out the on-disk interface to the XFS_ATTR_* values, but despite (or
> because?) of that the flag have still been a mess. Switch the
> internal interface to pass the on-disk XFS_ATTR_* flags for the
> namespace and the Linux XATTR_* flags for the actual flags instead.
> The ATTR_* values that are actually used are move to xfs_fs.h with a
> new XFS_IOC_* prefix to not conflict with the userspace version that
> has the same name and must have the same value.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Chandan Rajendra <chandanrlinux@xxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_attr.c | 16 ++++++-------
> fs/xfs/libxfs/xfs_attr.h | 22 +-----------------
> fs/xfs/libxfs/xfs_attr_leaf.c | 14 +++++------
> fs/xfs/libxfs/xfs_da_btree.h | 3 ++-
> fs/xfs/libxfs/xfs_da_format.h | 12 ----------
> fs/xfs/libxfs/xfs_fs.h | 12 +++++++++-
> fs/xfs/scrub/attr.c | 5 +---
> fs/xfs/xfs_acl.c | 4 ++--
> fs/xfs/xfs_ioctl.c | 44 +++++++++++++++++++++++++----------
> fs/xfs/xfs_iops.c | 3 +--
> fs/xfs/xfs_linux.h | 1 +
> fs/xfs/xfs_trace.h | 35 +++++++++++++++++-----------
> fs/xfs/xfs_xattr.c | 18 +++++---------
> 13 files changed, 93 insertions(+), 96 deletions(-)
This cleans up a lot of messy checks. Nice. Couple of minor things
below.
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> @@ -489,10 +489,10 @@ xfs_attr_leaf_addname(
> * the given flags produce an error or call for an atomic rename.
> */
> retval = xfs_attr3_leaf_lookup_int(bp, args);
> - if (retval == -ENOATTR && (args->flags & ATTR_REPLACE))
> + if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
> goto out_brelse;
> if (retval == -EEXIST) {
> - if (args->flags & ATTR_CREATE) /* pure create op */
> + if (args->attr_flags & XATTR_CREATE) /* pure create op */
> goto out_brelse;
Comment can die.
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index d93cb8385b52..f3660ae9eb3e 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -59,7 +59,8 @@ typedef struct xfs_da_args {
> uint8_t filetype; /* filetype of inode for directories */
> void *value; /* set of bytes (maybe contain NULLs) */
> int valuelen; /* length of value */
> - int flags; /* argument flags (eg: ATTR_NOCREATE) */
> + unsigned int attr_filter; /* XFS_ATTR_{ROOT,SECURE} */
> + unsigned int attr_flags; /* XATTR_{CREATE,REPLACE} */
At this point, these are really operation flags. I would have named
the variable attr_opflags but I don't think it's worth redoing the
entire patch and others over this.
> @@ -50,7 +54,7 @@ DECLARE_EVENT_CLASS(xfs_attr_list_class,
> __field(int, count)
> __field(int, firstu)
> __field(int, dupcnt)
> - __field(int, flags)
> + __field(int, attr_filter)
uint?
> @@ -174,7 +179,7 @@ TRACE_EVENT(xfs_attr_list_node_descend,
> __field(int, count)
> __field(int, firstu)
> __field(int, dupcnt)
> - __field(int, flags)
> + __field(int, attr_filter)
same.
> @@ -1701,7 +1707,7 @@ DECLARE_EVENT_CLASS(xfs_attr_class,
> __field(int, namelen)
> __field(int, valuelen)
> __field(xfs_dahash_t, hashval)
> - __field(int, flags)
> + __field(int, attr_filter)
And once more.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
