On 9/14/20 2:48 PM, Eric Sandeen wrote:
> On 9/14/20 2:29 AM, Christoph Hellwig wrote:
>> On Fri, Sep 11, 2020 at 09:43:11AM -0700, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>>>
>>> The V4 filesystem format contains known weaknesses in the on-disk format
>>> that make metadata verification diffiult. In addition, the format will
>>> does not support dates past 2038 and will not be upgraded to do so.
>>> Therefore, we should start the process of retiring the old format to
>>> close off attack surfaces and to encourage users to migrate onto V5.
>>>
>>> Therefore, make XFS V4 support a configurable option. For the first
>>> period it will be default Y in case some distributors want to withdraw
>>> support early; for the second period it will be default N so that anyone
>>> who wishes to continue support can do so; and after that, support will
>>> be removed from the kernel.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>>> ---
>>> v3: be a little more helpful about old xfsprogs and warn more loudly
>>> about deprecation
>>> v2: define what is a V4 filesystem, update the administrator guide
>> Whie this patch itself looks good, I think the ifdef as is is rather
>> silly as it just prevents mounting v4 file systems without reaping any
>> benefits from that.
>>
>> So at very least we should add a little helper like this:
>>
>> static inline bool xfs_sb_is_v4(truct xfs_sb *sbp)
>> {
>> if (IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
>> return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
>> return false;
>> }
>>
>> and use it in all the feature test macros to let the compile eliminate
>> all the dead code.
>>
> Makes sense, I think - something like this?
>
> xfs: short-circuit version tests if V4 is disabled at compile time
>
> Replace open-coded checks for == XFS_SB_VERSION_[45] with helpers
> which can be compiled away if CONFIG_XFS_SUPPORT_V4 is disabled.
>
> Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> NB: this is compile-tested only
>
> Honestly I'd like to replace lots of the has_crc() checks with is_v5()
> as well, unless the test is specifically related to CRC use. *shrug*
>
>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index 31b7ece985bb..18b187e38017 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -283,6 +283,23 @@ typedef struct xfs_dsb {
> /*
> * The first XFS version we support is a v4 superblock with V2 directories.
> */
> +
> +static inline bool xfs_sb_version_is_v4(struct xfs_sb *sbp)
> +{
> + if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> + return false;
> +
> + return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4;
> +}
> +
> +static inline bool xfs_sb_version_is_v5(struct xfs_sb *sbp)
> +{
> + if (!IS_ENABLED(CONFIG_XFS_SUPPORT_V4))
> + return true;
> +
> + return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
> +}
Oh, I suppose the tests in the mount path would then need to be open-coded,
won't they. After we've gotten past that then we can assume that we only have
V5 if we've mounted successfully ....
-Eric
