Re: [PATCH] xfs: honor init_xattrs in xfs_init_new_inode for !attr && attr2 fs

[Dave Chinner <david@fromorbit.com>]

On Tue, Jun 18, 2024 at 06:06:22PM -0700, Darrick J. Wong wrote:
> On Tue, Jun 18, 2024 at 04:21:12PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > xfs_init_new_inode ignores the init_xattrs parameter for filesystems
> > that have ATTR2 enabled but not ATTR.  As a result, the first file to be
> > created by the kernel will not have an attr fork created to store acls.
> > Storing that first acl will add ATTR to the superblock flags, so chances
> > are nobody has noticed this previously.
> > 
> > However, this is disastrous on a filesystem with parent pointers because
> > it requires that a new linkable file /must/ have a pre-existing attr
> > fork.

How are we creating a parent pointer filesystem that doesn't have
XFS_SB_VERSION_ATTRBIT set in it? I thought that mkfs.xfs was going
to always set this....

> > The preproduction version of mkfs.xfs have always set this, but
> > as xfs_sb.c doesn't validate that pptrs filesystems have ATTR set, we
> > must catch this case.

Which is sounds like it is supposed to - how is parent pointers
getting enabled such that XFS_SB_VERSION_ATTRBIT is not actually
set?

> > Note that the sb verifier /does/ require ATTR2 for V5 filesystems, so we
> > can fix both problems by amending xfs_init_new_inode to set up the attr
> > fork for either ATTR or ATTR2.

True, but it still seems to me like we should be fixing mkfs.xfs and
the superblock verifier to do the right thing given this is all
still experimental and we're allowed to fix on-disk format bugs
like this.

Can we add that to the superblock verifier so that the parent
pointer filesystems will not mount if mkfs is not setting the 
XFS_SB_VERSION_ATTRBIT correctly when the parent pointer feature is
enabled?

Worst case is that early testers will need to use xfs_db to set the
XFS_SB_VERSION_ATTRBIT and then the filesystem will mount again...

-Dave.
-- 
Dave Chinner
david@fromorbit.com