[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
arguments!

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.

 brassow


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