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