On Mon, Feb 12, 2018 at 9:49 PM, Eric Sandeen <sandeen@xxxxxxxxxxx> 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.
>
> Anyway, for the changes here:
>
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>
I read it as "do check, but do not repair". The previous "dry run"
description was unclear about that (at least from my understanding of
"dry run" phrase), but the changed version seems clear to me.
Reviewed-by: Jan Tulak <jtulak@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
