On Wed, Apr 10, 2024 at 08:32:44PM -0700, Christoph Hellwig wrote:
> On Wed, Apr 10, 2024 at 03:18:44PM -0700, Darrick J. Wong wrote:
> > > Is there a good reason to have a separate remove helper and not
> > > overload a NULL value like we do for the normal xattr interface?
> >
> > xfs_repair uses xfs_parent_unset -> xfs_attr_removename to erase any
> > XFS_ATTR_PARENT attribute that doesn't validate, so it needs to be able
> > to pass in a non-NULL value. Perhaps I'll add a comment about that,
> > since this isn't the first time this has come up.
> >
> > Come to think of it you can't removename a remote parent value, so I
> > guess in that bad case xfs_repair will have to drop the entire attr
> > structure <frown>.
>
> Maybe we'll need to fix that. How about you leave the xattr_flags in
> place for now, and then I or you if you really want) replace it with
> a new enum argument:
>
> enum xfs_attr_change {
> XFS_ATTR_CREATE,
> XFS_ATTR_REPLACE,
> XFS_ATTR_CREATE_OR_REPLACE,
> XFS_ATTR_REMOVE,
> };
Heh, I almost did that:
enum xfs_attr_change {
XAC_CREATE = XATTR_CREATE,
XAC_REPLACE = XATTR_REPLACE,
XAC_UPSERT,
XAC_REMOVE,
};
(500 patches from now when I get around to removing xattr_flags & making
it a parameter.)
> and we pass that to xfs_attr_set and what is current xfs_attr_setname
> (which btw is a name that feels really odd). That way repair can
> also use the libxfs attr helpers with a value match for parent pointers?
I think this is a good idea. Maybe even worth rebasing through the
tree.
--D
