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

Re: [lvm-devel] Re: [LVM2 PATCH 2/2] lvrename: add mirrored LV support



Hi Jon,

Thanks for testing and reviewing.

Jonathan Brassow wrote:
>> -    if ((lv->status & MIRRORED) ||
>> -        (lv->status & MIRROR_LOG) ||
>> -        (lv->status & MIRROR_IMAGE)) {
>> -        log_error("Mirrored LV, \"%s\" cannot be renamed: %s",
>> -              lv->name, strerror(ENOSYS));
>> -        return 0;
>> -    }
>> -
> 
> I don't think you want to remove this wholesale.  Testing reveals that
> you are allowed to change a sub-lv name by itself, which is not
> something we want, I don't think.  Instead, leave the checks in for
> MIRROR_LOG and MIRROR_IMAGE.

You are right.
I didn't intend to remove the limitation.

I fixed it and modified 2 other parts:
  - Removed "Failed on <LV name>" messages from _for_each_sublv(),
    as they have no useful information.
    Callback function is responsible to put necessary info about
    failure. Added a comment about it.
  - Changed _rename_sub_lv() to return success if it can't
    determine a new name for the sub LV.
    e.g. if "lv1" is a sub LV of "lv0", renaming "lv0" will
         success without changing "lv1".
    This situation won't happen with current LVM2 code.
    But if such LVs exist, I think expected behaviour is leaving
    them unchanged and renaming others as far as possible.

Revised patch is attached.
(and fixed subject to "2/2" as it should have been)

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
Enables mirrored LV renaming.

Mirrored LV contains "sub LV"s.
e.g. "lv0" contains "lv0_mimage_0", .., "lv0_mimage_n" and "lv0_mlog".
"_mimage" and "_mlog" are reserved words, so normal LVs never has
these in their names.

To rename mirrored LV, we have to rename those sub LVs accordingly.

With this patch, rename_lv() will walk through the list of sub LVs.
It checks the validity of sub LV names:
  - is it exactly a main LV name + known suffix?

MIRROR_LOG/MIRROR_IMAGE check is moved to earlier part of the
function as it's stronger and quicker check than others.

Index: LVM2.work/tools/lvrename.c
===================================================================
--- LVM2.work.orig/tools/lvrename.c
+++ LVM2.work/tools/lvrename.c
@@ -16,6 +16,125 @@
 #include "tools.h"
 #include "lvm-types.h"
 
+/*
+ * Minimal LV renaming function.
+ * Metadata transaction should be made by caller.
+ * Assumes newname is allocated from cmd->mem pool.
+ */
+static int _rename_single_lv(struct logical_volume *lv, char *newname)
+{
+	struct volume_group *vg = lv->vg;
+
+	if (find_lv_in_vg(vg, newname)) {
+		log_error("Logical volume \"%s\" already exists in "
+			  "volume group \"%s\"", newname, vg->name);
+		return_0;
+	}
+
+	if (lv->status & LOCKED) {
+		log_error("Cannot rename locked LV %s", lv->name);
+		return_0;
+	}
+
+	lv->name = newname;
+
+	return 1;
+}
+
+/*
+ * Returns a pointer to LV name suffix.
+ * Returns NULL if the LV doesn't have suffix.
+ */
+static char * lvname_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.
+ * Returns 0 on failure, 1 on success.
+ * If a new name for the sub LV cannot be determined, 1 is returned.
+ * 'lv_main_old' and 'lv_main_new' are old and new names of the main LV.
+ */
+static int _rename_sub_lv(struct cmd_context *cmd,
+			  struct logical_volume *lv,
+			  char *lv_main_old, char *lv_main_new)
+{
+	char *suffix, *newname;
+	size_t l;
+
+	/* Rename only if the lv has known suffix */
+	if (!(suffix = lvname_suffix(lv->name)))
+		return 1;
+
+	/* Make sure that lv->name is exactly a lv_main_old + suffix */
+	l = suffix - lv->name;
+	if (strlen(lv_main_old) != l || strncmp(lv->name, lv_main_old, l))
+		return 1;
+
+ 	/*
+	 * Compose a new name for sub lv:
+	 *   e.g. new name is "lvol1_mlog"
+	 *        if the sub LV is "lvol0_mlog" and
+	 *        a new name for main LV is "lvol1"
+	 */
+	l = strlen(lv_main_new) + strlen(suffix) + 1;
+	newname = dm_pool_alloc(cmd->mem, l);
+	if (!newname) {
+		log_error("Failed to allocate space for new name");
+		return_0;
+	}
+	if (!dm_snprintf(newname, l, "%s%s", lv_main_new, suffix)) {
+		log_error("dm_snprintf failed");
+		return_0;
+	}
+
+	/* Rename it */
+	return _rename_single_lv(lv, newname);
+}
+
+/* Callback for _for_each_sub_lv */
+static int _rename_cb(struct cmd_context *cmd, struct logical_volume *lv,
+		      void *data)
+{
+	char **names = (char **) data;
+
+	return _rename_sub_lv(cmd, lv, names[0], names[1]);
+}
+
+/*
+ * Loop down sub LVs and call "func" for each.
+ * "func" is responsible to log necessary information on failure.
+ */
+static int _for_each_sub_lv(struct cmd_context *cmd, struct logical_volume *lv,
+			    int (*func)(struct cmd_context *cmd,
+				        struct logical_volume *lv,
+				        void *data),
+			    void *data)
+{
+	struct lv_segment *seg;
+	int s;
+
+	list_iterate_items(seg, &lv->segments) {
+		if (seg->log_lv && !func(cmd, seg->log_lv, data))
+			return_0;
+		for (s = 0; s < seg->area_count; s++)
+			if (seg_type(seg, s) == AREA_LV &&
+			    !func(cmd, seg_lv(seg, s), data))
+				return_0;
+	}
+
+	return 1;
+}
+
 
 /*
  * Core of LV renaming routine.
@@ -26,6 +145,13 @@ static int rename_lv(struct cmd_context 
 		     char *newname)
 {
 	struct volume_group *vg = lv->vg;
+	char *names[2];
+
+	/* rename is not allowed for hidden LVs */
+	if ((lv->status & MIRROR_LOG) || (lv->status & MIRROR_IMAGE)) {
+		log_error("Cannot rename hidden LV \%s\.", lv->name);
+		return 0;
+	}
 
 	if (find_lv_in_vg(vg, newname)) {
 		log_error("Logical volume \"%s\" already exists in "
@@ -38,17 +164,16 @@ static int rename_lv(struct cmd_context 
 		return 0;
 	}
 
-	if ((lv->status & MIRRORED) ||
-	    (lv->status & MIRROR_LOG) ||
-	    (lv->status & MIRROR_IMAGE)) {
-		log_error("Mirrored LV, \"%s\" cannot be renamed: %s",
-			  lv->name, strerror(ENOSYS));
-		return 0;
-	}
-
 	if (!archive(vg))
 		return_0;
 
+	/* rename sub LVs */
+	names[0] = lv->name;
+	names[1] = newname;
+	if (!_for_each_sub_lv(cmd, lv, _rename_cb, (void *) names))
+		return 0;
+
+	/* rename main LV */
 	if (!(lv->name = dm_pool_strdup(cmd->mem, newname))) {
 		log_error("Failed to allocate space for new name");
 		return 0;

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