Re: [PATCH 33/45] libxfs: replace xfs_sb_version checks with feature flag checks

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Jan 28, 2022 at 04:13:24PM -0600, Eric Sandeen wrote:
> On 1/19/22 6:20 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Convert the xfs_sb_version_hasfoo() to checks against mp->m_features.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >   libxfs/init.c |    6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/libxfs/init.c b/libxfs/init.c
> > index adee90d5..8fe2f963 100644
> > --- a/libxfs/init.c
> > +++ b/libxfs/init.c
> > @@ -432,17 +432,17 @@ rtmount_init(
> >   	xfs_daddr_t	d;	/* address of last block of subvolume */
> >   	int		error;
> > -	if (mp->m_sb.sb_rblocks == 0)
> > +	if (!xfs_has_realtime(mp))
> 
> This seems a little gratuitous, I think after this we still have several
> checks of mp->m_sb.sb_rblocks [!=]= 0 elsewhere ... since this isn't a
> conversion of sb_version, I'd like to be pedantic and drop this hunk,

<shrug> we only set XFS_FEAT_REALTIME (aka the bit behind
xfs_has_realtime) if the primary super has sb_rblocks > 0, so as far as
I'm concerned they're the same thing.

> and consider an intentional change to switch sb_rblocks checks to
> xfs_has_realtime(mp) across all the tools?

But it might be clearer to do that as a separate conversion of all the
things.  I'll leave that decision up to you. :)

--D

> 
> Otherwise,
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> >   		return 0;
> > -	if (xfs_sb_version_hasreflink(&mp->m_sb)) {
> > +	if (xfs_has_reflink(mp)) {
> >   		fprintf(stderr,
> >   	_("%s: Reflink not compatible with realtime device. Please try a newer xfsprogs.\n"),
> >   				progname);
> >   		return -1;
> >   	}
> > -	if (xfs_sb_version_hasrmapbt(&mp->m_sb)) {
> > +	if (xfs_has_rmapbt(mp)) {
> >   		fprintf(stderr,
> >   	_("%s: Reverse mapping btree not compatible with realtime device. Please try a newer xfsprogs.\n"),
> >   				progname);
> >