From 9c6954bc61b22ca03df8897d88eb9618e65fc3c6 Mon Sep 17 00:00:00 2001

From: Zdenek Kabelac <zkabelac@redhat.com>

Date: Wed, 13 Apr 2022 15:09:08 +0200

Subject: [PATCH 47/54] vdo: support --vdosettings

Allow to use --vdosettings with lvcreate,lvconvert,lvchange.

Support settings currenly only configurable via lvm.conf.

With lvchange we require inactivate LV for changes to be applied.

Settings block_map_era_length has supported alias block_map_period.

---

 device_mapper/vdo/target.h  |   6 +-

 man/lvmvdo.7_main           |  39 ++++++--

 test/shell/lvchange-vdo.sh  |   8 ++

 test/shell/lvconvert-vdo.sh |   8 +-

 test/shell/lvcreate-vdo.sh  |   6 +-

 tools/args.h                |   9 ++

 tools/command-lines.in      |   6 +-

 tools/lvchange.c            |  43 ++++++++-

 tools/lvconvert.c           |   9 +-

 tools/lvcreate.c            |  34 ++++---

 tools/toollib.c             | 177 ++++++++++++++++++++++++++++++++++++

 tools/toollib.h             |   6 ++

 12 files changed, 312 insertions(+), 39 deletions(-)

diff --git a/device_mapper/vdo/target.h b/device_mapper/vdo/target.h

index 51dde3f4d..60c5bff56 100644

--- a/device_mapper/vdo/target.h

+++ b/device_mapper/vdo/target.h

@@ -77,8 +77,10 @@ enum dm_vdo_write_policy {

 struct dm_vdo_target_params {

 	uint32_t minimum_io_size;       // in sectors

 	uint32_t block_map_cache_size_mb;

-	uint32_t block_map_era_length;	// format period

-

+	union {

+		uint32_t block_map_era_length;	// format period

+		uint32_t block_map_period;      // supported alias

+	};

 	uint32_t check_point_frequency;

 	uint32_t index_memory_size_mb;  // format

diff --git a/man/lvmvdo.7_main b/man/lvmvdo.7_main

index 3b77173c4..14bd640b5 100644

--- a/man/lvmvdo.7_main

+++ b/man/lvmvdo.7_main

@@ -132,6 +132,19 @@ that can keep 100% incompressible data there.

 # lvconvert --type vdo-pool -n vdo0 -V10G vg/ExistingLV

 .fi

 .

+.SS \n+[step]. Change the compression and deduplication of a VDOPoolLV

+.

+Disable or enable the compression and deduplication for VDOPoolLV

+(the volume that maintains all VDO LV(s) associated with it).

+.P

+.B lvchange --compression y|n --deduplication y|n VG/VDOPoolLV

+.P

+.I Example

+.nf

+# lvchange --compression n  vg/vdopool0

+# lvchange --deduplication y vg/vdopool1

+.fi

+.

 .SS \n+[step]. Change the default settings used for creating a VDOPoolLV

 .

 VDO allows to set a large variety of options. Lots of these settings

@@ -173,17 +186,27 @@ EOF

 # lvcreate --vdo -L10G --config 'allocation/vdo_cpu_threads=4' vg/vdopool1

 .fi

 .

-.SS \n+[step]. Change the compression and deduplication of a VDOPoolLV

-.

-Disable or enable the compression and deduplication for VDOPoolLV

-(the volume that maintains all VDO LV(s) associated with it).

-.P

-.B lvchange --compression y|n --deduplication y|n VG/VDOPoolLV

+.SS \n+[step]. Set or change VDO settings with option --vdosettings

+.

+Use the form 'option=value' or 'option1=value option2=value',

+or repeat --vdosettings for each option being set.

+Options are listed in the Example section above, for the full description see

+.BR lvm.conf (5).

+Options can omit 'vdo_' and 'vdo_use_' prefixes and all its underscores.

+So i.e.  vdo_use_metadata_hints=1  and  metadatahints=1 are equivalent.

+To change the option for an already existing VDOPoolLV use

+.BR lvchange (8)

+command. However not all option can be changed.

+Only compression and deduplication options can be also changed for an active VDO LV.

+Lowest priority options are specified with configuration file,

+then with --vdosettings and highest are expliction option --compression

+and --deduplication.

 .P

 .I Example

+.P

 .nf

-# lvchange --compression n  vg/vdopool0

-# lvchange --deduplication y vg/vdopool1

+# lvcreate --vdo -L10G --vdosettings 'ack_threads=1 hash_zone_threads=2' vg/vdopool0

+# lvchange --vdosettings 'bio_threads=2 deduplication=1' vg/vdopool0

 .fi

 .

 .SS \n+[step]. Checking the usage of VDOPoolLV

diff --git a/test/shell/lvchange-vdo.sh b/test/shell/lvchange-vdo.sh

index 461b7821f..7cc44d6bc 100644

--- a/test/shell/lvchange-vdo.sh

+++ b/test/shell/lvchange-vdo.sh

@@ -48,9 +48,17 @@ check grep_dmsetup status $vg-vdopool-vpool " online online "

 lvchange --compression n --deduplication n $vg/vdopool

 check grep_dmsetup status $vg-vdopool-vpool " offline offline "

+# --vdosettings needs inactive LV

+not lvchange --vdosettings 'ack_threads=8' $vg/vdopool

 lvchange -an $vg/$lv1

+# With inactive vdo-pool changes are applied

+# explicit option --compression has highest priority

+lvchange --vdosettings 'ack_threads=5 compression=0' --compression y $vg/vdopool

+check lv_field $vg/$lv1 vdo_ack_threads "5"

+check lv_field $vg/$lv1 vdo_compression "enabled"

+

 # Test activation

 lvchange -aly $vg/$lv1

 check active $vg $lv1

diff --git a/test/shell/lvconvert-vdo.sh b/test/shell/lvconvert-vdo.sh

index 529f325bd..c42d8f25a 100644

--- a/test/shell/lvconvert-vdo.sh

+++ b/test/shell/lvconvert-vdo.sh

@@ -28,12 +28,12 @@ lvcreate -L5G -n $lv1 $vg

 not lvconvert --type vdo-pool $vg/$lv1 |& tee out

 grep "WARNING" out

-

-lvconvert -y --type vdo-pool $vg/$lv1

+# Check --vdosettings is also applied to converted vdo-pool

+lvconvert -y --type vdo-pool --vdosettings 'ack_threads=5' $vg/$lv1

+check lv_field $vg/$lv1 vdo_ack_threads "5"

 lvremove -f $vg

-

-# 

+#

 lvcreate -L5G -n $lv1 $vg

 lvconvert -y --vdopool $vg/$lv1

 lvremove -f $vg

diff --git a/test/shell/lvcreate-vdo.sh b/test/shell/lvcreate-vdo.sh

index 44f8bf094..3e807ac94 100644

--- a/test/shell/lvcreate-vdo.sh

+++ b/test/shell/lvcreate-vdo.sh

@@ -79,8 +79,12 @@ not fsck -n "$DM_DEV_DIR/mapper/$vg-${lv2}"

 lvremove -ff $vg

+# Unknown settings does not pass

+# TODO: try to catch this in parser and 'fail'

+not lvcreate --type vdo --vdosettings 'ack_Xthreads=4' -L10G -V1T -ky -n $lv1 $vg

-lvcreate --type vdo -L10G -V1T -ky -n $lv1 $vg

+lvcreate --type vdo --vdosettings 'ack_threads=4' -L10G -V1T -ky -n $lv1 $vg

+check lv_field $vg/$lv1 vdo_ack_threads "4"

 lvs -a $vg

 lvremove -ff $vg

diff --git a/tools/args.h b/tools/args.h

index 00a2ec817..bfd848ce9 100644

--- a/tools/args.h

+++ b/tools/args.h

@@ -900,6 +900,15 @@ arg(vdopool_ARG, '\0', "vdopool", lv_VAL, 0, 0,

     "The name of a VDO pool LV.\n"

     "See \\fBlvmvdo\\fP(7) for more information about VDO usage.\n")

+arg(vdosettings_ARG, '\0', "vdosettings", string_VAL, ARG_GROUPABLE, 0,

+    "Specifies tunable VDO options for VDO LVs.\n"

+    "Use the form 'option=value' or 'option1=value option2=value', or\n"

+    "repeat --vdosettings for each option being set.\n"

+    "These settings override the default VDO behaviors.\n"

+    "To remove vdosettings and revert to the default\n"

+    "VDO behaviors, use --vdosettings 'default'.\n"

+    "See \\fBlvmvdo\\fP(7) for more information.\n")

+

 arg(version_ARG, '\0', "version", 0, 0, 0,

     "Display version information.\n")

diff --git a/tools/command-lines.in b/tools/command-lines.in

index 00ac08934..08302b34f 100644

--- a/tools/command-lines.in

+++ b/tools/command-lines.in

@@ -243,6 +243,7 @@ OO_LVCHANGE_META: --addtag Tag, --deltag Tag,

 --setautoactivation Bool, --errorwhenfull Bool, --discards Discards, --zero Bool,

 --cachemode CacheMode, --cachepolicy String, --cachesettings String,

 --minrecoveryrate SizeKB, --maxrecoveryrate SizeKB,

+--vdosettings String,

 --writebehind Number, --writemostly WriteMostlyPV, --persistent n

 # It's unfortunate that activate needs to be optionally allowed here;

@@ -341,7 +342,8 @@ OO_LVCONVERT_CACHE: --cachemetadataformat CacheMetadataFormat,

 --cachesettings String, --zero Bool

 OO_LVCONVERT_VDO: --metadataprofile String, --readahead Readahead,

---compression Bool, --deduplication Bool, --zero Bool

+--compression Bool, --deduplication Bool, --vdosettings String,

+--zero Bool

 OO_LVCONVERT: --alloc Alloc, --background, --force, --noudevsync

@@ -839,7 +841,7 @@ OO_LVCREATE_POOL: --poolmetadatasize SizeMB, --poolmetadataspare Bool, --chunksi

 OO_LVCREATE_THINPOOL: --discards Discards, --errorwhenfull Bool

-OO_LVCREATE_VDO: --compression Bool, --deduplication Bool

+OO_LVCREATE_VDO: --compression Bool, --deduplication Bool, --vdosettings String

 ---

 lvcreate --type error --size SizeMB VG

diff --git a/tools/lvchange.c b/tools/lvchange.c

index 0525bc53c..dc51786d7 100644

--- a/tools/lvchange.c

+++ b/tools/lvchange.c

@@ -755,6 +755,43 @@ out:

 	return r;

 }

+static int _lvchange_vdo(struct cmd_context *cmd,

+			 struct logical_volume *lv,

+			 uint32_t *mr)

+{

+	struct lv_segment *seg;

+	int updated = 0;

+

+	seg = first_seg(lv);

+

+	// With VDO LV given flip to VDO pool

+	if (seg_is_vdo(seg))

+		seg = first_seg(seg_lv(seg, 0));

+

+	if (!get_vdo_settings(cmd, &seg->vdo_params, &updated))

+		return_0;

+

+	if ((updated & VDO_CHANGE_OFFLINE) &&

+	    lv_info(cmd, seg->lv, 1, NULL, 0, 0)) {

+		log_error("Cannot change VDO settings for active VDO pool %s.",

+			  display_lvname(seg->lv));

+		// TODO maybe add --force support with prompt here

+		log_print_unless_silent("VDO pool %s with all its LVs needs to be deactivated.",

+					display_lvname(seg->lv));

+		return 0;

+	}

+

+	if (updated) {

+		if (!dm_vdo_validate_target_params(&seg->vdo_params, 0 /* vdo_size */))

+			return_0;

+

+		/* Request caller to commit and reload metadata */

+		*mr |= MR_RELOAD;

+	}

+

+	return 1;

+}

+

 static int _lvchange_tag(struct cmd_context *cmd, struct logical_volume *lv,

 			 int arg, uint32_t *mr)

 {

@@ -1154,6 +1191,7 @@ static int _option_requires_direct_commit(int opt_enum)

 		cachemode_ARG,

 		cachepolicy_ARG,

 		cachesettings_ARG,

+		vdosettings_ARG,

 		-1

 	};

@@ -1354,7 +1392,10 @@ static int _lvchange_properties_single(struct cmd_context *cmd,

 			docmds++;

 			doit += _lvchange_cache(cmd, lv, &mr;;

 			break;

-

+		case vdosettings_ARG:

+			docmds++;

+			doit += _lvchange_vdo(cmd, lv, &mr;;

+			break;

 		default:

 			log_error(INTERNAL_ERROR "Failed to check for option %s",

 				  arg_long_option_name(i));

diff --git a/tools/lvconvert.c b/tools/lvconvert.c

index a90946173..3d4b24fe3 100644

--- a/tools/lvconvert.c

+++ b/tools/lvconvert.c

@@ -5456,13 +5456,8 @@ static int _lvconvert_to_vdopool_single(struct cmd_context *cmd,

 	if (!fill_vdo_target_params(cmd, &vdo_params, &vdo_pool_header_size, vg->profile))

 		goto_out;

-	if (arg_is_set(cmd, compression_ARG))

-		vdo_params.use_compression =

-			arg_int_value(cmd, compression_ARG, 0);

-

-	if (arg_is_set(cmd, deduplication_ARG))

-		vdo_params.use_deduplication =

-			arg_int_value(cmd, deduplication_ARG, 0);

+	if (!get_vdo_settings(cmd, &vdo_params, NULL))

+		return_0;

 	if (!activate_lv(cmd, lv)) {

 		log_error("Cannot activate %s.", display_lvname(lv));

diff --git a/tools/lvcreate.c b/tools/lvcreate.c

index 79af42685..8de6f3408 100644

--- a/tools/lvcreate.c

+++ b/tools/lvcreate.c

@@ -698,6 +698,23 @@ static int _read_cache_params(struct cmd_context *cmd,

 	return 1;

 }

+static int _read_vdo_params(struct cmd_context *cmd,

+			    struct lvcreate_params *lp)

+{

+	if (!seg_is_vdo(lp))

+		return 1;

+

+	// prefiling settings here

+	if (!fill_vdo_target_params(cmd, &lp->vdo_params,  &lp->vdo_pool_header_size, NULL))

+		return_0;

+

+	// override with optional vdo settings

+	if (!get_vdo_settings(cmd, &lp->vdo_params, NULL))

+		return_0;

+

+	return 1;

+}

+

 static int _read_activation_params(struct cmd_context *cmd,

 				   struct volume_group *vg,

 				   struct lvcreate_params *lp)

@@ -888,7 +905,8 @@ static int _lvcreate_params(struct cmd_context *cmd,

 #define VDO_POOL_ARGS \

 	vdopool_ARG,\

 	compression_ARG,\

-	deduplication_ARG

+	deduplication_ARG,\

+	vdosettings_ARG

 	/* Cache and cache-pool segment type */

 	if (seg_is_cache(lp)) {

@@ -1098,19 +1116,6 @@ static int _lvcreate_params(struct cmd_context *cmd,

 						zero_ARG,

 						-1))

 			return_0;

-

-		// FIXME: prefiling here - this is wrong place

-		// but will work for this moment

-		if (!fill_vdo_target_params(cmd, &lp->vdo_params, &lp->vdo_pool_header_size, NULL))

-			return_0;

-

-		if (arg_is_set(cmd, compression_ARG))

-			lp->vdo_params.use_compression =

-				arg_int_value(cmd, compression_ARG, 0);

-

-		if (arg_is_set(cmd, deduplication_ARG))

-			lp->vdo_params.use_deduplication =

-				arg_int_value(cmd, deduplication_ARG, 0);

 	}

 	/* Check options shared between more segment types */

@@ -1198,6 +1203,7 @@ static int _lvcreate_params(struct cmd_context *cmd,

 			      &lp->pool_metadata_size, &lp->pool_metadata_spare,

 			      &lp->chunk_size, &lp->discards, &lp->zero_new_blocks)) ||

 	    !_read_cache_params(cmd, lp) ||

+	    !_read_vdo_params(cmd, lp) ||

 	    !_read_mirror_and_raid_params(cmd, lp))

 		return_0;

diff --git a/tools/toollib.c b/tools/toollib.c

index 16be336d4..697baee82 100644

--- a/tools/toollib.c

+++ b/tools/toollib.c

@@ -1192,6 +1192,183 @@ out:

 	return ok;

 }

+/*

+ * Compare VDO option name, skip any '_' in name

+ * and also allow to use it without  vdo_[use_] prefix

+ */

+static int _compare_vdo_option(const char *b1, const char *b2)

+{

+	if (strncasecmp(b1, "vdo", 3) == 0) // skip vdo prefix

+		b1 += 3;

+

+	if ((tolower(*b1) != tolower(*b2)) &&

+	    (strncmp(b2, "use_", 4) == 0))

+		b2 += 4;  // try again with skipped prefix 'use_'

+

+	while (*b1 && *b2) {

+		if (tolower(*b1) == tolower(*b2)) {

+			++b1;

+			++b2;

+			continue;	// matching char

+		}

+

+		if (*b1 == '_')

+			++b1;           // skip to next char

+		else if (*b2 == '_')

+			++b2;           // skip to next char

+		else

+			break;          // mismatch

+	}

+

+	return (*b1 || *b2) ? 0 : 1;

+}

+

+#define CHECK_AND_SET(var, onoff) \

+	option = #var;\

+	if (_compare_vdo_option(cn->key, option)) {\

+		if (is_lvchange || !cn->v || (cn->v->type != DM_CFG_INT))\

+			goto err;\

+		if (vtp->var != cn->v->v.i) {\

+			vtp->var = cn->v->v.i;\

+			u |= onoff;\

+		}\

+		continue;\

+	}

+

+#define DO_OFFLINE(var) \

+	CHECK_AND_SET(var, VDO_CHANGE_OFFLINE)

+

+#define DO_ONLINE(var) \

+	CHECK_AND_SET(var, VDO_CHANGE_ONLINE)

+

+int get_vdo_settings(struct cmd_context *cmd,

+		     struct dm_vdo_target_params *vtp,

+		     int *updated)

+{

+	const char *str, *option = NULL;

+	struct arg_value_group_list *group;

+	struct dm_config_tree *result = NULL, *prev = NULL, *current = NULL;

+	struct dm_config_node *cn;

+	int r = 0, u = 0, is_lvchange;

+	int use_compression = vtp->use_compression;

+	int use_deduplication = vtp->use_deduplication;

+	int checked_lvchange;

+

+	if (updated)

+		*updated = 0;

+

+	// Group all --vdosettings

+	dm_list_iterate_items(group, &cmd->arg_value_groups) {

+		if (!grouped_arg_is_set(group->arg_values, vdosettings_ARG))

+			continue;

+

+		if (!(current = dm_config_create()))

+			goto_out;

+		if (prev)

+			current->cascade = prev;

+		prev = current;

+

+		if (!(str = grouped_arg_str_value(group->arg_values,

+						  vdosettings_ARG,

+						  NULL)))

+			goto_out;

+

+		if (!dm_config_parse_without_dup_node_check(current, str, str + strlen(str)))

+			goto_out;

+	}

+

+	if (current) {

+		if (!(result = dm_config_flatten(current)))

+			goto_out;

+

+		checked_lvchange = !strcmp(cmd->name, "lvchange");

+

+		/* Use all acceptable VDO options */

+		for (cn = result->root; cn; cn = cn->sib) {

+			is_lvchange = 0;

+			DO_OFFLINE(ack_threads);

+			DO_OFFLINE(bio_rotation);

+			DO_OFFLINE(bio_threads);

+			DO_OFFLINE(block_map_cache_size_mb);

+			DO_OFFLINE(block_map_era_length);

+			DO_OFFLINE(block_map_period); // alias for block_map_era_length

+			DO_OFFLINE(cpu_threads);

+			DO_OFFLINE(hash_zone_threads);

+			DO_OFFLINE(logical_threads);

+			DO_OFFLINE(max_discard);

+			DO_OFFLINE(physical_threads);

+

+			// Support also these - even when we have regular opts for them

+			DO_ONLINE(use_compression);

+			DO_ONLINE(use_deduplication);

+

+			// Settings bellow cannot be changed with lvchange command

+			is_lvchange = checked_lvchange;

+

+			DO_OFFLINE(check_point_frequency);

+			DO_OFFLINE(index_memory_size_mb);

+			DO_OFFLINE(minimum_io_size);

+			DO_OFFLINE(slab_size_mb);

+			DO_OFFLINE(use_metadata_hints);

+			DO_OFFLINE(use_sparse_index);

+

+			option = "write_policy";

+			if (_compare_vdo_option(cn->key, option)) {

+				if (is_lvchange || !cn->v || (cn->v->type != DM_CFG_STRING))

+					goto err;

+				if (!set_vdo_write_policy(&vtp->write_policy, cn->v->v.str))

+					goto_out;

+				u |= VDO_CHANGE_OFFLINE;

+				continue;

+			}

+

+			log_error("Unknown VDO setting \"%s\".", cn->key);

+			goto out;

+		}

+	}

+

+	if (arg_is_set(cmd, compression_ARG)) {

+		vtp->use_compression = arg_int_value(cmd, compression_ARG, 0);

+		if (vtp->use_compression != use_compression)

+			u |= VDO_CHANGE_ONLINE;

+	}

+

+	if (arg_is_set(cmd, deduplication_ARG)) {

+		vtp->use_deduplication = arg_int_value(cmd, deduplication_ARG, 0);

+		if (vtp->use_deduplication != use_deduplication)

+			u |= VDO_CHANGE_ONLINE;

+	}

+

+	if (updated) {

+		// validation of updated VDO option

+		if (!dm_vdo_validate_target_params(vtp, 0 /* vdo_size */)) {

+err:

+			if (is_lvchange)

+				log_error("Cannot change VDO setting \"vdo_%s\" in existing VDO pool.",

+					  option);

+			else

+				log_error("Invalid argument for VDO setting \"vdo_%s\".",

+					  option);

+			goto out;

+		}

+

+		*updated = u;

+	}

+

+	r = 1;

+out:

+	if (result)

+		dm_config_destroy(result);

+

+	while (prev) {

+		current = prev->cascade;

+		dm_config_destroy(prev);

+		prev = current;

+	}

+

+	return r;

+}

+

 static int _get_one_writecache_setting(struct cmd_context *cmd, struct writecache_settings *settings,

 				       char *key, char *val, uint32_t *block_size_sectors)

 {

diff --git a/tools/toollib.h b/tools/toollib.h

index f3a60fbc4..2b38e4e4f 100644

--- a/tools/toollib.h

+++ b/tools/toollib.h

@@ -217,6 +217,12 @@ int get_cache_params(struct cmd_context *cmd,

 		     const char **name,

 		     struct dm_config_tree **settings);

+#define VDO_CHANGE_ONLINE  1

+#define VDO_CHANGE_OFFLINE 2

+int get_vdo_settings(struct cmd_context *cmd,

+		     struct dm_vdo_target_params *vtp,

+		     int *updated);

+

 int get_writecache_settings(struct cmd_context *cmd, struct writecache_settings *settings,

                             uint32_t *block_size_sectors);

-- 

2.34.3