commands: new method for defining commands

scripts/create-commands.c

@@ -548,13 +548,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
@@ -1061,52 +1054,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;
@@ -1256,31 +1203,6 @@ void print_command_struct(int only_usage)
 
 	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");
-
-
-	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_LINE_ID_COUNT,\n");
-	printf("}\n");
-	printf("\n");
-
 
 	for (i = 0; i < cmd_count; i++) {
 		cmd = &cmd_array[i];
@@ -1293,8 +1215,6 @@ void print_command_struct(int only_usage)
 		}
 
 		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);
@@ -1435,67 +1355,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 +1383,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 +1460,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';
@@ -1642,11 +1498,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);
@@ -1704,8 +1555,6 @@ 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 146
+#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
@@ -131,13 +113,9 @@ struct command {
 	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 */
 
 	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 +144,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

@@ -85,20 +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_LINE_ID_COUNT] = {
-	{ lvmconfig_general_CMD, lvmconfig },
-	{ lvchange_properties_CMD, lvchange_properties_cmd },
-	{ lvchange_activate_CMD, lvchange_activate_cmd },
-	{ lvchange_refresh_CMD, lvchange_refresh_cmd },
-};
-
 /* Command line args */
 unsigned arg_count(const struct cmd_context *cmd, int a)
 {
@@ -801,17 +787,6 @@ static struct command_name *_find_command_name(const char *name)
 	return NULL;
 }
 
-statict struct command_function *_find_command_function(int command_line_enum)
-{
-	int i;
-
-	for (i = 0; i < COMMAND_LINE_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 */
@@ -835,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;
@@ -989,7 +963,7 @@ 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(const char *usage)
 {
 	char buf[HELP_LINE_SIZE];
 	int optional_ui = 0;
@@ -1046,9 +1020,6 @@ static void _print_usage(const char *usage, int only_required)
 	if (bi)
 		log_print("%s", buf);
 
-	if (only_required)
-		return;
-
 	/*
 	 * copy the optional opt_args
 	 */
@@ -1170,7 +1141,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;
@@ -1240,10 +1211,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;
 		}
@@ -1275,10 +1246,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(_cmdline.commands[closest_i].usage);
 		}
 		return NULL;
 	}
@@ -1321,17 +1292,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));
@@ -1371,7 +1331,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];
 }
@@ -1403,14 +1363,14 @@ static int _usage(const char *name, int help_count)
 
 		usage_common = _cmdline.commands[i].usage_common;
 
-		_print_usage(_cmdline.commands[i].usage, 0);
+		_print_usage(_cmdline.commands[i].usage);
 		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(usage_common);
 		log_print(" "); /* for built-in \n */
 	}
 
@@ -1425,8 +1385,6 @@ static int _usage(const char *name, int help_count)
 		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(". 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.");
@@ -2258,7 +2216,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);
 
@@ -2416,12 +2374,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);