1 files changed, 318 insertions, 87 deletions

diff --git a/tools/lvconvert.c b/tools/lvconvert.c
index 772c26834..5ca930ccd 100644
--- a/tools/lvconvert.c
+++ b/tools/lvconvert.c
@@ -1364,7 +1364,8 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 				  struct lvconvert_params *lp,
 				  struct dm_list *operable_pvs,
 				  uint32_t new_mimage_count,
-				  uint32_t new_log_count)
+				  uint32_t new_log_count,
+				  struct dm_list *pvh)
 {
 	uint32_t region_size;
 	struct lv_segment *seg = first_seg(lv);
@@ -1384,7 +1385,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 						  vg_is_clustered(lv->vg));
 
 	if (!operable_pvs)
-		operable_pvs = lp->pvh;
+		operable_pvs = pvh;
 
 	/*
 	 * Up-convert from linear to mirror
@@ -1393,7 +1394,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 		/* FIXME Share code with lvcreate */
 
 		/*
-		 * FIXME should we give not only lp->pvh, but also all PVs
+		 * FIXME should we give not only pvh, but also all PVs
 		 * currently taken by the mirror? Would make more sense from
 		 * user perspective.
 		 */
@@ -1402,7 +1403,7 @@ static int _lvconvert_mirrors_aux(struct cmd_context *cmd,
 				    lp->alloc, MIRROR_BY_LV))
 			return_0;
 
-		if (lp->wait_completion)
+		if (!arg_is_set(cmd, background_ARG))
 			lp->need_polling = 1;
 
 		goto out;
@@ -1579,7 +1580,8 @@ int mirror_remove_missing(struct cmd_context *cmd,
  */
 static int _lvconvert_mirrors_repair(struct cmd_context *cmd,
 				     struct logical_volume *lv,
-				     struct lvconvert_params *lp)
+				     struct lvconvert_params *lp,
+				     struct dm_list *pvh)
 {
 	int failed_logs;
 	int failed_mimages;
@@ -1590,7 +1592,6 @@ static int _lvconvert_mirrors_repair(struct cmd_context *cmd,
 	uint32_t original_mimages = lv_mirror_count(lv);
 	uint32_t original_logs = _get_log_count(lv);
 
-	cmd->handles_missing_pvs = 1;
 	cmd->partial_activation = 1;
 	lp->need_polling = 0;
 
@@ -1646,7 +1647,7 @@ static int _lvconvert_mirrors_repair(struct cmd_context *cmd,
 	while (replace_mimages || replace_logs) {
 		log_warn("Trying to up-convert to %d images, %d logs.", lp->mirrors, log_count);
 		if (_lvconvert_mirrors_aux(cmd, lv, lp, NULL,
-					   lp->mirrors, log_count))
+					   lp->mirrors, log_count, pvh))
 			break;
 		if (lp->mirrors > 2)
 			--lp->mirrors;
@@ -1766,11 +1767,8 @@ static int _lvconvert_mirrors(struct cmd_context *cmd,
 	    (old_log_count == new_log_count) && !lp->repair)
 		return 1;
 
-	if (lp->repair)
-		return _lvconvert_mirrors_repair(cmd, lv, lp);
-
 	if (!_lvconvert_mirrors_aux(cmd, lv, lp, NULL,
-				    new_mimage_count, new_log_count))
+				    new_mimage_count, new_log_count, lp->pvh))
 		return 0;
 
 	if (!lp->need_polling)
@@ -1801,33 +1799,6 @@ static int _is_valid_raid_conversion(const struct segment_type *from_segtype,
 	return 1;
 }
 
-static void _lvconvert_raid_repair_ask(struct cmd_context *cmd,
-				       struct lvconvert_params *lp,
-				       int *replace_dev)
-{
-	const char *dev_policy;
-
-	*replace_dev = 1;
-
-	if (arg_is_set(cmd, usepolicies_ARG)) {
-		dev_policy = find_config_tree_str(cmd, activation_raid_fault_policy_CFG, NULL);
-
-		if (!strcmp(dev_policy, "allocate") ||
-		    !strcmp(dev_policy, "replace"))
-			return;
-
-		/* else if (!strcmp(dev_policy, "anything_else")) -- no replace */
-		*replace_dev = 0;
-		return;
-	}
-
-	if (!lp->yes &&
-	    yes_no_prompt("Attempt to replace failed RAID images "
-			  "(requires full device resync)? [y/n]: ") == 'n') {
-		*replace_dev = 0;
-	}
-}
-
 static int _lvconvert_raid(struct logical_volume *lv, struct lvconvert_params *lp)
 {
 	int replace = 0, image_count = 0;
@@ -1963,49 +1934,6 @@ static int _lvconvert_raid(struct logical_volume *lv, struct lvconvert_params *l
 		return 1;
 	}
 
-	if (lp->replace)
-		return lv_raid_replace(lv, lp->replace_pvh, lp->pvh);
-
-	if (lp->repair) {
-		if (!lv_is_active_exclusive_locally(lv_lock_holder(lv))) {
-			log_error("%s must be active %sto perform this operation.",
-				  display_lvname(lv),
-				  vg_is_clustered(lv->vg) ?
-				  "exclusive locally " : "");
-			return 0;
-		}
-
-		if (seg_is_striped(seg)) {
-			log_error("Cannot repair LV %s of type raid0.",
-				  display_lvname(lv));
-			return 0;
-		}
-
-		_lvconvert_raid_repair_ask(cmd, lp, &replace);
-
-		if (replace) {
-			if (!(failed_pvs = _failed_pv_list(lv->vg)))
-				return_0;
-
-			if (!lv_raid_replace(lv, failed_pvs, lp->pvh)) {
-				log_error("Failed to replace faulty devices in %s.",
-					  display_lvname(lv));
-				return 0;
-			}
-
-			log_print_unless_silent("Faulty devices in %s successfully replaced.",
-						display_lvname(lv));
-			return 1;
-		}
-
-		/* "warn" if policy not set to replace */
-		if (arg_is_set(cmd, usepolicies_ARG))
-			log_warn("Use 'lvconvert --repair %s' to replace "
-				 "failed device.", display_lvname(lv));
-		return 1;
-	}
-
-
 try_new_takeover_or_reshape:
 
 	/* FIXME This needs changing globally. */
@@ -2509,7 +2437,7 @@ out:
 
 static int _lvconvert_thin_pool_repair(struct cmd_context *cmd,
 				       struct logical_volume *pool_lv,
-				       struct lvconvert_params *lp)
+				       struct dm_list *pvh, int poolmetadataspare)
 {
 	const char *dmdir = dm_dir();
 	const char *thin_dump =
@@ -2538,7 +2466,7 @@ static int _lvconvert_thin_pool_repair(struct cmd_context *cmd,
 	pmslv = pool_lv->vg->pool_metadata_spare_lv;
 
 	/* Check we have pool metadata spare LV */
-	if (!handle_pool_metadata_spare(pool_lv->vg, 0, lp->pvh, 1))
+	if (!handle_pool_metadata_spare(pool_lv->vg, 0, pvh, 1))
 		return_0;
 
 	if (pmslv != pool_lv->vg->pool_metadata_spare_lv) {
@@ -2663,8 +2591,8 @@ deactivate_pmslv:
 	}
 
 	/* Try to allocate new pool metadata spare LV */
-	if (!handle_pool_metadata_spare(pool_lv->vg, 0, lp->pvh,
-					lp->poolmetadataspare))
+	if (!handle_pool_metadata_spare(pool_lv->vg, 0, pvh,
+					poolmetadataspare))
 		stack;
 
 	if (dm_snprintf(meta_path, sizeof(meta_path), "%s_meta%%d", pool_lv->name) < 0) {
@@ -3543,7 +3471,8 @@ static int _convert_thin_pool_uncache(struct cmd_context *cmd, struct logical_vo
 static int _convert_thin_pool_repair(struct cmd_context *cmd, struct logical_volume *lv,
 				     struct lvconvert_params *lp)
 {
-	return _lvconvert_thin_pool_repair(cmd, lv, lp);
+	/* return _lvconvert_thin_pool_repair(cmd, lv, lp); */
+	return 0;
 }
 
 /*
@@ -3731,7 +3660,7 @@ static int _convert_mirror_repair(struct cmd_context *cmd, struct logical_volume
 	struct dm_list *failed_pvs;
 	int ret;
 
-	ret = _lvconvert_mirrors_repair(cmd, lv, lp);
+	ret = _lvconvert_mirrors_repair(cmd, lv, lp, lp->pvh);
 
 	if (ret && arg_is_set(cmd, usepolicies_ARG)) {
 		if ((failed_pvs = _failed_pv_list(lv->vg)))
@@ -4711,3 +4640,305 @@ out:
 
 	return ret;
 }
+
+
+/*
+ * Below is code that has transitioned to using command defs.
+ * ----------------------------------------------------------
+ *
+ * This code does not use read_params (or any other param reading
+ * functions associated with it), or the lp struct.  Those have
+ * been primary vehicles for entangling all the lvconvert operations,
+ * so avoiding them is important for untangling.  They were also
+ * heavily used for trying to figure out what the lvconvert operation
+ * was meant to be doing, and that is no longer needed since the
+ * command def provides it.
+ *
+ * All input data is already available from cmd->arg_values and
+ * cmd->position_argv (the --option args in the former, the position
+ * args in the later.)  There is no need to copy these values into
+ * another redundant struct of input values which just obfuscates.
+ *
+ * The new lvconvert_result struct, passed via custom_handle, is
+ * used for *returning* data from processing, not for passing data
+ * into processing.
+ */
+
+
+/*
+ * Data/results accumulated during processing.
+ */
+struct lvconvert_result {
+	int need_polling;
+	struct dm_list poll_idls;
+};
+
+static int _lvconvert_repair_pvs_mirror(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle,
+			struct dm_list *use_pvh)
+{
+	struct lvconvert_result *lr = (struct lvconvert_result *) handle->custom_handle;
+	struct lvconvert_params lp = { 0 };
+	struct convert_poll_id_list *idl;
+	struct lvinfo info;
+	int ret;
+
+	/*
+	 * FIXME: temporary use of lp because _lvconvert_mirrors_repair()
+	 * and _aux() still use lp fields everywhere.
+	 * Migrate them away from using lp (for the most part just use
+	 * local variables, and check arg_values directly).
+	 */
+
+	/*
+	 * Fill in any lp fields here that this fn expects to be set before
+	 * it's called.  It's hard to tell by reading old code, but it seems
+	 * that repair takes nothing like stripes/stripsize.
+	 */
+	lp.alloc = (alloc_policy_t) arg_uint_value(cmd, alloc_ARG, ALLOC_INHERIT);
+
+	ret = _lvconvert_mirrors_repair(cmd, lv, &lp, use_pvh);
+
+	if (lp.need_polling) {
+		if (!lv_info(cmd, lp.lv_to_poll, 0, &info, 0, 0) || !info.exists)
+			log_print_unless_silent("Conversion starts after activation.");
+		else {
+			if (!(idl = _convert_poll_id_list_create(cmd, lp.lv_to_poll)))
+				return_ECMD_FAILED;
+			dm_list_add(&lr->poll_idls, &idl->list);
+		}
+		lr->need_polling = 1;
+	}
+
+	return ret;
+}
+
+static void _lvconvert_repair_pvs_raid_ask(struct cmd_context *cmd, int *do_it)
+{
+	const char *dev_policy;
+
+	*do_it = 1;
+
+	if (arg_is_set(cmd, usepolicies_ARG)) {
+		dev_policy = find_config_tree_str(cmd, activation_raid_fault_policy_CFG, NULL);
+
+		if (!strcmp(dev_policy, "allocate") ||
+		    !strcmp(dev_policy, "replace"))
+			return;
+
+		/* else if (!strcmp(dev_policy, "anything_else")) -- no replace */
+		*do_it = 0;
+		return;
+	}
+
+	if (!arg_count(cmd, yes_ARG) &&
+	    yes_no_prompt("Attempt to replace failed RAID images "
+			  "(requires full device resync)? [y/n]: ") == 'n') {
+		*do_it = 0;
+	}
+}
+
+static int _lvconvert_repair_pvs_raid(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle,
+			struct dm_list *use_pvh)
+{
+	struct dm_list *failed_pvs;
+	int do_it;
+
+	if (!lv_is_active_exclusive_locally(lv_lock_holder(lv))) {
+		log_error("%s must be active %sto perform this operation.",
+			  display_lvname(lv),
+			  vg_is_clustered(lv->vg) ?
+			  "exclusive locally " : "");
+		return 0;
+	}
+
+	_lvconvert_repair_pvs_raid_ask(cmd, &do_it);
+
+	if (do_it) {
+		if (!(failed_pvs = _failed_pv_list(lv->vg)))
+			return_0;
+
+		if (!lv_raid_replace(lv, failed_pvs, use_pvh)) {
+			log_error("Failed to replace faulty devices in %s.",
+				  display_lvname(lv));
+			return 0;
+		}
+
+		log_print_unless_silent("Faulty devices in %s successfully replaced.",
+					display_lvname(lv));
+		return 1;
+	}
+
+	/* "warn" if policy not set to replace */
+	if (arg_is_set(cmd, usepolicies_ARG))
+		log_warn("Use 'lvconvert --repair %s' to replace "
+			 "failed device.", display_lvname(lv));
+	return 1;
+}
+
+static int _lvconvert_repair_pvs(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	struct dm_list *failed_pvs;
+	struct dm_list *use_pvh;
+	int ret;
+
+	/* First pos arg is required LV, remaining are optional PVs. */
+	if (cmd->position_argc > 1) {
+		if (!(use_pvh = create_pv_list(cmd->mem, lv->vg, cmd->position_argc, cmd->position_argv, 0)))
+			return_ECMD_FAILED;
+	} else
+		use_pvh = &lv->vg->pvs;
+
+	if (lv_is_raid(lv))
+		ret = _lvconvert_repair_pvs_raid(cmd, lv, handle, use_pvh);
+	else if (lv_is_mirror(lv))
+		ret = _lvconvert_repair_pvs_mirror(cmd, lv, handle, use_pvh);
+	else
+		ret = 0;
+
+	if (ret && arg_is_set(cmd, usepolicies_ARG)) {
+		if ((failed_pvs = _failed_pv_list(lv->vg)))
+			_remove_missing_empty_pv(lv->vg, failed_pvs);
+	}
+
+	return ret ? ECMD_PROCESSED : ECMD_FAILED;
+}
+
+static int _lvconvert_repair_thinpool(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	int poolmetadataspare = arg_int_value(cmd, poolmetadataspare_ARG, DEFAULT_POOL_METADATA_SPARE);
+	struct dm_list *use_pvh;
+
+	/* First pos arg is required LV, remaining are optional PVs. */
+	if (cmd->position_argc > 1) {
+		if (!(use_pvh = create_pv_list(cmd->mem, lv->vg, cmd->position_argc, cmd->position_argv, 0)))
+			return_ECMD_FAILED;
+	} else
+		use_pvh = &lv->vg->pvs;
+
+	return _lvconvert_thin_pool_repair(cmd, lv, use_pvh, poolmetadataspare);
+}
+
+static int _lvconvert_repair_pvs_or_thinpool(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	if (lv_is_thin_pool(lv))
+		return _lvconvert_repair_thinpool(cmd, lv, handle);
+	else if (lv_is_raid(lv) || lv_is_mirror(lv))
+		return _lvconvert_repair_pvs(cmd, lv, handle);
+	else
+		return_ECMD_FAILED;
+}
+
+/*
+ * FIXME: add option --repair-pvs to call _lvconvert_repair_pvs() directly,
+ * and option --repair-thinpool to call _lvconvert_repair_thinpool().
+ */
+int lvconvert_repair_pvs_or_thinpool_fn(struct cmd_context *cmd, int argc, char **argv)
+{
+	struct processing_handle *handle;
+	struct lvconvert_result lr = { 0 };
+	struct convert_poll_id_list *idl;
+	int saved_ignore_suspended_devices;
+	int ret, poll_ret;
+
+	dm_list_init(&lr.poll_idls);
+
+	if (!(handle = init_processing_handle(cmd, NULL))) {
+		log_error("Failed to initialize processing handle.");
+		return ECMD_FAILED;
+	}
+
+	handle->custom_handle = &lr;
+
+	saved_ignore_suspended_devices = ignore_suspended_devices();
+	init_ignore_suspended_devices(1);
+
+	cmd->handles_missing_pvs = 1;
+
+	ret = process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL,
+			      READ_FOR_UPDATE, handle,
+			      &_lvconvert_repair_pvs_or_thinpool);
+
+	init_ignore_suspended_devices(saved_ignore_suspended_devices);
+
+	if (lr.need_polling) {
+		dm_list_iterate_items(idl, &lr.poll_idls)
+			poll_ret = _lvconvert_poll_by_id(cmd, idl->id,
+						arg_is_set(cmd, background_ARG), 0, 0);
+		if (poll_ret > ret)
+			ret = poll_ret;
+	}
+
+	destroy_processing_handle(cmd, handle);
+
+	return ret;
+}
+
+static int _lvconvert_replace_pv(struct cmd_context *cmd, struct logical_volume *lv,
+			struct processing_handle *handle)
+{
+	struct arg_value_group_list *group;
+	struct dm_list *use_pvh;
+	struct dm_list *replace_pvh;
+	char **replace_pvs;
+	const char *tmp_str;
+	int replace_pv_count;
+	int i;
+
+	/* First pos arg is required LV, remaining are optional PVs. */
+	if (cmd->position_argc > 1) {
+		if (!(use_pvh = create_pv_list(cmd->mem, lv->vg, cmd->position_argc, cmd->position_argv, 0)))
+			return_ECMD_FAILED;
+	} else
+		use_pvh = &lv->vg->pvs;
+
+	if (!(replace_pv_count = arg_count(cmd, replace_ARG)))
+		return_ECMD_FAILED;
+
+	if (!(replace_pvs = dm_pool_alloc(cmd->mem, sizeof(char *) * replace_pv_count)))
+		return_ECMD_FAILED;
+
+	i = 0;
+	dm_list_iterate_items(group, &cmd->arg_value_groups) {
+		if (!grouped_arg_is_set(group->arg_values, replace_ARG))
+			continue;
+		if (!(tmp_str = grouped_arg_str_value(group->arg_values, replace_ARG, NULL))) {
+			log_error("Failed to get '--replace' argument");
+			return_ECMD_FAILED;
+		}
+		if (!(replace_pvs[i++] = dm_pool_strdup(cmd->mem, tmp_str)))
+			return_ECMD_FAILED;
+	}
+
+	if (!(replace_pvh = create_pv_list(cmd->mem, lv->vg, replace_pv_count, replace_pvs, 0)))
+		return_ECMD_FAILED;
+
+	return lv_raid_replace(lv, replace_pvh, use_pvh);
+}
+
+int lvconvert_replace_pv_fn(struct cmd_context *cmd, int argc, char **argv)
+{
+	struct processing_handle *handle;
+	struct lvconvert_result lr = { 0 };
+	int ret;
+
+	if (!(handle = init_processing_handle(cmd, NULL))) {
+		log_error("Failed to initialize processing handle.");
+		return ECMD_FAILED;
+	}
+
+	handle->custom_handle = &lr;
+
+	ret = process_each_lv(cmd, 1, cmd->position_argv, NULL, NULL,
+			      READ_FOR_UPDATE, handle,
+			      &_lvconvert_replace_pv);
+
+	destroy_processing_handle(cmd, handle);
+
+	return ret;
+}
+