lvconvert: use command defs for repair and replace

Shift --repair and --replace to the new system based on
command defs.

tools/args.h

@@ -17,8 +17,6 @@
  * Put all long args that don't have a corresponding short option first.
  */
 /* *INDENT-OFF* */
-arg(ARG_UNUSED, '-', "", 0, 0, 0)  /* place holder for unused 0 value */
-
 arg(abort_ARG, '\0', "abort", 0, 0, 0)
 arg(activationmode_ARG, '\0', "activationmode", activationmode_VAL, 0, 0)
 arg(addtag_ARG, '\0', "addtag", tag_VAL, ARG_GROUPABLE, 0)

tools/command-lines.in

@@ -115,16 +115,6 @@
 # included in the text as indicators of new lines when printing
 # the descriptions for help/man output.
 #
-# RULE: rules that a given command must follow, i.e. required (and)
-# or invalid (not) combinations of options, LV types or LV properties.
-#
-# RULE: --opt|LV_type|lv_is_prop|all and|not --opt|LV_type|lv_is_prop
-# RULE: --opt1 not --opt2
-# RULE: --opt1 and --opt2
-# RULE: --opt1 LV_type1 lv_is_prop1 and --opt2
-# RULE: --opt1 LV_type1 and lv_is_prop1
-# RULE: LV_type1 and lv_is_prop1
-#
 # Note that one the most difficult aspect of these definitions is
 # the variants of --thin / --type thin / --type thin-pool,
 # --cache / --type cache / --type cache-pool.
@@ -133,6 +123,42 @@
 # to be tested individually to see what it means.
 #
 
+#
+# Another capability we might want to add here is a way to express
+# rules, per definition, of what arg combinations are allowed or
+# required, e.g.
+#
+# if --foo is set, then --bar cannot be set could be encoded as:
+# RULE_OPT_INVALID_OPT: --foo --bar
+#
+# if --foo is set, then --bar is required could be encoded as:
+# RULE_OPT_REQUIRES_OPT: --foo --bar
+#
+# if --foo is set, then positional arg 1 is required
+# (the type of value in that arg is specified by the command def):
+# RULE_OPT_REQUIRES_POS: --foo 1
+#
+#
+# The rules could also specify validation for positional LV args:
+#
+# if --foo is set, then specified lv checks must pass:
+# RULE_OPT_REQUIRES_LV_CHECK: --foo lv_is_merging_origin
+#
+# if --foo is set, then specified lv checks must not pass:
+# RULE_OPT_INVALID_LV_CHECK: --foo lv_is_merging_origin
+#
+# command def requires specified lv checks must pass:
+# RULE_OPT_REQUIRES_LV_CHECK: * lv_is_merging_origin
+#
+# command def requires specified lv checks must not pass:
+# RULE_OPT_INVALID_LV_CHECK: * lv_is_merging_origin
+#
+# 
+# To implement would require a rule structure, and an array of
+# rule structures would be added to struct command, to be filled
+# in like the args arrays are.
+#
+
 #
 # For efficiency, sets of options can be defined and reused
 # in multiple command definitions.
@@ -216,11 +242,6 @@ lvchange OO_LVCHANGE_META VG|LV|Tag|Select ...
 OO: OO_LVCHANGE
 ID: lvchange_properties
 DESC: Change a general LV property.
-RULE: all not lv_is_pvmove
-RULE: --contiguous not --alloc
-RULE: --minor and --persistent
-RULE: --profile not --detachprofile
-RULE: --metadataprofile not --detachprofile
 
 lvchange --resync VG|LV_raid_mirror|Tag|Select ...
 OO: OO_LVCHANGE
@@ -282,7 +303,6 @@ OO: OO_LVCONVERT_RAID, OO_LVCONVERT
 OP: PV ...
 ID: lvconvert_raid_types
 DESC: Convert LV to linear.
-RULE: all not lv_is_locked lv_is_pvmove
 
 lvconvert --type striped LV
 OO: OO_LVCONVERT_RAID, OO_LVCONVERT
@@ -461,7 +481,6 @@ ID: lvconvert_merge
 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.
-RULE: all not lv_is_merging_origin lv_is_virtual_origin lv_is_external_origin lv_is_merging_cow
 
 ---
 

tools/command.h

@@ -54,6 +54,27 @@ struct command_name {
 #define ARG_DEF_FLAG_NEW                1 << 0
 #define ARG_DEF_FLAG_MAY_REPEAT         1 << 1
 
+/* arg_def lv_types */
+enum {
+	ARG_DEF_LV_ANY		= 0,
+	ARG_DEF_LV_LINEAR	= 1 << 0,
+	ARG_DEF_LV_STRIPED	= 1 << 1,
+	ARG_DEF_LV_SNAPSHOT	= 1 << 2,
+	ARG_DEF_LV_MIRROR	= 1 << 3,
+	ARG_DEF_LV_RAID		= 1 << 4,
+	ARG_DEF_LV_RAID0	= 1 << 5,
+	ARG_DEF_LV_RAID1	= 1 << 6,
+	ARG_DEF_LV_RAID4	= 1 << 7,
+	ARG_DEF_LV_RAID5	= 1 << 8,
+	ARG_DEF_LV_RAID6	= 1 << 9,
+	ARG_DEF_LV_RAID10	= 1 << 10,
+	ARG_DEF_LV_THIN		= 1 << 11,
+	ARG_DEF_LV_THINPOOL	= 1 << 12,
+	ARG_DEF_LV_CACHE	= 1 << 13,
+	ARG_DEF_LV_CACHEPOOL	= 1 << 14,
+	ARG_DEF_LV_LAST		= 1 << 15,
+};
+
 static inline int val_bit_is_set(uint64_t val_bits, int val_enum)
 {
 	return (val_bits & (1 << val_enum)) ? 1 : 0;
@@ -64,33 +85,13 @@ static inline uint64_t val_enum_to_bit(int val_enum)
 	return (1ULL << val_enum);
 }
 
-static inline int lvp_bit_is_set(uint64_t lvp_bits, int lvp_enum)
-{
-	return (lvp_bits & (1 << lvp_enum)) ? 1 : 0;
-}
-
-static inline uint64_t lvp_enum_to_bit(int lvp_enum)
-{
-	return (1ULL << lvp_enum);
-}
-
-static inline int lvt_bit_is_set(uint64_t lvt_bits, int lvt_enum)
-{
-	return (lvt_bits & (1 << lvt_enum)) ? 1 : 0;
-}
-
-static inline uint64_t lvt_enum_to_bit(int lvt_enum)
-{
-	return (1ULL << lvt_enum);
-}
-
 /* Description a value that follows an option or exists in a position. */
 
 struct arg_def {
 	uint64_t val_bits;   /* bits of x_VAL, can be multiple for pos_arg */
-	uint64_t lvt_bits;   /* lvt_enum_to_bit(x_LVT) for lv_VAL, can be multiple */
 	uint64_t num;        /* a literal number for conststr_VAL */
 	const char *str;     /* a literal string for constnum_VAL */
+	uint32_t lv_types;   /* ARG_DEF_LV_, for lv_VAL, can be multiple */
 	uint32_t flags;      /* ARG_DEF_FLAG_ */
 };
 
@@ -108,37 +109,6 @@ struct pos_arg {
 	struct arg_def def;  /* defines accepted values */
 };
 
-/*
- * When all values before the require|invalid match a given command,
- * then all the values after are verified to be true|false.
- *
- * Rules that include only options can be verified before the VG
- * is read.  Otherwise, the rules are verified in process_each
- * after the VG is read.
- *
- * RULE: --opt|LV_type|lv_is_prop|all require|invalid --opt|LV_type|lv_is_prop
- * RULE: --opt1 invalid --opt2
- * RULE: --opt1 require --opt2
- * RULE: --opt1 LV_type1 lv_is_prop1 require --opt2
- * RULE: --opt1 LV_type1 require lv_is_prop1
- * RULE: LV_type1 require lv_is_prop1
- */
-
-#define RULE_INVALID 1
-#define RULE_REQUIRE 2
-
-struct cmd_rule {
-	int opt;           /* apply rule when this option is set (foo_ARG) */
-	uint64_t lvt_bits; /* apply rule when LV has one of these types (lvt_enum_to_bit) */
-	uint64_t lvp_bits; /* apply rule when LV has all these properties (lvp_enum_to_bit) */
-
-	uint32_t rule;     /* RULE_INVALID, RULE_REQUIRE: check values must [not] be true */
-
-	int check_opt;            /* this option must [not] be set */
-	uint64_t check_lvt_bits;  /* LV must [not] have this type */
-	uint64_t check_lvp_bits;  /* LV must [not] have these properties */
-};
-
 /*
  * CMD_RO_ARGS needs to accomodate a list of options,
  * of which one is required after which the rest are
@@ -148,7 +118,6 @@ struct cmd_rule {
 #define CMD_OO_ARGS 150         /* optional opt args */
 #define CMD_RP_ARGS 8           /* required positional args */
 #define CMD_OP_ARGS 8           /* optional positional args */
-#define CMD_MAX_RULES 16        /* max number of rules per command def */
 
 /*
  * one or more from required_opt_args is required,
@@ -189,8 +158,6 @@ struct command {
 	/* optional positional args */
 	struct pos_arg optional_pos_args[CMD_OP_ARGS];
 
-	struct cmd_rule rules[CMD_MAX_RULES];
-
 	int ro_count;
 	int oo_count;
 	int rp_count;
@@ -199,7 +166,42 @@ struct command {
 	/* used for processing current position */
 	int pos_count;
 
-	int rule_count;
+	/* struct cmd_rule rules[CMD_RULES]; */
 };
 
+#if 0
+
+/*
+ * if rule.opt is set, then rule.type specifies if rule.check values
+ * are required or invalid.
+ *
+ * if (arg_is_set(rule.opt) &&
+ *     (rule.type & RULE_OPT_INVALID_OPT) && arg_is_set(rule.check.opt)) {
+ *     log_error("option %s and option %s cannot be used together");
+ * }
+ *
+ * if (arg_is_set(rule.opt) &&
+ *     (rule.type & RULE_OPT_REQUIRES_LV_CHECK) && !lv_check(lv, rule.check.bits, &fail_bits)) {
+ *     log_error("LV %s must be %s", lv, lv_check_to_str(fail_bits));
+ * }
+ *
+ * if (arg_is_set(rule.opt) &&
+ *     (rule.type & RULE_OPT_INVALID_LV_CHECK) && lv_check(lv, rule.check.bits, &fail_bits)) {
+ *     log_error("LV %s must not be %s", lv, lv_check_to_str(fail_bits));
+ * }
+ *
+ */
+
+struct cmd_rule {
+	int opt;  /* foo_ARG, or INT_MAX for command def in general */
+	int type; /* RULE_ specifies how to require/prohibit check value */
+
+	union {
+		int opt;
+		int pos;
+		uint64_t bits;
+	} check;
+};
+#endif
+
 #endif

tools/create-commands.c

@@ -57,42 +57,28 @@ int reportformat_arg(struct cmd_context *cmd __attribute__((unused)), struct arg
 int configreport_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
 int configtype_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
 
-/* also see arg_props in tools.h and args.h */
+/* also see arg_props */
 struct opt_name {
-	const char *name;       /* "foo_ARG" */
-	int opt_enum;           /* foo_ARG */
-	const char short_opt;   /* -f */
+	const char *name;
+	int opt_enum;         /* enum from args.h */
+	const char short_opt;
 	char _padding[7];
-	const char *long_opt;   /* --foo */
-	int val_enum;           /* xyz_VAL when --foo takes a val like "--foo xyz" */
+	const char *long_opt;
+	int val_enum;         /* enum from vals.h */
 	uint32_t unused1;
 	uint32_t unused2;
 };
 
-/* also see val_props in tools.h and vals.h */
+/* also see val_props */
 struct val_name {
-	const char *enum_name;  /* "foo_VAL" */
-	int val_enum;           /* foo_VAL */
-	int (*fn) (struct cmd_context *cmd, struct arg_values *av); /* foo_arg() */
-	const char *name;       /* FooVal */
+	const char *enum_name;
+	int val_enum;         /* enum from vals.h */
+	int (*fn) (struct cmd_context *cmd, struct arg_values *av); /* unused here */
+	const char *name;
 	const char *usage;
 };
 
-/* also see lv_props in tools.h and lv_props.h */
-struct lvp_name {
-	const char *enum_name; /* "is_foo_LVP" */
-	int lvp_enum;          /* is_foo_LVP */
-	const char *name;      /* "lv_is_foo" */
-};
-
-/* also see lv_types in tools.h and lv_types.h */
-struct lvt_name {
-	const char *enum_name; /* "foo_LVT" */
-	int lvt_enum;          /* foo_LVT */
-	const char *name;      /* "foo" */
-};
-
-/* create foo_VAL enums for option and position values */
+/* create foo_VAL enums */
 
 enum {
 #define val(a, b, c, d) a ,
@@ -100,7 +86,7 @@ enum {
 #undef val
 };
 
-/* create foo_ARG enums for --option's */
+/* create foo_ARG enums */
 
 enum {
 #define arg(a, b, c, d, e, f) a ,
@@ -108,22 +94,6 @@ enum {
 #undef arg
 };
 
-/* create foo_LVP enums for LV properties */
-
-enum {
-#define lvp(a, b, c) a,
-#include "lv_props.h"
-#undef lvp
-};
-
-/* create foo_LVT enums for LV types */
-
-enum {
-#define lvt(a, b, c) a,
-#include "lv_types.h"
-#undef lvt
-};
-
 /* create table of value names, e.g. String, and corresponding enum from vals.h */
 
 static struct val_name val_names[VAL_COUNT + 1] = {
@@ -140,22 +110,6 @@ static struct opt_name opt_names[ARG_COUNT + 1] = {
 #undef arg
 };
 
-/* create table of lv property names, e.g. lv_is_foo, and corresponding enum from lv_props.h */
-
-static struct lvp_name lvp_names[LVP_COUNT + 1] = {
-#define lvp(a, b, c) { # a, a, b },
-#include "lv_props.h"
-#undef lvp
-};
-
-/* create table of lv type names, e.g. linear and corresponding enum from lv_types.h */
-
-static struct lvt_name lvt_names[LVT_COUNT + 1] = {
-#define lvt(a, b, c) { # a, a, b },
-#include "lv_types.h"
-#undef lvt
-};
-
 #include "command.h"
 
 #define MAX_CMD_NAMES 128
@@ -309,7 +263,7 @@ static int opt_str_to_num(char *str)
 
 static char *val_bits_to_str(uint64_t val_bits)
 {
-	static char buf[1024];
+	static char buf[128];
 	int i;
 	int or = 0;
 
@@ -329,130 +283,212 @@ static char *val_bits_to_str(uint64_t val_bits)
 }
 
 /*
- * When bits for foo_LVP and bar_LVP are both set in bits, print:
- * lvp_enum_to_bit(foo_LVP) | lvp_enum_to_bit(bar_LVP)
+ * The _<lvtype> and _new suffixes are only used by the command definitions and
+ * are not exposed to lvm at large, which uses only the ARG_DEF values.
  */
 
-static char *lvp_bits_to_str(uint64_t bits)
+static uint32_t lv_str_to_types(char *str)
 {
-	static char lvp_buf[1024];
-	int i;
-	int or = 0;
-
-	memset(lvp_buf, 0, sizeof(lvp_buf));
-
-	for (i = 0; i < LVP_COUNT; i++) {
-		if (bits & lvp_enum_to_bit(i)) {
-			if (or) strcat(lvp_buf, " | ");
-			strcat(lvp_buf, "lvp_enum_to_bit(");
-			strcat(lvp_buf, lvp_names[i].enum_name);
-			strcat(lvp_buf, ")");
-			or = 1;
-		}
-	}
-
-	return lvp_buf;
-}
-
-/*
- * When bits for foo_LVT and bar_LVT are both set in bits, print:
- * lvt_enum_to_bit(foo_LVT) | lvt_enum_to_bit(bar_LVT)
- */
-
-static char *lvt_bits_to_str(uint64_t bits)
-{
-	static char lvt_buf[1024];
-	int i;
-	int or = 0;
-
-	memset(lvt_buf, 0, sizeof(lvt_buf));
-
-	for (i = 1; i < LVT_COUNT; i++) {
-		if (bits & lvt_enum_to_bit(i)) {
-			if (or) strcat(lvt_buf, " | ");
-			strcat(lvt_buf, "lvt_enum_to_bit(");
-			strcat(lvt_buf, lvt_names[i].enum_name);
-			strcat(lvt_buf, ")");
-			or = 1;
-		}
-	}
-
-	return lvt_buf;
-}
-
-/* "lv_is_prop" to is_prop_LVP */
-
-static int lvp_name_to_enum(char *str)
-{
-	int i;
-
-	for (i = 1; i < LVP_COUNT; i++) {
-		if (!strcmp(str, lvp_names[i].name))
-			return lvp_names[i].lvp_enum;
-	}
-	printf("unknown lv property %s\n", str);
-	exit(1);
-}
-
-/* type_LVT to "type" */
-
-static const char *lvt_enum_to_name(int lvt_enum)
-{
-	return lvt_names[lvt_enum].name;
-}
-
-/* "type" to type_LVT */
-
-static int lvt_name_to_enum(char *str)
-{
-	int i;
-
-	for (i = 1; i < LVT_COUNT; i++) {
-		if (!strcmp(str, lvt_names[i].name))
-			return lvt_names[i].lvt_enum;
-	}
-	printf("unknown lv type %s\n", str);
-	exit(1);
-}
-
-/* LV_<type> to <type>_LVT */
-
-int lv_to_enum(char *name)
-{
-	return lvt_name_to_enum(name + 3);
-}
-
-/*
- * LV_<type1>_<type2> to lvt_bits
- *
- * type1 to lvt_enum
- * lvt_bits |= lvt_enum_to_bit(lvt_enum)
- * type2 to lvt_enum
- * lvt_bits |= lvt_enum_to_bit(lvt_enum)
- */
-
-uint64_t lv_to_bits(char *name)
-{
-	char buf[64];
+	char copy[128] = { 0 };
 	char *argv[MAX_LINE_ARGC];
-	uint64_t lvt_bits = 0;
-	int lvt_enum;
 	int argc;
+	char *name;
+	uint32_t types = 0;
 	int i;
 
-	strcpy(buf, name);
+	strncpy(copy, str, 128);
 
-	split_line(buf, &argc, argv, '_');
+	split_line(copy, &argc, argv, '_');
 
-	/* 0 is "LV" */
-	for (i = 1; i < argc; i++) {
-		if (!strcmp(argv[i], "new"))
-			continue;
-		lvt_enum = lvt_name_to_enum(argv[i]);
-		lvt_bits |= lvt_enum_to_bit(lvt_enum);
+	for (i = 0; i < argc; i++) {
+		name = argv[i];
+
+		if (!strcmp(name, "linear"))
+			types |= ARG_DEF_LV_LINEAR;
+
+		if (!strcmp(name, "striped"))
+			types |= ARG_DEF_LV_STRIPED;
+
+		if (!strcmp(name, "snapshot"))
+			types |= ARG_DEF_LV_SNAPSHOT;
+
+		if (!strcmp(name, "mirror"))
+			types |= ARG_DEF_LV_MIRROR;
+
+		if (!strcmp(name, "thin"))
+			types |= ARG_DEF_LV_THIN;
+
+		if (!strcmp(name, "thinpool"))
+			types |= ARG_DEF_LV_THINPOOL;
+
+		if (!strcmp(name, "cache"))
+			types |= ARG_DEF_LV_CACHE;
+
+		if (!strcmp(name, "cachepool"))
+			types |= ARG_DEF_LV_CACHEPOOL;
+
+		if (!strcmp(name, "raid0"))
+			types |= ARG_DEF_LV_RAID0;
+
+		if (!strcmp(name, "raid1"))
+			types |= ARG_DEF_LV_RAID1;
+
+		if (!strcmp(name, "raid4"))
+			types |= ARG_DEF_LV_RAID4;
+
+		if (!strcmp(name, "raid5"))
+			types |= ARG_DEF_LV_RAID5;
+
+		if (!strcmp(name, "raid6"))
+			types |= ARG_DEF_LV_RAID6;
+
+		if (!strcmp(name, "raid10"))
+			types |= ARG_DEF_LV_RAID10;
+
+		if (!strcmp(name, "raid"))
+			types |= ARG_DEF_LV_RAID;
 	}
 
-	return lvt_bits;
+	return types;
+}
+
+static const char *lv_num_to_str(int num)
+{
+	switch (num) {
+	case ARG_DEF_LV_LINEAR:
+		return "linear";
+	case ARG_DEF_LV_STRIPED:
+		return "striped";
+	case ARG_DEF_LV_SNAPSHOT:
+		return "snapshot";
+	case ARG_DEF_LV_MIRROR:
+		return "mirror";
+	case ARG_DEF_LV_RAID:
+		return "raid";
+	case ARG_DEF_LV_RAID0:
+		return "raid0";
+	case ARG_DEF_LV_RAID1:
+		return "raid1";
+	case ARG_DEF_LV_RAID4:
+		return "raid4";
+	case ARG_DEF_LV_RAID5:
+		return "raid5";
+	case ARG_DEF_LV_RAID6:
+		return "raid6";
+	case ARG_DEF_LV_RAID10:
+		return "raid10";
+	case ARG_DEF_LV_THIN:
+		return "thin";
+	case ARG_DEF_LV_THINPOOL:
+		return "thinpool";
+	case ARG_DEF_LV_CACHE:
+		return "cache";
+	case ARG_DEF_LV_CACHEPOOL:
+		return "cachepool";
+	default:
+		printf("lv_num_to_str: unknown LV num: %d\n", num);
+		exit(1);
+	}
+}
+
+static char *lv_types_to_flags(int lv_types)
+{
+	static char buf_lv_types[128];
+	int or = 0;
+
+	memset(buf_lv_types, 0, sizeof(buf_lv_types));
+
+	if (lv_types & ARG_DEF_LV_LINEAR) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_LINEAR");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_STRIPED) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_STRIPED");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_SNAPSHOT) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_SNAPSHOT");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_MIRROR) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_MIRROR");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID0) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID0");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID1) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID1");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID4) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID4");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID5) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID5");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID6) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID6");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_RAID10) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_RAID10");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_THIN) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_THIN");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_THINPOOL) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_THINPOOL");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_CACHE) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_CACHE");
+		or = 1;
+	}
+
+	if (lv_types & ARG_DEF_LV_CACHEPOOL) {
+		if (or) strcat(buf_lv_types, " | ");
+		strcat(buf_lv_types, "ARG_DEF_LV_CACHEPOOL");
+		or = 1;
+	}
+
+	return buf_lv_types;
 }
 
 static const char *is_command_name(char *str)
@@ -531,7 +567,7 @@ static int is_pos_name(char *str)
 
 static int is_oo_definition(char *str)
 {
-	if (!strncmp(str, "OO_", 3) && strstr(str, ":"))
+	if (!strncmp(str, "OO_", 3))
 		return 1;
 	return 0;
 }
@@ -564,13 +600,6 @@ static int is_flags_line(char *str)
 	return 0;
 }
 
-static int is_rule_line(char *str)
-{
-	if (!strncmp(str, "RULE:", 5))
-		return 1;
-	return 0;
-}
-
 static int is_id_line(char *str)
 {
 	if (!strncmp(str, "ID:", 3))
@@ -606,7 +635,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->lvt_bits = lv_to_bits(name);
+			def->lv_types = lv_str_to_types(name);
 
 		if (strstr(name, "_new"))
 			def->flags |= ARG_DEF_FLAG_NEW;
@@ -658,7 +687,7 @@ static void set_opt_def(struct command *cmd, char *str, struct arg_def *def)
 
 		if (val_enum == lv_VAL) {
 			if (strstr(name, "_"))
-				def->lvt_bits = lv_to_bits(name);
+				def->lv_types = lv_str_to_types(name);
 		}
 
 		if ((val_enum == vg_VAL) || (val_enum == lv_VAL) || (val_enum == pv_VAL)) {
@@ -976,7 +1005,6 @@ static void add_required_line(struct command *cmd, int argc, char *argv[])
 static void print_def(struct arg_def *def, int usage)
 {
 	int val_enum;
-	int lvt_enum;
 	int sep = 0;
 	int i;
 
@@ -1000,10 +1028,10 @@ static void print_def(struct arg_def *def, int usage)
 				sep = 1;
 			}
 
-			if (val_enum == lv_VAL && def->lvt_bits) {
-				for (lvt_enum = 1; lvt_enum < LVT_COUNT; lvt_enum++) {
-					if (lvt_bit_is_set(def->lvt_bits, lvt_enum))
-						printf("_%s", lvt_enum_to_name(lvt_enum));
+			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));
 				}
 			}
 
@@ -1162,79 +1190,6 @@ static void add_flags(struct command *cmd, char *line)
 		cmd->cmd_flags |= CMD_FLAG_SECONDARY_SYNTAX;
 }
 
-static void add_rule(struct command *cmd, char *line)
-{
-	struct cmd_rule *rule;
-	char *line_argv[MAX_LINE_ARGC];
-	char *arg;
-	int line_argc;
-	int i, lvt_enum, lvp_enum;
-	int check = 0;
-
-	if (cmd->rule_count == CMD_MAX_RULES) {
-		printf("too many rules for cmd\n");
-		exit(1);
-	}
-
-	rule = &cmd->rules[cmd->rule_count++];
-
-	split_line(line, &line_argc, line_argv, ' ');
-
-	for (i = 0; i < line_argc; i++) {
-		arg = line_argv[i];
-
-		if (!strcmp(arg, "not")) {
-			rule->rule = RULE_INVALID;
-			check = 1;
-		}
-
-		else if (!strcmp(arg, "and")) {
-			rule->rule = RULE_REQUIRE;
-			check = 1;
-		}
-
-		else if (!strncmp(arg, "all", 3)) {
-			/* opt/lvt_bits/lvp_bits all remain 0 to mean all */
-			continue;
-		}
-
-		else if (!strncmp(arg, "--", 2)) {
-			if (check)
-				rule->check_opt = opt_str_to_num(arg);
-			else
-				rule->opt = opt_str_to_num(arg);
-		}
-
-		else if (!strncmp(arg, "LV_", 3)) {
-			lvt_enum = lv_to_enum(arg);
-
-			if (check)
-				rule->check_lvt_bits |= lvt_enum_to_bit(lvt_enum);
-			else
-				rule->lvt_bits |= lvt_enum_to_bit(lvt_enum);
-		}
-
-		else if (!strncmp(arg, "lv_is_", 6)) {
-			lvp_enum = lvp_name_to_enum(arg);
-
-			if (check)
-				rule->check_lvp_bits |= lvp_enum_to_bit(lvp_enum);
-			else
-				rule->lvp_bits |= lvp_enum_to_bit(lvp_enum);
-		}
-	}
-}
-
-static char *rule_to_define_str(int rule_type)
-{
-	switch (rule_type) {
-	case RULE_INVALID:
-		return "RULE_INVALID";
-	case RULE_REQUIRE:
-		return "RULE_REQUIRE";
-	}
-}
-
 static char *cmd_flags_to_str(uint32_t flags)
 {
 	static char buf_cmd_flags[32];
@@ -1612,7 +1567,6 @@ static void print_val_man(const char *str)
 static void print_def_man(struct arg_def *def, int usage)
 {
 	int val_enum;
-	int lvt_enum;
 	int sep = 0;
 	int i;
 
@@ -1645,11 +1599,11 @@ static void print_def_man(struct arg_def *def, int usage)
 				sep = 1;
 			}
 
-			if (val_enum == lv_VAL && def->lvt_bits) {
+			if (val_enum == lv_VAL && def->lv_types) {
 				printf("\\fI");
-				for (lvt_enum = 1; lvt_enum < LVT_COUNT; lvt_enum++) {
-					if (lvt_bit_is_set(def->lvt_bits, lvt_enum))
-						printf("_%s", lvt_enum_to_name(lvt_enum));
+				for (i = 0; i < 32; i++) {
+					if (def->lv_types & (1 << i))
+						printf("_%s", lv_num_to_str(1 << i));
 				}
 				printf("\\fP");
 			}
@@ -2361,7 +2315,7 @@ void print_man_command(void)
 void print_command_struct(int only_usage)
 {
 	struct command *cmd;
-	int i, j, ro, rp, oo, op, ru;
+	int i, j, ro, rp, oo, op;
 
 	include_optional_opt_args(&lvm_all, "OO_USAGE_COMMON");
 
@@ -2387,7 +2341,6 @@ void print_command_struct(int only_usage)
 		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);
-		printf("commands[%d].rule_count = %d;\n", i, cmd->rule_count);
 
 		if (cmd->cmd_flags)
 			printf("commands[%d].cmd_flags = %s;\n", i, cmd_flags_to_str(cmd->cmd_flags));
@@ -2416,9 +2369,9 @@ void print_command_struct(int only_usage)
 				printf("commands[%d].required_opt_args[%d].def.val_bits = %s;\n",
 					i, ro, val_bits_to_str(cmd->required_opt_args[ro].def.val_bits));
 
-				if (cmd->required_opt_args[ro].def.lvt_bits)
-					printf("commands[%d].required_opt_args[%d].def.lvt_bits = %s;\n",
-						i, ro, lvt_bits_to_str(cmd->required_opt_args[ro].def.lvt_bits));
+				if (cmd->required_opt_args[ro].def.lv_types)
+					printf("commands[%d].required_opt_args[%d].def.lv_types = %s;\n",
+						i, ro, lv_types_to_flags(cmd->required_opt_args[ro].def.lv_types));
 
 				if (cmd->required_opt_args[ro].def.flags)
 					printf("commands[%d].required_opt_args[%d].def.flags = %s;\n",
@@ -2445,9 +2398,9 @@ void print_command_struct(int only_usage)
 				printf("commands[%d].required_pos_args[%d].def.val_bits = %s;\n",
 					i, rp, val_bits_to_str(cmd->required_pos_args[rp].def.val_bits));
 
-				if (cmd->required_pos_args[rp].def.lvt_bits)
-					printf("commands[%d].required_pos_args[%d].def.lvt_bits = %s;\n",
-						i, rp, lvt_bits_to_str(cmd->required_pos_args[rp].def.lvt_bits));
+				if (cmd->required_pos_args[rp].def.lv_types)
+					printf("commands[%d].required_pos_args[%d].def.lv_types = %s;\n",
+						i, rp, lv_types_to_flags(cmd->required_pos_args[rp].def.lv_types));
 
 				if (cmd->required_pos_args[rp].def.flags)
 					printf("commands[%d].required_pos_args[%d].def.flags = %s;\n",
@@ -2474,9 +2427,9 @@ void print_command_struct(int only_usage)
 				printf("commands[%d].optional_opt_args[%d].def.val_bits = %s;\n",
 					i, oo, val_bits_to_str(cmd->optional_opt_args[oo].def.val_bits));
 
-				if (cmd->optional_opt_args[oo].def.lvt_bits)
-					printf("commands[%d].optional_opt_args[%d].def.lvt_bits = %s;\n",
-						i, oo, lvt_bits_to_str(cmd->optional_opt_args[oo].def.lvt_bits));
+				if (cmd->optional_opt_args[oo].def.lv_types)
+					printf("commands[%d].optional_opt_args[%d].def.lv_types = %s;\n",
+						i, oo, lv_types_to_flags(cmd->optional_opt_args[oo].def.lv_types));
 
 				if (cmd->optional_opt_args[oo].def.flags)
 					printf("commands[%d].optional_opt_args[%d].def.flags = %s;\n",
@@ -2503,9 +2456,9 @@ void print_command_struct(int only_usage)
 				printf("commands[%d].optional_pos_args[%d].def.val_bits = %s;\n",
 					i, op, val_bits_to_str(cmd->optional_pos_args[op].def.val_bits));
 
-				if (cmd->optional_pos_args[op].def.lvt_bits)
-					printf("commands[%d].optional_pos_args[%d].def.lvt_bits = %s;\n",
-						i, op, lvt_bits_to_str(cmd->optional_pos_args[op].def.lvt_bits));
+				if (cmd->optional_pos_args[op].def.lv_types)
+					printf("commands[%d].optional_pos_args[%d].def.lv_types = %s;\n",
+						i, op, lv_types_to_flags(cmd->optional_pos_args[op].def.lv_types));
 
 				if (cmd->optional_pos_args[op].def.flags)
 					printf("commands[%d].optional_pos_args[%d].def.flags = %s;\n",
@@ -2521,31 +2474,6 @@ void print_command_struct(int only_usage)
 			}
 		}
 
-		if (cmd->rule_count) {
-			for (ru = 0; ru < cmd->rule_count; ru++) {
-				printf("commands[%d].rules[%d].opt = %s;\n", i, ru,
-					cmd->rules[ru].opt ? opt_to_enum_str(cmd->rules[ru].opt) : "0");
-
-				printf("commands[%d].rules[%d].lvt_bits = %s;\n", i, ru,
-					cmd->rules[ru].lvt_bits ? lvt_bits_to_str(cmd->rules[ru].lvt_bits) : "0");
-
-				printf("commands[%d].rules[%d].lvp_bits = %s;\n", i, ru,
-					cmd->rules[ru].lvp_bits ? lvp_bits_to_str(cmd->rules[ru].lvp_bits) : "0");
-
-				printf("commands[%d].rules[%d].rule = %s;\n", i, ru,
-					rule_to_define_str(cmd->rules[ru].rule));
-
-				printf("commands[%d].rules[%d].check_opt = %s;\n", i, ru,
-					cmd->rules[ru].check_opt ? opt_to_enum_str(cmd->rules[ru].check_opt) : "0");
-
-				printf("commands[%d].rules[%d].check_lvt_bits = %s;\n", i, ru,
-					cmd->rules[ru].check_lvt_bits ? lvt_bits_to_str(cmd->rules[ru].check_lvt_bits) : "0");
-
-				printf("commands[%d].rules[%d].check_lvp_bits = %s;\n", i, ru,
-					cmd->rules[ru].check_lvp_bits ? lvp_bits_to_str(cmd->rules[ru].check_lvp_bits) : "0");
-			}
-		}
-
 		printf("\n");
 	}
 }
@@ -2772,11 +2700,6 @@ int main(int argc, char *argv[])
 			continue;
 		}
 
-		if (is_rule_line(line_argv[0])) {
-			add_rule(cmd, line_orig);
-			continue;
-		}
-
 		if (is_id_line(line_argv[0])) {
 			cmd->command_line_id = strdup(line_argv[1]);
 			continue;

tools/lvconvert.c

@@ -1364,7 +1364,8 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 				  struct lvconvert_params *lp,
 				  struct dm_list *operable_pvs,
 				  uint32_t new_mimage_count,
-				  uint32_t new_log_count)
+				  uint32_t new_log_count,
+				  struct dm_list *pvh)
 {
 	uint32_t region_size;
 	struct lv_segment *seg = first_seg(lv);
@@ -1384,7 +1385,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 						  vg_is_clustered(lv->vg));
 
 	if (!operable_pvs)
-		operable_pvs = lp->pvh;
+		operable_pvs = pvh;
 
 	/*
 	 * Up-convert from linear to mirror
@@ -1393,7 +1394,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 		/* FIXME Share code with lvcreate */
 
 		/*
-		 * FIXME should we give not only lp->pvh, but also all PVs
+		 * FIXME should we give not only pvh, but also all PVs
 		 * currently taken by the mirror? Would make more sense from
 		 * user perspective.
 		 */
@@ -1402,7 +1403,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 				    lp->alloc, MIRROR_BY_LV))
 			return_0;
 
-		if (lp->wait_completion)
+		if (!arg_is_set(cmd, background_ARG))
 			lp->need_polling = 1;
 
 		goto out;
@@ -1579,7 +1580,8 @@ int mirror_remove_missing(struct cmd_context *cmd,
  */
 static int _lvconvert_mirrors_repair(struct cmd_context *cmd,
 				     struct logical_volume *lv,
-				     struct lvconvert_params *lp)
+				     struct lvconvert_params *lp,
+				     struct dm_list *pvh)
 {
 	int failed_logs;
 	int failed_mimages;
@@ -1590,7 +1592,6 @@ static int _lvconvert_mirrors_repair(struct cmd_context *cmd,
 	uint32_t original_mimages = lv_mirror_count(lv);
 	uint32_t original_logs = _get_log_count(lv);
 
-	cmd->handles_missing_pvs = 1;
 	cmd->partial_activation = 1;
 	lp->need_polling = 0;
 
@@ -1646,7 +1647,7 @@ static int _lvconvert_mirrors_repair(struct cmd_context *cmd,
 	while (replace_mimages || replace_logs) {
 		log_warn("Trying to up-convert to %d images, %d logs.", lp->mirrors, log_count);
 		if (_lvconvert_mirrors_aux(cmd, lv, lp, NULL,
-					   lp->mirrors, log_count))
+					   lp->mirrors, log_count, pvh))
 			break;
 		if (lp->mirrors > 2)
 			--lp->mirrors;
@@ -1766,11 +1767,8 @@ static int _lvconvert_mirrors(struct cmd_context *cmd,
 	    (old_log_count == new_log_count) && !lp->repair)
 		return 1;
 
-	if (lp->repair)
-		return _lvconvert_mirrors_repair(cmd, lv, lp);
-
 	if (!_lvconvert_mirrors_aux(cmd, lv, lp, NULL,
-				    new_mimage_count, new_log_count))
+				    new_mimage_count, new_log_count, lp->pvh))
 		return 0;
 
 	if (!lp->need_polling)
@@ -1801,33 +1799,6 @@ static int _is_valid_raid_conversion(const struct segment_type *from_segtype,
 	return 1;
 }
 
-static void _lvconvert_raid_repair_ask(struct cmd_context *cmd,
-				       struct lvconvert_params *lp,
-				       int *replace_dev)
-{
-	const char *dev_policy;
-
-	*replace_dev = 1;
-
-	if (arg_is_set(cmd, usepolicies_ARG)) {
-		dev_policy = find_config_tree_str(cmd, activation_raid_fault_policy_CFG, NULL);
-
-		if (!strcmp(dev_policy, "allocate") ||
-		    !strcmp(dev_policy, "replace"))
-			return;
-
-		/* else if (!strcmp(dev_policy, "anything_else")) -- no replace */
-		*replace_dev = 0;
-		return;
-	}
-
-	if (!lp->yes &&
-	    yes_no_prompt("Attempt to replace failed RAID images "
-			  "(requires full device resync)? [y/n]: ") == 'n') {
-		*replace_dev = 0;
-	}
-}
-
 static int _lvconvert_raid(struct logical_volume *lv, struct lvconvert_params *lp)
 {
 	int replace = 0, image_count = 0;
@@ -1963,49 +1934,6 @@ static int _lvconvert_raid(struct logical_volume *lv, struct lvconvert_params *l
 		return 1;
 	}
 
-	if (lp->replace)
-		return lv_raid_replace(lv, lp->replace_pvh, lp->pvh);
-
-	if (lp->repair) {
-		if (!lv_is_active_exclusive_locally(lv_lock_holder(lv))) {
-			log_error("%s must be active %sto perform this operation.",
-				  display_lvname(lv),
-				  vg_is_clustered(lv->vg) ?
-				  "exclusive locally " : "");
-			return 0;
-		}
-
-		if (seg_is_striped(seg)) {
-			log_error("Cannot repair LV %s of type raid0.",
-				  display_lvname(lv));
-			return 0;
-		}
-
-		_lvconvert_raid_repair_ask(cmd, lp, &replace);
-
-		if (replace) {
-			if (!(failed_pvs = _failed_pv_list(lv->vg)))
-				return_0;
-
-			if (!lv_raid_replace(lv, failed_pvs, lp->pvh)) {
-				log_error("Failed to replace faulty devices in %s.",
-					  display_lvname(lv));
-				return 0;
-			}
-
-			log_print_unless_silent("Faulty devices in %s successfully replaced.",
-						display_lvname(lv));
-			return 1;
-		}
-
-		/* "warn" if policy not set to replace */
-		if (arg_is_set(cmd, usepolicies_ARG))
-			log_warn("Use 'lvconvert --repair %s' to replace "
-				 "failed device.", display_lvname(lv));
-		return 1;
-	}
-
-
 try_new_takeover_or_reshape:
 
 	/* FIXME This needs changing globally. */
@@ -2509,7 +2437,7 @@ out:
 
 static int _lvconvert_thin_pool_repair(struct cmd_context *cmd,
 				       struct logical_volume *pool_lv,
-				       struct lvconvert_params *lp)
+				       struct dm_list *pvh, int poolmetadataspare)
 {
 	const char *dmdir = dm_dir();
 	const char *thin_dump =
@@ -2538,7 +2466,7 @@ static int _lvconvert_thin_pool_repair(struct cmd_context *cmd,
 	pmslv = pool_lv->vg->pool_metadata_spare_lv;
 
 	/* Check we have pool metadata spare LV */
-	if (!handle_pool_metadata_spare(pool_lv->vg, 0, lp->pvh, 1))
+	if (!handle_pool_metadata_spare(pool_lv->vg, 0, pvh, 1))
 		return_0;
 
 	if (pmslv != pool_lv->vg->pool_metadata_spare_lv) {
@@ -2663,8 +2591,8 @@ deactivate_pmslv:
 	}
 
 	/* Try to allocate new pool metadata spare LV */
-	if (!handle_pool_metadata_spare(pool_lv->vg, 0, lp->pvh,
-					lp->poolmetadataspare))
+	if (!handle_pool_metadata_spare(pool_lv->vg, 0, pvh,
+					poolmetadataspare))
 		stack;
 
 	if (dm_snprintf(meta_path, sizeof(meta_path), "%s_meta%%d", pool_lv->name) < 0) {
@@ -3543,7 +3471,8 @@ static int _convert_thin_pool_uncache(struct cmd_context *cmd, struct logical_vo
 static int _convert_thin_pool_repair(struct cmd_context *cmd, struct logical_volume *lv,
 				     struct lvconvert_params *lp)
 {
-	return _lvconvert_thin_pool_repair(cmd, lv, lp);
+	/* return _lvconvert_thin_pool_repair(cmd, lv, lp); */
+	return 0;
 }
 
 /*
@@ -3731,7 +3660,7 @@ static int _convert_mirror_repair(struct cmd_context *cmd, struct logical_volume
 	struct dm_list *failed_pvs;
 	int ret;
 
-	ret = _lvconvert_mirrors_repair(cmd, lv, lp);
+	ret = _lvconvert_mirrors_repair(cmd, lv, lp, lp->pvh);
 
 	if (ret && arg_is_set(cmd, usepolicies_ARG)) {
 		if ((failed_pvs = _failed_pv_list(lv->vg)))
@@ -4711,3 +4640,305 @@ out:
 
 	return ret;
 }
+
+
+/*
+ * Below is code that has transitioned to using command defs.
+ * ----------------------------------------------------------
+ *
+ * This code does not use read_params (or any other param reading
+ * functions associated with it), or the lp struct.  Those have
+ * been primary vehicles for entangling all the lvconvert operations,
+ * so avoiding them is important for untangling.  They were also
+ * heavily used for trying to figure out what the lvconvert operation
+ * was meant to be doing, and that is no longer needed since the
+ * command def provides it.
+ *
+ * All input data is already available from cmd->arg_values and
+ * cmd->position_argv (the --option args in the former, the position
+ * args in the later.)  There is no need to copy these values into
+ * another redundant struct of input values which just obfuscates.
+ *
+ * The new lvconvert_result struct, passed via custom_handle, is
+ * used for *returning* data from processing, not for passing data
+ * into processing.
+ */
+
+
+/*
+ * Data/results accumulated during processing.
+ */
+struct lvconvert_result {
+	int need_polling;
+	struct dm_list poll_idls;
+};
+
+static int _lvconvert_repair_pvs_mirror(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle,
+			struct dm_list *use_pvh)
+{
+	struct lvconvert_result *lr = (struct lvconvert_result *) handle->custom_handle;
+	struct lvconvert_params lp = { 0 };
+	struct convert_poll_id_list *idl;
+	struct lvinfo info;
+	int ret;
+
+	/*
+	 * FIXME: temporary use of lp because _lvconvert_mirrors_repair()
+	 * and _aux() still use lp fields everywhere.
+	 * Migrate them away from using lp (for the most part just use
+	 * local variables, and check arg_values directly).
+	 */
+
+	/*
+	 * Fill in any lp fields here that this fn expects to be set before
+	 * it's called.  It's hard to tell by reading old code, but it seems
+	 * that repair takes nothing like stripes/stripsize.
+	 */
+	lp.alloc = (alloc_policy_t) arg_uint_value(cmd, alloc_ARG, ALLOC_INHERIT);
+
+	ret = _lvconvert_mirrors_repair(cmd, lv, &lp, use_pvh);
+
+	if (lp.need_polling) {
+		if (!lv_info(cmd, lp.lv_to_poll, 0, &info, 0, 0) || !info.exists)
+			log_print_unless_silent("Conversion starts after activation.");
+		else {
+			if (!(idl = _convert_poll_id_list_create(cmd, lp.lv_to_poll)))
+				return_ECMD_FAILED;
+			dm_list_add(&lr->poll_idls, &idl->list);
+		}
+		lr->need_polling = 1;
+	}
+
+	return ret;
+}
+
+static void _lvconvert_repair_pvs_raid_ask(struct cmd_context *cmd, int *do_it)
+{
+	const char *dev_policy;
+
+	*do_it = 1;
+
+	if (arg_is_set(cmd, usepolicies_ARG)) {
+		dev_policy = find_config_tree_str(cmd, activation_raid_fault_policy_CFG, NULL);
+
+		if (!strcmp(dev_policy, "allocate") ||
+		    !strcmp(dev_policy, "replace"))
+			return;
+
+		/* else if (!strcmp(dev_policy, "anything_else")) -- no replace */
+		*do_it = 0;
+		return;
+	}
+
+	if (!arg_count(cmd, yes_ARG) &&
+	    yes_no_prompt("Attempt to replace failed RAID images "
+			  "(requires full device resync)? [y/n]: ") == 'n') {
+		*do_it = 0;
+	}
+}
+
+static int _lvconvert_repair_pvs_raid(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle,
+			struct dm_list *use_pvh)
+{
+	struct dm_list *failed_pvs;
+	int do_it;
+
+	if (!lv_is_active_exclusive_locally(lv_lock_holder(lv))) {
+		log_error("%s must be active %sto perform this operation.",
+			  display_lvname(lv),
+			  vg_is_clustered(lv->vg) ?
+			  "exclusive locally " : "");
+		return 0;
+	}
+
+	_lvconvert_repair_pvs_raid_ask(cmd, &do_it);
+
+	if (do_it) {
+		if (!(failed_pvs = _failed_pv_list(lv->vg)))
+			return_0;
+
+		if (!lv_raid_replace(lv, failed_pvs, use_pvh)) {
+			log_error("Failed to replace faulty devices in %s.",
+				  display_lvname(lv));
+			return 0;
+		}
+
+		log_print_unless_silent("Faulty devices in %s successfully replaced.",
+					display_lvname(lv));
+		return 1;
+	}
+
+	/* "warn" if policy not set to replace */
+	if (arg_is_set(cmd, usepolicies_ARG))
+		log_warn("Use 'lvconvert --repair %s' to replace "
+			 "failed device.", display_lvname(lv));
+	return 1;
+}
+
+static int _lvconvert_repair_pvs(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	struct dm_list *failed_pvs;
+	struct dm_list *use_pvh;
+	int ret;
+
+	/* First pos arg is required LV, remaining are optional PVs. */
+	if (cmd->position_argc > 1) {
+		if (!(use_pvh = create_pv_list(cmd->mem, lv->vg, cmd->position_argc, cmd->position_argv, 0)))
+			return_ECMD_FAILED;
+	} else
+		use_pvh = &lv->vg->pvs;
+
+	if (lv_is_raid(lv))
+		ret = _lvconvert_repair_pvs_raid(cmd, lv, handle, use_pvh);
+	else if (lv_is_mirror(lv))
+		ret = _lvconvert_repair_pvs_mirror(cmd, lv, handle, use_pvh);
+	else
+		ret = 0;
+
+	if (ret && arg_is_set(cmd, usepolicies_ARG)) {
+		if ((failed_pvs = _failed_pv_list(lv->vg)))
+			_remove_missing_empty_pv(lv->vg, failed_pvs);
+	}
+
+	return ret ? ECMD_PROCESSED : ECMD_FAILED;
+}
+
+static int _lvconvert_repair_thinpool(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	int poolmetadataspare = arg_int_value(cmd, poolmetadataspare_ARG, DEFAULT_POOL_METADATA_SPARE);
+	struct dm_list *use_pvh;
+
+	/* First pos arg is required LV, remaining are optional PVs. */
+	if (cmd->position_argc > 1) {
+		if (!(use_pvh = create_pv_list(cmd->mem, lv->vg, cmd->position_argc, cmd->position_argv, 0)))
+			return_ECMD_FAILED;
+	} else
+		use_pvh = &lv->vg->pvs;
+
+	return _lvconvert_thin_pool_repair(cmd, lv, use_pvh, poolmetadataspare);
+}
+
+static int _lvconvert_repair_pvs_or_thinpool(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	if (lv_is_thin_pool(lv))
+		return _lvconvert_repair_thinpool(cmd, lv, handle);
+	else if (lv_is_raid(lv) || lv_is_mirror(lv))
+		return _lvconvert_repair_pvs(cmd, lv, handle);
+	else
+		return_ECMD_FAILED;
+}
+
+/*
+ * FIXME: add option --repair-pvs to call _lvconvert_repair_pvs() directly,
+ * and option --repair-thinpool to call _lvconvert_repair_thinpool().
+ */
+int lvconvert_repair_pvs_or_thinpool_fn(struct cmd_context *cmd, int argc, char **argv)
+{
+	struct processing_handle *handle;
+	struct lvconvert_result lr = { 0 };
+	struct convert_poll_id_list *idl;
+	int saved_ignore_suspended_devices;
+	int ret, poll_ret;
+
+	dm_list_init(&lr.poll_idls);
+
+	if (!(handle = init_processing_handle(cmd, NULL))) {
+		log_error("Failed to initialize processing handle.");
+		return ECMD_FAILED;
+	}
+
+	handle->custom_handle = &lr;
+
+	saved_ignore_suspended_devices = ignore_suspended_devices();
+	init_ignore_suspended_devices(1);
+
+	cmd->handles_missing_pvs = 1;
+
+	ret = process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL,
+			      READ_FOR_UPDATE, handle,
+			      &_lvconvert_repair_pvs_or_thinpool);
+
+	init_ignore_suspended_devices(saved_ignore_suspended_devices);
+
+	if (lr.need_polling) {
+		dm_list_iterate_items(idl, &lr.poll_idls)
+			poll_ret = _lvconvert_poll_by_id(cmd, idl->id,
+						arg_is_set(cmd, background_ARG), 0, 0);
+		if (poll_ret > ret)
+			ret = poll_ret;
+	}
+
+	destroy_processing_handle(cmd, handle);
+
+	return ret;
+}
+
+static int _lvconvert_replace_pv(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	struct arg_value_group_list *group;
+	struct dm_list *use_pvh;
+	struct dm_list *replace_pvh;
+	char **replace_pvs;
+	const char *tmp_str;
+	int replace_pv_count;
+	int i;
+
+	/* First pos arg is required LV, remaining are optional PVs. */
+	if (cmd->position_argc > 1) {
+		if (!(use_pvh = create_pv_list(cmd->mem, lv->vg, cmd->position_argc, cmd->position_argv, 0)))
+			return_ECMD_FAILED;
+	} else
+		use_pvh = &lv->vg->pvs;
+
+	if (!(replace_pv_count = arg_count(cmd, replace_ARG)))
+		return_ECMD_FAILED;
+
+	if (!(replace_pvs = dm_pool_alloc(cmd->mem, sizeof(char *) * replace_pv_count)))
+		return_ECMD_FAILED;
+
+	i = 0;
+	dm_list_iterate_items(group, &cmd->arg_value_groups) {
+		if (!grouped_arg_is_set(group->arg_values, replace_ARG))
+			continue;
+		if (!(tmp_str = grouped_arg_str_value(group->arg_values, replace_ARG, NULL))) {
+			log_error("Failed to get '--replace' argument");
+			return_ECMD_FAILED;
+		}
+		if (!(replace_pvs[i++] = dm_pool_strdup(cmd->mem, tmp_str)))
+			return_ECMD_FAILED;
+	}
+
+	if (!(replace_pvh = create_pv_list(cmd->mem, lv->vg, replace_pv_count, replace_pvs, 0)))
+		return_ECMD_FAILED;
+
+	return lv_raid_replace(lv, replace_pvh, use_pvh);
+}
+
+int lvconvert_replace_pv_fn(struct cmd_context *cmd, int argc, char **argv)
+{
+	struct processing_handle *handle;
+	struct lvconvert_result lr = { 0 };
+	int ret;
+
+	if (!(handle = init_processing_handle(cmd, NULL))) {
+		log_error("Failed to initialize processing handle.");
+		return ECMD_FAILED;
+	}
+
+	handle->custom_handle = &lr;
+
+	ret = process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL,
+			      READ_FOR_UPDATE, handle,
+			      &_lvconvert_replace_pv);
+
+	destroy_processing_handle(cmd, handle);
+
+	return ret;
+}
+

tools/lvmcmdline.c

@@ -79,31 +79,16 @@ struct command_name command_names[MAX_COMMAND_NAMES] = {
 #undef xx
 };
 
-/*
- * Table of LV properties
- */
-static struct lv_props _lv_props[LVP_COUNT + 1] = {
-#define lvp(a, b, c) {a, b, c},
-#include "lv_props.h"
-#undef lvp
-};
-
-/*
- * Table of LV types
- */
-static struct lv_types _lv_types[LVT_COUNT + 1] = {
-#define lvt(a, b, c) {a, b, c},
-#include "lv_types.h"
-#undef lvt
-};
-
-
 /*
  * Table of valid command lines
  */
 static struct command commands[COMMAND_COUNT];
 static struct cmdline_context _cmdline;
 
+
+int lvconvert_replace_pv_fn(struct cmd_context *cmd, int argc, char **argv);
+int lvconvert_repair_pvs_or_thinpool_fn(struct cmd_context *cmd, int argc, char **argv);
+
 /*
  * Table of command line functions
  *
@@ -112,19 +97,24 @@ static struct cmdline_context _cmdline;
  * 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 },
+	{ lvmconfig_general_CMD,			lvmconfig },
+
+	/* lvconvert: utilities related to snapshots and repair */
+	{ lvconvert_repair_pvs_or_thinpool_CMD,		lvconvert_repair_pvs_or_thinpool_fn },
+	{ lvconvert_replace_pv_CMD,			lvconvert_replace_pv_fn },
 };
+
 #if 0
 	/* all raid-related type conversions */
 
 	{ lvconvert_raid_types_CMD,			lvconvert_raid_types_fn },
 
-	/* raid-related utilities (move into lvconvert_raid_types?) */
+	/* lvconvert: raid-related utilities (move into lvconvert_raid_types?) */
 
 	{ lvconvert_split_mirror_images_CMD,		lvconvert_split_mirror_images_fn },
 	{ lvconvert_change_mirrorlog_CMD,		lvconvert_change_mirrorlog_fn },
 
-	/* utilities for creating/maintaining thin and cache objects. */
+	/* lvconvert: utilities for creating/maintaining thin and cache objects. */
 
 	{ lvconvert_to_thin_with_external_CMD,		lvconvert_to_thin_with_external_fn },
 	{ lvconvert_to_cache_vol_CMD,			lvconvert_to_cache_vol_fn },
@@ -134,12 +124,10 @@ struct command_function command_functions[COMMAND_ID_COUNT] = {
 	{ lvconvert_split_and_delete_cachepool_CMD,	lvconvert_split_and_delete_cachepool_fn },
 	{ lvconvert_swap_pool_metadata_CMD,		lvconvert_swap_pool_metadata_fn },
 
-	/* utilities related to snapshots and repair. */
+	/* lvconvert: utilities related to snapshots and repair. */
 
 	{ lvconvert_merge_CMD,				lvconvert_merge_fn },
 	{ lvconvert_combine_split_snapshot_CMD,		lvconvert_combine_split_snapshot_fn },
-	{ lvconvert_repair_pvs_or_thinpool_CMD,		lvconvert_repair_pvs_or_thinpool_fn },
-	{ lvconvert_replace_pv_CMD,			lvconvert_replace_pv_fn },
 	{ lvconvert_split_cow_snapshot_CMD,		lvconvert_split_cow_snapshot_fn },
 	{ lvconvert_poll_start_CMD,			lvconvert_poll_start_fn },
 
@@ -1098,20 +1086,6 @@ void lvm_register_commands(void)
 		_set_valid_args_for_command_name(i);
 }
 
-struct lv_props *get_lv_prop(int lvp_enum)
-{
-	if (!lvp_enum)
-		return NULL;
-	return &_lv_props[lvp_enum];
-}
-
-struct lv_types *get_lv_type(int lvt_enum)
-{
-	if (!lvt_enum)
-		return NULL;
-	return &_lv_types[lvt_enum];
-}
-
 /*
  * Also see merge_synonym().  The command definitions
  * are written using just one variation of the option
@@ -1457,7 +1431,6 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	int i, j;
 	int opt_enum, opt_i;
 	int accepted, count;
-	int check_is_set;
 
 	name = last_path_component(path);
 
@@ -1678,42 +1651,6 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			return NULL;
 		}
 	}
-
-	/*
-	 * Check any rules related to option combinations.  If the only
-	 * qualification for this rule to apply is that rule->opt is set (it's
-	 * not also related to a specific LV type or property), and the rule
-	 * check is related to another option, then we can do the option
-	 * validation here.
-	 */
-
-	for (i = 0; i < commands[best_i].rule_count; i++) {
-		struct cmd_rule *rule;
-		rule = &commands[best_i].rules[i];
-
-		if (rule->opt && rule->check_opt && !rule->lvt_bits && !rule->lvp_bits &&
-		    arg_is_set(cmd, rule->opt)) {
-
-			check_is_set = arg_is_set(cmd, rule->check_opt);
-
-			if (check_is_set && (rule->rule == RULE_INVALID)) {
-				log_error("Invalid option combination for command (%s %d): %s and %s",
-					  commands[best_i].command_line_id, best_i,
-					  arg_long_option_name(rule->opt),
-					  arg_long_option_name(rule->check_opt));
-				return NULL;
-			}
-
-			if (!check_is_set && (rule->rule == RULE_REQUIRE)) {
-				log_error("Invalid option usage for command (%s %d): %s requires %s",
-					  commands[best_i].command_line_id, best_i,
-					  arg_long_option_name(rule->opt),
-					  arg_long_option_name(rule->check_opt));
-				return NULL;
-			}
-		}
-	}
-
 out:
 	log_debug("command line id: %s %d", commands[best_i].command_line_id, best_i);
 
@@ -1773,7 +1710,7 @@ static int _usage(const char *name, int help_count)
 		log_print("  be omitted if the --select option is used.");
 		log_print(". --size Number can be replaced with --extents NumberExtents.");
 		log_print(". When --name is omitted from lvcreate, a new LV name is");
-		log_print("  generated with the \"lvol\" prefix and a unique numeric suffix.");
+		log_print("  generated with the \"lvol\" prefix and a unique numeral suffix.");
 		log_print(". The required VG parameter in lvcreate may be omitted when");
 		log_print("  the VG name is included in another option, e.g. --name VG/LV.");
 		log_print(". For required options listed in parentheses, e.g. (--A, --B),");

tools/toollib.c

@@ -2326,407 +2326,6 @@ static struct lv_segment _historical_lv_segment = {
 	.origin_list = DM_LIST_HEAD_INIT(_historical_lv_segment.origin_list),
 };
 
-static void lvp_bits_to_str(uint64_t bits, char *buf, int len)
-{
-	struct lv_props *prop;
-	int lvp_enum;
-	int pos = 0;
-	int ret;
-
-	for (lvp_enum = 0; lvp_enum < LVP_COUNT; lvp_enum++) {
-		if (!(prop = get_lv_prop(lvp_enum)))
-			continue;
-
-		if (lvp_bit_is_set(bits, lvp_enum)) {
-			ret = snprintf(buf + pos, len - pos, "%s ", prop->name);
-			if (ret >= len - pos)
-				break;
-			pos += ret;
-		}
-	}
-	buf[len - 1] = '\0';
-}
-
-static const char *lvt_bits_to_str(uint64_t bits)
-{
-	struct lv_types *type;
-	int lvt_enum;
-
-	for (lvt_enum = 0; lvt_enum < LVT_COUNT; lvt_enum++) {
-		if (!(type = get_lv_type(lvt_enum)))
-			continue;
-
-		if (lvt_bit_is_set(bits, lvt_enum))
-			return type->name;
-	}
-
-	return "unknown";
-}
-
-/*
- * This is the lv_prop function pointer used for lv_is_foo() #defines.
- * Alternatively, lv_is_foo() could all be turned into functions.
- */
-
-static int _lv_is_prop(struct cmd_context *cmd, struct logical_volume *lv, int lvp_enum)
-{
-	switch (lvp_enum) {
-	case is_locked_LVP:
-		return lv_is_locked(lv);
-	case is_partial_LVP:
-		return lv_is_partial(lv);
-	case is_virtual_LVP:
-		return lv_is_virtual(lv);
-	case is_merging_LVP:
-		return lv_is_merging(lv);
-	case is_merging_origin_LVP:
-		return lv_is_merging_origin(lv);
-	case is_converting_LVP:
-		return lv_is_converting(lv);
-	case is_external_origin_LVP:
-		return lv_is_external_origin(lv);
-	case is_virtual_origin_LVP:
-		return lv_is_virtual_origin(lv);
-	case is_not_synced_LVP:
-		return lv_is_not_synced(lv);
-	case is_pending_delete_LVP:
-		return lv_is_pending_delete(lv);
-	case is_error_when_full_LVP:
-		return lv_is_error_when_full(lv);
-	case is_pvmove_LVP:
-		return lv_is_pvmove(lv);
-	case is_removed_LVP:
-		return lv_is_removed(lv);
-	case is_thinpool_data_LVP:
-		return lv_is_thin_pool_data(lv);
-	case is_thinpool_metadata_LVP:
-		return lv_is_thin_pool_metadata(lv);
-	case is_cachepool_data_LVP:
-		return lv_is_cache_pool_data(lv);
-	case is_cachepool_metadata_LVP:
-		return lv_is_cache_pool_metadata(lv);
-	case is_mirror_image_LVP:
-		return lv_is_mirror_image(lv);
-	case is_mirror_log_LVP:
-		return lv_is_mirror_log(lv);
-	case is_raid_image_LVP:
-		return lv_is_raid_image(lv);
-	case is_raid_metadata_LVP:
-		return lv_is_raid_metadata(lv);
-	case is_origin_LVP:
-		return lv_is_origin(lv);
-	case is_thin_origin_LVP:
-		return lv_is_thin_origin(lv, NULL);
-	case is_cache_origin_LVP:
-		return lv_is_cache_origin(lv);
-	case is_merging_cow_LVP:
-		return lv_is_merging_cow(lv);
-	case is_cow_covering_origin_LVP:
-		return lv_is_cow_covering_origin(lv);
-	case is_visible_LVP:
-		return lv_is_visible(lv);
-	case is_historical_LVP:
-		return lv_is_historical(lv);
-	case is_raid_with_tracking_LVP:
-		return lv_is_raid_with_tracking(lv);
-	default:
-		log_error(INTERNAL_ERROR "unknown lv property value lvp_enum %d", lvp_enum);
-	}
-
-	return 0;
-}
-
-/*
- * Check if an LV matches a given LV type enum.
- */
-
-static int _lv_is_type(struct cmd_context *cmd, struct logical_volume *lv, int lvt_enum)
-{
-	struct lv_segment *seg = first_seg(lv);
-
-	switch (lvt_enum) {
-	case striped_LVT:
-		return seg_is_striped(seg);
-	case linear_LVT:
-		return seg_is_linear(seg);
-	case snapshot_LVT:
-		return lv_is_cow(lv);
-	case thin_LVT:
-		return lv_is_thin_volume(lv);
-	case thinpool_LVT:
-		return lv_is_thin_pool(lv);
-	case cache_LVT:
-		return lv_is_cache(lv);
-	case cachepool_LVT:
-		return lv_is_cache_pool(lv);
-	case mirror_LVT:
-		return lv_is_mirror(lv);
-	case raid_LVT:
-		return lv_is_raid(lv);
-	case raid0_LVT:
-		return seg_is_raid0(seg);
-	case raid1_LVT:
-		return seg_is_raid1(seg);
-	case raid4_LVT:
-		return seg_is_raid4(seg);
-#if 0
-	case raid5_LVT:
-		return seg_is_raid5(seg);
-	case raid6_LVT:
-		return seg_is_raid6(seg);
-#endif
-	case raid10_LVT:
-		return seg_is_raid10(seg);
-	default:
-		log_error(INTERNAL_ERROR "unknown lv type value lvt_enum %d", lvt_enum);
-	}
-
-	return 0;
-}
-
-static int _get_lvt_enum(struct logical_volume *lv)
-{
-	struct lv_segment *seg = first_seg(lv);
-
-	if (seg_is_striped(seg))
-		return striped_LVT;
-	if (seg_is_linear(seg))
-		return linear_LVT;
-	if (lv_is_cow(lv))
-		return snapshot_LVT;
-	if (lv_is_thin_volume(lv))
-		return thin_LVT;
-	if (lv_is_thin_pool(lv))
-		return thinpool_LVT;
-	if (lv_is_cache(lv))
-		return cache_LVT;
-	if (lv_is_cache_pool(lv))
-		return cachepool_LVT;
-	if (lv_is_mirror(lv))
-		return mirror_LVT;
-	if (lv_is_raid(lv))
-		return raid_LVT;
-	if (seg_is_raid0(seg))
-		return raid0_LVT;
-	if (seg_is_raid1(seg))
-		return raid1_LVT;
-	if (seg_is_raid4(seg))
-		return raid4_LVT;
-#if 0
-	if (seg_is_raid5(seg))
-		return raid5_LVT;
-	if (seg_is_raid6(seg))
-		return raid6_LVT;
-#endif
-	if (seg_is_raid10(seg))
-		return raid10_LVT;
-
-	log_error(INTERNAL_ERROR "unknown lv type for %s", display_lvname(lv));
-	return 0;
-}
-
-/* Call lv_is_<type> for each <type>_LVT bit set in lvt_bits. */
-
-static int _lv_types_match(struct cmd_context *cmd, struct logical_volume *lv, uint64_t lvt_bits)
-{
-	struct lv_types *type;
-	int lvt_enum;
-	int ret = 1;
-
-	for (lvt_enum = 1; lvt_enum < LVT_COUNT; lvt_enum++) {
-		if (!lvt_bit_is_set(lvt_bits, lvt_enum))
-			continue;
-
-		if (!(type = get_lv_type(lvt_enum)))
-			continue;
-
-		/*
-		 * All types are currently handled by _lv_is_type()
-		 * because lv_is_type() are #defines and not exposed
-		 * in tools.h
-		 */
-
-		if (!type->fn)
-			ret = _lv_is_type(cmd, lv, lvt_enum);
-		else
-			ret = type->fn(cmd, lv);
-	}
-
-	return ret;
-}
-
-/* Call lv_is_<prop> for each <prop>_LVP bit set in lvp_bits. */
-
-static int _lv_props_match(struct cmd_context *cmd, struct logical_volume *lv, uint64_t lvp_bits,
-			   uint64_t *match_bits, uint64_t *unmatch_bits)
-{
-	struct lv_props *prop;
-	int lvp_enum;
-	int ret = 1;
-
-	if (match_bits)
-		*match_bits = 0;
-	if (unmatch_bits)
-		*unmatch_bits = 0;
-
-	for (lvp_enum = 1; lvp_enum < LVP_COUNT; lvp_enum++) {
-		if (!lvp_bit_is_set(lvp_bits, lvp_enum))
-			continue;
-
-		if (!(prop = get_lv_prop(lvp_enum)))
-			continue;
-
-		if (!prop->fn)
-			ret = _lv_is_prop(cmd, lv, lvp_enum);
-		else
-			ret = prop->fn(cmd, lv);
-
-		if (match_bits && ret)
-			*match_bits |= lvp_enum_to_bit(lvp_enum);
-
-		if (unmatch_bits && !ret)
-			*unmatch_bits |= lvp_enum_to_bit(lvp_enum);
-	}
-
-	return ret;
-}
-
-/*
- * 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.
- */
-
-static int _check_lv_types(struct cmd_context *cmd, struct logical_volume *lv)
-{
-	int ret = 1;
-
-	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.lvt_bits) {
-		ret = _lv_types_match(cmd, lv, cmd->command->required_pos_args[0].def.lvt_bits);
-		if (!ret) {
-			int lvt_enum = _get_lvt_enum(lv);
-			struct lv_types *type = get_lv_type(lvt_enum);
-			log_warn("Operation on LV %s which has invalid type %s.",
-				 display_lvname(lv), type ? type->name : "unknown");
-		}
-	}
-
-	return ret;
-}
-
-/* Check if LV passes each rule specified in command definition. */
-
-static int _check_lv_rules(struct cmd_context *cmd, struct logical_volume *lv)
-{
-	char buf[64];
-	struct cmd_rule *rule;
-	struct lv_types *lvtype = NULL;
-	int lvt_enum;
-	int opt_is_set, lv_types_match, lv_props_match;
-	uint64_t match_bits, unmatch_bits;
-	int ret = 1;
-	int i;
-
-	lvt_enum = _get_lvt_enum(lv);
-	if (lvt_enum)
-		lvtype = get_lv_type(lvt_enum);
-
-
-	for (i = 0; i < cmd->command->rule_count; i++) {
-		rule = &cmd->command->rules[i];
-
-		/*
-		 * For the rule to apply, any option, LV type or LV property
-		 * that is specified before the not|and rule needs to match the
-		 * command/LV.
-		 *
-		 * If all qualifications are zero (!opt && !lvt_bits && !lvp_bits),
-		 * then there are no qualifications and the rule always applies.
-		 */
-
-		if (rule->opt && !arg_is_set(cmd, rule->opt))
-			continue;
-
-		if (rule->lvt_bits && !_lv_types_match(cmd, lv, rule->lvt_bits))
-			continue;
-
-		if (rule->lvp_bits && !_lv_props_match(cmd, lv, rule->lvp_bits, NULL, NULL))
-			continue;
-
-		/*
-		 * Check the option, LV types, LV properties specified after
-		 * the not|and rule.
-		 */
-
-		if (rule->check_opt)
-			opt_is_set = arg_is_set(cmd, rule->check_opt);
-
-		if (rule->check_lvt_bits)
-			lv_types_match = _lv_types_match(cmd, lv, rule->check_lvt_bits);
-
-		if (rule->check_lvp_bits)
-			lv_props_match = _lv_props_match(cmd, lv, rule->check_lvp_bits, &match_bits, &unmatch_bits);
-		
-		/*
-		 * Evaluate if the check results pass based on the rule.
-		 */
-
-		if (rule->check_opt && (rule->rule == RULE_INVALID) && opt_is_set) {
-			log_warn("Command option %s invalid with option %s.",
-				 arg_long_option_name(rule->opt), arg_long_option_name(rule->check_opt));
-			ret = 0;
-		}
-
-		if (rule->check_opt && (rule->rule == RULE_REQUIRE) && !opt_is_set)  {
-			log_warn("Command option %s requires option %s.",
-				 arg_long_option_name(rule->opt), arg_long_option_name(rule->check_opt));
-			ret = 0;
-		}
-
-		if (rule->check_lvt_bits && (rule->rule == RULE_INVALID) && lv_types_match) {
-			log_warn("Command on LV %s with invalid type: %s.",
-				 display_lvname(lv), lvtype ? lvtype->name : "unknown");
-			ret = 0;
-		}
-
-		if (rule->check_lvt_bits && (rule->rule == RULE_REQUIRE) && !lv_types_match) {
-			log_warn("Command on LV %s type %s requires type: %s.",
-				 display_lvname(lv), lvtype ? lvtype->name : "unknown",
-				 lvt_bits_to_str(rule->check_lvt_bits));
-			ret = 0;
-		}
-
-		if (rule->check_lvp_bits && (rule->rule == RULE_INVALID) && lv_props_match) {
-			memset(buf, 0, sizeof(buf));
-			lvp_bits_to_str(match_bits, buf, sizeof(buf));
-			log_warn("Command on LV %s with invalid properties: %s.",
-				 display_lvname(lv), buf);
-			ret = 0;
-		}
-
-		if (rule->check_lvp_bits && (rule->rule == RULE_REQUIRE) && !lv_props_match) {
-			memset(buf, 0, sizeof(buf));
-			lvp_bits_to_str(unmatch_bits, buf, sizeof(buf));
-			log_warn("Command on LV %s requires properties: %s.",
-				 display_lvname(lv), buf);
-			ret = 0;
-		}
-	}
-
-	return ret;
-}
-
 int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 			  struct dm_list *arg_lvnames, const struct dm_list *tags_in,
 			  int stop_on_error,
@@ -2751,6 +2350,9 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 	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);
 
@@ -2902,28 +2504,65 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 			continue;
 
 		/*
-		 * The command definition may include restrictions on the
-		 * types and properties of LVs that can be processed.
+		 * 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 (!_check_lv_types(cmd, lvl->lv)) {
-			log_warn("Operation not permitted (%s %d) on LV %s.",
-				 cmd->command->command_line_id, cmd->command->command_line_enum,
-				 display_lvname(lvl->lv));
-			if (str_list_match_item(&found_arg_lvnames, lvl->lv->name))
-				ret_max = ECMD_FAILED;
-			/* FIXME: include this result in report log? */
-			continue;
-		}
+		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) {
 
-		if (!_check_lv_rules(cmd, lvl->lv)) {
-			log_warn("Operation not permitted (%s %d) on LV %s.",
-				 cmd->command->command_line_id, cmd->command->command_line_enum,
-				 display_lvname(lvl->lv));
-			if (str_list_match_item(&found_arg_lvnames, lvl->lv->name))
-				ret_max = ECMD_FAILED;
-			/* FIXME: include this result in report log? */
-			continue;
+			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 (%s %d) on LV %s with type %s.",
+						  cmd->command->command_line_id, cmd->command->command_line_enum,
+						  display_lvname(lv), seg->segtype->name);
+					ret_max = ECMD_FAILED;
+				} else {
+					log_warn("Operation not permitted (%s %d) on LV %s with type %s.",
+						  cmd->command->command_line_id, cmd->command->command_line_enum,
+						  display_lvname(lv), seg->segtype->name);
+				}
+				continue;
+			}
 		}
 
 		log_very_verbose("Processing LV %s in VG %s.", lvl->lv->name, vg->name);
@@ -3035,7 +2674,8 @@ out:
  */
 static int _get_arg_lvnames(struct cmd_context *cmd,
 			    int argc, char **argv,
-			    const char *one_vgname, const char *one_lvname,
+			    const char *one_vgname,
+			    const char *one_lvname,
 			    struct dm_list *arg_vgnames,
 			    struct dm_list *arg_lvnames,
 			    struct dm_list *arg_tags)
@@ -3156,6 +2796,134 @@ static int _get_arg_lvnames(struct cmd_context *cmd,
 	return ret_max;
 }
 
+/*
+ * Some commands will look in specific options to:
+ *
+ * - find the intended LV name if only the VG name was found
+ *   in the position arg or env.
+ *
+ *   command --foo=lvname VG
+ *   . add VG/lvname to arg_lvnames
+ *
+ * - find both the intended VG name and LV name if nothing
+ *   was found in the position arg or env.
+ *
+ *   command --foo=vgname/lvname
+ *   . add vgname to arg_vgnames
+ *   . add vgname/lvname to arg_lvnames
+ *
+ * If a VG name was found in the position arg or from env,
+ * and options are searched for an LV name, then if VG name
+ * is repeated in the option name, verify it matches the VG
+ * name found previously.
+ *
+ *   command --foo=vgname/lvname VG
+ *   . verify vgname matches VG
+ *   . add vgname/lvname to arg_lvnames
+ *
+ *
+ * In some cases, lvconvert wants to get the intended vg/lv
+ * from --thinpool, --cachepool.
+ *
+ *
+ * N.B.
+ * lvconvert --snapshot is a special case where the first
+ * positional arg is saved away and skipped, and the second
+ * positional arg is the LV that is passed to process_each.
+ */
+
+#if 0
+static int _get_arg_lvnames_from_options(struct cmd_context *cmd,
+					 struct dm_list *arg_vgnames,
+					 struct dm_list *arg_lvnames)
+{
+	struct str_list *sl;
+	const char *arg_name = NULL;;
+	const char *pos_vgname = NULL;
+	const char *pos_lvname = NULL;
+	const char *opt_lvname = NULL;
+	const char *opt_vgname = NULL;
+	const char *use_vgname = NULL;
+	char *tmp_name;
+	char *split;
+	char *vglv;
+	size_t vglv_sz;
+	int i;
+
+	dm_list_iterate_items(sl, arg_vgnames) {
+		pos_vgname = sl->str;
+		break;
+	}
+
+	dm_list_iterate_items(sl, arg_lvnames) {
+		if ((pos_lvname = strchr(sl->str, '/')))
+			pos_lvname++;
+		break;
+	}
+
+
+	if (arg_is_set(cmd, thinpool_ARG))
+		arg_name = arg_str_value(cmd, thinpool_ARG, NULL);
+	else if (arg_is_set(cmd, cachepool_ARG))
+		arg_name = arg_str_value(cmd, cachepool_ARG, NULL);
+
+	if (arg_name) {
+		if ((split = strchr(arg_name, '/'))) {
+			/* combined VG/LV */
+
+			if (!(tmp_name = dm_pool_strdup(cmd->mem, arg_name))) {
+				log_error("string alloc failed.");
+				return ECMD_FAILED;
+			}
+
+			if (!(split = strchr(tmp_name, '/')))
+				return ECMD_FAILED;
+
+			opt_vgname = tmp_name;
+			opt_lvname = split + 1;
+			*split = '\0';
+		} else {
+			/* only LV */
+			opt_lvname = arg_name;
+		}
+
+		if (pos_vgname && opt_vgname && strcmp(pos_vgname, opt_vgname)) {
+			log_error("VG name mismatch from position arg (%s) and option arg (%s).",
+				  pos_vgname, opt_vgname);
+			return ECMD_FAILED; 
+		}
+
+		if (!pos_vgname && opt_vgname) {
+			if (!str_list_add(cmd->mem, arg_vgnames,
+					  dm_pool_strdup(cmd->mem, opt_vgname))) {
+				log_error("strlist allocation failed.");
+				return ECMD_FAILED;
+			}
+			use_vgname = opt_vgname;
+		} else {
+			use_vgname = pos_vgname;
+		}
+
+		if (use_vgname && !pos_lvname && opt_lvname) {
+			vglv_sz = strlen(use_vgname) + strlen(opt_lvname) + 2;
+
+			if (!(vglv = dm_pool_alloc(cmd->mem, vglv_sz)) ||
+			    dm_snprintf(vglv, vglv_sz, "%s/%s", use_vgname, opt_lvname) < 0) {
+				log_error("vg/lv string alloc failed.");
+				return ECMD_FAILED;
+			}
+
+			if (!str_list_add(cmd->mem, arg_lvnames, vglv)) {
+				log_error("strlist allocation failed.");
+				return ECMD_FAILED;
+			}
+		}
+	}
+
+	return ECMD_PROCESSED;
+}
+#endif
+
 static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 				     struct dm_list *vgnameids_to_process,
 				     struct dm_list *arg_vgnames,
@@ -3318,6 +3086,19 @@ int process_each_lv(struct cmd_context *cmd,
 		goto_out;
 	}
 
+#if 0
+	/*
+	 * Some commands will search for VG/LV position args from option
+	 * values, e.g. lvconvert.
+	 */
+	if (cmd->command->flags & ALLOW_VGLV_ARG_FROM_OPTIONS) {
+		if ((ret = _get_arg_lvnames_from_options(cmd, argc, argv, &arg_vgnames, &arg_lvnames) != ECMD_PROCESSED)) {
+			ret_max = ret;
+			goto_out;
+		}
+	}
+#endif
+
 	if (!handle && !(handle = init_processing_handle(cmd, NULL))) {
 		ret_max = ECMD_FAILED;
 		goto_out;

tools/tools.h

@@ -50,14 +50,14 @@
 #define CMD_LEN 256
 #define MAX_ARGS 64
 
-/* define the enums for the values accepted by command line --options, foo_VAL */
+/* define the enums for the values accepted by command line --options */
 enum {
 #define val(a, b, c, d) a ,
 #include "vals.h"
 #undef val
 };
 
-/* define the enums for the command line --options, foo_ARG */
+/* define the enums for the command line --options */
 enum {
 #define arg(a, b, c, d, e, f) a ,
 #include "args.h"
@@ -69,20 +69,6 @@ enum {
 #include "commands.h"
 #undef xx
 
-/* define enums for LV properties, foo_LVP */
-enum {
-#define lvp(a, b, c) a ,
-#include "lv_props.h"
-#undef lvp
-};
-
-/* define enums for LV types, foo_LVT */
-enum {
-#define lvt(a, b, c) a ,
-#include "lv_types.h"
-#undef lvt
-};
-
 #include "command.h"
 
 #define ARG_COUNTABLE 0x00000001	/* E.g. -vvvv */
@@ -125,21 +111,6 @@ struct val_props {
 	const char *usage;
 };
 
-/* a global table of possible LV properties */
-struct lv_props {
-	int lvp_enum; /* is_foo_LVP from lv_props.h */
-	const char *name; /* "lv_is_foo" used in command-lines.in */
-	int (*fn) (struct cmd_context *cmd, struct logical_volume *lv); /* lv_is_foo() */
-};
-
-/* a global table of possible LV types */
-/* (as exposed externally in command line interface, not exactly as internal segtype is used) */
-struct lv_types {
-	int lvt_enum; /* is_foo_LVT from lv_types.h */
-	const char *name; /* "foo" used in command-lines.in, i.e. LV_foo */
-	int (*fn) (struct cmd_context *cmd, struct logical_volume *lv); /* lv_is_foo() */
-};
-
 #define CACHE_VGMETADATA	0x00000001
 #define PERMITTED_READ_ONLY 	0x00000002
 /* Process all VGs if none specified on the command line. */
@@ -238,7 +209,4 @@ int vgchange_activate(struct cmd_context *cmd, struct volume_group *vg,
 
 int vgchange_background_polling(struct cmd_context *cmd, struct volume_group *vg);
 
-struct lv_props *get_lv_prop(int lvp_enum);
-struct lv_types *get_lv_type(int lvt_enum);
-
 #endif