On Mon, Feb 12, 2018 at 02:49:01PM -0600, Eric Sandeen wrote:
>
>
> On 2/5/18 5:22 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > While it's true that the kernel can tell us whether something needs
> > repairs or it needs optimizing, from the admin's perspective there's
> > no point in having an optimize-only mode -- either fix everything, or
> > don't. This is what xfs_repair does w.r.t. -n, so let's do the same
> > thing too.
>
> <ok and a few other changes but sure> :)
>
> nitpick, does -n skip the "read all the data blocks" pass? I'm not sure
> the manpage really addresses the data block scrubbing yet.
-n will not skip the data block scrub, it only prevents metadata repairs.
--D
> Anyway, for the changes here:
>
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > man/man8/xfs_scrub.8 | 51 +++++++++++++++++++++++---------------------------
> > scrub/phase1.c | 6 +-----
> > scrub/phase4.c | 19 -------------------
> > scrub/scrub.c | 2 +-
> > scrub/xfs_scrub.c | 33 +++++++-------------------------
> > scrub/xfs_scrub.h | 3 ---
> > 6 files changed, 32 insertions(+), 82 deletions(-)
> >
> >
> > diff --git a/man/man8/xfs_scrub.8 b/man/man8/xfs_scrub.8
> > index 4c394a5..77fed92 100644
> > --- a/man/man8/xfs_scrub.8
> > +++ b/man/man8/xfs_scrub.8
> > @@ -1,10 +1,10 @@
> > .TH xfs_scrub 8
> > .SH NAME
> > -xfs_scrub \- scrub the contents of an XFS filesystem
> > +xfs_scrub \- check the contents of a mounted XFS filesystem
> > .SH SYNOPSIS
> > .B xfs_scrub
> > [
> > -.B \-abCemnTvxy
> > +.B \-abCemnTvx
> > ]
> > .RI "[" mount-point " | " block-device "]"
> > .br
> > @@ -13,6 +13,12 @@ xfs_scrub \- scrub the contents of an XFS filesystem
> > .B xfs_scrub
> > attempts to check and repair all metadata in a mounted XFS filesystem.
> > .PP
> > +.B WARNING!
> > +This program is
> > +.BR EXPERIMENTAL ","
> > +which means that its behavior and interface
> > +could change at any time!
> > +.PP
> > .B xfs_scrub
> > asks the kernel to scrub all metadata objects in the filesystem.
> > Metadata records are scanned for obviously bad values and then
> > @@ -28,19 +34,17 @@ the standard error stream.
> > Enabling verbose mode will increase the amount of status information
> > sent to the output.
> > .PP
> > -This utility does not know how to correct all errors.
> > -If the tool cannot fix the detected errors, you must unmount the
> > -filesystem and run
> > +If the kernel scrub reports that metadata needs repairs or optimizations and
> > +the user does not pass
> > +.B -n
> > +on the command line, this program will ask the kernel to make the repairs and
> > +to perform the optimizations.
> > +See the sections about optimizations and repairs for a list of optimizations
> > +and repairs known to this program.
> > +The kernel may not support repairing or optimizing the filesystem.
> > +If this is the case, the filesystem must be unmounted and
> > .BR xfs_repair (8)
> > -to fix the problems.
> > -If this tool is not run with either of the
> > -.B \-n
> > -or
> > -.B \-y
> > -options, then it will optimize the filesystem when possible,
> > -but it will not try to fix errors.
> > -See the optimizations section below for a list of optimizations
> > -supported by this program.
> > +run on the filesystem to fix the problems.
> > .SH OPTIONS
> > .TP
> > .BI \-a " errors"
> > @@ -73,14 +77,14 @@ is given, no action is taken if errors are found; this is the default
> > behavior.
> > .TP
> > .B \-k
> > -Do not call FITRIM on the free space.
> > +Do not call TRIM on the free space.
> > .TP
> > .BI \-m " file"
> > Search this file for mounted filesystems instead of /etc/mtab.
> > .TP
> > .B \-n
> > -Dry run, do not modify anything in the filesystem.
> > -This disables all optimization and repair behaviors.
> > +Only check filesystem metadata.
> > +Do not repair or optimize anything.
> > .TP
> > .BI \-T
> > Print timing and memory usage information for each phase.
> > @@ -98,20 +102,11 @@ will issue O_DIRECT reads to the block device directly.
> > If the block device is a SCSI disk, it will instead issue READ VERIFY commands
> > directly to the disk.
> > These actions will confirm that all file data blocks can be read from storage.
> > -.TP
> > -.B \-y
> > -Try to repair all filesystem errors.
> > -If the errors cannot be fixed online, then the filesystem must be taken
> > -offline for repair.
> > .SH OPTIMIZATIONS
> > -Optimizations supported by this program include:
> > +Optimizations supported by this program include, but are not limited to:
> > .IP \[bu] 2
> > -Updating secondary superblocks to match the primary superblock.
> > -.IP \[bu]
> > -Turning off shared block write checks for files that no longer share blocks.
> > -.IP \[bu]
> > Instructing the underlying storage to discard unused extents via the
> > -.B FITRIM
> > +.B TRIM
> > ioctl.
> > .SH REPAIRS
> > This program currently does not support making any repairs.
> > diff --git a/scrub/phase1.c b/scrub/phase1.c
> > index 75da296..af93d0f 100644
> > --- a/scrub/phase1.c
> > +++ b/scrub/phase1.c
> > @@ -181,11 +181,7 @@ _("Kernel metadata scrubbing facility is not available."));
> >
> > /* Do we need kernel-assisted metadata repair? */
> > if (ctx->mode != SCRUB_MODE_DRY_RUN && !xfs_can_repair(ctx)) {
> > - if (ctx->mode == SCRUB_MODE_PREEN)
> > - str_error(ctx, ctx->mntpoint,
> > -_("Kernel metadata optimization facility is not available. Use -n to scrub."));
> > - else
> > - str_error(ctx, ctx->mntpoint,
> > + str_error(ctx, ctx->mntpoint,
> > _("Kernel metadata repair facility is not available. Use -n to scrub."));
> > return false;
> > }
> > diff --git a/scrub/phase4.c b/scrub/phase4.c
> > index 3100d75..1fb8da9 100644
> > --- a/scrub/phase4.c
> > +++ b/scrub/phase4.c
> > @@ -62,25 +62,6 @@ xfs_repair_fs(
> > return xfs_process_action_items(ctx);
> > }
> >
> > -/* Run the optimize-only phase if there are no errors. */
> > -bool
> > -xfs_optimize_fs(
> > - struct scrub_ctx *ctx)
> > -{
> > - /*
> > - * In preen mode, corruptions are immediately recorded as errors,
> > - * so if there are any corruptions on the filesystem errors_found
> > - * will be non-zero and we won't do anything.
> > - */
> > - if (ctx->errors_found) {
> > - str_info(ctx, ctx->mntpoint,
> > -_("Errors found, please re-run with -y."));
> > - return true;
> > - }
> > -
> > - return xfs_process_action_items(ctx);
> > -}
> > -
> > /* Estimate how much work we're going to do. */
> > bool
> > xfs_estimate_repair_work(
> > diff --git a/scrub/scrub.c b/scrub/scrub.c
> > index 6abca2a..bc0e2f0 100644
> > --- a/scrub/scrub.c
> > +++ b/scrub/scrub.c
> > @@ -279,7 +279,7 @@ _("Repairs are required."));
> > * otherwise complain.
> > */
> > if (is_unoptimized(meta)) {
> > - if (ctx->mode < SCRUB_MODE_PREEN) {
> > + if (ctx->mode != SCRUB_MODE_REPAIR) {
> > if (!is_inode) {
> > /* AG or FS metadata, always warn. */
> > str_info(ctx, buf,
> > diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
> > index 5ab557d..6efcf77 100644
> > --- a/scrub/xfs_scrub.c
> > +++ b/scrub/xfs_scrub.c
> > @@ -187,7 +187,6 @@ usage(void)
> > fprintf(stderr, _(" -v Verbose output.\n"));
> > fprintf(stderr, _(" -V Print version.\n"));
> > fprintf(stderr, _(" -x Scrub file data too.\n"));
> > - fprintf(stderr, _(" -y Repair all errors.\n"));
> >
> > exit(SCRUB_RET_SYNTAX);
> > }
> > @@ -441,16 +440,11 @@ run_scrub_phases(
> > /* Turn on certain phases if user said to. */
> > if (sp->fn == DATASCAN_DUMMY_FN && scrub_data) {
> > sp->fn = xfs_scan_blocks;
> > - } else if (sp->fn == REPAIR_DUMMY_FN) {
> > - if (ctx->mode == SCRUB_MODE_PREEN) {
> > - sp->descr = _("Optimize filesystem.");
> > - sp->fn = xfs_optimize_fs;
> > - sp->must_run = true;
> > - } else if (ctx->mode == SCRUB_MODE_REPAIR) {
> > - sp->descr = _("Repair filesystem.");
> > - sp->fn = xfs_repair_fs;
> > - sp->must_run = true;
> > - }
> > + } else if (sp->fn == REPAIR_DUMMY_FN &&
> > + ctx->mode == SCRUB_MODE_REPAIR) {
> > + sp->descr = _("Repair filesystem.");
> > + sp->fn = xfs_repair_fs;
> > + sp->must_run = true;
> > }
> >
> > /* Skip certain phases unless they're turned on. */
> > @@ -524,9 +518,9 @@ main(
> > textdomain(PACKAGE);
> >
> > pthread_mutex_init(&ctx.lock, NULL);
> > - ctx.mode = SCRUB_MODE_DEFAULT;
> > + ctx.mode = SCRUB_MODE_REPAIR;
> > ctx.error_action = ERRORS_CONTINUE;
> > - while ((c = getopt(argc, argv, "a:bC:de:km:nTvxVy")) != EOF) {
> > + while ((c = getopt(argc, argv, "a:bC:de:km:nTvxV")) != EOF) {
> > switch (c) {
> > case 'a':
> > ctx.max_errors = cvt_u64(optarg, 10);
> > @@ -574,11 +568,6 @@ main(
> > mtab = optarg;
> > break;
> > case 'n':
> > - if (ctx.mode != SCRUB_MODE_DEFAULT) {
> > - fprintf(stderr,
> > -_("Only one of the options -n or -y may be specified.\n"));
> > - usage();
> > - }
> > ctx.mode = SCRUB_MODE_DRY_RUN;
> > break;
> > case 'T':
> > @@ -595,14 +584,6 @@ _("Only one of the options -n or -y may be specified.\n"));
> > case 'x':
> > scrub_data = true;
> > break;
> > - case 'y':
> > - if (ctx.mode != SCRUB_MODE_DEFAULT) {
> > - fprintf(stderr,
> > -_("Only one of the options -n or -y may be specified.\n"));
> > - usage();
> > - }
> > - ctx.mode = SCRUB_MODE_REPAIR;
> > - break;
> > case '?':
> > /* fall through */
> > default:
> > diff --git a/scrub/xfs_scrub.h b/scrub/xfs_scrub.h
> > index 8407885..89b46a4 100644
> > --- a/scrub/xfs_scrub.h
> > +++ b/scrub/xfs_scrub.h
> > @@ -34,10 +34,8 @@ extern bool stdout_isatty;
> >
> > enum scrub_mode {
> > SCRUB_MODE_DRY_RUN,
> > - SCRUB_MODE_PREEN,
> > SCRUB_MODE_REPAIR,
> > };
> > -#define SCRUB_MODE_DEFAULT SCRUB_MODE_PREEN
> >
> > enum error_action {
> > ERRORS_CONTINUE,
> > @@ -111,7 +109,6 @@ bool xfs_scan_connections(struct scrub_ctx *ctx);
> > bool xfs_scan_blocks(struct scrub_ctx *ctx);
> > bool xfs_scan_summary(struct scrub_ctx *ctx);
> > bool xfs_repair_fs(struct scrub_ctx *ctx);
> > -bool xfs_optimize_fs(struct scrub_ctx *ctx);
> >
> > /* Progress estimator functions */
> > uint64_t xfs_estimate_inodes(struct scrub_ctx *ctx);
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
