From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: sandeen@redhat.com, linux-xfs@vger.kernel.org
Subject: Re: [PATCH 03/27] xfs_scrub: set up command line argument parsing
Date: Thu, 11 Jan 2018 18:03:07 -0800 [thread overview]
Message-ID: <20180112020307.GU5602@magnolia> (raw)
In-Reply-To: <e9e65539-8811-af76-ebf6-0d75cc261401@sandeen.net>
On Thu, Jan 11, 2018 at 07:30:11PM -0600, Eric Sandeen wrote:
> On 1/5/18 7:51 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > Parse command line options in order to set up the context in which we
> > will scrub the filesystem.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> > scrub/common.h | 8 ++
> > scrub/xfs_scrub.c | 207 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > scrub/xfs_scrub.h | 34 +++++++++
> > 3 files changed, 249 insertions(+)
> >
> >
> > diff --git a/scrub/common.h b/scrub/common.h
> > index f620620..15a59bd 100644
> > --- a/scrub/common.h
> > +++ b/scrub/common.h
> > @@ -48,4 +48,12 @@ void __record_preen(struct scrub_ctx *ctx, const char *descr, const char *file,
> > #define str_info(ctx, str, ...) __str_info(ctx, str, __FILE__, __LINE__, __VA_ARGS__)
> > #define dbg_printf(fmt, ...) {if (debug > 1) {printf(fmt, __VA_ARGS__);}}
> >
> > +/* Is this debug tweak enabled? */
> > +static inline bool
> > +debug_tweak_on(
> > + const char *name)
> > +{
> > + return debug && getenv(name) != NULL;
>
> since it's debug anyway, I wonder if printing
> "FOO_BAR_TWEAK is on" would be useful here.
>
> > +}
> > +
> > #endif /* XFS_SCRUB_COMMON_H_ */
> > diff --git a/scrub/xfs_scrub.c b/scrub/xfs_scrub.c
> > index 10116a8..9db3b41 100644
> > --- a/scrub/xfs_scrub.c
> > +++ b/scrub/xfs_scrub.c
> > @@ -20,7 +20,12 @@
> > #include <stdio.h>
> > #include <pthread.h>
> > #include <stdbool.h>
> > +#include <stdlib.h>
> > +#include "platform_defs.h"
> > +#include "xfs.h"
> > +#include "input.h"
> > #include "xfs_scrub.h"
> > +#include "common.h"
> >
> > /*
> > * XFS Online Metadata Scrub (and Repair)
> > @@ -107,11 +112,213 @@ unsigned int debug;
> > /* Should we dump core if errors happen? */
> > bool dumpcore;
> >
> > +/* Display resource usage at the end of each phase? */
> > +bool display_rusage;
> > +
> > +/* Background mode; higher values insert more pauses between scrub calls. */
> > +unsigned int bg_mode;
> > +
> > +/* Maximum number of processors available to us. */
> > +int nproc;
> > +
> > +/* Number of threads we're allowed to use. */
> > +unsigned int nr_threads;
> > +
> > +/* Verbosity; higher values print more information. */
> > +bool verbose;
> > +
> > +/* Should we scrub the data blocks? */
> > +bool scrub_data;
> > +
> > +/* Size of a memory page. */
> > +long page_size;
> > +
> > +static void __attribute__((noreturn))
> > +usage(void)
> > +{
> > + fprintf(stderr, _("Usage: %s [OPTIONS] mountpoint\n"), progname);
> > + fprintf(stderr, _("-a:\tStop after this many errors are found.\n"));
> > + fprintf(stderr, _("-b:\tBackground mode.\n"));
> > + fprintf(stderr, _("-e:\tWhat to do if errors are found.\n"));
> > + fprintf(stderr, _("-m:\tPath to /etc/mtab.\n"));
> > + fprintf(stderr, _("-n:\tDry run. Do not modify anything.\n"));
> > + fprintf(stderr, _("-T:\tDisplay timing/usage information.\n"));
> > + fprintf(stderr, _("-v:\tVerbose output.\n"));
> > + fprintf(stderr, _("-V:\tPrint version.\n"));
> > + fprintf(stderr, _("-x:\tScrub file data too.\n"));
> > + fprintf(stderr, _("-y:\tRepair all errors.\n"));
> > +
> > + exit(16);
> > +}
> > +
> > int
> > main(
> > int argc,
> > char **argv)
> > {
> > + int c;
> > + char *mtab = NULL;
> > + char *repairstr = "";
> > + struct scrub_ctx ctx = {0};
> > + unsigned long long total_errors;
> > + bool moveon = true;
> > + static bool injected;
> > + int ret = 0;
> > +
> > fprintf(stderr, "XXX: This program is not complete!\n");
> > return 4;
> > +
> > + progname = basename(argv[0]);
> > + setlocale(LC_ALL, "");
> > + bindtextdomain(PACKAGE, LOCALEDIR);
> > + textdomain(PACKAGE);
> > +
> > + pthread_mutex_init(&ctx.lock, NULL);
> > + ctx.mode = SCRUB_MODE_DEFAULT;
> > + ctx.error_action = ERRORS_CONTINUE;
> > + while ((c = getopt(argc, argv, "a:bde:m:nTvxVy")) != EOF) {
> > + switch (c) {
> > + case 'a':
> > + ctx.max_errors = cvt_u64(optarg, 10);
> > + if (errno) {
> > + perror(optarg);
> > + usage();
> > + }
> > + break;
> > + case 'b':
> > + nr_threads = 1;
> > + bg_mode++;
> > + break;
> > + case 'd':
> > + debug++;
> > + dumpcore = true;
> > + break;
> > + case 'e':
> > + if (!strcmp("continue", optarg))
> > + ctx.error_action = ERRORS_CONTINUE;
> > + else if (!strcmp("shutdown", optarg))
> > + ctx.error_action = ERRORS_SHUTDOWN;
> > + else
> > + usage();
>
> Nothing tells me what I did wrong here,
>
> # scrub/xfs_scrub -e make_it_so /mnt/test
> Usage: xfs_scrub [OPTIONS] mountpoint
> -a: Stop after this many errors are found.
> -b: Background mode.
> -C: Print progress information to this fd.
> -e: What to do if errors are found.
> ...
>
> I told it what to do... what's wrong?
Unknown error behavior "$optarg". ?
> > + break;
> > + case 'm':
> > + 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"));
> > + return 1;
> > + }
> > + ctx.mode = SCRUB_MODE_DRY_RUN;
> > + break;
> > + case 'T':
> > + display_rusage = true;
> > + break;
> > + case 'v':
> > + verbose = true;
> > + break;
> > + case 'V':
> > + fprintf(stdout, _("%s version %s\n"), progname,
> > + VERSION);
> > + fflush(stdout);
> > + exit(0);
> > + 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"));
> > + return 1;
> > + }
> > + ctx.mode = SCRUB_MODE_REPAIR;
> > + break;
> > + case '?':
>
> '?' isn't in the getopt string ...
The getopt manpage says it returns '?' for an unknown parameter, so I
provide the specific case here so that nobody can accidentally add a
second (case '?') statement.
IOWs, it's a defensive move.
> # scrub/xfs_scrub ?
> xfs_scrub: could not stat: ?: No such file or directory
>
>
> > + /* fall through */
> > + default:
> > + usage();
> > + }
> > + }
> > +
> > + /* Override thread count if debugger */
> > + if (debug_tweak_on("XFS_SCRUB_THREADS")) {
>
> can you document all these tweaks somewhere near the top in a comment?
/*
* Known debug tweaks (pass -d and set the environment variable):
* XFS_SCRUB_FORCE_ERROR -- pretend all metadata is corrupt
* XFS_SCRUB_FORCE_REPAIR -- repair all metadata even if it's ok
* XFS_SCRUB_NO_KERNEL -- pretend there is no kernel ioctl
* XFS_SCRUB_NO_SCSI_VERIFY -- disable SCSI VERIFY (if present)
* XFS_SCRUB_PHASE -- run only this scrub phase
* XFS_SCRUB_THREADS -- start exactly this number of threads
*/
> > + unsigned int x;
> > +
> > + x = cvt_u32(getenv("XFS_SCRUB_THREADS"), 10);
> > + if (errno) {
> > + perror("nr_threads");
> > + usage();
> > + }
> > + nr_threads = x;
> > + }
> > +
> > + if (optind != argc - 1)
> > + usage();
> > +
> > + ctx.mntpoint = strdup(argv[optind]);
> > +
> > + /*
> > + * If the user did not specify an explicit mount table, try to use
> > + * /proc/mounts if it is available, else /etc/mtab. We prefer
> > + * /proc/mounts because it is kernel controlled, while /etc/mtab
> > + * may contain garbage that userspace tools like pam_mounts wrote
> > + * into it.
> > + */
> > + if (!mtab) {
> > + if (access(_PATH_PROC_MOUNTS, R_OK) == 0)
> > + mtab = _PATH_PROC_MOUNTS;
> > + else
> > + mtab = _PATH_MOUNTED;
> > + }
> > +
> > + /* How many CPUs? */
> > + nproc = sysconf(_SC_NPROCESSORS_ONLN);
> > + if (nproc < 1)
> > + nproc = 1;
> > +
> > + /* Set up a page-aligned buffer for read verification. */
> > + page_size = sysconf(_SC_PAGESIZE);
> > + if (page_size < 0) {
> > + str_errno(&ctx, ctx.mntpoint);
> > + goto out;
> > + }
> > +
> > + if (debug_tweak_on("XFS_SCRUB_FORCE_REPAIR") && !injected) {
> > + ctx.mode = SCRUB_MODE_REPAIR;
> > + injected = true;
> > + }
>
> what is "injected" used for? How could it already be set?.
Not needed here, will remove.
> > +
> > + if (xfs_scrub_excessive_errors(&ctx))
> > + str_info(&ctx, ctx.mntpoint, _("Too many errors; aborting."));
>
> wait wut? oh right, you'll add $DO_STUFF above this in later patches ;)
>
>
> Rest looks ok
Ok.
--D
>
> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-01-12 2:03 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-06 1:51 [PATCH v11 00/27] xfsprogs: online scrub/repair support Darrick J. Wong
2018-01-06 1:51 ` [PATCH 01/27] xfs_scrub: create online filesystem scrub program Darrick J. Wong
2018-01-12 0:16 ` Eric Sandeen
2018-01-12 1:08 ` Darrick J. Wong
2018-01-12 1:07 ` Eric Sandeen
2018-01-12 1:10 ` Darrick J. Wong
2018-01-06 1:51 ` [PATCH 02/27] xfs_scrub: common error handling Darrick J. Wong
2018-01-12 1:15 ` Eric Sandeen
2018-01-12 1:23 ` Darrick J. Wong
2018-01-06 1:51 ` [PATCH 03/27] xfs_scrub: set up command line argument parsing Darrick J. Wong
2018-01-11 23:39 ` Eric Sandeen
2018-01-12 1:53 ` Darrick J. Wong
2018-01-12 1:30 ` Eric Sandeen
2018-01-12 2:03 ` Darrick J. Wong [this message]
2018-01-06 1:51 ` [PATCH 04/27] xfs_scrub: dispatch the various phases of the scrub program Darrick J. Wong
2018-01-06 1:51 ` [PATCH 05/27] xfs_scrub: figure out how many threads we're going to need Darrick J. Wong
2018-01-06 1:52 ` [PATCH 06/27] xfs_scrub: create an abstraction for a block device Darrick J. Wong
2018-01-11 23:24 ` Eric Sandeen
2018-01-11 23:59 ` Darrick J. Wong
2018-01-12 0:04 ` Eric Sandeen
2018-01-12 1:27 ` Darrick J. Wong
2018-01-06 1:52 ` [PATCH 07/27] xfs_scrub: find XFS filesystem geometry Darrick J. Wong
2018-01-06 1:52 ` [PATCH 08/27] xfs_scrub: add inode iteration functions Darrick J. Wong
2018-01-06 1:52 ` [PATCH 09/27] xfs_scrub: add space map " Darrick J. Wong
2018-01-06 1:52 ` [PATCH 10/27] xfs_scrub: add file " Darrick J. Wong
2018-01-11 23:19 ` Eric Sandeen
2018-01-12 0:24 ` Darrick J. Wong
2018-01-06 1:52 ` [PATCH 11/27] xfs_scrub: filesystem counter collection functions Darrick J. Wong
2018-01-06 1:52 ` [PATCH 12/27] xfs_scrub: wrap the scrub ioctl Darrick J. Wong
2018-01-11 23:12 ` Eric Sandeen
2018-01-12 0:28 ` Darrick J. Wong
2018-01-06 1:52 ` [PATCH 13/27] xfs_scrub: scan filesystem and AG metadata Darrick J. Wong
2018-01-06 1:52 ` [PATCH 14/27] xfs_scrub: thread-safe stats counter Darrick J. Wong
2018-01-06 1:53 ` [PATCH 15/27] xfs_scrub: scan inodes Darrick J. Wong
2018-01-06 1:53 ` [PATCH 16/27] xfs_scrub: check directory connectivity Darrick J. Wong
2018-01-06 1:53 ` [PATCH 17/27] xfs_scrub: warn about suspicious characters in directory/xattr names Darrick J. Wong
2018-01-06 1:53 ` [PATCH 18/27] xfs_scrub: warn about normalized Unicode name collisions Darrick J. Wong
2018-01-16 23:52 ` Eric Sandeen
2018-01-16 23:57 ` Eric Sandeen
2018-01-16 23:59 ` Darrick J. Wong
2018-01-06 1:53 ` [PATCH 19/27] xfs_scrub: create a bitmap data structure Darrick J. Wong
2018-01-06 1:53 ` [PATCH 20/27] xfs_scrub: create infrastructure to read verify data blocks Darrick J. Wong
2018-01-06 1:53 ` [PATCH 21/27] xfs_scrub: scrub file " Darrick J. Wong
2018-01-11 23:25 ` Eric Sandeen
2018-01-12 0:29 ` Darrick J. Wong
2018-01-06 1:53 ` [PATCH 22/27] xfs_scrub: optionally use SCSI READ VERIFY commands to scrub data blocks on disk Darrick J. Wong
2018-01-06 1:53 ` [PATCH 23/27] xfs_scrub: check summary counters Darrick J. Wong
2018-01-06 1:54 ` [PATCH 24/27] xfs_scrub: fstrim the free areas if there are no errors on the filesystem Darrick J. Wong
2018-01-16 22:07 ` Eric Sandeen
2018-01-16 22:23 ` Darrick J. Wong
2018-01-06 1:54 ` [PATCH 25/27] xfs_scrub: progress indicator Darrick J. Wong
2018-01-11 23:27 ` Eric Sandeen
2018-01-12 0:32 ` Darrick J. Wong
2018-01-06 1:54 ` [PATCH 26/27] xfs_scrub: create a script to scrub all xfs filesystems Darrick J. Wong
2018-01-06 1:54 ` [PATCH 27/27] xfs_scrub: integrate services with systemd Darrick J. Wong
2018-01-06 3:50 ` [PATCH 07/27] xfs_scrub: find XFS filesystem geometry Darrick J. Wong
2018-01-12 4:17 ` [PATCH v11 00/27] xfsprogs: online scrub/repair support Eric Sandeen
2018-01-17 1:31 ` Darrick J. Wong
2018-01-16 19:21 ` [PATCH 28/27] xfs_scrub: wire up repair ioctl Darrick J. Wong
2018-01-16 19:21 ` [PATCH 29/27] xfs_scrub: schedule and manage repairs to the filesystem Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2017-11-17 21:00 [PATCH v10 00/27] xfsprogs-4.15: online scrub/repair support Darrick J. Wong
2017-11-17 21:00 ` [PATCH 03/27] xfs_scrub: set up command line argument parsing Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180112020307.GU5602@magnolia \
--to=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
--cc=sandeen@redhat.com \
--cc=sandeen@sandeen.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.