commands: new method for defining commands

Only a command that conforms to one of the specifications
will be processed.  (LV type conformance is checked after
vg_read and before single_fn processing.)

scripts/command-lines.in

@@ -54,7 +54,8 @@
 # it is followed by ..., e.g. VG|LV|Tag ...
 #
 # LV can have a suffix indicating the LV type, e.g. LV_linear, LV_thinpool.
-# LV_raid represents any raidN.
+# LV_raid represents any raidN.  LV_type1_type2_type3 when the LV is
+# limited to multiple specific types.
 #
 # VG, LV can have the suffix _new, indicating the named VG or LV
 # does not yet exist.
@@ -84,11 +85,6 @@
 # maxrecoveryrate (raidmaxrecoveryrate)
 # writebehind (raidwritebehind)
 #
-# DESC: describe what the command does, with a separate
-# sentence ending in '.' for each unique operation.
-# A new line is printed after each '.' so be careful
-# about where '.' is used.
-#
 
 #
 # For efficiency, sets of options can be defined and reused
@@ -118,6 +114,19 @@ OO_ALL: --commandprofile String, --config String, --debug,
 --driverloaded Bool, --help, --profile String, --quiet,
 --verbose, --version, --yes
 
+#
+# These common options are displayed once at the end of
+# a given command's usage.  This is done to avoid excessive
+# repetition of common options, which may obscure the more
+# interesting and relevant parts of a common prototype.
+# This definition is *only* used when generating the command
+# usage strings, and is the basis for the division between
+# the "usage" and "usage_common" strings. This OO defn does
+# not relate to which optional opts are accepted by commands,
+# which is defined by the OO line.
+#
+OO_USAGE_COMMON: OO_ALL, --force, --test, --noudevsync
+
 #
 # pvs, lvs, vgs, fullreport
 #
@@ -221,72 +230,77 @@ OO_LVCONVERT: --alloc Alloc, --background, --force, --noudevsync,
 --test, --usepolicies
 
 # FIXME: use different option names for different operations
-lvconvert --merge LV_linear|LV_striped|LV_raid|LV_thin|LV_snapshot|VG|Tag ...
+lvconvert --merge LV_linear_striped_raid_thin_snapshot|VG|Tag ...
 OO: --background, --interval Number
 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 snapshot LV_linear|LV_striped|LV_raid LV_snapshot
+# FIXME: by using two different positional args, this is the
+# single violation of the standard method of using process_each_lv().
+# Before calling process_each, it steals the first positional arg
+# and adjusts argv/argc so it's not seen by process_each.
+lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
 OO: --chunksize SizeKB, --zero Bool, OO_LVCONVERT
 DESC: Combine LV with a previously split snapshot LV.
 
-lvconvert --type thin --thinpool LV LV_linear|LV_striped|LV_raid
+lvconvert --type thin --thinpool LV LV_linear_striped_raid
 OO: --originname LV_new, OO_LVCONVERT_POOL, OO_LVCONVERT
 DESC: Convert LV to type thin with an external origin.
 
 # alternate form of lvconvert --type thin
-lvconvert --thin --thinpool LV LV_linear|LV_striped|LV_raid
+lvconvert --thin --thinpool LV LV_linear_striped_raid
 OO: --type thin, --originname LV_new, OO_LVCONVERT_POOL, OO_LVCONVERT
 DESC: Convert LV to type thin with an external origin (infers --type thin).
 
-lvconvert --type cache --cachepool LV LV_linear|LV_striped|LV_raid|LV_thinpool
+lvconvert --type cache --cachepool LV LV_linear_striped_raid_thinpool
 OO: --cachepolicy String, --cachesettings String, OO_LVCONVERT_POOL, OO_LVCONVERT
 DESC: Convert LV to type cache.
 
 # alternate form of lvconvert --type cache
-lvconvert --cache --cachepool LV LV_linear|LV_striped|LV_raid|LV_thinpool
+lvconvert --cache --cachepool LV LV_linear_striped_raid_thinpool
 OO: --type cache, --cachepolicy String, --cachesettings String, OO_LVCONVERT_POOL, OO_LVCONVERT
 DESC: Convert LV to type cache (infers --type cache).
 
-lvconvert --type thin-pool LV_linear|LV_striped|LV_raid|LV_cache
+lvconvert --type thin-pool LV_linear_striped_raid_cache
 OO: --discards Discards, --zero Bool, OO_LVCONVERT_POOL, OO_LVCONVERT
 DESC: Convert LV to type thin-pool.
 
-lvconvert --type cache-pool LV_linear|LV_striped|LV_raid
+lvconvert --type cache-pool LV_linear_striped_raid
 OO: OO_LVCONVERT_POOL, OO_LVCONVERT
 DESC: Convert LV to type cache-pool.
 
-lvconvert --type mirror LV_linear|LV_striped|LV_raid
+lvconvert --type mirror LV_linear_striped_raid
 OO: OO_LVCONVERT_RAID, OO_LVCONVERT
 OP: PV ...
 DESC: Convert LV to type mirror.
 
-lvconvert --type raid LV_linear|LV_striped|LV_mirror|LV_raid
+lvconvert --type raid LV_linear_striped_mirror_raid
 OO: OO_LVCONVERT_RAID, OO_LVCONVERT
 OP: PV ...
 DESC: Convert LV to type raid.
 DESC: Change LV raid type.
 
-lvconvert --mirrors SNumber LV_raid|LV_mirror
+lvconvert --mirrors SNumber LV_raid_mirror
 OO: OO_LVCONVERT
 OP: PV ...
 DESC: Change the number of mirror images in the LV.
 
-lvconvert --mirrors SNumber LV_linear|LV_striped
+lvconvert --mirrors SNumber LV_linear_striped
 OO: OO_LVCONVERT_RAID, OO_LVCONVERT
 OP: PV ...
-DESC: Alternate form to convert LV to type raid1 or mirror (use --type raid1|mirror).
+DESC: Alternate form to convert LV to type raid1 or mirror
+DESC: (use --type raid1|mirror).
 
-lvconvert --splitmirrors Number --name LV_new LV_raid|LV_mirror|LV_cache
+lvconvert --splitmirrors Number --name LV_new LV_raid1_mirror_cache
 OO: OO_LVCONVERT
 DESC: Split images from a raid1 or mirror LV and use them to create a new LV.
 
-lvconvert --splitmirrors Number --trackchanges LV_raid|LV_cache
+lvconvert --splitmirrors Number --trackchanges LV_raid1_cache
 OO: OO_LVCONVERT
 DESC: Split images from a raid1 LV and use them to create a new LV.
 
-lvconvert --repair LV_raid|LV_mirror|LV_thinpool
+lvconvert --repair LV_raid_mirror_thinpool
 OO: OO_LVCONVERT
 DESC: Replace failed PVs in a mirror or raid LV.
 DESC: Repair a thin pool.
@@ -301,7 +315,7 @@ OO: OO_LVCONVERT_RAID, OO_LVCONVERT
 OP: PV ...
 DESC: Convert LV to type striped.
 
-lvconvert --type linear LV_raid|LV_mirror
+lvconvert --type linear LV_raid_mirror
 OO: OO_LVCONVERT
 DESC: Convert LV to type linear.
 
@@ -309,11 +323,11 @@ lvconvert --mirrorlog MirrorLog LV_mirror
 OO: OO_LVCONVERT
 DESC: Change the type of log used by LV.
 
-lvconvert --splitcache LV_cachepool|LV_cache|LV_thinpool
+lvconvert --splitcache LV_cachepool_cache_thinpool
 OO: OO_LVCONVERT
 DESC: Separate and preserve a cache pool from a cache LV.
 
-lvconvert --uncache LV_cache|LV_thinpool
+lvconvert --uncache LV_cache_thinpool
 OO: OO_LVCONVERT
 DESC: Separate and remove a cache pool from a cache LV.
 
@@ -337,7 +351,7 @@ lvconvert LV_mirror
 DESC: Poll LV to collapse resync layers.
 
 # FIXME: add a new option defining this operation, e.g. --swapmetadata
-lvconvert --poolmetadata LV LV_thinpool|LV_cachepool
+lvconvert --poolmetadata LV LV_thinpool_cachepool
 DESC: Swap metadata LV in a thin pool or cache pool (temporary command).
 
 
@@ -385,7 +399,7 @@ DESC: Create a mirror LV.
 lvcreate --type raid --size SizeMB VG
 OO: OO_LVCREATE_RAID, OO_LVCREATE
 OP: PV ...
-DESC: Create a raid LV (a specific raid level must be used, for example, raid1).
+DESC: Create a raid LV (a specific raid level must be used, e.g. raid1.
 
 lvcreate --type snapshot --size SizeMB LV
 OO: OO_LVCREATE
@@ -417,17 +431,20 @@ DESC: Create a thin LV that is a snapshot of an existing thin LV.
 
 lvcreate --type thin --snapshot --thinpool LV_thinpool LV
 OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
-DESC: Create a thin LV that is a snapshot of an external origin LV named in arg pos 1.
+DESC: Create a thin LV that is a snapshot of an external origin LV
+DESC: named in arg pos 1.
 
 lvcreate --type thin --virtualsize SizeMB --size SizeMB --thinpool LV_new
 OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
 OP: PV ...
-DESC: Create a thin LV, first creating a thin pool for it, where the new thin pool is named by the --thinpool arg.
+DESC: Create a thin LV, first creating a thin pool for it,
+DESC: where the new thin pool is named by the --thinpool arg.
 
 lvcreate --type thin --virtualsize SizeMB --size SizeMB LV_new
 OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
 OP: PV ...
-DESC: Create a thin LV, first creating a thin pool for it, where the new thin pool is named in arg pos 1.
+DESC: Create a thin LV, first creating a thin pool for it,
+DESC: where the new thin pool is named in arg pos 1.
 
 lvcreate --type thin --virtualsize SizeMB --size SizeMB VG
 OO: OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
@@ -438,17 +455,19 @@ DESC: Create a thin LV, first creating a thin pool for it.
 lvcreate --type cache --size SizeMB LV
 OO: OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
 OP: PV ...
-DESC: Convert the specified LV to type cache after creating a new cache pool LV to use.
+DESC: Convert the specified LV to type cache after creating a new
+DESC: cache pool LV to use.
 
 lvcreate --type cache --size SizeMB --cachepool LV_cachepool
 OO: OO_LVCREATE_POOL, OO_LVCREATE_CACHE, OO_LVCREATE
 OP: PV ...
-DESC: Create a cache LV, first creating a new origin LV, then combining it with the existing cache pool in arg pos 1.
+DESC: Create a cache LV, first creating a new origin LV,
+DESC: then combining it with the existing cache pool in arg pos 1.
 
 lvcreate --size SizeMB VG
 OO: --type linear, OO_LVCREATE
 OP: PV ...
-DESC: Create a linear LV. (default --type linear)
+DESC: Create a linear LV, --name is usually specified (default --type linear).
 
 lvcreate --stripes Number --size SizeMB VG
 OO: --type striped, --stripesize SizeKB, OO_LVCREATE
@@ -463,7 +482,8 @@ DESC: Create a raid1 or mirror LV (infers --type raid1|mirror).
 lvcreate --snapshot --size SizeMB LV
 OO: --type snapshot, OO_LVCREATE
 OP: PV ...
-DESC: Create a COW snapshot LV of the origin LV in arg pos 1. (infers --type snapshot)
+DESC: Create a COW snapshot LV of the origin LV in arg pos 1
+DESC: (infers --type snapshot).
 
 lvcreate --thin --size SizeMB VG
 OO: --type thin-pool, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
@@ -477,11 +497,13 @@ DESC: Create a cache pool (infers --type cache-pool).
 
 lvcreate --snapshot LV_thin
 OO: --type thin, OO_LVCREATE_THIN, OO_LVCREATE
-DESC: Create a thin LV that is a snapshot of an existing thin LV (infers --type thin).
+DESC: Create a thin LV that is a snapshot of an existing thin LV
+DESC: (infers --type thin).
 
 lvcreate --snapshot --thinpool LV_thinpool LV
 OO: --type thin, OO_LVCREATE_THIN, OO_LVCREATE
-DESC: Create a thin LV that is a snapshot of an external origin LV (infers --type thin).
+DESC: Create a thin LV that is a snapshot of an external origin LV
+DESC: (infers --type thin).
 
 lvcreate --virtualsize SizeMB --thinpool LV_thinpool
 OO: --type thin, OO_LVCREATE_THIN, OO_LVCREATE
@@ -490,17 +512,20 @@ DESC: Create a thin LV in a thin pool (infers --type thin).
 lvcreate --size SizeMB --cachepool LV_cachepool
 OO: --type cache, OO_LVCREATE_CACHE, OO_LVCREATE
 OP: PV ...
-DESC: Create a new origin LV, combining it with an existing cache pool to create a new cache LV (infers --type cache).
+DESC: Create a new origin LV, combining it with an existing
+DESC: cache pool to create a new cache LV (infers --type cache).
 
 lvcreate --thin --virtualsize SizeMB --size SizeMB --thinpool LV_new
 OO: --type thin, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
 OP: PV ...
-DESC: Create a thin LV, first creating a thin pool for it, where the new thin pool is named by the --thinpool arg (infers --type thin).
+DESC: Create a thin LV, first creating a thin pool for it, where the new
+DESC: thin pool is named by the --thinpool arg (infers --type thin).
 
 lvcreate --thin --virtualsize SizeMB --size SizeMB LV_new
 OO: --type thin, OO_LVCREATE_POOL, OO_LVCREATE_THIN, OO_LVCREATE
 OP: PV ...
-DESC: Create a thin LV, first creating a thin pool for it, where the new thin pool is named in arg pos 1 (infers --type thin).
+DESC: Create a thin LV, first creating a thin pool for it, where the new
+DESC: thin pool is named in arg pos 1 (infers --type thin).
 
 lvcreate --size SizeMB --virtualsize SizeMB VG
 OO: --type thin, --type snapshot, --thin, --snapshot,
@@ -539,7 +564,7 @@ OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --test
 OP: PV ...
 
-lvextend --usepolicies LV_thinpool|LV_snapshot
+lvextend --usepolicies LV_thinpool_snapshot
 OO: --alloc Alloc, --autobackup Bool, --force, --mirrors SNumber,
 --nofsck, --nosync, --noudevsync,
 --reportformat String, --resizefs,

scripts/create-commands.c

@@ -19,7 +19,6 @@
 #include <ctype.h>
 #include <getopt.h>
 
-
 /* needed to include args.h */
 #define ARG_COUNTABLE 0x00000001
 #define ARG_GROUPABLE 0x00000002
@@ -133,6 +132,8 @@ struct oo_line {
 int cmd_count;
 struct command cmd_array[MAX_CMDS];
 
+struct command common_options; /* for printing common usage */
+
 #define MAX_OO_LINES 256
 int oo_line_count;
 struct oo_line oo_lines[MAX_OO_LINES];
@@ -140,6 +141,36 @@ struct oo_line oo_lines[MAX_OO_LINES];
 
 static void add_optional_opt_line(struct command *cmd, int argc, char *argv[]);
 
+/*
+ * modifies buf, replacing the sep characters with \0
+ * argv pointers point to positions in buf
+ */
+
+static char *split_line(char *buf, int *argc, char **argv, char sep)
+{
+	char *p = buf, *rp = NULL;
+	int i;
+
+	argv[0] = p;
+
+	for (i = 1; i < MAX_LINE_ARGC; i++) {
+		p = strchr(buf, sep);
+		if (!p)
+			break;
+		*p = '\0';
+
+		argv[i] = p + 1;
+		buf = p + 1;
+	}
+	*argc = i;
+
+	/* we ended by hitting \0, return the point following that */
+	if (!rp)
+		rp = strchr(buf, '\0') + 1;
+
+	return rp;
+}
+
 /* convert value string, e.g. Number, to foo_VAL enum */
 
 static int val_str_to_num(char *str)
@@ -237,103 +268,104 @@ static char *val_bits_to_str(int val_bits)
  * are not exposed to lvm at large, which uses only the ARG_DEF values.
  */
 
-static int lv_str_to_num(char *str)
+static uint32_t lv_str_to_types(char *str)
 {
-	char name[32] = { 0 };
-	char *new;
+	char copy[128] = { 0 };
+	char *argv[MAX_LINE_ARGC];
+	int argc;
+	char *name;
+	uint32_t types = 0;
 	int i;
 
-	/* compare the lv name before the _new suffix */
+	strncpy(copy, str, 128);
 
-	strncpy(name, str, 31);
-	if ((new = strstr(name, "_new")))
-		*new = '\0';
+	split_line(copy, &argc, argv, '_');
 
-	if (!strcmp(name, "LV"))
-		return 0;
+	for (i = 0; i < argc; i++) {
+		name = argv[i];
 
-	if (!strcmp(name, "LV_linear"))
-		return ARG_DEF_LV_LINEAR;
+		if (!strcmp(name, "linear"))
+			types |= ARG_DEF_LV_LINEAR;
 
-	if (!strcmp(name, "LV_striped"))
-		return ARG_DEF_LV_STRIPED;
+		if (!strcmp(name, "striped"))
+			types |= ARG_DEF_LV_STRIPED;
 
-	if (!strcmp(name, "LV_snapshot"))
-		return ARG_DEF_LV_SNAPSHOT;
+		if (!strcmp(name, "snapshot"))
+			types |= ARG_DEF_LV_SNAPSHOT;
 
-	if (!strcmp(name, "LV_mirror"))
-		return ARG_DEF_LV_MIRROR;
+		if (!strcmp(name, "mirror"))
+			types |= ARG_DEF_LV_MIRROR;
 
-	if (!strcmp(name, "LV_raid"))
-		return ARG_DEF_LV_RAID;
+		if (!strcmp(name, "thin"))
+			types |= ARG_DEF_LV_THIN;
 
-	if (!strcmp(name, "LV_raid0"))
-		return ARG_DEF_LV_RAID0;
+		if (!strcmp(name, "thinpool"))
+			types |= ARG_DEF_LV_THINPOOL;
 
-	if (!strcmp(name, "LV_raid1"))
-		return ARG_DEF_LV_RAID1;
+		if (!strcmp(name, "cache"))
+			types |= ARG_DEF_LV_CACHE;
 
-	if (!strcmp(name, "LV_raid4"))
-		return ARG_DEF_LV_RAID4;
+		if (!strcmp(name, "cachepool"))
+			types |= ARG_DEF_LV_CACHEPOOL;
 
-	if (!strcmp(name, "LV_raid5"))
-		return ARG_DEF_LV_RAID5;
+		if (!strcmp(name, "raid0"))
+			types |= ARG_DEF_LV_RAID0;
 
-	if (!strcmp(name, "LV_raid6"))
-		return ARG_DEF_LV_RAID6;
+		if (!strcmp(name, "raid1"))
+			types |= ARG_DEF_LV_RAID1;
 
-	if (!strcmp(name, "LV_raid10"))
-		return ARG_DEF_LV_RAID10;
+		if (!strcmp(name, "raid4"))
+			types |= ARG_DEF_LV_RAID4;
 
-	if (!strcmp(name, "LV_thin"))
-		return ARG_DEF_LV_THIN;
+		if (!strcmp(name, "raid5"))
+			types |= ARG_DEF_LV_RAID5;
 
-	if (!strcmp(name, "LV_thinpool"))
-		return ARG_DEF_LV_THINPOOL;
+		if (!strcmp(name, "raid6"))
+			types |= ARG_DEF_LV_RAID6;
 
-	if (!strcmp(name, "LV_cache"))
-		return ARG_DEF_LV_CACHE;
+		if (!strcmp(name, "raid10"))
+			types |= ARG_DEF_LV_RAID10;
 
-	if (!strcmp(name, "LV_cachepool"))
-		return ARG_DEF_LV_CACHEPOOL;
+		if (!strcmp(name, "raid"))
+			types |= ARG_DEF_LV_RAID;
+	}
 
-	printf("Unknown LV type: \"%s\" \"%s\"\n", name, str);
-	exit(1);
+	return types;
 }
 
 static const char *lv_num_to_str(int num)
 {
 	switch (num) {
 	case ARG_DEF_LV_LINEAR:
-		return "LV_linear";
+		return "linear";
 	case ARG_DEF_LV_STRIPED:
-		return "LV_striped";
+		return "striped";
 	case ARG_DEF_LV_SNAPSHOT:
-		return "LV_snapshot";
+		return "snapshot";
 	case ARG_DEF_LV_MIRROR:
-		return "LV_mirror";
+		return "mirror";
 	case ARG_DEF_LV_RAID:
-		return "LV_raid";
+		return "raid";
 	case ARG_DEF_LV_RAID0:
-		return "LV_raid0";
+		return "raid0";
 	case ARG_DEF_LV_RAID1:
-		return "LV_raid1";
+		return "raid1";
 	case ARG_DEF_LV_RAID4:
-		return "LV_raid4";
+		return "raid4";
 	case ARG_DEF_LV_RAID5:
-		return "LV_raid5";
+		return "raid5";
 	case ARG_DEF_LV_RAID6:
-		return "LV_raid6";
+		return "raid6";
 	case ARG_DEF_LV_RAID10:
-		return "LV_raid10";
+		return "raid10";
 	case ARG_DEF_LV_THIN:
-		return "LV_thin";
+		return "thin";
 	case ARG_DEF_LV_THINPOOL:
-		return "LV_thinpool";
+		return "thinpool";
 	case ARG_DEF_LV_CACHE:
-		return "LV_cache";
+		return "cache";
 	case ARG_DEF_LV_CACHEPOOL:
-		return "LV_cachepool";
+		return "cachepool";
 	default:
 		printf("lv_num_to_str: unknown LV num: %d\n", num);
 		exit(1);
@@ -440,36 +472,6 @@ static char *lv_types_to_flags(int lv_types)
 	return buf_lv_types;
 }
 
-/*
- * modifies buf, replacing the sep characters with \0
- * argv pointers point to positions in buf
- */
-
-static char *split_line(char *buf, int *argc, char **argv, char sep)
-{
-	char *p = buf, *rp = NULL;
-	int i;
-
-	argv[0] = p;
-
-	for (i = 1; i < MAX_LINE_ARGC; i++) {
-		p = strchr(buf, sep);
-		if (!p)
-			break;
-		*p = '\0';
-
-		argv[i] = p + 1;
-		buf = p + 1;
-	}
-	*argc = i;
-
-	/* we ended by hitting \0, return the point following that */
-	if (!rp)
-		rp = strchr(buf, '\0') + 1;
-
-	return rp;
-}
-
 static const char *is_command_name(char *str)
 {
 	int i;
@@ -574,7 +576,7 @@ static void set_pos_def(struct command *cmd, char *str, struct arg_def *def)
 		def->val_bits |= val_enum_to_bit(val_enum);
 
 		if ((val_enum == lv_VAL) && strstr(name, "_"))
-			def->lv_types |= lv_str_to_num(name);
+			def->lv_types = lv_str_to_types(name);
 
 		if (strstr(name, "_new"))
 			def->flags |= ARG_DEF_FLAG_NEW;
@@ -626,7 +628,7 @@ static void set_opt_def(struct command *cmd, char *str, struct arg_def *def)
 
 		if (val_enum == lv_VAL) {
 			if (strstr(name, "_"))
-				def->lv_types |= lv_str_to_num(name);
+				def->lv_types = lv_str_to_types(name);
 		}
 
 		if ((val_enum == vg_VAL) || (val_enum == lv_VAL) || (val_enum == pv_VAL)) {
@@ -956,20 +958,7 @@ static void print_def(struct arg_def *def, int usage)
 			else if (val_enum == constnum_VAL)
 				printf("ll%u", (unsigned long long)def->num);
 
-			else if ((val_enum == lv_VAL) && !def->lv_types) {
-				if (sep) printf("|");
-				printf("%s", val_names[val_enum].name);
-				sep = 1;
-
-			} else if ((val_enum == lv_VAL) && def->lv_types) {
-				for (i = 0; i < 32; i++) {
-					if (def->lv_types & (1 << i)) {
-						if (sep) printf("|");
-						printf("%s", lv_num_to_str(1 << i));
-						sep = 1;
-					}
-				}
-			} else {
+			else {
 				if (sep) printf("|");
 
 				if (!usage || !val_names[val_enum].usage)
@@ -980,6 +969,13 @@ static void print_def(struct arg_def *def, int usage)
 				sep = 1;
 			}
 
+			if (val_enum == lv_VAL && def->lv_types) {
+				for (i = 0; i < 32; i++) {
+					if (def->lv_types & (1 << i))
+						printf("_%s", lv_num_to_str(1 << i));
+				}
+			}
+
 			if ((val_enum == pv_VAL) || (val_enum == vg_VAL) || (val_enum == lv_VAL)) {
 				if (def->flags & ARG_DEF_FLAG_NEW)
 					printf("_new");
@@ -1084,10 +1080,35 @@ void print_define_command_count(void)
 	printf("#define COMMAND_COUNT %d\n", cmd_count);
 }
 
-void print_usage(struct command *cmd)
+static int is_common_opt(int opt)
+{
+	int oo;
+
+	for (oo = 0; oo < common_options.oo_count; oo++) {
+		if (common_options.optional_opt_args[oo].opt == opt)
+			return 1;
+	}
+	return 0;
+}
+
+/*
+ * For certain commands (esp commands like lvcreate with many variants), common
+ * options should not be printed for every variation, but once for all.  The
+ * list of commands this applies to is fixed for now but could be encoded in
+ * command-lines.in.
+ *
+ * The common options are defined in OO_USAGE_COMMON.  Those options
+ * are skipped when creating the usage strings for each variation of
+ * these commands.  Instead they are set in the usage_common string.
+ */
+
+void print_usage(struct command *cmd, int skip_required)
 {
 	int onereq = (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) ? 1 : 0;
-	int i, ro, rp, oo, op;
+	int i, sep, ro, rp, oo, op;
+
+	if (skip_required)
+		goto oo_count;
 
 	printf("\"%s", cmd->name);
 
@@ -1119,25 +1140,37 @@ void print_usage(struct command *cmd)
 
 	printf("\"");
 
+ oo_count:
 	if (!cmd->oo_count)
 		goto op_count;
 
-	printf("\n");
-	printf("\" [");
+	sep = 0;
 
 	if (cmd->oo_count) {
 		for (oo = 0; oo < cmd->oo_count; oo++) {
-			if (oo)
+			/* skip common opts which are in the usage_common string */
+			if ((cmd != &common_options) && is_common_opt(cmd->optional_opt_args[oo].opt))
+				continue;
+
+			if (!sep) {
+				printf("\n");
+				printf("\" [");
+			}
+
+			if (sep)
 				printf(",");
+
 			printf(" %s", opt_names[cmd->optional_opt_args[oo].opt].long_opt);
 			if (cmd->optional_opt_args[oo].def.val_bits) {
 				printf(" ");
 				print_def(&cmd->optional_opt_args[oo].def, 1);
 			}
+			sep = 1;
 		}
 	}
 
-	printf(" ]\"");
+	if (sep)
+		printf(" ]\"");
 
  op_count:
 	if (!cmd->op_count)
@@ -1166,6 +1199,8 @@ void print_command_struct(int only_usage)
 	struct command *cmd;
 	int i, j, ro, rp, oo, op;
 
+	include_optional_opt_args(&common_options, "OO_USAGE_COMMON");
+
 	printf("/* Do not edit. This file is generated by scripts/create-commands */\n");
 	printf("/* using command definitions from scripts/command-lines.in */\n");
 
@@ -1173,7 +1208,8 @@ void print_command_struct(int only_usage)
 		cmd = &cmd_array[i];
 
 		if (only_usage) {
-			print_usage(cmd);
+			print_usage(cmd, 0);
+			print_usage(&common_options, 1);
 			printf("\n");
 			continue;
 		}
@@ -1190,7 +1226,14 @@ void print_command_struct(int only_usage)
 
 		printf("commands[%d].desc = \"%s\";\n", i, cmd->desc ?: "");
 		printf("commands[%d].usage = ", i);
-		print_usage(cmd);
+		print_usage(cmd, 0);
+
+		if (cmd->oo_count) {
+			printf("commands[%d].usage_common = ", i);
+			print_usage(&common_options, 1);
+		} else {
+			printf("commands[%d].usage_common = \"NULL\";\n", i);
+		}
 
 		if (cmd->ro_count) {
 			for (ro = 0; ro < cmd->ro_count; ro++) {
@@ -1444,7 +1487,7 @@ int main(int argc, char *argv[])
 		}
 
 		if (is_desc_line(line_argv[0])) {
-			char *desc = strdup(strstr(line_orig, ":") + 2);
+			char *desc = strdup(line_orig);
 			if (cmd->desc) {
 				cmd->desc = realloc((char *)cmd->desc, strlen(cmd->desc) + strlen(desc) + 2);
 				strcat((char *)cmd->desc, "  ");

tools/command.h

@@ -111,7 +111,8 @@ struct pos_arg {
 struct command {
 	const char *name;
 	const char *desc; /* specific command description from command-lines.h */
-	const char *usage;
+	const char *usage; /* excludes common options like --help, --debug */
+	const char *usage_common; /* includes commmon options like --help, --debug */
 
 	struct command_name *cname;
 	command_fn fn;

tools/lvmcmdline.c

@@ -963,16 +963,20 @@ static int _command_required_pos_matches(struct cmd_context *cmd, int ci, int rp
 
 #define HELP_LINE_SIZE 1024
 
-static void _print_usage(int ci, int include_optional)
+static void _print_usage(const char *usage, int only_required)
 {
-	const char *usage = _cmdline.commands[ci].usage;
-	char buf[HELP_LINE_SIZE] = {0};
+	char buf[HELP_LINE_SIZE];
 	int optional_ui = 0;
-	int ui = 0;
-	int bi = 0;
+	int optional_pos_ui = 0;
+	int ui;
+	int bi;
+
+	if (!usage || !strlen(usage))
+		return;
 
 	/*
-	 * Print the required portions of the usage string.
+	 * copy the required opt_args/pos_args
+	 *
  	 * The optional portions of the usage string are enclosed
 	 * in [] and follow the required portions.
 	 *
@@ -981,63 +985,123 @@ static void _print_usage(int ci, int include_optional)
 	 * include [ in cases like --option Number[units].
 	 */
 
+	memset(buf, 0, sizeof(buf));
+	bi = 0;
+
 	for (ui = 0; ui < strlen(usage); ui++) {
+		if (!bi && ((usage[ui] == ' ') || (usage[ui] == '\n')))
+			continue;
+
+		/* The first "[ " indicates the start of the optional opt_args. */
 		if ((usage[ui] == '[') && (usage[ui+1] == ' ')) {
 			optional_ui = ui;
 			break;
 		}
+
 		if (usage[ui] == '\0')
 			break;
-		if (usage[ui] == '\n')
-			break;
 
 		buf[bi++] = usage[ui];
 
 		if (usage[ui] == ',') {
 			buf[bi++] = '\n';
 			buf[bi++] = '\t';
+			buf[bi++] = ' ';
 		}
 
 		if (bi == (HELP_LINE_SIZE - 1))
 			break;
 	}
 
-	if (!include_optional) {
-		log_print("%s\n", buf);
-		return;
-	}
+	/*
+	 * print the required opt_args/pos_args
+	 */
 
-	log_print("%s", buf);
+	if (bi)
+		log_print("%s", buf);
+
+	if (only_required)
+		return;
+
+	/*
+	 * copy the optional opt_args
+	 */
+
+	if (!optional_ui)
+		goto out;
 
 	memset(buf, 0, sizeof(buf));
 	bi = 0;
 
-	buf[bi++] = '\t';
-
 	for (ui = optional_ui; ui < strlen(usage); ui++) {
+
+		/* The second "[ " indicates the start of the optional pos_args. */
+		if ((ui > optional_ui) && (usage[ui] == '[') && (usage[ui+1] == ' ')) {
+			optional_pos_ui = ui;
+			break;
+		}
+
 		if (usage[ui] == '\0')
 			break;
 		if (usage[ui] == '\n')
 			break;
 
+		if (!bi)
+			buf[bi++] = '\t';
+
 		buf[bi++] = usage[ui];
 
 		if (usage[ui] == ',') {
 			buf[bi++] = '\n';
 			buf[bi++] = '\t';
+			buf[bi++] = ' ';
 		}
 
 		if (bi == (HELP_LINE_SIZE - 1))
 			break;
 	}
 
-	log_print("%s\n", buf);
-}
+	/*
+	 * print the optional opt_args
+	 */
 
-/*
- * A description is a string with multiple sentences each ending in periods.
- * Print each sentence on a new line.
- */
+	if (bi)
+		log_print("%s", buf);
+
+	/*
+	 * copy the optional pos_args
+	 */
+
+	if (!optional_pos_ui)
+		goto out;
+
+	memset(buf, 0, sizeof(buf));
+	bi = 0;
+
+	for (ui = optional_pos_ui; ui < strlen(usage); ui++) {
+		if (usage[ui] == '\0')
+			break;
+		if (usage[ui] == '\n')
+			break;
+
+		if (!bi)
+			buf[bi++] = '\t';
+
+		buf[bi++] = usage[ui];
+
+		if (bi == (HELP_LINE_SIZE - 1))
+			break;
+	}
+
+	/*
+	 * print the optional pos_args
+	 */
+
+	if (bi)
+		log_print("%s", buf);
+ out:
+	return;
+}
 
 static void _print_description(int ci)
 {
@@ -1052,20 +1116,27 @@ static void _print_description(int ci)
 		if (desc[di] == '\n')
 			continue;
 
+		if (!strncmp(&desc[di], "DESC:", 5)) {
+			if (bi) {
+				log_print("%s", buf);
+				memset(buf, 0, sizeof(buf));
+				bi = 0;
+			}
+			di += 5;
+			continue;
+		}
+
 		if (!bi && desc[di] == ' ')
 			continue;
 
 		buf[bi++] = desc[di];
 
-		if (desc[di] == '.') {
-			log_print("%s", buf);
-			memset(buf, 0, sizeof(buf));
-			bi = 0;
-		}
-
 		if (bi == (HELP_LINE_SIZE - 1))
 			break;
 	}
+
+	if (bi)
+		log_print("%s", buf);
 }
 
 static struct command *_find_command(struct cmd_context *cmd, const char *path, int *argc, char **argv)
@@ -1073,7 +1144,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	const char *name;
 	int match_count, match_count_ro, match_count_rp, mismatch_count;
 	int best_i = 0, best_count = 0;
-	int closest_i = 0, closest_count = 0;
+	int closest_i = 0, closest_count_ro = 0;
 	int ro, rp;
 	int i, j;
 	int accepted, count;
@@ -1143,10 +1214,10 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 		/* if cmd is missing any required opt/pos args, it can't be this command. */
 
 		if (mismatch_count) {
-			/* save i/match_count for "closest" command that doesn't match */
-			if (!closest_count || (match_count > closest_count)) {
+			/* save "closest" command that doesn't match */
+			if (match_count_ro && (match_count_ro > closest_count_ro)) {
 				closest_i = i;
-				closest_count = match_count;
+				closest_count_ro = match_count_ro;
 			}
 			continue;
 		}
@@ -1178,10 +1249,10 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 
 	if (!best_count) {
 		/* cmd did not have all the required opt/pos args of any command */
-		log_error("Failed to find a matching command definition.\n");
-		if (closest_count) {
+		log_error("Failed to find a matching command definition.");
+		if (closest_count_ro) {
 			log_warn("Closest command usage is:");
-			_print_usage(closest_i, 0);
+			_print_usage(_cmdline.commands[closest_i].usage, 1);
 		}
 		return NULL;
 	}
@@ -1224,6 +1295,17 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			}
 		}
 
+		/*
+		 * --type is one option that when set but not accepted by the
+		 * command, will not be ignored to make a match.  Perhaps there
+		 * are others like this, and perhaps this is a property that
+		 * should be encoded in args.h?
+		 */
+		if (!accepted && (i == type_ARG)) {
+			log_error("Failed to find a matching command definition with --type.");
+			return NULL;
+		}
+
 		if (!accepted) {
 			log_warn("Ignoring option which is not used by the specified command: %s.",
 				 arg_long_option_name(i));
@@ -1276,6 +1358,7 @@ static void _short_usage(const char *name)
 static int _usage(const char *name, int help_count)
 {
 	struct command_name *cname = _find_command_name(name);
+	const char *usage_common = NULL;
 	int i;
 
 	if (!cname) {
@@ -1292,7 +1375,17 @@ static int _usage(const char *name, int help_count)
 		if (strlen(_cmdline.commands[i].desc))
 			_print_description(i);
 
-		_print_usage(i, help_count > 1);
+		usage_common = _cmdline.commands[i].usage_common;
+
+		_print_usage(_cmdline.commands[i].usage, 0);
+		log_print(" "); /* for built-in \n */
+	}
+
+	/* Common options are printed once for all variants of a command name. */
+	if (usage_common) {
+		log_print("Common options:");
+		_print_usage(usage_common, 0);
+		log_print(" "); /* for built-in \n */
 	}
 
 	if (help_count > 1) {
@@ -1303,9 +1396,9 @@ static int _usage(const char *name, int help_count)
 		 */
 		log_print("Usage notes:");
 		log_print(". Variable parameters are: Number, String, PV, VG, LV, Tag.");
-		log_print(". The --size option can be replaced with --extents.");
-		log_print(". Select indicates that the positional arg can be omitted");
-		log_print("  if the --select option is used.");
+		log_print(". Select indicates that a required positional parameter can");
+		log_print("  be omitted if the --select option is used.");
+		log_print(". --size Number can be replaced with --extents NumberExtents.");
 		log_print(". For required options listed in parentheses, e.g. (--A, --B),");
 		log_print("  any one is required, after which the others are optional.");
 		log_print(". The _new suffix indicates the VG or LV must not yet exist.");
@@ -1757,7 +1850,7 @@ static int _process_common_commands(struct cmd_context *cmd)
 		_usage(cmd->command->name, arg_count(cmd, help_ARG));
 
 		if (arg_count(cmd, help_ARG) < 2)
-			log_print("(Use --help --help to include optional parameters.)");
+			log_print("(Use --help --help for usage notes.)");
 		return ECMD_PROCESSED;
 	}
 
@@ -2137,7 +2230,7 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 	log_debug("Parsing: %s", cmd->cmd_line);
 
 	if (!(cmd->command = _find_command(cmd, cmd_name, &argc, argv)))
-		return ENO_SUCH_CMD;
+		return_ECMD_FAILED;
 
 	set_cmd_name(cmd_name);
 

tools/toollib.c

@@ -2346,8 +2346,12 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 	struct dm_str_list *sl;
 	struct dm_list final_lvs;
 	struct lv_list *final_lvl;
+	struct dm_list found_arg_lvnames;
 	struct glv_list *glvl, *tglvl;
 	int do_report_ret_code = 1;
+	uint32_t lv_types;
+	struct logical_volume *lv;
+	struct lv_segment *seg;
 
 	log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_LV);
 
@@ -2356,6 +2360,7 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 		stack;
 
 	dm_list_init(&final_lvs);
+	dm_list_init(&found_arg_lvnames);
 
 	if (!vg_check_status(vg, EXPORTED_VG)) {
 		ret_max = ECMD_FAILED;
@@ -2449,6 +2454,7 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 		if (lvargs_supplied && str_list_match_item(arg_lvnames, lvl->lv->name)) {
 			/* Remove LV from list of unprocessed LV names */
 			str_list_del(arg_lvnames, lvl->lv->name);
+			str_list_add(cmd->mem, &found_arg_lvnames, lvl->lv->name);
 			process_lv = 1;
 		}
 
@@ -2496,6 +2502,64 @@ int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 		if (lv_is_removed(lvl->lv))
 			continue;
 
+		/*
+		 * If the command definition specifies one required positional
+		 * LV (possibly repeatable), and specifies accepted LV types,
+		 * then verify that the LV being processed matches one of those
+		 * types.
+		 *
+		 * process_each_lv() can only be used for commands that have
+		 * one positional LV arg (optionally repeating, where each is
+		 * processed independently.)  It cannot work for commands that
+		 * have different required LVs in designated positions, like
+		 * 'lvrename LV1 LV2', where each LV is not processed
+		 * independently.  That means that this LV type check only
+		 * needs to check the lv_type of the first positional arg.
+		 *
+		 * There is one command that violates this rule by stealing
+		 * the first positional LV arg before calling process_each_lv:
+		 * lvconvert --type snapshot LV_linear_striped_raid LV_snapshot
+		 * This code cannot validate that case.  process_each_lv() sees
+		 * a single LV name arg, but it's in pos 2.  Could we work around
+		 * this by looking at the final positional arg rather than always
+		 * looking at pos 1?
+		 *
+		 * This only validates types for required LV positional args
+		 * (currently there are no command specifications that include
+		 * specific LV types in optional positional args.)
+		 */
+
+		if ((cmd->command->rp_count == 1) &&
+		     val_bit_is_set(cmd->command->required_pos_args[0].def.val_bits, lv_VAL) &&
+		     cmd->command->required_pos_args[0].def.lv_types) {
+
+			lv_types = cmd->command->required_pos_args[0].def.lv_types;
+			lv = lvl->lv;
+			seg = first_seg(lv);
+
+			if ((lv_is_cow(lv) && !(lv_types & ARG_DEF_LV_SNAPSHOT)) ||
+			    (lv_is_thin_volume(lv) && !(lv_types & ARG_DEF_LV_THIN)) ||
+			    (lv_is_thin_pool(lv) && !(lv_types & ARG_DEF_LV_THINPOOL)) ||
+			    (lv_is_cache(lv) && !(lv_types & ARG_DEF_LV_CACHE)) ||
+			    (lv_is_cache_pool(lv) && !(lv_types & ARG_DEF_LV_CACHEPOOL)) ||
+			    (lv_is_mirror(lv) && !(lv_types & ARG_DEF_LV_MIRROR)) ||
+			    (lv_is_raid(lv) && !(lv_types & (ARG_DEF_LV_RAID | ARG_DEF_LV_RAID0 | ARG_DEF_LV_RAID1 | ARG_DEF_LV_RAID4 | ARG_DEF_LV_RAID5 | ARG_DEF_LV_RAID6 | ARG_DEF_LV_RAID10))) ||
+			    (segtype_is_striped(seg->segtype) && !(lv_types & ARG_DEF_LV_STRIPED)) ||
+			    (segtype_is_linear(seg->segtype) && !(lv_types & ARG_DEF_LV_LINEAR))) {
+				/*
+				 * If a named LV arg cannot be processed it's an error, otherwise
+				 * the LV is skipped and doesn't cause the command to fail.
+				 */
+				if (str_list_match_item(&found_arg_lvnames, lv->name)) {
+					log_error("Operation not permitted on LV %s with type %s.", display_lvname(lv), seg->segtype->name);
+					ret_max = ECMD_FAILED;
+				} else {
+					log_warn("Operation not permitted on LV %s with type %s.", display_lvname(lv), seg->segtype->name);
+				}
+				continue;
+			}
+		}
+
 		log_very_verbose("Processing LV %s in VG %s.", lvl->lv->name, vg->name);
 
 		ret = process_single_lv(cmd, lvl->lv, handle);

tools/vals.h

@@ -1,22 +1,20 @@
 
 /*
  * Define value types which describe values accepted
- * by the --option's in arg.h, and can also describe
+ * by the --option's in args.h, and can also describe
  * the values accepted as positional args.
  *
  * Previously, accepted values were only "described"
- * by identifying the parsing function to use.  The
- * value types expand this with enum values and names
- * to identify the accepted values.
+ * by identifying the parsing function to use.
  *
  * Some standard val types are used by many options,
  * e.g. many options (aa_ARG, bb_ARG, cc_ARG) all
- * accept a number_VAL, e.g. for various sizes or counts.
+ * accept a number_VAL.
  *
  * Other special val types are used by only one option,
  * e.g. only mirrorlog_ARG accepts a mirrorlog_VAL.
- * This typically means there are a few specific words
- * that are recognized after the option.
+ * This typically means that there are some specific
+ * words that are recognized after the option.
  *
  * Some options currently take a standard val type,
  * (esp string_VAL), but they could be given their
@@ -34,14 +32,11 @@
  * more descriptive usage string can be specified
  * as opposed to just "String".
  *
- * An option that takes a user-given name would be a
- * good example of when an option may take a string_VAL
- * because the string is arbitrary.
- *
- * The val types defined here are mostly used following
- * options, i.e. referenced by foo_ARG.  But some are
- * not, and are only used to represent positional values
- * in command definitions.
+ * Most of the val types defined here are used after
+ * --option's, and are referenced in foo_ARG entries
+ * in args.h.  But, some val types are only used to
+ * represent positional values in command definitions,
+ * e.g. vg_VAL.
  *
  * val(a, b, c, d)
  *
@@ -51,43 +46,50 @@
  * d:  what to display in usage output for this value
  *
  * command defintions will use --option NAME, where NAME
- * is the string in field c.  NAME will be translated to
+ * is shown in val() field c.  NAME will be translated to
  * foo_VAL enum in field a, which is used in commands[]
  * structs.
  *
  * option definitions (arg.h) will reference foo_VAL enum
  * in field a.
  *
- * FIXME: for specialized val types, the set of accepted
- * values is not defined or stored in a consistent way,
+ * FIXME: for specialized val types, the set of recognized
+ * 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.  (The set of accepted values for some
- * options is not fixed, but can be dynamic, which would
- * be an exception to the rule.)
+ * consistent way, perhaps in struct val_props.
  *
  * The usage text for an option is not always the full
- * set of strings accepted for an option, but may be a
- * subset.  i.e. an outdated option that no longer does
- * anything may not be listed, but may be recognized
+ * set of words accepted for an option, but may be a
+ * subset.  i.e. an outdated word that no longer does
+ * anything may not be shown, but may still be recognized
  * and ignored, or an option that shouldn't be used in
- * general.  These options are not recommended, so they
- * are not shown, even though they may still be recognized
- * by the parsing function. e.g. for --activate we show
- * the most common "y|n|ay" without showing the lockd
- * variations "ey|sy" which are not application to most.
+ * general isn't shown to avoid suggesting it.
+ * e.g. for --activate we show the most common "y|n|ay"
+ * without showing the lvmlockd variations "ey|sy" which
+ * are not applicable in general.
  *
  * FIXME: are there some specialized or irrelevant
  * options included in the usage text below that should
  * be removed?  Should "lvm1" be removed?
  *
- * "Number[units]" is a shorter form of "Number[bBsSkKmMgGtTpPeE]",
- * but perhaps the full string should be shown instead?  Or maybe
- * a subset of the more common letters, e.g. [kKmMgG] ?
+ * For Number args that take optional units, a full usage
+ * could be "Number[bBsSkKmMgGtTpPeE]" (with implied |),
+ * but repeating this full specification produces cluttered
+ * output, and doesn't indicate which unit is the default.
+ * "Number[units]" would be cleaner, as would a subset of
+ * common units, e.g. "Number[kmg...]", but neither helps
+ * with default.  "Number[k|unit]" and "Number[m|unit]" show
+ * the default, and "unit" indicates that other units
+ * are possible without listing them all.  This also
+ * suggests using the preferred lower case letters, because
+ * --size and other option args treat upper/lower letters
+ * the same, all as 1024 SI base.  For this reason, we
+ * should avoid suggesting the upper case letters.
  */
 
 val(none_VAL, NULL, "None", "")             /* unused, for enum value 0 */
@@ -106,10 +108,10 @@ val(activation_VAL, activation_arg, "Active", "y|n|ay")
 val(cachemode_VAL, cachemode_arg, "CacheMode", "writethrough|writeback")
 val(discards_VAL, discards_arg, "Discards", "passdown|nopassdown|ignore")
 val(mirrorlog_VAL, mirrorlog_arg, "MirrorLog", "core|disk")
-val(sizekb_VAL, size_kb_arg, "SizeKB", "Number[units]")
-val(sizemb_VAL, size_mb_arg, "SizeMB", "Number[units]")
+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, "SNumber%", "[+|-]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")