commands: new method for defining commands

Only a command that conforms to one of the specifications
will be processed.  (LV type conformance is checked after
vg_read and before single_fn processing.)

scripts/create-commands.c

@@ -19,7 +19,6 @@
 #include <ctype.h>
 #include <getopt.h>
 
-
 /* needed to include args.h */
 #define ARG_COUNTABLE 0x00000001
 #define ARG_GROUPABLE 0x00000002
@@ -133,6 +132,8 @@ struct oo_line {
 int cmd_count;
 struct command cmd_array[MAX_CMDS];
 
+struct command common_options; /* for printing common usage */
+
 #define MAX_OO_LINES 256
 int oo_line_count;
 struct oo_line oo_lines[MAX_OO_LINES];
@@ -140,6 +141,36 @@ struct oo_line oo_lines[MAX_OO_LINES];
 
 static void add_optional_opt_line(struct command *cmd, int argc, char *argv[]);
 
+/*
+ * modifies buf, replacing the sep characters with \0
+ * argv pointers point to positions in buf
+ */
+
+static char *split_line(char *buf, int *argc, char **argv, char sep)
+{
+	char *p = buf, *rp = NULL;
+	int i;
+
+	argv[0] = p;
+
+	for (i = 1; i < MAX_LINE_ARGC; i++) {
+		p = strchr(buf, sep);
+		if (!p)
+			break;
+		*p = '\0';
+
+		argv[i] = p + 1;
+		buf = p + 1;
+	}
+	*argc = i;
+
+	/* we ended by hitting \0, return the point following that */
+	if (!rp)
+		rp = strchr(buf, '\0') + 1;
+
+	return rp;
+}
+
 /* convert value string, e.g. Number, to foo_VAL enum */
 
 static int val_str_to_num(char *str)
@@ -237,103 +268,104 @@ static char *val_bits_to_str(int val_bits)
  * are not exposed to lvm at large, which uses only the ARG_DEF values.
  */
 
-static int lv_str_to_num(char *str)
+static uint32_t lv_str_to_types(char *str)
 {
-	char name[32] = { 0 };
-	char *new;
+	char copy[128] = { 0 };
+	char *argv[MAX_LINE_ARGC];
+	int argc;
+	char *name;
+	uint32_t types = 0;
 	int i;
 
-	/* compare the lv name before the _new suffix */
+	strncpy(copy, str, 128);
 
-	strncpy(name, str, 31);
-	if ((new = strstr(name, "_new")))
-		*new = '\0';
+	split_line(copy, &argc, argv, '_');
 
-	if (!strcmp(name, "LV"))
-		return 0;
+	for (i = 0; i < argc; i++) {
+		name = argv[i];
 
-	if (!strcmp(name, "LV_linear"))
-		return ARG_DEF_LV_LINEAR;
+		if (!strcmp(name, "linear"))
+			types |= ARG_DEF_LV_LINEAR;
 
-	if (!strcmp(name, "LV_striped"))
-		return ARG_DEF_LV_STRIPED;
+		if (!strcmp(name, "striped"))
+			types |= ARG_DEF_LV_STRIPED;
 
-	if (!strcmp(name, "LV_snapshot"))
-		return ARG_DEF_LV_SNAPSHOT;
+		if (!strcmp(name, "snapshot"))
+			types |= ARG_DEF_LV_SNAPSHOT;
 
-	if (!strcmp(name, "LV_mirror"))
-		return ARG_DEF_LV_MIRROR;
+		if (!strcmp(name, "mirror"))
+			types |= ARG_DEF_LV_MIRROR;
 
-	if (!strcmp(name, "LV_raid"))
-		return ARG_DEF_LV_RAID;
+		if (!strcmp(name, "thin"))
+			types |= ARG_DEF_LV_THIN;
 
-	if (!strcmp(name, "LV_raid0"))
-		return ARG_DEF_LV_RAID0;
+		if (!strcmp(name, "thinpool"))
+			types |= ARG_DEF_LV_THINPOOL;
 
-	if (!strcmp(name, "LV_raid1"))
-		return ARG_DEF_LV_RAID1;
+		if (!strcmp(name, "cache"))
+			types |= ARG_DEF_LV_CACHE;
 
-	if (!strcmp(name, "LV_raid4"))
-		return ARG_DEF_LV_RAID4;
+		if (!strcmp(name, "cachepool"))
+			types |= ARG_DEF_LV_CACHEPOOL;
 
-	if (!strcmp(name, "LV_raid5"))
-		return ARG_DEF_LV_RAID5;
+		if (!strcmp(name, "raid0"))
+			types |= ARG_DEF_LV_RAID0;
 
-	if (!strcmp(name, "LV_raid6"))
-		return ARG_DEF_LV_RAID6;
+		if (!strcmp(name, "raid1"))
+			types |= ARG_DEF_LV_RAID1;
 
-	if (!strcmp(name, "LV_raid10"))
-		return ARG_DEF_LV_RAID10;
+		if (!strcmp(name, "raid4"))
+			types |= ARG_DEF_LV_RAID4;
 
-	if (!strcmp(name, "LV_thin"))
-		return ARG_DEF_LV_THIN;
+		if (!strcmp(name, "raid5"))
+			types |= ARG_DEF_LV_RAID5;
 
-	if (!strcmp(name, "LV_thinpool"))
-		return ARG_DEF_LV_THINPOOL;
+		if (!strcmp(name, "raid6"))
+			types |= ARG_DEF_LV_RAID6;
 
-	if (!strcmp(name, "LV_cache"))
-		return ARG_DEF_LV_CACHE;
+		if (!strcmp(name, "raid10"))
+			types |= ARG_DEF_LV_RAID10;
 
-	if (!strcmp(name, "LV_cachepool"))
-		return ARG_DEF_LV_CACHEPOOL;
+		if (!strcmp(name, "raid"))
+			types |= ARG_DEF_LV_RAID;
+	}
 
-	printf("Unknown LV type: \"%s\" \"%s\"\n", name, str);
-	exit(1);
+	return types;
 }
 
 static const char *lv_num_to_str(int num)
 {
 	switch (num) {
 	case ARG_DEF_LV_LINEAR:
-		return "LV_linear";
+		return "linear";
 	case ARG_DEF_LV_STRIPED:
-		return "LV_striped";
+		return "striped";
 	case ARG_DEF_LV_SNAPSHOT:
-		return "LV_snapshot";
+		return "snapshot";
 	case ARG_DEF_LV_MIRROR:
-		return "LV_mirror";
+		return "mirror";
 	case ARG_DEF_LV_RAID:
-		return "LV_raid";
+		return "raid";
 	case ARG_DEF_LV_RAID0:
-		return "LV_raid0";
+		return "raid0";
 	case ARG_DEF_LV_RAID1:
-		return "LV_raid1";
+		return "raid1";
 	case ARG_DEF_LV_RAID4:
-		return "LV_raid4";
+		return "raid4";
 	case ARG_DEF_LV_RAID5:
-		return "LV_raid5";
+		return "raid5";
 	case ARG_DEF_LV_RAID6:
-		return "LV_raid6";
+		return "raid6";
 	case ARG_DEF_LV_RAID10:
-		return "LV_raid10";
+		return "raid10";
 	case ARG_DEF_LV_THIN:
-		return "LV_thin";
+		return "thin";
 	case ARG_DEF_LV_THINPOOL:
-		return "LV_thinpool";
+		return "thinpool";
 	case ARG_DEF_LV_CACHE:
-		return "LV_cache";
+		return "cache";
 	case ARG_DEF_LV_CACHEPOOL:
-		return "LV_cachepool";
+		return "cachepool";
 	default:
 		printf("lv_num_to_str: unknown LV num: %d\n", num);
 		exit(1);
@@ -440,36 +472,6 @@ static char *lv_types_to_flags(int lv_types)
 	return buf_lv_types;
 }
 
-/*
- * modifies buf, replacing the sep characters with \0
- * argv pointers point to positions in buf
- */
-
-static char *split_line(char *buf, int *argc, char **argv, char sep)
-{
-	char *p = buf, *rp = NULL;
-	int i;
-
-	argv[0] = p;
-
-	for (i = 1; i < MAX_LINE_ARGC; i++) {
-		p = strchr(buf, sep);
-		if (!p)
-			break;
-		*p = '\0';
-
-		argv[i] = p + 1;
-		buf = p + 1;
-	}
-	*argc = i;
-
-	/* we ended by hitting \0, return the point following that */
-	if (!rp)
-		rp = strchr(buf, '\0') + 1;
-
-	return rp;
-}
-
 static const char *is_command_name(char *str)
 {
 	int i;
@@ -574,7 +576,7 @@ static void set_pos_def(struct command *cmd, char *str, struct arg_def *def)
 		def->val_bits |= val_enum_to_bit(val_enum);
 
 		if ((val_enum == lv_VAL) && strstr(name, "_"))
-			def->lv_types |= lv_str_to_num(name);
+			def->lv_types = lv_str_to_types(name);
 
 		if (strstr(name, "_new"))
 			def->flags |= ARG_DEF_FLAG_NEW;
@@ -626,7 +628,7 @@ static void set_opt_def(struct command *cmd, char *str, struct arg_def *def)
 
 		if (val_enum == lv_VAL) {
 			if (strstr(name, "_"))
-				def->lv_types |= lv_str_to_num(name);
+				def->lv_types = lv_str_to_types(name);
 		}
 
 		if ((val_enum == vg_VAL) || (val_enum == lv_VAL) || (val_enum == pv_VAL)) {
@@ -956,20 +958,7 @@ static void print_def(struct arg_def *def, int usage)
 			else if (val_enum == constnum_VAL)
 				printf("ll%u", (unsigned long long)def->num);
 
-			else if ((val_enum == lv_VAL) && !def->lv_types) {
-				if (sep) printf("|");
-				printf("%s", val_names[val_enum].name);
-				sep = 1;
-
-			} else if ((val_enum == lv_VAL) && def->lv_types) {
-				for (i = 0; i < 32; i++) {
-					if (def->lv_types & (1 << i)) {
-						if (sep) printf("|");
-						printf("%s", lv_num_to_str(1 << i));
-						sep = 1;
-					}
-				}
-			} else {
+			else {
 				if (sep) printf("|");
 
 				if (!usage || !val_names[val_enum].usage)
@@ -980,6 +969,13 @@ static void print_def(struct arg_def *def, int usage)
 				sep = 1;
 			}
 
+			if (val_enum == lv_VAL && def->lv_types) {
+				for (i = 0; i < 32; i++) {
+					if (def->lv_types & (1 << i))
+						printf("_%s", lv_num_to_str(1 << i));
+				}
+			}
+
 			if ((val_enum == pv_VAL) || (val_enum == vg_VAL) || (val_enum == lv_VAL)) {
 				if (def->flags & ARG_DEF_FLAG_NEW)
 					printf("_new");
@@ -1058,6 +1054,52 @@ void print_expanded(void)
 	}
 }
 
+static int opt_arg_matches(struct opt_arg *oa1, struct opt_arg *oa2)
+{
+	if (oa1->opt != oa2->opt)
+		return 0;
+
+	/* FIXME: some cases may need more specific val_bits checks */
+	if (oa1->def.val_bits != oa2->def.val_bits)
+		return 0;
+
+	if (oa1->def.str && oa2->def.str && strcmp(oa1->def.str, oa2->def.str))
+		return 0;
+
+	if (oa1->def.num != oa2->def.num)
+		return 0;
+
+	/*
+	 * Do NOT compare lv_types because we are checking if two
+	 * command lines are ambiguous before the LV type is known.
+	 */
+
+	return 1;
+}
+
+static int pos_arg_matches(struct pos_arg *pa1, struct pos_arg *pa2)
+{
+	if (pa1->pos != pa2->pos)
+		return 0;
+
+	/* FIXME: some cases may need more specific val_bits checks */
+	if (pa1->def.val_bits != pa2->def.val_bits)
+		return 0;
+
+	if (pa1->def.str && pa2->def.str && strcmp(pa1->def.str, pa2->def.str))
+		return 0;
+
+	if (pa1->def.num != pa2->def.num)
+		return 0;
+
+	/*
+	 * Do NOT compare lv_types because we are checking if two
+	 * command lines are ambiguous before the LV type is known.
+	 */
+
+	return 1;
+}
+
 static const char *opt_to_enum_str(int opt)
 {
 	return opt_names[opt].name;
@@ -1084,10 +1126,35 @@ void print_define_command_count(void)
 	printf("#define COMMAND_COUNT %d\n", cmd_count);
 }
 
-void print_usage(struct command *cmd)
+static int is_common_opt(int opt)
+{
+	int oo;
+
+	for (oo = 0; oo < common_options.oo_count; oo++) {
+		if (common_options.optional_opt_args[oo].opt == opt)
+			return 1;
+	}
+	return 0;
+}
+
+/*
+ * For certain commands (esp commands like lvcreate with many variants), common
+ * options should not be printed for every variation, but once for all.  The
+ * list of commands this applies to is fixed for now but could be encoded in
+ * command-lines.in.
+ *
+ * The common options are defined in OO_USAGE_COMMON.  Those options
+ * are skipped when creating the usage strings for each variation of
+ * these commands.  Instead they are set in the usage_common string.
+ */
+
+void print_usage(struct command *cmd, int skip_required)
 {
 	int onereq = (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) ? 1 : 0;
-	int i, ro, rp, oo, op;
+	int i, sep, ro, rp, oo, op;
+
+	if (skip_required)
+		goto oo_count;
 
 	printf("\"%s", cmd->name);
 
@@ -1119,25 +1186,37 @@ void print_usage(struct command *cmd)
 
 	printf("\"");
 
+ oo_count:
 	if (!cmd->oo_count)
 		goto op_count;
 
-	printf("\n");
-	printf("\" [");
+	sep = 0;
 
 	if (cmd->oo_count) {
 		for (oo = 0; oo < cmd->oo_count; oo++) {
-			if (oo)
+			/* skip common opts which are in the usage_common string */
+			if ((cmd != &common_options) && is_common_opt(cmd->optional_opt_args[oo].opt))
+				continue;
+
+			if (!sep) {
+				printf("\n");
+				printf("\" [");
+			}
+
+			if (sep)
 				printf(",");
+
 			printf(" %s", opt_names[cmd->optional_opt_args[oo].opt].long_opt);
 			if (cmd->optional_opt_args[oo].def.val_bits) {
 				printf(" ");
 				print_def(&cmd->optional_opt_args[oo].def, 1);
 			}
+			sep = 1;
 		}
 	}
 
-	printf(" ]\"");
+	if (sep)
+		printf(" ]\"");
 
  op_count:
 	if (!cmd->op_count)
@@ -1166,6 +1245,8 @@ void print_command_struct(int only_usage)
 	struct command *cmd;
 	int i, j, ro, rp, oo, op;
 
+	include_optional_opt_args(&common_options, "OO_USAGE_COMMON");
+
 	printf("/* Do not edit. This file is generated by scripts/create-commands */\n");
 	printf("/* using command definitions from scripts/command-lines.in */\n");
 
@@ -1173,7 +1254,8 @@ void print_command_struct(int only_usage)
 		cmd = &cmd_array[i];
 
 		if (only_usage) {
-			print_usage(cmd);
+			print_usage(cmd, 0);
+			print_usage(&common_options, 1);
 			printf("\n");
 			continue;
 		}
@@ -1190,7 +1272,14 @@ void print_command_struct(int only_usage)
 
 		printf("commands[%d].desc = \"%s\";\n", i, cmd->desc ?: "");
 		printf("commands[%d].usage = ", i);
-		print_usage(cmd);
+		print_usage(cmd, 0);
+
+		if (cmd->oo_count) {
+			printf("commands[%d].usage_common = ", i);
+			print_usage(&common_options, 1);
+		} else {
+			printf("commands[%d].usage_common = \"NULL\";\n", i);
+		}
 
 		if (cmd->ro_count) {
 			for (ro = 0; ro < cmd->ro_count; ro++) {
@@ -1312,6 +1401,67 @@ void print_command_struct(int only_usage)
 	}
 }
 
+struct cmd_pair {
+	int i, j;
+};
+
+static void print_ambiguous(void)
+{
+	struct command *cmd, *dup;
+	struct cmd_pair dups[64] = { 0 };
+	int found = 0;
+	int i, j, f, ro, rp;
+
+	for (i = 0; i < cmd_count; i++) {
+		cmd = &cmd_array[i];
+
+		for (j = 0; j < cmd_count; j++) {
+			dup = &cmd_array[j];
+
+			if (i == j)
+				continue;
+			if (strcmp(cmd->name, dup->name))
+				continue;
+			if (cmd->ro_count != dup->ro_count)
+				continue;
+			if (cmd->oo_count != dup->oo_count)
+				continue;
+			if (cmd->rp_count != dup->rp_count)
+				continue;
+			if (cmd->op_count != dup->op_count)
+				continue;
+
+			for (ro = 0; ro < cmd->ro_count; ro++) {
+				if (!opt_arg_matches(&cmd->required_opt_args[ro],
+						     &dup->required_opt_args[ro]))
+					goto next;
+			}
+
+			for (rp = 0; rp < cmd->rp_count; rp++) {
+				if (!pos_arg_matches(&cmd->required_pos_args[rp],
+						     &dup->required_pos_args[rp]))
+					goto next;
+			}
+
+			for (f = 0; f < found; f++) {
+				if ((dups[f].i == j) && (dups[f].j == i))
+					goto next;
+			}
+
+			printf("Ambiguous commands %d and %d:\n", i, j);
+			print_usage(cmd, 0);
+			print_usage(dup, 0);
+			printf("\n");
+
+			dups[found].i = i;
+			dups[found].j = j;
+			found++;
+next:
+			;
+		}
+	}
+}
+
 void print_command_list(void)
 {
 	int i;
@@ -1340,10 +1490,11 @@ static void print_help(int argc, char *argv[])
 {
 	printf("%s --output struct|count|usage|expanded <filename>\n", argv[0]);
 	printf("\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");
+	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");
+	printf("ambiguous: print commands differing only by LV types\n");
 }
 
 int main(int argc, char *argv[])
@@ -1417,6 +1568,8 @@ int main(int argc, char *argv[])
 			continue;
 		if (line[0] == '\n')
 			continue;
+		if (line[0] == '-' && line[1] == '-' && line[2] == '-')
+			continue;
 
 		if ((n = strchr(line, '\n')))
 			*n = '\0';
@@ -1444,7 +1597,7 @@ int main(int argc, char *argv[])
 		}
 
 		if (is_desc_line(line_argv[0])) {
-			char *desc = strdup(strstr(line_orig, ":") + 2);
+			char *desc = strdup(line_orig);
 			if (cmd->desc) {
 				cmd->desc = realloc((char *)cmd->desc, strlen(cmd->desc) + strlen(desc) + 2);
 				strcat((char *)cmd->desc, "  ");
@@ -1512,6 +1665,8 @@ int main(int argc, char *argv[])
 		print_command_struct(1);
 	else if (!strcmp(outputformat, "expanded"))
 		print_expanded();
+	else if (!strcmp(outputformat, "ambiguous"))
+		print_ambiguous();
 	else
 		print_help(argc, argv);
 }

tools/command-lines-count.h

@@ -1,3 +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
+#define COMMAND_COUNT 146

tools/command.h

@@ -111,7 +111,8 @@ struct pos_arg {
 struct command {
 	const char *name;
 	const char *desc; /* specific command description from command-lines.h */
-	const char *usage;
+	const char *usage; /* excludes common options like --help, --debug */
+	const char *usage_common; /* includes commmon options like --help, --debug */
 
 	struct command_name *cname;
 	command_fn fn;

tools/lvmcmdline.c

@@ -963,15 +963,17 @@ static int _command_required_pos_matches(struct cmd_context *cmd, int ci, int rp
 
 #define HELP_LINE_SIZE 1024
 
-static void _print_usage(int ci, int include_optional)
+static void _print_usage(const char *usage, int only_required)
 {
-	const char *usage = _cmdline.commands[ci].usage;
 	char buf[HELP_LINE_SIZE];
 	int optional_ui = 0;
 	int optional_pos_ui = 0;
 	int ui;
 	int bi;
 
+	if (!usage || !strlen(usage))
+		return;
+
 	/*
 	 * copy the required opt_args/pos_args
 	 *
@@ -987,6 +989,8 @@ static void _print_usage(int ci, int include_optional)
 	bi = 0;
 
 	for (ui = 0; ui < strlen(usage); ui++) {
+		if (!bi && ((usage[ui] == ' ') || (usage[ui] == '\n')))
+			continue;
 
 		/* The first "[ " indicates the start of the optional opt_args. */
 		if ((usage[ui] == '[') && (usage[ui+1] == ' ')) {
@@ -996,43 +1000,39 @@ static void _print_usage(int ci, int include_optional)
 
 		if (usage[ui] == '\0')
 			break;
-		if (usage[ui] == '\n')
-			break;
 
 		buf[bi++] = usage[ui];
 
 		if (usage[ui] == ',') {
 			buf[bi++] = '\n';
 			buf[bi++] = '\t';
+			buf[bi++] = ' ';
 		}
 
 		if (bi == (HELP_LINE_SIZE - 1))
 			break;
 	}
 
-	if (!include_optional) {
-		/* print only the required opt_args/pos_args */
-		log_print("%s", buf);
-		return;
-	}
-
 	/*
 	 * print the required opt_args/pos_args
 	 */
-	log_print("%s", buf);
+
+	if (bi)
+		log_print("%s", buf);
+
+	if (only_required)
+		return;
 
 	/*
 	 * copy the optional opt_args
 	 */
 
 	if (!optional_ui)
-		return;
+		goto out;
 
 	memset(buf, 0, sizeof(buf));
 	bi = 0;
 
-	buf[bi++] = '\t';
-
 	for (ui = optional_ui; ui < strlen(usage); ui++) {
 
 		/* The second "[ " indicates the start of the optional pos_args. */
@@ -1046,11 +1046,15 @@ static void _print_usage(int ci, int include_optional)
 		if (usage[ui] == '\n')
 			break;
 
+		if (!bi)
+			buf[bi++] = '\t';
+
 		buf[bi++] = usage[ui];
 
 		if (usage[ui] == ',') {
 			buf[bi++] = '\n';
 			buf[bi++] = '\t';
+			buf[bi++] = ' ';
 		}
 
 		if (bi == (HELP_LINE_SIZE - 1))
@@ -1060,26 +1064,29 @@ static void _print_usage(int ci, int include_optional)
 	/*
 	 * print the optional opt_args
 	 */
-	log_print("%s", buf);
+
+	if (bi)
+		log_print("%s", buf);
 
 	/*
 	 * copy the optional pos_args
 	 */
 
 	if (!optional_pos_ui)
-		return;
+		goto out;
 
 	memset(buf, 0, sizeof(buf));
 	bi = 0;
 
-	buf[bi++] = '\t';
-
 	for (ui = optional_pos_ui; ui < strlen(usage); ui++) {
 		if (usage[ui] == '\0')
 			break;
 		if (usage[ui] == '\n')
 			break;
 
+		if (!bi)
+			buf[bi++] = '\t';
+
 		buf[bi++] = usage[ui];
 
 		if (bi == (HELP_LINE_SIZE - 1))
@@ -1089,13 +1096,12 @@ static void _print_usage(int ci, int include_optional)
 	/*
 	 * print the optional pos_args
 	 */
-	log_print("%s", buf);
-}
 
-/*
- * A description is a string with multiple sentences each ending in periods.
- * Print each sentence on a new line.
- */
+	if (bi)
+		log_print("%s", buf);
+ out:
+	return;
+}
 
 static void _print_description(int ci)
 {
@@ -1110,20 +1116,27 @@ static void _print_description(int ci)
 		if (desc[di] == '\n')
 			continue;
 
+		if (!strncmp(&desc[di], "DESC:", 5)) {
+			if (bi) {
+				log_print("%s", buf);
+				memset(buf, 0, sizeof(buf));
+				bi = 0;
+			}
+			di += 5;
+			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;
 	}
+
+	if (bi)
+		log_print("%s", buf);
 }
 
 static struct command *_find_command(struct cmd_context *cmd, const char *path, int *argc, char **argv)
@@ -1131,7 +1144,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	const char *name;
 	int match_count, match_count_ro, match_count_rp, mismatch_count;
 	int best_i = 0, best_count = 0;
-	int closest_i = 0, closest_count = 0;
+	int closest_i = 0, closest_count_ro = 0;
 	int ro, rp;
 	int i, j;
 	int accepted, count;
@@ -1201,10 +1214,10 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 		/* 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)) {
+			/* save "closest" command that doesn't match */
+			if (match_count_ro && (match_count_ro > closest_count_ro)) {
 				closest_i = i;
-				closest_count = match_count;
+				closest_count_ro = match_count_ro;
 			}
 			continue;
 		}
@@ -1236,10 +1249,10 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 
 	if (!best_count) {
 		/* 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_error("Failed to find a matching command definition.");
+		if (closest_count_ro) {
 			log_warn("Closest command usage is:");
-			_print_usage(closest_i, 0);
+			_print_usage(_cmdline.commands[closest_i].usage, 1);
 		}
 		return NULL;
 	}
@@ -1282,6 +1295,17 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			}
 		}
 
+		/*
+		 * --type is one option that when set but not accepted by the
+		 * command, will not be ignored to make a match.  Perhaps there
+		 * are others like this, and perhaps this is a property that
+		 * should be encoded in args.h?
+		 */
+		if (!accepted && (i == type_ARG)) {
+			log_error("Failed to find a matching command definition with --type.");
+			return NULL;
+		}
+
 		if (!accepted) {
 			log_warn("Ignoring option which is not used by the specified command: %s.",
 				 arg_long_option_name(i));
@@ -1334,6 +1358,7 @@ static void _short_usage(const char *name)
 static int _usage(const char *name, int help_count)
 {
 	struct command_name *cname = _find_command_name(name);
+	const char *usage_common = NULL;
 	int i;
 
 	if (!cname) {
@@ -1350,7 +1375,16 @@ static int _usage(const char *name, int help_count)
 		if (strlen(_cmdline.commands[i].desc))
 			_print_description(i);
 
-		_print_usage(i, help_count > 1);
+		usage_common = _cmdline.commands[i].usage_common;
+
+		_print_usage(_cmdline.commands[i].usage, 0);
+		log_print(" "); /* for built-in \n */
+	}
+
+	/* Common options are printed once for all variants of a command name. */
+	if (usage_common) {
+		log_print("Common options:");
+		_print_usage(usage_common, 0);
 		log_print(" "); /* for built-in \n */
 	}
 
@@ -1362,9 +1396,11 @@ static int _usage(const char *name, int help_count)
 		 */
 		log_print("Usage notes:");
 		log_print(". Variable parameters are: Number, String, PV, VG, LV, Tag.");
+		log_print(". Select indicates that a required positional parameter can");
+		log_print("  be omitted if the --select option is used.");
 		log_print(". --size Number can be replaced with --extents NumberExtents.");
-		log_print(". Select indicates that the positional arg can be omitted");
-		log_print("  if the --select option is used.");
+		log_print(". --name is usually specified for lvcreate, but is not among");
+		log_print("  the required parameters because names can be generated.");
 		log_print(". For required options listed in parentheses, e.g. (--A, --B),");
 		log_print("  any one is required, after which the others are optional.");
 		log_print(". The _new suffix indicates the VG or LV must not yet exist.");
@@ -1816,7 +1852,7 @@ static int _process_common_commands(struct cmd_context *cmd)
 		_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.)");
+			log_print("(Use --help --help for usage notes.)");
 		return ECMD_PROCESSED;
 	}
 
@@ -2196,7 +2232,7 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 	log_debug("Parsing: %s", cmd->cmd_line);
 
 	if (!(cmd->command = _find_command(cmd, cmd_name, &argc, argv)))
-		return ENO_SUCH_CMD;
+		return_ECMD_FAILED;
 
 	set_cmd_name(cmd_name);
 

tools/toollib.c

@@ -2346,8 +2346,12 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 	struct dm_str_list *sl;
 	struct dm_list final_lvs;
 	struct lv_list *final_lvl;
+	struct dm_list found_arg_lvnames;
 	struct glv_list *glvl, *tglvl;
 	int do_report_ret_code = 1;
+	uint32_t lv_types;
+	struct logical_volume *lv;
+	struct lv_segment *seg;
 
 	log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_LV);
 
@@ -2356,6 +2360,7 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 		stack;
 
 	dm_list_init(&final_lvs);
+	dm_list_init(&found_arg_lvnames);
 
 	if (!vg_check_status(vg, EXPORTED_VG)) {
 		ret_max = ECMD_FAILED;
@@ -2449,6 +2454,7 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 		if (lvargs_supplied && str_list_match_item(arg_lvnames, lvl->lv->name)) {
 			/* Remove LV from list of unprocessed LV names */
 			str_list_del(arg_lvnames, lvl->lv->name);
+			str_list_add(cmd->mem, &found_arg_lvnames, lvl->lv->name);
 			process_lv = 1;
 		}
 
@@ -2496,6 +2502,64 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 		if (lv_is_removed(lvl->lv))
 			continue;
 
+		/*
+		 * If the command definition specifies one required positional
+		 * LV (possibly repeatable), and specifies accepted LV types,
+		 * then verify that the LV being processed matches one of those
+		 * types.
+		 *
+		 * process_each_lv() can only be used for commands that have
+		 * one positional LV arg (optionally repeating, where each is
+		 * processed independently.)  It cannot work for commands that
+		 * have different required LVs in designated positions, like
+		 * 'lvrename LV1 LV2', where each LV is not processed
+		 * independently.  That means that this LV type check only
+		 * needs to check the lv_type of the first positional arg.
+		 *
+		 * There is one command that violates this rule by stealing
+		 * the first positional LV arg before calling process_each_lv:
+		 * lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
+		 * This code cannot validate that case.  process_each_lv() sees
+		 * a single LV name arg, but it's in pos 2.  Could we work around
+		 * this by looking at the final positional arg rather than always
+		 * looking at pos 1?
+		 *
+		 * This only validates types for required LV positional args
+		 * (currently there are no command specifications that include
+		 * specific LV types in optional positional args.)
+		 */
+
+		if ((cmd->command->rp_count == 1) &&
+		     val_bit_is_set(cmd->command->required_pos_args[0].def.val_bits, lv_VAL) &&
+		     cmd->command->required_pos_args[0].def.lv_types) {
+
+			lv_types = cmd->command->required_pos_args[0].def.lv_types;
+			lv = lvl->lv;
+			seg = first_seg(lv);
+
+			if ((lv_is_cow(lv) && !(lv_types & ARG_DEF_LV_SNAPSHOT)) ||
+			    (lv_is_thin_volume(lv) && !(lv_types & ARG_DEF_LV_THIN)) ||
+			    (lv_is_thin_pool(lv) && !(lv_types & ARG_DEF_LV_THINPOOL)) ||
+			    (lv_is_cache(lv) && !(lv_types & ARG_DEF_LV_CACHE)) ||
+			    (lv_is_cache_pool(lv) && !(lv_types & ARG_DEF_LV_CACHEPOOL)) ||
+			    (lv_is_mirror(lv) && !(lv_types & ARG_DEF_LV_MIRROR)) ||
+			    (lv_is_raid(lv) && !(lv_types & (ARG_DEF_LV_RAID | ARG_DEF_LV_RAID0 | ARG_DEF_LV_RAID1 | ARG_DEF_LV_RAID4 | ARG_DEF_LV_RAID5 | ARG_DEF_LV_RAID6 | ARG_DEF_LV_RAID10))) ||
+			    (segtype_is_striped(seg->segtype) && !(lv_types & ARG_DEF_LV_STRIPED)) ||
+			    (segtype_is_linear(seg->segtype) && !(lv_types & ARG_DEF_LV_LINEAR))) {
+				/*
+				 * If a named LV arg cannot be processed it's an error, otherwise
+				 * the LV is skipped and doesn't cause the command to fail.
+				 */
+				if (str_list_match_item(&found_arg_lvnames, lv->name)) {
+					log_error("Operation not permitted on LV %s with type %s.", display_lvname(lv), seg->segtype->name);
+					ret_max = ECMD_FAILED;
+				} else {
+					log_warn("Operation not permitted on LV %s with type %s.", display_lvname(lv), seg->segtype->name);
+				}
+				continue;
+			}
+		}
+
 		log_very_verbose("Processing LV %s in VG %s.", lvl->lv->name, vg->name);
 
 		ret = process_single_lv(cmd, lvl->lv, handle);