Re: [PATCH 10/10] xfs_admin: support adding features to V5 filesystems

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

On Tue, Feb 09, 2021 at 12:22:04PM -0500, Brian Foster wrote:
> On Mon, Feb 08, 2021 at 08:11:00PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Teach the xfs_admin script how to add features to V5 filesystems.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  db/xfs_admin.sh      |    6 ++++--
> >  man/man8/xfs_admin.8 |   22 ++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > 
> > diff --git a/db/xfs_admin.sh b/db/xfs_admin.sh
> > index 430872ef..7a467dbe 100755
> > --- a/db/xfs_admin.sh
> > +++ b/db/xfs_admin.sh
> > @@ -8,9 +8,10 @@ status=0
> >  DB_OPTS=""
> >  REPAIR_OPTS=""
> >  REPAIR_DEV_OPTS=""
> > -USAGE="Usage: xfs_admin [-efjlpuV] [-c 0|1] [-L label] [-r rtdev] [-U uuid] device [logdev]"
> > +DB_LOG_OPTS=""
> > +USAGE="Usage: xfs_admin [-efjlpuV] [-c 0|1] [-L label] [-O v5_feature] [-r rtdev] [-U uuid] device [logdev]"
> 
> Technically this would pass through the lazy counter variant on a v4
> super, right? I wonder if we should just call it "[-O feature]" here.
> 
> >  
> > -while getopts "c:efjlL:pr:uU:V" c
> > +while getopts "c:efjlL:O:pr:uU:V" c
> >  do
> >  	case $c in
> >  	c)	REPAIR_OPTS=$REPAIR_OPTS" -c lazycount="$OPTARG;;
> ...
> > diff --git a/man/man8/xfs_admin.8 b/man/man8/xfs_admin.8
> > index cccbb224..3f3aeea8 100644
> > --- a/man/man8/xfs_admin.8
> > +++ b/man/man8/xfs_admin.8
> ...
> > @@ -106,6 +108,26 @@ The filesystem label can be cleared using the special "\c
> >  " value for
> >  .IR label .
> >  .TP
> > +.BI \-O " feature1" = "status" , "feature2" = "status..."
> > +Add or remove features on an existing a V5 filesystem.
> 
> s/existing a/existing/

Fixed, thanks.

> Also, similar question around the lazycount variant. If it works, should
> it not be documented here?

It would work, but I'd rather stick to the existing narrative that
"xfs_admin -c 0|1" is how one changes the lazycount feature.  Mentioning
both avenues in the manpage introduces the potential for confusion as to
whether one is particularly better than the other.

--D

> Brian
> 
> > +The features should be specified as a comma-separated list.
> > +.I status
> > +should be either 0 to disable the feature or 1 to enable the feature.
> > +Note, however, that most features cannot be disabled.
> > +.IP
> > +.B NOTE:
> > +Administrators must ensure the filesystem is clean by running
> > +.B xfs_repair -n
> > +to inspect the filesystem before performing the upgrade.
> > +If corruption is found, recovery procedures (e.g. reformat followed by
> > +restoration from backup; or running
> > +.B xfs_repair
> > +without the
> > +.BR -n )
> > +must be followed to clean the filesystem.
> > +.IP
> > +There are currently no feature options.
> > +.TP
> >  .BI \-U " uuid"
> >  Set the UUID of the filesystem to
> >  .IR uuid .
> > 
>