From: Zdenek Kabelac Date: Tue, 9 Sep 2014 12:40:32 +0000 (+0200) Subject: lv_update_and_reload: replace code sequence X-Git-Tag: v2_02_112~372 X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=c710f02e0181cc2db5455f0c98033247a70ecc30;p=lvm2.git lv_update_and_reload: replace code sequence Use lv_update_and_reload() and lv_update_and_reload_origin() to handle write/suspend/commit/resume sequence. In few places this properly handle vg_revert() after suspend failure, and also ensures there is metadata backup after successful vg_commit(). --- diff --git a/lib/metadata/cache_manip.c b/lib/metadata/cache_manip.c index 04e165fa5..73df448bb 100644 --- a/lib/metadata/cache_manip.c +++ b/lib/metadata/cache_manip.c @@ -175,7 +175,6 @@ static int _cleanup_orphan_lv(struct logical_volume *lv) */ int lv_cache_remove(struct logical_volume *cache_lv) { - struct cmd_context *cmd = cache_lv->vg->cmd; const char *policy_name; uint64_t dirty_blocks; struct lv_segment *cache_seg = first_seg(cache_lv); @@ -225,14 +224,8 @@ int lv_cache_remove(struct logical_volume *cache_lv) cache_seg->policy_argv = NULL; /* update the kernel to put the cleaner policy in place */ - if (!vg_write(cache_lv->vg)) - return_0; - if (!suspend_lv(cmd, cache_lv)) - return_0; - if (!vg_commit(cache_lv->vg)) - return_0; - if (!resume_lv(cmd, cache_lv)) - return_0; + if (lv_update_and_reload(cache_lv)) + return_0; } //FIXME: use polling to do this... @@ -256,7 +249,7 @@ int lv_cache_remove(struct logical_volume *cache_lv) if (!remove_layer_from_lv(cache_lv, corigin_lv)) return_0; - if (!vg_write(cache_lv->vg)) + if (!lv_update_and_reload(cache_lv)) return_0; /* @@ -264,20 +257,12 @@ int lv_cache_remove(struct logical_volume *cache_lv) * - the top-level cache LV * - the origin * - the cache_pool _cdata and _cmeta - */ - if (!suspend_lv(cmd, cache_lv)) - return_0; - - if (!vg_commit(cache_lv->vg)) - return_0; - - /* resume_lv on this (former) cache LV will resume all */ - /* + * + * resume_lv on this (former) cache LV will resume all + * * FIXME: currently we can't easily avoid execution of * blkid on resumed error device */ - if (!resume_lv(cmd, cache_lv)) - return_0; /* * cleanup orphan devices diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index cec9f8452..cc7b6dace 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -5039,26 +5039,8 @@ int lv_resize(struct cmd_context *cmd, struct logical_volume *lv, } /* store vg on disk(s) */ - if (!vg_write(vg)) - goto_out; - - if (!suspend_lv(cmd, lock_lv)) { - log_error("Failed to suspend %s", lock_lv->name); - vg_revert(vg); - goto bad; - } - - if (!vg_commit(vg)) { - stack; - if (!resume_lv(cmd, lock_lv)) - stack; - goto bad; - } - - if (!resume_lv(cmd, lock_lv)) { - log_error("Problem reactivating %s", lock_lv->name); - goto bad; - } + if (!lv_update_and_reload(lock_lv)) + goto_bad; if (lv_is_cow_covering_origin(lv)) if (!monitor_dev_for_events(cmd, lv, 0, 0)) @@ -5069,15 +5051,14 @@ int lv_resize(struct cmd_context *cmd, struct logical_volume *lv, if (!update_pool_lv(lock_lv, 0)) goto_bad; + backup(vg); + if (inactive && !deactivate_lv(cmd, lock_lv)) { log_error("Problem deactivating %s.", lock_lv->name); - backup(vg); return 0; } } - backup(vg); - log_print_unless_silent("Logical volume %s successfully resized", lp->lv_name); if (lp->resizefs && (lp->resize == LV_EXTEND) && @@ -5085,10 +5066,7 @@ int lv_resize(struct cmd_context *cmd, struct logical_volume *lv, return_0; return 1; - bad: - backup(vg); -out: if (inactive && !deactivate_lv(cmd, lock_lv)) log_error("Problem deactivating %s.", lock_lv->name); @@ -6951,9 +6929,10 @@ static struct logical_volume *_lv_create_an_lv(struct volume_group *vg, * I say that would be cleaner, but I'm not sure * about the effects on thinpool yet... */ - if (!vg_write(vg) || !suspend_lv(cmd, lv) || - !vg_commit(vg) || !resume_lv(cmd, lv)) + if (!lv_update_and_reload(lv)) { + stack; goto deactivate_and_revert_new_lv; + } if (!(lvl = find_lv_in_vg(vg, lp->origin))) goto deactivate_and_revert_new_lv; diff --git a/lib/metadata/raid_manip.c b/lib/metadata/raid_manip.c index ab30af924..d39dbc8fa 100644 --- a/lib/metadata/raid_manip.c +++ b/lib/metadata/raid_manip.c @@ -502,7 +502,6 @@ static int _raid_add_images(struct logical_volume *lv, uint32_t old_count = lv_raid_image_count(lv); uint32_t count = new_count - old_count; uint64_t status_mask = -1; - struct cmd_context *cmd = lv->vg->cmd; struct lv_segment *seg = first_seg(lv); struct dm_list meta_lvs, data_lvs; struct lv_list *lvl; @@ -679,29 +678,8 @@ to be left for these sub-lvs. dm_list_iterate_items(lvl, &data_lvs) lv_set_hidden(lvl->lv); - if (!vg_write(lv->vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!suspend_lv_origin(cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - lv->vg->name, lv->name); - return 0; - } - - if (!vg_commit(lv->vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!resume_lv_origin(cmd, lv)) { - log_error("Failed to resume %s/%s after committing changes", - lv->vg->name, lv->name); - return 0; - } + if (!lv_update_and_reload_origin(lv)) + return_0; /* * Now that the 'REBUILD' has made its way to the kernel, we must @@ -1223,34 +1201,12 @@ int lv_raid_split_and_track(struct logical_volume *lv, return 0; } - if (!vg_write(lv->vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!suspend_lv(lv->vg->cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - lv->vg->name, lv->name); - return 0; - } - - if (!vg_commit(lv->vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } + if (!lv_update_and_reload(lv)) + return_0; log_print_unless_silent("%s split from %s for read-only purposes.", seg_lv(seg, s)->name, lv->name); - /* Resume original LV */ - if (!resume_lv(lv->vg->cmd, lv)) { - log_error("Failed to resume %s/%s after committing changes", - lv->vg->name, lv->name); - return 0; - } - /* Activate the split (and tracking) LV */ if (!_activate_sublv_preserving_excl(lv, seg_lv(seg, s))) return 0; @@ -1316,29 +1272,8 @@ int lv_raid_merge(struct logical_volume *image_lv) image_lv->status |= (lv->status & LVM_WRITE); image_lv->status |= RAID_IMAGE; - if (!vg_write(vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, vg->name); - return 0; - } - - if (!suspend_lv(vg->cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - vg->name, lv->name); - return 0; - } - - if (!vg_commit(vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, vg->name); - return 0; - } - - if (!resume_lv(vg->cmd, lv)) { - log_error("Failed to resume %s/%s after committing changes", - vg->name, lv->name); - return 0; - } + if (!lv_update_and_reload(lv)) + return_0; log_print_unless_silent("%s/%s successfully merged back into %s/%s", vg->name, image_lv->name, vg->name, lv->name); @@ -1439,29 +1374,8 @@ static int _convert_mirror_to_raid1(struct logical_volume *lv, lv->status |= RAID; seg->status |= RAID; - if (!vg_write(lv->vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!suspend_lv(lv->vg->cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - lv->vg->name, lv->name); - return 0; - } - - if (!vg_commit(lv->vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!resume_lv(lv->vg->cmd, lv)) { - log_error("Failed to resume %s/%s after committing changes", - lv->vg->name, lv->name); - return 0; - } + if (!lv_update_and_reload(lv)) + return_0; return 1; } @@ -1806,29 +1720,8 @@ try_again: } } - if (!vg_write(lv->vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!suspend_lv_origin(lv->vg->cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - lv->vg->name, lv->name); - return 0; - } - - if (!vg_commit(lv->vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!resume_lv_origin(lv->vg->cmd, lv)) { - log_error("Failed to resume %s/%s after committing changes", - lv->vg->name, lv->name); - return 0; - } + if (!lv_update_and_reload_origin(lv)) + return_0; dm_list_iterate_items(lvl, &old_lvs) { if (!deactivate_lv(lv->vg->cmd, lvl->lv)) @@ -1848,29 +1741,8 @@ try_again: } } - if (!vg_write(lv->vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!suspend_lv_origin(lv->vg->cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - lv->vg->name, lv->name); - return 0; - } - - if (!vg_commit(lv->vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!resume_lv_origin(lv->vg->cmd, lv)) { - log_error("Failed to resume %s/%s after committing changes", - lv->vg->name, lv->name); - return 0; - } + if (!lv_update_and_reload_origin(lv)) + return_0; return 1; } @@ -1879,7 +1751,6 @@ int lv_raid_remove_missing(struct logical_volume *lv) { uint32_t s; struct lv_segment *seg = first_seg(lv); - struct cmd_context *cmd = lv->vg->cmd; if (!(lv->status & PARTIAL_LV)) { log_error(INTERNAL_ERROR "%s/%s is not a partial LV", @@ -1915,25 +1786,7 @@ int lv_raid_remove_missing(struct logical_volume *lv) } } - if (!vg_write(lv->vg)) { - log_error("Failed to write changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!suspend_lv(cmd, lv)) { - log_error("Failed to suspend %s/%s before committing changes", - lv->vg->name, lv->name); - return 0; - } - - if (!vg_commit(lv->vg)) { - log_error("Failed to commit changes to %s in %s", - lv->name, lv->vg->name); - return 0; - } - - if (!resume_lv(cmd, lv)) + if (!lv_update_and_reload(lv)) return_0; return 1; diff --git a/tools/lvchange.c b/tools/lvchange.c index 31c397b69..bbb030ff0 100644 --- a/tools/lvchange.c +++ b/tools/lvchange.c @@ -20,7 +20,6 @@ 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); @@ -73,38 +72,15 @@ static int lvchange_permission(struct cmd_context *cmd, lv->name); } - log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name); - if (!vg_write(lv->vg)) + if (!lv_update_and_reload(lv)) return_0; - if (!suspend_lv(cmd, lv)) { - log_error("Failed to lock %s", lv->name); - vg_revert(lv->vg); - goto out; - } - - if (!vg_commit(lv->vg)) { - if (!resume_lv(cmd, lv)) - stack; - 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); - goto out; - } - - r = 1; -out: - backup(lv->vg); - return r; + return 1; } static int lvchange_pool_update(struct cmd_context *cmd, struct logical_volume *lv) { - int r = 0; int update = 0; unsigned val; thin_discards_t discards; @@ -143,32 +119,10 @@ static int lvchange_pool_update(struct cmd_context *cmd, if (!update) return 0; - log_very_verbose("Updating logical volume \"%s\" on disk(s).", lv->name); - if (!vg_write(lv->vg)) + if (!lv_update_and_reload_origin(lv)) return_0; - if (!suspend_lv_origin(cmd, lv)) { - log_error("Failed to update active %s/%s (deactivation is needed).", - lv->vg->name, lv->name); - vg_revert(lv->vg); - goto out; - } - - if (!vg_commit(lv->vg)) { - if (!resume_lv_origin(cmd, lv)) - stack; - goto_out; - } - - if (!resume_lv_origin(cmd, lv)) { - log_error("Problem reactivating %s.", lv->name); - goto out; - } - - r = 1; -out: - backup(lv->vg); - return r; + return 1; } static int lvchange_monitoring(struct cmd_context *cmd, @@ -555,7 +509,6 @@ 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); @@ -590,32 +543,10 @@ static int lvchange_readahead(struct cmd_context *cmd, log_verbose("Setting read ahead to %u for \"%s\"", read_ahead, lv->name); - log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name); - if (!vg_write(lv->vg)) + if (!lv_update_and_reload(lv)) return_0; - if (!suspend_lv(cmd, lv)) { - log_error("Failed to lock %s", lv->name); - vg_revert(lv->vg); - goto out; - } - - if (!vg_commit(lv->vg)) { - if (!resume_lv(cmd, lv)) - stack; - 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); - goto out; - } - - r = 1; -out: - backup(lv->vg); - return r; + return 1; } static int lvchange_persistent(struct cmd_context *cmd, @@ -814,25 +745,8 @@ static int lvchange_writemostly(struct logical_volume *lv) } } - if (!vg_write(lv->vg)) - return_0; - - if (!suspend_lv(cmd, lv)) { - vg_revert(lv->vg); - return_0; - } - - if (!vg_commit(lv->vg)) { - if (!resume_lv(cmd, lv)) - stack; + if (!lv_update_and_reload(lv)) return_0; - } - - log_very_verbose("Updating writemostly for \"%s\" in kernel", lv->name); - if (!resume_lv(cmd, lv)) { - log_error("Problem reactivating %s", lv->name); - return 0; - } return 1; } @@ -862,26 +776,8 @@ static int lvchange_recovery_rate(struct logical_volume *lv) return 0; } - if (!vg_write(lv->vg)) - return_0; - - if (!suspend_lv(cmd, lv)) { - vg_revert(lv->vg); + if (!lv_update_and_reload(lv)) return_0; - } - - if (!vg_commit(lv->vg)) { - if (!resume_lv(cmd, lv)) - stack; - return_0; - } - - log_very_verbose("Updating recovery rate for \"%s\" in kernel", - lv->name); - if (!resume_lv(cmd, lv)) { - log_error("Problem reactivating %s", lv->name); - return 0; - } return 1; }