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;
+}
+
static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp)
{
if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT))
@@ -301,9 +318,9 @@ static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp)
static inline bool xfs_sb_good_version(struct xfs_sb *sbp)
{
- if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5)
+ if (xfs_sb_version_is_v5(sbp))
return true;
- if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4)
+ if (xfs_sb_version_is_v4(sbp))
return xfs_sb_good_v4_features(sbp);
return false;
}
@@ -344,7 +361,7 @@ static inline void xfs_sb_version_addquota(struct xfs_sb *sbp)
static inline bool xfs_sb_version_hasalign(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 ||
+ return (xfs_sb_version_is_v5(sbp) ||
(sbp->sb_versionnum & XFS_SB_VERSION_ALIGNBIT));
}
@@ -355,7 +372,7 @@ static inline bool xfs_sb_version_hasdalign(struct xfs_sb *sbp)
static inline bool xfs_sb_version_haslogv2(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 ||
+ return xfs_sb_version_is_v5(sbp) ||
(sbp->sb_versionnum & XFS_SB_VERSION_LOGV2BIT);
}
@@ -371,7 +388,7 @@ static inline bool xfs_sb_version_hasasciici(struct xfs_sb *sbp)
static inline bool xfs_sb_version_hasmorebits(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 ||
+ return xfs_sb_version_is_v5(sbp) ||
(sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT);
}
@@ -380,14 +397,14 @@ static inline bool xfs_sb_version_hasmorebits(struct xfs_sb *sbp)
*/
static inline bool xfs_sb_version_haslazysbcount(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) ||
+ return (xfs_sb_version_is_v5(sbp)) ||
(xfs_sb_version_hasmorebits(sbp) &&
(sbp->sb_features2 & XFS_SB_VERSION2_LAZYSBCOUNTBIT));
}
static inline bool xfs_sb_version_hasattr2(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) ||
+ return (xfs_sb_version_is_v5(sbp)) ||
(xfs_sb_version_hasmorebits(sbp) &&
(sbp->sb_features2 & XFS_SB_VERSION2_ATTR2BIT));
}
@@ -407,7 +424,7 @@ static inline void xfs_sb_version_removeattr2(struct xfs_sb *sbp)
static inline bool xfs_sb_version_hasprojid32bit(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) ||
+ return (xfs_sb_version_is_v5(sbp)) ||
(xfs_sb_version_hasmorebits(sbp) &&
(sbp->sb_features2 & XFS_SB_VERSION2_PROJID32BIT));
}
@@ -494,7 +511,7 @@ xfs_sb_has_incompat_log_feature(
*/
static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+ return xfs_sb_version_is_v5(sbp);
}
/*
@@ -503,7 +520,7 @@ static inline bool xfs_sb_version_hascrc(struct xfs_sb *sbp)
*/
static inline bool xfs_sb_version_has_v3inode(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+ return xfs_sb_version_is_v5(sbp);
}
static inline bool xfs_dinode_good_version(struct xfs_sb *sbp,
@@ -516,12 +533,12 @@ static inline bool xfs_dinode_good_version(struct xfs_sb *sbp,
static inline bool xfs_sb_version_has_pquotino(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5;
+ return xfs_sb_version_is_v5(sbp);
}
static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+ return (xfs_sb_version_is_v5(sbp) &&
xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE)) ||
(xfs_sb_version_hasmorebits(sbp) &&
(sbp->sb_features2 & XFS_SB_VERSION2_FTYPE));
@@ -529,13 +546,13 @@ static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
static inline bool xfs_sb_version_hasfinobt(xfs_sb_t *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
+ return (xfs_sb_version_is_v5(sbp)) &&
(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_FINOBT);
}
static inline bool xfs_sb_version_hassparseinodes(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+ return xfs_sb_version_is_v5(sbp) &&
xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_SPINODES);
}
@@ -547,19 +564,19 @@ static inline bool xfs_sb_version_hassparseinodes(struct xfs_sb *sbp)
*/
static inline bool xfs_sb_version_hasmetauuid(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
+ return (xfs_sb_version_is_v5(sbp)) &&
(sbp->sb_features_incompat & XFS_SB_FEAT_INCOMPAT_META_UUID);
}
static inline bool xfs_sb_version_hasrmapbt(struct xfs_sb *sbp)
{
- return (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5) &&
+ return (xfs_sb_version_is_v5(sbp)) &&
(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_RMAPBT);
}
static inline bool xfs_sb_version_hasreflink(struct xfs_sb *sbp)
{
- return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+ return xfs_sb_version_is_v5(sbp) &&
(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_REFLINK);
}
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index ae9aaf1f34bf..fcd1e674be73 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -97,7 +97,7 @@ xfs_validate_sb_read(
struct xfs_mount *mp,
struct xfs_sb *sbp)
{
- if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5)
+ if (!xfs_sb_version_is_v5(sbp))
return 0;
/*
@@ -164,7 +164,7 @@ xfs_validate_sb_write(
return -EFSCORRUPTED;
}
- if (XFS_SB_VERSION_NUM(sbp) != XFS_SB_VERSION_5)
+ if (!xfs_sb_version_is_v5(sbp))
return 0;
/*
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index e2ec91b2d0f4..503fc6baabaa 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3420,7 +3420,7 @@ xlog_recover(
* (e.g. unsupported transactions, then simply reject the
* attempt at recovery before touching anything.
*/
- if (XFS_SB_VERSION_NUM(&log->l_mp->m_sb) == XFS_SB_VERSION_5 &&
+ if (xfs_sb_version_is_v5(&log->l_mp->m_sb) &&
xfs_sb_has_incompat_log_feature(&log->l_mp->m_sb,
XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN)) {
xfs_warn(log->l_mp,
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 501b9e14ce38..fc44d9891f79 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1504,7 +1504,7 @@ xfs_fc_fill_super(
set_posix_acl_flag(sb);
/* version 5 superblocks support inode version counters. */
- if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
+ if (xfs_sb_version_is_v5(&mp->m_sb))
sb->s_flags |= SB_I_VERSION;
if (mp->m_flags & XFS_MOUNT_DAX_ALWAYS) {
@@ -1622,7 +1622,7 @@ xfs_remount_rw(
return -EINVAL;
}
- if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+ if (xfs_sb_version_is_v5(&mp->m_sb) &&
xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
xfs_warn(mp,
"ro->rw transition prohibited on unknown (0x%x) ro-compat filesystem",
@@ -1735,7 +1735,7 @@ xfs_fc_reconfigure(
int error;
/* version 5 superblocks always support version counters. */
- if (XFS_SB_VERSION_NUM(&mp->m_sb) == XFS_SB_VERSION_5)
+ if (xfs_sb_version_is_v5(&mp->m_sb))
fc->sb_flags |= SB_I_VERSION;
error = xfs_fc_validate_params(new_mp);
