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/command-lines.in

@@ -236,6 +236,10 @@ DESC: Merge LV that was previously split from a mirror.
 DESC: Merge thin LV into its origin LV.
 DESC: Merge COW snapshot LV into its origin.
 
+# FIXME: by using two different positional args, this is the
+# single violation of the standard method of using process_each_lv().
+# Before calling process_each, it steals the first positional arg
+# and adjusts argv/argc so it's not seen by process_each.
 lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
 OO: --chunksize SizeKB, --zero Bool, OO_LVCONVERT
 DESC: Combine LV with a previously split snapshot LV.

tools/lvmcmdline.c

@@ -963,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)
+static void _print_usage(const char *usage, int only_required)
 {
 	char buf[HELP_LINE_SIZE];
 	int optional_ui = 0;
@@ -1020,6 +1020,9 @@ static void _print_usage(const char *usage)
 	if (bi)
 		log_print("%s", buf);
 
+	if (only_required)
+		return;
+
 	/*
 	 * copy the optional opt_args
 	 */
@@ -1141,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;
@@ -1211,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;
 		}
@@ -1246,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(_cmdline.commands[closest_i].usage);
+			_print_usage(_cmdline.commands[closest_i].usage, 1);
 		}
 		return NULL;
 	}
@@ -1292,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));
@@ -1363,14 +1377,14 @@ static int _usage(const char *name, int help_count)
 
 		usage_common = _cmdline.commands[i].usage_common;
 
-		_print_usage(_cmdline.commands[i].usage);
+		_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);
+		_print_usage(usage_common, 0);
 		log_print(" "); /* for built-in \n */
 	}
 
@@ -2216,7 +2230,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);