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, --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
ID: lvresize_by_pv
DESC: Resize an LV by specified PV extents.
FLAGS: SECONDARY_SYNTAX

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

Command definitions that are not to be advertised/suggested
have the flag SECONDARY_SYNTAX.  These commands will not be
printed in the normal help output.

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,6 +94,13 @@ 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.
 	 */

lib/metadata/lv_manip.c

@@ -319,7 +319,6 @@ static int _lv_layout_and_role_thin(struct dm_pool *mem,
 {
 	int top_level = 0;
 	unsigned snap_count;
-	struct lv_segment *seg;
 
 	/* non-top-level LVs */
 	if (lv_is_thin_pool_metadata(lv)) {
@@ -353,7 +352,7 @@ static int _lv_layout_and_role_thin(struct dm_pool *mem,
 			    !str_list_add_no_dup_check(mem, role, _lv_type_names[LV_TYPE_MULTITHINORIGIN]))
 				goto_bad;
 		}
-		if ((seg = first_seg(lv)) && (seg->origin || seg->external_lv))
+		if (lv_is_thin_snapshot(lv))
 			if (!str_list_add(mem, role, _lv_type_names[LV_TYPE_SNAPSHOT]) ||
 			    !str_list_add_no_dup_check(mem, role, _lv_type_names[LV_TYPE_THINSNAPSHOT]))
 				goto_bad;

lib/metadata/metadata-exported.h

@@ -1066,9 +1066,16 @@ struct lv_segment *get_only_segment_using_this_lv(const struct logical_volume *l
 * Useful functions for managing snapshots.
 */
 int lv_is_origin(const struct logical_volume *lv);
+#define lv_is_thick_origin lv_is_origin
+
 int lv_is_thin_origin(const struct logical_volume *lv, unsigned *snap_count);
-int lv_is_cache_origin(const struct logical_volume *lv);
+int lv_is_thin_snapshot(const struct logical_volume *lv);
+
 int lv_is_cow(const struct logical_volume *lv);
+#define lv_is_thick_snapshot lv_is_cow
+
+int lv_is_cache_origin(const struct logical_volume *lv);
+
 int lv_is_merging_cow(const struct logical_volume *cow);
 uint32_t cow_max_extents(const struct logical_volume *origin, uint32_t chunk_size);
 int cow_has_min_chunks(const struct volume_group *vg, uint32_t cow_extents, uint32_t chunk_size);

lib/metadata/thin_manip.c

@@ -748,6 +748,19 @@ int lv_is_thin_origin(const struct logical_volume *lv, unsigned int *snap_count)
 	return r;
 }
 
+int lv_is_thin_snapshot(const struct logical_volume *lv)
+{
+	struct lv_segment *seg;
+
+	if (!lv_is_thin_volume(lv))
+		return 0;
+
+	if ((seg = first_seg(lv)) && (seg->origin || seg->external_lv))
+		return 1;
+
+	return 0;
+}
+
 /*
  * Explict check of new thin pool for usability
  *

man/Makefile.in

@@ -140,6 +140,13 @@ 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)/

tools/args.h

@@ -17,6 +17,8 @@
  * 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)
@@ -31,8 +33,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", string_VAL, ARG_GROUPABLE, 1)
-arg(configtype_ARG, '\0', "typeconfig", string_VAL, 0, 0)
+arg(configreport_ARG, '\0', "configreport", configreport_VAL, ARG_GROUPABLE, 1)
+arg(configtype_ARG, '\0', "typeconfig", configtype_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 +83,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", string_VAL, 0, 0)
+arg(polloperation_ARG, '\0', "polloperation", polloperation_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", string_VAL, ARG_GROUPABLE, 0)
+arg(raidrebuild_ARG, '\0', "raidrebuild", pv_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", string_VAL, 0, 0)
+arg(raidsyncaction_ARG, '\0', "raidsyncaction", syncaction_VAL, 0, 0)
 arg(raidwritebehind_ARG, '\0', "raidwritebehind", number_VAL, 0, 0)
-arg(raidwritemostly_ARG, '\0', "raidwritemostly", string_VAL, ARG_GROUPABLE, 0)
+arg(raidwritemostly_ARG, '\0', "raidwritemostly", writemostly_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", string_VAL, 0, 0)
+arg(reportformat_ARG, '\0', "reportformat", reportformat_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 +118,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", string_VAL, 0, 0) /* FIXME Use custom VAL */
+arg(syncaction_ARG, '\0', "syncaction", syncaction_VAL, 0, 0)
 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)
@@ -140,7 +142,7 @@ 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", string_VAL, ARG_GROUPABLE, 0)
+arg(writemostly_ARG, '\0', "writemostly", writemostly_VAL, ARG_GROUPABLE, 0)
 
 /* Allow some variations */
 arg(allocation_ARG, '\0', "allocation", bool_VAL, 0, 0)

tools/command-lines.in

@@ -83,6 +83,13 @@
 # Also, --thinpool VG/LV or --cachepool VG/LV can be used in
 # place of --name to provide the VG name instead of pos 1.
 #
+# 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.
+# There are no consistent rules to follow and the behaviors are
+# unpredictable; each possible variation and combination needs
+# to be tested individually to see what it means.
+#
 # Some options have multiple names, but only one form of the name
 # is used in these definitions.  Synonyms will be recognized when
 # matching a command to a command definition.
@@ -99,6 +106,8 @@
 # maxrecoveryrate (raidmaxrecoveryrate)
 # writebehind (raidwritebehind)
 # virtualsize (virtualoriginsize)
+# vgmetadatacopies (metadatacopies)
+# pvmetadatacopies (metadatacopies)
 #
 # "---" is like a comment line, used to separate text for readability
 #
@@ -113,25 +122,42 @@
 # included in the text as indicators of new lines when printing
 # the descriptions for help/man output.
 #
-# 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.
-# There are no consistent rules to follow and the behaviors are
-# unpredictable; each possible variation and combination needs
-# to be tested individually to see what it means.
+# RULE: rules that a given command must follow, i.e. required (and)
+# or invalid (not) combinations of options, LV types or LV properties.
 #
-# 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.
+# RULE: A and|not B
 #
-# if --foo is set, then --bar cannot be set could be encoded as:
-# RULE_OPT_INCOMPAT_OPT: --foo --bar
+# Conditions in A are applied to a given command+LV.
+# If the conditions in A are true, then the checks in B
+# are applied.  If the checks in B are true|false according
+# to and|not, then the command fails|continues.
 #
-# if --foo is set, then --bar is required could be encoded as:
-# RULE_OPT_REQUIRES_OPT: --foo --bar
+# When A is "all", the conditions in B are always applied.
+#
+# Conditions:
+# . if any --option listed is set, the checks may apply
+# . if any LV_type listed matches LV, the checks may apply
+# . if all lv_is_prop listed matches LV, the checks may apply
+# . if all of the above pass, then perform the checks
+#
+# Checks for "and":
+# . if any --option listed is not set, then fail
+# . if none of the LV_types matches the LV, then fail
+# . if any of the lv_is_prop do not match the LV, then fail
+#
+# Checks for "not":
+# . if any --option listed is set, then fail
+# . if any of the LV_types matches the LV, then fail
+# . if any of the lv_is_prop match the LV, then fail
+#
+# RULE: --option|LV_type|lv_is_prop|all ... and|not --option|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
 #
-# if --foo is set, then positional arg 1 is required
-# RULE_OPT_REQUIRES_POS: --foo 1
 #
 
 #
@@ -177,17 +203,17 @@ OO_USAGE_COMMON: OO_ALL, --force, --noudevsync
 #
 # options for pvs, lvs, vgs, fullreport
 #
-OO_REPORT: --aligned, --all, --binary, --configreport String, --foreign,
+OO_REPORT: --aligned, --all, --binary, --configreport ConfigReport, --foreign,
 --ignorelockingfailure, --ignoreskippedcluster, --logonly,
 --nameprefixes, --noheadings, --nolocking, --nosuffix,
---options String, --partial, --readonly, --reportformat String, --rows,
+--options String, --partial, --readonly, --reportformat ReportFmt, --rows,
 --select String, --separator String, --shared, --sort String,
 --trustcache, --unbuffered, --units Units, --unquoted
 
 #
 # options for config, dumpconfig, lvmconfig
 #
-OO_CONFIG: --atversion String, --typeconfig String, --file String, --ignoreadvanced,
+OO_CONFIG: --atversion String, --typeconfig ConfigType, --file String, --ignoreadvanced,
 --ignoreunsupported, --ignorelocal, --list, --mergedconfig, --metadataprofile String,
 --sinceversion String, --showdeprecated, --showunsupported, --validate, --withsummary,
 --withcomments, --withspaces, --unconfigured, --withversions
@@ -196,9 +222,9 @@ OO_CONFIG: --atversion String, --typeconfig String, --file String, --ignoreadvan
 
 # None of these can function as a required option for lvchange.
 
-OO_LVCHANGE: --autobackup Bool, --force, --ignorelockingfailure,
---ignoremonitoring, --ignoreskippedcluster, --noudevsync,
---reportformat String, --sysinit, --select String
+OO_LVCHANGE: --autobackup Bool, --force, --ignoremonitoring,
+--ignoreskippedcluster, --noudevsync, --reportformat ReportFmt,
+--select String
 
 # Any of these can function as a required option for lvchange.
 # profile is also part of OO_ALL, but is repeated in OO_LVCHANGE_META
@@ -211,30 +237,47 @@ OO_LVCHANGE_META: --addtag Tag, --deltag Tag,
 --errorwhenfull Bool, --discards Discards, --zero Bool,
 --cachemode CacheMode, --cachepolicy String, --cachesettings String,
 --minrecoveryrate SizeKB, --maxrecoveryrate SizeKB,
---writebehind Number, --writemostly PV
+--writebehind Number, --writemostly WriteMostlyPV, --persistent n
 
 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 lv_is_mirror_log lv_is_mirror_image
+RULE: all and lv_is_vg_writable
+RULE: --contiguous not --alloc
+RULE: --profile not --detachprofile
+RULE: --metadataprofile not --detachprofile
+RULE: --minrecoveryrate --maxrecoveryrate and LV_raid
+RULE: --writebehind --writemostly and LV_raid1
+RULE: --cachemode --cachepolicy --cachesettings and LV_cache LV_cachepool
+RULE: --errorwhenfull --discards --zero and LV_thinpool
+RULE: --permission not lv_is_external_origin lv_is_raid_metadata lv_is_raid_image LV_thinpool
+RULE: --alloc --contiguous --metadataprofile --permission --persistent --profile --readahead not lv_is_thick_origin
 
 lvchange --resync VG|LV_raid_mirror|Tag|Select ...
 OO: OO_LVCHANGE
 ID: lvchange_resync
 DESC: Resyncronize a mirror or raid LV.
+RULE: all not lv_is_pvmove lv_is_locked
+RULE: all not LV_raid0
 
-lvchange --syncaction String VG|LV_raid|Tag|Select ...
+lvchange --syncaction SyncAction VG|LV_raid|Tag|Select ...
 OO: OO_LVCHANGE
 ID: lvchange_syncaction
 DESC: Resynchronize or check a raid LV.
+RULE: all not LV_raid0
 
 lvchange --rebuild PV VG|LV_raid|Tag|Select ...
 OO: OO_LVCHANGE
 ID: lvchange_rebuild
 DESC: Reconstruct data on specific PVs of a raid LV.
+RULE: all not LV_raid0
 
+# try removing the META change options from here?
 lvchange --activate Active VG|LV|Tag|Select ...
-OO: --activationmode ActivationMode, --partial, --ignoreactivationskip, OO_LVCHANGE_META, OO_LVCHANGE
+OO: --activationmode ActivationMode, --partial, --ignoreactivationskip,
+--ignorelockingfailure, --sysinit, OO_LVCHANGE_META, OO_LVCHANGE
 ID: lvchange_activate
 DESC: Activate or deactivate an LV.
 
@@ -247,16 +290,18 @@ lvchange --monitor Bool VG|LV|Tag|Select ...
 OO: --poll Bool, OO_LVCHANGE
 ID: lvchange_monitor
 DESC: Start or stop monitoring an LV from dmeventd.
+RULE: all not lv_is_pvmove
 
 lvchange --poll Bool VG|LV|Tag|Select ...
 OO: --monitor Bool, OO_LVCHANGE
 ID: lvchange_poll
 DESC: Start or stop processing an LV conversion.
 
-lvchange --persistent Bool VG|LV|Tag|Select ...
-OO: --minor Number, --major Number, OO_LVCHANGE
+lvchange --persistent y --minor Number LV
+OO: --major Number, OO_LVCHANGE
 ID: lvchange_persistent
 DESC: Make the minor device number persistent for an LV.
+RULE: all not LV_thinpool LV_cachepool
 
 ---
 
@@ -266,63 +311,72 @@ OO_LVCONVERT_RAID: --mirrors SNumber, --stripes_long Number,
 OO_LVCONVERT_POOL: --poolmetadata LV, --poolmetadatasize SizeMB,
 --poolmetadataspare Bool, --readahead Readahead, --chunksize SizeKB
 
-OO_LVCONVERT: --alloc Alloc, --background, --force, --noudevsync,
---usepolicies
+OO_LVCONVERT: --alloc Alloc, --background, --force, --noudevsync
 
 ---
 
-# FIXME: lvconvert --merge is an extremely ambiguous command.
-# It can do very different operations, but which one depends
-# on knowing the LV type.  So, the command doesn't know what
-# it's actually doing until quite late, when processing a
-# single LV.
-#
-# Use different option names for different merge operations
-# so that we can have different command definitions,
-# different behaviors, different optional options, etc:
-#
-# lvconvert --merge-mirror LV_linear_striped_raid ...
-# DESC: Merge LV that was previously split from a mirror.
-#
-# lvconvert --merge-thin LV_thin
-# DESC: Merge thin LV into its origin LV.
-#
-# lvconvert --merge-snapshot LV_snapshot
-# DESC: Merge COW snapshot LV into its origin.
-#
-# Then we could add VG|Tag to --merge-mirror arg pos 1, because
-# "lvconvert --merge VG|Tag" is a terrible command.  It will do
-# different operations on each LV it finds, depending on the
-# current LV type.
+# These cover all the core, raid-related type conversions.
+# They are all routed into the core raid conversion code.
 
-lvconvert --merge LV_linear_striped_raid_thin_snapshot|VG|Tag ...
-OO: --background, --interval Number, OO_LVCONVERT
-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.
+lvconvert --type linear LV
+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
+OP: PV ...
+ID: lvconvert_raid_types
+DESC: Convert LV to striped.
+
+lvconvert --type mirror LV
+OO: OO_LVCONVERT_RAID, OO_LVCONVERT, --mirrorlog MirrorLog
+OP: PV ...
+ID: lvconvert_raid_types
+DESC: Convert LV to type mirror (also see type raid1).
+
+lvconvert --type raid LV
+OO: OO_LVCONVERT_RAID, OO_LVCONVERT
+OP: PV ...
+ID: lvconvert_raid_types
+DESC: Convert LV to raid.
+
+lvconvert --mirrors SNumber LV
+OO: OO_LVCONVERT_RAID, OO_LVCONVERT, --mirrorlog MirrorLog
+OP: PV ...
+ID: lvconvert_raid_types
+DESC: Convert LV to raid1 or mirror, or change number of mirror images.
 
 ---
 
-# FIXME: by using two different positional args, this is the
-# single violation of the standard method of using process_each_lv().
-# Before calling process_each, it steals the first positional arg
-# and adjusts argv/argc so it's not seen by process_each.
+# lvconvert raid-related utilities
+# Create a new command set for these and migrate them out of lvconvert?
 
-# alternate form of lvconvert --snapshot
-lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
-OO: --snapshot, --chunksize SizeKB, --zero Bool, OO_LVCONVERT
-ID: lvconvert_combine_split_snapshot
-DESC: Combine LV with a previously split snapshot LV.
-FLAGS: SECONDARY_SYNTAX
+lvconvert --splitmirrors Number --name LV_new LV_raid1_mirror_cache
+OO: OO_LVCONVERT
+OP: PV ...
+ID: lvconvert_split_mirror_images
+DESC: Split images from a raid1 or mirror LV and use them to create a new LV.
 
-lvconvert --snapshot LV_linear_striped_raid LV_snapshot
-OO: --type snapshot, --chunksize SizeKB, --zero Bool, OO_LVCONVERT
-ID: lvconvert_combine_split_snapshot
-DESC: Combine LV with a previously split snapshot LV.
+lvconvert --splitmirrors Number --trackchanges LV_raid1_cache
+OO: OO_LVCONVERT
+OP: PV ...
+ID: lvconvert_split_mirror_images
+DESC: Split images from a raid1 LV and track changes to origin.
+
+lvconvert --mirrorlog MirrorLog LV_mirror
+OO: OO_LVCONVERT
+OP: PV ...
+ID: lvconvert_change_mirrorlog
+DESC: Change the type of mirror log used by a mirror LV.
 
 ---
 
+# lvconvert utilities for creating/maintaining thin and cache objects.
+# Create a new command set for these and migrate them out of lvconvert?
+
 lvconvert --type thin --thinpool LV LV_linear_striped_raid
 OO: --thin, --originname LV_new, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
 ID: lvconvert_to_thin_with_external
@@ -357,6 +411,7 @@ FLAGS: SECONDARY_SYNTAX
 lvconvert --type thin-pool LV_linear_striped_raid_cache
 OO: --stripes_long Number, --stripesize SizeKB,
 --discards Discards, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
+OP: PV ...
 ID: lvconvert_to_thinpool
 DESC: Convert LV to type thin-pool.
 
@@ -365,6 +420,7 @@ DESC: Convert LV to type thin-pool.
 lvconvert --thinpool LV_linear_striped_raid_cache
 OO: --type thin-pool, --stripes_long Number, --stripesize SizeKB,
 --discards Discards, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
+OP: PV ...
 ID: lvconvert_to_thinpool
 DESC: Convert LV to type thin-pool (variant, use --type thin-pool).
 FLAGS: SECONDARY_SYNTAX
@@ -374,6 +430,7 @@ FLAGS: SECONDARY_SYNTAX
 lvconvert --type cache-pool LV_linear_striped_raid
 OO: OO_LVCONVERT_POOL, OO_LVCONVERT,
 --cachemode CacheMode, --cachepolicy String, --cachesettings String
+OP: PV ...
 ID: lvconvert_to_cachepool
 DESC: Convert LV to type cache-pool.
 
@@ -388,109 +445,83 @@ FLAGS: SECONDARY_SYNTAX
 
 ---
 
-# FIXME: we should be able to remove LV_mirror from the list of accepted
-# LV types, but there are some dubious commands in the test suite that
-# fail without it (the tests should be cleaned up to avoid using commands
-# that don't make sense.)
-#
-# FIXME: it would be nice to remove LV_raid1 from the list of accepted
-# LV types and let raid1 be covered by just the second definition, but
-# unfortunatley lvconvert --type mirror --mirrors N LV_raid1 will
-# match the first definition since the LV type cannot be used when
-# choosing a matching command definition.
-
-lvconvert --type mirror --mirrors SNumber LV_linear_striped_raid1_mirror
-OO: --mirrorlog MirrorLog, OO_LVCONVERT_RAID, OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_to_mirror
-DESC: Convert LV to type mirror.
-
-lvconvert --type mirror LV_raid1
-OO: --mirrors SNumber, OO_LVCONVERT_RAID, OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_to_mirror
-DESC: Convert LV from type raid1 to type mirror.
-
----
-
-lvconvert --type raid LV_linear_striped_mirror_raid
-OO: OO_LVCONVERT_RAID, OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_to_raid
-DESC: Convert LV to type raid.
-DESC: Change LV between raid types.
-
----
-
-# FIXME: there are two unique operations here that differ
-# by only the LV type, so they have to be defined together.
-# We don't know what this command is going to do until after
-# the LV is read.  It would be better if the alternate form
-# variant were dropped to remove the ambiguity
-# (then --type raid1|mirror would be required to change a
-# linear or striped LV to raid1|mirror.)
-#
-# First command definition
-# this changes the number of mirror images in a raid1|mirror LV.
-# lvconvert --mirrors SNumber LV_raid_mirror
-# ID: lvconvert_change_mirror_images
-# DESC: Change the number of mirror images in the LV.
-#
-# Second command definition
-# alternate form of: lvconvert --type raid1|mirror LV_linear_striped
-# lvconvert --mirrors SNumber LV_linear_striped
-# ID: lvconvert_to_raid1_or_mirror
-# DESC: Convert LV to type raid1 or mirror
-# DESC: (variant, infers --type raid1|mirror).
-
-# first def is unique, second def is alternate form of lvconvert --type raid1|mirror
-lvconvert --mirrors SNumber LV_raid_mirror_linear_striped
-OO: --type raid1, --type mirror, --mirrorlog MirrorLog, OO_LVCONVERT_RAID, OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_to_mirrored_or_change_image_count
-DESC: Change the number of mirror images in a raid1 or mirror LV.
-DESC: Convert a linear or striped LV to type raid1 or mirror
-DESC: (variant, infers --type raid1|mirror).
-
----
-
-lvconvert --type striped LV_raid
-OO: OO_LVCONVERT_RAID, OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_raid_to_striped
-DESC: Convert LV to type striped.
-
----
-
-lvconvert --type linear LV_raid_mirror
+lvconvert --splitcache LV_cachepool_cache_thinpool
 OO: OO_LVCONVERT
-ID: lvconvert_raid_or_mirror_to_linear
-DESC: Convert LV to type linear.
+ID: lvconvert_split_and_keep_cachepool
+DESC: Separate and keep the cache pool from a cache LV.
 
-# FIXME: the 'mirrors 0' trick as an alias for linear
-# is used inconsistently, confusing things and making
-# definitions difficult.
+---
 
-# alternate form of lvconvert --type linear
-lvconvert --mirrors 0 LV_raid_mirror
-OO: --type linear, --type mirror, OO_LVCONVERT
-ID: lvconvert_raid_or_mirror_to_linear
-DESC: Convert LV to type linear (variant, infers --type linear).
+lvconvert --uncache LV_cache_thinpool
+OO: OO_LVCONVERT
+ID: lvconvert_split_and_delete_cachepool
+DESC: Separate and delete the cache pool from a cache LV.
+
+---
+
+# FIXME: add a new option defining this operation, e.g. --swapmetadata
+
+lvconvert --poolmetadata LV LV_thinpool_cachepool
+OO: OO_LVCONVERT
+ID: lvconvert_swap_pool_metadata
+DESC: Swap metadata LV in a thin pool or cache pool (temporary command).
 FLAGS: SECONDARY_SYNTAX
 
 ---
 
-lvconvert --splitmirrors Number --name LV_new LV_raid1_mirror_cache
-OO: OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_split_mirror_images_to_new
-DESC: Split images from a raid1 or mirror LV and use them to create a new LV.
+# lvconvert utilities related to snapshots and repair.
+# Create a new command set for these and migrate them out of lvconvert?
 
-lvconvert --splitmirrors Number --trackchanges LV_raid1_cache
-OO: OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_split_mirror_images_and_track
-DESC: Split images from a raid1 LV and track changes to origin.
+# FIXME: lvconvert --merge is an extremely ambiguous command.
+# It can do very different operations, but which one depends
+# on knowing the LV type.  So, the command doesn't know what
+# it's actually doing until quite late, when processing a
+# single LV.
+#
+# Use different option names for different merge operations
+# so that we can have different command definitions,
+# different behaviors, different optional options, etc:
+#
+# lvconvert --merge-mirror LV_linear_striped_raid ...
+# DESC: Merge LV that was previously split from a mirror.
+#
+# lvconvert --merge-thin LV_thin
+# DESC: Merge thin LV into its origin LV.
+#
+# lvconvert --merge-snapshot LV_snapshot
+# DESC: Merge COW snapshot LV into its origin.
+#
+# Then we could add VG|Tag to --merge-mirror arg pos 1, because
+# "lvconvert --merge VG|Tag" is a terrible command.  It will do
+# different operations on each LV it finds, depending on the
+# current LV type.
+
+lvconvert --merge LV_linear_striped_raid_thin_snapshot|VG|Tag ...
+OO: --background, --interval Number, OO_LVCONVERT
+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
+
+---
+
+# FIXME: by using two different positional args, this is the
+# single violation of the standard method of using process_each_lv().
+# Before calling process_each, it steals the first positional arg
+# and adjusts argv/argc so it's not seen by process_each.
+
+# alternate form of lvconvert --snapshot
+lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
+OO: --snapshot, --chunksize SizeKB, --zero Bool, OO_LVCONVERT
+ID: lvconvert_combine_split_snapshot
+DESC: Combine LV with a previously split snapshot LV.
+FLAGS: SECONDARY_SYNTAX
+
+lvconvert --snapshot LV_linear_striped_raid LV_snapshot
+OO: --type snapshot, --chunksize SizeKB, --zero Bool, OO_LVCONVERT
+ID: lvconvert_combine_split_snapshot
+DESC: Combine LV with a previously split snapshot LV.
 
 ---
 
@@ -509,7 +540,8 @@ DESC: Split images from a raid1 LV and track changes to origin.
 # and the LV type is known.
 
 lvconvert --repair LV_raid_mirror_thinpool
-OO: OO_LVCONVERT
+OO: --usepolicies, OO_LVCONVERT
+OP: PV ...
 ID: lvconvert_repair_pvs_or_thinpool
 DESC: Replace failed PVs in a raid or mirror LV.
 DESC: Repair a thin pool.
@@ -524,28 +556,6 @@ DESC: Replace specific PV(s) in a raid* LV with another PV.
 
 ---
 
-lvconvert --mirrorlog MirrorLog LV_mirror
-OO: OO_LVCONVERT
-OP: PV ...
-ID: lvconvert_change_mirrorlog
-DESC: Change the type of mirror log used by a mirror LV.
-
----
-
-lvconvert --splitcache LV_cachepool_cache_thinpool
-OO: OO_LVCONVERT
-ID: lvconvert_split_and_keep_cachepool
-DESC: Separate and keep the cache pool from a cache LV.
-
----
-
-lvconvert --uncache LV_cache_thinpool
-OO: OO_LVCONVERT
-ID: lvconvert_split_and_delete_cachepool
-DESC: Separate and delete the cache pool from a cache LV.
-
----
-
 lvconvert --splitsnapshot LV_snapshot
 OO: OO_LVCONVERT
 ID: lvconvert_split_cow_snapshot
@@ -558,29 +568,19 @@ DESC: Separate a COW snapshot from its origin LV.
 
 lvconvert LV_mirror
 OO: OO_LVCONVERT
-ID: lvconvert_poll_mirror
+ID: lvconvert_poll_start
 DESC: Poll mirror LV to collapse resync layers.
 FLAGS: SECONDARY_SYNTAX
 
 ---
 
-# FIXME: add a new option defining this operation, e.g. --swapmetadata
-
-lvconvert --poolmetadata LV LV_thinpool_cachepool
-OO: OO_LVCONVERT
-ID: lvconvert_swap_pool_metadata
-DESC: Swap metadata LV in a thin pool or cache pool (temporary command).
-FLAGS: SECONDARY_SYNTAX
-
----
-
 # --extents is not specified; it's an automatic alternative for --size
 
 OO_LVCREATE: --addtag Tag, --alloc Alloc, --autobackup Bool, --activate Active,
 --contiguous Bool, --ignoreactivationskip, --ignoremonitoring, --major Number,
 --metadataprofile String, --minor Number, --monitor Bool, --name String, --nosync,
 --noudevsync, --permission Permission, --persistent Bool, --readahead Readahead,
---reportformat String, --setactivationskip Bool, --wipesignatures Bool,
+--reportformat ReportFmt, --setactivationskip Bool, --wipesignatures Bool,
 --zero Bool
 
 OO_LVCREATE_CACHE: --cachemode CacheMode, --cachepolicy String, --cachesettings String
@@ -1005,10 +1005,10 @@ FLAGS: SECONDARY_SYNTAX
 
 lvdisplay
 OO: --aligned, --all, --binary, --colon, --columns,
---configreport String, --foreign, --history, --ignorelockingfailure,
+--configreport ConfigReport, --foreign, --history, --ignorelockingfailure,
 --ignoreskippedcluster, --logonly, --maps, --noheadings,
 --nosuffix, --options String, --sort String, --partial, --readonly,
---reportformat String, --segments, --select String, --separator String,
+--reportformat ReportFmt, --segments, --select String, --separator String,
 --shared, --unbuffered, --units Units
 OP: VG|LV|Tag ...
 ID: lvdisplay_general
@@ -1019,7 +1019,7 @@ ID: lvdisplay_general
 
 lvextend --size SizeMB LV
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
---nofsck, --nosync, --noudevsync, --reportformat String, --resizefs,
+--nofsck, --nosync, --noudevsync, --reportformat ReportFmt, --resizefs,
 --stripes Number, --stripesize SizeKB, --poolmetadatasize SizeMB
 OP: PV ...
 ID: lvextend_by_size
@@ -1028,7 +1028,7 @@ DESC: Extend an LV by a specified size.
 lvextend LV PV ...
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --nofsck, --nosync, --noudevsync,
---reportformat String, --resizefs, --stripes Number, --stripesize SizeKB
+--reportformat ReportFmt, --resizefs, --stripes Number, --stripesize SizeKB
 ID: lvextend_by_pv
 DESC: Extend an LV by specified PV extents.
 FLAGS: SECONDARY_SYNTAX
@@ -1036,7 +1036,7 @@ FLAGS: SECONDARY_SYNTAX
 lvextend --poolmetadatasize SizeMB LV_thinpool
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --nofsck, --nosync, --noudevsync,
---reportformat String, --stripes Number, --stripesize SizeKB
+--reportformat ReportFmt, --stripes Number, --stripesize SizeKB
 OP: PV ...
 ID: lvextend_pool_metadata_by_size
 DESC: Extend a pool metadata SubLV by a specified size.
@@ -1044,7 +1044,8 @@ DESC: Extend a pool metadata SubLV by a specified size.
 lvextend --usepolicies LV_thinpool_snapshot
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --nofsck, --nosync, --noudevsync,
---reportformat String, --resizefs
+--reportformat ReportFmt, --resizefs
+OP: PV ...
 ID: lvextend_by_policy
 DESC: Extend an LV according to a predefined policy.
 
@@ -1052,37 +1053,38 @@ DESC: Extend an LV according to a predefined policy.
 
 lvmconfig
 OO: OO_CONFIG
+OP: String ...
 ID: lvmconfig_general
 
 ---
 
 lvreduce --size SizeMB LV
 OO: --autobackup Bool, --force, --nofsck, --noudevsync,
---reportformat String, --resizefs
+--reportformat ReportFmt, --resizefs
 ID: lvreduce_general
 
 ---
 
 lvremove VG|LV|Tag|Select ...
 OO: --autobackup Bool, --force, --nohistory, --noudevsync,
---reportformat String, --select String
+--reportformat ReportFmt, --select String
 ID: lvremove_general
 
 ---
 
 lvrename VG LV LV_new
-OO: --autobackup Bool, --noudevsync, --reportformat String
+OO: --autobackup Bool, --noudevsync, --reportformat ReportFmt
 ID: lvrename_vg_lv_lv
 
 lvrename LV LV_new
-OO: --autobackup Bool, --noudevsync, --reportformat String
+OO: --autobackup Bool, --noudevsync, --reportformat ReportFmt
 ID: lvrename_lv_lv
 
 ---
 
 lvresize --size SizeMB LV
 OO: --alloc Alloc, --autobackup Bool, --force,
---nofsck, --nosync, --noudevsync, --reportformat String, --resizefs,
+--nofsck, --nosync, --noudevsync, --reportformat ReportFmt, --resizefs,
 --stripes Number, --stripesize SizeKB, --poolmetadatasize SizeMB
 OP: PV ...
 ID: lvresize_by_size
@@ -1091,7 +1093,7 @@ 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
+--reportformat ReportFmt, --resizefs, --stripes Number, --stripesize SizeKB
 ID: lvresize_by_pv
 DESC: Resize an LV by specified PV extents.
 FLAGS: SECONDARY_SYNTAX
@@ -1099,7 +1101,7 @@ FLAGS: SECONDARY_SYNTAX
 lvresize --poolmetadatasize SizeMB LV_thinpool
 OO: --alloc Alloc, --autobackup Bool, --force,
 --nofsck, --nosync, --noudevsync,
---reportformat String, --stripes Number, --stripesize SizeKB
+--reportformat ReportFmt, --stripes Number, --stripesize SizeKB
 OP: PV ...
 ID: lvresize_pool_metadata_by_size
 DESC: Resize a pool metadata SubLV by a specified size.
@@ -1115,14 +1117,20 @@ ID: lvs_general
 
 lvscan
 OO: --all, --blockdevice, --ignorelockingfailure, --partial,
---readonly, --reportformat String, --cache_long
+--readonly, --reportformat ReportFmt
 ID: lvscan_general
 
+lvscan --cache_long
+OO: --blockdevice, --ignorelockingfailure, --partial,
+--readonly, --reportformat ReportFmt
+OP: LV ...
+ID: lvscan_cache
+
 ---
 
 # None of these can function as a required option for pvchange.
 OO_PVCHANGE: --autobackup Bool, --force, --ignoreskippedcluster,
---reportformat String, --uuid
+--reportformat ReportFmt, --uuid
 
 # Any of these can function as a required option for pvchange.
 OO_PVCHANGE_META: --allocatable Bool, --addtag Tag, --deltag Tag,
@@ -1141,7 +1149,7 @@ DESC: Change properties of specified PVs.
 ---
 
 pvresize PV ...
-OO: --setphysicalvolumesize SizeMB, --reportformat String
+OO: --setphysicalvolumesize SizeMB, --reportformat ReportFmt
 ID: pvresize_general
 
 ---
@@ -1160,16 +1168,16 @@ OO: --dataalignment SizeKB, --dataalignmentoffset SizeKB, --bootloaderareasize S
 --force, --labelsector Number, --metadatatype MetadataType,
 --pvmetadatacopies MetadataCopiesPV, --metadatasize SizeMB,
 --metadataignore Bool, --norestorefile, --setphysicalvolumesize SizeMB,
---reportformat String, --restorefile String, --uuidstr String, --zero Bool
+--reportformat ReportFmt, --restorefile String, --uuidstr String, --zero Bool
 ID: pvcreate_general
 
 ---
 
 pvdisplay
-OO: --aligned, --all, --binary, --colon, --columns, --configreport String,
+OO: --aligned, --all, --binary, --colon, --columns, --configreport ConfigReport,
 --foreign, --ignorelockingfailure, --ignoreskippedcluster,
 --logonly, --maps, --noheadings, --nosuffix, --options String,
---readonly, --reportformat String, --select String, --separator String, --shared,
+--readonly, --reportformat ReportFmt, --select String, --separator String, --shared,
 --short, --sort String, --unbuffered, --units Units
 OP: PV|Tag ...
 ID: pvdisplay_general
@@ -1178,20 +1186,20 @@ ID: pvdisplay_general
 
 pvmove PV
 OO: --abort, --alloc Alloc, --atomic, --autobackup Bool, --background,
---interval Number, --name LV, --noudevsync, --reportformat String
+--interval Number, --name LV, --noudevsync, --reportformat ReportFmt
 OP: PV ...
 ID: pvmove_one
 DESC: Move PV extents.
 
 pvmove
-OO: --abort, --background
+OO: --abort, --background, --interval Number
 ID: pvmove_any
 DESC: Continue or abort existing pvmove operations.
 
 ---
 
 pvremove PV ...
-OO: --force, --reportformat String
+OO: --force, --reportformat ReportFmt
 ID: pvremove_general
 
 ---
@@ -1204,14 +1212,14 @@ ID: pvs_general
 ---
 
 pvscan
-OO: --ignorelockingfailure, --reportformat String, --exported, --novolumegroup,
+OO: --ignorelockingfailure, --reportformat ReportFmt, --exported, --novolumegroup,
 --short, --uuid
 ID: pvscan_show
 DESC: Display PV information.
 
 pvscan --cache_long
-OO: --ignorelockingfailure, --reportformat String, --background,
---activate Active, --major Number, --minor Number,
+OO: --ignorelockingfailure, --reportformat ReportFmt, --background,
+--activate Active, --major Number, --minor Number
 OP: PV|String ...
 ID: pvscan_cache
 DESC: Populate the lvmetad cache by scanning PVs.
@@ -1220,25 +1228,45 @@ DESC: Populate the lvmetad cache by scanning PVs.
 
 vgcfgbackup
 OO: --file String, --foreign, --ignorelockingfailure, --partial, --readonly,
---reportformat String
+--reportformat ReportFmt
 OP: VG ...
 ID: vgcfgbackup_general
 
 ---
 
+OO_VGCFGRESTORE: --force_long, --metadatatype MetadataType
+
 vgcfgrestore VG
-OO: --file String, --force_long, --list, --metadatatype MetadataType
+OO: OO_VGCFGRESTORE
 ID: vgcfgrestore_by_vg
+DESC: Restore VG metadata from last backup.
+
+vgcfgrestore --file String VG
+OO: OO_VGCFGRESTORE
+ID: vgcfgrestore_by_file
+DESC: Restore VG metadata from specified file.
+
+vgcfgrestore --list VG
+OO: OO_VGCFGRESTORE
+ID: vgcfgrestore_list_by_vg
+DESC: List all VG metadata backups.
+
+# FIXME: the optional VG pos arg is not used or checked and can be
+# anything, but a test in the test suite uses it.  Fix the test or
+# verify that the positional VG is correct?
 
 vgcfgrestore --list --file String
-ID: vgcfgrestore_by_file
+OO: OO_VGCFGRESTORE
+OP: VG
+ID: vgcfgrestore_list_by_file
+DESC: List one VG metadata backup file.
 
 ---
 
 # None of these can function as a required option for vgchange.
 
 OO_VGCHANGE: --autobackup Bool, --ignoremonitoring, --ignoreskippedcluster,
---noudevsync, --reportformat String, --select String, --force
+--noudevsync, --reportformat ReportFmt, --select String, --force
 
 # Any of these can function as a required option for vgchange.
 # profile is also part of OO_ALL, but is repeated in OO_VGCHANGE_META
@@ -1248,7 +1276,7 @@ OO_VGCHANGE_META: --addtag Tag, --deltag Tag,
 --logicalvolume Number, --maxphysicalvolumes Number, --alloc Alloc, --uuid,
 --clustered Bool, --pvmetadatacopies MetadataCopiesPV, --vgmetadatacopies MetadataCopiesVG,
 --physicalextentsize SizeMB, --resizeable Bool, --systemid String, --locktype LockType,
---profile String, --detachprofile, --metadataprofile String,
+--profile String, --detachprofile, --metadataprofile String
 
 vgchange OO_VGCHANGE_META
 OO: OO_VGCHANGE
@@ -1296,7 +1324,7 @@ DESC: Stop the lockspace of a shared VG in lvmlockd.
 ---
 
 vgck
-OO: --reportformat String
+OO: --reportformat ReportFmt
 OP: VG|Tag ...
 ID: vgck_general
 
@@ -1305,7 +1333,7 @@ ID: vgck_general
 vgconvert VG ...
 OO: --force, --labelsector Number, --bootloaderareasize SizeMB,
 --metadatatype MetadataType, --pvmetadatacopies MetadataCopiesPV,
---metadatasize SizeMB, --reportformat String
+--metadatasize SizeMB, --reportformat ReportFmt
 ID: vgconvert_general
 
 ---
@@ -1315,7 +1343,7 @@ OO: --addtag Tag, --alloc Alloc, --autobackup Bool, --clustered Bool, --maxlogic
 --maxphysicalvolumes Number, --metadataprofile String, --metadatatype MetadataType,
 --physicalextentsize SizeMB, --force, --zero Bool, --labelsector Number,
 --metadatasize SizeMB, --pvmetadatacopies MetadataCopiesPV, --vgmetadatacopies MetadataCopiesVG,
---reportformat String, --dataalignment SizeKB, --dataalignmentoffset SizeKB,
+--reportformat ReportFmt, --dataalignment SizeKB, --dataalignmentoffset SizeKB,
 --shared, --systemid String, --locktype LockType, --lockopt String
 ID: vgcreate_general
 
@@ -1323,16 +1351,16 @@ ID: vgcreate_general
 
 vgdisplay
 OO: --activevolumegroups, --aligned, --binary, --colon, --columns,
---configreport String, --foreign, --ignorelockingfailure,
+--configreport ConfigReport, --foreign, --ignorelockingfailure,
 --ignoreskippedcluster, --logonly, --noheadings, --nosuffix,
---options String, --partial, --readonly, --reportformat String, --select String,
+--options String, --partial, --readonly, --reportformat ReportFmt, --select String,
 --shared, --short, --separator String, --sort String, --unbuffered, --units Units
 OP: VG|Tag ...
 ID: vgdisplay_general
 
 ---
 
-OO_VGEXPORT: --reportformat String
+OO_VGEXPORT: --reportformat ReportFmt
 
 vgexport VG|Tag|Select ...
 OO: --select String, OO_VGEXPORT
@@ -1352,12 +1380,12 @@ OO: --autobackup Bool,
 --force, --zero Bool, --labelsector Number, --metadatatype MetadataType,
 --metadatasize SizeMB, --pvmetadatacopies MetadataCopiesPV,
 --metadataignore Bool, --dataalignment SizeKB, --dataalignmentoffset SizeKB,
---reportformat String, --restoremissing
+--reportformat ReportFmt, --restoremissing
 ID: vgextend_general
 
 ---
 
-OO_VGIMPORT: --force, --reportformat String
+OO_VGIMPORT: --force, --reportformat ReportFmt
 
 vgimport VG|Tag|Select ...
 OO: --select String, OO_VGIMPORT
@@ -1384,13 +1412,13 @@ ID: vgmerge_general
 ---
 
 vgmknodes
-OO: --ignorelockingfailure, --refresh, --reportformat String
+OO: --ignorelockingfailure, --refresh, --reportformat ReportFmt
 OP: VG|LV|Tag ...
 ID: vgmknodes_general
 
 ---
 
-OO_VGREDUCE: --autobackup Bool, --force, --reportformat String
+OO_VGREDUCE: --autobackup Bool, --force, --reportformat ReportFmt
 
 vgreduce VG PV ...
 OO: OO_VGREDUCE
@@ -1410,18 +1438,18 @@ DESC: Remove all missing PVs from a VG.
 ---
 
 vgremove VG|Tag|Select ...
-OO: --force, --noudevsync, --reportformat String, --select String
+OO: --force, --noudevsync, --reportformat ReportFmt, --select String
 ID: vgremove_general
 
 ---
 
 vgrename VG VG_new
-OO: --autobackup Bool, --force, --reportformat String
+OO: --autobackup Bool, --force, --reportformat ReportFmt
 ID: vgrename_by_name
 DESC: Rename a VG.
 
 vgrename String VG_new
-OO: --autobackup Bool, --force, --reportformat String
+OO: --autobackup Bool, --force, --reportformat ReportFmt
 ID: vgrename_by_uuid
 DESC: Rename a VG by specifying the VG UUID.
 
@@ -1436,7 +1464,7 @@ ID: vgs_general
 
 vgscan
 OO: --cache_long, --ignorelockingfailure, --mknodes, --notifydbus,
---partial, --reportformat String
+--partial, --reportformat ReportFmt
 ID: vgscan_general
 
 ---
@@ -1462,19 +1490,23 @@ DESC: Split a VG by PVs in a specified LV.
 
 # built-in and deprecated commands
 
+# use lvmconfig
 config
 OO: OO_CONFIG
 OP: String ...
 ID: lvmconfig_general
+FLAGS: SECONDARY_SYNTAX
 
+# use lvmconfig
 dumpconfig
 OO: OO_CONFIG
 OP: String ...
 ID: lvmconfig_general
+FLAGS: SECONDARY_SYNTAX
 
 devtypes
 OO: --aligned, --binary, --nameprefixes, --noheadings,
---nosuffix, --options String, --reportformat String, --rows,
+--nosuffix, --options String, --reportformat ReportFmt, --rows,
 --select String, --separator String, --sort String, --unbuffered, --unquoted
 ID: devtypes_general
 
@@ -1484,10 +1516,10 @@ OP: VG ...
 ID: fullreport_general
 
 lastlog
-OO: --reportformat String, --select String
+OO: --reportformat ReportFmt, --select String
 ID: lastlog_general
 
-lvpoll --polloperation String LV ...
+lvpoll --polloperation PollOp LV ...
 OO: --abort, --autobackup Bool, --handlemissingpvs, --interval Number
 ID: lvpoll_general
 
@@ -1500,8 +1532,10 @@ ID: help_general
 version
 ID: version_general
 
+# deprecated
 pvdata
 ID: pvdata_general
+FLAGS: SECONDARY_SYNTAX
 
 segtypes
 ID: segtypes_general
@@ -1512,17 +1546,25 @@ ID: systemid_general
 tags
 ID: tags_general
 
+# deprecated
 lvmchange
 ID: lvmchange_general
+FLAGS: SECONDARY_SYNTAX
 
+# deprecated
 lvmdiskscan
 OO: --lvmpartition, --readonly
 ID: lvmdiskscan_general
+FLAGS: SECONDARY_SYNTAX
 
+# deprecated
 lvmsadc
 ID: lvmsadc_general
+FLAGS: SECONDARY_SYNTAX
 
+# deprecated
 lvmsar
 OO: --full, --stdin
 ID: lvmsar_general
+FLAGS: SECONDARY_SYNTAX
 

tools/command.h

@@ -54,27 +54,6 @@ 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;
@@ -82,16 +61,36 @@ 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 1 << 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_ */
 };
 
@@ -109,6 +108,33 @@ struct pos_arg {
 	struct arg_def def;  /* defines accepted values */
 };
 
+/*
+ *
+ * Commands using a given command definition must follow a set
+ * of rules.  If a given command+LV matches the conditions in
+ * opts/lvt_bits/lvp_bits, then the checks are applied.
+ * If one condition is not met, the checks are not applied.
+ * If no conditions are set, the checks are always applied.
+ */
+
+#define RULE_INVALID 1
+#define RULE_REQUIRE 2
+
+struct cmd_rule {
+	int *opts;			/* if any option in this list is set, the check may apply */
+	uint64_t lvt_bits;		/* if LV has one of these types (lvt_enum_to_bit), the check may apply */
+	uint64_t lvp_bits;		/* if LV has all of these properties (lvp_enum_to_bit), the check may apply */
+
+	int *check_opts;		/* used options must [not] be in this list */
+	uint64_t check_lvt_bits;	/* LV must [not] have one of these type */
+	uint64_t check_lvp_bits;	/* LV must [not] have all of these properties */
+
+	uint32_t rule;			/* RULE_INVALID, RULE_REQUIRE: check values must [not] be true */
+	int opts_count;			/* entries in opts[] */
+	int check_opts_count;		/* entries in check_opts[] */
+
+};
+
 /*
  * CMD_RO_ARGS needs to accomodate a list of options,
  * of which one is required after which the rest are
@@ -118,6 +144,7 @@ struct pos_arg {
 #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 32        /* max number of rules per command def */
 
 /*
  * one or more from required_opt_args is required,
@@ -158,6 +185,8 @@ 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;
@@ -165,6 +194,8 @@ struct command {
 
 	/* used for processing current position */
 	int pos_count;
+
+	int rule_count;
 };
 
 #endif

tools/lv_props.h

@@ -0,0 +1,56 @@
+
+/*
+ * NULL in the last arg can be replaced with actual
+ * calls to the lv_is_prop() function when those
+ * become functions (are #define now), take uniform
+ * args (e.g. some take cmd others don't), and are
+ * exposed in tools.h
+ *
+ * Until then, the lv_is_prop() functions are
+ * called indirectly through _lv_is_prop().
+ */
+
+lvp(LVP_NONE, "", NULL) /* enum value 0 means none */
+lvp(is_locked_LVP, "lv_is_locked", NULL)
+lvp(is_partial_LVP, "lv_is_partial", NULL)
+lvp(is_virtual_LVP, "lv_is_virtual", NULL)
+lvp(is_merging_LVP, "lv_is_merging", NULL)
+lvp(is_merging_origin_LVP, "lv_is_merging_origin", NULL)
+lvp(is_converting_LVP, "lv_is_converting", NULL)
+lvp(is_external_origin_LVP, "lv_is_external_origin", NULL)
+lvp(is_virtual_origin_LVP, "lv_is_virtual_origin", NULL)
+lvp(is_not_synced_LVP, "lv_is_not_synced", NULL)
+lvp(is_pending_delete_LVP, "lv_is_pending_delete", NULL)
+lvp(is_error_when_full_LVP, "lv_is_error_when_full", NULL)
+lvp(is_pvmove_LVP, "lv_is_pvmove", NULL)
+lvp(is_removed_LVP, "lv_is_removed", NULL)
+lvp(is_vg_writable_LVP, "lv_is_vg_writable", NULL)
+
+/* kinds of sub LV */
+lvp(is_thinpool_data_LVP, "lv_is_thinpool_data", NULL)
+lvp(is_thinpool_metadata_LVP, "lv_is_thinpool_metadata", NULL)
+lvp(is_cachepool_data_LVP, "lv_is_cachepool_data", NULL)
+lvp(is_cachepool_metadata_LVP, "lv_is_cachepool_metadata", NULL)
+lvp(is_mirror_image_LVP, "lv_is_mirror_image", NULL)
+lvp(is_mirror_log_LVP, "lv_is_mirror_log", NULL)
+lvp(is_raid_image_LVP, "lv_is_raid_image", NULL)
+lvp(is_raid_metadata_LVP, "lv_is_raid_metadata", NULL)
+
+/*
+ * is_thick_origin should be used instead of is_origin
+ * is_thick_snapshot is generally used as LV_snapshot from lv_types.h
+ */
+lvp(is_origin_LVP, "lv_is_origin", NULL)
+lvp(is_thick_origin_LVP, "lv_is_thick_origin", NULL)
+lvp(is_thick_snapshot_LVP, "lv_is_thick_snapshot", NULL)
+lvp(is_thin_origin_LVP, "lv_is_thin_origin", NULL)
+lvp(is_thin_snapshot_LVP, "lv_is_thin_snapshot", NULL)
+
+lvp(is_cache_origin_LVP, "lv_is_cache_origin", NULL)
+lvp(is_merging_cow_LVP, "lv_is_merging_cow", NULL)
+lvp(is_cow_covering_origin_LVP, "lv_is_cow_covering_origin", NULL)
+lvp(is_visible_LVP, "lv_is_visible", NULL)
+lvp(is_historical_LVP, "lv_is_historical", NULL)
+lvp(is_raid_with_tracking_LVP, "lv_is_raid_with_tracking", NULL)
+lvp(LVP_COUNT, "", NULL)
+

tools/lv_types.h

@@ -0,0 +1,32 @@
+
+
+/*
+ * LV types used in command definitions.  The type strings are used
+ * as LV suffixes, e.g. LV_type or LV_type1_type2.
+ *
+ * The final NULL arg can be replaced with lv_is_type() functions
+ * if the current lv_is_type #defines become functions and are
+ * moved to tools.h
+ *
+ * Until then, the lv_is_type() functions are called indirectly
+ * through _lv_is_type().
+ */
+
+lvt(LVT_NONE, "", NULL)
+lvt(linear_LVT, "linear", NULL)
+lvt(striped_LVT, "striped", NULL)
+lvt(snapshot_LVT, "snapshot", NULL) /* lv_is_cow, lv_is_thick_snapshot */
+lvt(thin_LVT, "thin", NULL)
+lvt(thinpool_LVT, "thinpool", NULL)
+lvt(cache_LVT, "cache", NULL)
+lvt(cachepool_LVT, "cachepool", NULL)
+lvt(mirror_LVT, "mirror", NULL)
+lvt(raid_LVT, "raid", NULL)
+lvt(raid0_LVT, "raid0", NULL)
+lvt(raid1_LVT, "raid1", NULL)
+lvt(raid4_LVT, "raid4", NULL)
+lvt(raid5_LVT, "raid5", NULL)
+lvt(raid6_LVT, "raid6", NULL)
+lvt(raid10_LVT, "raid10", NULL)
+lvt(LVT_COUNT, "", NULL)
+

tools/lvchange.c

@@ -1436,286 +1436,3 @@ int lvchange(struct cmd_context *cmd, int argc, char **argv)
 			       update ? READ_FOR_UPDATE : 0, NULL,
 			       &_lvchange_single);
 }
-
-#if 0
-/*
- * Check if the status of the LV allows running lvchange.
- *
- * FIXME: check for invalid VG/LV properties in a way that is not prone
- * to missing some.  Currently, there are some checks here, some in the
- * functions above, some in process_each, and some may be missing.
- */
-static int _lvchange_status_is_valid(struct cmd_context *cmd, struct logical_volume *lv)
-{
-	if (!(lv->vg->status & LVM_WRITE)) {
-		log_error("Operation not permitted on LV %s: writable VG required.",
-			  display_lvname(lv));
-		return 0;
-	}
-
-	if (lv_is_pvmove(lv)) {
-		log_error("Operation not permitted on LV %s: used for pvmove.",
-			  display_lvname(lv));
-		if (arg_is_set(cmd, activate_ARG))
-			log_error("Use 'pvmove --abort' to abandon a pvmove");
-		return 0;
-	}
-
-	if (lv_is_mirror_log(lv)) {
-		log_error("Operation not permitted on LV %s: is mirror log.",
-			  display_lvname(lv));
-		return 0;
-	}
-
-	if (lv_is_mirror_image(lv)) {
-		log_error("Operation not permitted on LV %s: is mirror image.",
-			  display_lvname(lv));
-		return 0;
-	}
-
-	if (lv_is_origin(lv) && !lv_is_thin_volume(lv)) {
-		log_error("Operation not permitted on LV %s: is under snapshot.",
-			  display_lvname(lv));
-		return 0;
-	}
-
-	return 1;
-}
-
-static int _lvchange_properties_single(struct cmd_context *cmd,
-			               struct logical_volume *lv,
-			               struct processing_handle *handle)
-{
-	int doit = 0, docmds = 0;
-
-	/* FIXME: sort out hidden/internal LVs, e.g. _lvchange_hidden_is_valid() */
-
-	if (!_lvchange_status_is_valid(cmd, lv))
-		return_ECMD_FAILED;
-
-	if (arg_is_set(cmd, persistent_ARG) && lv_is_pool(lv)) {
-		log_error("Operation not permitted on LV %s: persistent device numbers are not supported with pools.",
-			  display_lvname(lv));
-		return ECMD_FAILED;
-	}
-
-	/*
-	 * If a persistent lv lock already exists from activation
-	 * (with the needed mode or higher), this will be a no-op.
-	 * Otherwise, the lv lock will be taken as non-persistent
-	 * and released when this command exits.
-	 */
-	if (!lockd_lv(cmd, lv, "ex", 0)) {
-		stack;
-		return ECMD_FAILED;
-	}
-
-	for (i = 0; i < cmd->command->ro_count; i++) {
-		opt_enum = cmd->command->required_opt_args[i].opt;
-
-		if (!arg_is_set(cmd, opt_enum))
-			continue;
-
-		if (!archive(lv->vg))
-			return_ECMD_FAILED;
-
-		docmds++;
-
-		switch (opt_enum) {
-		case permission_ARG:
-			doit += _lvchange_permission(cmd, lv);
-			break;
-
-		case alloc_ARG:
-		case contiguous_ARG:
-			doit += _lvchange_alloc(cmd, lv);
-			break;
-
-		case errorwhenfull_ARG:
-			doit += _lvchange_errorwhenfull(cmd, lv);
-			break;
-
-		case readahead_ARG:
-			doit += _lvchange_readahead(cmd, lv);
-			break;
-
-		case persistent_ARG:
-			doit += _lvchange_persistent(cmd, lv);
-			break;
-
-		case discards_ARG:
-		case zero_ARG:
-			doit += _lvchange_pool_update(cmd, lv);
-			break;
-
-		case addtag_ARG:
-		case deltag_ARG:
-			doit += _lvchange_tag(cmd, lv, opt_enum);
-			break;
-
-		case writemostly_ARG:
-		case writebehind_ARG:
-			doit += _lvchange_writemostly(lv);
-			break;
-
-		case minrecoveryrate_ARG:
-		case maxrecoveryrate_ARG:
-			doit += _lvchange_recovery_rate(lv);
-			break;
-
-		case profile_ARG:
-		case metadataprofile_ARG:
-		case detachprofile_ARG:
-			doit += _lvchange_profile(lv);
-			break;
-
-		case setactivationskip_ARG:
-			doit += _lvchange_activation_skip(lv);
-			break;
-
-		case cachemode_ARG:
-		case cachepolicy_ARG:
-		case cachesettings_ARG:
-			doit += _lvchange_cache(cmd, lv);
-			break;
-
-		default:
-			log_error(INTERNAL_ERROR "Failed to check for option %s",
-				  arg_long_option_name(i));
-	}
-
-	if (doit)
-		log_print_unless_silent("Logical volume %s changed.", display_lvname(lv));
-
-	if (doit != docmds)
-		return_ECMD_FAILED;
-
-	return ECMD_PROCESSED;
-}
-
-int lvchange_properties_cmd(struct cmd_context *cmd, int argc, char **argv)
-{
-	struct processing_handle *handle = init_processing_handle(cmd, NULL);
-	int ret;
-
-	ret = process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, handle, _lvchange_properties_single);
-
-	destroy_processing_handle(cmd, handle);
-	return ret;
-}
-
-static int _lvchange_activate_single(struct cmd_context *cmd,
-				     struct logical_volume *lv,
-				     struct processing_handle *handle)
-{
-	struct logical_volume *origin;
-	char snaps_msg[128];
-
-	/* FIXME: sort out hidden/internal LVs, e.g. _lvchange_hidden_is_valid() */
-
-	if (!_lvchange_status_is_valid(cmd, lv))
-		return_ECMD_FAILED;
-
-	/* FIXME: untangle the proper logic for cow / sparse / virtual origin */
-
-	/* If LV is sparse, activate origin instead */
-	if (lv_is_cow(lv) && lv_is_virtual_origin(origin = origin_from_cow(lv)))
-		lv = origin;
-
-	if (lv_is_cow(lv)) {
-		origin = origin_from_cow(lv);
-		if (origin->origin_count < 2)
-			snaps_msg[0] = '\0';
-		else if (dm_snprintf(snaps_msg, sizeof(snaps_msg),
-				     " and %u other snapshot(s)",
-				     origin->origin_count - 1) < 0) {
-			log_error("Failed to prepare message.");
-			return ECMD_FAILED;
-		}
-
-		if (!arg_is_set(cmd, yes_ARG) &&
-		    (yes_no_prompt("Change of snapshot %s will also change its "
-				   "origin %s%s. Proceed? [y/n]: ",
-				   display_lvname(lv), display_lvname(origin),
-				   snaps_msg) == 'n')) {
-			log_error("Logical volume %s not changed.", display_lvname(lv));
-			return ECMD_FAILED;
-		}
-	}
-
-	/*
-	 * If --sysinit -aay is used and at the same time lvmetad is used,
-	 * we want to rely on autoactivation to take place. Also, we
-	 * need to take special care here as lvmetad service does
-	 * not neet to be running at this moment yet - it could be
-	 * just too early during system initialization time.
-	 */
-	if (arg_is_set(cmd, sysinit_ARG) && (arg_uint_value(cmd, activate_ARG, 0) == CHANGE_AAY)) {
-		if (lvmetad_used()) {
-			log_warn("WARNING: lvmetad is active, skipping direct activation during sysinit.");
-			return ECMD_PROCESSED;
-		}
-	}
-
-	if (!_lvchange_activate(cmd, lv))
-		return_ECMD_FAILED;
-
-	return ECMD_PROCESSED;
-}
-
-int lvchange_activate_cmd(struct cmd_context *cmd, int argc, char **argv)
-{
-	struct processing_handle *handle = init_processing_handle(cmd, NULL);
-
-	cmd->handles_missing_pvs = 1;
-	cmd->lockd_vg_default_sh = 1;
-
-	/*
-	 * Include foreign VGs that contain active LVs.
-	 * That shouldn't happen in general, but if it does by some
-	 * mistake, then we want to allow those LVs to be deactivated.
-	 */
-	cmd->include_active_foreign_vgs = 1;
-
-	/* Allow deactivating if locks fail. */
-	if (is_change_activating((activation_change_t)arg_uint_value(cmd, activate_ARG, CHANGE_AY)))
-		cmd->lockd_vg_enforce_sh = 1;
-
-	ret = process_each_lv(cmd, argc, argv, NULL, NULL, 0, handle, _lvchange_activate_single);
-
-	destroy_processing_handle(cmd, handle);
-	return ret;
-}
-
-static int _lvchange_refresh_single(struct cmd_context *cmd,
-				    struct logical_volume *lv,
-				    struct processing_handle *handle)
-{
-	/* FIXME: sort out hidden/internal LVs, e.g. _lvchange_hidden_is_valid() */
-
-	if (!_lvchange_status_is_valid(cmd, lv))
-		return_ECMD_FAILED;
-
-	log_verbose("Refreshing logical volume %s (if active).", display_lvname(lv));
-
-	if (!_lv_refresh(cmd, lv))
-		return_ECMD_FAILED;
-
-	return ECMD_PROCESSED;
-}
-
-int lvchange_refresh_cmd(struct cmd_context *cmd, int argc, char **argv)
-{
-	struct processing_handle *handle = init_processing_handle(cmd, NULL);
-	int ret;
-
-	cmd->handles_missing_pvs = 1;
-	cmd->lockd_vg_default_sh = 1;
-
-	ret = process_each_lv(cmd, argc, argv, NULL, NULL, 0, handle, _lvchange_refresh_single);
-
-	destroy_processing_handle(cmd, handle);
-	return ret;
-}
-#endif
-

tools/lvmcmdline.c

@@ -79,6 +79,25 @@ 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
  */
@@ -96,9 +115,34 @@ struct command_function command_functions[COMMAND_ID_COUNT] = {
 	{ lvmconfig_general_CMD, lvmconfig },
 };
 #if 0
-	{ lvchange_properties_CMD, lvchange_properties_cmd },
-	{ lvchange_activate_CMD, lvchange_activate_cmd },
-	{ lvchange_refresh_CMD, lvchange_refresh_cmd },
+	/* all raid-related type conversions */
+
+	{ lvconvert_raid_types_CMD,			lvconvert_raid_types_fn },
+
+	/* 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_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 },
+
+	/* 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 },
+
 #endif
 
 /* Command line args */
@@ -772,6 +816,66 @@ int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *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.
@@ -994,6 +1098,20 @@ 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
@@ -1319,7 +1437,7 @@ static void _print_description(int ci)
  * set to match.
  *
  * required_pos_args[0].types & select_VAL means
- * cmd->argv[] in that pos can be NULL if arg_is_set(select_ARG)
+ * 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 */
@@ -1328,6 +1446,7 @@ static void _print_description(int ci)
 static struct command *_find_command(struct cmd_context *cmd, const char *path, int *argc, char **argv)
 {
 	const char *name;
+	char buf[64];
 	int match_required, match_ro, match_rp, match_type, match_unused, mismatch_required;
 	int best_i = 0, best_required = 0, best_type = 0, best_unused = 0;
 	int close_i = 0, close_ro = 0, close_type;
@@ -1335,6 +1454,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	int temp_unused_count;
 	int best_unused_options[MAX_UNUSED_COUNT] = { 0 };
 	int best_unused_count = 0;
+	int opts_match_count, opts_unmatch_count;
 	int ro, rp;
 	int i, j;
 	int opt_enum, opt_i;
@@ -1559,7 +1679,55 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			return NULL;
 		}
 	}
+
 out:
+	/*
+	 * Check any rules related to option combinations.
+	 * Other rules are checked after VG is read.
+	 */
+
+	for (i = 0; i < commands[best_i].rule_count; i++) {
+		struct cmd_rule *rule;
+		rule = &commands[best_i].rules[i];
+
+		/*
+		 * The rule wants to validate options (check_opts). That can be
+		 * done here if the only qualification for the validation is
+		 * other options (and not specific LV type or LV property which
+		 * are not known here.)
+		 */
+
+		if (rule->check_opts_count && !rule->lvt_bits && !rule->lvp_bits) {
+			/*
+			 * When no opt is specified for applying the rule, then
+			 * the rule is always applied, otherwise the rule is
+			 * applied when the specific option is set.
+			 */
+			if (rule->opts_count &&
+			    !opt_in_list_is_set(cmd, rule->opts, rule->opts_count, NULL, NULL))
+				continue;
+
+			opt_in_list_is_set(cmd, rule->check_opts, rule->check_opts_count,
+					   &opts_match_count, &opts_unmatch_count);
+
+			if (opts_match_count && (rule->rule == RULE_INVALID)) {
+				memset(buf, 0, sizeof(buf));
+				opt_array_to_str(cmd, rule->check_opts, rule->check_opts_count, buf, sizeof(buf));
+				log_error("Invalid options for command (%s %d): %s",
+					  commands[best_i].command_line_id, best_i, buf);
+				return NULL;
+			}
+
+			if (opts_unmatch_count && (rule->rule == RULE_REQUIRE)) {
+				memset(buf, 0, sizeof(buf));
+				opt_array_to_str(cmd, rule->check_opts, rule->check_opts_count, buf, sizeof(buf));
+				log_error("Required options for command (%s %d): %s",
+					  commands[best_i].command_line_id, best_i, buf);
+				return NULL;
+			}
+		}
+	}
+
 	log_debug("command line id: %s %d", commands[best_i].command_line_id, best_i);
 
 	return &commands[best_i];
@@ -1618,7 +1786,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 numeral suffix.");
+		log_print("  generated with the \"lvol\" prefix and a unique numeric 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),");
@@ -2457,6 +2625,12 @@ 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

@@ -2326,6 +2326,541 @@ static struct lv_segment _historical_lv_segment = {
 	.origin_list = DM_LIST_HEAD_INIT(_historical_lv_segment.origin_list),
 };
 
+int opt_in_list_is_set(struct cmd_context *cmd, int *opts, int count,
+		       int *match_count, int *unmatch_count)
+{
+	int match = 0;
+	int unmatch = 0;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		if (arg_is_set(cmd, opts[i]))
+			match++;
+		else
+			unmatch++;
+	}
+
+	if (match_count)
+		*match_count = match;
+	if (unmatch_count)
+		*unmatch_count = unmatch;
+
+	return match ? 1 : 0;
+}
+      
+void opt_array_to_str(struct cmd_context *cmd, int *opts, int count,
+		      char *buf, int len)
+{
+	int pos = 0;
+	int ret;
+	int i;
+
+	for (i = 0; i < count; i++) {
+		ret = snprintf(buf + pos, len - pos, "%s ", arg_long_option_name(opts[i]));
+		if (ret >= len - pos)
+			break;
+		pos += ret;
+	}
+
+	buf[len - 1] = '\0';
+}
+
+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 void lvt_bits_to_str(uint64_t bits, char *buf, int len)
+{
+	struct lv_types *type;
+	int lvt_enum;
+	int pos = 0;
+	int ret;
+
+	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)) {
+			ret = snprintf(buf + pos, len - pos, "%s ", type->name);
+			if (ret >= len - pos)
+				break;
+			pos += ret;
+		}
+	}
+	buf[len - 1] = '\0';
+}
+
+/*
+ * 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_vg_writable_LVP:
+		return (lv->vg->status & LVM_WRITE) ? 1 : 0;
+	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: /* use lv_is_thick_origin */
+		return lv_is_origin(lv);
+	case is_thick_origin_LVP:
+		return lv_is_thick_origin(lv);
+	case is_thick_snapshot_LVP:
+		return lv_is_thick_snapshot(lv);
+	case is_thin_origin_LVP:
+		return lv_is_thin_origin(lv, NULL);
+	case is_thin_snapshot_LVP:
+		return lv_is_thin_snapshot(lv);
+	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.
+ * If lv matches one of the specified lv types, then return 1.
+ */
+
+static int _lv_types_match(struct cmd_context *cmd, struct logical_volume *lv, uint64_t lvt_bits,
+			   uint64_t *match_bits, uint64_t *unmatch_bits)
+{
+	struct lv_types *type;
+	int lvt_enum;
+	int found_a_match = 0;
+	int match;
+
+	if (match_bits)
+		*match_bits = 0;
+	if (unmatch_bits)
+		*unmatch_bits = 0;
+
+	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)
+			match = _lv_is_type(cmd, lv, lvt_enum);
+		else
+			match = type->fn(cmd, lv);
+
+		if (match)
+			found_a_match = 1;
+
+		if (match_bits && match)
+			*match_bits |= lvt_enum_to_bit(lvt_enum);
+
+		if (unmatch_bits && !match)
+			*unmatch_bits |= lvt_enum_to_bit(lvt_enum);
+	}
+
+	return found_a_match;
+}
+
+/*
+ * Call lv_is_<prop> for each <prop>_LVP bit set in lvp_bits.
+ * If lv matches all of the specified lv properties, then return 1.
+ */
+
+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 found_a_mismatch = 0;
+	int match;
+
+	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)
+			match = _lv_is_prop(cmd, lv, lvp_enum);
+		else
+			match = prop->fn(cmd, lv);
+
+		if (!match)
+			found_a_mismatch = 1;
+
+		if (match_bits && match)
+			*match_bits |= lvp_enum_to_bit(lvp_enum);
+
+		if (unmatch_bits && !match)
+			*unmatch_bits |= lvp_enum_to_bit(lvp_enum);
+	}
+
+	return !found_a_mismatch;
+}
+
+/*
+ * 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, NULL, NULL);
+		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;
+	uint64_t lv_props_match_bits, lv_props_unmatch_bits;
+	uint64_t lv_types_match_bits, lv_types_unmatch_bits;
+	int opts_match_count, opts_unmatch_count;
+	int lvt_enum;
+	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];
+
+		/*
+		 * RULE: <conditions> INVALID|REQUIRE <checks>
+		 *
+		 * If all the conditions apply to the command+LV, then
+		 * the checks are performed.  If all conditions are zero
+		 * (!opts_count, !lvt_bits, !lvp_bits), then the check
+		 * is always performed.
+		 *
+		 * Conditions:
+		 *
+		 * 1. options (opts): if any of the specified options are set,
+		 *    then the checks may apply.
+		 *
+		 * 2. LV types (lvt_bits): if any of the specified LV types
+		 *    match the LV, then the checks may apply.
+		 *
+		 * 3. LV properties (lvp_bits): if all of the specified
+		 *    LV properties match the LV, then the checks may apply.
+		 *
+		 * If conditions 1, 2, 3 all pass, then the checks apply.
+		 *
+		 * Checks:
+		 *
+		 * 1. options (check_opts):
+		 *    INVALID: if any of the specified options are set,
+		 *    then the command fails.
+		 *    REQUIRE: if any of the specified options are not set,
+		 *    then the command fails.
+		 *
+		 * 2. LV types (check_lvt_bits):
+		 *    INVALID: if any of the specified LV types match the LV,
+		 *    then the command fails.
+		 *    REQUIRE: if none of the specified LV types match the LV,
+		 *    then the command fails.
+		 *
+		 * 3. LV properties (check_lvp_bits):
+		 *    INVALID: if any of the specified LV properties match
+		 *    the LV, then the command fails.
+		 *    REQUIRE: if any of the specified LV properties do not match
+		 *    the LV, then the command fails.
+		 */
+
+		if (rule->opts_count && !opt_in_list_is_set(cmd, rule->opts, rule->opts_count, NULL, NULL))
+			continue;
+
+		/* If LV matches one type in lvt_bits, this returns 1. */
+		if (rule->lvt_bits && !_lv_types_match(cmd, lv, rule->lvt_bits, NULL, NULL))
+			continue;
+
+		/* If LV matches all properties in lvp_bits, this returns 1. */
+		if (rule->lvp_bits && !_lv_props_match(cmd, lv, rule->lvp_bits, NULL, NULL))
+			continue;
+
+		/*
+		 * Check the options, LV types, LV properties.
+		 */
+
+		if (rule->check_opts)
+			opt_in_list_is_set(cmd, rule->check_opts, rule->check_opts_count,
+					   &opts_match_count, &opts_unmatch_count);
+
+		if (rule->check_lvt_bits)
+			_lv_types_match(cmd, lv, rule->check_lvt_bits,
+					&lv_types_match_bits, &lv_types_unmatch_bits);
+
+		if (rule->check_lvp_bits)
+			_lv_props_match(cmd, lv, rule->check_lvp_bits,
+					&lv_props_match_bits, &lv_props_unmatch_bits);
+		
+		/*
+		 * Evaluate if the check results pass based on the rule.
+		 * The options are checked again here because the previous
+		 * option validation (during command matching) does not cover
+		 * cases where the option is combined with conditions of LV types
+		 * or properties.
+		 */
+
+		/* Fail if any invalid options are set. */
+
+		if (rule->check_opts && (rule->rule == RULE_INVALID) && opts_match_count) {
+			memset(buf, 0, sizeof(buf));
+			opt_array_to_str(cmd, rule->check_opts, rule->check_opts_count, buf, sizeof(buf));
+			log_warn("An invalid option is set: %s", buf);
+			ret = 0;
+		}
+
+		/* Fail if any required options are not set. */
+
+		if (rule->check_opts && (rule->rule == RULE_REQUIRE) && opts_unmatch_count)  {
+			memset(buf, 0, sizeof(buf));
+			opt_array_to_str(cmd, rule->check_opts, rule->check_opts_count, buf, sizeof(buf));
+			log_warn("A required option is not set: %s", buf);
+			ret = 0;
+		}
+
+		/* Fail if the LV matches any of the invalid LV types. */
+
+		if (rule->check_lvt_bits && (rule->rule == RULE_INVALID) && lv_types_match_bits) {
+			log_warn("Command on LV %s with invalid type: %s",
+				 display_lvname(lv), lvtype ? lvtype->name : "unknown");
+			ret = 0;
+		}
+
+		/* Fail if the LV does not match any of the required LV types. */
+
+		if (rule->check_lvt_bits && (rule->rule == RULE_REQUIRE) && !lv_types_match_bits) {
+			memset(buf, 0, sizeof(buf));
+			lvt_bits_to_str(rule->check_lvt_bits, buf, sizeof(buf));
+			log_warn("Command on LV %s with type %s does not match required type: %s",
+				 display_lvname(lv), lvtype ? lvtype->name : "unknown", buf);
+			ret = 0;
+		}
+
+		/* Fail if the LV matches any of the invalid LV properties. */
+
+		if (rule->check_lvp_bits && (rule->rule == RULE_INVALID) && lv_props_match_bits) {
+			memset(buf, 0, sizeof(buf));
+			lvp_bits_to_str(lv_props_match_bits, buf, sizeof(buf));
+			log_warn("Command on LV %s with invalid properties: %s",
+				 display_lvname(lv), buf);
+			ret = 0;
+		}
+
+		/* Fail if the LV does not match any of the required LV properties. */
+
+		if (rule->check_lvp_bits && (rule->rule == RULE_REQUIRE) && lv_props_unmatch_bits) {
+			memset(buf, 0, sizeof(buf));
+			lvp_bits_to_str(lv_props_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,
@@ -2350,9 +2885,6 @@ 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);
 
@@ -2504,65 +3036,42 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 			continue;
 
 		/*
-		 * If the command definition specifies one required positional
-		 * LV (possibly repeatable), and specifies accepted LV types,
-		 * then verify that the LV being processed matches one of those
-		 * types.
-		 *
-		 * process_each_lv() can only be used for commands that have
-		 * one positional LV arg (optionally repeating, where each is
-		 * processed independently.)  It cannot work for commands that
-		 * have different required LVs in designated positions, like
-		 * 'lvrename LV1 LV2', where each LV is not processed
-		 * independently.  That means that this LV type check only
-		 * needs to check the lv_type of the first positional arg.
-		 *
-		 * There is one command that violates this rule by stealing
-		 * the first positional LV arg before calling process_each_lv:
-		 * lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
-		 * This code cannot validate that case.  process_each_lv() sees
-		 * a single LV name arg, but it's in pos 2.  Could we work around
-		 * this by looking at the final positional arg rather than always
-		 * looking at pos 1?
-		 *
-		 * This only validates types for required LV positional args
-		 * (currently there are no command specifications that include
-		 * specific LV types in optional positional args.)
+		 * The command definition may include restrictions on the
+		 * types and properties of LVs that can be processed.
 		 */
 
-		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_types(cmd, lvl->lv)) {
+			/* FIXME: include this result in report log? */
+			/* FIXME: avoid duplicating message for each level */
 
-			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;
+			if (str_list_match_item(&found_arg_lvnames, lvl->lv->name)) {
+				log_error("Operation not permitted (%s %d) on LV %s.",
+					  cmd->command->command_line_id, cmd->command->command_line_enum,
+					  display_lvname(lvl->lv));
+				ret_max = ECMD_FAILED;
+			} else {
+				log_warn("Operation not permitted (%s %d) on LV %s.",
+					 cmd->command->command_line_id, cmd->command->command_line_enum,
+					 display_lvname(lvl->lv));
 			}
+			continue;
+		}
+
+		if (!_check_lv_rules(cmd, lvl->lv)) {
+			/* FIXME: include this result in report log? */
+			/* FIXME: avoid duplicating message for each level */
+
+			if (str_list_match_item(&found_arg_lvnames, lvl->lv->name)) {
+				log_error("Operation not permitted (%s %d) on LV %s.",
+					  cmd->command->command_line_id, cmd->command->command_line_enum,
+					  display_lvname(lvl->lv));
+				ret_max = ECMD_FAILED;
+			} else {
+				log_warn("Operation not permitted (%s %d) on LV %s.",
+					 cmd->command->command_line_id, cmd->command->command_line_enum,
+					 display_lvname(lvl->lv));
+			}
+			continue;
 		}
 
 		log_very_verbose("Processing LV %s in VG %s.", lvl->lv->name, vg->name);

tools/toollib.h

@@ -159,6 +159,12 @@ const char *extract_vgname(struct cmd_context *cmd, const char *lv_name);
 const char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
 			 unsigned *dev_dir_found);
 
+int opt_in_list_is_set(struct cmd_context *cmd, int *opts, int count,
+		       int *match_count, int *unmatch_count);
+
+void opt_array_to_str(struct cmd_context *cmd, int *opts, int count,
+		      char *buf, int len);
+
 int pvcreate_params_from_args(struct cmd_context *cmd, struct pvcreate_params *pp);
 int pvcreate_each_device(struct cmd_context *cmd, struct processing_handle *handle, struct pvcreate_params *pp);
 

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 */
+/* define the enums for the values accepted by command line --options, foo_VAL */
 enum {
 #define val(a, b, c, d) a ,
 #include "vals.h"
 #undef val
 };
 
-/* define the enums for the command line --options */
+/* define the enums for the command line --options, foo_ARG */
 enum {
 #define arg(a, b, c, d, e, f) a ,
 #include "args.h"
@@ -69,6 +69,20 @@ 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 */
@@ -111,6 +125,21 @@ 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. */
@@ -162,6 +191,12 @@ 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);
@@ -203,4 +238,7 @@ 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

tools/vals.h

@@ -57,11 +57,13 @@
  * 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
- * 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.
+ * 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.
  *
  * The usage text for an option is not always the full
  * set of words accepted for an option, but may be a
@@ -92,9 +94,9 @@
  * should avoid suggesting the upper case letters.
  */
 
-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(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(bool_VAL, yes_no_arg, "Bool", "y|n")
 val(number_VAL, int_arg, "Number", NULL)
 val(string_VAL, string_arg, "String", NULL)
@@ -111,7 +113,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")
@@ -122,16 +124,13 @@ 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")
 
 /* 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
- */
-