From 5800560602a08c0a860dd119acf27055c9c14b2b Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Tue, 21 Apr 2009 14:31:57 +0000 Subject: [PATCH] Move metadata backup call after vg_commit. The backup() call store metadata from memory. But in cluster backup() call performs remote nodes metadata backup and it reads data from disk. For metadata backup consistency, patch moves all backup() calls after vg_commit. (Moreover, some tools already do that this way.) --- WHATS_NEW | 1 + lib/metadata/lv_manip.c | 22 +++++++---------- lib/metadata/mirror.c | 25 +++++++++++--------- tools/lvchange.c | 52 +++++++++++++++++++---------------------- tools/lvconvert.c | 45 +++++++++++++++++++---------------- tools/lvcreate.c | 10 ++++---- tools/lvresize.c | 7 ++++-- tools/pvmove.c | 21 +++++++++-------- 8 files changed, 94 insertions(+), 89 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index c59ee20a7..db9221ec7 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.46 - ================================ + Fix metadata backup to run after vg_commit always. Tidy clvmd volume lock cache functions. Fix pvs report for orphan PVs when segment attributes are requested. Fix pvs -a output to not read volume groups from non-PV devices. diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index ec9e19e7b..7214b27c9 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -1736,6 +1736,7 @@ int lv_rename(struct cmd_context *cmd, struct logical_volume *lv, { struct volume_group *vg = lv->vg; struct lv_names lv_names; + int r = 0; /* rename is not allowed on sub LVs */ if (!lv_is_displayable(lv)) { @@ -1773,23 +1774,21 @@ int lv_rename(struct cmd_context *cmd, struct logical_volume *lv, if (!vg_write(vg)) return 0; - backup(vg); - if (!suspend_lv(cmd, lv)) { - stack; vg_revert(vg); - return 0; + goto_out; } if (!vg_commit(vg)) { - stack; resume_lv(cmd, lv); - return 0; + goto_out; } resume_lv(cmd, lv); - - return 1; + r = 1; +out: + backup(vg); + return r; } char *generate_lv_name(struct volume_group *vg, const char *format, @@ -2051,14 +2050,11 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv, } /* store it on disks */ - if (!vg_write(vg)) - return 0; + if (!vg_write(vg) || !vg_commit(vg)) + return_0; backup(vg); - if (!vg_commit(vg)) - return 0; - /* If no snapshots left, reload without -real. */ if (origin && !lv_is_origin(origin)) { if (!suspend_lv(cmd, origin)) diff --git a/lib/metadata/mirror.c b/lib/metadata/mirror.c index e10d0dc74..c53aec3a3 100644 --- a/lib/metadata/mirror.c +++ b/lib/metadata/mirror.c @@ -270,14 +270,11 @@ static int _init_mirror_log(struct cmd_context *cmd, } /* store mirror log on disk(s) */ - if (!vg_write(log_lv->vg)) + if (!vg_write(log_lv->vg) || !vg_commit(log_lv->vg)) goto activate_lv; backup(log_lv->vg); - if (!vg_commit(log_lv->vg)) - goto activate_lv; - if (!activate_lv(cmd, log_lv)) { log_error("Aborting. Failed to activate mirror log."); goto revert_new_lv; @@ -334,10 +331,12 @@ revert_new_lv: return 0; } - if (!vg_write(log_lv->vg) || - (backup(log_lv->vg), !vg_commit(log_lv->vg))) + if (!vg_write(log_lv->vg) || !vg_commit(log_lv->vg)) log_error("Manual intervention may be required to " "remove/restore abandoned log LV before retrying."); + else + backup(log_lv->vg); + activate_lv: if (was_active && !remove_on_failure && !activate_lv(cmd, log_lv)) return_0; @@ -1504,11 +1503,15 @@ int add_mirror_images(struct cmd_context *cmd, struct logical_volume *lv, return 1; out_remove_log: - if (log_lv && (!lv_remove(log_lv) || !vg_write(log_lv->vg) || - (backup(log_lv->vg), !vg_commit(log_lv->vg)))) - log_error("Manual intervention may be required to remove " - "abandoned log LV before retrying."); - + if (log_lv) { + if (!lv_remove(log_lv) || + !vg_write(log_lv->vg) || + !vg_commit(log_lv->vg)) + log_error("Manual intervention may be required to remove " + "abandoned log LV before retrying."); + else + backup(log_lv->vg); + } out_remove_imgs: alloc_destroy(ah); return 0; diff --git a/tools/lvchange.c b/tools/lvchange.c index cd0ff5af8..f5832ee01 100644 --- a/tools/lvchange.c +++ b/tools/lvchange.c @@ -20,6 +20,7 @@ static int lvchange_permission(struct cmd_context *cmd, { uint32_t lv_access; struct lvinfo info; + int r = 0; lv_access = arg_uint_value(cmd, permission_ARG, 0); @@ -56,26 +57,27 @@ static int lvchange_permission(struct cmd_context *cmd, if (!vg_write(lv->vg)) return_0; - backup(lv->vg); - if (!suspend_lv(cmd, lv)) { log_error("Failed to lock %s", lv->name); vg_revert(lv->vg); - return 0; + goto out; } if (!vg_commit(lv->vg)) { resume_lv(cmd, lv); - return 0; + goto_out; } log_very_verbose("Updating permissions for \"%s\" in kernel", lv->name); if (!resume_lv(cmd, lv)) { log_error("Problem reactivating %s", lv->name); - return 0; + goto out; } - return 1; + r = 1; +out: + backup(lv->vg); + return r; } static int lvchange_monitoring(struct cmd_context *cmd, @@ -274,8 +276,6 @@ static int lvchange_resync(struct cmd_context *cmd, return 0; } - backup(lv->vg); - if (!vg_commit(lv->vg)) { log_error("Failed to commit intermediate VG metadata."); if (!attach_mirror_log(first_seg(lv), log_lv)) @@ -285,6 +285,8 @@ static int lvchange_resync(struct cmd_context *cmd, return 0; } + backup(lv->vg); + if (!activate_lv(cmd, log_lv)) { log_error("Unable to activate %s for mirror log resync", log_lv->name); @@ -349,15 +351,12 @@ static int lvchange_alloc(struct cmd_context *cmd, struct logical_volume *lv) log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name); - if (!vg_write(lv->vg)) + /* No need to suspend LV for this change */ + if (!vg_write(lv->vg) || !vg_commit(lv->vg)) return_0; backup(lv->vg); - /* No need to suspend LV for this change */ - if (!vg_commit(lv->vg)) - return_0; - return 1; } @@ -366,6 +365,7 @@ static int lvchange_readahead(struct cmd_context *cmd, { unsigned read_ahead = 0; unsigned pagesize = (unsigned) lvm_getpagesize() >> SECTOR_SHIFT; + int r = 0; read_ahead = arg_uint_value(cmd, readahead_ARG, 0); @@ -401,26 +401,27 @@ static int lvchange_readahead(struct cmd_context *cmd, if (!vg_write(lv->vg)) return_0; - backup(lv->vg); - if (!suspend_lv(cmd, lv)) { log_error("Failed to lock %s", lv->name); vg_revert(lv->vg); - return 0; + goto out; } if (!vg_commit(lv->vg)) { resume_lv(cmd, lv); - return 0; + goto_out; } log_very_verbose("Updating permissions for \"%s\" in kernel", lv->name); if (!resume_lv(cmd, lv)) { log_error("Problem reactivating %s", lv->name); - return 0; + goto out; } - return 1; + r = 1; +out: + backup(lv->vg); + return r; } static int lvchange_persistent(struct cmd_context *cmd, @@ -477,14 +478,11 @@ static int lvchange_persistent(struct cmd_context *cmd, } log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name); - if (!vg_write(lv->vg)) + if (!vg_write(lv->vg) || !vg_commit(lv->vg)) return_0; backup(lv->vg); - if (!vg_commit(lv->vg)) - return_0; - if (active) { log_verbose("Re-activating logical volume \"%s\"", lv->name); if (!activate_lv(cmd, lv)) { @@ -527,15 +525,13 @@ static int lvchange_tag(struct cmd_context *cmd, struct logical_volume *lv, } log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name); - if (!vg_write(lv->vg)) - return_0; - - backup(lv->vg); /* No need to suspend LV for this change */ - if (!vg_commit(lv->vg)) + if (!vg_write(lv->vg) || !vg_commit(lv->vg)) return_0; + backup(lv->vg); + return 1; } diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 2ded226a3..c95426bed 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -269,6 +269,8 @@ static int _finish_lvconvert_mirror(struct cmd_context *cmd, struct logical_volume *lv, struct dm_list *lvs_changed __attribute((unused))) { + int r = 0; + if (!collapse_mirrored_lv(lv)) { log_error("Failed to remove temporary sync layer."); return 0; @@ -281,29 +283,29 @@ static int _finish_lvconvert_mirror(struct cmd_context *cmd, if (!vg_write(vg)) return_0; - backup(vg); - if (!suspend_lv(cmd, lv)) { log_error("Failed to lock %s", lv->name); vg_revert(vg); - return 0; + goto out; } if (!vg_commit(vg)) { resume_lv(cmd, lv); - return 0; + goto_out; } log_very_verbose("Updating \"%s\" in kernel", lv->name); if (!resume_lv(cmd, lv)) { log_error("Problem reactivating %s", lv->name); - return 0; + goto out; } + r = 1; log_print("Logical volume %s converted.", lv->name); - - return 1; +out: + backup(vg); + return r; } static struct poll_functions _lvconvert_mirror_fns = { @@ -383,6 +385,7 @@ static int lvconvert_mirrors(struct cmd_context * cmd, struct logical_volume * l const char *mirrorlog; unsigned corelog = 0; struct logical_volume *original_lv; + int r = 0; seg = first_seg(lv); existing_mirrors = lv_mirror_count(lv); @@ -589,30 +592,31 @@ commit_changes: if (!vg_write(lv->vg)) return_0; - backup(lv->vg); - if (!suspend_lv(cmd, lv)) { log_error("Failed to lock %s", lv->name); vg_revert(lv->vg); - return 0; + goto out; } if (!vg_commit(lv->vg)) { resume_lv(cmd, lv); - return 0; + goto_out; } log_very_verbose("Updating \"%s\" in kernel", lv->name); if (!resume_lv(cmd, lv)) { log_error("Problem reactivating %s", lv->name); - return 0; + goto out; } if (!lp->need_polling) log_print("Logical volume %s converted.", lv->name); - return 1; + r = 1; +out: + backup(lv->vg); + return r; } static int lvconvert_snapshot(struct cmd_context *cmd, @@ -620,6 +624,7 @@ static int lvconvert_snapshot(struct cmd_context *cmd, struct lvconvert_params *lp) { struct logical_volume *org; + int r = 0; if (!(org = find_lv(lv->vg, lp->origin))) { log_error("Couldn't find origin volume '%s'.", lp->origin); @@ -664,25 +669,25 @@ static int lvconvert_snapshot(struct cmd_context *cmd, if (!vg_write(lv->vg)) return_0; - backup(lv->vg); - if (!suspend_lv(cmd, org)) { log_error("Failed to suspend origin %s", org->name); vg_revert(lv->vg); - return 0; + goto out; } if (!vg_commit(lv->vg)) - return_0; + goto_out; if (!resume_lv(cmd, org)) { log_error("Problem reactivating origin %s", org->name); - return 0; + goto out; } log_print("Logical volume %s converted to snapshot.", lv->name); - - return 1; + r = 1; +out: + backup(lv->vg); + return r; } static int lvconvert_single(struct cmd_context *cmd, struct logical_volume *lv, diff --git a/tools/lvcreate.c b/tools/lvcreate.c index e5916db95..78dc6b3a5 100644 --- a/tools/lvcreate.c +++ b/tools/lvcreate.c @@ -786,14 +786,11 @@ static int _lvcreate(struct cmd_context *cmd, struct volume_group *vg, } /* store vg on disk(s) */ - if (!vg_write(vg)) + if (!vg_write(vg) || !vg_commit(vg)) return_0; backup(vg); - if (!vg_commit(vg)) - return_0; - if (lp->snapshot) { if (!activate_lv_excl(cmd, lv)) { log_error("Aborting. Failed to activate snapshot " @@ -878,9 +875,12 @@ deactivate_and_revert_new_lv: revert_new_lv: /* FIXME Better to revert to backup of metadata? */ - if (!lv_remove(lv) || !vg_write(vg) || (backup(vg), !vg_commit(vg))) + if (!lv_remove(lv) || !vg_write(vg) || !vg_commit(vg)) log_error("Manual intervention may be required to remove " "abandoned LV(s) before retrying."); + else + backup(vg); + return 0; } diff --git a/tools/lvresize.c b/tools/lvresize.c index 0f3ddc27a..5737558e7 100644 --- a/tools/lvresize.c +++ b/tools/lvresize.c @@ -626,8 +626,6 @@ static int _lvresize(struct cmd_context *cmd, struct volume_group *vg, return ECMD_FAILED; } - backup(vg); - /* If snapshot, must suspend all associated devices */ if (lv_is_cow(lv)) lock_lv = origin_from_cow(lv); @@ -637,20 +635,25 @@ static int _lvresize(struct cmd_context *cmd, struct volume_group *vg, if (!suspend_lv(cmd, lock_lv)) { log_error("Failed to suspend %s", lp->lv_name); vg_revert(vg); + backup(vg); return ECMD_FAILED; } if (!vg_commit(vg)) { stack; resume_lv(cmd, lock_lv); + backup(vg); return ECMD_FAILED; } if (!resume_lv(cmd, lock_lv)) { log_error("Problem reactivating %s", lp->lv_name); + backup(vg); return ECMD_FAILED; } + backup(vg); + log_print("Logical volume %s successfully resized", lp->lv_name); if (lp->resizefs && (lp->resize == LV_EXTEND) && diff --git a/tools/pvmove.c b/tools/pvmove.c index f89ea934b..55285df47 100644 --- a/tools/pvmove.c +++ b/tools/pvmove.c @@ -280,6 +280,7 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg, { unsigned exclusive = _pvmove_is_exclusive(cmd, vg); unsigned first_time = (flags & PVMOVE_FIRST_TIME) ? 1 : 0; + int r = 0; log_verbose("Updating volume group metadata"); if (!vg_write(vg)) { @@ -287,19 +288,16 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg, return 0; } - backup(vg); - /* Suspend lvs_changed */ if (!suspend_lvs(cmd, lvs_changed)) - return_0; + goto_out; /* Suspend mirrors on subsequent calls */ if (!first_time) { if (!suspend_lv(cmd, lv_mirr)) { - stack; resume_lvs(cmd, lvs_changed); vg_revert(vg); - return 0; + goto_out; } } @@ -309,7 +307,7 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg, if (!first_time) resume_lv(cmd, lv_mirr); resume_lvs(cmd, lvs_changed); - return 0; + goto out; } /* Activate the temporary mirror LV */ @@ -323,22 +321,25 @@ static int _update_metadata(struct cmd_context *cmd, struct volume_group *vg, "Run pvmove --abort."); /* FIXME Resume using *original* metadata here! */ resume_lvs(cmd, lvs_changed); - return 0; + goto out; } } else if (!resume_lv(cmd, lv_mirr)) { log_error("Unable to reactivate logical volume \"%s\"", lv_mirr->name); resume_lvs(cmd, lvs_changed); - return 0; + goto out; } /* Unsuspend LVs */ if (!resume_lvs(cmd, lvs_changed)) { log_error("Unable to resume logical volumes"); - return 0; + goto out; } - return 1; + r = 1; +out: + backup(vg); + return r; } static int _set_up_pvmove(struct cmd_context *cmd, const char *pv_name, -- 2.43.5