Re: [PATCH v1] xfs: Fail remount with noattr2 on a v5 xfs with CONFIG_XFS_SUPPORT_V4=y

[Christoph Hellwig <hch@infradead.org>]

On Tue, Apr 15, 2025 at 12:48:39PM +0530, Nirjhar Roy (IBM) wrote:
> condition(noattr2 on v5) is not caught in xfs_fs_validate_params() because
> the superblock isn't read yet and "struct xfs_mount    *mp" is still not
> aware of whether the underlying filesystem is v5 or v4 (i.e, whether crc=0
> or crc=1). So, even if the underlying filesystem is v5, xfs_has_attr2() will
> return false, because the m_features isn't populated yet.

Yes.

> However, once
> xfs_readsb() is done, m_features is populated (mp->m_features |=
> xfs_sb_version_to_features(sbp); called at the end of xfs_readsb()). After
> that, when xfs_finish_flags() is called, the invalid mount option (i.e,
> noattr2 with crc=1) is caught, and the mount fails correctly. So, m_features
> is partially populated while xfs_fs_validate_params() is getting executed, I
> am not sure if that is done intentionally.

As you pointed out above it can't be fully populated becaue we don't
have all the information.  And that can't be fixed because some of the
options are needed to even get to reading the superblock.

So we do need a second pass of verification for everything that depends
on informationtion from the superblock.  The fact that m_features
mixes user options and on-disk feature bits is unfortunately not very
helpful for a clear structure here.