mirror: correct locking for mirror log initialization

The code was not acking proper lock holding LVs when trying to
initialize mirror log to predefined values.

WHATS_NEW

@@ -1,5 +1,6 @@
 Version 2.02.178 - 
 =====================================
+  Enhance mirror log initialization for old mirror target.
   Skip private crypto and stratis devices.
   Skip frozen raid devices from scanning.
   Activate RAID SubLVs on read_only_volume_list readwrite

lib/metadata/mirror.c

@@ -317,8 +317,13 @@ static int _init_mirror_log(struct cmd_context *cmd,
 			    struct dm_list *tagsl, int remove_on_failure)
 {
 	struct dm_str_list *sl;
-	uint64_t orig_status = log_lv->status;
+
-	int was_active = 0;
+	if (log_lv != lv_lock_holder(log_lv) || !lv_is_visible(log_lv)) {
+		/* Expect fully visible device for init */
+		log_error(INTERNAL_ERROR "Log LV %s is not top level LV for initialization.",
+			  display_lvname(log_lv));
+		return 0;
+	}
 
 	if (test_mode()) {
 		log_verbose("Test mode: Skipping mirror log initialisation.");
@@ -331,53 +336,22 @@ static int _init_mirror_log(struct cmd_context *cmd,
 		return 0;
 	}
 
-	/* If the LV is active, deactivate it first. */
-	if (lv_is_active(log_lv)) {
-		(void) deactivate_lv(cmd, log_lv);
-		/*
-		 * FIXME: workaround to fail early
-		 * Ensure that log is really deactivated because deactivate_lv
-		 * on cluster do not fail if there is log_lv with different UUID.
-		 */
-		if (lv_is_active(log_lv)) {
-			log_error("Aborting. Unable to deactivate mirror log.");
-			goto revert_new_lv;
-		}
-		was_active = 1;
-	}
-
-	/* Temporary make it visible for set_lv() */
-	lv_set_visible(log_lv);
-
 	/* Temporary tag mirror log for activation */
 	dm_list_iterate_items(sl, tagsl)
 		if (!str_list_add(cmd->mem, &log_lv->tags, sl->str)) {
 			log_error("Aborting. Unable to tag mirror log.");
-			goto activate_lv;
+			return 0;
 		}
 
 	/* store mirror log on disk(s) */
 	if (!vg_write(log_lv->vg) || !vg_commit(log_lv->vg))
-		goto activate_lv;
+		return_0;
 
-	backup(log_lv->vg);
+	if (!activate_lv_excl_local(cmd, log_lv)) {
-
-	/* Wait for events following any deactivation before reactivating */
-	if (!sync_local_dev_names(cmd)) {
-		log_error("Aborting. Failed to sync local devices before initialising mirror log %s.",
-			  display_lvname(log_lv));
-		goto revert_new_lv;
-	}
-
-	if (!activate_lv(cmd, log_lv)) {
 		log_error("Aborting. Failed to activate mirror log.");
 		goto revert_new_lv;
 	}
 
-	/* Remove the temporary tags */
-	dm_list_iterate_items(sl, tagsl)
-		str_list_del(&log_lv->tags, sl->str);
-
 	if (activation()) {
 		if (!wipe_lv(log_lv, (struct wipe_params)
 			     { .do_zero = 1, .zero_sectors = log_lv->size,
@@ -385,23 +359,29 @@ static int _init_mirror_log(struct cmd_context *cmd,
 			log_error("Aborting. Failed to wipe mirror log.");
 			goto deactivate_and_revert_new_lv;
 		}
-	}
 
-	if (activation() && !_write_log_header(cmd, log_lv)) {
+		if (!_write_log_header(cmd, log_lv)) {
-		log_error("Aborting. Failed to write mirror log header.");
+			log_error("Aborting. Failed to write mirror log header.");
-		goto deactivate_and_revert_new_lv;
+			goto deactivate_and_revert_new_lv;
+		}
 	}
 
 	if (!deactivate_lv(cmd, log_lv)) {
 		log_error("Aborting. Failed to deactivate mirror log. "
 			  "Manual intervention required.");
-		return 0;
+		goto revert_new_lv;
 	}
 
-	lv_set_hidden(log_lv);
+	/* Wait for events following any deactivation before reactivating */
+	if (!sync_local_dev_names(cmd)) {
+		log_error("Aborting. Failed to sync local devices before initialising mirror log %s.",
+			  display_lvname(log_lv));
+		goto revert_new_lv;
+	}
 
-	if (was_active && !activate_lv(cmd, log_lv))
+	/* Remove the temporary tags */
-		return_0;
+	dm_list_iterate_items(sl, tagsl)
+		str_list_del(&log_lv->tags, sl->str);
 
 	return 1;
 
@@ -413,8 +393,6 @@ deactivate_and_revert_new_lv:
 	}
 
 revert_new_lv:
-	log_lv->status = orig_status;
-
 	dm_list_iterate_items(sl, tagsl)
 		str_list_del(&log_lv->tags, sl->str);
 
@@ -430,10 +408,6 @@ revert_new_lv:
 	else
 		backup(log_lv->vg);
 
-activate_lv:
-	if (was_active && !remove_on_failure && !activate_lv(cmd, log_lv))
-		return_0;
-
 	return 0;
 }
 
@@ -657,6 +631,7 @@ static int _split_mirror_images(struct logical_volume *lv,
 	struct lv_list *lvl;
 	struct cmd_context *cmd = lv->vg->cmd;
 	char layer_name[NAME_LEN], format[NAME_LEN];
+	int act;
 
 	if (!lv_is_mirrored(lv)) {
 		log_error("Unable to split non-mirrored LV %s.",
@@ -791,30 +766,22 @@ static int _split_mirror_images(struct logical_volume *lv,
 	 * Suspend and resume the mirror - this includes all
 	 * the sub-LVs and soon-to-be-split sub-LVs
 	 */
-	if (!lv_update_and_reload(mirrored_seg->lv))
+	if (!lv_update_and_reload(lv))
 		return_0;
 
-	/*
+	act = lv_is_active(lv_lock_holder(lv));
-	 * Recycle newly split LV so it is properly renamed.
+
-	 *   Cluster requires the extra deactivate/activate calls.
+	if (act && !_activate_lv_like_model(lv, new_lv)) {
-	 */
-	if (vg_is_clustered(lv->vg) &&
-	    (!deactivate_lv(cmd, new_lv) ||
-	     !_activate_lv_like_model(lv, new_lv))) {
-		log_error("Failed to rename newly split LV in the kernel");
-		return 0;
-	}
-	if (!suspend_lv(cmd, new_lv) || !resume_lv(cmd, new_lv)) {
 		log_error("Failed to rename newly split LV in the kernel");
 		return 0;
 	}
 
 	/* Remove original mirror layer if it has been converted to linear */
-	if (sub_lv && !_delete_lv(lv, sub_lv, 1))
+	if (sub_lv && !_delete_lv(lv, sub_lv, act))
 		return_0;
 
 	/* Remove the log if it has been converted to linear */
-	if (detached_log_lv && !_delete_lv(lv, detached_log_lv, 1))
+	if (detached_log_lv && !_delete_lv(lv, detached_log_lv, act))
 		return_0;
 
 	return 1;
@@ -1051,15 +1018,17 @@ static int _remove_mirror_images(struct logical_volume *lv,
 
 	/* Mirror with only 1 area is 'in sync'. */
 	if (new_area_count == 1 && is_temporary_mirror_layer(lv)) {
-		if (first_seg(lv)->log_lv &&
+		detached_log_lv = detach_mirror_log(mirrored_seg);
-		    !_init_mirror_log(lv->vg->cmd, first_seg(lv)->log_lv,
+		if (!_init_mirror_log(lv->vg->cmd, detached_log_lv,
 				      1, &lv->tags, 0)) {
 			/* As a result, unnecessary sync may run after
 			 * collapsing. But safe.*/
 			log_error("Failed to initialize log device %s.",
-				  display_lvname(first_seg(lv)->log_lv));
+				  display_lvname(detached_log_lv));
 			return 0;
 		}
+		if (!attach_mirror_log(mirrored_seg, detached_log_lv))
+			return_0;
 	}
 
 	if (removed)