commands: new method for defining commands

scripts/create-commands.c

@@ -19,6 +19,7 @@
 #include <ctype.h>
 #include <getopt.h>
 
+
 /* needed to include args.h */
 #define ARG_COUNTABLE 0x00000001
 #define ARG_GROUPABLE 0x00000002
@@ -132,8 +133,6 @@ 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];
@@ -141,36 +140,6 @@ 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)
@@ -268,104 +237,103 @@ static char *val_bits_to_str(int val_bits)
  * are not exposed to lvm at large, which uses only the ARG_DEF values.
  */
 
-static uint32_t lv_str_to_types(char *str)
+static int lv_str_to_num(char *str)
 {
-	char copy[128] = { 0 };
-	char *argv[MAX_LINE_ARGC];
-	int argc;
-	char *name;
-	uint32_t types = 0;
+	char name[32] = { 0 };
+	char *new;
 	int i;
 
-	strncpy(copy, str, 128);
+	/* compare the lv name before the _new suffix */
 
-	split_line(copy, &argc, argv, '_');
+	strncpy(name, str, 31);
+	if ((new = strstr(name, "_new")))
+		*new = '\0';
 
-	for (i = 0; i < argc; i++) {
-		name = argv[i];
+	if (!strcmp(name, "LV"))
+		return 0;
 
-		if (!strcmp(name, "linear"))
-			types |= ARG_DEF_LV_LINEAR;
+	if (!strcmp(name, "LV_linear"))
+		return ARG_DEF_LV_LINEAR;
 
-		if (!strcmp(name, "striped"))
-			types |= ARG_DEF_LV_STRIPED;
+	if (!strcmp(name, "LV_striped"))
+		return ARG_DEF_LV_STRIPED;
 
-		if (!strcmp(name, "snapshot"))
-			types |= ARG_DEF_LV_SNAPSHOT;
+	if (!strcmp(name, "LV_snapshot"))
+		return ARG_DEF_LV_SNAPSHOT;
 
-		if (!strcmp(name, "mirror"))
-			types |= ARG_DEF_LV_MIRROR;
+	if (!strcmp(name, "LV_mirror"))
+		return ARG_DEF_LV_MIRROR;
 
-		if (!strcmp(name, "thin"))
-			types |= ARG_DEF_LV_THIN;
+	if (!strcmp(name, "LV_raid"))
+		return ARG_DEF_LV_RAID;
 
-		if (!strcmp(name, "thinpool"))
-			types |= ARG_DEF_LV_THINPOOL;
+	if (!strcmp(name, "LV_raid0"))
+		return ARG_DEF_LV_RAID0;
 
-		if (!strcmp(name, "cache"))
-			types |= ARG_DEF_LV_CACHE;
+	if (!strcmp(name, "LV_raid1"))
+		return ARG_DEF_LV_RAID1;
 
-		if (!strcmp(name, "cachepool"))
-			types |= ARG_DEF_LV_CACHEPOOL;
+	if (!strcmp(name, "LV_raid4"))
+		return ARG_DEF_LV_RAID4;
 
-		if (!strcmp(name, "raid0"))
-			types |= ARG_DEF_LV_RAID0;
+	if (!strcmp(name, "LV_raid5"))
+		return ARG_DEF_LV_RAID5;
 
-		if (!strcmp(name, "raid1"))
-			types |= ARG_DEF_LV_RAID1;
+	if (!strcmp(name, "LV_raid6"))
+		return ARG_DEF_LV_RAID6;
 
-		if (!strcmp(name, "raid4"))
-			types |= ARG_DEF_LV_RAID4;
+	if (!strcmp(name, "LV_raid10"))
+		return ARG_DEF_LV_RAID10;
 
-		if (!strcmp(name, "raid5"))
-			types |= ARG_DEF_LV_RAID5;
+	if (!strcmp(name, "LV_thin"))
+		return ARG_DEF_LV_THIN;
 
-		if (!strcmp(name, "raid6"))
-			types |= ARG_DEF_LV_RAID6;
+	if (!strcmp(name, "LV_thinpool"))
+		return ARG_DEF_LV_THINPOOL;
 
-		if (!strcmp(name, "raid10"))
-			types |= ARG_DEF_LV_RAID10;
+	if (!strcmp(name, "LV_cache"))
+		return ARG_DEF_LV_CACHE;
 
-		if (!strcmp(name, "raid"))
-			types |= ARG_DEF_LV_RAID;
-	}
+	if (!strcmp(name, "LV_cachepool"))
+		return ARG_DEF_LV_CACHEPOOL;
 
-	return types;
+	printf("Unknown LV type: \"%s\" \"%s\"\n", name, str);
+	exit(1);
 }
 
 static const char *lv_num_to_str(int num)
 {
 	switch (num) {
 	case ARG_DEF_LV_LINEAR:
-		return "linear";
+		return "LV_linear";
 	case ARG_DEF_LV_STRIPED:
-		return "striped";
+		return "LV_striped";
 	case ARG_DEF_LV_SNAPSHOT:
-		return "snapshot";
+		return "LV_snapshot";
 	case ARG_DEF_LV_MIRROR:
-		return "mirror";
+		return "LV_mirror";
 	case ARG_DEF_LV_RAID:
-		return "raid";
+		return "LV_raid";
 	case ARG_DEF_LV_RAID0:
-		return "raid0";
+		return "LV_raid0";
 	case ARG_DEF_LV_RAID1:
-		return "raid1";
+		return "LV_raid1";
 	case ARG_DEF_LV_RAID4:
-		return "raid4";
+		return "LV_raid4";
 	case ARG_DEF_LV_RAID5:
-		return "raid5";
+		return "LV_raid5";
 	case ARG_DEF_LV_RAID6:
-		return "raid6";
+		return "LV_raid6";
 	case ARG_DEF_LV_RAID10:
-		return "raid10";
+		return "LV_raid10";
 	case ARG_DEF_LV_THIN:
-		return "thin";
+		return "LV_thin";
 	case ARG_DEF_LV_THINPOOL:
-		return "thinpool";
+		return "LV_thinpool";
 	case ARG_DEF_LV_CACHE:
-		return "cache";
+		return "LV_cache";
 	case ARG_DEF_LV_CACHEPOOL:
-		return "cachepool";
+		return "LV_cachepool";
 	default:
 		printf("lv_num_to_str: unknown LV num: %d\n", num);
 		exit(1);
@@ -472,6 +440,36 @@ 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;
@@ -548,13 +546,6 @@ static int is_desc_line(char *str)
 	return 0;
 }
 
-static int is_id_line(char *str)
-{
-	if (!strncmp(str, "ID:", 3))
-		return 1;
-	return 0;
-}
-
 /*
  * parse str for anything that can appear in a position,
  * like VG, VG|LV, VG|LV_linear|LV_striped, etc
@@ -583,7 +574,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_types(name);
+			def->lv_types |= lv_str_to_num(name);
 
 		if (strstr(name, "_new"))
 			def->flags |= ARG_DEF_FLAG_NEW;
@@ -635,7 +626,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_types(name);
+				def->lv_types |= lv_str_to_num(name);
 		}
 
 		if ((val_enum == vg_VAL) || (val_enum == lv_VAL) || (val_enum == pv_VAL)) {
@@ -965,7 +956,20 @@ 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 {
+			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 {
 				if (sep) printf("|");
 
 				if (!usage || !val_names[val_enum].usage)
@@ -976,13 +980,6 @@ 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");
@@ -1061,52 +1058,6 @@ 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;
@@ -1126,66 +1077,17 @@ static char *flags_to_str(int flags)
 	return buf_flags;
 }
 
-void print_command_count(void)
+void print_define_command_count(void)
 {
-	struct command *cmd;
-	int i, j;
-
 	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);
-
-	printf("enum {\n");
-	for (i = 0; i < cmd_count; i++) {
-		cmd = &cmd_array[i];
-
-		if (!cmd->command_line_id) {
-			printf("Missing ID: at %d\n", i);
-			exit(1);
-		}
-
-		for (j = 0; j < i; j++) {
-			if (!strcmp(cmd->command_line_id, cmd_array[j].command_line_id))
-				goto next;
-		}
-
-		printf("\t%s_CMD,\n", cmd->command_line_id);
-	next:
-		;
-	}
-	printf("\tCOMMAND_ID_COUNT,\n");
-	printf("};\n");
 }
 
-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)
+void print_usage(struct command *cmd)
 {
 	int onereq = (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) ? 1 : 0;
-	int i, sep, ro, rp, oo, op;
-
-	if (skip_required)
-		goto oo_count;
+	int i, ro, rp, oo, op;
 
 	printf("\"%s", cmd->name);
 
@@ -1217,37 +1119,25 @@ void print_usage(struct command *cmd, int skip_required)
 
 	printf("\"");
 
- oo_count:
 	if (!cmd->oo_count)
 		goto op_count;
 
-	sep = 0;
+	printf("\n");
+	printf("\" [");
 
 	if (cmd->oo_count) {
 		for (oo = 0; oo < cmd->oo_count; 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)
+			if (oo)
 				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;
 		}
 	}
 
-	if (sep)
-		printf(" ]\"");
+	printf(" ]\"");
 
  op_count:
 	if (!cmd->op_count)
@@ -1276,25 +1166,19 @@ 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");
-	printf("\n");
 
 	for (i = 0; i < cmd_count; i++) {
 		cmd = &cmd_array[i];
 
 		if (only_usage) {
-			print_usage(cmd, 0);
-			print_usage(&common_options, 1);
+			print_usage(cmd);
 			printf("\n");
 			continue;
 		}
 
 		printf("commands[%d].name = \"%s\";\n", i, cmd->name);
-		printf("commands[%d].command_line_id = \"%s\";\n", i, cmd->command_line_id);
-		printf("commands[%d].command_line_enum = %s_CMD;\n", i, cmd->command_line_id);
 		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);
@@ -1306,14 +1190,7 @@ void print_command_struct(int only_usage)
 
 		printf("commands[%d].desc = \"%s\";\n", i, cmd->desc ?: "");
 		printf("commands[%d].usage = ", i);
-		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);
-		}
+		print_usage(cmd);
 
 		if (cmd->ro_count) {
 			for (ro = 0; ro < cmd->ro_count; ro++) {
@@ -1435,67 +1312,6 @@ 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;
@@ -1524,11 +1340,10 @@ 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("ambiguous: print commands differing only by LV types\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[])
@@ -1602,8 +1417,6 @@ 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';
@@ -1631,7 +1444,7 @@ int main(int argc, char *argv[])
 		}
 
 		if (is_desc_line(line_argv[0])) {
-			char *desc = strdup(line_orig);
+			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, "  ");
@@ -1642,11 +1455,6 @@ int main(int argc, char *argv[])
 			continue;
 		}
 
-		if (is_id_line(line_argv[0])) {
-			cmd->command_line_id = strdup(line_argv[1]);
-			continue;
-		}
-
 		/* OO_FOO: ... */
 		if (is_oo_definition(line_argv[0])) {
 			add_oo_definition_line(line_argv[0], line_orig);
@@ -1699,13 +1507,11 @@ int main(int argc, char *argv[])
 	else if (!strcmp(outputformat, "struct"))
 		print_command_struct(0);
 	else if (!strcmp(outputformat, "count"))
-		print_command_count();
+		print_define_command_count();
 	else if (!strcmp(outputformat, "usage"))
 		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,132 +1,3 @@
 /* Do not edit. This file is generated by scripts/create-commands */
 /* using command definitions from scripts/command-lines.in */
-#define COMMAND_COUNT 146
-enum {
-	lvchange_properties_CMD,
-	lvchange_resync_CMD,
-	lvchange_syncaction_CMD,
-	lvchange_rebuild_CMD,
-	lvchange_activate_CMD,
-	lvchange_refresh_CMD,
-	lvchange_monitor_CMD,
-	lvchange_poll_CMD,
-	lvchange_persistent_CMD,
-	lvconvert_merge_CMD,
-	lvconvert_combine_split_snapshot_CMD,
-	lvconvert_to_thin_with_external_CMD,
-	lvconvert_to_cache_vol_CMD,
-	lvconvert_to_thinpool_CMD,
-	lvconvert_to_cachepool_CMD,
-	lvconvert_to_mirror_CMD,
-	lvconvert_to_mirror_or_raid1_CMD,
-	lvconvert_raid1_to_mirror_CMD,
-	lvconvert_mirror_to_raid1_CMD,
-	lvconvert_general_to_raid_CMD,
-	lvconvert_change_mirror_images_CMD,
-	lvconvert_raid_to_striped_CMD,
-	lvconvert_raid_or_mirror_to_linear_CMD,
-	lvconvert_split_mirror_images_to_new_CMD,
-	lvconvert_split_mirror_images_and_track_CMD,
-	lvconvert_repair_pvs_or_thinpool_CMD,
-	lvconvert_replace_pv_CMD,
-	lvconvert_change_mirrorlog_CMD,
-	lvconvert_split_and_keep_cachepool_CMD,
-	lvconvert_split_and_delete_cachepool_CMD,
-	lvconvert_split_cow_snapshot_CMD,
-	lvconvert_poll_mirror_CMD,
-	lvconvert_swap_pool_metadata_CMD,
-	lvcreate_error_vol_CMD,
-	lvcreate_zero_vol_CMD,
-	lvcreate_linear_CMD,
-	lvcreate_striped_CMD,
-	lvcreate_mirror_CMD,
-	lvcreate_raid_any_CMD,
-	lvcreate_cow_snapshot_CMD,
-	lvcreate_cow_snapshot_with_virtual_origin_CMD,
-	lvcreate_thinpool_CMD,
-	lvcreate_cachepool_CMD,
-	lvcreate_thin_vol_CMD,
-	lvcreate_thin_snapshot_CMD,
-	lvcreate_thin_snapshot_of_external_CMD,
-	lvcreate_thin_vol_with_thinpool_CMD,
-	lvcreate_thin_vol_with_thinpool_or_sparse_snapshot_CMD,
-	lvcreate_convert_to_cache_vol_with_cachepool_CMD,
-	lvcreate_cache_vol_with_new_origin_CMD,
-	lvdisplay_general_CMD,
-	lvextend_by_size_CMD,
-	lvextend_by_pv_CMD,
-	lvextend_pool_metadata_by_size_CMD,
-	lvextend_by_policy_CMD,
-	lvmconfig_general_CMD,
-	lvreduce_general_CMD,
-	lvremove_general_CMD,
-	lvrename_vg_lv_lv_CMD,
-	lvrename_lv_lv_CMD,
-	lvresize_by_size_CMD,
-	lvresize_by_pv_CMD,
-	lvresize_pool_metadata_by_size_CMD,
-	lvs_general_CMD,
-	lvscan_general_CMD,
-	pvchange_properties_all_CMD,
-	pvchange_properties_some_CMD,
-	pvresize_general_CMD,
-	pvck_general_CMD,
-	pvcreate_general_CMD,
-	pvdisplay_general_CMD,
-	pvmove_one_CMD,
-	pvmove_any_CMD,
-	pvremove_general_CMD,
-	pvs_general_CMD,
-	pvscan_show_CMD,
-	pvscan_cache_CMD,
-	vgcfgbackup_general_CMD,
-	vgcfgrestore_by_vg_CMD,
-	vgcfgrestore_by_file_CMD,
-	vgchange_properties_CMD,
-	vgchange_monitor_CMD,
-	vgchange_poll_CMD,
-	vgchange_activate_CMD,
-	vgchange_refresh_CMD,
-	vgchange_lockstart_CMD,
-	vgchange_lockstop_CMD,
-	vgck_general_CMD,
-	vgconvert_general_CMD,
-	vgcreate_general_CMD,
-	vgdisplay_general_CMD,
-	vgexport_some_CMD,
-	vgexport_all_CMD,
-	vgextend_general_CMD,
-	vgimport_some_CMD,
-	vgimport_all_CMD,
-	vgimportclone_general_CMD,
-	vgmerge_general_CMD,
-	vgmknodes_general_CMD,
-	vgreduce_by_pv_CMD,
-	vgreduce_all_CMD,
-	vgreduce_missing_CMD,
-	vgremove_general_CMD,
-	vgrename_by_name_CMD,
-	vgrename_by_uuid_CMD,
-	vgs_general_CMD,
-	vgscan_general_CMD,
-	vgsplit_by_pv_to_existing_CMD,
-	vgsplit_by_lv_to_existing_CMD,
-	vgsplit_by_pv_to_new_CMD,
-	vgsplit_by_lv_to_new_CMD,
-	devtypes_general_CMD,
-	fullreport_general_CMD,
-	lastlog_general_CMD,
-	lvpoll_general_CMD,
-	formats_general_CMD,
-	help_general_CMD,
-	version_general_CMD,
-	pvdata_general_CMD,
-	segtypes_general_CMD,
-	systemid_general_CMD,
-	tags_general_CMD,
-	lvmchange_general_CMD,
-	lvmdiskscan_general_CMD,
-	lvmsadc_general_CMD,
-	lvmsar_general_CMD,
-	COMMAND_ID_COUNT,
-};
+#define COMMAND_COUNT 144

tools/command.h

@@ -18,26 +18,8 @@
 
 struct cmd_context;
 
-/* old per-command-name function */
-typedef int (*command_fn) (struct cmd_context *cmd, int argc, char **argv);
-
-/* new per-command-line-id functions */
-typedef int (*command_line_fn) (struct cmd_context *cmd, int argc, char **argv);
-
-struct command_function {
-	int command_line_enum;
-	command_line_fn fn;
-};
-
-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;
-};
+/* command functions */
+typedef int (*command_fn) (struct cmd_context * cmd, int argc, char **argv);
 
 /*
  * Command defintion
@@ -129,15 +111,10 @@ struct pos_arg {
 struct command {
 	const char *name;
 	const char *desc; /* specific command description from command-lines.h */
-	const char *usage; /* excludes common options like --help, --debug */
-	const char *usage_common; /* includes commmon options like --help, --debug */
-	const char *command_line_id;
-	int command_line_enum; /* <command_line_id>_CMD */
+	const char *usage;
 
 	struct command_name *cname;
-
-	command_fn fn;                      /* old style */
-	struct command_function *functions; /* new style */
+	command_fn fn;
 
 	unsigned int flags; /* copied from command_name.flags from commands.h */
 
@@ -166,4 +143,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/lvmcmdline.c

@@ -49,7 +49,7 @@ extern char *optarg;
 #  define OPTIND_INIT 1
 #endif
 
-#include "command-lines-count.h"
+#include "command-lines-count.h" /* #define COMMAND_COUNT, generated from command-lines.in */
 
 /*
  * Table of valid --option values.
@@ -85,22 +85,6 @@ struct command_name command_names[MAX_COMMAND_NAMES] = {
 static struct command commands[COMMAND_COUNT];
 static struct cmdline_context _cmdline;
 
-/*
- * Table of command line functions
- *
- * This table could be auto-generated once all commands have been converted
- * to use these functions instead of the old per-command-name function.
- * For now, any command id not included here uses the old command fn.
- */
-struct command_function command_functions[COMMAND_ID_COUNT] = {
-	{ lvmconfig_general_CMD, lvmconfig },
-};
-#if 0
-	{ lvchange_properties_CMD, lvchange_properties_cmd },
-	{ lvchange_activate_CMD, lvchange_activate_cmd },
-	{ lvchange_refresh_CMD, lvchange_refresh_cmd },
-#endif
-
 /* Command line args */
 unsigned arg_count(const struct cmd_context *cmd, int a)
 {
@@ -803,17 +787,6 @@ static struct command_name *_find_command_name(const char *name)
 	return NULL;
 }
 
-static struct command_function *_find_command_function(int command_line_enum)
-{
-	int i;
-
-	for (i = 0; i < COMMAND_ID_COUNT; i++) {
-		if (command_functions[i].command_line_enum == command_line_enum)
-			return &command_functions[i];
-	}
-	return NULL;
-}
-
 static void _define_commands(void)
 {
 /* command-lines.h defines command[] structs, generated from command-lines.in */
@@ -837,7 +810,6 @@ void lvm_register_commands(void)
 			log_error(INTERNAL_ERROR "Failed to find command name %s.", commands[i].name);
 		commands[i].cname = cname;
 		commands[i].flags = cname->flags;
-		commands[i].functions = _find_command_function(commands[i].command_line_enum);
 	}
 
 	_cmdline.command_names = command_names;
@@ -991,17 +963,15 @@ static int _command_required_pos_matches(struct cmd_context *cmd, int ci, int rp
 
 #define HELP_LINE_SIZE 1024
 
-static void _print_usage(const char *usage, int only_required)
+static void _print_usage(int ci, int include_optional)
 {
+	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
 	 *
@@ -1017,8 +987,6 @@ static void _print_usage(const char *usage, int only_required)
 	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] == ' ')) {
@@ -1028,39 +996,43 @@ static void _print_usage(const char *usage, int only_required)
 
 		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
 	 */
-
-	if (bi)
-		log_print("%s", buf);
-
-	if (only_required)
-		return;
+	log_print("%s", buf);
 
 	/*
 	 * copy the optional opt_args
 	 */
 
 	if (!optional_ui)
-		goto out;
+		return;
 
 	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. */
@@ -1074,15 +1046,11 @@ static void _print_usage(const char *usage, int only_required)
 		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))
@@ -1092,29 +1060,26 @@ static void _print_usage(const char *usage, int only_required)
 	/*
 	 * print the optional opt_args
 	 */
-
-	if (bi)
-		log_print("%s", buf);
+	log_print("%s", buf);
 
 	/*
 	 * copy the optional pos_args
 	 */
 
 	if (!optional_pos_ui)
-		goto out;
+		return;
 
 	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))
@@ -1124,13 +1089,14 @@ static void _print_usage(const char *usage, int only_required)
 	/*
 	 * print the optional pos_args
 	 */
-
-	if (bi)
-		log_print("%s", buf);
- out:
-	return;
+	log_print("%s", 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;
@@ -1144,27 +1110,20 @@ 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)
@@ -1172,7 +1131,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_ro = 0;
+	int closest_i = 0, closest_count = 0;
 	int ro, rp;
 	int i, j;
 	int accepted, count;
@@ -1242,10 +1201,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 "closest" command that doesn't match */
-			if (match_count_ro && (match_count_ro > closest_count_ro)) {
+			/* save i/match_count for "closest" command that doesn't match */
+			if (!closest_count || (match_count > closest_count)) {
 				closest_i = i;
-				closest_count_ro = match_count_ro;
+				closest_count = match_count;
 			}
 			continue;
 		}
@@ -1277,10 +1236,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.");
-		if (closest_count_ro) {
+		log_error("Failed to find a matching command definition.\n");
+		if (closest_count) {
 			log_warn("Closest command usage is:");
-			_print_usage(_cmdline.commands[closest_i].usage, 1);
+			_print_usage(closest_i, 0);
 		}
 		return NULL;
 	}
@@ -1323,17 +1282,6 @@ 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));
@@ -1373,7 +1321,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 		}
 	}
 out:
-	log_debug("command line id: %s (%d)", commands[best_i].command_line_id, best_i);
+	log_debug("Command definition (%d): %s", best_i, commands[best_i].usage);
 
 	return &commands[best_i];
 }
@@ -1386,7 +1334,6 @@ 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) {
@@ -1403,16 +1350,7 @@ static int _usage(const char *name, int help_count)
 		if (strlen(_cmdline.commands[i].desc))
 			_print_description(i);
 
-		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);
+		_print_usage(i, help_count > 1);
 		log_print(" "); /* for built-in \n */
 	}
 
@@ -1424,11 +1362,9 @@ 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(". --name is usually specified for lvcreate, but is not among");
-		log_print("  the required parameters because names can be generated.");
+		log_print(". Select indicates that the positional arg can be omitted");
+		log_print("  if the --select option is used.");
 		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.");
@@ -1880,7 +1816,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 for usage notes.)");
+			log_print("(Use --help --help to include optional parameters.)");
 		return ECMD_PROCESSED;
 	}
 
@@ -2260,7 +2196,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_ECMD_FAILED;
+		return ENO_SUCH_CMD;
 
 	set_cmd_name(cmd_name);
 
@@ -2418,12 +2354,10 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		}
 	}
 
-	if (cmd->command->functions)
-		/* A command-line--specific function is used */
-		ret = cmd->command->functions->fn(cmd, argc, argv);
-	else
-		/* The old style command-name function is used */
-		ret = cmd->command->fn(cmd, argc, argv);
+	/*
+	 * FIXME Break up into multiple functions.
+	 */
+	ret = cmd->command->fn(cmd, argc, argv);
 
 	lvmlockd_disconnect();
 	fin_locking();

tools/toollib.c

@@ -2346,12 +2346,8 @@ 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);
 
@@ -2360,7 +2356,6 @@ 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;
@@ -2454,7 +2449,6 @@ 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;
 		}
 
@@ -2502,64 +2496,6 @@ 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);