Re: [PATCH 1/2] xfs_db: add inobtcnt upgrade path

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

On Wed, Jan 13, 2021 at 01:35:05PM -0500, Brian Foster wrote:
> On Fri, Jan 08, 2021 at 10:28:25PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Enable users to upgrade their filesystems to support inode btree block
> > counters.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> These two look Ok to me, but I noticed that both commands have weird
> error semantics when run through xfs_admin and the associated features
> are already set. E.g.:
> 
> # xfs_admin -O inobtcount /dev/test/tmp 
> Upgrading V5 filesystem
> Upgraded V5 filesystem.  Please run xfs_repair.
> versionnum [0xb4a5+0x18a] = V5,NLINK,DIRV2,ALIGN,LOGV2,EXTFLG,MOREBITS,ATTR2,LAZYSBCOUNT,PROJID32BIT,CRC,FTYPE,FINOBT,SPARSE_INODES,REFLINK,INOBTCNT
> Running xfs_repair to ensure filesystem consistency.
> # xfs_admin -O inobtcount /dev/test/tmp 
> inode btree counter feature is already enabled
> Running xfs_repair to ensure filesystem consistency.
> Conversion failed, is the filesystem unmounted?
> # mount /dev/test/tmp /mnt/
> # umount /mnt/
> 
> So it looks like we run repair again the second time around even though
> the bit was already set, which is probably unnecessary, but then also
> for some reason report the result as failed.

Hm.  I guess I could define a second exitcode that means "no action
taken", and have xfs_admin exit.

> (I also realized that repair is fixing up some agi metadata in this
> case, so my previous thought around a special repair verify mode is
> probably not relevant..).

<nod>

--D

> Brian
> 
> >  db/sb.c              |   22 ++++++++++++++++++++++
> >  man/man8/xfs_admin.8 |    7 +++++++
> >  man/man8/xfs_db.8    |    3 +++
> >  3 files changed, 32 insertions(+)
> > 
> > 
> > diff --git a/db/sb.c b/db/sb.c
> > index 93e4c405..b89ccdbe 100644
> > --- a/db/sb.c
> > +++ b/db/sb.c
> > @@ -597,6 +597,7 @@ version_help(void)
> >  " 'version attr2'    - enable v2 inline extended attributes\n"
> >  " 'version log2'     - enable v2 log format\n"
> >  " 'version needsrepair' - flag filesystem as requiring repair\n"
> > +" 'version inobtcount' - enable inode btree counters\n"
> >  "\n"
> >  "The version function prints currently enabled features for a filesystem\n"
> >  "according to the version field of its primary superblock.\n"
> > @@ -857,6 +858,27 @@ version_f(
> >  			}
> >  
> >  			v5features.incompat |= XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR;
> > +		} else if (!strcasecmp(argv[1], "inobtcount")) {
> > +			if (xfs_sb_version_hasinobtcounts(&mp->m_sb)) {
> > +				dbprintf(
> > +		_("inode btree counter feature is already enabled\n"));
> > +				exitcode = 1;
> > +				return 1;
> > +			}
> > +			if (!xfs_sb_version_hasfinobt(&mp->m_sb)) {
> > +				dbprintf(
> > +		_("inode btree counter feature cannot be enabled on filesystems lacking free inode btrees\n"));
> > +				exitcode = 1;
> > +				return 1;
> > +			}
> > +			if (!xfs_sb_version_hascrc(&mp->m_sb)) {
> > +				dbprintf(
> > +		_("inode btree counter feature cannot be enabled on pre-V5 filesystems\n"));
> > +				exitcode = 1;
> > +				return 1;
> > +			}
> > +
> > +			v5features.ro_compat |= XFS_SB_FEAT_RO_COMPAT_INOBTCNT;
> >  		} else if (!strcasecmp(argv[1], "extflg")) {
> >  			switch (XFS_SB_VERSION_NUM(&mp->m_sb)) {
> >  			case XFS_SB_VERSION_1:
> > diff --git a/man/man8/xfs_admin.8 b/man/man8/xfs_admin.8
> > index b423981d..a776b375 100644
> > --- a/man/man8/xfs_admin.8
> > +++ b/man/man8/xfs_admin.8
> > @@ -116,6 +116,13 @@ If this is a V5 filesystem, flag the filesystem as needing repairs.
> >  Until
> >  .BR xfs_repair (8)
> >  is run, the filesystem will not be mountable.
> > +.TP
> > +.B inobtcount
> > +Upgrade a V5 filesystem to support the inode btree counters feature.
> > +This reduces mount time by caching the size of the inode btrees in the
> > +allocation group metadata.
> > +Once enabled, the filesystem will not be writable by older kernels.
> > +The filesystem cannot be downgraded after this feature is enabled.
> >  .RE
> >  .TP
> >  .BI \-U " uuid"
> > diff --git a/man/man8/xfs_db.8 b/man/man8/xfs_db.8
> > index 7331cf19..1b826e5d 100644
> > --- a/man/man8/xfs_db.8
> > +++ b/man/man8/xfs_db.8
> > @@ -976,6 +976,9 @@ The filesystem can be flagged as requiring a run through
> >  if the
> >  .B needsrepair
> >  option is specified and the filesystem is formatted with the V5 format.
> > +Support for the inode btree counters feature can be enabled by using the
> > +.B inobtcount
> > +option if the filesystem is formatted with the V5 format.
> >  .IP
> >  If no argument is given, the current version and feature bits are printed.
> >  With one argument, this command will write the updated version number
> > 
>