commands: new method for defining commands

lib/commands/toolcontext.h

@@ -89,8 +89,8 @@ struct cmd_context {
 	 */
 	const char *cmd_line;
 	struct command *command;
-	char **argv;
-	struct arg_values *arg_values;
+	char **pos_arg_values;
+	struct arg_values *opt_arg_values;
 	struct dm_list arg_value_groups;
 
 	/*

scripts/command-lines.in

@@ -1,3 +1,12 @@
+#
+# When this file is changed, tools/command-lines.h
+# and tools/command-lines-count.h must be regenerated
+# with:
+#
+# scripts/create-commands --output count scripts/command-lines.in > tools/command-lines-count.h
+# scripts/create-commands --output struct scripts/command-lines.in > tools/command-lines.h
+#
+
 #
 # A new command has a unique combination of:
 # command name, required option args and required
@@ -28,7 +37,7 @@
 # --foo is accepted by the command.  (This is uncommon.)
 #
 # Possible option arg types that can follow --opt are:
-# Bool, Number, String, PV, VG, LV, Tag, Select.
+# Bool, Number, String, PV, VG, LV, Tag.
 #
 # Option args outside the list of types are treated as literal
 # (non-variable) strings or numbers.

scripts/create-commands.c

@@ -78,6 +78,52 @@ static struct opt_name opt_names[ARG_COUNT + 1] = {
 
 #include "command.h"
 
+#define ARG_DEF_TYPES 16
+struct arg_def_type {
+	const char *name;
+	int flag;
+};
+
+/* The names used for arg_def types in command-lines.in */
+static struct arg_def_type arg_def_types[ARG_DEF_TYPES] = {
+        { "None",           ARG_DEF_TYPE_NONE},
+        { "Bool",           ARG_DEF_TYPE_BOOL},
+        { "Number",         ARG_DEF_TYPE_NUM_ANY},
+        { "String",         ARG_DEF_TYPE_STR_ANY},
+        { "Name",           ARG_DEF_TYPE_NAME_ANY},
+        { "PV",             ARG_DEF_TYPE_NAME_PV},
+        { "VG",             ARG_DEF_TYPE_NAME_VG},
+        { "LV",             ARG_DEF_TYPE_NAME_LV},
+        { "Tag",            ARG_DEF_TYPE_TAG},
+        { "Select",         ARG_DEF_TYPE_SELECT},
+};
+
+#define ARG_DEF_LVS 64
+struct arg_def_lv {
+	const char *name;
+	int flag;
+};
+
+/* The names used for arg_def lv_types in command-lines.in */
+static struct arg_def_lv arg_def_lvs[ARG_DEF_LVS] = {
+        { "LV",           ARG_DEF_LV_ANY},
+        { "LV_linear",    ARG_DEF_LV_LINEAR},
+        { "LV_striped",   ARG_DEF_LV_STRIPED},
+        { "LV_snapshot",  ARG_DEF_LV_SNAPSHOT},
+        { "LV_mirror",    ARG_DEF_LV_MIRROR},
+        { "LV_raid",      ARG_DEF_LV_RAID},
+        { "LV_raid0",     ARG_DEF_LV_RAID0},
+        { "LV_raid1",     ARG_DEF_LV_RAID1},
+        { "LV_raid4",     ARG_DEF_LV_RAID4},
+        { "LV_raid5",     ARG_DEF_LV_RAID5},
+        { "LV_raid6",     ARG_DEF_LV_RAID6},
+        { "LV_raid10",    ARG_DEF_LV_RAID10},
+        { "LV_thin",      ARG_DEF_LV_THIN},
+        { "LV_thinpool",  ARG_DEF_LV_THINPOOL},
+        { "LV_cache",     ARG_DEF_LV_CACHE},
+        { "LV_cachepool", ARG_DEF_LV_CACHEPOOL},
+};
+
 #define MAX_CMD_NAMES 128
 struct cmd_name {
 	const char *name;
@@ -844,23 +890,33 @@ static void print_def(struct arg_def *def)
 		printf(" ...");
 }
 
-void print_data_expanded(void)
+void print_expanded(void)
 {
 	struct command *cmd;
+	int onereq;
 	int i, ro, rp, oo, op;
 
 	for (i = 0; i < cmd_count; i++) {
 		cmd = &cmd_array[i];
 		printf("%s", cmd->name);
 
+		onereq = (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) ? 1 : 0;
+
 		if (cmd->ro_count) {
+			if (onereq)
+				printf(" (");
+
 			for (ro = 0; ro < cmd->ro_count; ro++) {
+				if (ro && onereq)
+					printf(",");
 				printf(" %s", opt_names[cmd->required_opt_args[ro].opt].long_opt);
 				if (cmd->required_opt_args[ro].def.types) {
 					printf(" ");
 					print_def(&cmd->required_opt_args[ro].def);
 				}
 			}
+			if (onereq)
+				printf(" )");
 		}
 
 		if (cmd->rp_count) {
@@ -1103,6 +1159,8 @@ static char *flags_to_str(int flags)
 
 void print_define_command_count(void)
 {
+	printf("/* Do not edit. This file is generated by scripts/create-commands */\n");
+	printf("/* using command definitions from scripts/command-lines.in */\n");
 	printf("#define COMMAND_COUNT %d\n", cmd_count);
 }
 
@@ -1145,7 +1203,7 @@ void print_usage(struct command *cmd)
 		goto op_count;
 
 	printf("\n");
-	printf("\"[");
+	printf("\" [");
 
 	if (cmd->oo_count) {
 		for (oo = 0; oo < cmd->oo_count; oo++) {
@@ -1166,7 +1224,7 @@ void print_usage(struct command *cmd)
 		goto done;
 
 	printf("\n");
-	printf("\"[");
+	printf("\" [");
 
 	if (cmd->op_count) {
 		for (op = 0; op < cmd->op_count; op++) {
@@ -1183,21 +1241,33 @@ void print_usage(struct command *cmd)
 	printf(";\n");
 }
 
-void print_command_count(void)
-{
-	printf("#define COMMAND_COUNT %d\n", cmd_count);
-}
-
-void print_command_structs(void)
+void print_command_struct(int only_usage)
 {
 	struct command *cmd;
 	int i, j, ro, rp, oo, op;
 
+	printf("/* Do not edit. This file is generated by scripts/create-commands */\n");
+	printf("/* using command definitions from scripts/command-lines.in */\n");
+
 	for (i = 0; i < cmd_count; i++) {
 		cmd = &cmd_array[i];
 
+		if (only_usage) {
+			print_usage(cmd);
+			printf("\n");
+			continue;
+		}
+
 		printf("commands[%d].name = \"%s\";\n", i, cmd->name);
 		printf("commands[%d].fn = %s;\n", i, cmd->name);
+		printf("commands[%d].ro_count = %d;\n", i, cmd->ro_count);
+		printf("commands[%d].rp_count = %d;\n", i, cmd->rp_count);
+		printf("commands[%d].oo_count = %d;\n", i, cmd->oo_count);
+		printf("commands[%d].op_count = %d;\n", i, cmd->op_count);
+
+		if (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT)
+			printf("commands[%d].cmd_flags = CMD_FLAG_ONE_REQUIRED_OPT;\n", i);
+
 		printf("commands[%d].desc = \"%s\";\n", i, cmd->desc ?: "");
 		printf("commands[%d].usage = ", i);
 		print_usage(cmd);
@@ -1356,18 +1426,20 @@ void print_option_list(void)
 	int i;
 
 	for (i = 0; i < ARG_COUNT; i++)
-		printf("%d %s %s %c\n",
+		printf("%d %s %s %c (%d)\n",
 			opt_names[i].enum_val, opt_names[i].enum_name,
-			opt_names[i].long_opt, opt_names[i].short_opt ?: ' ');
+			opt_names[i].long_opt, opt_names[i].short_opt ?: ' ',
+			opt_names[i].short_opt ? opt_names[i].short_opt : 0);
 }
 
 static void print_help(int argc, char *argv[])
 {
-	printf("%s --output struct|count|expand <filename>\n", argv[0]);
+	printf("%s --output struct|count|usage|expanded <filename>\n", argv[0]);
 	printf("\n");
-	printf("struct: print C structures for command definitions.\n");
-	printf("expand: print expanded input format.\n");
-	printf("count:  print #define COMMAND_COUNT <Number>\n");
+	printf("struct:   print C structures.\n");
+	printf("usage:    print usage format.\n");
+	printf("expanded: print expanded input format.\n");
+	printf("count:    print #define COMMAND_COUNT <Number>\n");
 }
 
 int main(int argc, char *argv[])
@@ -1391,6 +1463,12 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	if (!strcmp(argv[1], "debug")) {
+		print_command_list();
+		print_option_list();
+		return 0;
+	}
+
 	static struct option long_options[] = {
 		{"help",      no_argument,       0, 'h' },
 		{"output",    required_argument, 0, 'o' },
@@ -1465,7 +1543,7 @@ int main(int argc, char *argv[])
 			char *desc = strdup(strstr(line_orig, ":") + 2);
 			if (cmd->desc) {
 				cmd->desc = realloc((char *)cmd->desc, strlen(cmd->desc) + strlen(desc) + 2);
-				strcat((char *)cmd->desc, "\n");
+				strcat((char *)cmd->desc, "  ");
 				strcat((char *)cmd->desc, desc);
 				free(desc);
 			} else
@@ -1521,13 +1599,15 @@ int main(int argc, char *argv[])
 	fclose(file);
 
 	if (!outputformat)
-		print_data_expanded();
+		print_command_struct(1);
 	else if (!strcmp(outputformat, "struct"))
-		print_command_structs();
+		print_command_struct(0);
 	else if (!strcmp(outputformat, "count"))
-		print_command_count();
-	else if (!strcmp(outputformat, "expand"))
-		print_data_expanded();
+		print_define_command_count();
+	else if (!strcmp(outputformat, "usage"))
+		print_command_struct(1);
+	else if (!strcmp(outputformat, "expanded"))
+		print_expanded();
 	else
 		print_help(argc, argv);
 }

tools/args.h

@@ -169,7 +169,7 @@ arg(physicalextent_ARG, 'E', "physicalextent", NULL, 0, 0)
 arg(file_ARG, 'f', "file", string_arg, 0, 0)
 arg(force_ARG, 'f', "force", NULL, ARG_COUNTABLE, 0)
 arg(full_ARG, 'f', "full", NULL, 0, 0)
-arg(help_ARG, 'h', "help", NULL, 0, 0)
+arg(help_ARG, 'h', "help", NULL, ARG_COUNTABLE, 0)
 arg(cache_ARG, 'H', "cache", NULL, 0, 0)
 arg(history_ARG, 'H', "history", NULL, 0, 0)
 arg(help2_ARG, '?', "", NULL, 0, 0)

tools/command-lines-count.h

@@ -0,0 +1,3 @@
+/* Do not edit. This file is generated by scripts/create-commands */
+/* using command definitions from scripts/command-lines.in */
+#define COMMAND_COUNT 144

tools/command.h

@@ -47,7 +47,7 @@ enum {
 	ARG_DEF_TYPE_STR_ANY    = 1 << 2,
 	ARG_DEF_TYPE_NUM_CONST  = 1 << 3,
 	ARG_DEF_TYPE_STR_CONST  = 1 << 4,
-	ARG_DEF_TYPE_STR_SET    = 1 << 5, /* a set of specific accepted string values */
+	ARG_DEF_TYPE_STR_SET    = 1 << 5,
 	ARG_DEF_TYPE_NAME_ANY   = 1 << 6,
 	ARG_DEF_TYPE_NAME_PV    = 1 << 7,
 	ARG_DEF_TYPE_NAME_VG    = 1 << 8,
@@ -56,26 +56,6 @@ enum {
 	ARG_DEF_TYPE_SELECT     = 1 << 11,
 };
 
-#define ARG_DEF_TYPES 16
-struct arg_def_type {
-	const char *name;
-	int flag;
-};
-
-/* The names used for arg_def types in command-lines.in */
-static struct arg_def_type arg_def_types[ARG_DEF_TYPES] = {
-	{ "None",           ARG_DEF_TYPE_NONE},
-	{ "Bool",           ARG_DEF_TYPE_BOOL},
-	{ "Number",         ARG_DEF_TYPE_NUM_ANY},
-	{ "String",         ARG_DEF_TYPE_STR_ANY},
-	{ "Name",           ARG_DEF_TYPE_NAME_ANY},
-	{ "PV",             ARG_DEF_TYPE_NAME_PV},
-	{ "VG",             ARG_DEF_TYPE_NAME_VG},
-	{ "LV",             ARG_DEF_TYPE_NAME_LV},
-	{ "Tag",            ARG_DEF_TYPE_TAG},
-	{ "Select",         ARG_DEF_TYPE_SELECT},
-};
-
 /* arg_def lv_types, can be multiple */
 enum {
 	ARG_DEF_LV_ANY       = 0,
@@ -96,32 +76,6 @@ enum {
 	ARG_DEF_LV_CACHEPOOL = 1 << 14,
 };
 
-#define ARG_DEF_LVS 64
-struct arg_def_lv {
-	const char *name;
-	int flag;
-};
-
-/* The names used for arg_def lv_types in command-lines.in */
-static struct arg_def_lv arg_def_lvs[ARG_DEF_LVS] = {
-	{ "LV",           ARG_DEF_LV_ANY},
-	{ "LV_linear",    ARG_DEF_LV_LINEAR},
-	{ "LV_striped",   ARG_DEF_LV_STRIPED},
-	{ "LV_snapshot",  ARG_DEF_LV_SNAPSHOT},
-	{ "LV_mirror",    ARG_DEF_LV_MIRROR},
-	{ "LV_raid",      ARG_DEF_LV_RAID},
-	{ "LV_raid0",     ARG_DEF_LV_RAID0},
-	{ "LV_raid1",     ARG_DEF_LV_RAID1},
-	{ "LV_raid4",     ARG_DEF_LV_RAID4},
-	{ "LV_raid5",     ARG_DEF_LV_RAID5},
-	{ "LV_raid6",     ARG_DEF_LV_RAID6},
-	{ "LV_raid10",    ARG_DEF_LV_RAID10},
-	{ "LV_thin",      ARG_DEF_LV_THIN},
-	{ "LV_thinpool",  ARG_DEF_LV_THINPOOL},
-	{ "LV_cache",     ARG_DEF_LV_CACHE},
-	{ "LV_cachepool", ARG_DEF_LV_CACHEPOOL},
-};
-
 /* Description a value that follows an option or exists in a position. */
 
 struct arg_def {
@@ -152,8 +106,8 @@ struct pos_arg {
  * of which one is required after which the rest are
  * optional.
  */
-#define CMD_RO_ARGS 32          /* required opt args */
-#define CMD_OO_ARGS ARG_COUNT   /* optional opt args */
+#define CMD_RO_ARGS 64          /* required opt args */
+#define CMD_OO_ARGS 150         /* optional opt args */
 #define CMD_RP_ARGS 8           /* required positional args */
 #define CMD_OP_ARGS 8           /* optional positional args */
 
@@ -166,11 +120,13 @@ struct pos_arg {
 /* a register of the lvm commands */
 struct command {
 	const char *name;
-	const char *desc;
+	const char *desc; /* specific command description from command-lines.h */
 	const char *usage;
+
+	struct command_name *cname;
 	command_fn fn;
 
-	unsigned int flags;
+	unsigned int flags; /* copied from command_name.flags from commands.h */
 
 	unsigned int cmd_flags; /* CMD_FLAG_ */
 
@@ -197,4 +153,14 @@ struct command {
 	int pos_count;
 };
 
+struct command_name {
+	const char *name;
+	const char *desc; /* general command description from commands.h */
+	unsigned int flags;
+
+	/* union of {required,optional}_opt_args for all commands with this name */
+	int valid_args[ARG_COUNT];
+	int num_args;
+};
+
 #endif

tools/lvm.c

@@ -45,9 +45,9 @@ static char *_list_cmds(const char *text, int state)
 		len = strlen(text);
 	}
 
-	while (i < _cmdline->num_commands)
-		if (!strncmp(text, _cmdline->commands[i++].name, len))
-			return strdup(_cmdline->commands[i - 1].name);
+	while (i < _cmdline->num_command_names)
+		if (!strncmp(text, _cmdline->command_names[i++].name, len))
+			return strdup(_cmdline->command_names[i - 1].name);
 
 	return NULL;
 }
@@ -57,7 +57,7 @@ static char *_list_args(const char *text, int state)
 {
 	static int match_no = 0;
 	static size_t len = 0;
-	static struct command *com;
+	static struct command_name *cname;
 
 	/* Initialise if this is a new completion attempt */
 	if (!state) {
@@ -65,40 +65,40 @@ static char *_list_args(const char *text, int state)
 		int j;
 
 		match_no = 0;
-		com = NULL;
+		cname = NULL;
 		len = strlen(text);
 
 		/* Find start of first word in line buffer */
 		while (isspace(*s))
 			s++;
 
-		/* Look for word in list of commands */
-		for (j = 0; j < _cmdline->num_commands; j++) {
+		/* Look for word in list of command names */
+		for (j = 0; j < _cmdline->num_command_names; j++) {
 			const char *p;
 			char *q = s;
 
-			p = _cmdline->commands[j].name;
+			p = _cmdline->command_names[j].name;
 			while (*p == *q) {
 				p++;
 				q++;
 			}
 			if ((!*p) && *q == ' ') {
-				com = _cmdline->commands + j;
+				cname = _cmdline->command_names + j;
 				break;
 			}
 		}
 	}
 
-	if (!com)
+	if (!cname)
 		return NULL;
 
 	/* Short form arguments */
 	if (len < 3) {
-		while (match_no < com->num_args) {
+		while (match_no < cname->num_args) {
 			char s[3];
 			char c;
 			if (!(c = (_cmdline->arg_props +
-				   com->valid_args[match_no++])->short_arg))
+				   cname->valid_args[match_no++])->short_arg))
 				continue;
 
 			sprintf(s, "-%c", c);
@@ -108,13 +108,13 @@ static char *_list_args(const char *text, int state)
 	}
 
 	/* Long form arguments */
-	if (match_no < com->num_args)
-		match_no = com->num_args;
+	if (match_no < cname->num_args)
+		match_no = cname->num_args;
 
-	while (match_no - com->num_args < com->num_args) {
+	while (match_no - cname->num_args < cname->num_args) {
 		const char *l;
 		l = (_cmdline->arg_props +
-		     com->valid_args[match_no++ - com->num_args])->long_arg;
+		     cname->valid_args[match_no++ - cname->num_args])->long_arg;
 		if (*(l + 2) && !strncmp(text, l, len))
 			return strdup(l);
 	}

tools/lvm2cmdline.h

@@ -19,10 +19,11 @@
 struct cmd_context;
 
 struct cmdline_context {
-        struct arg_props *arg_props;
-        struct command *commands;
-        int num_commands;
-        int commands_size;
+	struct arg_props *arg_props;
+	struct command *commands;
+	int num_commands;
+	struct command_name *command_names;
+	int num_command_names;
 };
 
 int lvm2_main(int argc, char **argv);

tools/lvmcmdline.c

@@ -49,16 +49,13 @@ extern char *optarg;
 #  define OPTIND_INIT 1
 #endif
 
-#if 0
 #include "command-lines-count.h" /* #define COMMAND_COUNT, generated from command-lines.in */
-#endif
-#define COMMAND_COUNT 128
 
 /*
  * Table of valid switches
  */
 static struct arg_props _arg_props[ARG_COUNT + 1] = {
-#define arg(a, b, c, d, e, f) {b, "", "--" c, d, e, f},
+#define arg(a, b, c, d, e, f) {a, b, "", "--" c, d, e, f},
 #include "args.h"
 #undef arg
 };
@@ -67,17 +64,11 @@ static struct arg_props _arg_props[ARG_COUNT + 1] = {
  * Table of valid command names
  */
 #define MAX_COMMAND_NAMES 64
-struct command_name {
-	const char *name;
-	const char *desc;
-	unsigned int flags;
-};
-
 struct command_name command_names[MAX_COMMAND_NAMES] = {
-#define xx(a, b, c...) { # a, b, c }
+#define xx(a, b, c...) { # a, b, c },
 #include "commands.h"
 #undef xx
-}
+};
 
 /*
  * Table of valid command lines
@@ -734,7 +725,47 @@ int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
 	return int_arg(cmd, av);
 }
 
-static struct command_name *_find_command_name(char *name)
+/*
+ * The valid args for a command name in general is a union of
+ * required_opt_args and optional_opt_args for all commands[]
+ * with the given name.
+ */
+
+static void _set_valid_args_for_command_name(int ci)
+{
+	int all_args[ARG_COUNT] = { 0 };
+	int num_args = 0;
+	int opt_enum_val;
+	int i, ro, oo;
+
+	/*
+	 * all_args is indexed by the foo_ARG enum vals
+	 */
+
+	for (i = 0; i < COMMAND_COUNT; i++) {
+		if (strcmp(commands[i].name, command_names[ci].name))
+			continue;
+
+		for (ro = 0; ro < commands[i].ro_count; ro++) {
+			opt_enum_val = commands[i].required_opt_args[ro].opt;
+			all_args[opt_enum_val] = 1;
+		}
+		for (oo = 0; oo < commands[i].oo_count; oo++) {
+			opt_enum_val = commands[i].optional_opt_args[oo].opt;
+			all_args[opt_enum_val] = 1;
+		}
+	}
+
+	for (i = 0; i < ARG_COUNT; i++) {
+		if (all_args[i]) {
+			command_names[ci].valid_args[num_args] = _cmdline.arg_props[i].enum_val;
+			num_args++;
+		}
+	}
+	command_names[ci].num_args = num_args;
+}
+
+static struct command_name *_find_command_name(const char *name)
 {
 	int i;
 	
@@ -747,7 +778,7 @@ static struct command_name *_find_command_name(char *name)
 	return NULL;
 }
 
-void _define_commands(void)
+static void _define_commands(void)
 {
 /* command-lines.h defines command[] structs, generated from command-lines.in */
 #include "command-lines.h" /* generated from command-lines.in */
@@ -758,16 +789,30 @@ void lvm_register_commands(void)
 	struct command_name *cname;
 	int i;
 
+	memset(&commands, 0, sizeof(commands));
+
 	_define_commands();
 
-	_cmdline.commands = &commands;
+	_cmdline.commands = commands;
 	_cmdline.num_commands = COMMAND_COUNT;
 
 	for (i = 0; i < COMMAND_COUNT; i++) {
 		if (!(cname = _find_command_name(commands[i].name)))
 			log_error(INTERNAL_ERROR "Failed to find command name %s.", commands[i].name);
+		commands[i].cname = cname;
 		commands[i].flags = cname->flags;
 	}
+
+	_cmdline.command_names = command_names;
+
+	for (i = 0; i < MAX_COMMAND_NAMES; i++) {
+		if (!command_names[i].name)
+			break;
+		_cmdline.num_command_names++;
+	}
+
+	for (i = 0; i < _cmdline.num_command_names; i++)
+		_set_valid_args_for_command_name(i);
 }
 
 /*
@@ -794,37 +839,37 @@ void lvm_register_commands(void)
 
 static int _opt_equivalent_is_set(struct cmd_context *cmd, int opt)
 {
-	if ((opt == mirrorlog_ARG) && arg_is_set(cmd, corelog_ARG, NULL))
+	if ((opt == mirrorlog_ARG) && arg_is_set(cmd, corelog_ARG))
 		return 1;
 
-	if ((opt == resizeable_ARG) && arg_is_set(cmd, resizable_ARG, NULL))
+	if ((opt == resizeable_ARG) && arg_is_set(cmd, resizable_ARG))
 		return 1;
 
-	if ((opt == allocatable_ARG) && arg_is_set(cmd, allocation_ARG, NULL))
+	if ((opt == allocatable_ARG) && arg_is_set(cmd, allocation_ARG))
 		return 1;
 
-	if ((opt == resizeable_ARG) && arg_is_set(cmd, allocation_ARG, NULL))
+	if ((opt == resizeable_ARG) && arg_is_set(cmd, allocation_ARG))
 		return 1;
 
-	if ((opt == activate_ARG) && arg_is_set(cmd, available_ARG, NULL))
+	if ((opt == activate_ARG) && arg_is_set(cmd, available_ARG))
 		return 1;
 
-	if ((opt == rebuild_ARG) && arg_is_set(cmd, raidrebuild_ARG, NULL))
+	if ((opt == rebuild_ARG) && arg_is_set(cmd, raidrebuild_ARG))
 		return 1;
 
-	if ((opt == syncaction_ARG) && arg_is_set(cmd, raidsyncaction_ARG, NULL))
+	if ((opt == syncaction_ARG) && arg_is_set(cmd, raidsyncaction_ARG))
 		return 1;
 
-	if ((opt == writemostly_ARG) && arg_is_set(cmd, raidwritemostly_ARG, NULL))
+	if ((opt == writemostly_ARG) && arg_is_set(cmd, raidwritemostly_ARG))
 		return 1;
 
-	if ((opt == minrecoveryrate_ARG) && arg_is_set(cmd, raidminrecoveryrate_ARG, NULL))
+	if ((opt == minrecoveryrate_ARG) && arg_is_set(cmd, raidminrecoveryrate_ARG))
 		return 1;
 
-	if ((opt == maxrecoveryrate_ARG) && arg_is_set(cmd, raidmaxrecoveryrate_ARG, NULL))
+	if ((opt == maxrecoveryrate_ARG) && arg_is_set(cmd, raidmaxrecoveryrate_ARG))
 		return 1;
 
-	if ((opt == writebehind_ARG) && arg_is_set(cmd, raidwritebehind_ARG, NULL))
+	if ((opt == writebehind_ARG) && arg_is_set(cmd, raidwritebehind_ARG))
 		return 1;
 
 	return 0;
@@ -832,18 +877,18 @@ static int _opt_equivalent_is_set(struct cmd_context *cmd, int opt)
 
 static int _command_required_opt_matches(struct cmd_context *cmd, int ci, int ro)
 {
-	if (arg_is_set(cmd, commands[ci].required_opt_args[ro].opt, NULL))
+	if (arg_is_set(cmd, commands[ci].required_opt_args[ro].opt))
 		return 1;
 
 	/*
 	 * For some commands, --size and --extents are interchanable,
 	 * but command[] definitions use only --size.
 	 */
-	if ((commands[ci].required_opt_args[ro].opt == size_ARG) && arg_is_set(cmd, extents_ARG, NULL)) {
-		if (!strcmp(commands[i].name, "lvcreate") ||
-		    !strcmp(commands[i].name, "lvresize")
-		    !strcmp(commands[i].name, "lvextend")
-		    !strcmp(commands[i].name, "lvreduce"))
+	if ((commands[ci].required_opt_args[ro].opt == size_ARG) && arg_is_set(cmd, extents_ARG)) {
+		if (!strcmp(commands[ci].name, "lvcreate") ||
+		    !strcmp(commands[ci].name, "lvresize") ||
+		    !strcmp(commands[ci].name, "lvextend") ||
+		    !strcmp(commands[ci].name, "lvreduce"))
 			return 1;
 	}
 
@@ -851,6 +896,8 @@ static int _command_required_opt_matches(struct cmd_context *cmd, int ci, int ro
 
 	if (_opt_equivalent_is_set(cmd, commands[ci].required_opt_args[ro].opt))
 		return 1;
+
+	return 0;
 }
 
 static int _command_required_pos_matches(struct cmd_context *cmd, int ci, int rp)
@@ -866,17 +913,124 @@ static int _command_required_pos_matches(struct cmd_context *cmd, int ci, int rp
 	 * empty if --select is used.
 	 */
 	if ((commands[ci].required_pos_args[rp].def.types & ARG_DEF_TYPE_SELECT) &&
-	    arg_is_set(cmd, select_ARG, NULL))
+	    arg_is_set(cmd, select_ARG))
 		return 1;
 
 	return 0;
 }
 
+
+#define HELP_LINE_SIZE 1024
+
+static void _print_usage(int ci, int include_optional)
+{
+	const char *usage = _cmdline.commands[ci].usage;
+	char buf[HELP_LINE_SIZE] = {0};
+	int optional_ui = 0;
+	int ui = 0;
+	int bi = 0;
+
+	/*
+	 * Print the required portions of the usage string.
+ 	 * The optional portions of the usage string are enclosed
+	 * in [] and follow the required portions.
+	 */
+
+	for (ui = 0; ui < strlen(usage); ui++) {
+		if (usage[ui] == '[') {
+			optional_ui = ui;
+			break;
+		}
+		if (usage[ui] == '\0')
+			break;
+		if (usage[ui] == '\n')
+			break;
+
+		buf[bi++] = usage[ui];
+
+		if (usage[ui] == ',') {
+			buf[bi++] = '\n';
+			buf[bi++] = '\t';
+		}
+
+		if (bi == (HELP_LINE_SIZE - 1))
+			break;
+	}
+
+	if (!include_optional) {
+		log_print("%s\n", buf);
+		return;
+	}
+
+	log_print("%s", buf);
+
+	memset(buf, 0, sizeof(buf));
+	bi = 0;
+
+	buf[bi++] = '\t';
+
+	for (ui = optional_ui; ui < strlen(usage); ui++) {
+		if (usage[ui] == '\0')
+			break;
+		if (usage[ui] == '\n')
+			break;
+
+		buf[bi++] = usage[ui];
+
+		if (usage[ui] == ',') {
+			buf[bi++] = '\n';
+			buf[bi++] = '\t';
+		}
+
+		if (bi == (HELP_LINE_SIZE - 1))
+			break;
+	}
+
+	log_print("%s\n", buf);
+}
+
+/*
+ * A description is a string with multiple sentences each ending in periods.
+ * Print each sentence on a new line.
+ */
+
+static void _print_description(int ci)
+{
+	const char *desc = _cmdline.commands[ci].desc;
+	char buf[HELP_LINE_SIZE] = {0};
+	int di = 0;
+	int bi = 0;
+
+	for (di = 0; di < strlen(desc); di++) {
+		if (desc[di] == '\0')
+			break;
+		if (desc[di] == '\n')
+			continue;
+
+		if (!bi && desc[di] == ' ')
+			continue;
+
+		buf[bi++] = desc[di];
+
+		if (desc[di] == '.') {
+			log_print("%s", buf);
+			memset(buf, 0, sizeof(buf));
+			bi = 0;
+		}
+
+		if (bi == (HELP_LINE_SIZE - 1))
+			break;
+	}
+}
+
 static struct command *_find_command(struct cmd_context *cmd, const char *path)
 {
 	const char *name;
+	int match_count, mismatch_count;
+	int best_match_i = 0, best_match_count = 0;
+	int closest_i = 0, closest_count = 0;
+	int ro_matches;
 	int ro, rp;
-	int found = 0;
 	int i;
 
 	name = last_path_component(path);
@@ -885,39 +1039,64 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path)
 		if (strcmp(name, commands[i].name))
 			continue;
 
-		match_count = 1; /* for command name matching */
+		/* For help and version just return the first entry with matching name. */
+		if (arg_is_set(cmd, help_ARG) || arg_is_set(cmd, help2_ARG) || arg_is_set(cmd, version_ARG))
+			return &commands[i];
+
+		match_count = 0;
 		mismatch_count = 0;
+		ro_matches = 0;
+
+		/* if the command name alone is enough, then that's a match */
+
+		if (!commands[i].ro_count && !commands[i].rp_count) {
+			/* log_warn("match %d command name %s", i, name); */
+			match_count = 1;
+		}
 
 		/* match required_opt_args */
 
 		for (ro = 0; ro < commands[i].ro_count; ro++) {
-			if (_command_required_opt_matches(cmd, i, ro))
+			if (_command_required_opt_matches(cmd, i, ro)) {
+				/* log_warn("match %d ro opt %d", i, commands[i].required_opt_args[ro].opt); */
 				match_count++;
-			else
+				ro_matches++;
+			} else {
+				/* cmd is missing a required opt arg */
+				/* log_warn("mismatch %d ro opt %d", i, commands[i].required_opt_args[ro].opt); */
 				mismatch_count++;
+			}
 		}
 
 		/*
-		 * One item in required_opt_args must be set for
-		 * a match, and the rest are optional (don't count
-		 * as mismatches).
+		 * Special case where missing required_opt_arg's does not matter
+		 * if one required_opt_arg did match.
 		 */
-		if (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) {
-			if (match_count >= 2) {
-				match_count = 2;
+		if (commands[i].cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) {
+			if (ro_matches) {
+				/* one or more of the required_opt_args is used */
 				mismatch_count = 0;
+			} else {
+				/* not even one of the required_opt_args is used */
+				mismatch_count = 1;
 			}
 		}
 
 		/* match required_pos_args */
 
 		for (rp = 0; rp < commands[i].rp_count; rp++) {
-			if (_command_required_pos_matches(cmd, i, rp))
+			if (_command_required_pos_matches(cmd, i, rp)) {
+				/* log_warn("match %d rp %d", i, commands[i].required_pos_args[rp].pos); */
 				match_count++;
-			else
+			} else {
+				/* cmd is missing a required pos arg */
+				/* log_warn("mismatch %d rp %d", i, commands[i].required_pos_args[rp].pos); */
 				mismatch_count++;
+			}
 		}
 
+		/* if cmd is missing any required opt/pos args, it can't be this command. */
+
 		if (mismatch_count) {
 			/* save i/match_count for "closest" command that doesn't match */
 			if (!closest_count || (match_count > closest_count)) {
@@ -927,19 +1106,21 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path)
 			continue;
 		}
 
+		/* use command that has the most matching required opt/pos */
+
 		if (!best_match_count || (match_count > best_match_count)) {
+			/* log_warn("match best i %d count %d", i, match_count); */
 			best_match_i = i;
 			best_match_count = match_count;
 		}
 	}
 
 	if (!best_match_count) {
-		/* nothing matches */
-		/* TODO: report closest matching command and report missing required opt/pos args for that? */
-		log_error("Failed to find a matching command definition.");
+		/* cmd did not have all the required opt/pos args of any command */
+		log_error("Failed to find a matching command definition.\n");
 		if (closest_count) {
 			log_warn("Closest command usage is:");
-			log_warn("%s", commands[closest_i].usage);
+			_print_usage(closest_i, 0);
 		}
 		return NULL;
 	}
@@ -952,7 +1133,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path)
 	 * Same for pos args.
 	 */
 
-	log_debug("matched command usage: %.80s ...", commands[best_match_i].usage);
+	/* log_warn("command matched: %.256s ...", commands[best_match_i].usage); */
 
 	return &commands[best_match_i];
 }
@@ -962,69 +1143,109 @@ static void _short_usage(const char *name)
 	log_error("Run `%s --help' for more information.", name);
 }
 
-static int _usage(const char *name)
+static int _usage(const char *name, int help_count)
 {
 	struct command_name *cname = _find_command_name(name);
+	int i;
 
 	if (!cname) {
 		log_print("%s: no such command.", name);
 		return 0;
 	}
 
-	/* FIXME: print usage strings from matching commands[] entries? */
+	log_print("%s - %s\n", name, cname->desc);
+
+	for (i = 0; i < _cmdline.num_commands; i++) {
+		if (strcmp(_cmdline.commands[i].name, name))
+			continue;
+
+		if (strlen(_cmdline.commands[i].desc))
+			_print_description(i);
+
+		_print_usage(i, help_count > 1);
+	}
 
-	log_print("%s: %s", cname->name, cname->desc);
 	return 1;
 }
 
 /*
+ * Sets up the arguments to pass to getopt_long().
+ *
+ * getopt_long() takes a string of short option characters
+ * where the char is followed by ":" if the option takes an arg,
+ * e.g. "abc:d:"  This string is created in optstrp.
+ *
+ * getopt_long() also takes an array of struct option which
+ * has the name of the long option, if it takes an arg, etc,
+ * e.g.
+ *
+ * option long_options[] = {
+ * 	{ "foo", required_argument, 0,  0  },
+ * 	{ "bar", no_argument,       0, 'b' }
+ * };
+ *
+ * this array is created in longoptsp.
+ *
+ * Original comment:
  * Sets up the short and long argument.  If there
  * is no short argument then the index of the
  * argument in the the_args array is set as the
  * long opt value.  Yuck.  Of course this means we
  * can't have more than 'a' long arguments.
  */
-static void _add_getopt_arg(int arg, char **ptr, struct option **o)
+
+static void _add_getopt_arg(int arg_enum, char **optstrp, struct option **longoptsp)
 {
-	struct arg_props *a = _cmdline.arg_props + arg;
+	struct arg_props *a = _cmdline.arg_props + arg_enum;
 
 	if (a->short_arg) {
-		*(*ptr)++ = a->short_arg;
+		*(*optstrp)++ = a->short_arg;
 
 		if (a->fn)
-			*(*ptr)++ = ':';
+			*(*optstrp)++ = ':';
 	}
 #ifdef HAVE_GETOPTLONG
+	/* long_arg is "--foo", so +2 is the offset of the name after "--" */
+
 	if (*(a->long_arg + 2)) {
-		(*o)->name = a->long_arg + 2;
-		(*o)->has_arg = a->fn ? 1 : 0;
-		(*o)->flag = NULL;
+		(*longoptsp)->name = a->long_arg + 2;
+		(*longoptsp)->has_arg = a->fn ? 1 : 0;
+		(*longoptsp)->flag = NULL;
+
+		/*
+		 * When getopt_long() sees an option that has an associated
+		 * single letter, it returns the ascii value of that letter.
+		 * e.g. getopt_long() returns 100 for '-d' or '--debug'
+		 * (100 is the ascii value of 'd').
+		 *
+		 * When getopt_long() sees an option that does not have an
+		 * associated single letter, it returns the value of the
+		 * the enum for that long option name plus 128.
+		 * e.g. getopt_long() returns 139 for --cachepool
+		 * (11 is the enum value for --cachepool, so 11+128)
+		 */
+
 		if (a->short_arg)
-			(*o)->val = a->short_arg;
+			(*longoptsp)->val = a->short_arg;
 		else
-			(*o)->val = arg + 128;
-		(*o)++;
+			(*longoptsp)->val = arg_enum + 128;
+		(*longoptsp)++;
 	}
 #endif
 }
 
-static int _find_arg(struct command *com, int opt)
+static int _find_arg(int goval)
 {
-	struct arg_props *a;
-	int i, arg;
+	int i;
 
-	for (i = 0; i < com->num_args; i++) {
-		arg = com->valid_args[i];
-		a = _cmdline.arg_props + arg;
+	for (i = 0; i < ARG_COUNT; i++) {
+		/* the value returned by getopt matches the ascii value of single letter option */
+		if (_cmdline.arg_props[i].short_arg && (goval == _cmdline.arg_props[i].short_arg))
+			return _cmdline.arg_props[i].enum_val;
 
-		/*
-		 * opt should equal either the
-		 * short arg, or the index into
-		 * the_args.
-		 */
-		if ((a->short_arg && (opt == a->short_arg)) ||
-		    (!a->short_arg && (opt == (arg + 128))))
-			return arg;
+		/* the value returned by getopt matches the enum value plus 128 */
+		if (!_cmdline.arg_props[i].short_arg && (goval == _cmdline.arg_props[i].enum_val + 128))
+			return _cmdline.arg_props[i].enum_val;
 	}
 
 	return -1;
@@ -1033,21 +1254,27 @@ static int _find_arg(struct command *com, int opt)
 static int _process_command_line(struct cmd_context *cmd, int *argc,
 				 char ***argv)
 {
-	int i, opt, arg;
 	char str[((ARG_COUNT + 1) * 2) + 1], *ptr = str;
 	struct option opts[ARG_COUNT + 1], *o = opts;
 	struct arg_props *a;
 	struct arg_values *av;
 	struct arg_value_group_list *current_group = NULL;
+	int arg_enum; /* e.g. foo_ARG */
+	int goval;    /* the number returned from getopt_long identifying what it found */
+	int i;
 
 	if (!(cmd->opt_arg_values = dm_pool_zalloc(cmd->mem, sizeof(*cmd->opt_arg_values) * ARG_COUNT))) {
 		log_fatal("Unable to allocate memory for command line arguments.");
 		return 0;
 	}
 
-	/* fill in the short and long opts */
-	for (i = 0; i < cmd->command->num_args; i++)
-		_add_getopt_arg(cmd->command->valid_args[i], &ptr, &o);
+	/*
+	 * create the short-form character array (str) and the long-form option
+	 * array (opts) to pass to the getopt_long() function.  IOW we generate
+	 * the arguments to pass to getopt_long() from the args.h/arg_props data.
+	 */
+	for (i = 0; i < ARG_COUNT; i++)
+		_add_getopt_arg(_cmdline.arg_props[i].enum_val, &ptr, &o);
 
 	*ptr = '\0';
 	memset(o, 0, sizeof(*o));
@@ -1055,19 +1282,23 @@ static int _process_command_line(struct cmd_context *cmd, int *argc,
 	/* initialise getopt_long & scan for command line switches */
 	optarg = 0;
 	optind = OPTIND_INIT;
-	while ((opt = GETOPTLONG_FN(*argc, *argv, str, opts, NULL)) >= 0) {
+	while ((goval = GETOPTLONG_FN(*argc, *argv, str, opts, NULL)) >= 0) {
 
-		if (opt == '?')
+		if (goval == '?')
 			return 0;
 
-		if ((arg = _find_arg(cmd->command, opt)) < 0) {
+		/*
+		 * translate the option value used by getopt into the enum
+		 * value (e.g. foo_ARG) from the args array....
+		 */
+		if ((arg_enum = _find_arg(goval)) < 0) {
 			log_fatal("Unrecognised option.");
 			return 0;
 		}
 
-		a = _cmdline.arg_props + arg;
+		a = _cmdline.arg_props + arg_enum;
 
-		av = &cmd->opt_arg_values[arg];
+		av = &cmd->opt_arg_values[arg_enum];
 
 		if (a->flags & ARG_GROUPABLE) {
 			/*
@@ -1077,7 +1308,7 @@ static int _process_command_line(struct cmd_context *cmd, int *argc,
 			 *   - or if argument has higher priority than current group.
 			 */
 			if (!current_group ||
-			    (current_group->arg_values[arg].count && !(a->flags & ARG_COUNTABLE)) ||
+			    (current_group->arg_values[arg_enum].count && !(a->flags & ARG_COUNTABLE)) ||
 			    (current_group->prio < a->prio)) {
 				/* FIXME Reduce size including only groupable args */
 				if (!(current_group = dm_pool_zalloc(cmd->mem, sizeof(struct arg_value_group_list) + sizeof(*cmd->opt_arg_values) * ARG_COUNT))) {
@@ -1090,7 +1321,7 @@ static int _process_command_line(struct cmd_context *cmd, int *argc,
 			}
 			/* Maintain total argument count as well as count within each group */
 			av->count++;
-			av = &current_group->arg_values[arg];
+			av = &current_group->arg_values[arg_enum];
 		}
 
 		if (av->count && !(a->flags & ARG_COUNTABLE)) {
@@ -1338,7 +1569,10 @@ static int _get_settings(struct cmd_context *cmd)
 static int _process_common_commands(struct cmd_context *cmd)
 {
 	if (arg_is_set(cmd, help_ARG) || arg_is_set(cmd, help2_ARG)) {
-		_usage(cmd->command->name);
+		_usage(cmd->command->name, arg_count(cmd, help_ARG));
+
+		if (arg_count(cmd, help_ARG) < 2)
+			log_print("(Use --help --help to include optional parameters.)");
 		return ECMD_PROCESSED;
 	}
 
@@ -1358,10 +1592,10 @@ static void _display_help(void)
 	log_error("Use 'lvm help <command>' for more information");
 	log_error(" ");
 
-	for (i = 0; i < _cmdline.num_commands; i++) {
-		struct command *com = _cmdline.commands + i;
+	for (i = 0; i < _cmdline.num_command_names; i++) {
+		struct command_name *cname = _cmdline.command_names + i;
 
-		log_error("%-16.16s%s", com->name, com->desc);
+		log_error("%-16.16s%s", cname->name, cname->desc);
 	}
 }
 
@@ -1374,7 +1608,7 @@ int help(struct cmd_context *cmd __attribute__((unused)), int argc, char **argv)
 	else {
 		int i;
 		for (i = 0; i < argc; i++)
-			if (!_usage(argv[i]))
+			if (!_usage(argv[i], 0))
 				ret = EINVALID_CMD_LINE;
 	}
 
@@ -1652,6 +1886,7 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 {
 	struct dm_config_tree *config_string_cft, *config_profile_command_cft, *config_profile_metadata_cft;
 	const char *reason = NULL;
+	const char *cmd_name;
 	int ret = 0;
 	int locking_type;
 	int monitoring;
@@ -1665,6 +1900,8 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 	/* each command should start out with sigint flag cleared */
 	sigint_clear();
 
+	cmd_name = strdup(argv[0]);
+
 	/* eliminate '-' from all options starting with -- */
 	for (i = 1; i < argc; i++) {
 
@@ -1708,10 +1945,10 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		return EINVALID_CMD_LINE;
 	}
 
-	if (!(cmd->command = _find_command(cmd, argv[0])))
+	if (!(cmd->command = _find_command(cmd, cmd_name)))
 		return ENO_SUCH_CMD;
 
-	set_cmd_name(cmd->command->name);
+	set_cmd_name(cmd_name);
 
 	if (arg_is_set(cmd, backgroundfork_ARG)) {
 		if (!become_daemon(cmd, 1)) {

tools/tools.h

@@ -44,19 +44,12 @@
 #include "toollib.h"
 #include "lvmnotify.h"
 
-#include "command.h"
-
 #include <ctype.h>
 #include <sys/types.h>
 
 #define CMD_LEN 256
 #define MAX_ARGS 64
 
-/* command functions */
-#define xx(a, b...) int a(struct cmd_context *cmd, int argc, char **argv);
-#include "commands.h"
-#undef xx
-
 /* define the enums for the command line switches */
 enum {
 #define arg(a, b, c, d, e, f) a ,
@@ -64,6 +57,13 @@ enum {
 #undef arg
 };
 
+/* command functions */
+#define xx(a, b...) int a(struct cmd_context *cmd, int argc, char **argv);
+#include "commands.h"
+#undef xx
+
+#include "command.h"
+
 #define ARG_COUNTABLE 0x00000001	/* E.g. -vvvv */
 #define ARG_GROUPABLE 0x00000002	/* E.g. --addtag */
 
@@ -81,6 +81,7 @@ struct arg_values {
 
 /* a global table of possible arguments */
 struct arg_props {
+	int enum_val;
 	const char short_arg;
 	char _padding[7];
 	const char *long_arg;