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

[lvm-devel] [PATCH] "lenient" (backward-compatible) flag field



Hi.

For description see below. The patch stood fairly well in my limited testing. I
have quickly ported the partial VG patches to use "flags" instead of "status"
for its new flags (MISSING_PV, specifically) and everything seemed to work
fine. I have also verified that metadata looks all right.

This should also facilitate a relatively easy fix for #454349, without
compromising backward compatibility.

Please review, thanks.

Wed Jul  9 16:22:39 CEST 2008  me mornfall net
  * Implement lenient flag mechanism.
  
  There are now two fields in metadata for flags: status and flags. The "status"
  field is treated as it ever has been, unknown flags there are treated as fatal
  metadata errors. However, in the "flags" field, any unknown flags will be
  ignored and silently dropped. This improves backward-compatibility
  possibilities. (Any versions without support for this new "flag" field will
  drop the field altogether, which is same as ignoring all the flags there.)
diff -rN -u old-lenient-flags/lib/format_text/export.c new-lenient-flags/lib/format_text/export.c
--- old-lenient-flags/lib/format_text/export.c	2008-07-09 16:58:14.075344316 +0200
+++ new-lenient-flags/lib/format_text/export.c	2008-07-09 16:58:14.131345337 +0200
@@ -321,6 +321,20 @@
 	return 1;
 }
 
+static int _print_flags(struct formatter *f, int status, int type)
+{
+	char buffer[4096];
+	if (!print_flags(status, type | STATUS_FLAGS, buffer, sizeof(buffer)))
+		return_0;
+	outf(f, "status = %s", buffer);
+
+	if (!print_flags(status, type, buffer, sizeof(buffer)))
+		return_0;
+	outf(f, "flags = %s", buffer);
+
+	return 1;
+}
+
 static int _print_vg(struct formatter *f, struct volume_group *vg)
 {
 	char buffer[4096];
@@ -332,9 +346,8 @@
 
 	outf(f, "seqno = %u", vg->seqno);
 
-	if (!print_flags(vg->status, VG_FLAGS, buffer, sizeof(buffer)))
+	if (!_print_flags(f, vg->status, VG_FLAGS))
 		return_0;
-	outf(f, "status = %s", buffer);
 
 	if (!list_empty(&vg->tags)) {
 		if (!print_tags(&vg->tags, buffer, sizeof(buffer)))
@@ -408,9 +421,8 @@
 			return_0;
 		outnl(f);
 
-		if (!print_flags(pv->status, PV_FLAGS, buffer, sizeof(buffer)))
+		if (!_print_flags(f, pv->status, PV_FLAGS))
 			return_0;
-		outf(f, "status = %s", buffer);
 
 		if (!list_empty(&pv->tags)) {
 			if (!print_tags(&pv->tags, buffer, sizeof(buffer)))
@@ -520,9 +532,8 @@
 
 	outf(f, "id = \"%s\"", buffer);
 
-	if (!print_flags(lv->status, LV_FLAGS, buffer, sizeof(buffer)))
+	if (!_print_flags(f, lv->status, LV_FLAGS))
 		return_0;
-	outf(f, "status = %s", buffer);
 
 	if (!list_empty(&lv->tags)) {
 		if (!print_tags(&lv->tags, buffer, sizeof(buffer)))
diff -rN -u old-lenient-flags/lib/format_text/flags.c new-lenient-flags/lib/format_text/flags.c
--- old-lenient-flags/lib/format_text/flags.c	2008-07-09 16:58:14.075344316 +0200
+++ new-lenient-flags/lib/format_text/flags.c	2008-07-09 16:58:14.131345337 +0200
@@ -25,48 +25,49 @@
 struct flag {
 	const int mask;
 	const char *description;
+	int status;
 };
 
 static struct flag _vg_flags[] = {
-	{EXPORTED_VG, "EXPORTED"},
-	{RESIZEABLE_VG, "RESIZEABLE"},
-	{PARTIAL_VG, "PARTIAL"},
-	{PVMOVE, "PVMOVE"},
-	{LVM_READ, "READ"},
-	{LVM_WRITE, "WRITE"},
-	{CLUSTERED, "CLUSTERED"},
-	{SHARED, "SHARED"},
-	{PRECOMMITTED, NULL},
-	{0, NULL}
+	{EXPORTED_VG, "EXPORTED", 1},
+	{RESIZEABLE_VG, "RESIZEABLE", 1},
+	{PARTIAL_VG, "PARTIAL", 1},
+	{PVMOVE, "PVMOVE", 1},
+	{LVM_READ, "READ", 1},
+	{LVM_WRITE, "WRITE", 1},
+	{CLUSTERED, "CLUSTERED", 1},
+	{SHARED, "SHARED", 1},
+	{PRECOMMITTED, NULL, 1},
+	{0, NULL, 0}
 };
 
 static struct flag _pv_flags[] = {
-	{ALLOCATABLE_PV, "ALLOCATABLE"},
-	{EXPORTED_VG, "EXPORTED"},
-	{0, NULL}
+	{ALLOCATABLE_PV, "ALLOCATABLE", 1},
+	{EXPORTED_VG, "EXPORTED", 1},
+	{0, NULL, 0}
 };
 
 static struct flag _lv_flags[] = {
-	{LVM_READ, "READ"},
-	{LVM_WRITE, "WRITE"},
-	{FIXED_MINOR, "FIXED_MINOR"},
-	{VISIBLE_LV, "VISIBLE"},
-	{PVMOVE, "PVMOVE"},
-	{LOCKED, "LOCKED"},
-	{MIRROR_NOTSYNCED, "NOTSYNCED"},
-	{MIRROR_IMAGE, NULL},
-	{MIRROR_LOG, NULL},
-	{MIRRORED, NULL},
-	{VIRTUAL, NULL},
-	{SNAPSHOT, NULL},
-	{ACTIVATE_EXCL, NULL},
-	{CONVERTING, NULL},
-	{0, NULL}
+	{LVM_READ, "READ", 1},
+	{LVM_WRITE, "WRITE", 1},
+	{FIXED_MINOR, "FIXED_MINOR", 1},
+	{VISIBLE_LV, "VISIBLE", 1},
+	{PVMOVE, "PVMOVE", 1},
+	{LOCKED, "LOCKED", 1},
+	{MIRROR_NOTSYNCED, "NOTSYNCED", 1},
+	{MIRROR_IMAGE, NULL, 1},
+	{MIRROR_LOG, NULL, 1},
+	{MIRRORED, NULL, 1},
+	{VIRTUAL, NULL, 1},
+	{SNAPSHOT, NULL, 1},
+	{ACTIVATE_EXCL, NULL, 1},
+	{CONVERTING, NULL, 1},
+	{0, NULL, 0}
 };
 
 static struct flag *_get_flags(int type)
 {
-	switch (type) {
+	switch (type & ~STATUS_FLAGS) {
 	case VG_FLAGS:
 		return _vg_flags;
 
@@ -101,6 +102,11 @@
 		if (status & flags[f].mask) {
 			status &= ~flags[f].mask;
 
+			if (!(type & STATUS_FLAGS) && flags[f].status)
+				continue;
+			if ((type & STATUS_FLAGS) && !flags[f].status)
+				continue;
+
 			/* Internal-only flag? */
 			if (!flags[f].description)
 				continue;
@@ -151,7 +157,7 @@
 				break;
 			}
 
-		if (!flags[f].description) {
+		if (!flags[f].description && (type & STATUS_FLAGS)) {
 			log_err("Unknown status flag '%s'.", cv->v.str);
 			return 0;
 		}
@@ -160,6 +166,6 @@
 	}
 
       out:
-	*status = s;
+	*status |= s;
 	return 1;
 }
diff -rN -u old-lenient-flags/lib/format_text/import-export.h new-lenient-flags/lib/format_text/import-export.h
--- old-lenient-flags/lib/format_text/import-export.h	2008-07-09 16:58:14.075344316 +0200
+++ new-lenient-flags/lib/format_text/import-export.h	2008-07-09 16:58:14.135346213 +0200
@@ -38,7 +38,8 @@
 enum {
 	VG_FLAGS,
 	PV_FLAGS,
-	LV_FLAGS
+	LV_FLAGS,
+	STATUS_FLAGS = 0x8
 };
 
 struct text_vg_version_ops {
diff -rN -u old-lenient-flags/lib/format_text/import_vsn1.c new-lenient-flags/lib/format_text/import_vsn1.c
--- old-lenient-flags/lib/format_text/import_vsn1.c	2008-07-09 16:58:14.075344316 +0200
+++ new-lenient-flags/lib/format_text/import_vsn1.c	2008-07-09 16:58:14.131345337 +0200
@@ -125,6 +125,31 @@
 	return 1;
 }
 
+static int _read_flag_config(struct config_node *n, int *status, int type)
+{
+	struct config_node *cn;
+	*status = 0;
+
+	if (!(cn = find_config_node(n, "status"))) {
+		log_error("Could not find status flags.");
+		return 0;
+	}
+
+	if (!(read_flags(status, type | STATUS_FLAGS, cn->v))) {
+		log_error("Could not read status flags.");
+		return 0;
+	}
+
+	if (cn = find_config_node(n, "flags")) {
+		if (!(read_flags(status, type, cn->v))) {
+			log_error("Could not read flags.");
+			return 0;
+		}
+	}
+
+	return 1;
+}
+
 static int _read_pv(struct format_instance *fid, struct dm_pool *mem,
 		    struct volume_group *vg, struct config_node *pvn,
 		    struct config_node *vgn __attribute((unused)),
@@ -181,12 +206,7 @@
 
 	memcpy(&pv->vgid, &vg->id, sizeof(vg->id));
 
-	if (!(cn = find_config_node(pvn, "status"))) {
-		log_error("Couldn't find status flags for physical volume.");
-		return 0;
-	}
-
-	if (!(read_flags(&pv->status, PV_FLAGS, cn->v))) {
+	if (!_read_flag_config(pvn, &pv->status, PV_FLAGS)) {
 		log_error("Couldn't read status flags for physical volume.");
 		return 0;
 	}
@@ -493,13 +513,9 @@
 		return 0;
 	}
 
-	if (!(cn = find_config_node(lvn, "status"))) {
-		log_error("Couldn't find status flags for logical volume.");
-		return 0;
-	}
-
-	if (!(read_flags(&lv->status, LV_FLAGS, cn->v))) {
-		log_error("Couldn't read status flags for logical volume.");
+	if (!_read_flag_config(lvn, &lv->status, LV_FLAGS)) {
+		log_error("Couldn't read status flags for logical volume %s.",
+			  lv->name);
 		return 0;
 	}
 
@@ -692,14 +708,8 @@
 		goto bad;
 	}
 
-	if (!(cn = find_config_node(vgn, "status"))) {
-		log_error("Couldn't find status flags for volume group %s.",
-			  vg->name);
-		goto bad;
-	}
-
-	if (!(read_flags(&vg->status, VG_FLAGS, cn->v))) {
-		log_error("Couldn't read status flags for volume group %s.",
+	if (!_read_flag_config(vgn, &vg->status, VG_FLAGS)) {
+		log_error("Error reading flags of volume group %s.",
 			  vg->name);
 		goto bad;
 	}
@@ -855,18 +865,12 @@
 		return 0;
 	}
 
-	if (!(cn = find_config_node(vgn, "status"))) {
+	if (!_read_flag_config(vgn, vgstatus, VG_FLAGS)) {
 		log_error("Couldn't find status flags for volume group %s.",
 			  vgname);
 		return 0;
 	}
 
-	if (!(read_flags(vgstatus, VG_FLAGS, cn->v))) {
-		log_error("Couldn't read status flags for volume group %s.",
-			  vgname);
-		return 0;
-	}
-
 	return vgname;
 }
 

Yours,
   Peter.

-- 
Peter Rockai | me()mornfall!net | prockai()redhat!com
 http://blog.mornfall.net | http://web.mornfall.net

"In My Egotistical Opinion, most people's C programs should be
 indented six feet downward and covered with dirt."
     -- Blair P. Houghton on the subject of C program indentation

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