| Message ID | 172437085484.57482.17914105316962083187.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/26] xfs: define the on-disk format for the metadir feature | expand |
On Thu, Aug 22, 2024 at 05:06:50PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > If metadata directory trees are enabled, it's possible that some future > metadata file might want to store information in extended attributes. > Or, if parent pointers are enabled, then children of the metadir tree > need parent pointers. Either way, we start allowing xattr data when > metadir is enabled, so we now need check and repair to examine attr > forks for metadata files on metadir filesystems. I think the parent pointer case is the relevant here, so maybe state that more clearly? Otherwise looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Thu, Aug 22, 2024 at 09:50:16PM -0700, Christoph Hellwig wrote: > On Thu, Aug 22, 2024 at 05:06:50PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > If metadata directory trees are enabled, it's possible that some future > > metadata file might want to store information in extended attributes. > > Or, if parent pointers are enabled, then children of the metadir tree > > need parent pointers. Either way, we start allowing xattr data when > > metadir is enabled, so we now need check and repair to examine attr > > forks for metadata files on metadir filesystems. > > I think the parent pointer case is the relevant here, so maybe state > that more clearly? I'll change this to: "If parent pointers are enabled, then metadata files will store parent pointers in xattrs, just like files in the user visible directory tree. Therefore, scrub and repair need to handle attr forks for metadata files on metadir filesystems." > Otherwise looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> Thanks! --D
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c index 72cec56f52eb1..f3fa9f2770d4a 100644 --- a/fs/xfs/scrub/common.c +++ b/fs/xfs/scrub/common.c @@ -1245,12 +1245,6 @@ xchk_metadata_inode_forks( return 0; } - /* They also should never have extended attributes. */ - if (xfs_inode_hasattr(sc->ip)) { - xchk_ino_set_corrupt(sc, sc->ip->i_ino); - return 0; - } - /* Invoke the data fork scrubber. */ error = xchk_metadata_inode_subtype(sc, XFS_SCRUB_TYPE_BMBTD); if (error || (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)) @@ -1267,6 +1261,21 @@ xchk_metadata_inode_forks( xchk_ino_set_corrupt(sc, sc->ip->i_ino); } + /* + * Metadata files can only have extended attributes on metadir + * filesystems, either for parent pointers or for actual xattr data. + */ + if (xfs_inode_hasattr(sc->ip)) { + if (!xfs_has_metadir(sc->mp)) { + xchk_ino_set_corrupt(sc, sc->ip->i_ino); + return 0; + } + + error = xchk_metadata_inode_subtype(sc, XFS_SCRUB_TYPE_BMBTA); + if (error || (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)) + return error; + } + return 0; } diff --git a/fs/xfs/scrub/repair.c b/fs/xfs/scrub/repair.c index 155bbaaa496e4..01c0e863775d4 100644 --- a/fs/xfs/scrub/repair.c +++ b/fs/xfs/scrub/repair.c @@ -1083,7 +1083,12 @@ xrep_metadata_inode_forks( if (error) return error; - /* Make sure the attr fork looks ok before we delete it. */ + /* + * Metadata files can only have extended attributes on metadir + * filesystems, either for parent pointers or for actual xattr data. + * For a non-metadir filesystem, make sure the attr fork looks ok + * before we delete it. + */ if (xfs_inode_hasattr(sc->ip)) { error = xrep_metadata_inode_subtype(sc, XFS_SCRUB_TYPE_BMBTA); if (error) @@ -1099,8 +1104,11 @@ xrep_metadata_inode_forks( return error; } - /* Clear the attr forks since metadata shouldn't have that. */ - if (xfs_inode_hasattr(sc->ip)) { + /* + * Metadata files on non-metadir filesystems cannot have attr forks, + * so clear them now. + */ + if (xfs_inode_hasattr(sc->ip) && !xfs_has_metadir(sc->mp)) { if (!dirty) { dirty = true; xfs_trans_ijoin(sc->tp, sc->ip, 0);