From: Zdenek Kabelac <zkabelac@redhat.com>
Date: Wed, 14 Dec 2016 10:16:47 +0000 (+0100)
Subject: raid: activation with list
X-Git-Tag: v2_02_169~562
X-Git-Url: https://sourceware.org/git/?a=commitdiff_plain;h=77e09c3fb4d019ab3a0472059481ef990c64b415;p=lvm2.git

raid: activation with list

Commit 069039204002e5c8514050fe541bbd378c383a02 revealed a problem
in raid metadata manipulation.

We do two operations in one table reload:
- raid leg/image extraction
- rename remaining raid legs

This should be made in separate steps. Otherwise we do an
uncorrectable table change on error path (leaving tables
for admin and dmsetup).

As a hotfix - restore the previous logic and use a single
new function _lv_update_and_reload_list which activates exclusively
extracted LVs on the list before resuming suspended raid LV.
This restore 'rename' functionality upon resume.

Also still preserve the 'origin_only' logic - although we know
it's not working properly for cluster and LV stacking.

Further fixes are needed.
---

diff --git a/lib/metadata/raid_manip.c b/lib/metadata/raid_manip.c
index 049102ab2..ed4823991 100644
--- a/lib/metadata/raid_manip.c
+++ b/lib/metadata/raid_manip.c
@@ -233,11 +233,6 @@ static int _deactivate_and_remove_lvs(struct volume_group *vg, struct dm_list *r
 {
 	struct lv_list *lvl;
 
-	/* Need to take lock/resume for proper deactivation */
-	dm_list_iterate_items(lvl, removal_lvs)
-		if (!activate_lv_excl_local(vg->cmd, lvl->lv))
-			return_0;
-
 	dm_list_iterate_items(lvl, removal_lvs) {
 		if (!deactivate_lv(vg->cmd, lvl->lv))
 			return_0;
@@ -363,6 +358,62 @@ static int _raid_remove_top_layer(struct logical_volume *lv,
 	return 1;
 }
 
+/*
+ * Assisted excl_local activation of lvl listed LVs before resume
+ *
+ * FIXME: code which needs to use this function is usually unsafe
+ *	  againt crashes as it's doing more then 1 operation per commit
+ *	  and as such is currently irreversible on error path.
+ *
+ * Function is not making backup as this is usually not the last
+ * metadata changing operation.
+ *
+ * Also we should take 'struct lv_list'...
+ */
+static int _lv_update_and_reload_list(struct logical_volume *lv, int origin_only, struct dm_list *lv_list)
+{
+	struct volume_group *vg = lv->vg;
+	const struct logical_volume *lock_lv = lv_lock_holder(lv);
+	struct lv_list *lvl;
+	int r;
+
+	log_very_verbose("Updating logical volume %s on disk(s)%s.",
+			 display_lvname(lock_lv), origin_only ? " (origin only)": "");
+
+	if (!vg_write(vg))
+		return_0;
+
+	if (!(r = (origin_only ? suspend_lv_origin(vg->cmd, lock_lv) : suspend_lv(vg->cmd, lock_lv)))) {
+		log_error("Failed to lock logical volume %s.",
+			  display_lvname(lock_lv));
+		vg_revert(vg);
+	} else if (!(r = vg_commit(vg)))
+		stack; /* !vg_commit() has implict vg_revert() */
+
+	if (r && lv_list) {
+		dm_list_iterate_items(lvl, lv_list) {
+			log_very_verbose("Activating logical volume %s before %s in kernel.",
+					 display_lvname(lvl->lv), display_lvname(lock_lv));
+			if (!activate_lv_excl_local(vg->cmd, lvl->lv)) {
+				log_error("Failed to activate %s before resuming %s.",
+					  display_lvname(lvl->lv), display_lvname(lock_lv));
+				r = 0; /* But lets try with the rest */
+			}
+		}
+	}
+
+	log_very_verbose("Updating logical volume %s in kernel.",
+			 display_lvname(lock_lv));
+
+	if (!(origin_only ? resume_lv_origin(vg->cmd, lock_lv) : resume_lv(vg->cmd, lock_lv))) {
+		log_error("Problem reactivating logical volume %s.",
+			  display_lvname(lock_lv));
+		r = 0;
+	}
+
+	return r;
+}
+
 /* Makes on-disk metadata changes
  * If LV is active:
  *	clear first block of device
@@ -1233,7 +1284,7 @@ static int _raid_remove_images(struct logical_volume *lv,
 	if (!commit)
 		return 1;
 
-	if (!lv_update_and_reload(lv))
+	if (!_lv_update_and_reload_list(lv, 0, removal_lvs))
 		return_0;
 
 	/*
@@ -1927,7 +1978,7 @@ static int _raid0_add_or_remove_metadata_lvs(struct logical_volume *lv,
 		return_0;
 
 	if (update_and_reload) {
-		if (!lv_update_and_reload_origin(lv))
+		if (!_lv_update_and_reload_list(lv, 1, removal_lvs))
 			return_0;
 
 		/* If any residual LVs, eliminate them, write VG, commit it and take a backup */
@@ -1977,7 +2028,7 @@ static int _lv_update_reload_fns_reset_eliminate_lvs(struct logical_volume *lv,
 {
 	int flags_were_cleared = 0, r = 0;
 
-	if (!lv_update_and_reload_origin(lv))
+	if (!_lv_update_and_reload_list(lv, 1, removal_lvs))
 		return_0;
 
 	/* Eliminate any residual LV and don't commit the metadata */