On Fri, Feb 25, 2022 at 04:04:21PM -0800, Darrick J. Wong wrote:
> On Fri, Feb 25, 2022 at 04:14:13PM -0600, Eric Sandeen wrote:
> > On 1/19/22 7:32 PM, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> > >
> > > Ted Ts'o reported brittleness in the fstests logic in generic/45[34] to
> > > detect whether or not xfs_scrub is capable of detecting Unicode mischief
> > > in directory and xattr names. This is a compile-time feature, since we
> > > do not assume that all distros will want to ship xfsprogs with libicu.
> > >
> > > Rather than relying on ldd tests (which don't work at all if xfs_scrub
> > > is compiled statically), let's have -V print whether or not the feature
> > > is built into the tool. Phase 5 still requires the presence of "UTF-8"
> > > in LC_MESSAGES to enable Unicode confusable detection; this merely makes
> > > the feature easier to discover.
> > >
> > > Reported-by: Theodore Ts'o <tytso@xxxxxxx>
> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > ---
> > > v2: correct the name of the reporter
> > > ---
> >
> > Hum, every single other utility just does "$progname version $version"
> > and I'm not that keen to tack on something for everyone, if it won't
> > really mean anything to anyone except xfstests scripts ;)
> >
> > What about adding an "-F" to display features, and xfstests can use that,
> > and xfs_scrub -V will keep acting like every other utility?
> >
> > Other utilities could use this too if we ever cared (though xfs_db
> > and xfs_io already have an "-F" option ... we could choose -Z for
> > featureZ, which is unused as a primary option anywhere ...)
> >
> > like so:
> >
> > ===
> >
> > diff --git a/man/man8/xfs_scrub.8 b/man/man8/xfs_scrub.8
> > index e881ae76..65d8f4a2 100644
> > --- a/man/man8/xfs_scrub.8
> > +++ b/man/man8/xfs_scrub.8
> > @@ -8,7 +8,7 @@ xfs_scrub \- check and repair the contents of a mounted XFS filesystem
> > ]
> > .I mount-point
> > .br
> > -.B xfs_scrub \-V
> > +.B xfs_scrub \-V | \-F
> > .SH DESCRIPTION
> > .B xfs_scrub
> > attempts to check and repair all metadata in a mounted XFS filesystem.
> > @@ -76,6 +76,9 @@ If
> > is given, no action is taken if errors are found; this is the default
> > behavior.
> > .TP
> > +.B \-F
> > +Prints the version number along with optional build-time features and exits.
> > +.TP
> > .B \-k
> > Do not call TRIM on the free space.
> > .TP
> > diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
> > index bc2e84a7..9e9a098c 100644
> > --- a/scrub/xfs_scrub.c
> > +++ b/scrub/xfs_scrub.c
> > @@ -582,6 +582,13 @@ report_outcome(
> > }
> > }
> >
> > +/* Compile-time features discoverable via version strings */
> > +#ifdef HAVE_LIBICU
> > +# define XFS_SCRUB_HAVE_UNICODE "+"
> > +#else
> > +# define XFS_SCRUB_HAVE_UNICODE "-"
> > +#endif
> > +
> > int
> > main(
> > int argc,
> > @@ -613,7 +620,7 @@ main(
> > pthread_mutex_init(&ctx.lock, NULL);
> > ctx.mode = SCRUB_MODE_REPAIR;
> > ctx.error_action = ERRORS_CONTINUE;
> > - while ((c = getopt(argc, argv, "a:bC:de:km:nTvxV")) != EOF) {
> > + while ((c = getopt(argc, argv, "a:bC:de:Fkm:nTvxV")) != EOF) {
> > switch (c) {
> > case 'a':
> > ctx.max_errors = cvt_u64(optarg, 10);
> > @@ -654,6 +661,12 @@ main(
> > usage();
> > }
> > break;
> > + case 'F':
> > + fprintf(stdout, _("%s version %s %sUnicode\n"),
> > + progname, VERSION,
> > + XFS_SCRUB_HAVE_UNICODE);
> > + fflush(stdout);
> > + return SCRUB_RET_SUCCESS;
>
> Works for me!
Actually, I take it back, let's keep -F unused for now and simply make
'-VV' the magic command that triggers the feature reporting. I'll send
a v3 with this.
--D
>
> --D
>
> > case 'k':
> > want_fstrim = false;
> > break;
