On Tue, Apr 09, 2024 at 10:51:18PM -0700, Christoph Hellwig wrote: > On Tue, Apr 09, 2024 at 05:59:30PM -0700, Darrick J. Wong wrote: > > From: Allison Henderson <allison.henderson@xxxxxxxxxx> > > > > Parent pointers returned to the get_fattr tool cause errors since > > the tool cannot parse parent pointers. Fix this by filtering parent > > parent pointers from xfs_xattr_put_listent. > > With the new format returning the attrs should not cause parsing errors. > OTOH we now have duplicate names, which means a get operation based on > the name can't actually work in that case. > > I'd also argue that parent pointers are internal enough that they > should not be exposed through the normal xattr interfaces. Yeah, I probably should change the commit message to: "xfs: don't return XFS_ATTR_PARENT attributes via listxattr "Parent pointers are internal filesystem metadata. They're not intended to be directly visible to userspace, so filter them out of xfs_xattr_put_listent so that they don't appear in listxattr." > > +/* > > + * This file defines functions to work with externally visible extended > > + * attributes, such as those in user, system, or security namespaces. They > > + * should not be used for internally used attributes. Consider xfs_attr.c. > > + */ > > As long as xfs_attr_change and xfs_attr_grab_log_assist are xfs_xattr.c > that is not actually true. However I think they should be moved to > xfs_attr.c (and in case of xfs_attr_change merged into xfs_attr_set) > to make this comment true. I don't want to hoist all the larp enabling jun^Wmachinery to libxfs and then have to stub that out in userspace. I'd rather get rid of larp mode entirely, after which point xfs_attr_change becomes a trivial helper that can be collapsed. > However I'd make it part of the top of file comment above the include > statements. And please add it in a separate commit as it has nothing > to do with the other changes here. Or just get rid of the comment entirely? It came from the verity series. --D
