xfs
[Top] [All Lists]

Re: [RFC PATCH 02/11] xfs: reserve v5 superblock read-only compat. featu

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH 02/11] xfs: reserve v5 superblock read-only compat. feature bit for finobt
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 5 Sep 2013 10:39:02 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1378232708-57156-3-git-send-email-bfoster@xxxxxxxxxx>
References: <1378232708-57156-1-git-send-email-bfoster@xxxxxxxxxx> <1378232708-57156-3-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Sep 03, 2013 at 02:24:59PM -0400, Brian Foster wrote:
> Reserve a v5 read-only compatibility feature bit for the finobt and
> create the xfs_sb_version_hasfinobt() helper to determine whether
> an fs has the feature enabled.
> 
> The finobt does not change existing on-disk structures, but must
> remain consistent with the ialloc btree. Modifications from older
> kernels would violate that constrant. Therefore, we restrict older
> kernels to read-only mounts of finobt-enabled filesystems.
> 
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_sb.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_sb.h b/fs/xfs/xfs_sb.h
> index 6835b44..c48d95d 100644
> --- a/fs/xfs/xfs_sb.h
> +++ b/fs/xfs/xfs_sb.h
> @@ -585,7 +585,9 @@ xfs_sb_has_compat_feature(
>       return (sbp->sb_features_compat & feature) != 0;
>  }
>  
> -#define XFS_SB_FEAT_RO_COMPAT_ALL 0
> +#define XFS_SB_FEAT_RO_COMPAT_FINOBT   (1 << 0)              /* free inode 
> btree */
> +#define XFS_SB_FEAT_RO_COMPAT_ALL \
> +             (XFS_SB_FEAT_RO_COMPAT_FINOBT)
>  #define XFS_SB_FEAT_RO_COMPAT_UNKNOWN        ~XFS_SB_FEAT_RO_COMPAT_ALL

The only thing I'd suggest here is that the last patch in the series
should add the XFS_SB_FEAT_RO_COMPAT_FINOBT bit to the
XFS_SB_FEAT_RO_COMPAT_ALL mask.

Otherwise we can have the problem of bisects landing in the middle
of the series and thinking that the feature is fully supported when
it isn't. So it's fine to add the xfs_sb_version_hasfinobt() helper
here and define the bit but don't add it to the supported mask
until all the changes for the feature are complete.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>