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

[Andrey Albershteyn <aalbersh@redhat.com>]

On Wed, Dec 14, 2022 at 12:03:56PM +1100, Dave Chinner wrote:
> 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.

Agreed.

> 
> 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.

Thanks, will describe that.

> 
> That way it becomes more obvious that we handle internal vs external
> xattr namespaces very differently.
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 

-- 
- Andrey