commands: new method for defining commands

. Define a prototype for every lvm command.
. Match every user command with one definition.
. Generate help text and man pages from them.

The new file command-lines.in defines a prototype for every
unique lvm command.  A unique lvm command is a unique
combination of: command name + required option args +
required positional args.  Each of these prototypes also
includes the optional option args and optional positional
args that the command will accept, a description, and a
unique string ID for the definition.  Any valid command
will match one of the prototypes.

Here's an example of the lvresize command definitions from
command-lines.in, there are three unique lvresize commands:

lvresize --size SizeMB LV
OO: --alloc Alloc, --autobackup Bool, --force,
--nofsck, --nosync, --noudevsync, --reportformat String, --resizefs,
--stripes Number, --stripesize SizeKB, --test, --poolmetadatasize SizeMB
OP: PV ...
ID: lvresize_by_size
DESC: Resize an LV by a specified size.

lvresize LV PV ...
OO: --alloc Alloc, --autobackup Bool, --force,
--nofsck, --nosync, --noudevsync,
--reportformat String, --resizefs, --stripes Number, --stripesize SizeKB,
--test
ID: lvresize_by_pv
DESC: Resize an LV by a specified PV.

lvresize --poolmetadatasize SizeMB LV_thinpool
OO: --alloc Alloc, --autobackup Bool, --force,
--nofsck, --nosync, --noudevsync,
--reportformat String, --stripes Number, --stripesize SizeKB,
--test
OP: PV ...
ID: lvresize_pool_metadata_by_size
DESC: Resize a pool metadata SubLV by a specified size.

The three commands have separate definitions because they have
different required parameters.  Required parameters are specified
on the first line of the definition.  Optional options are
listed after OO, and optional positional args are listed after OP.

This data is used to generate corresponding command definition
structures for lvm in command-lines.h.  usage/help output is also
auto generated, so it is always in sync with the definitions.

Example of the corresponding generated structure in
command-lines.h for the first lvresize prototype
(these structures are never edited directly):

commands[85].name = "lvresize";
commands[85].command_line_id = "lvresize_by_size";
commands[85].command_line_enum = lvresize_by_size_CMD;
commands[85].fn = lvresize;
commands[85].ro_count = 1;
commands[85].rp_count = 1;
commands[85].oo_count = 22;
commands[85].op_count = 1;
commands[85].desc = "DESC: Resize an LV by a specified size.";
commands[85].usage = "lvresize --size Number[m|unit] LV"
" [ --resizefs, --poolmetadatasize Number[m|unit], COMMON_OPTIONS ]"
" [ PV ... ]";
commands[85].usage_common =
" [ --alloc contiguous|cling|cling_by_tags|normal|anywhere|inherit, --commandprofile String, --config String, --driverloaded y|n, --nosync, --noudevsync, --profile String, --reportformat String, --version, --autobackup y|n, --debug, --force, --help, --stripes Number, --stripesize Number[k|unit], --nofsck, --quiet, --test, --verbose, --yes, --commandprofile String, --config String, --debug, --driverloaded y|n, --help, --profile String, --quiet, --verbose, --version, --yes, --force, --test, --noudevsync ]";
commands[85].required_opt_args[0].opt = size_ARG;
commands[85].required_opt_args[0].def.val_bits = val_enum_to_bit(sizemb_VAL);
commands[85].required_pos_args[0].pos = 1;
commands[85].required_pos_args[0].def.val_bits = val_enum_to_bit(lv_VAL);
commands[85].optional_opt_args[0].opt = commandprofile_ARG;
commands[85].optional_opt_args[0].def.val_bits = val_enum_to_bit(string_VAL);
commands[85].optional_opt_args[1].opt = config_ARG;
commands[85].optional_opt_args[1].def.val_bits = val_enum_to_bit(string_VAL);
commands[85].optional_opt_args[2].opt = debug_ARG;
commands[85].optional_opt_args[3].opt = driverloaded_ARG;
commands[85].optional_opt_args[3].def.val_bits = val_enum_to_bit(bool_VAL);
commands[85].optional_opt_args[4].opt = help_ARG;
commands[85].optional_opt_args[5].opt = profile_ARG;
commands[85].optional_opt_args[5].def.val_bits = val_enum_to_bit(string_VAL);
commands[85].optional_opt_args[6].opt = quiet_ARG;
commands[85].optional_opt_args[7].opt = verbose_ARG;
commands[85].optional_opt_args[8].opt = version_ARG;
commands[85].optional_opt_args[9].opt = yes_ARG;
commands[85].optional_opt_args[10].opt = alloc_ARG;
commands[85].optional_opt_args[10].def.val_bits = val_enum_to_bit(alloc_VAL);
commands[85].optional_opt_args[11].opt = autobackup_ARG;
commands[85].optional_opt_args[11].def.val_bits = val_enum_to_bit(bool_VAL);
commands[85].optional_opt_args[12].opt = force_ARG;
commands[85].optional_opt_args[13].opt = nofsck_ARG;
commands[85].optional_opt_args[14].opt = nosync_ARG;
commands[85].optional_opt_args[15].opt = noudevsync_ARG;
commands[85].optional_opt_args[16].opt = reportformat_ARG;
commands[85].optional_opt_args[16].def.val_bits = val_enum_to_bit(string_VAL);
commands[85].optional_opt_args[17].opt = resizefs_ARG;
commands[85].optional_opt_args[18].opt = stripes_ARG;
commands[85].optional_opt_args[18].def.val_bits = val_enum_to_bit(number_VAL);
commands[85].optional_opt_args[19].opt = stripesize_ARG;
commands[85].optional_opt_args[19].def.val_bits = val_enum_to_bit(sizekb_VAL);
commands[85].optional_opt_args[20].opt = test_ARG;
commands[85].optional_opt_args[21].opt = poolmetadatasize_ARG;
commands[85].optional_opt_args[21].def.val_bits = val_enum_to_bit(sizemb_VAL);
commands[85].optional_pos_args[0].pos = 2;
commands[85].optional_pos_args[0].def.val_bits = val_enum_to_bit(pv_VAL);
commands[85].optional_pos_args[0].def.flags = ARG_DEF_FLAG_MAY_REPEAT;

Every user-entered command is compared against the set of
command structures, and matched with one.  An error is
reported if an entered command does not have the required
parameters for any definition.  The closest match is printed
as a suggestion, and running lvresize --help will display
the usage for each possible lvresize command.

The prototype syntax used for help/man output includes
required --option and positional args on the first line,
and optional --option and positional args enclosed in [ ]
on subsequent lines.

  command_name <required_opt_args> <required_pos_args>
          [ <optional_opt_args> ]
          [ <optional_pos_args> ]

$ lvresize --help
  lvresize - Resize a logical volume

  Resize an LV by a specified size.
  lvresize --size Number[m|unit] LV
        [ --resizefs,
          --poolmetadatasize Number[m|unit],
          COMMON_OPTIONS ]
        [ PV ... ]

  Resize an LV by a specified PV.
  lvresize LV PV ...
        [ --resizefs,
          COMMON_OPTIONS ]

  Resize a pool metadata SubLV by a specified size.
  lvresize --poolmetadatasize Number[m|unit] LV_thinpool
        [ COMMON_OPTIONS ]
        [ PV ... ]

  Common options:
        [ --alloc contiguous|cling|cling_by_tags|normal|anywhere|inherit,
          --commandprofile String,
          --config String,
          --driverloaded y|n,
          --nosync,
          --noudevsync,
          --profile String,
          --reportformat String,
          --version,
          --autobackup y|n,
          --debug,
          --force,
          --help,
          --stripes Number,
          --stripesize Number[k|unit],
          --nofsck,
          --quiet,
          --test,
          --verbose,
          --yes,
          --commandprofile String,
          --config String,
          --debug,
          --driverloaded y|n,
          --help,
          --profile String,
          --quiet,
          --verbose,
          --version,
          --yes,
          --force,
          --test,
          --noudevsync ]

  (Use --help --help for usage notes.)

$ lvresize --poolmetadatasize 4
  Failed to find a matching command definition.
  Closest command usage is:
  lvresize --poolmetadatasize Number[m|unit] LV_thinpool

Man page prototypes are also generated from the same original
command definitions, and are always in sync with the code
and help text.

Very early in command execution, a matching command definition
is found.  lvm then knows the operation being done, and that
the provided args conform to the definition.  This will allow
lots of ad hoc checking/validation to be removed throughout
the code.

Each command definition can also be routed to a specific
function to implement it.  The function is associated with
an enum value for the command definition (generated from
the ID string.)  These per-command-definition implementation
functions have not yet been created, so all commands
currently fall back to the existing per-command-name
implementation functions.

Using per-command-definition functions will allow lots of
code to be removed which tries to figure out what the
command is meant to do.  This is currently based on ad hoc
and complicated option analysis.  When using the new
functions, what the command is doing is already known
from the associated command definition.

So, this first phase validates every user-entered command
against the set of command prototypes, then calls the existing
implementation.  The second phase can associate an implementation
function with each definition, and take further advantage of the
known operation to avoid the complicated option analysis.

lib/commands/toolcontext.h

@@ -94,13 +94,6 @@ struct cmd_context {
 	struct arg_values *opt_arg_values;
 	struct dm_list arg_value_groups;
 
-	/*
-	 * Position args remaining after command name
-	 * and --options are removed from original argc/argv.
-	 */
-	int position_argc;
-	char **position_argv;
-
 	/*
 	 * Format handlers.
 	 */

man/Makefile.in

@@ -140,13 +140,6 @@ Makefile: Makefile.in
 	  *) echo "Creating $@" ; $(SED) -e "s+#VERSION#+$(LVM_VERSION)+;s+#DEFAULT_SYS_DIR#+$(DEFAULT_SYS_DIR)+;s+#DEFAULT_ARCHIVE_DIR#+$(DEFAULT_ARCHIVE_DIR)+;s+#DEFAULT_BACKUP_DIR#+$(DEFAULT_BACKUP_DIR)+;s+#DEFAULT_PROFILE_DIR#+$(DEFAULT_PROFILE_DIR)+;s+#DEFAULT_CACHE_DIR#+$(DEFAULT_CACHE_DIR)+;s+#DEFAULT_LOCK_DIR#+$(DEFAULT_LOCK_DIR)+;s+#CLVMD_PATH#+@CLVMD_PATH@+;s+#LVM_PATH#+@LVM_PATH@+;s+#DEFAULT_RUN_DIR#+@DEFAULT_RUN_DIR@+;s+#DEFAULT_PID_DIR#+@DEFAULT_PID_DIR@+;s+#SYSTEMD_GENERATOR_DIR#+$(SYSTEMD_GENERATOR_DIR)+;s+#DEFAULT_MANGLING#+$(DEFAULT_MANGLING)+;" $< > $@ ;; \
 	esac
 
-ccmd: ../tools/create-commands.c
-	$(CC) ../tools/create-commands.c -o ccmd
-
-generate: ccmd
-	./ccmd --output man -s 0 -p 1 -c lvcreate ../tools/command-lines.in > lvcreate.8.a
-	cat lvcreate.8.a lvcreate.8.b > lvcreate.8.in
-
 install_man5: $(MAN5)
 	$(INSTALL) -d $(MAN5DIR)
 	$(INSTALL_DATA) $(MAN5) $(MAN5DIR)/

test/shell/listings.sh

@@ -81,7 +81,8 @@ lvcreate -l1 -s -n inval $vg/$lv3
 lvcreate -l4 -I4 -i2 -n stripe $vg
 # Invalidate snapshot
 not dd if=/dev/zero of="$DM_DEV_DIR/$vg/inval" bs=4K
-invalid lvscan "$dev1"
+# ignores unused positional arg dev1
+lvscan "$dev1"
 lvdisplay --maps
 lvscan --all
 
@@ -108,13 +109,16 @@ vgmknodes --refresh
 lvscan
 lvmdiskscan
 
-invalid pvscan "$dev1"
+# ignores unused arg
+pvscan "$dev1"
 invalid pvscan -aay
 invalid pvscan --major 254
 invalid pvscan --minor 0
 invalid pvscan --novolumegroup -e
-invalid vgscan $vg
-invalid lvscan $vg
+# ignores unsed arg
+vgscan $vg
+# ignroes unused arg
+lvscan $vg
 
 if aux have_readline; then
 cat <<EOF | lvm

test/shell/vgextend-restoremissing.sh

@@ -23,6 +23,7 @@ lvcreate -l 1 -n lv1 $vg "$dev1"
 invalid vgextend
 # --metadatacopies => use --pvmetadatacopies
 invalid vgextend --metadatacopies 3 $vg "$dev1" 2>&1 | tee out
+grep -- "use --pvmetadatacopies" out
 
 # VG name should exist
 fail vgextend --restoremissing $vg-invalid "$dev1"

tools/args.h

@@ -31,8 +31,8 @@ arg(cachemode_ARG, '\0', "cachemode", cachemode_VAL, 0, 0)
 arg(cachepool_ARG, '\0', "cachepool", lv_VAL, 0, 0)
 arg(commandprofile_ARG, '\0', "commandprofile", string_VAL, 0, 0)
 arg(config_ARG, '\0', "config", string_VAL, 0, 0)
-arg(configreport_ARG, '\0', "configreport", configreport_VAL, ARG_GROUPABLE, 1)
-arg(configtype_ARG, '\0', "typeconfig", configtype_VAL, 0, 0)
+arg(configreport_ARG, '\0', "configreport", string_VAL, ARG_GROUPABLE, 1)
+arg(configtype_ARG, '\0', "typeconfig", string_VAL, 0, 0)
 arg(corelog_ARG, '\0', "corelog", 0, 0, 0)
 arg(dataalignment_ARG, '\0', "dataalignment", sizekb_VAL, 0, 0)
 arg(dataalignmentoffset_ARG, '\0', "dataalignmentoffset", sizekb_VAL, 0, 0)
@@ -81,26 +81,26 @@ arg(noudevsync_ARG, '\0', "noudevsync", 0, 0, 0)
 arg(originname_ARG, '\0', "originname", lv_VAL, 0, 0)
 arg(physicalvolumesize_ARG, '\0', "setphysicalvolumesize", sizemb_VAL, 0, 0)
 arg(poll_ARG, '\0', "poll", bool_VAL, 0, 0)
-arg(polloperation_ARG, '\0', "polloperation", polloperation_VAL, 0, 0)
+arg(polloperation_ARG, '\0', "polloperation", string_VAL, 0, 0)
 arg(pooldatasize_ARG, '\0', "pooldatasize", sizemb_VAL, 0, 0)
 arg(poolmetadata_ARG, '\0', "poolmetadata", lv_VAL, 0, 0)
 arg(poolmetadatasize_ARG, '\0', "poolmetadatasize", sizemb_VAL, 0, 0)
 arg(poolmetadataspare_ARG, '\0', "poolmetadataspare", bool_VAL, 0, 0)
 arg(profile_ARG, '\0', "profile", string_VAL, 0, 0)
-arg(pvmetadatacopies_ARG, '\0', "pvmetadatacopies", pvmetadatacopies_VAL, 0, 0)
-arg(raidrebuild_ARG, '\0', "raidrebuild", pv_VAL, ARG_GROUPABLE, 0)
+arg(pvmetadatacopies_ARG, '\0', "pvmetadatacopies", number_VAL, 0, 0)
+arg(raidrebuild_ARG, '\0', "raidrebuild", string_VAL, ARG_GROUPABLE, 0)
 arg(raidmaxrecoveryrate_ARG, '\0', "raidmaxrecoveryrate", sizekb_VAL, 0, 0)
 arg(raidminrecoveryrate_ARG, '\0', "raidminrecoveryrate", sizekb_VAL, 0, 0)
-arg(raidsyncaction_ARG, '\0', "raidsyncaction", syncaction_VAL, 0, 0)
+arg(raidsyncaction_ARG, '\0', "raidsyncaction", string_VAL, 0, 0)
 arg(raidwritebehind_ARG, '\0', "raidwritebehind", number_VAL, 0, 0)
-arg(raidwritemostly_ARG, '\0', "raidwritemostly", writemostly_VAL, ARG_GROUPABLE, 0)
+arg(raidwritemostly_ARG, '\0', "raidwritemostly", string_VAL, ARG_GROUPABLE, 0)
 arg(readonly_ARG, '\0', "readonly", 0, 0, 0)
 arg(refresh_ARG, '\0', "refresh", 0, 0, 0)
 arg(removemissing_ARG, '\0', "removemissing", 0, 0, 0)
 arg(rebuild_ARG, '\0', "rebuild", pv_VAL, ARG_GROUPABLE, 0)
 arg(repair_ARG, '\0', "repair", 0, 0, 0)
 arg(replace_ARG, '\0', "replace", pv_VAL, ARG_GROUPABLE, 0)
-arg(reportformat_ARG, '\0', "reportformat", reportformat_VAL, 0, 0)
+arg(reportformat_ARG, '\0', "reportformat", string_VAL, 0, 0)
 arg(restorefile_ARG, '\0', "restorefile", string_VAL, 0, 0)
 arg(restoremissing_ARG, '\0', "restoremissing", 0, 0, 0)
 arg(resync_ARG, '\0', "resync", 0, 0, 0)
@@ -116,7 +116,7 @@ arg(splitsnapshot_ARG, '\0', "splitsnapshot", 0, 0, 0)
 arg(showdeprecated_ARG, '\0', "showdeprecated", 0, 0, 0)
 arg(showunsupported_ARG, '\0', "showunsupported", 0, 0, 0)
 arg(stripes_long_ARG, '\0', "stripes", number_VAL, 0, 0)
-arg(syncaction_ARG, '\0', "syncaction", syncaction_VAL, 0, 0)
+arg(syncaction_ARG, '\0', "syncaction", string_VAL, 0, 0) /* FIXME Use custom VAL */
 arg(sysinit_ARG, '\0', "sysinit", 0, 0, 0)
 arg(systemid_ARG, '\0', "systemid", string_VAL, 0, 0)
 arg(thinpool_ARG, '\0', "thinpool", lv_VAL, 0, 0)
@@ -133,14 +133,14 @@ arg(unquoted_ARG, '\0', "unquoted", 0, 0, 0)
 arg(usepolicies_ARG, '\0', "usepolicies", 0, 0, 0)
 arg(validate_ARG, '\0', "validate", 0, 0, 0)
 arg(version_ARG, '\0', "version", 0, 0, 0)
-arg(vgmetadatacopies_ARG, '\0', "vgmetadatacopies", vgmetadatacopies_VAL, 0, 0)
+arg(vgmetadatacopies_ARG, '\0', "vgmetadatacopies", metadatacopies_VAL, 0, 0)
 arg(virtualoriginsize_ARG, '\0', "virtualoriginsize", sizemb_VAL, 0, 0)
 arg(withsummary_ARG, '\0', "withsummary", 0, 0, 0)
 arg(withcomments_ARG, '\0', "withcomments", 0, 0, 0)
 arg(withspaces_ARG, '\0', "withspaces", 0, 0, 0)
 arg(withversions_ARG, '\0', "withversions", 0, 0, 0)
 arg(writebehind_ARG, '\0', "writebehind", number_VAL, 0, 0)
-arg(writemostly_ARG, '\0', "writemostly", writemostly_VAL, ARG_GROUPABLE, 0)
+arg(writemostly_ARG, '\0', "writemostly", string_VAL, ARG_GROUPABLE, 0)
 
 /* Allow some variations */
 arg(allocation_ARG, '\0', "allocation", bool_VAL, 0, 0)

tools/command.h

@@ -51,8 +51,8 @@ struct command_name {
  */
 
 /* arg_def flags */
-#define ARG_DEF_FLAG_NEW                1 << 0
-#define ARG_DEF_FLAG_MAY_REPEAT         1 << 1
+#define ARG_DEF_FLAG_NEW        1 << 0
+#define ARG_DEF_FLAG_MAY_REPEAT 1 << 1
 
 /* arg_def lv_types */
 enum {
@@ -82,7 +82,7 @@ static inline int val_bit_is_set(uint64_t val_bits, int val_enum)
 
 static inline uint64_t val_enum_to_bit(int val_enum)
 {
-	return (1ULL << val_enum);
+	return 1 << val_enum;
 }
 
 /* Description a value that follows an option or exists in a position. */
@@ -123,8 +123,7 @@ struct pos_arg {
  * one or more from required_opt_args is required,
  * then the rest are optional.
  */
-#define CMD_FLAG_ONE_REQUIRED_OPT   1
-#define CMD_FLAG_SECONDARY_SYNTAX   2
+#define CMD_FLAG_ONE_REQUIRED_OPT 1
 
 /* a register of the lvm commands */
 struct command {
@@ -165,43 +164,6 @@ struct command {
 
 	/* used for processing current position */
 	int pos_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

@@ -47,15 +47,7 @@ int segtype_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
 int alloc_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
 int locktype_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
 int readahead_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
-int vgmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int pvmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
 int metadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int polloperation_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int writemostly_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int syncaction_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int reportformat_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-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 */
 struct opt_name {
@@ -116,13 +108,8 @@ static struct opt_name opt_names[ARG_COUNT + 1] = {
 struct cmd_name {
 	const char *name;
 	const char *desc;
-	int common_options[ARG_COUNT + 1];
-	int all_options[ARG_COUNT + 1];
 	int variants;
-	int variant_has_ro;
-	int variant_has_rp;
-	int variant_has_oo;
-	int variant_has_op;
+	int common_options[ARG_COUNT + 1];
 };
 
 /* create table of command names, e.g. vgcreate */
@@ -154,9 +141,6 @@ struct command lvm_all; /* for printing common options for all lvm commands */
 int oo_line_count;
 struct oo_line oo_lines[MAX_OO_LINES];
 
-static int include_man_secondary = 1;
-static int include_man_primary = 1;
-static char *man_command_name = NULL;
 
 static void add_optional_opt_line(struct command *cmd, int argc, char *argv[]);
 
@@ -261,7 +245,7 @@ static int opt_str_to_num(char *str)
 	exit(1);
 }
 
-static char *val_bits_to_str(uint64_t val_bits)
+static char *val_bits_to_str(int val_bits)
 {
 	static char buf[128];
 	int i;
@@ -593,13 +577,6 @@ static int is_desc_line(char *str)
 	return 0;
 }
 
-static int is_flags_line(char *str)
-{
-	if (!strncmp(str, "FLAGS:", 6))
-		return 1;
-	return 0;
-}
-
 static int is_id_line(char *str)
 {
 	if (!strncmp(str, "ID:", 3))
@@ -1170,46 +1147,14 @@ static char *flags_to_str(int flags)
 
 	memset(buf_flags, 0, sizeof(buf_flags));
 
-	if (flags & ARG_DEF_FLAG_MAY_REPEAT) {
-		if (buf_flags[0])
-			strcat(buf_flags, " | ");
+	if (flags & ARG_DEF_FLAG_MAY_REPEAT)
 		strcat(buf_flags, "ARG_DEF_FLAG_MAY_REPEAT");
-	}
-	if (flags & ARG_DEF_FLAG_NEW) {
-		if (buf_flags[0])
-			strcat(buf_flags, " | ");
+	if (flags & ARG_DEF_FLAG_NEW)
 		strcat(buf_flags, "ARG_DEF_FLAG_NEW");
-	}
 
 	return buf_flags;
 }
 
-static void add_flags(struct command *cmd, char *line)
-{
-	if (strstr(line, "SECONDARY_SYNTAX"))
-		cmd->cmd_flags |= CMD_FLAG_SECONDARY_SYNTAX;
-}
-
-static char *cmd_flags_to_str(uint32_t flags)
-{
-	static char buf_cmd_flags[32];
-
-	memset(buf_cmd_flags, 0, sizeof(buf_cmd_flags));
-
-	if (flags & CMD_FLAG_SECONDARY_SYNTAX) {
-		if (buf_cmd_flags[0])
-			strcat(buf_cmd_flags, " | ");
-		strcat(buf_cmd_flags, "CMD_FLAG_SECONDARY_SYNTAX");
-	}
-	if (flags & CMD_FLAG_ONE_REQUIRED_OPT) {
-		if (buf_cmd_flags[0])
-			strcat(buf_cmd_flags, " | ");
-		strcat(buf_cmd_flags, "CMD_FLAG_ONE_REQUIRED_OPT");
-	}
-
-	return buf_cmd_flags;
-}
-
 void print_command_count(void)
 {
 	struct command *cmd;
@@ -1256,7 +1201,7 @@ static int is_lvm_all_opt(int opt)
 
 static void factor_common_options(void)
 {
-	int cn, opt_enum, ci, oo, ro, found;
+	int cn, opt_enum, ci, oo, found;
 	struct command *cmd;
 
 	for (cn = 0; cn < MAX_CMD_NAMES; cn++) {
@@ -1280,24 +1225,6 @@ static void factor_common_options(void)
 				if (strcmp(cmd->name, cmd_names[cn].name))
 					continue;
 
-				if (cmd->ro_count)
-					cmd_names[cn].variant_has_ro = 1;
-				if (cmd->rp_count)
-					cmd_names[cn].variant_has_rp = 1;
-				if (cmd->oo_count)
-					cmd_names[cn].variant_has_oo = 1;
-				if (cmd->op_count)
-					cmd_names[cn].variant_has_op = 1;
-
-				for (ro = 0; ro < cmd->ro_count; ro++) {
-					cmd_names[cn].all_options[cmd->required_opt_args[ro].opt] = 1;
-
-					if ((cmd->required_opt_args[ro].opt == size_ARG) && !strncmp(cmd->name, "lv", 2))
-						cmd_names[cn].all_options[extents_ARG] = 1;
-				}
-				for (oo = 0; oo < cmd->oo_count; oo++)
-					cmd_names[cn].all_options[cmd->optional_opt_args[oo].opt] = 1;
-
 				found = 0;
 
 				for (oo = 0; oo < cmd->oo_count; oo++) {
@@ -1538,21 +1465,11 @@ static void print_val_man(const char *str)
 	}
 
 	if (strstr(str, "|")) {
-		int len = strlen(str);
 		line = strdup(str);
 		split_line(line, &line_argc, line_argv, '|');
 		for (i = 0; i < line_argc; i++) {
-			if (i) {
+			if (i)
 				printf("|");
-
-				/* this is a hack to add a line break for
-				   a long string of opt values */
-				if ((len > 40) && (i >= (line_argc / 2) + 1)) {
-					printf("\n");
-					printf("       ");
-					len = 0;
-				}
-			}
 			if (strstr(line_argv[i], "Number"))
 				printf("\\fI%s\\fP", line_argv[i]);
 			else
@@ -1622,48 +1539,6 @@ static void print_def_man(struct arg_def *def, int usage)
 		printf(" ...");
 }
 
-static char *man_long_opt_name(const char *cmdname, int opt_enum)
-{
-	static char long_opt_name[64];
-
-	memset(&long_opt_name, 0, sizeof(long_opt_name));
-
-	switch (opt_enum) {
-	case syncaction_ARG:
-		strncpy(long_opt_name, "--[raid]syncaction", 63);
-		break;
-	case writemostly_ARG:
-		strncpy(long_opt_name, "--[raid]writemostly", 63);
-		break;
-	case minrecoveryrate_ARG:
-		strncpy(long_opt_name, "--[raid]minrecoveryrate", 63);
-		break;
-	case maxrecoveryrate_ARG:
-		strncpy(long_opt_name, "--[raid]maxrecoveryrate", 63);
-		break;
-	case writebehind_ARG:
-		strncpy(long_opt_name, "--[raid]writebehind", 63);
-		break;
-	case vgmetadatacopies_ARG:
-		if (!strncmp(cmdname, "vg", 2))
-			strncpy(long_opt_name, "--[vg]metadatacopies", 63);
-		else
-			strncpy(long_opt_name, "--vgmetadatacopies", 63);
-		break;
-	case pvmetadatacopies_ARG:
-		if (!strncmp(cmdname, "pv", 2))
-			strncpy(long_opt_name, "--[pv]metadatacopies", 63);
-		else
-			strncpy(long_opt_name, "--pvmetadatacopies", 63);
-		break;
-	default:
-		strncpy(long_opt_name, opt_names[opt_enum].long_opt, 63);
-		break;
-	}
-
-	return long_opt_name;
-}
-
 void print_man_usage(struct command *cmd)
 {
 	struct cmd_name *cname;
@@ -1698,9 +1573,7 @@ void print_man_usage(struct command *cmd)
 
 		/* print required options with a short opt */
 		for (ro = 0; ro < cmd->ro_count; ro++) {
-			opt_enum = cmd->required_opt_args[ro].opt;
-
-			if (!opt_names[opt_enum].short_opt)
+			if (!opt_names[cmd->required_opt_args[ro].opt].short_opt)
 				continue;
 
 			if (sep) {
@@ -1709,13 +1582,13 @@ void print_man_usage(struct command *cmd)
 				printf(" ");
 			}
 
-			if (opt_names[opt_enum].short_opt) {
+			if (opt_names[cmd->required_opt_args[ro].opt].short_opt) {
 				printf(" \\fB-%c\\fP|\\fB%s\\fP",
-				       opt_names[opt_enum].short_opt,
-				       man_long_opt_name(cmd->name, opt_enum));
+				       opt_names[cmd->required_opt_args[ro].opt].short_opt,
+				       opt_names[cmd->required_opt_args[ro].opt].long_opt);
 			} else {
 				printf("   ");
-				printf(" \\fB%s\\fP", man_long_opt_name(cmd->name, opt_enum));
+				printf(" \\fB%s\\fP", opt_names[cmd->required_opt_args[ro].opt].long_opt);
 			}
 
 			if (cmd->required_opt_args[ro].def.val_bits) {
@@ -1728,9 +1601,7 @@ void print_man_usage(struct command *cmd)
 
 		/* print required options without a short opt */
 		for (ro = 0; ro < cmd->ro_count; ro++) {
-			opt_enum = cmd->required_opt_args[ro].opt;
-
-			if (opt_names[opt_enum].short_opt)
+			if (opt_names[cmd->required_opt_args[ro].opt].short_opt)
 				continue;
 
 			if (sep) {
@@ -1740,7 +1611,7 @@ void print_man_usage(struct command *cmd)
 			}
 
 			printf("   ");
-			printf(" \\fB%s\\fP", man_long_opt_name(cmd->name, opt_enum));
+			printf(" \\fB%s\\fP", opt_names[cmd->required_opt_args[ro].opt].long_opt);
 
 			if (cmd->required_opt_args[ro].def.val_bits) {
 				printf(" ");
@@ -1754,7 +1625,7 @@ void print_man_usage(struct command *cmd)
 		printf(".RE\n");
 	}
 
-	/* print required position args on a new line after the onereq set */
+	/* print required positional args on a new line after the onereq set */
 	if (cmd->rp_count) {
 		printf(".RS 4\n");
 		for (rp = 0; rp < cmd->rp_count; rp++) {
@@ -1782,12 +1653,10 @@ void print_man_usage(struct command *cmd)
 
 	if (cmd->ro_count) {
 		for (ro = 0; ro < cmd->ro_count; ro++) {
-			opt_enum = cmd->required_opt_args[ro].opt;
-
-			if (opt_names[opt_enum].short_opt) {
+			if (opt_names[cmd->required_opt_args[ro].opt].short_opt) {
 				printf(" \\fB-%c\\fP|\\fB%s\\fP",
-				       opt_names[opt_enum].short_opt,
-				       man_long_opt_name(cmd->name, opt_enum));
+				       opt_names[cmd->required_opt_args[ro].opt].short_opt,
+				       opt_names[cmd->required_opt_args[ro].opt].long_opt);
 			} else {
 				printf(" \\fB%s\\fP", opt_names[cmd->required_opt_args[ro].opt].long_opt);
 			}
@@ -1799,7 +1668,7 @@ void print_man_usage(struct command *cmd)
 		}
 	}
 
-	/* print required position args on the same line as the required options */
+	/* print required positional args on the same line as the required options */
 	if (cmd->rp_count) {
 		for (rp = 0; rp < cmd->rp_count; rp++) {
 			if (cmd->required_pos_args[rp].def.val_bits) {
@@ -1853,7 +1722,7 @@ void print_man_usage(struct command *cmd)
 
 			printf(" \\fB-%c\\fP|\\fB%s\\fP",
 				opt_names[opt_enum].short_opt,
-				man_long_opt_name(cmd->name, opt_enum));
+				opt_names[opt_enum].long_opt);
 
 			if (cmd->optional_opt_args[oo].def.val_bits) {
 				printf(" ");
@@ -1891,7 +1760,7 @@ void print_man_usage(struct command *cmd)
 			/* space alignment without short opt */
 			printf("   ");
 
-			printf(" \\fB%s\\fP", man_long_opt_name(cmd->name, opt_enum));
+			printf(" \\fB%s\\fP", opt_names[opt_enum].long_opt);
 
 			if (cmd->optional_opt_args[oo].def.val_bits) {
 				printf(" ");
@@ -1929,7 +1798,7 @@ void print_man_usage(struct command *cmd)
 		}
 	}
 
-	printf(" ]\n");
+	printf("]\n");
 	printf(".RE\n");
 
  done:
@@ -1980,7 +1849,7 @@ void print_man_usage_common(struct command *cmd)
 
 			printf(" \\fB-%c\\fP|\\fB%s\\fP",
 				opt_names[opt_enum].short_opt,
-				man_long_opt_name(cmd->name, opt_enum));
+				opt_names[opt_enum].long_opt);
 
 			if (cmd->optional_opt_args[oo].def.val_bits) {
 				printf(" ");
@@ -2016,7 +1885,7 @@ void print_man_usage_common(struct command *cmd)
 			/* space alignment without short opt */
 			printf("   ");
 
-			printf(" \\fB%s\\fP", man_long_opt_name(cmd->name, opt_enum));
+			printf(" \\fB%s\\fP", opt_names[opt_enum].long_opt);
 
 			if (cmd->optional_opt_args[oo].def.val_bits) {
 				printf(" ");
@@ -2044,7 +1913,7 @@ void print_man_usage_common(struct command *cmd)
 
 		printf(" \\fB-%c\\fP|\\fB%s\\fP",
 			opt_names[opt_enum].short_opt,
-			man_long_opt_name(cmd->name, opt_enum));
+			opt_names[opt_enum].long_opt);
 
 		if (lvm_all.optional_opt_args[oo].def.val_bits) {
 			printf(" ");
@@ -2069,7 +1938,7 @@ void print_man_usage_common(struct command *cmd)
 		/* space alignment without short opt */
 		printf("   ");
 
-		printf(" \\fB%s\\fP", man_long_opt_name(cmd->name, opt_enum));
+		printf(" \\fB%s\\fP", opt_names[opt_enum].long_opt);
 
 		if (lvm_all.optional_opt_args[oo].def.val_bits) {
 			printf(" ");
@@ -2077,78 +1946,8 @@ void print_man_usage_common(struct command *cmd)
 		}
 		sep = 1;
 	}
-	printf(" ]\n");
-}
-
-void print_man_all_options(struct cmd_name *cname)
-{
-	int opt_enum, val_enum;
-	int sep = 0;
-
-	/* print those with short opts */
-	for (opt_enum = 0; opt_enum < ARG_COUNT; opt_enum++) {
-		if (!cname->all_options[opt_enum])
-			continue;
-
-		if (!opt_names[opt_enum].short_opt)
-			continue;
-
-		if (sep)
-			printf("\n.br\n");
-
-		printf(" \\fB-%c\\fP|\\fB%s\\fP",
-			opt_names[opt_enum].short_opt,
-			man_long_opt_name(cname->name, opt_enum));
-
-		val_enum = opt_names[opt_enum].val_enum;
-
-		if (!val_names[val_enum].fn) {
-			/* takes no arg */
-		} else if (!val_names[val_enum].usage) {
-			printf(" ");
-			printf("\\fI");
-			printf("%s", val_names[val_enum].name);
-			printf("\\fP");
-		} else {
-			printf(" ");
-			print_val_man(val_names[val_enum].usage);
-		}
-
-		sep = 1;
-	}
-
-	/* print those without short opts */
-	for (opt_enum = 0; opt_enum < ARG_COUNT; opt_enum++) {
-		if (!cname->all_options[opt_enum])
-			continue;
-
-		if (opt_names[opt_enum].short_opt)
-			continue;
-
-		if (sep)
-			printf("\n.br\n");
-
-		/* space alignment without short opt */
-		printf("   ");
-
-		printf(" \\fB%s\\fP", man_long_opt_name(cname->name, opt_enum));
-
-		val_enum = opt_names[opt_enum].val_enum;
-
-		if (!val_names[val_enum].fn) {
-			/* takes no arg */
-		} else if (!val_names[val_enum].usage) {
-			printf(" ");
-			printf("\\fI");
-			printf("%s", val_names[val_enum].name);
-			printf("\\fP");
-		} else {
-			printf(" ");
-			print_val_man(val_names[val_enum].usage);
-		}
-
-		sep = 1;
-	}
+	printf(" ]\"");
+	printf(";\n");
 }
 
 #define DESC_LINE 256
@@ -2191,56 +1990,26 @@ void print_desc_man(const char *desc)
 	}
 }
 
-static char *upper_command_name(char *str)
-{
-	static char str_upper[32];
-	int i = 0;
-
-	while (*str) {
-		str_upper[i++] = toupper(*str);
-		str++;
-	}
-	str_upper[i] = '\0';
-	return str_upper;
-}
-
 void print_man_command(void)
 {
-	struct cmd_name *cname;
-	struct command *cmd, *prev_cmd = NULL;
+	struct command *cmd;
+	const char *last_cmd_name = NULL;
 	const char *desc;
 	int i, j, ro, rp, oo, op;
 
 	include_optional_opt_args(&lvm_all, "OO_USAGE_COMMON");
 
-	printf(".TH %s 8 \"LVM TOOLS #VERSION#\" \"Sistina Software UK\"\n",
-		man_command_name ? upper_command_name(man_command_name) : "LVM_COMMANDS");
+	printf(".TH LVM_COMMANDS 8\n");
 
 	for (i = 0; i < cmd_count; i++) {
 
 		cmd = &cmd_array[i];
 
-		if (prev_cmd && strcmp(prev_cmd->name, cmd->name)) {
-			printf("Common options:\n");
-			printf(".\n");
-			print_man_usage_common(prev_cmd);
-			prev_cmd = NULL;
-		}
-
-		if ((cmd->cmd_flags & CMD_FLAG_SECONDARY_SYNTAX) && !include_man_secondary)
-			continue;
-
-		if (!(cmd->cmd_flags & CMD_FLAG_SECONDARY_SYNTAX) && !include_man_primary)
-			continue;
-
-		if (man_command_name && strcmp(man_command_name, cmd->name))
-			continue;
-
-		if (!prev_cmd || strcmp(prev_cmd->name, cmd->name)) {
+		if (!last_cmd_name || strcmp(last_cmd_name, cmd->name)) {
 			printf(".SH NAME\n");
 			printf(".\n");
 			if ((desc = cmd_name_desc(cmd->name)))
-				printf("%s \\- %s\n", cmd->name, desc);
+				printf("%s - %s\n", cmd->name, desc);
 			else
 				printf("%s\n", cmd->name);
 			printf(".br\n");
@@ -2250,48 +2019,7 @@ void print_man_command(void)
 			printf(".br\n");
 			printf(".P\n");
 			printf(".\n");
-			prev_cmd = cmd;
-
-			if (!(cname = find_command_name(cmd->name)))
-				return;
-
-			if (cname->variant_has_ro && cname->variant_has_rp)
-				printf("\\fB%s\\fP \\fIrequired_option_args\\fP \\fIrequired_position_args\\fP\n", cmd->name);
-			else if (cname->variant_has_ro && !cname->variant_has_rp)
-				printf("\\fB%s\\fP \\fIrequired_option_args\\fP\n", cmd->name);
-			else if (!cname->variant_has_ro && cname->variant_has_rp)
-				printf("\\fB%s\\fP \\fIrequired_position_args\\fP\n", cmd->name);
-			else if (!cname->variant_has_ro && !cname->variant_has_rp)
-				printf("\\fB%s\\fP\n", cmd->name);
-
-			printf(".br\n");
-
-			if (cname->variant_has_oo) {
-				printf("    [ \\fIoptional_option_args\\fP ]\n");
-				printf(".br\n");
-			}
-
-			if (cname->variant_has_op) {
-				printf("    [ \\fIoptional_position_args\\fP ]\n");
-				printf(".br\n");
-			}
-
-			printf(".P\n");
-			printf("\n");
-
-			/* listing them all when there's only 1 or 2 is just repetative */
-			if (cname->variants > 2) {
-				printf(".P\n");
-				print_man_all_options(cname);
-				printf("\n");
-				printf(".P\n");
-				printf("\n");
-			}
-
-			printf(".SH USAGE\n");
-			printf(".br\n");
-			printf(".P\n");
-			printf(".\n");
+			last_cmd_name = cmd->name;
 		}
 
 		if (cmd->desc) {
@@ -2301,7 +2029,7 @@ void print_man_command(void)
 
 		print_man_usage(cmd);
 
-		if (i == (cmd_count - 1)) {
+		if ((i == (cmd_count - 1)) || strcmp(cmd->name, cmd_array[i+1].name)) {
 			printf("Common options:\n");
 			printf(".\n");
 			print_man_usage_common(cmd);
@@ -2342,10 +2070,8 @@ void print_command_struct(int only_usage)
 		printf("commands[%d].oo_count = %d;\n", i, cmd->oo_count);
 		printf("commands[%d].op_count = %d;\n", i, cmd->op_count);
 
-		if (cmd->cmd_flags)
-			printf("commands[%d].cmd_flags = %s;\n", i, cmd_flags_to_str(cmd->cmd_flags));
-		else
-			printf("commands[%d].cmd_flags = 0;\n", i, cmd_flags_to_str(cmd->cmd_flags));
+		if (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT)
+			printf("commands[%d].cmd_flags = CMD_FLAG_ONE_REQUIRED_OPT;\n", i);
 
 		printf("commands[%d].desc = \"%s\";\n", i, cmd->desc ?: "");
 		printf("commands[%d].usage = ", i);
@@ -2600,9 +2326,6 @@ int main(int argc, char *argv[])
 	static struct option long_options[] = {
 		{"help",      no_argument,       0, 'h' },
 		{"output",    required_argument, 0, 'o' },
-		{"man-primary", required_argument, 0, 'p' },
-		{"man-secondary", required_argument, 0, 's' },
-		{"man-command", required_argument, 0, 'c' },
 		{0, 0, 0, 0 }
 	};
 
@@ -2610,7 +2333,7 @@ int main(int argc, char *argv[])
 		int c;
 		int option_index = 0;
 
-		c = getopt_long(argc, argv, "ho:p:s:c:",
+		c = getopt_long(argc, argv, "ho:",
 				long_options, &option_index);
 		if (c == -1)
 			break;
@@ -2624,15 +2347,6 @@ int main(int argc, char *argv[])
 		case 'o':
 			outputformat = strdup(optarg);
 			break;
-		case 'p':
-			include_man_primary = atoi(optarg);
-			break;
-		case 's':
-			include_man_secondary = atoi(optarg);
-			break;
-		case 'c':
-			man_command_name = strdup(optarg);
-			break;
 		}
 	}
 
@@ -2695,11 +2409,6 @@ int main(int argc, char *argv[])
 			continue;
 		}
 
-		if (is_flags_line(line_argv[0])) {
-			add_flags(cmd, line_orig);
-			continue;
-		}
-
 		if (is_id_line(line_argv[0])) {
 			cmd->command_line_id = strdup(line_argv[1]);
 			continue;

tools/lvconvert.c

@@ -1364,8 +1364,7 @@ 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,
-				  struct dm_list *pvh)
+				  uint32_t new_log_count)
 {
 	uint32_t region_size;
 	struct lv_segment *seg = first_seg(lv);
@@ -1385,7 +1384,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 						  vg_is_clustered(lv->vg));
 
 	if (!operable_pvs)
-		operable_pvs = pvh;
+		operable_pvs = lp->pvh;
 
 	/*
 	 * Up-convert from linear to mirror
@@ -1394,7 +1393,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 		/* FIXME Share code with lvcreate */
 
 		/*
-		 * FIXME should we give not only pvh, but also all PVs
+		 * FIXME should we give not only lp->pvh, but also all PVs
 		 * currently taken by the mirror? Would make more sense from
 		 * user perspective.
 		 */
@@ -1403,7 +1402,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 				    lp->alloc, MIRROR_BY_LV))
 			return_0;
 
-		if (!arg_is_set(cmd, background_ARG))
+		if (lp->wait_completion)
 			lp->need_polling = 1;
 
 		goto out;
@@ -1580,8 +1579,7 @@ 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 dm_list *pvh)
+				     struct lvconvert_params *lp)
 {
 	int failed_logs;
 	int failed_mimages;
@@ -1592,6 +1590,7 @@ 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;
 
@@ -1647,7 +1646,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, pvh))
+					   lp->mirrors, log_count))
 			break;
 		if (lp->mirrors > 2)
 			--lp->mirrors;
@@ -1767,8 +1766,11 @@ 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, lp->pvh))
+				    new_mimage_count, new_log_count))
 		return 0;
 
 	if (!lp->need_polling)
@@ -1799,6 +1801,33 @@ 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;
@@ -1934,6 +1963,49 @@ 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. */
@@ -2437,7 +2509,7 @@ out:
 
 static int _lvconvert_thin_pool_repair(struct cmd_context *cmd,
 				       struct logical_volume *pool_lv,
-				       struct dm_list *pvh, int poolmetadataspare)
+				       struct lvconvert_params *lp)
 {
 	const char *dmdir = dm_dir();
 	const char *thin_dump =
@@ -2466,7 +2538,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, pvh, 1))
+	if (!handle_pool_metadata_spare(pool_lv->vg, 0, lp->pvh, 1))
 		return_0;
 
 	if (pmslv != pool_lv->vg->pool_metadata_spare_lv) {
@@ -2591,8 +2663,8 @@ deactivate_pmslv:
 	}
 
 	/* Try to allocate new pool metadata spare LV */
-	if (!handle_pool_metadata_spare(pool_lv->vg, 0, pvh,
-					poolmetadataspare))
+	if (!handle_pool_metadata_spare(pool_lv->vg, 0, lp->pvh,
+					lp->poolmetadataspare))
 		stack;
 
 	if (dm_snprintf(meta_path, sizeof(meta_path), "%s_meta%%d", pool_lv->name) < 0) {
@@ -3471,8 +3543,7 @@ 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 0;
+	return _lvconvert_thin_pool_repair(cmd, lv, lp);
 }
 
 /*
@@ -3660,7 +3731,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, lp->pvh);
+	ret = _lvconvert_mirrors_repair(cmd, lv, lp);
 
 	if (ret && arg_is_set(cmd, usepolicies_ARG)) {
 		if ((failed_pvs = _failed_pv_list(lv->vg)))
@@ -4640,305 +4711,3 @@ 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

@@ -85,10 +85,6 @@ struct command_name command_names[MAX_COMMAND_NAMES] = {
 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
  *
@@ -97,40 +93,12 @@ int lvconvert_repair_pvs_or_thinpool_fn(struct cmd_context *cmd, int argc, char
  * 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 },
-
-	/* 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 },
+	{ lvmconfig_general_CMD, lvmconfig },
 };
-
 #if 0
-	/* all raid-related type conversions */
-
-	{ lvconvert_raid_types_CMD,			lvconvert_raid_types_fn },
-
-	/* 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 },
-
-	/* 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 },
-	{ lvconvert_to_thinpool_CMD,			lvconvert_to_thinpool_fn },
-	{ lvconvert_to_cachepool_CMD,			lvconvert_to_cachepool_fn },
-	{ lvconvert_split_and_keep_cachepool_CMD,	lvconvert_split_and_keep_cachepool_fn },
-	{ lvconvert_split_and_delete_cachepool_CMD,	lvconvert_split_and_delete_cachepool_fn },
-	{ lvconvert_swap_pool_metadata_CMD,		lvconvert_swap_pool_metadata_fn },
-
-	/* lvconvert: utilities related to snapshots and repair. */
-
-	{ lvconvert_merge_CMD,				lvconvert_merge_fn },
-	{ lvconvert_combine_split_snapshot_CMD,		lvconvert_combine_split_snapshot_fn },
-	{ lvconvert_split_cow_snapshot_CMD,		lvconvert_split_cow_snapshot_fn },
-	{ lvconvert_poll_start_CMD,			lvconvert_poll_start_fn },
-
+	{ lvchange_properties_CMD, lvchange_properties_cmd },
+	{ lvchange_activate_CMD, lvchange_activate_cmd },
+	{ lvchange_refresh_CMD, lvchange_refresh_cmd },
 #endif
 
 /* Command line args */
@@ -765,105 +733,23 @@ int readahead_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_va
 /*
  * Non-zero, positive integer, "all", or "unmanaged"
  */
-int vgmetadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
+int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
 {
-	if (!strcasecmp(av->value, "all")) {
-		av->ui_value = VGMETADATACOPIES_ALL;
-		return 1;
-	}
+	if (!strncmp(cmd->name, "vg", 2)) {
+		if (!strcasecmp(av->value, "all")) {
+			av->ui_value = VGMETADATACOPIES_ALL;
+			return 1;
+		}
 
-	if (!strcasecmp(av->value, "unmanaged")) {
-		av->ui_value = VGMETADATACOPIES_UNMANAGED;
-		return 1;
+		if (!strcasecmp(av->value, "unmanaged")) {
+			av->ui_value = VGMETADATACOPIES_UNMANAGED;
+			return 1;
+		}
 	}
 
 	return int_arg(cmd, av);
 }
 
-int pvmetadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	int num;
-
-	if (!int_arg(cmd, av))
-		return 0;
-
-	num = av->i_value;
-
-	if ((num != 0) && (num != 1) && (num != 2))
-		return 0;
-
-	return 1;
-}
-
-int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strncmp(cmd->name, "pv", 2))
-		return pvmetadatacopies_arg(cmd, av);
-	if (!strncmp(cmd->name, "vg", 2))
-		return vgmetadatacopies_arg(cmd, av);
-	return 0;
-}
-
-int polloperation_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "pvmove") ||
-	    !strcmp(av->value, "convert") ||
-	    !strcmp(av->value, "merge") ||
-	    !strcmp(av->value, "merge_thin"))
-		return 1;
-	return 0;
-}
-
-int writemostly_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	/* Could we verify that a PV arg looks like /dev/foo ? */
-	return 1;
-}
-
-int syncaction_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "check") ||
-	    !strcmp(av->value, "repair"))
-		return 1;
-	return 0;
-}
-
-int reportformat_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "basic") ||
-	    !strcmp(av->value, "json"))
-		return 1;
-	return 0;
-}
-
-int configreport_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "log") ||
-	    !strcmp(av->value, "vg") ||
-	    !strcmp(av->value, "lv") ||
-	    !strcmp(av->value, "pv") ||
-	    !strcmp(av->value, "pvseg") ||
-	    !strcmp(av->value, "seg"))
-		return 1;
-	return 0;
-}
-
-int configtype_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "current") ||
-	    !strcmp(av->value, "default") ||
-	    !strcmp(av->value, "diff") ||
-	    !strcmp(av->value, "full") ||
-	    !strcmp(av->value, "list") ||
-	    !strcmp(av->value, "missing") ||
-	    !strcmp(av->value, "new") ||
-	    !strcmp(av->value, "profilable") ||
-	    !strcmp(av->value, "profilable-command") ||
-	    !strcmp(av->value, "profilable-metadata"))
-		return 1;
-	return 0;
-}
-
 /*
  * FIXME: there's been a confusing mixup among:
  * resizeable, resizable, allocatable, allocation.
@@ -904,14 +790,6 @@ static int _opt_standard_to_synonym(const char *cmd_name, int opt)
 		return raidwritebehind_ARG;
 	case virtualsize_ARG:
 		return virtualoriginsize_ARG;
-	case pvmetadatacopies_ARG:
-		if (!strncmp(cmd_name, "pv", 2))
-			return metadatacopies_ARG;
-		return 0;
-	case vgmetadatacopies_ARG:
-		if (!strncmp(cmd_name, "vg", 2))
-			return metadatacopies_ARG;
-		return 0;
 	}
 	return 0;
 }
@@ -941,12 +819,6 @@ static int _opt_synonym_to_standard(const char *cmd_name, int opt)
 		return writebehind_ARG;
 	case virtualoriginsize_ARG:
 		return virtualsize_ARG;
-	case metadatacopies_ARG:
-		if (!strncmp(cmd_name, "pv", 2))
-			return pvmetadatacopies_ARG;
-		if (!strncmp(cmd_name, "vg", 2))
-			return vgmetadatacopies_ARG;
-		return 0;
 	}
 	return 0;
 }
@@ -1411,7 +1283,7 @@ static void _print_description(int ci)
  * set to match.
  *
  * required_pos_args[0].types & select_VAL means
- * argv[] in that pos can be NULL if arg_is_set(select_ARG)
+ * cmd->argv[] in that pos can be NULL if arg_is_set(select_ARG)
  */
 
 /* The max number of unused options we keep track of to warn about */
@@ -1617,14 +1489,16 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	}
 
 	/*
-	 * If the user provided a positional arg that is not accepted by
-	 * the mached command, then fail.
+	 * Warn about positional args that are set but are not used by the command.
 	 *
 	 * If the last required_pos_arg or the last optional_pos_arg may repeat,
 	 * then there won't be unused positional args.
 	 *
-	 * FIXME: same question as above, should there be a config setting
-	 * to just warn/ignore about unused positional args?
+	 * Otherwise, warn about positional args that exist beyond the number of
+	 * required + optional pos_args.
+	 *
+	 * FIXME: should an unused positional arg cause the command to fail
+	 * like an unused option?
 	 */
 
 	count = commands[best_i].rp_count;
@@ -1640,15 +1514,10 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			break;
 
 		if (count >= (commands[best_i].rp_count + commands[best_i].op_count)) {
-			log_error("Invalid positional argument for command (%s %d): %s.",
-				  commands[best_i].command_line_id, best_i, argv[count]);
-
-			/* FIXME: to warn/ignore, clear so it can't be used when processing. */
-			/*
+			log_warn("Ignoring positional argument which is not used by this command: %s.", argv[count]);
+			/* clear so it can't be used when processing. */
 			argv[count] = NULL;
 			(*argc)--;
-			*/
-			return NULL;
 		}
 	}
 out:
@@ -1679,9 +1548,6 @@ static int _usage(const char *name, int help_count)
 		if (strcmp(_cmdline.commands[i].name, name))
 			continue;
 
-		if ((_cmdline.commands[i].cmd_flags & CMD_FLAG_SECONDARY_SYNTAX) && (help_count < 3))
-			continue;
-
 		if (strlen(_cmdline.commands[i].desc))
 			_print_description(i);
 
@@ -1718,11 +1584,6 @@ static int _usage(const char *name, int help_count)
 		log_print(". The _new suffix indicates the VG or LV must not yet exist.");
 		log_print(". LV followed by _<type> indicates that an LV of the given type");
 		log_print("  is required.  (raid represents any raid<N> type.)");
-		log_print(". The default output unit is specified by letter, followed by |unit");
-		log_print("  which represents other possible units: hHbBsSkKmMgGtTpPeE.");
-		log_print(". Output units are 1024 SI base, regardless of unit capitalization.");
-		log_print(". Use --help --help --help to print secondary command syntax");
-		log_print("  formats that are recognized, e.g. for compatibility.");
 		log_print(". See man pages for short option equivalents of long option names,");
 		log_print("  and for more detailed descriptions of variable parameters.");
 	}
@@ -2549,12 +2410,6 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 	if (!(cmd->command = _find_command(cmd, cmd->name, &argc, argv)))
 		return EINVALID_CMD_LINE;
 
-	/*
-	 * Remaining position args after command name and --options are removed.
-	 */
-	cmd->position_argc = argc;
-	cmd->position_argv = argv;
-
 	set_cmd_name(cmd->name);
 
 	if (arg_is_set(cmd, backgroundfork_ARG)) {

tools/toollib.c

@@ -801,7 +801,10 @@ int vgcreate_params_set_from_args(struct cmd_context *cmd,
 		return 0;
 	}
 
-	if (arg_is_set(cmd, vgmetadatacopies_ARG))
+	if (arg_is_set(cmd, metadatacopies_ARG))
+		vp_new->vgmetadatacopies = arg_int_value(cmd, metadatacopies_ARG,
+							DEFAULT_VGMETADATACOPIES);
+	else if (arg_is_set(cmd, vgmetadatacopies_ARG))
 		vp_new->vgmetadatacopies = arg_int_value(cmd, vgmetadatacopies_ARG,
 							DEFAULT_VGMETADATACOPIES);
 	else
@@ -2674,8 +2677,7 @@ 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)
@@ -2796,134 +2798,6 @@ 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,
@@ -3086,19 +2960,6 @@ 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;
@@ -4143,6 +4004,11 @@ int pvcreate_params_from_args(struct cmd_context *cmd, struct pvcreate_params *p
 	if (pp->pva.pvmetadatacopies < 0)
 		pp->pva.pvmetadatacopies = find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL);
 
+	if (pp->pva.pvmetadatacopies > 2) {
+		log_error("Metadatacopies may only be 0, 1 or 2");
+		return 0;
+	}
+
 	pp->pva.ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, pp->pva.ba_size);
 
 	return 1;

tools/tools.h

@@ -159,15 +159,7 @@ int segtype_arg(struct cmd_context *cmd, struct arg_values *av);
 int alloc_arg(struct cmd_context *cmd, struct arg_values *av);
 int locktype_arg(struct cmd_context *cmd, struct arg_values *av);
 int readahead_arg(struct cmd_context *cmd, struct arg_values *av);
-int vgmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int pvmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
 int metadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int polloperation_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int writemostly_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int syncaction_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int reportformat_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int configreport_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
-int configtype_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
 
 /* we use the enums to access the switches */
 unsigned arg_count(const struct cmd_context *cmd, int a);

tools/vals.h

@@ -57,13 +57,11 @@
  * words is not defined or stored in a consistent way,
  * but is just whatever the parsing function happens to look
  * for, so adding a new accepted value for the val type is
- * generally making the parsing function recognize a new
- * word, and making the implementation code also recognize
- * that word to do something different.  This new word should
- * then also be added to the usage string for the val type here.
- * It would be nice if the accepted values could be defined in a
- * more consistent way, and perhaps in a single place, perhaps in
- * struct val_props.
+ * often just making the parsing function recognize a new
+ * word.  This new word should then also be added to the
+ * usage string for the val type here.  It would be nice
+ * if the accepted values could be defined in a more
+ * consistent way, perhaps in struct val_props.
  *
  * The usage text for an option is not always the full
  * set of words accepted for an option, but may be a
@@ -94,9 +92,9 @@
  * should avoid suggesting the upper case letters.
  */
 
-val(none_VAL, NULL, "None", "ERR")             /* unused, for enum value 0 */
-val(conststr_VAL, NULL, "ConstString", "ERR")  /* used only for command defs */
-val(constnum_VAL, NULL, "ConstNumber", "ERR")  /* used only for command defs */
+val(none_VAL, NULL, "None", "")             /* unused, for enum value 0 */
+val(conststr_VAL, NULL, "ConstString", "")  /* used only for command defs */
+val(constnum_VAL, NULL, "ConstNumber", "")  /* used only for command defs */
 val(bool_VAL, yes_no_arg, "Bool", "y|n")
 val(number_VAL, int_arg, "Number", NULL)
 val(string_VAL, string_arg, "String", NULL)
@@ -113,7 +111,7 @@ val(mirrorlog_VAL, mirrorlog_arg, "MirrorLog", "core|disk")
 val(sizekb_VAL, size_kb_arg, "SizeKB", "Number[k|unit]")
 val(sizemb_VAL, size_mb_arg, "SizeMB", "Number[m|unit]")
 val(numsigned_VAL, int_arg_with_sign, "SNumber", "[+|-]Number")
-val(numsignedper_VAL, int_arg_with_sign_and_percent, "SNumberP", "[+|-]Number[%VG|%PVS|%FREE]")
+val(numsignedper_VAL, int_arg_with_sign_and_percent, "SNumberP", "[+|-]Number[%{VG|PVS|FREE}]")
 val(permission_VAL, permission_arg, "Permission", "rw|r")
 val(metadatatype_VAL, metadatatype_arg, "MetadataType", "lvm2|lvm1")
 val(units_VAL, string_arg, "Units", "hHbBsSkKmMgGtTpPeE")
@@ -121,16 +119,17 @@ val(segtype_VAL, segtype_arg, "SegType", "linear|striped|snapshot|mirror|raid*|t
 val(alloc_VAL, alloc_arg, "Alloc", "contiguous|cling|cling_by_tags|normal|anywhere|inherit")
 val(locktype_VAL, locktype_arg, "LockType", "sanlock|dlm|none")
 val(readahead_VAL, readahead_arg, "Readahead", "auto|none|NumberSectors")
-val(vgmetadatacopies_VAL, vgmetadatacopies_arg, "MetadataCopiesVG", "all|unmanaged|Number")
-val(pvmetadatacopies_VAL, pvmetadatacopies_arg, "MetadataCopiesPV", "0|1|2")
-val(metadatacopies_VAL, metadatacopies_arg, "unused", "unused")
-val(polloperation_VAL, polloperation_arg, "PollOp", "pvmove|convert|merge|merge_thin")
-val(writemostly_VAL, writemostly_arg, "WriteMostlyPV", "PV[:t|n|y]")
-val(syncaction_VAL, syncaction_arg, "SyncAction", "check|repair")
-val(reportformat_VAL, reportformat_arg, "ReportFmt", "basic|json")
-val(configreport_VAL, configreport_arg, "ConfigReport", "log|vg|lv|pv|pvseg|seg")
-val(configtype_VAL, configtype_arg, "ConfigType", "current|default|diff|full|list|missing|new|profilable|profilable-command|profilable-metadata")
+val(metadatacopies_VAL, metadatacopies_arg, "MetadataCopies", "all|unmanaged|Number")
 
 /* this should always be last */
 val(VAL_COUNT, NULL, NULL, NULL)
 
+/*
+ * FIXME: I suspect many of the following are good candidates for a custom VAL
+ * enum for the benefit of custom parsing, or custom usage, or both:
+ *
+ * configreport_ARG, configtype_ARG, polloperation_ARG, raidrebuild_ARG,
+ * raidsyncaction_ARG, raidwritemostly_ARG, reportformat_ARG, syncaction_ARG,
+ * cachepolicy_ARG, cachesettings_ARG, writemostly_ARG
+ */
+

tools/vgchange.c

@@ -482,9 +482,6 @@ static int _vgchange_metadata_copies(struct cmd_context *cmd,
 {
 	uint32_t mda_copies = arg_uint_value(cmd, vgmetadatacopies_ARG, DEFAULT_VGMETADATACOPIES);
 
-	log_warn("vgchange_metadata_copies new %u vg_mda_copies %u D %u",
-		 mda_copies, vg_mda_copies(vg), DEFAULT_VGMETADATACOPIES);
-
 	if (mda_copies == vg_mda_copies(vg)) {
 		if (vg_mda_copies(vg) == VGMETADATACOPIES_UNMANAGED)
 			log_warn("Number of metadata copies for VG %s is already unmanaged.",

tools/vgconvert.c

@@ -157,12 +157,24 @@ int vgconvert(struct cmd_context *cmd, int argc, char **argv)
 		return EINVALID_CMD_LINE;
 	}
 
+	if (arg_is_set(cmd, metadatacopies_ARG)) {
+		log_error("Invalid option --metadatacopies, "
+			  "use --pvmetadatacopies instead.");
+		return EINVALID_CMD_LINE;
+	}
 	if (!(cmd->fmt->features & FMT_MDAS) &&
-	    arg_is_set(cmd, pvmetadatacopies_ARG)) {
+	    (arg_is_set(cmd, pvmetadatacopies_ARG) ||
+	     arg_is_set(cmd, metadatasize_ARG))) {
 		log_error("Metadata parameters only apply to text format");
 		return EINVALID_CMD_LINE;
 	}
 
+	if (arg_is_set(cmd, pvmetadatacopies_ARG) &&
+	    arg_int_value(cmd, pvmetadatacopies_ARG, -1) > 2) {
+		log_error("Metadatacopies may only be 0, 1 or 2");
+		return EINVALID_CMD_LINE;
+	}
+
 	if (!(cmd->fmt->features & FMT_BAS) &&
 		arg_is_set(cmd, bootloaderareasize_ARG)) {
 		log_error("Bootloader area parameters only apply to text format");

tools/vgextend.c

@@ -136,6 +136,12 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv)
 		return EINVALID_CMD_LINE;
 	}
 
+	if (arg_is_set(cmd, metadatacopies_ARG)) {
+		log_error("Invalid option --metadatacopies, "
+			  "use --pvmetadatacopies instead.");
+		return EINVALID_CMD_LINE;
+	}
+
 	vg_name = skip_dev_dir(cmd, argv[0], NULL);
 	argc--;
 	argv++;