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

Re: [lvm-devel] Re: LVM2/lib/metadata lv_manip.c



Dave,
thanks a lot for rapidly applying fixes for Alasdair's comments.

Alasdair,
as for the comments about extra checks in _rename_sub_lv(),
they were to allow future enhancements like stacked LV, having
LVs with independent names as sub LVs, not only the internal ones.
But, for now, the checks are not necessary unless metadata is broken.
Also we can seek for other solutions like marking independently
named LV as you mentioned.

So, I'm ok with this:
> One simple way out perhaps?
>   Check if it begins with lv_name_old: if so, replace that part of the
>   string with lv_name_new;  if not, log_error and make the entire rename
>   fail.  (Add complete name validation to vg_validate, if you wish.)

A patch is attached.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
Removed extra checks for sub LV renaming.
https://www.redhat.com/archives/lvm-devel/2007-August/msg00020.html

Index: LVM2.work/lib/metadata/lv_manip.c
===================================================================
--- LVM2.work.orig/lib/metadata/lv_manip.c
+++ LVM2.work/lib/metadata/lv_manip.c
@@ -1491,25 +1491,7 @@ static int _rename_single_lv(struct logi
 }
 
 /*
- * Returns a pointer to LV name suffix.
- * Returns NULL if the LV doesn't have suffix.
- */
-static char *_sub_lv_name_suffix(const char *lvname)
-{
-	char *s;
-
-	if ((s = strstr(lvname, "_mimage")))
-		return s;
-
-	if ((s = strstr(lvname, "_mlog")))
-		return s;
-
-	return NULL;
-}
-
-/*
  * Rename sub LV.
- * If a new name for the sub LV cannot be determined, 1 is returned.
  * 'lv_name_old' and 'lv_name_new' are old and new names of the main LV.
  */
 static int _rename_sub_lv(struct cmd_context *cmd,
@@ -1519,14 +1501,16 @@ static int _rename_sub_lv(struct cmd_con
 	char *suffix, *new_name;
 	size_t len;
 
-	/* Rename only if the lv has known suffix */
-	if (!(suffix = _sub_lv_name_suffix(lv->name)))
-		return 1;
-
-	/* Make sure that lv->name is exactly a lv_name_old + suffix */
-	len = suffix - lv->name;
-	if (strlen(lv_name_old) != len || strncmp(lv->name, lv_name_old, len))
-		return 1;
+	/* Make sure that lv->name starts with lv_name_old + '_' */
+	len = strlen(lv_name_old);
+	if (len >= strlen(lv->name) ||
+	    lv->name[len] != '_' ||
+	    strncmp(lv->name, lv_name_old, len)) {
+		log_error("Cannot rename \"%s\" with unknown internal LV "
+			  "\"%s\"", lv_name_old, lv->name);
+		return 0;
+	}
+	suffix = lv->name + len;
 
  	/*
 	 * Compose a new name for sub lv:

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