On Mon, Aug 20, 2018 at 6:49 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> The superblock verifiers are one of the last places that use the sb
> version functions to do feature checks. This are all quite simple
> uses, and there aren't many of them so open code them all.
>
> Also, move the good version number check into xfs_sb.c instead of it
> being an inline function in xfs_format.h
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_format.h | 26 ---------
> fs/xfs/libxfs/xfs_sb.c | 116 +++++++++++++++++++++++++------------
> fs/xfs/libxfs/xfs_sb.h | 1 +
> 3 files changed, 81 insertions(+), 62 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index e21c39b13890..1f1107892dcd 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -280,32 +280,6 @@ typedef struct xfs_dsb {
>
> #define XFS_SB_VERSION_NUM(sbp) ((sbp)->sb_versionnum & XFS_SB_VERSION_NUMBITS)
>
> -/*
> - * The first XFS version we support is a v4 superblock with V2 directories.
> - */
> -static inline bool xfs_sb_good_v4_features(struct xfs_sb *sbp)
> -{
> - if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT))
> - return false;
> -
> - /* check for unknown features in the fs */
> - if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) ||
> - ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) &&
> - (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS)))
> - return false;
> -
> - return true;
> -}
> -
> -static inline bool xfs_sb_good_version(struct xfs_sb *sbp)
> -{
> - if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5)
> - return true;
> - if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_4)
> - return xfs_sb_good_v4_features(sbp);
> - return false;
> -}
> -
As I understand this removed code, if, in the future, we would have
e.g. superblock v6, then xfs_sb_good_version would return false,
right? Which I think is not correctly replicated in the new function
below.
> static inline bool xfs_sb_version_hasrealtime(struct xfs_sb *sbp)
> {
> return sbp->sb_rblocks > 0;
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index bedf6c6bf990..b83cf8adca1a 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -96,6 +96,35 @@ xfs_perag_put(
> trace_xfs_perag_put(pag->pag_mount, pag->pag_agno, ref, _RET_IP_);
> }
>
> +/*
> + * We support all XFS versions newer than a v4 superblock with V2 directories.
> + */
> +bool
> +xfs_sb_good_version(
> + struct xfs_sb *sbp)
> +{
> + /* all v5 filesystems are supported */
> + if (XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5)
> + return true;
> +
> + /* versions prior to v4 are not supported */
> + if (XFS_SB_VERSION_NUM(sbp) < XFS_SB_VERSION_4)
> + return false;
> +
> + /* V4 filesystems need v2 directories */
> + if (!(sbp->sb_versionnum & XFS_SB_VERSION_DIRV2BIT))
> + return false;
> +
> + /* And must not have any unknown v4 feature bits set */
> + if ((sbp->sb_versionnum & ~XFS_SB_VERSION_OKBITS) ||
> + ((sbp->sb_versionnum & XFS_SB_VERSION_MOREBITSBIT) &&
> + (sbp->sb_features2 & ~XFS_SB_VERSION2_OKBITS)))
> + return false;
> +
> + /* It's a supported v4 filesystem */
> + return true;
> +}
> +
If we call this xfs_sb_good_version() with superblock v6 or higher, it
returns true too, not only for a supported v4. Unless the V4 specific
checks (v2 directories and feature bits) somehow implicitly prevents
that from happening, which is something I can't tell. :-)
Cheers,
Jan
--
Jan Tulak
jtulak@xxxxxxxxxx / jan@xxxxxxxx
