From a46f1a28adea64c0651969324e0ebc197210facc Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac Date: Tue, 29 Oct 2024 15:47:32 +0100 Subject: [PATCH] text_import: use text_import_values on striped and thin Convert segment reader for striped and thin segtype to use new text_import_value(). --- lib/format_text/import_vsn1.c | 112 ++++++++++++++++++---------------- lib/striped/striped.c | 26 +++++--- lib/thin/thin.c | 99 +++++++++++++----------------- 3 files changed, 117 insertions(+), 120 deletions(-) diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c index df393caa2..652e04daf 100644 --- a/lib/format_text/import_vsn1.c +++ b/lib/format_text/import_vsn1.c @@ -395,8 +395,9 @@ static int _read_segment(struct cmd_context *cmd, uint32_t area_count = 0u; struct lv_segment *seg; const struct dm_config_node *sn_child = sn->child; - const struct dm_config_value *cv; - uint32_t area_extents, start_extent, extent_count, reshape_count, data_copies; + const struct dm_config_value *cv = NULL; + uint32_t area_extents, start_extent, extent_count; + uint32_t reshape_count = 0, data_copies = 1; struct segment_type *segtype; const char *segtype_str; @@ -405,28 +406,20 @@ static int _read_segment(struct cmd_context *cmd, return 0; } - if (!_read_int32(sn_child, "start_extent", &start_extent)) { - log_error("Couldn't read 'start_extent' for segment '%s' " - "of logical volume %s.", sn->key, lv->name); - return 0; - } - - if (!_read_int32(sn_child, "extent_count", &extent_count)) { - log_error("Couldn't read 'extent_count' for segment '%s' " - "of logical volume %s.", sn->key, lv->name); - return 0; - } - - if (!_read_int32(sn_child, "reshape_count", &reshape_count)) - reshape_count = 0; - - if (!_read_int32(sn_child, "data_copies", &data_copies)) - data_copies = 1; - segtype_str = SEG_TYPE_NAME_STRIPED; - if (!dm_config_get_str(sn_child, "type", &segtype_str)) { - log_error("Segment type must be a string."); + struct config_value values[] = { /* sorted! */ + { "data_copies", &data_copies, CONFIG_VALUE_UINT32 }, + { "extent_count", &extent_count, CONFIG_VALUE_UINT32, 1 }, + { "reshape_count", &reshape_count, CONFIG_VALUE_UINT32, }, + { "start_extent", &start_extent, CONFIG_VALUE_UINT32, 1 }, + { "tags", &cv, CONFIG_VALUE_LIST, }, + { "type", &segtype_str, CONFIG_VALUE_STRING, 1 }, + }; + + if (!text_import_values(sn_child, values, DM_ARRAY_SIZE(values))) { + log_error("Could not import values for logical volume %s.", + display_lvname(lv)); return 0; } @@ -454,8 +447,7 @@ static int _read_segment(struct cmd_context *cmd, return_0; /* Optional tags */ - if (dm_config_get_list(sn_child, "tags", &cv) && - !(_read_str_list(mem, &seg->tags, cv))) { + if (cv && !(_read_str_list(mem, &seg->tags, cv))) { log_error("Couldn't read tags for a segment of %s/%s.", lv->vg->name, lv->name); return 0; @@ -703,6 +695,25 @@ static int _read_lvnames(struct cmd_context *cmd, const char *alloc = NULL, *profile = NULL, *lock_args = NULL; unsigned seg_count = 0; uint32_t read_ahead = UINT32_C(-2); + const struct dm_config_value *status_cv = NULL, *flags_cv = NULL; + const char *uuid = NULL; + int major = -1, minor = -1; + + struct config_value values[] = { /* sorted! */ + { "allocation_policy", &alloc, CONFIG_VALUE_STRING, }, + { "creation_host", &hostname, CONFIG_VALUE_STRING, }, + { "creation_time", ×tamp, CONFIG_VALUE_UINT64, }, + { "flags", &flags_cv, CONFIG_VALUE_LIST, }, + { "id", &uuid, CONFIG_VALUE_STRING, 1 }, + { "lock_args", &lock_args, CONFIG_VALUE_STRING, }, + { "major", &major, CONFIG_VALUE_UINT32, }, + { "minor", &minor, CONFIG_VALUE_UINT32, }, + { "profile", &profile, CONFIG_VALUE_STRING, }, + { "read_ahead", &read_ahead, CONFIG_VALUE_UINT32, }, + { "segment_count", &seg_count, CONFIG_VALUE_UINT32, 1 }, + { "status", &status_cv, CONFIG_VALUE_LIST, 1 }, + { "tags", &tags_cv, CONFIG_VALUE_LIST, }, + }; if (!(str = dm_pool_strdup(mem, lvn->key))) return_0; @@ -723,34 +734,33 @@ static int _read_lvnames(struct cmd_context *cmd, log_debug_metadata("Importing logical volume %s.", lv->name); + if (!text_import_values(lvn, values, DM_ARRAY_SIZE(values))) { + log_error("Could not import values for logical volume %s.", + display_lvname(lv)); + return 0; + } + memcpy(&lv->lvid.id[0], &lv->vg->id, sizeof(lv->lvid.id[0])); - /* FIXME: read full lvid */ - if (!_read_id(&lv->lvid.id[1], lvn, "id")) { - log_error("Couldn't read uuid for logical volume %s.", + + if (!uuid || !id_read_format(&lv->lvid.id[1], uuid)) { + log_error("Can not use invalid uuid %s for logical volume %s.", + ((uuid) ? : ""), display_lvname(lv)); + return 0; + } + + /* For backward compatible metadata accept both type of flags */ + if (!status_cv || !(read_flags(&lvstatus, LV_FLAGS, STATUS_FLAG | SEGTYPE_FLAG, status_cv))) { + log_error("Could not read status flags for logical volume %s.", display_lvname(lv)); return 0; } - if (!dm_config_get_uint32(lvn, "segment_count", &seg_count)) { - log_error("Couldn't read segment count for logical volume %s.", - lv->name); - return 0; - } - - if (!_read_flag_config(lvn, &lvstatus, LV_FLAGS)) { - log_error("Couldn't read status flags for logical volume %s.", + if (flags_cv && !(read_flags(&lvstatus, LV_FLAGS, COMPATIBLE_FLAG, flags_cv))) { + log_error("Could not read flags for logical volume %s.", display_lvname(lv)); return 0; } - dm_config_get_str(lvn, "allocation_policy", &alloc); - dm_config_get_str(lvn, "creation_host", &hostname); - dm_config_get_uint64(lvn, "creation_time", ×tamp); - dm_config_get_str(lvn, "lock_args", &lock_args); - dm_config_get_str(lvn, "profile", &profile); - dm_config_get_uint32(lvn, "read_ahead", &read_ahead); - dm_config_get_list(lvn, "tags", &tags_cv); - if (lvstatus & LVM_WRITE_LOCKED) { lvstatus |= LVM_WRITE; lvstatus &= ~LVM_WRITE_LOCKED; @@ -854,26 +864,22 @@ static int _read_lvnames(struct cmd_context *cmd, } if (lv->status & FIXED_MINOR) { - if (!_read_int32(lvn, "minor", &lv->minor)) { + if (minor == -1) { log_error("Couldn't read minor number for logical volume %s.", display_lvname(lv)); return 0; } - if (!dm_config_has_node(lvn, "major")) + if (major == -1) /* If major is missing, pick default */ lv->major = cmd->dev_types->device_mapper_major; - else if (!_read_int32(lvn, "major", &lv->major)) { - log_warn("WARNING: Couldn't read major number for logical " - "volume %s.", display_lvname(lv)); - lv->major = cmd->dev_types->device_mapper_major; - } - if (!validate_major_minor(cmd, fmt, lv->major, lv->minor)) { + if (validate_major_minor(cmd, fmt, major, minor)) { + lv->major = major; + lv->minor = minor; + } else log_warn("WARNING: Ignoring invalid major, minor number for " "logical volume %s.", display_lvname(lv)); - lv->major = lv->minor = -1; - } } lv->size = seg_count; /* Use temporarily to store seg_count for validation */ diff --git a/lib/striped/striped.c b/lib/striped/striped.c index 09c3fca8e..be29e92a5 100644 --- a/lib/striped/striped.c +++ b/lib/striped/striped.c @@ -71,19 +71,27 @@ static int _striped_text_import_area_count(const struct dm_config_node *sn, uint static int _striped_text_import(struct lv_segment *seg, const struct dm_config_node *sn) { - const struct dm_config_value *cv; + const struct dm_config_value *cv = NULL; + uint32_t stripe_size = 0; - if ((seg->area_count != 1) && - !dm_config_get_uint32(sn, "stripe_size", &seg->stripe_size)) { - log_error("Couldn't read stripe_size for segment %s " - "of logical volume %s.", dm_config_parent_name(sn), seg->lv->name); + struct config_value values[] = { /* sorted! */ + { "stripe_size", &stripe_size, CONFIG_VALUE_UINT32, }, + { "stripes", &cv, CONFIG_VALUE_LIST, 1 }, + }; + + if (!text_import_values(sn, values, DM_ARRAY_SIZE(values))) { + log_error("Could not read segment values for %s", + display_lvname(seg->lv)); return 0; } - if (!dm_config_get_list(sn, "stripes", &cv)) { - log_error("Couldn't find stripes array for segment %s " - "of logical volume %s.", dm_config_parent_name(sn), seg->lv->name); - return 0; + if (seg->area_count != 1) { + if (!stripe_size) { + log_error("Couldn't read stripe_size for segment %s " + "of logical volume %s.", dm_config_parent_name(sn), seg->lv->name); + return 0; + } + seg->stripe_size = stripe_size; } seg->area_len /= seg->area_count; diff --git a/lib/thin/thin.c b/lib/thin/thin.c index 4407294b6..9115429f8 100644 --- a/lib/thin/thin.c +++ b/lib/thin/thin.c @@ -21,6 +21,7 @@ #include "lib/config/config.h" #include "lib/activate/activate.h" #include "lib/datastruct/str_list.h" +#include "lib/format_text/text_import.h" /* Dm kernel module name for thin provisioning */ static const char _thin_pool_module[] = "thin-pool"; @@ -87,36 +88,24 @@ static int _thin_pool_text_import(struct lv_segment *seg, uint32_t zero = 0; uint32_t crop = UINT32_MAX; - if (!dm_config_get_str(sn, "metadata", &meta_name)) - return SEG_LOG_ERROR("Metadata must be a string in"); + struct config_value values[] = { /* sorted! */ + { "chunk_size", &seg->chunk_size, CONFIG_VALUE_UINT32, 1 }, + { "crop_metadata", &crop, CONFIG_VALUE_UINT32, }, + { "discards", &discards_str, CONFIG_VALUE_STRING, }, + { "metadata", &meta_name, CONFIG_VALUE_STRING, 1 }, + { "pool", &lv_name, CONFIG_VALUE_STRING, 1 }, + { "transaction_id", &seg->transaction_id, CONFIG_VALUE_UINT64, 1 }, + { "zero_new_blocks", &zero, CONFIG_VALUE_UINT32, }, + }; - if (!dm_config_get_str(sn, "pool", &lv_name)) - return SEG_LOG_ERROR("Pool must be a string in"); - - if (!dm_config_get_uint64(sn, "transaction_id", &seg->transaction_id)) - return SEG_LOG_ERROR("Could not read transaction_id for"); - - if (!dm_config_get_uint32(sn, "chunk_size", &seg->chunk_size)) - return SEG_LOG_ERROR("Could not read chunk_size"); - - if (dm_config_has_node(sn, "discards") && - !dm_config_get_str(sn, "discards", &discards_str)) - return SEG_LOG_ERROR("Could not read discards for"); - - if (dm_config_has_node(sn, "zero_new_blocks") && - !dm_config_get_uint32(sn, "zero_new_blocks", &zero)) - return SEG_LOG_ERROR("Could not read zero_new_blocks for"); - - if (dm_config_has_node(sn, "crop_metadata")) { - if (!dm_config_get_uint32(sn, "crop_metadata", &crop)) - return SEG_LOG_ERROR("Could not read crop_metadata for"); - } + if (!text_import_values(sn, values, DM_ARRAY_SIZE(values))) + return SEG_LOG_ERROR("Could not read segment values for"); if (!(pool_data_lv = find_lv(seg->lv->vg, lv_name))) return SEG_LOG_ERROR("Unknown pool %s in", lv_name); if (!(pool_metadata_lv = find_lv(seg->lv->vg, meta_name))) - return SEG_LOG_ERROR("Unknown metadata %s in", lv_name); + return SEG_LOG_ERROR("Unknown metadata %s in", meta_name); if (!discards_str) seg->discards = THIN_DISCARDS_IGNORE; @@ -471,50 +460,44 @@ static void _thin_display(const struct lv_segment *seg) static int _thin_text_import(struct lv_segment *seg, const struct dm_config_node *sn) { - const char *lv_name; - struct logical_volume *pool_lv, *origin = NULL, *external_lv = NULL, *merge_lv = NULL; + const char *pool_name = NULL, *origin_name = NULL, *external_name = NULL, *merge_name = NULL; + struct logical_volume *pool_lv, *origin_lv = NULL, *external_lv = NULL, *merge_lv = NULL; struct generic_logical_volume *indirect_origin = NULL; - if (!dm_config_get_str(sn, "thin_pool", &lv_name)) - return SEG_LOG_ERROR("Thin pool must be a string in"); + struct config_value values[] = { /* sorted! */ + { "device_id", &seg->device_id, CONFIG_VALUE_UINT64, 1 }, + { "external_origin", &external_name, CONFIG_VALUE_STRING, }, + { "merge", &merge_name, CONFIG_VALUE_STRING, }, + { "origin", &origin_name, CONFIG_VALUE_STRING, }, + { "thin_pool", &pool_name, CONFIG_VALUE_STRING, 1 }, + { "transaction_id", &seg->transaction_id, CONFIG_VALUE_UINT64, 1 }, + }; - if (!(pool_lv = find_lv(seg->lv->vg, lv_name))) - return SEG_LOG_ERROR("Unknown thin pool %s in", lv_name); - - if (!dm_config_get_uint64(sn, "transaction_id", &seg->transaction_id)) - return SEG_LOG_ERROR("Could not read transaction_id for"); - - if (dm_config_has_node(sn, "origin")) { - if (!dm_config_get_str(sn, "origin", &lv_name)) - return SEG_LOG_ERROR("Origin must be a string in"); - - if (!(origin = find_lv(seg->lv->vg, lv_name))) - return SEG_LOG_ERROR("Unknown origin %s in", lv_name); - } - - if (dm_config_has_node(sn, "merge")) { - if (!dm_config_get_str(sn, "merge", &lv_name)) - return SEG_LOG_ERROR("Merge lv must be a string in"); - if (!(merge_lv = find_lv(seg->lv->vg, lv_name))) - return SEG_LOG_ERROR("Unknown merge lv %s in", lv_name); - } - - if (!dm_config_get_uint32(sn, "device_id", &seg->device_id)) - return SEG_LOG_ERROR("Could not read device_id for"); + if (!text_import_values(sn, values, DM_ARRAY_SIZE(values))) + return SEG_LOG_ERROR("Could not read segment values for"); if (seg->device_id > DM_THIN_MAX_DEVICE_ID) return SEG_LOG_ERROR("Unsupported value %u for device_id", seg->device_id); + if (!pool_name) + return SEG_LOG_ERROR("Thin pool must be a string in"); - if (dm_config_has_node(sn, "external_origin")) { - if (!dm_config_get_str(sn, "external_origin", &lv_name)) - return SEG_LOG_ERROR("External origin must be a string in"); + if (!(pool_lv = find_lv(seg->lv->vg, pool_name))) + return SEG_LOG_ERROR("Unknown thin pool %s in", pool_name); - if (!(external_lv = find_lv(seg->lv->vg, lv_name))) - return SEG_LOG_ERROR("Unknown external origin %s in", lv_name); - } + if (origin_name && + !(origin_lv = find_lv(seg->lv->vg, origin_name))) + return SEG_LOG_ERROR("Unknown origin %s in", origin_name); - if (!attach_pool_lv(seg, pool_lv, origin, indirect_origin, merge_lv)) + if (merge_name && + !(merge_lv = find_lv(seg->lv->vg, merge_name))) + return SEG_LOG_ERROR("Unknown merge lv %s in", merge_name); + + if (external_name && + !(external_lv = find_lv(seg->lv->vg, external_name))) + return SEG_LOG_ERROR("Unknown external origin %s in", external_name); + + if (!attach_pool_lv(seg, pool_lv, origin_lv, indirect_origin, merge_lv)) return_0; if (!attach_thin_external_origin(seg, external_lv))