Re: [RFC PATCH 03/11] xfs: add attribute type for fs-verity

[Dave Chinner <david@fromorbit.com>]

On Tue, Dec 13, 2022 at 11:43:42AM -0600, Eric Sandeen wrote:
> On 12/13/22 11:29 AM, Andrey Albershteyn wrote:
> > The Merkle tree pages and descriptor are stored in the extended
> > attributes of the inode. Add new attribute type for fs-verity
> > metadata. Skip fs-verity attributes for getfattr as it can not parse
> > binary page names.
> > 
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> 
> 
> >  DECLARE_EVENT_CLASS(xfs_attr_list_class,
> > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
> > index 5b57f6348d630..acbfa29d04af0 100644
> > --- a/fs/xfs/xfs_xattr.c
> > +++ b/fs/xfs/xfs_xattr.c
> > @@ -237,6 +237,9 @@ xfs_xattr_put_listent(
> >  	if (flags & XFS_ATTR_PARENT)
> >  		return;
> >  
> > +	if (flags & XFS_ATTR_VERITY)
> > +		return;
> > +
> 
> Just a nitpick, but now that there are already 2 cases like this, I wonder
> if it would be wise to #define something like an XFS_ATTR_VISIBLE_MASK
> (or maybe XFS_ATTR_INTERNAL_MASK) and use that to decide, rather than
> testing each one individually?

Seems like a good idea to me.

There's also a couple of other spots that a comment about internal
vs externally visible xattr namespaces might be appropriate. e.g
xfs_attr_filter() in the ioctl code should never have internal xattr
namespaces added to it, and similarly a comment at the top of
fs/xfs/xfs_xattr.c that the interfaces implemented in the file are
only for exposing externally visible xattr namespaces to users.

That way it becomes more obvious that we handle internal vs external
xattr namespaces very differently.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com