commands: new method for defining commands

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

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

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

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

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

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

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

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

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

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

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

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

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

$ lvresize --help
  lvresize - Resize a logical volume

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

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

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

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

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

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

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

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

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

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

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

test/shell/listings.sh

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

test/shell/vgextend-restoremissing.sh

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

tools/args.h

@@ -81,19 +81,19 @@ arg(noudevsync_ARG, '\0', "noudevsync", 0, 0, 0)
 arg(originname_ARG, '\0', "originname", lv_VAL, 0, 0)
 arg(physicalvolumesize_ARG, '\0', "setphysicalvolumesize", sizemb_VAL, 0, 0)
 arg(poll_ARG, '\0', "poll", bool_VAL, 0, 0)
-arg(polloperation_ARG, '\0', "polloperation", polloperation_VAL, 0, 0)
+arg(polloperation_ARG, '\0', "polloperation", string_VAL, 0, 0)
 arg(pooldatasize_ARG, '\0', "pooldatasize", sizemb_VAL, 0, 0)
 arg(poolmetadata_ARG, '\0', "poolmetadata", lv_VAL, 0, 0)
 arg(poolmetadatasize_ARG, '\0', "poolmetadatasize", sizemb_VAL, 0, 0)
 arg(poolmetadataspare_ARG, '\0', "poolmetadataspare", bool_VAL, 0, 0)
 arg(profile_ARG, '\0', "profile", string_VAL, 0, 0)
-arg(pvmetadatacopies_ARG, '\0', "pvmetadatacopies", pvmetadatacopies_VAL, 0, 0)
-arg(raidrebuild_ARG, '\0', "raidrebuild", pv_VAL, ARG_GROUPABLE, 0)
+arg(pvmetadatacopies_ARG, '\0', "pvmetadatacopies", number_VAL, 0, 0)
+arg(raidrebuild_ARG, '\0', "raidrebuild", string_VAL, ARG_GROUPABLE, 0)
 arg(raidmaxrecoveryrate_ARG, '\0', "raidmaxrecoveryrate", sizekb_VAL, 0, 0)
 arg(raidminrecoveryrate_ARG, '\0', "raidminrecoveryrate", sizekb_VAL, 0, 0)
-arg(raidsyncaction_ARG, '\0', "raidsyncaction", syncaction_VAL, 0, 0)
+arg(raidsyncaction_ARG, '\0', "raidsyncaction", string_VAL, 0, 0)
 arg(raidwritebehind_ARG, '\0', "raidwritebehind", number_VAL, 0, 0)
-arg(raidwritemostly_ARG, '\0', "raidwritemostly", writemostly_VAL, ARG_GROUPABLE, 0)
+arg(raidwritemostly_ARG, '\0', "raidwritemostly", string_VAL, ARG_GROUPABLE, 0)
 arg(readonly_ARG, '\0', "readonly", 0, 0, 0)
 arg(refresh_ARG, '\0', "refresh", 0, 0, 0)
 arg(removemissing_ARG, '\0', "removemissing", 0, 0, 0)
@@ -116,7 +116,7 @@ arg(splitsnapshot_ARG, '\0', "splitsnapshot", 0, 0, 0)
 arg(showdeprecated_ARG, '\0', "showdeprecated", 0, 0, 0)
 arg(showunsupported_ARG, '\0', "showunsupported", 0, 0, 0)
 arg(stripes_long_ARG, '\0', "stripes", number_VAL, 0, 0)
-arg(syncaction_ARG, '\0', "syncaction", syncaction_VAL, 0, 0)
+arg(syncaction_ARG, '\0', "syncaction", string_VAL, 0, 0) /* FIXME Use custom VAL */
 arg(sysinit_ARG, '\0', "sysinit", 0, 0, 0)
 arg(systemid_ARG, '\0', "systemid", string_VAL, 0, 0)
 arg(thinpool_ARG, '\0', "thinpool", lv_VAL, 0, 0)
@@ -133,14 +133,14 @@ arg(unquoted_ARG, '\0', "unquoted", 0, 0, 0)
 arg(usepolicies_ARG, '\0', "usepolicies", 0, 0, 0)
 arg(validate_ARG, '\0', "validate", 0, 0, 0)
 arg(version_ARG, '\0', "version", 0, 0, 0)
-arg(vgmetadatacopies_ARG, '\0', "vgmetadatacopies", vgmetadatacopies_VAL, 0, 0)
+arg(vgmetadatacopies_ARG, '\0', "vgmetadatacopies", metadatacopies_VAL, 0, 0)
 arg(virtualoriginsize_ARG, '\0', "virtualoriginsize", sizemb_VAL, 0, 0)
 arg(withsummary_ARG, '\0', "withsummary", 0, 0, 0)
 arg(withcomments_ARG, '\0', "withcomments", 0, 0, 0)
 arg(withspaces_ARG, '\0', "withspaces", 0, 0, 0)
 arg(withversions_ARG, '\0', "withversions", 0, 0, 0)
 arg(writebehind_ARG, '\0', "writebehind", number_VAL, 0, 0)
-arg(writemostly_ARG, '\0', "writemostly", writemostly_VAL, ARG_GROUPABLE, 0)
+arg(writemostly_ARG, '\0', "writemostly", string_VAL, ARG_GROUPABLE, 0)
 
 /* Allow some variations */
 arg(allocation_ARG, '\0', "allocation", bool_VAL, 0, 0)

tools/command-lines.in

@@ -99,8 +99,10 @@
 # maxrecoveryrate (raidmaxrecoveryrate)
 # writebehind (raidwritebehind)
 # virtualsize (virtualoriginsize)
-# vgmetadatacopies (metadatacopies)
-# pvmetadatacopies (metadatacopies)
+#
+# metadatacopies/pvmetadatacopies/vgmetadatacopies are not considered
+# synonyms for the purpose of command definitions, but each variant is
+# specified when it is accepted.
 #
 # "---" is like a comment line, used to separate text for readability
 #
@@ -111,29 +113,7 @@
 # could use differnet IDs, in which case the implementation would
 # know where to look for each parameter.
 #
-# DESC: A description of the command.  The "DESC:" tags will be
-# 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.
-#
-# Another capability we might want to add here is a way to express
-# rules, per definition, of what arg combinations are allowed or
-# required, e.g.
-#
-# if --foo is set, then --bar cannot be set could be encoded as:
-# RULE_OPT_INCOMPAT_OPT: --foo --bar
-#
-# if --foo is set, then --bar is required could be encoded as:
-# RULE_OPT_REQUIRES_OPT: --foo --bar
-#
-# if --foo is set, then positional arg 1 is required
-# RULE_OPT_REQUIRES_POS: --foo 1
+# DESC: A description of the command.
 #
 
 #
@@ -157,10 +137,12 @@
 
 #
 # OO_ALL is included in every command automatically.
+# FIXME: add --force and --test to OO_ALL so that all commands will
+# accept them even if they are not used?
 #
 OO_ALL: --commandprofile String, --config String, --debug,
 --driverloaded Bool, --help, --profile String, --quiet,
---verbose, --version, --yes, --test
+--verbose, --version, --yes
 
 #
 # This list only applies to printing the usage text.
@@ -174,7 +156,7 @@ OO_ALL: --commandprofile String, --config String, --debug,
 # not relate to which optional opts are accepted by commands,
 # which is defined by the OO line.
 #
-OO_USAGE_COMMON: OO_ALL, --force, --noudevsync
+OO_USAGE_COMMON: OO_ALL, --force, --test, --noudevsync
 
 #
 # options for pvs, lvs, vgs, fullreport
@@ -200,7 +182,7 @@ OO_CONFIG: --atversion String, --typeconfig String, --file String, --ignoreadvan
 
 OO_LVCHANGE: --autobackup Bool, --force, --ignorelockingfailure,
 --ignoremonitoring, --ignoreskippedcluster, --noudevsync,
---reportformat String, --sysinit, --select String
+--reportformat String, --sysinit, --test, --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
@@ -213,7 +195,7 @@ 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 WriteMostlyPV
+--writebehind Number, --writemostly PV
 
 lvchange OO_LVCHANGE_META VG|LV|Tag|Select ...
 OO: OO_LVCHANGE
@@ -225,7 +207,7 @@ OO: OO_LVCHANGE
 ID: lvchange_resync
 DESC: Resyncronize a mirror or raid LV.
 
-lvchange --syncaction SyncAction VG|LV_raid|Tag|Select ...
+lvchange --syncaction String VG|LV_raid|Tag|Select ...
 OO: OO_LVCHANGE
 ID: lvchange_syncaction
 DESC: Resynchronize or check a raid LV.
@@ -269,7 +251,7 @@ OO_LVCONVERT_POOL: --poolmetadata LV, --poolmetadatasize SizeMB,
 --poolmetadataspare Bool, --readahead Readahead, --chunksize SizeKB
 
 OO_LVCONVERT: --alloc Alloc, --background, --force, --noudevsync,
---usepolicies
+--test, --usepolicies
 
 ---
 
@@ -311,17 +293,17 @@ DESC: Merge COW snapshot LV into its origin.
 # 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
+OO: --chunksize SizeKB, --zero Bool, OO_LVCONVERT
 ID: lvconvert_combine_split_snapshot
 DESC: Combine LV with a previously split snapshot LV.
-FLAGS: SECONDARY_SYNTAX
 
+# alternate form of lvconvert --type snapshot
 lvconvert --snapshot LV_linear_striped_raid LV_snapshot
-OO: --type snapshot, --chunksize SizeKB, --zero Bool, OO_LVCONVERT
+OO: --chunksize SizeKB, --zero Bool, OO_LVCONVERT
 ID: lvconvert_combine_split_snapshot
-DESC: Combine LV with a previously split snapshot LV.
+DESC: Combine LV with a previously split snapshot LV
+DESC: (variant, infers --type snapshot).
 
 ---
 
@@ -336,10 +318,13 @@ OO: --type thin, --originname LV_new, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVE
 ID: lvconvert_to_thin_with_external
 DESC: Convert LV to type thin with an external origin
 DESC: (variant, infers --type thin).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
+# FIXME: I don't think --zero applies when creating cache LV,
+# but it's used in a test.  Should the test be fixed and
+# --zero removed here?
+
 lvconvert --type cache --cachepool LV LV_linear_striped_raid_thinpool
 OO: --cache, --cachemode CacheMode, --cachepolicy String,
 --cachesettings String, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
@@ -352,10 +337,15 @@ OO: --type cache, --cachemode CacheMode, --cachepolicy String,
 --cachesettings String, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
 ID: lvconvert_to_cache_vol
 DESC: Convert LV to type cache (variant, infers --type cache).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
+# FIXME: stripes is an odd option, but if the pool metadata LV
+# is created by the command, that LV is created with the
+# specified number of stripes.  Maybe we could separate the
+# command definitions for cases where pool metadata LV is
+# created and limit the use of stripes to that case?
+
 lvconvert --type thin-pool LV_linear_striped_raid_cache
 OO: --stripes_long Number, --stripesize SizeKB,
 --discards Discards, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
@@ -365,14 +355,17 @@ DESC: Convert LV to type thin-pool.
 # alternate form of lvconvert --type thin-pool
 # deprecated because of non-standard syntax (missing positional arg)
 lvconvert --thinpool LV_linear_striped_raid_cache
-OO: --type thin-pool, --stripes_long Number, --stripesize SizeKB,
+OO: --stripes_long Number, --stripesize SizeKB,
 --discards Discards, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
 ID: lvconvert_to_thinpool
 DESC: Convert LV to type thin-pool (variant, use --type thin-pool).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
+# FIXME: I don't think that --cachemode, --cachepolicy, --cachesettings
+# are meant to be used when creating a cache pool, but they are used
+# in one test.  Should that test be fixed and these options removed?
+
 lvconvert --type cache-pool LV_linear_striped_raid
 OO: OO_LVCONVERT_POOL, OO_LVCONVERT,
 --cachemode CacheMode, --cachepolicy String, --cachesettings String
@@ -386,7 +379,6 @@ OO: --type cache-pool, OO_LVCONVERT_POOL, OO_LVCONVERT,
 --cachemode CacheMode, --cachepolicy String, --cachesettings String
 ID: lvconvert_to_cachepool
 DESC: Convert LV to type cache-pool (variant, use --type cache-pool).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -415,10 +407,23 @@ DESC: Convert LV from type raid1 to type mirror.
 
 ---
 
+# FIXME: by using specific raid levels, e.g. raid1, we could
+# specify other required options, e.g. --mirrors.  This may
+# help the second fixme...
+#
+# FIXME: there are two different operations here, and it would
+# be nice to split them into two unambiguous command lines:
+#
+# 1. lvconvert --type raid LV_linear_striped_mirror
+#    DESC: Convert LV to type raid.
+#
+# 2. lvconvert --type raid LV_raid
+#    DESC: Change LV raid type.
+
 lvconvert --type raid LV_linear_striped_mirror_raid
 OO: OO_LVCONVERT_RAID, OO_LVCONVERT
 OP: PV ...
-ID: lvconvert_to_raid
+ID: lvconvert_general_to_raid
 DESC: Convert LV to type raid.
 DESC: Change LV between raid types.
 
@@ -478,7 +483,6 @@ 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).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -562,7 +566,6 @@ lvconvert LV_mirror
 OO: OO_LVCONVERT
 ID: lvconvert_poll_mirror
 DESC: Poll mirror LV to collapse resync layers.
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -572,7 +575,6 @@ 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
 
 ---
 
@@ -582,7 +584,7 @@ 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 String, --setactivationskip Bool, --test, --wipesignatures Bool,
 --zero Bool
 
 OO_LVCREATE_CACHE: --cachemode CacheMode, --cachepolicy String, --cachesettings String
@@ -605,7 +607,6 @@ lvcreate --type error --size SizeMB VG
 OO: OO_LVCREATE
 ID: lvcreate_error_vol
 DESC: Create an LV that returns errors when used.
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -613,7 +614,6 @@ lvcreate --type zero --size SizeMB VG
 OO: OO_LVCREATE
 ID: lvcreate_zero_vol
 DESC: Create an LV that returns zeros when read.
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -628,7 +628,6 @@ OO: --mirrors 0, --stripes 1, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_linear
 DESC: Create a linear LV.
-FLAGS: SECONDARY_SYNTAX
 
 # This is the one place we mention the optional --name
 # because it's the most common case and may be confusing
@@ -672,6 +671,10 @@ DESC: Create a raid1 or mirror LV (variant, infers --type raid1|mirror).
 
 ---
 
+# FIXME: we should probably expand this from raid* to
+# various specific raid levels, then narrow the range
+# of options applicable to each.
+
 lvcreate --type raid --size SizeMB VG
 OO: OO_LVCREATE_RAID, OO_LVCREATE
 OP: PV ...
@@ -685,40 +688,43 @@ DESC: Create a raid LV (a specific raid level must be used, e.g. raid1.)
 # snapshot, perhaps "snapshot" should not be considered an LV type, but
 # another new LV property?
 #
-# This is the one case where the --type variant is the unpreferred,
-# secondary syntax, because the LV type is not actually "snapshot".
+# Or, perhaps we should not promote --type snapshot, and
+# prefer --snapshot as the primary syntax, since the type
+# is not really snapshot (which makes it different from
+# all the other cases of using --type, where the resulting
+# LV does have the specified type.)
 
-# alternate form of lvcreate --snapshot
 lvcreate --type snapshot --size SizeMB LV
 OO: --snapshot, --stripes Number, --stripesize SizeKB,
 --chunksize SizeKB, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_cow_snapshot
 DESC: Create a COW snapshot LV from an origin LV.
-FLAGS: SECONDARY_SYNTAX
 
+# alternate form of lvcreate --type snapshot
 lvcreate --snapshot --size SizeMB LV
 OO: --type snapshot, --stripes Number, --stripesize SizeKB,
 --chunksize SizeKB, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_cow_snapshot
-DESC: Create a COW snapshot LV from an origin LV.
+DESC: Create a COW snapshot LV from an origin LV
+DESC: (infers --type snapshot).
 
 ---
 
-# alternate form of lvcreate --snapshot
 lvcreate --type snapshot --size SizeMB --virtualsize SizeMB VG
 OO: --snapshot, --chunksize SizeKB, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_cow_snapshot_with_virtual_origin
 DESC: Create a sparse COW snapshot LV of a virtual origin LV.
-FLAGS: SECONDARY_SYNTAX
 
+# alternate form of lvcreate --type snapshot
 lvcreate --snapshot --size SizeMB --virtualsize SizeMB VG
 OO: --type snapshot, --chunksize SizeKB, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_cow_snapshot_with_virtual_origin
-DESC: Create a sparse COW snapshot LV of a virtual origin LV.
+DESC: Create a sparse COW snapshot LV of a virtual origin LV
+DESC: (infers --type snapshot).
 
 ---
 
@@ -736,7 +742,6 @@ OO: --type thin-pool, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
 OP: PV ...
 ID: lvcreate_thinpool
 DESC: Create a thin pool (variant, infers --type thin-pool).
-FLAGS: SECONDARY_SYNTAX
 
 # alternate form of lvcreate --type thin-pool
 lvcreate --size SizeMB --thinpool LV_new VG
@@ -746,29 +751,29 @@ OP: PV ...
 ID: lvcreate_thinpool
 DESC: Create a thin pool named by the --thinpool arg
 DESC: (variant, infers --type thin-pool).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
-# NB. there are no alternate forms of these commands that
-# use --cache in place of --type cache-pool, but --cache
-# still needs to be listed as an optional addition to
-# --type cache-pool.
-
 lvcreate --type cache-pool --size SizeMB VG
-OO: --cache, OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
+OO: OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_cachepool
 DESC: Create a cache pool.
 
+# alternate form of lvcreate --type cache-pool
+lvcreate --cache --size SizeMB VG
+OO: --type cache-pool, OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
+OP: PV ...
+ID: lvcreate_cachepool
+DESC: Create a cache pool (variant, infers --type cache-pool).
+
 # alternate form of lvcreate --type cache-pool
 lvcreate --type cache-pool --size SizeMB --cachepool LV_new VG
-OO: --cache, OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
+OO: OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
 OP: PV ...
 ID: lvcreate_cachepool
 DESC: Create a cache pool named by the --cachepool arg
 DESC: (variant, uses --cachepool in place of --name).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -783,17 +788,13 @@ OO: --thin, OO_LVCREATE_THIN, OO_LVCREATE
 ID: lvcreate_thin_vol
 DESC: Create a thin LV in a thin pool named in the first arg
 DESC: (variant, also see --thinpool for naming pool).
-FLAGS: SECONDARY_SYNTAX
-
-# NB. this is the variant which can substitute
-# --thin for --type thin, even though --thin is in OO.
 
 # alternate form of lvcreate --type thin
-lvcreate --virtualsize SizeMB --thinpool LV_thinpool VG
+lvcreate --virtualsize SizeMB --thinpool LV_thinpool
 OO: --type thin, --thin, OO_LVCREATE_THIN, OO_LVCREATE
+OP: VG
 ID: lvcreate_thin_vol
 DESC: Create a thin LV in a thin pool (variant, infers --type thin).
-FLAGS: SECONDARY_SYNTAX
 
 # alternate form of lvcreate --type thin
 lvcreate --virtualsize SizeMB LV_thinpool
@@ -802,7 +803,6 @@ ID: lvcreate_thin_vol
 DESC: Create a thin LV in the thin pool named in the first arg
 DESC: (variant, infers --type thin, also see --thinpool for
 DESC: naming pool.)
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -817,7 +817,6 @@ OO: --type thin, OO_LVCREATE_THIN, OO_LVCREATE
 ID: lvcreate_thin_snapshot
 DESC: Create a thin LV that is a snapshot of an existing thin LV
 DESC: (infers --type thin).
-FLAGS: SECONDARY_SYNTAX
 
 # alternate form of lvcreate --type thin
 lvcreate --snapshot LV_thin
@@ -827,7 +826,7 @@ DESC: Create a thin LV that is a snapshot of an existing thin LV
 DESC: (infers --type thin).
 
 lvcreate --type thin --thinpool LV_thinpool LV
-OO: --thin, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
+OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
 ID: lvcreate_thin_snapshot_of_external
 DESC: Create a thin LV that is a snapshot of an external origin LV.
 
@@ -844,20 +843,8 @@ DESC: (infers --type thin).
 # but here it applies to creating the new thin pool that
 # is used for the thin LV
 
-# FIXME: there are commands here that differ only in that
-# one takes LV_new in arg pos 1, and the other takes a VG name
-# in arg pos 1.  The commands that take LV_new use that
-# name as the new name of the pool, but the commands that
-# take a VG automatically generate the LV name.  The problem
-# is that currently the command matching function cannot
-# distinguish between an LV name and a VG name being used
-# in arg pos 1, so a user-entered command would just match
-# the first it finds and not necessarily the correct
-# definition.  Note that when LV_new is used in arg pos 1,
-# it needs to include a VG name, i.e. VG/LV_new
-
 lvcreate --type thin --virtualsize SizeMB --size SizeMB --thinpool LV_new
-OO: --thin, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
+OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
 --stripes Number, --stripesize SizeKB
 OP: PV ...
 ID: lvcreate_thin_vol_and_thinpool
@@ -873,31 +860,31 @@ ID: lvcreate_thin_vol_and_thinpool
 DESC: Create a thin LV, first creating a thin pool for it,
 DESC: where the new thin pool is named by the --thinpool arg
 DESC: (variant, infers --type thin).
-FLAGS: SECONDARY_SYNTAX
 
-# alternate form of lvcreate --type thin
-lvcreate --type thin --virtualsize SizeMB --size SizeMB LV_new|VG
-OO: --thin, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
+lvcreate --type thin --virtualsize SizeMB --size SizeMB LV_new
+OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
 --stripes Number, --stripesize SizeKB
 OP: PV ...
 ID: lvcreate_thin_vol_and_thinpool
 DESC: Create a thin LV, first creating a thin pool for it,
-DESC: where the new thin pool is named in the first arg,
-DESC: or the new thin pool name is generated when the first
-DESC: arg is a VG name.
-FLAGS: SECONDARY_SYNTAX
+DESC: where the new thin pool is named in the first arg.
 
-# alternate form of lvcreate --type thin
-lvcreate --thin --virtualsize SizeMB --size SizeMB LV_new|VG
+lvcreate --thin --virtualsize SizeMB --size SizeMB LV_new
 OO: --type thin, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
 --stripes Number, --stripesize SizeKB
 OP: PV ...
 ID: lvcreate_thin_vol_and_thinpool
 DESC: Create a thin LV, first creating a thin pool for it,
-DESC: where the new thin pool is named in the first arg,
-DESC: or the new thin pool name is generated when the first
-DESC: arg is a VG name (variant, infers --type thin).
-FLAGS: SECONDARY_SYNTAX
+DESC: where the new thin pool is named in the first arg
+DESC: (variant, infers --type thin).
+
+lvcreate --type thin --virtualsize SizeMB --size SizeMB VG
+OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE,
+--stripes Number, --stripesize SizeKB
+OP: PV ...
+ID: lvcreate_thin_vol_and_thinpool
+DESC: Create a thin LV, first creating a thin pool for it,
+DESC: where the new thin pool name is generated automatically.
 
 ---
 
@@ -912,7 +899,6 @@ DESC: Create a sparse snapshot of a virtual origin LV
 DESC: (infers --type snapshot).
 DESC: Chooses --type thin or --type snapshot according to
 DESC: config setting sparse_segtype_default.
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -938,7 +924,6 @@ ID: lvcreate_cache_vol_with_new_origin
 DESC: Create a cache LV, first creating a new origin LV,
 DESC: then combining it with the existing cache pool named
 DESC: by the --cachepool arg (variant, infers --type cache).
-FLAGS: SECONDARY_SYNTAX
 
 # alternate form of lvcreate --type cache
 lvcreate --type cache --size SizeMB LV_cachepool
@@ -949,7 +934,6 @@ ID: lvcreate_cache_vol_with_new_origin
 DESC: Create a cache LV, first creating a new origin LV,
 DESC: then combining it with the existing cache pool named
 DESC: in the first arg (variant, also use --cachepool).
-FLAGS: SECONDARY_SYNTAX
 
 # This is a ridiculously crazy command which nobody could
 # understand.  It should be be eliminated.  It does two different
@@ -990,8 +974,7 @@ FLAGS: SECONDARY_SYNTAX
 # def1: alternate form of lvcreate --type cache, or
 # def2: it should be done by lvconvert.
 lvcreate --cache --size SizeMB LV
-OO: OO_LVCREATE_CACHE, OO_LVCREATE_POOL, OO_LVCREATE,
---stripes Number, --stripesize SizeKB
+OO: OO_LVCREATE_CACHE, OO_LVCREATE, --stripes Number, --stripesize SizeKB
 OP: PV ...
 ID: lvcreate_cache_vol_with_new_origin_or_convert_to_cache_vol_with_cachepool
 DESC: When LV is a cache pool, create a cache LV,
@@ -1001,7 +984,6 @@ DESC: (variant, infers --type cache, also use --cachepool).
 DESC: When LV is not a cache pool, convert the specified LV
 DESC: to type cache after creating a new cache pool LV to use
 DESC: (use lvconvert).
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
@@ -1022,7 +1004,7 @@ ID: lvdisplay_general
 lvextend --size SizeMB LV
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --nofsck, --nosync, --noudevsync, --reportformat String, --resizefs,
---stripes Number, --stripesize SizeKB, --poolmetadatasize SizeMB
+--stripes Number, --stripesize SizeKB, --test, --poolmetadatasize SizeMB
 OP: PV ...
 ID: lvextend_by_size
 DESC: Extend an LV by a specified size.
@@ -1030,15 +1012,16 @@ 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 String, --resizefs, --stripes Number, --stripesize SizeKB,
+--test
 ID: lvextend_by_pv
-DESC: Extend an LV by specified PV extents.
-FLAGS: SECONDARY_SYNTAX
+DESC: Extend an LV by a specified PV.
 
 lvextend --poolmetadatasize SizeMB LV_thinpool
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --nofsck, --nosync, --noudevsync,
---reportformat String, --stripes Number, --stripesize SizeKB
+--reportformat String, --stripes Number, --stripesize SizeKB,
+--test
 OP: PV ...
 ID: lvextend_pool_metadata_by_size
 DESC: Extend a pool metadata SubLV by a specified size.
@@ -1046,7 +1029,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 String, --resizefs,
+--test
 ID: lvextend_by_policy
 DESC: Extend an LV according to a predefined policy.
 
@@ -1060,24 +1044,24 @@ ID: lvmconfig_general
 
 lvreduce --size SizeMB LV
 OO: --autobackup Bool, --force, --nofsck, --noudevsync,
---reportformat String, --resizefs
+--reportformat String, --resizefs, --test
 ID: lvreduce_general
 
 ---
 
 lvremove VG|LV|Tag|Select ...
 OO: --autobackup Bool, --force, --nohistory, --noudevsync,
---reportformat String, --select String
+--reportformat String, --select String, --test
 ID: lvremove_general
 
 ---
 
 lvrename VG LV LV_new
-OO: --autobackup Bool, --noudevsync, --reportformat String
+OO: --autobackup Bool, --noudevsync, --reportformat String, --test
 ID: lvrename_vg_lv_lv
 
 lvrename LV LV_new
-OO: --autobackup Bool, --noudevsync, --reportformat String
+OO: --autobackup Bool, --noudevsync, --reportformat String, --test
 ID: lvrename_lv_lv
 
 ---
@@ -1085,7 +1069,7 @@ ID: lvrename_lv_lv
 lvresize --size SizeMB LV
 OO: --alloc Alloc, --autobackup Bool, --force,
 --nofsck, --nosync, --noudevsync, --reportformat String, --resizefs,
---stripes Number, --stripesize SizeKB, --poolmetadatasize SizeMB
+--stripes Number, --stripesize SizeKB, --test, --poolmetadatasize SizeMB
 OP: PV ...
 ID: lvresize_by_size
 DESC: Resize an LV by a specified size.
@@ -1093,15 +1077,16 @@ 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 String, --resizefs, --stripes Number, --stripesize SizeKB,
+--test
 ID: lvresize_by_pv
-DESC: Resize an LV by specified PV extents.
-FLAGS: SECONDARY_SYNTAX
+DESC: Resize an LV by a specified PV.
 
 lvresize --poolmetadatasize SizeMB LV_thinpool
 OO: --alloc Alloc, --autobackup Bool, --force,
 --nofsck, --nosync, --noudevsync,
---reportformat String, --stripes Number, --stripesize SizeKB
+--reportformat String, --stripes Number, --stripesize SizeKB,
+--test
 OP: PV ...
 ID: lvresize_pool_metadata_by_size
 DESC: Resize a pool metadata SubLV by a specified size.
@@ -1124,7 +1109,7 @@ ID: lvscan_general
 
 # None of these can function as a required option for pvchange.
 OO_PVCHANGE: --autobackup Bool, --force, --ignoreskippedcluster,
---reportformat String, --uuid
+--reportformat String, --test, --uuid
 
 # Any of these can function as a required option for pvchange.
 OO_PVCHANGE_META: --allocatable Bool, --addtag Tag, --deltag Tag,
@@ -1143,7 +1128,7 @@ DESC: Change properties of specified PVs.
 ---
 
 pvresize PV ...
-OO: --setphysicalvolumesize SizeMB, --reportformat String
+OO: --setphysicalvolumesize SizeMB, --reportformat String, --test
 ID: pvresize_general
 
 ---
@@ -1159,8 +1144,8 @@ ID: pvck_general
 
 pvcreate PV ...
 OO: --dataalignment SizeKB, --dataalignmentoffset SizeKB, --bootloaderareasize SizeMB,
---force, --labelsector Number, --metadatatype MetadataType,
---pvmetadatacopies MetadataCopiesPV, --metadatasize SizeMB,
+--force, --test, --labelsector Number, --metadatatype MetadataType,
+--pvmetadatacopies Number, --metadatacopies MetadataCopies, --metadatasize SizeMB,
 --metadataignore Bool, --norestorefile, --setphysicalvolumesize SizeMB,
 --reportformat String, --restorefile String, --uuidstr String, --zero Bool
 ID: pvcreate_general
@@ -1180,20 +1165,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 String, --test
 OP: PV ...
 ID: pvmove_one
 DESC: Move PV extents.
 
 pvmove
-OO: --abort, --background
+OO: --abort, --background, --test
 ID: pvmove_any
 DESC: Continue or abort existing pvmove operations.
 
 ---
 
 pvremove PV ...
-OO: --force, --reportformat String
+OO: --force, --reportformat String, --test
 ID: pvremove_general
 
 ---
@@ -1229,7 +1214,7 @@ ID: vgcfgbackup_general
 ---
 
 vgcfgrestore VG
-OO: --file String, --force_long, --list, --metadatatype MetadataType
+OO: --file String, --force_long, --list, --metadatatype MetadataType, --test
 ID: vgcfgrestore_by_vg
 
 vgcfgrestore --list --file String
@@ -1240,7 +1225,7 @@ ID: vgcfgrestore_by_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 String, --select String, --test, --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,50 +1233,50 @@ OO_VGCHANGE: --autobackup Bool, --ignoremonitoring, --ignoreskippedcluster,
 
 OO_VGCHANGE_META: --addtag Tag, --deltag Tag,
 --logicalvolume Number, --maxphysicalvolumes Number, --alloc Alloc, --uuid,
---clustered Bool, --pvmetadatacopies MetadataCopiesPV, --vgmetadatacopies MetadataCopiesVG,
+--clustered Bool, --metadatacopies MetadataCopies, --vgmetadatacopies MetadataCopies,
 --physicalextentsize SizeMB, --resizeable Bool, --systemid String, --locktype LockType,
 --profile String, --detachprofile, --metadataprofile String,
 
 vgchange OO_VGCHANGE_META
 OO: OO_VGCHANGE
-OP: VG|Tag|Select ...
+OP: VG|Tag ...
 ID: vgchange_properties
 DESC: Change a general VG property.
 
 vgchange --monitor Bool
-OO: --sysinit, --ignorelockingfailure, --poll Bool, OO_VGCHANGE
-OP: VG|Tag|Select ...
+OO: --sysinit, --ignorelockingfailure, --poll Bool, OO_VGCHANGE_META, OO_VGCHANGE
+OP: VG|Tag ...
 ID: vgchange_monitor
 DESC: Start or stop monitoring LVs from dmeventd.
 
 vgchange --poll Bool
-OO: --ignorelockingfailure, OO_VGCHANGE
-OP: VG|Tag|Select ...
+OO: --ignorelockingfailure, OO_VGCHANGE_META, OO_VGCHANGE
+OP: VG|Tag ...
 ID: vgchange_poll
 DESC: Start or stop processing LV conversions.
 
 vgchange --activate Active
 OO: --activationmode ActivationMode, --ignoreactivationskip, --partial, --sysinit,
---ignorelockingfailure, --monitor Bool, --poll Bool, OO_VGCHANGE
-OP: VG|Tag|Select ...
+--ignorelockingfailure, --monitor Bool, --poll Bool, OO_VGCHANGE_META, OO_VGCHANGE
+OP: VG|Tag ...
 ID: vgchange_activate
 DESC: Activate or deactivate LVs.
 
 vgchange --refresh
-OO: --sysinit, --ignorelockingfailure, --monitor Bool, --poll Bool, OO_VGCHANGE
-OP: VG|Tag|Select ...
+OO: --sysinit, --ignorelockingfailure, --monitor Bool, --poll Bool, OO_VGCHANGE_META, OO_VGCHANGE
+OP: VG|Tag ...
 ID: vgchange_refresh
 DESC: Reactivate LVs using the latest metadata.
 
 vgchange --lockstart
-OO: --lockopt String, OO_VGCHANGE
-OP: VG|Tag|Select ...
+OO: --lockopt String, OO_VGCHANGE_META, OO_VGCHANGE
+OP: VG|Tag ...
 ID: vgchange_lockstart
 DESC: Start the lockspace of a shared VG in lvmlockd.
 
 vgchange --lockstop
-OO: --lockopt String, OO_VGCHANGE
-OP: VG|Tag|Select ...
+OO: --lockopt String, OO_VGCHANGE_META, OO_VGCHANGE
+OP: VG|Tag ...
 ID: vgchange_lockstop
 DESC: Stop the lockspace of a shared VG in lvmlockd.
 
@@ -1305,8 +1290,8 @@ ID: vgck_general
 ---
 
 vgconvert VG ...
-OO: --force, --labelsector Number, --bootloaderareasize SizeMB,
---metadatatype MetadataType, --pvmetadatacopies MetadataCopiesPV,
+OO: --force, --test, --labelsector Number, --bootloaderareasize SizeMB,
+--metadatatype MetadataType, --pvmetadatacopies Number,
 --metadatasize SizeMB, --reportformat String
 ID: vgconvert_general
 
@@ -1315,9 +1300,9 @@ ID: vgconvert_general
 vgcreate VG_new PV ...
 OO: --addtag Tag, --alloc Alloc, --autobackup Bool, --clustered Bool, --maxlogicalvolumes Number,
 --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,
+--physicalextentsize SizeMB, --test, --force, --zero Bool, --labelsector Number,
+--metadatasize SizeMB, --pvmetadatacopies Number, --reportformat String, --metadatacopies MetadataCopies,
+--vgmetadatacopies MetadataCopies, --dataalignment SizeKB, --dataalignmentoffset SizeKB,
 --shared, --systemid String, --locktype LockType, --lockopt String
 ID: vgcreate_general
 
@@ -1334,7 +1319,7 @@ ID: vgdisplay_general
 
 ---
 
-OO_VGEXPORT: --reportformat String
+OO_VGEXPORT: --reportformat String, --test
 
 vgexport VG|Tag|Select ...
 OO: --select String, OO_VGEXPORT
@@ -1345,21 +1330,20 @@ vgexport --all
 OO: OO_VGEXPORT
 ID: vgexport_all
 DESC: Export all VGs.
-FLAGS: SECONDARY_SYNTAX
 
 ---
 
 vgextend VG PV ...
-OO: --autobackup Bool,
+OO: --autobackup Bool, --test,
 --force, --zero Bool, --labelsector Number, --metadatatype MetadataType,
---metadatasize SizeMB, --pvmetadatacopies MetadataCopiesPV,
+--metadatasize SizeMB, --pvmetadatacopies Number,
 --metadataignore Bool, --dataalignment SizeKB, --dataalignmentoffset SizeKB,
 --reportformat String, --restoremissing
 ID: vgextend_general
 
 ---
 
-OO_VGIMPORT: --force, --reportformat String
+OO_VGIMPORT: --force, --reportformat String, --test
 
 vgimport VG|Tag|Select ...
 OO: --select String, OO_VGIMPORT
@@ -1374,13 +1358,13 @@ DESC: Import all VGs.
 ---
 
 vgimportclone PV ...
-OO: --basevgname VG, --import
+OO: --basevgname VG, --test, --import
 ID: vgimportclone_general
 
 ---
 
 vgmerge VG VG
-OO: --autobackup Bool, --list
+OO: --autobackup Bool, --list, --test
 ID: vgmerge_general
 
 ---
@@ -1392,7 +1376,7 @@ ID: vgmknodes_general
 
 ---
 
-OO_VGREDUCE: --autobackup Bool, --force, --reportformat String
+OO_VGREDUCE: --autobackup Bool, --force, --reportformat String, --test
 
 vgreduce VG PV ...
 OO: OO_VGREDUCE
@@ -1412,18 +1396,18 @@ DESC: Remove all missing PVs from a VG.
 ---
 
 vgremove VG|Tag|Select ...
-OO: --force, --noudevsync, --reportformat String, --select String
+OO: --force, --noudevsync, --reportformat String, --select String, --test
 ID: vgremove_general
 
 ---
 
 vgrename VG VG_new
-OO: --autobackup Bool, --force, --reportformat String
+OO: --autobackup Bool, --force, --reportformat String, --test
 ID: vgrename_by_name
 DESC: Rename a VG.
 
 vgrename String VG_new
-OO: --autobackup Bool, --force, --reportformat String
+OO: --autobackup Bool, --force, --reportformat String, --test
 ID: vgrename_by_uuid
 DESC: Rename a VG by specifying the VG UUID.
 
@@ -1443,12 +1427,34 @@ ID: vgscan_general
 
 ---
 
-OO_VGSPLIT: --autobackup Bool
+OO_VGSPLIT: --autobackup Bool, --test
 
-# used only when the destination VG is new
 OO_VGSPLIT_NEW: --alloc Alloc, --clustered Bool,
 --maxlogicalvolumes Number, --maxphysicalvolumes Number,
---metadatatype MetadataType, --vgmetadatacopies MetadataCopiesVG
+--metadatatype MetadataType, --vgmetadatacopies MetadataCopies
+
+# FIXME: it would be nice to have separate definitions
+# for the cases where the destination VG exists or is new,
+# but when choosing the command definition, we don't yet
+# know if the destination VG exists or not.  So to do this
+# we'd need a command option to specify if the VG is new.
+#
+# this won't work:
+#   vgsplit VG VG PV
+#   ID: vgsplit_by_pv_to_existing
+#
+#   vgsplit VG VG_new PV ...
+#   ID: vgsplit_by_pv_to_new
+#
+# but this could:
+#   vgsplit VG VG PV
+#   ID: vgsplit_by_pv_to_existing
+#
+#   vgsplit --create-new VG VG_new PV ...
+#   ID: vgsplit_by_pv_to_new
+#
+# Then the OO_VGSPLIT_NEW options could be
+# included in the second case but not the first.
 
 vgsplit VG VG PV ...
 OO: OO_VGSPLIT, OO_VGSPLIT_NEW
@@ -1489,8 +1495,8 @@ lastlog
 OO: --reportformat String, --select String
 ID: lastlog_general
 
-lvpoll --polloperation PollOp LV ...
-OO: --abort, --autobackup Bool, --handlemissingpvs, --interval Number
+lvpoll --polloperation String LV ...
+OO: --abort, --autobackup Bool, --handlemissingpvs, --interval Number, --test
 ID: lvpoll_general
 
 formats

tools/command.h

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

tools/create-commands.c

@@ -47,13 +47,7 @@ int segtype_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
 int alloc_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
 int locktype_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
 int readahead_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; }
-int vgmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int pvmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
 int metadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int polloperation_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int writemostly_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-int syncaction_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; }
-
 
 /* also see arg_props */
 struct opt_name {
@@ -251,7 +245,7 @@ static int opt_str_to_num(char *str)
 	exit(1);
 }
 
-static char *val_bits_to_str(uint64_t val_bits)
+static char *val_bits_to_str(int val_bits)
 {
 	static char buf[128];
 	int i;
@@ -583,13 +577,6 @@ static int is_desc_line(char *str)
 	return 0;
 }
 
-static int is_flags_line(char *str)
-{
-	if (!strncmp(str, "FLAGS:", 6))
-		return 1;
-	return 0;
-}
-
 static int is_id_line(char *str)
 {
 	if (!strncmp(str, "ID:", 3))
@@ -1160,46 +1147,14 @@ static char *flags_to_str(int flags)
 
 	memset(buf_flags, 0, sizeof(buf_flags));
 
-	if (flags & ARG_DEF_FLAG_MAY_REPEAT) {
-		if (buf_flags[0])
-			strcat(buf_flags, " | ");
+	if (flags & ARG_DEF_FLAG_MAY_REPEAT)
 		strcat(buf_flags, "ARG_DEF_FLAG_MAY_REPEAT");
-	}
-	if (flags & ARG_DEF_FLAG_NEW) {
-		if (buf_flags[0])
-			strcat(buf_flags, " | ");
+	if (flags & ARG_DEF_FLAG_NEW)
 		strcat(buf_flags, "ARG_DEF_FLAG_NEW");
-	}
 
 	return buf_flags;
 }
 
-static void add_flags(struct command *cmd, char *line)
-{
-	if (strstr(line, "SECONDARY_SYNTAX"))
-		cmd->cmd_flags |= CMD_FLAG_SECONDARY_SYNTAX;
-}
-
-static char *cmd_flags_to_str(uint32_t flags)
-{
-	static char buf_cmd_flags[32];
-
-	memset(buf_cmd_flags, 0, sizeof(buf_cmd_flags));
-
-	if (flags & CMD_FLAG_SECONDARY_SYNTAX) {
-		if (buf_cmd_flags[0])
-			strcat(buf_cmd_flags, " | ");
-		strcat(buf_cmd_flags, "CMD_FLAG_SECONDARY_SYNTAX");
-	}
-	if (flags & CMD_FLAG_ONE_REQUIRED_OPT) {
-		if (buf_cmd_flags[0])
-			strcat(buf_cmd_flags, " | ");
-		strcat(buf_cmd_flags, "CMD_FLAG_ONE_REQUIRED_OPT");
-	}
-
-	return buf_cmd_flags;
-}
-
 void print_command_count(void)
 {
 	struct command *cmd;
@@ -2115,10 +2070,8 @@ void print_command_struct(int only_usage)
 		printf("commands[%d].oo_count = %d;\n", i, cmd->oo_count);
 		printf("commands[%d].op_count = %d;\n", i, cmd->op_count);
 
-		if (cmd->cmd_flags)
-			printf("commands[%d].cmd_flags = %s;\n", i, cmd_flags_to_str(cmd->cmd_flags));
-		else
-			printf("commands[%d].cmd_flags = 0;\n", i, cmd_flags_to_str(cmd->cmd_flags));
+		if (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT)
+			printf("commands[%d].cmd_flags = CMD_FLAG_ONE_REQUIRED_OPT;\n", i);
 
 		printf("commands[%d].desc = \"%s\";\n", i, cmd->desc ?: "");
 		printf("commands[%d].usage = ", i);
@@ -2456,11 +2409,6 @@ int main(int argc, char *argv[])
 			continue;
 		}
 
-		if (is_flags_line(line_argv[0])) {
-			add_flags(cmd, line_orig);
-			continue;
-		}
-
 		if (is_id_line(line_argv[0])) {
 			cmd->command_line_id = strdup(line_argv[1]);
 			continue;

tools/lvmcmdline.c

@@ -733,69 +733,23 @@ int readahead_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_va
 /*
  * Non-zero, positive integer, "all", or "unmanaged"
  */
-int vgmetadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
+int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
 {
-	if (!strcasecmp(av->value, "all")) {
-		av->ui_value = VGMETADATACOPIES_ALL;
-		return 1;
-	}
+	if (!strncmp(cmd->name, "vg", 2)) {
+		if (!strcasecmp(av->value, "all")) {
+			av->ui_value = VGMETADATACOPIES_ALL;
+			return 1;
+		}
 
-	if (!strcasecmp(av->value, "unmanaged")) {
-		av->ui_value = VGMETADATACOPIES_UNMANAGED;
-		return 1;
+		if (!strcasecmp(av->value, "unmanaged")) {
+			av->ui_value = VGMETADATACOPIES_UNMANAGED;
+			return 1;
+		}
 	}
 
 	return int_arg(cmd, av);
 }
 
-int pvmetadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	int num;
-
-	if (!int_arg(cmd, av))
-		return 0;
-
-	num = av->i_value;
-
-	if ((num != 0) && (num != 1) && (num != 2))
-		return 0;
-
-	return 1;
-}
-
-int metadatacopies_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strncmp(cmd->name, "pv", 2))
-		return pvmetadatacopies_arg(cmd, av);
-	if (!strncmp(cmd->name, "vg", 2))
-		return vgmetadatacopies_arg(cmd, av);
-	return 0;
-}
-
-int polloperation_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "pvmove") ||
-	    !strcmp(av->value, "convert") ||
-	    !strcmp(av->value, "merge") ||
-	    !strcmp(av->value, "merge_thin"))
-		return 1;
-	return 0;
-}
-
-int writemostly_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	/* Could we verify that a PV arg looks like /dev/foo ? */
-	return 1;
-}
-
-int syncaction_arg(struct cmd_context *cmd, struct arg_values *av)
-{
-	if (!strcmp(av->value, "check") ||
-	    !strcmp(av->value, "repair"))
-		return 1;
-	return 0;
-}
-
 /*
  * FIXME: there's been a confusing mixup among:
  * resizeable, resizable, allocatable, allocation.
@@ -836,14 +790,6 @@ static int _opt_standard_to_synonym(const char *cmd_name, int opt)
 		return raidwritebehind_ARG;
 	case virtualsize_ARG:
 		return virtualoriginsize_ARG;
-	case pvmetadatacopies_ARG:
-		if (!strncmp(cmd_name, "pv", 2))
-			return metadatacopies_ARG;
-		return 0;
-	case vgmetadatacopies_ARG:
-		if (!strncmp(cmd_name, "vg", 2))
-			return metadatacopies_ARG;
-		return 0;
 	}
 	return 0;
 }
@@ -873,12 +819,6 @@ static int _opt_synonym_to_standard(const char *cmd_name, int opt)
 		return writebehind_ARG;
 	case virtualoriginsize_ARG:
 		return virtualsize_ARG;
-	case metadatacopies_ARG:
-		if (!strncmp(cmd_name, "pv", 2))
-			return pvmetadatacopies_ARG;
-		if (!strncmp(cmd_name, "vg", 2))
-			return vgmetadatacopies_ARG;
-		return 0;
 	}
 	return 0;
 }
@@ -1549,14 +1489,16 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	}
 
 	/*
-	 * If the user provided a positional arg that is not accepted by
-	 * the mached command, then fail.
+	 * Warn about positional args that are set but are not used by the command.
 	 *
 	 * If the last required_pos_arg or the last optional_pos_arg may repeat,
 	 * then there won't be unused positional args.
 	 *
-	 * FIXME: same question as above, should there be a config setting
-	 * to just warn/ignore about unused positional args?
+	 * Otherwise, warn about positional args that exist beyond the number of
+	 * required + optional pos_args.
+	 *
+	 * FIXME: should an unused positional arg cause the command to fail
+	 * like an unused option?
 	 */
 
 	count = commands[best_i].rp_count;
@@ -1572,15 +1514,10 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			break;
 
 		if (count >= (commands[best_i].rp_count + commands[best_i].op_count)) {
-			log_error("Invalid positional argument for command (%s %d): %s.",
-				  commands[best_i].command_line_id, best_i, argv[count]);
-
-			/* FIXME: to warn/ignore, clear so it can't be used when processing. */
-			/*
+			log_warn("Ignoring positional argument which is not used by this command: %s.", argv[count]);
+			/* clear so it can't be used when processing. */
 			argv[count] = NULL;
 			(*argc)--;
-			*/
-			return NULL;
 		}
 	}
 out:
@@ -1611,9 +1548,6 @@ static int _usage(const char *name, int help_count)
 		if (strcmp(_cmdline.commands[i].name, name))
 			continue;
 
-		if ((_cmdline.commands[i].cmd_flags & CMD_FLAG_SECONDARY_SYNTAX) && (help_count < 3))
-			continue;
-
 		if (strlen(_cmdline.commands[i].desc))
 			_print_description(i);
 
@@ -1650,11 +1584,6 @@ static int _usage(const char *name, int help_count)
 		log_print(". The _new suffix indicates the VG or LV must not yet exist.");
 		log_print(". LV followed by _<type> indicates that an LV of the given type");
 		log_print("  is required.  (raid represents any raid<N> type.)");
-		log_print(". The default output unit is specified by letter, followed by |unit");
-		log_print("  which represents other possible units: hHbBsSkKmMgGtTpPeE.");
-		log_print(". Output units are 1024 SI base, regardless of unit capitalization.");
-		log_print(". Use --help --help --help to print secondary command syntax");
-		log_print("  formats that are recognized, e.g. for compatibility.");
 		log_print(". See man pages for short option equivalents of long option names,");
 		log_print("  and for more detailed descriptions of variable parameters.");
 	}

tools/toollib.c

@@ -801,7 +801,10 @@ int vgcreate_params_set_from_args(struct cmd_context *cmd,
 		return 0;
 	}
 
-	if (arg_is_set(cmd, vgmetadatacopies_ARG))
+	if (arg_is_set(cmd, metadatacopies_ARG))
+		vp_new->vgmetadatacopies = arg_int_value(cmd, metadatacopies_ARG,
+							DEFAULT_VGMETADATACOPIES);
+	else if (arg_is_set(cmd, vgmetadatacopies_ARG))
 		vp_new->vgmetadatacopies = arg_int_value(cmd, vgmetadatacopies_ARG,
 							DEFAULT_VGMETADATACOPIES);
 	else
@@ -4001,6 +4004,11 @@ int pvcreate_params_from_args(struct cmd_context *cmd, struct pvcreate_params *p
 	if (pp->pva.pvmetadatacopies < 0)
 		pp->pva.pvmetadatacopies = find_config_tree_int(cmd, metadata_pvmetadatacopies_CFG, NULL);
 
+	if (pp->pva.pvmetadatacopies > 2) {
+		log_error("Metadatacopies may only be 0, 1 or 2");
+		return 0;
+	}
+
 	pp->pva.ba_size = arg_uint64_value(cmd, bootloaderareasize_ARG, pp->pva.ba_size);
 
 	return 1;

tools/tools.h

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

tools/vals.h

@@ -92,9 +92,9 @@
  * should avoid suggesting the upper case letters.
  */
 
-val(none_VAL, NULL, "None", "ERR")             /* unused, for enum value 0 */
-val(conststr_VAL, NULL, "ConstString", "ERR")  /* used only for command defs */
-val(constnum_VAL, NULL, "ConstNumber", "ERR")  /* used only for command defs */
+val(none_VAL, NULL, "None", "")             /* unused, for enum value 0 */
+val(conststr_VAL, NULL, "ConstString", "")  /* used only for command defs */
+val(constnum_VAL, NULL, "ConstNumber", "")  /* used only for command defs */
 val(bool_VAL, yes_no_arg, "Bool", "y|n")
 val(number_VAL, int_arg, "Number", NULL)
 val(string_VAL, string_arg, "String", NULL)
@@ -119,20 +119,17 @@ val(segtype_VAL, segtype_arg, "SegType", "linear|striped|snapshot|mirror|raid*|t
 val(alloc_VAL, alloc_arg, "Alloc", "contiguous|cling|cling_by_tags|normal|anywhere|inherit")
 val(locktype_VAL, locktype_arg, "LockType", "sanlock|dlm|none")
 val(readahead_VAL, readahead_arg, "Readahead", "auto|none|NumberSectors")
-val(vgmetadatacopies_VAL, vgmetadatacopies_arg, "MetadataCopiesVG", "all|unmanaged|Number")
-val(pvmetadatacopies_VAL, pvmetadatacopies_arg, "MetadataCopiesPV", "0|1|2")
-val(metadatacopies_VAL, metadatacopies_arg, "unused", "unused")
-val(polloperation_VAL, polloperation_arg, "PollOp", "pvmove|convert|merge|merge_thin")
-val(writemostly_VAL, writemostly_arg, "WriteMostlyPV", "PV[:t|n|y]")
-val(syncaction_VAL, syncaction_arg, "SyncAction", "check|repair")
+val(metadatacopies_VAL, metadatacopies_arg, "MetadataCopies", "all|unmanaged|Number")
 
 /* this should always be last */
 val(VAL_COUNT, NULL, NULL, NULL)
 
 /*
- * These are also candidates for a custom VAL enum:
- * reportformat_ARG: basic|json
- * configreport_ARG: log|vg|lv|pv|pvseg|seg
- * configtype_ARG:   current|default|diff|full|list|missing|new|profilable|profilable-command|profilable-metadata
+ * FIXME: I suspect many of the following are good candidates for a custom VAL
+ * enum for the benefit of custom parsing, or custom usage, or both:
+ *
+ * configreport_ARG, configtype_ARG, polloperation_ARG, raidrebuild_ARG,
+ * raidsyncaction_ARG, raidwritemostly_ARG, reportformat_ARG, syncaction_ARG,
+ * cachepolicy_ARG, cachesettings_ARG, writemostly_ARG
  */
 

tools/vgchange.c

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

tools/vgconvert.c

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

tools/vgextend.c

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