File 0005-lvmlockd-fix-thick-to-thin-lv-conversion.patch of Package lvm2.38362
From 76a4599500aef12442c8c24be8aacc89f6a27e68 Mon Sep 17 00:00:00 2001
From: David Teigland <teigland@redhat.com>
Date: Tue, 15 Aug 2023 09:53:39 -0500
Subject: [PATCH 05/24] lvmlockd: fix thick to thin lv conversion
---
lib/locking/lvmlockd.c | 2 +
lib/metadata/metadata.c | 2 +
tools/lvconvert.c | 231 ++++++++++++++++++++++++----------------
3 files changed, 143 insertions(+), 92 deletions(-)
diff --git a/lib/locking/lvmlockd.c b/lib/locking/lvmlockd.c
index a8db25d7a..d44b7333a 100644
--- a/lib/locking/lvmlockd.c
+++ b/lib/locking/lvmlockd.c
@@ -2908,6 +2908,8 @@ static int _free_lv(struct cmd_context *cmd, struct volume_group *vg,
if (!id_write_format(lv_id, lv_uuid, sizeof(lv_uuid)))
return_0;
+ log_debug("lockd free LV %s/%s %s lock_args %s", vg->name, lv_name, lv_uuid, lock_args ?: "none");
+
reply = _lockd_send("free_lv",
"pid = " FMTd64, (int64_t) getpid(),
"vg_name = %s", vg->name,
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index f56b5002d..f8a4f6279 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2924,6 +2924,8 @@ int vg_write(struct volume_group *vg)
vgid[ID_LEN] = 0;
memcpy(vgid, &vg->id.uuid, ID_LEN);
+ log_debug("Writing metadata for VG %s.", vg->name);
+
if (vg_is_shared(vg)) {
dm_list_iterate_items(lvl, &vg->lvs) {
if (lvl->lv->lock_args && !strcmp(lvl->lv->lock_args, "pending")) {
diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index d98d34ce0..7c9540712 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -3007,9 +3007,22 @@ static struct logical_volume *_lvconvert_insert_thin_layer(struct logical_volume
if (!(thin_segtype = get_segtype_from_string(vg->cmd, SEG_TYPE_NAME_THIN)))
return_NULL;
+ /*
+ * input lv foo (often linear)
+ * creates new lv foo_tpoolN (no seg)
+ * segment from foo is moved to foo_tpoolN
+ * new linear segment is created for foo that maps to foo_tpoolN
+ * returns foo_tpoolN
+ *
+ * In spite of the "pool" variable naming, pool_lv foo_tpoolN is *not*
+ * yet a pool type, but rather is whatever type the input lv was.
+ */
if (!(pool_lv = insert_layer_for_lv(vg->cmd, lv, 0, "_tpool%d")))
return_NULL;
+ /*
+ * change lv foo to a thin LV using foo_tpoolN
+ */
lv->status |= THIN_VOLUME | VIRTUAL;
lv_set_visible(pool_lv);
@@ -3079,9 +3092,12 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
const char *pool_metadata_name; /* user-specified lv name */
char converted_names[3*NAME_LEN]; /* preserve names of converted lv */
struct segment_type *pool_segtype; /* thinpool or cachepool */
+ const char *str_seg_type = to_cachepool ? SEG_TYPE_NAME_CACHE_POOL : SEG_TYPE_NAME_THIN_POOL;
struct lv_segment *seg;
unsigned int target_attr = ~0;
unsigned int activate_pool;
+ unsigned int lock_active_pool;
+ unsigned int lock_active_pool_done = 0;
unsigned int zero_metadata;
uint64_t meta_size;
uint32_t meta_extents;
@@ -3096,16 +3112,18 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
thin_discards_t discards;
thin_zero_t zero_new_blocks;
int error_when_full;
- int r = 0;
+ int end_error = 0;
+ int is_active;
/* for handling lvmlockd cases */
char *lockd_data_args = NULL;
char *lockd_meta_args = NULL;
char *lockd_data_name = NULL;
char *lockd_meta_name = NULL;
+ uint32_t lockd_data_flags = 0;
+ uint32_t lockd_meta_flags = 0;
struct id lockd_data_id;
struct id lockd_meta_id;
- const char *str_seg_type = to_cachepool ? SEG_TYPE_NAME_CACHE_POOL : SEG_TYPE_NAME_THIN_POOL;
if (!_raid_split_image_conversion(lv))
return_0;
@@ -3124,8 +3142,10 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
return 0;
}
- /* Allow to have only thinpool active and restore it's active state. */
- activate_pool = to_thinpool && lv_is_active(lv);
+ is_active = lv_is_active(lv);
+
+ activate_pool = to_thinpool && is_active;
+ lock_active_pool = (to_thinpool || to_thin) && is_active;
/* Wipe metadata_lv by default, but allow skipping this for cache pools. */
zero_metadata = (to_cachepool) ? arg_int_value(cmd, zero_ARG, 1) : 1;
@@ -3356,20 +3376,11 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
}
}
- /*
- * When the LV referenced by the original function arg "lv"
- * is layered
- *
- * pool_name pool name taken from lv arg
- * data_name sub lv name, generated
- * meta_name sub lv name, generated
- *
- * pool_lv new lv for pool object, created here
- * data_lv sub lv, was lv arg, now renamed
- * metadata_lv sub lv, existing or created here
- */
-
if (to_thin) {
+ /*
+ * pool_lv is not yet a pool, when returned, pool_lv contains
+ * the segment that belonged to "lv".
+ */
if (!(pool_lv = _lvconvert_insert_thin_layer(lv)))
goto_bad;
} else {
@@ -3383,6 +3394,17 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
pool_lv = lv;
}
+ /*
+ * starts with pool_lv foo (not a pool yet)
+ * creates new data_lv foo_tdata
+ * segment from pool_lv foo is moved to data_lv foo_tdata
+ * pool_lv foo linear segment is created that maps to foo_tdata
+ * returns data_lv foo_tdata
+ *
+ * (In the to_thin case, the segment from the original lv is first
+ * moved to pool_lv by _lvconvert_insert_thin_layer, and now is
+ * moved to data_lv.)
+ */
if (!(data_lv = insert_layer_for_lv(cmd, pool_lv, 0,
(to_cachepool ? "_cdata" : "_tdata"))))
goto_bad;
@@ -3390,33 +3412,15 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
data_lv->status |= (to_cachepool) ? CACHE_POOL_DATA : THIN_POOL_DATA;
data_lv->status |= LVM_WRITE; /* Pool data LV is writable */
+ /*
+ * pool_lv now becomes a pool type.
+ * FIXME: change variable naming to avoid this confusion.
+ */
pool_lv->status |= (to_cachepool) ? CACHE_POOL : THIN_POOL;
seg = first_seg(pool_lv);
seg->segtype = pool_segtype;
- /*
- * Create a new lock for a thin pool LV. A cache pool LV has no lock.
- * Locks are removed from existing LVs that are being converted to
- * data and meta LVs (they are unlocked and deleted below.)
- */
- if (vg_is_shared(vg)) {
- lv->lock_args = NULL;
- pool_lv->lock_args = NULL;
- data_lv->lock_args = NULL;
- metadata_lv->lock_args = NULL;
-
- if (!to_cachepool) {
- if (!strcmp(vg->lock_type, "sanlock"))
- pool_lv->lock_args = "pending";
- else if (!strcmp(vg->lock_type, "dlm"))
- pool_lv->lock_args = "dlm";
- else if (!strcmp(vg->lock_type, "idm"))
- pool_lv->lock_args = "idm";
- /* The lock_args will be set in vg_write(). */
- }
- }
-
/* Apply settings to the new pool seg */
if (to_cachepool) {
if (!cache_set_params(seg, chunk_size, cache_metadata_format, cache_mode, policy_name, policy_settings))
@@ -3453,87 +3457,130 @@ static int _lvconvert_to_pool(struct cmd_context *cmd,
if (!_lvconvert_attach_metadata_to_pool(seg, metadata_lv))
goto_bad;
- if (!handle_pool_metadata_spare(vg,
- metadata_lv->le_count,
- use_pvh, pool_metadata_spare))
- goto_bad;
+ /*
+ * Create a new lock for a thin pool LV. A cache pool LV has no lock.
+ * Locks are removed from existing LVs that are being converted to
+ * data and meta LVs (they are unlocked and deleted below.)
+ * Acquire the new thin pool lock if the pool will remain active at
+ * the end of the command.
+ */
+ if (vg_is_shared(vg)) {
+ lv->lock_args = NULL;
+ pool_lv->lock_args = NULL;
+ data_lv->lock_args = NULL;
+ metadata_lv->lock_args = NULL;
- if (to_thin) {
- if (!lockd_lv(cmd, pool_lv, "ex", LDLV_PERSISTENT)) {
- log_error("Failed to lock pool LV %s.", display_lvname(pool_lv));
- goto out;
+ if (!to_cachepool) {
+ if (!strcmp(vg->lock_type, "sanlock")) {
+ if (!lockd_init_lv_args(cmd, vg, pool_lv,
+ vg->lock_type, &pool_lv->lock_args)) {
+ log_error("Cannot allocate lock for new pool LV.");
+ goto_bad;
+ }
+ pool_lv->new_lock_args = 1; /* tells vg_revert to lockd_free_lv */
+ } else if (!strcmp(vg->lock_type, "dlm")) {
+ pool_lv->lock_args = "dlm";
+ } else if (!strcmp(vg->lock_type, "idm")) {
+ pool_lv->lock_args = "idm";
+ }
+
+ if (lock_active_pool) {
+ if (!lockd_lv(cmd, pool_lv, "ex", LDLV_PERSISTENT)) {
+ log_error("Failed to lock new pool LV %s.", display_lvname(pool_lv));
+ goto_bad;
+ }
+ lock_active_pool_done = 1;
+ }
}
+ }
+
+ if (to_thin) {
if (!lv_update_and_reload(lv))
goto_bad;
} else {
if (!vg_write(vg) || !vg_commit(vg))
goto_bad;
+ }
- if (activate_pool) {
- if (!lockd_lv(cmd, pool_lv, "ex", LDLV_PERSISTENT)) {
- log_error("Failed to lock pool LV %s.", display_lvname(pool_lv));
- goto out;
- }
+ /*
+ * The main conversion is successfully committed. If any subsequent
+ * steps fail (creating spare, activating, unlocking), we do not
+ * currently have the ability to undo the changes committed up to this
+ * point. Failures in the remaining steps can print an error and cause
+ * the command to exit with an error, but no partial revert of the
+ * completed steps is attempted.
+ */
+ log_print_unless_silent("Converted %s to %s %s.", converted_names,
+ (to_cachepool) ? "cache" : "thin",
+ (to_thin) ? "volume" : "pool");
- if (!activate_lv(cmd, pool_lv)) {
- log_error("Failed to activate pool logical volume %s.",
- display_lvname(pool_lv));
-
- /* Deactivate subvolumes */
- if (!deactivate_lv(cmd, seg_lv(seg, 0)))
- log_error("Failed to deactivate pool data logical volume %s.",
- display_lvname(seg_lv(seg, 0)));
- if (!deactivate_lv(cmd, seg->metadata_lv))
- log_error("Failed to deactivate pool metadata logical volume %s.",
- display_lvname(seg->metadata_lv));
- goto out;
- }
- }
+ /*
+ * FIXME handle_pool_metadata_spare() calls vg_write() vg_commit()
+ * after creating a new lvolN, but then lvolN is renamed and hidden as
+ * [lvolN_pmspare] without any further vg_write(). So, there's an extra
+ * vg_write and vg_commit required here to cover the renaming/hiding.
+ */
+ if (!handle_pool_metadata_spare(vg, metadata_lv->le_count, use_pvh, pool_metadata_spare) ||
+ !vg_write(vg) || !vg_commit(vg)) {
+ log_error("Failed to set up spare metadata LV for thin pool.");
+ end_error = 1;
}
- r = 1;
-
-out:
- if (r)
- log_print_unless_silent("Converted %s to %s %s.",
- converted_names, (to_cachepool) ? "cache" : "thin",
- (to_thin) ? "volume" : "pool");
+ if (activate_pool && !activate_lv(cmd, pool_lv)) {
+ log_error("Failed to activate pool logical volume %s.", display_lvname(pool_lv));
+ end_error = 1;
+ }
/*
* Unlock and free the locks from existing LVs that became pool data
* and meta LVs.
*/
if (lockd_data_name) {
- if (!lockd_lv_name(cmd, vg, lockd_data_name, &lockd_data_id, lockd_data_args, "un", LDLV_PERSISTENT))
+ if (!lockd_lv_name(cmd, vg, lockd_data_name, &lockd_data_id, lockd_data_args, "un", lockd_data_flags)) {
log_error("Failed to unlock pool data LV %s/%s", vg->name, lockd_data_name);
- lockd_free_lv(cmd, vg, lockd_data_name, &lockd_data_id, lockd_data_args);
+ end_error = 1;
+ }
+ if (!lockd_free_lv(cmd, vg, lockd_data_name, &lockd_data_id, lockd_data_args)) {
+ log_error("Failed to free lock for pool data LV %s/%s", vg->name, lockd_data_name);
+ end_error = 1;
+ }
}
-
if (lockd_meta_name) {
- if (!lockd_lv_name(cmd, vg, lockd_meta_name, &lockd_meta_id, lockd_meta_args, "un", LDLV_PERSISTENT))
+ if (!lockd_lv_name(cmd, vg, lockd_meta_name, &lockd_meta_id, lockd_meta_args, "un", lockd_meta_flags)) {
log_error("Failed to unlock pool metadata LV %s/%s", vg->name, lockd_meta_name);
- lockd_free_lv(cmd, vg, lockd_meta_name, &lockd_meta_id, lockd_meta_args);
+ end_error = 1;
+ }
+ if (!lockd_free_lv(cmd, vg, lockd_meta_name, &lockd_meta_id, lockd_meta_args)) {
+ log_error("Failed to free lock for pool metadata LV %s/%s", vg->name, lockd_meta_name);
+ end_error = 1;
+ }
}
-bad:
+
if (policy_settings)
dm_config_destroy(policy_settings);
- return r;
-#if 0
-revert_new_lv:
- /* TBD */
- if (!pool_metadata_lv_name) {
- if (!deactivate_lv(cmd, metadata_lv)) {
- log_error("Failed to deactivate metadata lv.");
- return 0;
- }
- if (!lv_remove(metadata_lv) || !vg_write(vg) || !vg_commit(vg))
- log_error("Manual intervention may be required to remove "
- "abandoned LV(s) before retrying.");
+ if (end_error) {
+ log_error("Manual intervention may be required to handle reported errors.");
+ return 0;
}
+ return 1;
+
+ /*
+ * Error exit path for failures that occur before the main conversion
+ * is committed. Failures that occur after the main conversion is
+ * committed should not exit here. There is some cleanup missing here.
+ */
+bad:
+ if (lock_active_pool_done)
+ lockd_lv(cmd, pool_lv, "un", LDLV_PERSISTENT);
+ if (pool_lv && pool_lv->lock_args && pool_lv->new_lock_args)
+ lockd_free_lv(cmd, vg, pool_lv->name, &pool_lv->lvid.id[1], pool_lv->lock_args);
+
+ if (policy_settings)
+ dm_config_destroy(policy_settings);
+
return 0;
-#endif
}
static int _cache_vol_attach(struct cmd_context *cmd,
--
2.35.3
