From ae515916c05218a49b40afaaf26f9c4c0bb55e53 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac Date: Sat, 8 Feb 2025 21:51:09 +0100 Subject: [PATCH] mirror: enhance error path for pvmove finish When the pvmove operation is completing, it attempts to deactivate the temporary mirror and remove its mirror legs. However, if an external tool holds these volumes open, the operation would previously abort entirely, leaving the LVM2 metadata in a partially unusable state that required manual administrative fixes. To improve this, the code has been enhanced to handle such scenarios more gracefully. It will now complete the pvmove operation even if some volumes cannot be deactivated, marking them in the metadata with an error segment. While the command will report errors, the metadata will remain in a usable state. The administrator can then remove the orphaned volumes when they are no longer in use. (cherry picked from commit ed9468153ec3d9cec8d6fbb9f6b8e09e10427416) --- lib/metadata/mirror.c | 93 ++++++++++++++++++++++++++++++++++++++++--- tools/pvmove_poll.c | 13 ++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/lib/metadata/mirror.c b/lib/metadata/mirror.c index 46da57948..d5c988372 100644 --- a/lib/metadata/mirror.c +++ b/lib/metadata/mirror.c @@ -398,6 +398,59 @@ static int _activate_lv_like_model(struct logical_volume *model, return 1; } +/* + * Inherit tags @from_lv to @to_lv, tags maybe needed for activation + */ +static int _inherit_lv_tags(const struct logical_volume *from_lv, struct logical_volume *to_lv) +{ + struct dm_str_list *sl; + + if (to_lv && !str_list_match_list(&from_lv->tags, &to_lv->tags, NULL)) + dm_list_iterate_items(sl, &from_lv->tags) + if (!str_list_add(from_lv->vg->cmd->mem, &to_lv->tags, sl->str)) { + log_error("Aborting. Unable to inherit tag."); + return 0; + } + + return 1; +} + +/* + * Deactivates and removes an 'orphan' temporary @lv, + * which is expected to already have an 'error' segment. + * Sets @updated_mda (!NULL) to 1 when LV is removed. + * Sets @deactivation_failed (!NULL) to 1 when deactivation fails. + * + * Note: An external tool might still have the volume open, preventing + * immediate deactivation. If deactivation fails (even after retrying), + * it is safer to proceed with the command and leave the LV visible. + * This allows the user to manually remove it when it is no longer in use. + * + * Any errors will be detected later in the process, as there will be + * more visible LVs than expected. + */ +static int _deactivate_and_remove_lv(struct logical_volume *lv, + int *updated_mda, + int *deactivation_failed) +{ + if (lv) { + /* FIXME: convert to use lv_active_change() */ + if (!deactivate_lv(lv->vg->cmd, lv)) { + /* Note: still returns success here and fails later */ + log_warn("WARNING: Can't deactivate temporary volume %s.", + display_lvname(lv)); + if (deactivation_failed) + *deactivation_failed = 1; + } else if (!lv_remove(lv)) { + /* Can't continue with internal metadata problems */ + return_0; + } else if (updated_mda) + *updated_mda = 1; + } + + return 1; +} + /* * Delete independent/orphan LV, it must acquire lock. */ @@ -803,7 +856,6 @@ static int _remove_mirror_images(struct logical_volume *lv, struct lv_list *lvl; struct dm_list tmp_orphan_lvs; uint32_t orig_removed = num_removed; - int reactivate; if (removed) *removed = 0; @@ -967,6 +1019,19 @@ static int _remove_mirror_images(struct logical_volume *lv, return_0; } + if (!collapse) { + dm_list_iterate_items(lvl, &tmp_orphan_lvs) { + if (!_inherit_lv_tags(lv, lvl->lv)) + return_0; + if (!replace_lv_with_error_segment(lvl->lv)) + return_0; + } + } + + if (!_inherit_lv_tags(lv, temp_layer_lv) || + !_inherit_lv_tags(lv, detached_log_lv)) + return_0; + /* * To successfully remove these unwanted LVs we need to * remove the LVs from the mirror set, commit that metadata @@ -976,17 +1041,35 @@ static int _remove_mirror_images(struct logical_volume *lv, return_0; /* Save or delete the 'orphan' LVs */ - reactivate = lv_is_active(lv_lock_holder(lv)); + if (lv_is_active(lv_lock_holder(lv))) { + if (!collapse) { + dm_list_iterate_items(lvl, &tmp_orphan_lvs) + if (!_activate_lv_like_model(lv, lvl->lv)) + return_0; + } + + if (temp_layer_lv && + !_activate_lv_like_model(lv, temp_layer_lv)) + return_0; + + if (detached_log_lv && + !_activate_lv_like_model(lv, detached_log_lv)) + return_0; + + if (!sync_local_dev_names(lv->vg->cmd)) + stack; + } + if (!collapse) { dm_list_iterate_items(lvl, &tmp_orphan_lvs) - if (!_delete_lv(lv, lvl->lv, reactivate)) + if (!_deactivate_and_remove_lv(lvl->lv, NULL, NULL)) return_0; } - if (temp_layer_lv && !_delete_lv(lv, temp_layer_lv, reactivate)) + if (!_deactivate_and_remove_lv(temp_layer_lv, NULL, NULL)) return_0; - if (detached_log_lv && !_delete_lv(lv, detached_log_lv, reactivate)) + if (!_deactivate_and_remove_lv(detached_log_lv, NULL, NULL)) return_0; /* Mirror with only 1 area is 'in sync'. */ diff --git a/tools/pvmove_poll.c b/tools/pvmove_poll.c index 751313cd7..8b97905c0 100644 --- a/tools/pvmove_poll.c +++ b/tools/pvmove_poll.c @@ -87,6 +87,8 @@ int pvmove_update_metadata(struct cmd_context *cmd, struct volume_group *vg, int pvmove_finish(struct cmd_context *cmd, struct volume_group *vg, struct logical_volume *lv_mirr, struct dm_list *lvs_changed) { + uint32_t visible = vg_visible_lvs(lv_mirr->vg); + if (!dm_list_empty(lvs_changed) && (!_detach_pvmove_mirror(cmd, lv_mirr) || !replace_lv_with_error_segment(lv_mirr))) { @@ -94,6 +96,8 @@ int pvmove_finish(struct cmd_context *cmd, struct volume_group *vg, return 0; } + lv_set_visible(lv_mirr); + if (!lv_update_and_reload(lv_mirr)) return_0; @@ -120,5 +124,14 @@ int pvmove_finish(struct cmd_context *cmd, struct volume_group *vg, return 0; } + /* Allows the pvmove operation to complete even if 'orphaned' temporary volumes + * cannot be deactivated due to being held open by another process. + * The user can manually remove these volumes later when they are no longer in use. */ + if (visible < vg_visible_lvs(lv_mirr->vg)) { + log_error("ABORTING: Failed to remove temporary logical volume(s)."); + log_print_unless_silent("Please remove orphan temporary logical volume(s) when possible."); + return 0; + } + return 1; } -- 2.48.1