[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [lvm-devel] [PATCH] Ignore _mlog name restriction for lvconvert repair

On Feb 12, 2010, at 3:30 PM, Jonathan Brassow wrote:

On Feb 11, 2010, at 7:14 PM, Takahiro Yasui wrote:

malahal us ibm com wrote:
Takahiro Yasui [tyasui redhat com] wrote:
Malahal Naineni wrote:
lvconvert --repair is done on _mlog mirrored log logical volumes from
dmeventd if something fails.

diff -r 86200db56a7c -r 471e224a5713 tools/lvconvert.c
--- a/tools/lvconvert.c	Tue Feb 09 17:49:50 2010 -0800
+++ b/tools/lvconvert.c	Wed Feb 10 09:12:11 2010 -0800
@@ -105,8 +105,12 @@ static int _lvconvert_name_params(struct
	if ((ptr = strrchr(lp->lv_name_full, '/')))
		lp->lv_name = ptr + 1;

-	if (!apply_lvname_restrictions(lp->lv_name))
-		return_0;
+	/* _mlog is an internal name, but it could be mirrored, so
+	 * allow repairing it.
+	 */
+ if (!arg_count(cmd, repair_ARG) || !strstr(lp->lv_name, "_mlog"))
+		if (!apply_lvname_restrictions(lp->lv_name))
+			return_0;

	if (*pargc && lp->snapshot) {
		log_error("Too many arguments provided for snapshots");
lvname is better to be checked if a logical volume is not mirrored log but simple logical volume. How about using (lv->status & MIRRORED) for
the check?

I thought about it but such details are not available at that point. All the information available at that point is derived from the command line

Thank you for the explanation. I see we need to move this name check
at the place where lv->status check can be used.

It might make sense to do it in this very spot... If the repair_ARG is present, why should we be checking lvname restrictions anyway? No new LVs are being created.

Something more like:
if (!arg_count(cmd, repair_ARG) && !apply_lvname_restrictions(lp- >lv_name))

Anyone see anything wrong with that?

Taka, I've not been able to reproduce your seg faults with '--alloc anywhere' using two devices. Is there a special restriction you are using?

agk wants to see if we can /not/ use <lv>_mlog as the argument passed in. So, dmeventd would probably strip the '_mlog' portion.(?) There are definitely some items to clean up in this case. I'm going to give it a shot. If it turns out to be really bad, we might go with the above suggestion.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]