Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 12 Aug 2019 18:50:26 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r350929 - in stable/12/sbin/nvmecontrol: . modules/wdc
Message-ID:  <201908121850.x7CIoQ4K059320@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Mon Aug 12 18:50:26 2019
New Revision: 350929
URL: https://svnweb.freebsd.org/changeset/base/350929

Log:
  MFC r350057 (by imp): Create generic command / arg parsing routines
  
  Create a set of routines and structures to hold the data for the args
  for a command. Use them to generate help and to parse args. Convert
  all the current commands over to the new format. "comnd" is a hat-tip
  to the TOPS-20 %COMND JSYS that (very) loosely inspired much of the
  subsequent command line notions in the industry, but this is far
  simpler (the %COMND man page is longer than this code) and not in the
  kernel... Also, it implements today's de-facto
          command [verb]+ [opts]* [args]*
  format rather than the old, archaic TOPS-20 command format :)
  
  This is a snapshot of a work in progress to get the nvme passthru
  stuff committed. In time it will become a private library and used
  by some other programs in the tree that conform to the above pattern.

Added:
  stable/12/sbin/nvmecontrol/comnd.c
     - copied unchanged from r350057, head/sbin/nvmecontrol/comnd.c
  stable/12/sbin/nvmecontrol/comnd.h
     - copied unchanged from r350057, head/sbin/nvmecontrol/comnd.h
Modified:
  stable/12/sbin/nvmecontrol/Makefile
  stable/12/sbin/nvmecontrol/devlist.c
  stable/12/sbin/nvmecontrol/firmware.c
  stable/12/sbin/nvmecontrol/format.c
  stable/12/sbin/nvmecontrol/identify.c
  stable/12/sbin/nvmecontrol/logpage.c
  stable/12/sbin/nvmecontrol/modules/wdc/wdc.c
  stable/12/sbin/nvmecontrol/ns.c
  stable/12/sbin/nvmecontrol/nvmecontrol.c
  stable/12/sbin/nvmecontrol/nvmecontrol.h
  stable/12/sbin/nvmecontrol/perftest.c
  stable/12/sbin/nvmecontrol/power.c
  stable/12/sbin/nvmecontrol/reset.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sbin/nvmecontrol/Makefile
==============================================================================
--- stable/12/sbin/nvmecontrol/Makefile	Mon Aug 12 18:49:32 2019	(r350928)
+++ stable/12/sbin/nvmecontrol/Makefile	Mon Aug 12 18:50:26 2019	(r350929)
@@ -2,10 +2,13 @@
 
 PACKAGE=runtime
 PROG=	nvmecontrol
-SRCS=	nvmecontrol.c devlist.c firmware.c format.c identify.c identify_ext.c logpage.c \
-	perftest.c reset.c ns.c nvme_util.c power.c nc_util.c
+SRCS=	comnd.c nvmecontrol.c
+SRCS+=	devlist.c firmware.c format.c identify.c logpage.c ns.c perftest.c power.c reset.c
+#SRCS+=	passthru.c
+SRCS+=	identify_ext.c nvme_util.c nc_util.c
 MAN=	nvmecontrol.8
 LDFLAGS+= -rdynamic
+LIBADD+= util
 SUBDIR=	modules
 
 .PATH:	${SRCTOP}/sys/dev/nvme

Copied: stable/12/sbin/nvmecontrol/comnd.c (from r350057, head/sbin/nvmecontrol/comnd.c)
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ stable/12/sbin/nvmecontrol/comnd.c	Mon Aug 12 18:50:26 2019	(r350929, copy of r350057, head/sbin/nvmecontrol/comnd.c)
@@ -0,0 +1,326 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (C) 2019 Netflix, Inc
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <sys/cdefs.h>
+__FBSDID("$FreeBSD$");
+
+#include <sys/param.h>
+#include <sys/ioccom.h>
+
+#include <ctype.h>
+#include <dirent.h>
+#include <dlfcn.h>
+#include <err.h>
+#include <fcntl.h>
+#include <getopt.h>
+#include <libutil.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include "comnd.h"
+
+static struct cmd top;
+
+static void
+print_usage(const struct cmd *f)
+{
+
+	fprintf(stderr, "    %s %-15s - %s\n", getprogname(), f->name, f->descr);
+}
+
+static void
+gen_usage(const struct cmd *t)
+{
+	struct cmd *walker;
+
+	fprintf(stderr, "usage:\n");
+	SLIST_FOREACH(walker, &t->subcmd, link) {
+		print_usage(walker);
+	}
+	exit(1);
+}
+
+int
+cmd_dispatch(int argc, char *argv[], const struct cmd *t)
+{
+	struct cmd *walker;
+
+	if (t == NULL)
+		t = &top;
+
+	if (argv[1] == NULL) {
+		gen_usage(t);
+		return (1);
+	}
+	SLIST_FOREACH(walker, &t->subcmd, link) {
+		if (strcmp(argv[1], walker->name) == 0) {
+			walker->fn(walker, argc-1, &argv[1]);
+			return (0);
+		}
+	}
+	fprintf(stderr, "Unknown command: %s\n", argv[1]);
+	gen_usage(t);
+	return (1);
+}
+
+static void
+arg_suffix(char *buf, size_t len, arg_type at)
+{
+	switch (at) {
+	case arg_none:
+		break;
+	case arg_string:
+		strlcat(buf, "=<STRING>", len);
+		break;
+	case arg_path:
+		strlcat(buf, "=<FILE>", len);
+		break;
+	default:
+		strlcat(buf, "=<NUM>", len);
+		break;
+	}
+}
+
+void
+arg_help(int argc __unused, char * const *argv, const struct cmd *f)
+{
+	int i;
+	char buf[31];
+	const struct opts *opts = f->opts;
+	const struct args *args = f->args;
+
+	// XXX walk up the cmd list...
+	if (argv[optind])
+		fprintf(stderr, "Unknown argument: %s\n", argv[optind]);
+	fprintf(stderr, "Usage:\n    %s %s", getprogname(), argv[0]);
+	if (opts)
+		fprintf(stderr, " <args>");
+	if (args) {
+		while (args->descr != NULL) {
+			fprintf(stderr, " %s", args->descr);
+			args++;
+		}
+	}
+	fprintf(stderr, "\n\n%s\n", f->descr);
+	if (opts != NULL) {
+		fprintf(stderr, "Options:\n");
+		for (i = 0; opts[i].long_arg != NULL; i++) {
+			*buf = '\0';
+			if (isprint(opts[i].short_arg)) {
+				snprintf(buf, sizeof(buf), " -%c, ", opts[i].short_arg);
+			} else {
+				strlcpy(buf, "    ", sizeof(buf));
+			}
+			strlcat(buf, "--", sizeof(buf));
+			strlcat(buf, opts[i].long_arg, sizeof(buf));
+			arg_suffix(buf, sizeof(buf), opts[i].at);
+			fprintf(stderr, "%-30.30s - %s\n", buf, opts[i].descr);
+		}
+	}
+	exit(1);
+}
+
+static int
+find_long(struct option *lopts, int ch)
+{
+	int i;
+
+	for (i = 0; lopts[i].val != ch && lopts[i].name != NULL; i++)
+		continue;
+	return i;
+}
+
+int
+arg_parse(int argc, char * const * argv, const struct cmd *f)
+{
+	int i, n, idx, ch;
+	uint64_t v;
+	struct option *lopts;
+	char *shortopts, *p;
+	const struct opts *opts = f->opts;
+	const struct args *args = f->args;
+
+	if (opts == NULL)
+		n = 0;
+	else
+		for (n = 0; opts[n].long_arg != NULL;)
+			n++;
+	lopts = malloc((n + 2) * sizeof(struct option));
+	if (lopts == NULL)
+		err(1, "option memory");
+	p = shortopts = malloc((n + 3) * sizeof(char));
+	if (shortopts == NULL)
+		err(1, "shortopts memory");
+	idx = 0;
+	for (i = 0; i < n; i++) {
+		lopts[i].name = opts[i].long_arg;
+		lopts[i].has_arg = opts[i].at == arg_none ? no_argument : required_argument;
+		lopts[i].flag = NULL;
+		lopts[i].val = opts[i].short_arg;
+		if (isprint(opts[i].short_arg)) {
+			*p++ = opts[i].short_arg;
+			if (lopts[i].has_arg)
+				*p++ = ':';
+		}
+	}
+	lopts[n].name = "help";
+	lopts[n].has_arg = no_argument;
+	lopts[n].flag = NULL;
+	lopts[n].val = '?';
+	*p++ = '?';
+	*p++ = '\0';
+	memset(lopts + n + 1, 0, sizeof(struct option));
+	while ((ch = getopt_long(argc, argv, shortopts, lopts, &idx)) != -1) {
+		/*
+		 * If ch != 0, we've found a short option, and we have to
+		 * look it up lopts table. Otherwise idx is valid.
+		 */
+		if (ch != 0)
+			idx = find_long(lopts, ch);
+		if (idx == n)
+			arg_help(argc, argv, f);
+		switch (opts[idx].at) {
+		case arg_none:
+			*(bool *)opts[idx].ptr = true;
+			break;
+		case arg_string:
+		case arg_path:
+			*(const char **)opts[idx].ptr = optarg;
+			break;
+		case arg_uint8:
+			v = strtoul(optarg, NULL, 0);
+			if (v > 0xff)
+				goto bad_arg;
+			*(uint8_t *)opts[idx].ptr = v;
+			break;
+		case arg_uint16:
+			v = strtoul(optarg, NULL, 0);
+			if (v > 0xffff)
+				goto bad_arg;
+			*(uint16_t *)opts[idx].ptr = v;
+			break;
+		case arg_uint32:
+			v = strtoul(optarg, NULL, 0);
+			if (v > 0xffffffffu)
+				goto bad_arg;
+			*(uint32_t *)opts[idx].ptr = v;
+			break;
+		case arg_uint64:
+			v = strtoul(optarg, NULL, 0);
+			if (v > 0xffffffffffffffffull)
+				goto bad_arg;
+			*(uint64_t *)opts[idx].ptr = v;
+			break;
+		case arg_size:
+			if (expand_number(optarg, &v) < 0)
+				goto bad_arg;
+			*(uint64_t *)opts[idx].ptr = v;
+			break;
+		}
+	}
+	if (args) {
+		while (args->descr) {
+			if (optind >= argc) {
+				fprintf(stderr, "Missing arg %s\n", args->descr);
+				arg_help(argc, argv, f);
+				return (1);
+			}
+			*(char **)args->ptr = argv[optind++];
+			args++;
+		}
+	}
+	free(lopts);
+	return (0);
+bad_arg:
+	fprintf(stderr, "Bad value to --%s: %s\n", opts[idx].long_arg, optarg);
+	free(lopts);
+	exit(1);
+}
+
+/*
+ * Loads all the .so's from the specified directory.
+ */
+void
+cmd_load_dir(const char *dir __unused, cmd_load_cb_t cb __unused, void *argp __unused)
+{
+	DIR *d;
+	struct dirent *dent;
+	char *path = NULL;
+	void *h;
+
+	d = opendir(dir);
+	if (d == NULL)
+		return;
+	for (dent = readdir(d); dent != NULL; dent = readdir(d)) {
+		if (strcmp(".so", dent->d_name + dent->d_namlen - 3) != 0)
+			continue;
+		asprintf(&path, "%s/%s", dir, dent->d_name);
+		if (path == NULL)
+			err(1, "Can't malloc for path, giving up.");
+		if ((h = dlopen(path, RTLD_NOW | RTLD_GLOBAL)) == NULL)
+			warnx("Can't load %s: %s", path, dlerror());
+		else {
+			if (cb != NULL)
+				cb(argp, h);
+		}
+		free(path);
+		path = NULL;
+	}
+	closedir(d);
+}
+
+void
+cmd_register(struct cmd *up, struct cmd *cmd)
+{
+	struct cmd *walker, *last;
+
+	if (up == NULL)
+		up = &top;
+	SLIST_INIT(&cmd->subcmd);
+	cmd->parent = up;
+	last = NULL;
+	SLIST_FOREACH(walker, &up->subcmd, link) {
+		if (strcmp(walker->name, cmd->name) > 0)
+			break;
+		last = walker;
+	}
+	if (last == NULL) {
+		SLIST_INSERT_HEAD(&up->subcmd, cmd, link);
+	} else {
+		SLIST_INSERT_AFTER(last, cmd, link);
+	}
+}
+
+void
+cmd_init(void)
+{
+
+}

Copied: stable/12/sbin/nvmecontrol/comnd.h (from r350057, head/sbin/nvmecontrol/comnd.h)
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ stable/12/sbin/nvmecontrol/comnd.h	Mon Aug 12 18:50:26 2019	(r350929, copy of r350057, head/sbin/nvmecontrol/comnd.h)
@@ -0,0 +1,102 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2019 Netflix, Inc
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+#ifndef	COMND_H
+#define	COMND_H
+
+#include <sys/queue.h>
+#include <sys/linker_set.h>
+
+/*
+ * Regularized parsing of simple arguments built on top of getopt_long.
+ */
+
+typedef enum arg_type {
+	arg_none = 0,
+	arg_uint8,
+	arg_uint16,
+	arg_uint32,
+	arg_uint64,
+	arg_size,
+	arg_string,
+	arg_path,
+} arg_type;
+
+// XXX need to change to offsetof for opts and args.
+// we then need to allocate ctx and pass that into the cmd
+// stuff. this will be a little tricky and we may need to expand
+// arg_type stuff.
+
+struct opts {
+	const char	*long_arg;
+	int		short_arg;
+	arg_type	at;
+	void 		*ptr;			//  XXXX change to offset of
+	const char	*descr;
+};
+
+// XXX TDB: subcommand vs actual argument. maybe with subcmd?
+// XXX TBD: do we need parsing callback functions?
+struct args {
+	arg_type	at;
+	void 		*ptr;			//  XXXX change to offset of
+	const char	*descr;
+};
+
+typedef void (cmd_load_cb_t)(void *, void *);
+struct cmd;
+typedef void (cmd_fn_t)(const struct cmd *nf, int argc, char *argv[]);
+
+struct cmd  {
+	SLIST_ENTRY(cmd)	link;
+	const char		*name;
+	cmd_fn_t		*fn;
+	size_t			ctx_size;
+	const struct opts	*opts;
+	const struct args	*args;
+	const char		*descr;
+	SLIST_HEAD(,cmd)	subcmd;
+	struct cmd		*parent;
+};
+
+void cmd_register(struct cmd *, struct cmd *);
+#define CMD_COMMAND(c)							\
+    static void cmd_register_##c(void) __attribute__((constructor));	\
+    static void cmd_register_##c(void) { cmd_register(NULL, &c); }
+#define CMD_SUBCOMMAND(c,sc)						\
+    static void cmd_register_##c_##sc(void) __attribute__((constructor)); \
+    static void cmd_register_##c_##sc(void) { cmd_register(&c, &sc); }
+
+int arg_parse(int argc, char * const *argv, const struct cmd *f);
+void arg_help(int argc, char * const *argv, const struct cmd *f);
+void cmd_init(void);
+void cmd_load_dir(const char *dir, cmd_load_cb_t *cb, void *argp);
+int cmd_dispatch(int argc, char *argv[], const struct cmd *);
+
+#endif /* COMND_H */

Modified: stable/12/sbin/nvmecontrol/devlist.c
==============================================================================
--- stable/12/sbin/nvmecontrol/devlist.c	Mon Aug 12 18:49:32 2019	(r350928)
+++ stable/12/sbin/nvmecontrol/devlist.c	Mon Aug 12 18:50:26 2019	(r350929)
@@ -42,12 +42,24 @@ __FBSDID("$FreeBSD$");
 #include <unistd.h>
 
 #include "nvmecontrol.h"
+#include "comnd.h"
 
-#define DEVLIST_USAGE							       \
-	"devlist\n"
+/* Tables for command line parsing */
 
 #define NVME_MAX_UNIT 256
 
+static cmd_fn_t devlist;
+
+static struct cmd devlist_cmd = {
+	.name = "devlist",
+	.fn = devlist,
+	.descr = "Display a list of NVMe controllers and namespaces."
+};
+
+CMD_COMMAND(devlist_cmd);
+
+/* End of tables for command line parsing */
+
 static inline uint32_t
 ns_get_sector_size(struct nvme_namespace_data *nsdata)
 {
@@ -62,21 +74,17 @@ ns_get_sector_size(struct nvme_namespace_data *nsdata)
 }
 
 static void
-devlist(const struct nvme_function *nf, int argc, char *argv[])
+devlist(const struct cmd *f, int argc, char *argv[])
 {
 	struct nvme_controller_data	cdata;
 	struct nvme_namespace_data	nsdata;
 	char				name[64];
 	uint8_t				mn[64];
 	uint32_t			i;
-	int				ch, ctrlr, fd, found, ret;
+	int				ctrlr, fd, found, ret;
 
-	while ((ch = getopt(argc, argv, "")) != -1) {
-		switch ((char)ch) {
-		default:
-			usage(nf);
-		}
-	}
+	if (arg_parse(argc, argv, f))
+		return;
 
 	ctrlr = -1;
 	found = 0;
@@ -119,5 +127,3 @@ devlist(const struct nvme_function *nf, int argc, char
 
 	exit(1);
 }
-
-NVME_COMMAND(top, devlist, devlist, DEVLIST_USAGE);

Modified: stable/12/sbin/nvmecontrol/firmware.c
==============================================================================
--- stable/12/sbin/nvmecontrol/firmware.c	Mon Aug 12 18:49:32 2019	(r350928)
+++ stable/12/sbin/nvmecontrol/firmware.c	Mon Aug 12 18:50:26 2019	(r350929)
@@ -50,9 +50,53 @@ __FBSDID("$FreeBSD$");
 
 #include "nvmecontrol.h"
 
-#define FIRMWARE_USAGE							       \
-	"firmware [-s slot] [-f path_to_firmware] [-a] <controller id>\n"
+/* Tables for command line parsing */
 
+static cmd_fn_t firmware;
+
+#define NONE 0xffffffffu
+static struct options {
+	bool		activate;
+	uint32_t	slot;
+	const char	*fw_img;
+	const char	*dev;
+} opt = {
+	.activate = false,
+	.slot = NONE,
+	.fw_img = NULL,
+	.dev = NULL,
+};
+
+static const struct opts firmware_opts[] = {
+#define OPT(l, s, t, opt, addr, desc) { l, s, t, &opt.addr, desc }
+	OPT("activate", 'a', arg_none, opt, activate,
+	    "Attempt to activate firmware"),
+	OPT("slot", 's', arg_uint32, opt, slot,
+	    "Slot to activate and/or download firmware to"),
+	OPT("firmware", 'f', arg_path, opt, fw_img,
+	    "Firmware image to download"),
+	{ NULL, 0, arg_none, NULL, NULL }
+};
+#undef OPT
+
+static const struct args firmware_args[] = {
+	{ arg_string, &opt.dev, "controller-id" },
+	{ arg_none, NULL, NULL },
+};
+
+static struct cmd firmware_cmd = {
+	.name = "firmware",
+	.fn = firmware,
+	.descr = "Download firmware image to controller.",
+	.ctx_size = sizeof(opt),
+	.opts = firmware_opts,
+	.args = firmware_args,
+};
+
+CMD_COMMAND(firmware_cmd);
+
+/* End of tables for command line parsing */
+
 static int
 slot_has_valid_firmware(int fd, int slot)
 {
@@ -69,7 +113,7 @@ slot_has_valid_firmware(int fd, int slot)
 }
 
 static void
-read_image_file(char *path, void **buf, int32_t *size)
+read_image_file(const char *path, void **buf, int32_t *size)
 {
 	struct stat	sb;
 	int32_t		filesize;
@@ -174,74 +218,52 @@ activate_firmware(int fd, int slot, int activate_actio
 }
 
 static void
-firmware(const struct nvme_function *nf, int argc, char *argv[])
+firmware(const struct cmd *f, int argc, char *argv[])
 {
-	int				fd = -1, slot = 0;
-	int				a_flag, f_flag;
+	int				fd = -1;
 	int				activate_action, reboot_required;
-	int				opt;
-	char				*p, *image = NULL;
-	char				*controller = NULL, prompt[64];
+	char				prompt[64];
 	void				*buf = NULL;
 	int32_t				size = 0;
 	uint16_t			oacs_fw;
 	uint8_t				fw_slot1_ro, fw_num_slots;
 	struct nvme_controller_data	cdata;
 
-	a_flag = f_flag = false;
+	if (arg_parse(argc, argv, f))
+		return;
 
-	while ((opt = getopt(argc, argv, "af:s:")) != -1) {
-		switch (opt) {
-		case 'a':
-			a_flag = true;
-			break;
-		case 's':
-			slot = strtol(optarg, &p, 0);
-			if (p != NULL && *p != '\0') {
-				fprintf(stderr,
-				    "\"%s\" not valid slot.\n",
-				    optarg);
-				usage(nf);
-			} else if (slot == 0) {
-				fprintf(stderr,
-				    "0 is not a valid slot number. "
-				    "Slot numbers start at 1.\n");
-				usage(nf);
-			} else if (slot > 7) {
-				fprintf(stderr,
-				    "Slot number %s specified which is "
-				    "greater than max allowed slot number of "
-				    "7.\n", optarg);
-				usage(nf);
-			}
-			break;
-		case 'f':
-			image = optarg;
-			f_flag = true;
-			break;
-		}
+	if (opt.slot == 0) {
+		fprintf(stderr,
+		    "0 is not a valid slot number. "
+		    "Slot numbers start at 1.\n");
+		arg_help(argc, argv, f);
+	} else if (opt.slot > 7 && opt.slot != NONE) {
+		fprintf(stderr,
+		    "Slot number %s specified which is "
+		    "greater than max allowed slot number of "
+		    "7.\n", optarg);
+		arg_help(argc, argv, f);
 	}
 
-	/* Check that a controller (and not a namespace) was specified. */
-	if (optind >= argc || strstr(argv[optind], NVME_NS_PREFIX) != NULL)
-		usage(nf);
-
-	if (!f_flag && !a_flag) {
+	if (!opt.activate && opt.fw_img == NULL) {
 		fprintf(stderr,
 		    "Neither a replace ([-f path_to_firmware]) nor "
 		    "activate ([-a]) firmware image action\n"
 		    "was specified.\n");
-		usage(nf);
+		arg_help(argc, argv, f);
 	}
 
-	if (!f_flag && a_flag && slot == 0) {
+	/* Check that a controller (and not a namespace) was specified. */
+	if (strstr(opt.dev, NVME_NS_PREFIX) != NULL)
+		arg_help(argc, argv, f);
+
+	if (opt.activate && opt.fw_img == NULL && opt.slot == 0) {
 		fprintf(stderr,
 		    "Slot number to activate not specified.\n");
-		usage(nf);
+		arg_help(argc, argv, f);
 	}
 
-	controller = argv[optind];
-	open_dev(controller, &fd, 1, 1);
+	open_dev(opt.dev, &fd, 1, 1);
 	read_controller_data(fd, &cdata);
 
 	oacs_fw = (cdata.oacs >> NVME_CTRLR_DATA_OACS_FIRMWARE_SHIFT) &
@@ -254,44 +276,45 @@ firmware(const struct nvme_function *nf, int argc, cha
 	fw_slot1_ro = (cdata.frmw >> NVME_CTRLR_DATA_FRMW_SLOT1_RO_SHIFT) &
 		NVME_CTRLR_DATA_FRMW_SLOT1_RO_MASK;
 
-	if (f_flag && slot == 1 && fw_slot1_ro)
-		errx(1, "slot %d is marked as read only", slot);
+	if (opt.fw_img && opt.slot == 1 && fw_slot1_ro)
+		errx(1, "slot %d is marked as read only", opt.slot);
 
 	fw_num_slots = (cdata.frmw >> NVME_CTRLR_DATA_FRMW_NUM_SLOTS_SHIFT) &
 		NVME_CTRLR_DATA_FRMW_NUM_SLOTS_MASK;
 
-	if (slot > fw_num_slots)
+	if (opt.slot > fw_num_slots)
 		errx(1,
 		    "slot %d specified but controller only supports %d slots",
-		    slot, fw_num_slots);
+		    opt.slot, fw_num_slots);
 
-	if (a_flag && !f_flag && !slot_has_valid_firmware(fd, slot))
+	if (opt.activate && opt.fw_img == NULL &&
+	    !slot_has_valid_firmware(fd, opt.slot))
 		errx(1,
 		    "slot %d does not contain valid firmware,\n"
 		    "try 'nvmecontrol logpage -p 3 %s' to get a list "
 		    "of available images\n",
-		    slot, controller);
+		    opt.slot, opt.dev);
 
-	if (f_flag)
-		read_image_file(image, &buf, &size);
+	if (opt.fw_img)
+		read_image_file(opt.fw_img, &buf, &size);
 
-	if (f_flag && a_flag)
+	if (opt.fw_img != NULL&& opt.activate)
 		printf("You are about to download and activate "
 		       "firmware image (%s) to controller %s.\n"
 		       "This may damage your controller and/or "
 		       "overwrite an existing firmware image.\n",
-		       image, controller);
-	else if (a_flag)
+		       opt.fw_img, opt.dev);
+	else if (opt.activate)
 		printf("You are about to activate a new firmware "
 		       "image on controller %s.\n"
 		       "This may damage your controller.\n",
-		       controller);
-	else if (f_flag)
+		       opt.dev);
+	else if (opt.fw_img != NULL)
 		printf("You are about to download firmware image "
 		       "(%s) to controller %s.\n"
 		       "This may damage your controller and/or "
 		       "overwrite an existing firmware image.\n",
-		       image, controller);
+		       opt.fw_img, opt.dev);
 
 	printf("Are you sure you want to continue? (yes/no) ");
 	while (1) {
@@ -303,9 +326,9 @@ firmware(const struct nvme_function *nf, int argc, cha
 		printf("Please answer \"yes\" or \"no\". ");
 	}
 
-	if (f_flag) {
+	if (opt.fw_img != NULL) {
 		update_firmware(fd, buf, size);
-		if (a_flag)
+		if (opt.activate)
 			activate_action = NVME_AA_REPLACE_ACTIVATE;
 		else
 			activate_action = NVME_AA_REPLACE_NO_ACTIVATE;
@@ -313,9 +336,9 @@ firmware(const struct nvme_function *nf, int argc, cha
 		activate_action = NVME_AA_ACTIVATE;
 	}
 
-	reboot_required = activate_firmware(fd, slot, activate_action);
+	reboot_required = activate_firmware(fd, opt.slot, activate_action);
 
-	if (a_flag) {
+	if (opt.activate) {
 		if (reboot_required) {
 			printf("New firmware image activated but requires "
 			       "conventional reset (i.e. reboot) to "
@@ -325,12 +348,10 @@ firmware(const struct nvme_function *nf, int argc, cha
 			       "effect after next controller reset.\n"
 			       "Controller reset can be initiated via "
 			       "'nvmecontrol reset %s'\n",
-			       controller);
+			       opt.dev);
 		}
 	}
 
 	close(fd);
 	exit(0);
 }
-
-NVME_COMMAND(top, firmware, firmware, FIRMWARE_USAGE);

Modified: stable/12/sbin/nvmecontrol/format.c
==============================================================================
--- stable/12/sbin/nvmecontrol/format.c	Mon Aug 12 18:49:32 2019	(r350928)
+++ stable/12/sbin/nvmecontrol/format.c	Mon Aug 12 18:50:26 2019	(r350929)
@@ -35,6 +35,7 @@ __FBSDID("$FreeBSD$");
 #include <ctype.h>
 #include <err.h>
 #include <fcntl.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -43,57 +44,104 @@ __FBSDID("$FreeBSD$");
 
 #include "nvmecontrol.h"
 
-#define FORMAT_USAGE							       \
-	"format [-f fmt] [-m mset] [-p pi] [-l pil] [-E] [-C] <controller id|namespace id>\n"
+#define NONE 0xffffffffu
+#define SES_NONE 0
+#define SES_USER 1
+#define SES_CRYPTO 2
 
+/* Tables for command line parsing */
+
+static cmd_fn_t format;
+
+static struct options {
+	uint32_t	lbaf;
+	uint32_t	ms;
+	uint32_t	pi;
+	uint32_t	pil;
+	uint32_t	ses;
+	bool		Eflag;
+	bool		Cflag;
+	const char	*dev;
+} opt = {
+	.lbaf = NONE,
+	.ms = NONE,
+	.pi = NONE,
+	.pil = NONE,
+	.ses = SES_NONE,
+	.Eflag = false,
+	.Cflag = false,
+	.dev = NULL,
+};
+
+static const struct opts format_opts[] = {
+#define OPT(l, s, t, opt, addr, desc) { l, s, t, &opt.addr, desc }
+	OPT("crypto", 'C', arg_none, opt, Cflag,
+	    "Crptographically erase user data by forgetting key"),
+	OPT("erase", 'E', arg_none, opt, Eflag,
+	    "Erase user data"),
+	OPT("lbaf", 'f', arg_uint32, opt, lbaf,
+	    "Set the LBA Format to apply to the media"),
+	OPT("ms", 'm', arg_uint32, opt, ms,
+	    "Slot to activate and/or download format to"),
+	OPT("pi", 'p', arg_uint32, opt, pi,
+	    "Slot to activate and/or download format to"),
+	OPT("pil", 'l', arg_uint32, opt, pil,
+	    "Slot to activate and/or download format to"),
+	OPT("ses", 's', arg_uint32, opt, ses,
+	    "Slot to activate and/or download format to"),
+	{ NULL, 0, arg_none, NULL, NULL }
+};
+#undef OPT
+
+static const struct args format_args[] = {
+	{ arg_string, &opt.dev, "controller-id|namespace-id" },
+	{ arg_none, NULL, NULL },
+};
+
+static struct cmd format_cmd = {
+	.name = "format",
+	.fn = format,
+	.descr = "Format/erase one or all the namespaces.",
+	.ctx_size = sizeof(opt),
+	.opts = format_opts,
+	.args = format_args,
+};
+
+CMD_COMMAND(format_cmd);
+
+/* End of tables for command line parsing */
+
 static void
-format(const struct nvme_function *nf, int argc, char *argv[])
+format(const struct cmd *f, int argc, char *argv[])
 {
 	struct nvme_controller_data	cd;
 	struct nvme_namespace_data	nsd;
 	struct nvme_pt_command		pt;
 	char				path[64];
-	char				*target;
+	const char			*target;
 	uint32_t			nsid;
-	int				ch, fd;
-	int lbaf = -1, mset = -1, pi = -1, pil = -1, ses = 0;
+	int				lbaf, ms, pi, pil, ses, fd;
 
-	if (argc < 2)
-		usage(nf);
+	if (arg_parse(argc, argv, f))
+		return;
 
-	while ((ch = getopt(argc, argv, "f:m:p:l:EC")) != -1) {
-		switch ((char)ch) {
-		case 'f':
-			lbaf = strtol(optarg, NULL, 0);
-			break;
-		case 'm':
-			mset = strtol(optarg, NULL, 0);
-			break;
-		case 'p':
-			pi = strtol(optarg, NULL, 0);
-			break;
-		case 'l':
-			pil = strtol(optarg, NULL, 0);
-			break;
-		case 'E':
-			if (ses == 2)
-				errx(1, "-E and -C are mutually exclusive");
-			ses = 1;
-			break;
-		case 'C':
-			if (ses == 1)
-				errx(1, "-E and -C are mutually exclusive");
-			ses = 2;
-			break;
-		default:
-			usage(nf);
-		}
+	if (opt.Eflag || opt.Cflag || opt.ses != SES_NONE) {
+		fprintf(stderr,
+		    "Only one of -E, -C or -s may be specified\n");
+		arg_help(argc, argv, f);
 	}
 
-	/* Check that a controller or namespace was specified. */
-	if (optind >= argc)
-		usage(nf);
-	target = argv[optind];
+	target = opt.dev;
+	lbaf = opt.lbaf;
+	ms = opt.ms;
+	pi = opt.pi;
+	pil = opt.pil;
+	if (opt.Eflag)
+		ses = SES_USER;
+	else if (opt.Cflag)
+		ses = SES_CRYPTO;
+	else
+		ses = opt.ses;
 
 	/*
 	 * Check if the specified device node exists before continuing.
@@ -126,15 +174,15 @@ format(const struct nvme_function *nf, int argc, char 
 	    NVME_CTRLR_DATA_OACS_FORMAT_MASK) == 0)
 		errx(1, "controller does not support format");
 	if (((cd.fna >> NVME_CTRLR_DATA_FNA_CRYPTO_ERASE_SHIFT) &
-	    NVME_CTRLR_DATA_FNA_CRYPTO_ERASE_MASK) == 0 && ses == 2)
+	    NVME_CTRLR_DATA_FNA_CRYPTO_ERASE_MASK) == 0 && ses == SES_CRYPTO)
 		errx(1, "controller does not support cryptographic erase");
 
 	if (nsid != NVME_GLOBAL_NAMESPACE_TAG) {
 		if (((cd.fna >> NVME_CTRLR_DATA_FNA_FORMAT_ALL_SHIFT) &
-		    NVME_CTRLR_DATA_FNA_FORMAT_ALL_MASK) && ses == 0)
+		    NVME_CTRLR_DATA_FNA_FORMAT_ALL_MASK) && ses == SES_NONE)
 			errx(1, "controller does not support per-NS format");
 		if (((cd.fna >> NVME_CTRLR_DATA_FNA_ERASE_ALL_SHIFT) &
-		    NVME_CTRLR_DATA_FNA_ERASE_ALL_MASK) && ses != 0)
+		    NVME_CTRLR_DATA_FNA_ERASE_ALL_MASK) && ses != SES_NONE)
 			errx(1, "controller does not support per-NS erase");
 
 		/* Try to keep previous namespace parameters. */
@@ -144,8 +192,8 @@ format(const struct nvme_function *nf, int argc, char 
 			    & NVME_NS_DATA_FLBAS_FORMAT_MASK;
 		if (lbaf > nsd.nlbaf)
 			errx(1, "LBA format is out of range");
-		if (mset < 0)
-			mset = (nsd.flbas >> NVME_NS_DATA_FLBAS_EXTENDED_SHIFT)
+		if (ms < 0)
+			ms = (nsd.flbas >> NVME_NS_DATA_FLBAS_EXTENDED_SHIFT)
 			    & NVME_NS_DATA_FLBAS_EXTENDED_MASK;
 		if (pi < 0)
 			pi = (nsd.dps >> NVME_NS_DATA_DPS_MD_START_SHIFT)
@@ -158,8 +206,8 @@ format(const struct nvme_function *nf, int argc, char 
 		/* We have no previous parameters, so default to zeroes. */
 		if (lbaf < 0)
 			lbaf = 0;
-		if (mset < 0)
-			mset = 0;
+		if (ms < 0)
+			ms = 0;
 		if (pi < 0)
 			pi = 0;
 		if (pil < 0)
@@ -170,7 +218,7 @@ format(const struct nvme_function *nf, int argc, char 
 	pt.cmd.opc = NVME_OPC_FORMAT_NVM;
 	pt.cmd.nsid = htole32(nsid);
 	pt.cmd.cdw10 = htole32((ses << 9) + (pil << 8) + (pi << 5) +
-	    (mset << 4) + lbaf);
+	    (ms << 4) + lbaf);
 
 	if (ioctl(fd, NVME_PASSTHROUGH_CMD, &pt) < 0)
 		err(1, "format request failed");
@@ -180,5 +228,3 @@ format(const struct nvme_function *nf, int argc, char 
 	close(fd);
 	exit(0);
 }
-
-NVME_COMMAND(top, format, format, FORMAT_USAGE);

Modified: stable/12/sbin/nvmecontrol/identify.c
==============================================================================
--- stable/12/sbin/nvmecontrol/identify.c	Mon Aug 12 18:49:32 2019	(r350928)
+++ stable/12/sbin/nvmecontrol/identify.c	Mon Aug 12 18:50:26 2019	(r350929)
@@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$");
 #include <ctype.h>
 #include <err.h>
 #include <fcntl.h>
+#include <stdbool.h>
 #include <stddef.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -43,8 +44,15 @@ __FBSDID("$FreeBSD$");
 #include "nvmecontrol.h"

*** DIFF OUTPUT TRUNCATED AT 1000 LINES ***



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201908121850.x7CIoQ4K059320>