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

[lvm-devel] [PATCH] (5/11) vg_read error returns, part 1



Hi,

this patch is the first attempt at allowing vg_read* users to infer more
information about the error, apart from "an error has happened and been
reported to the user". This will be useful whenever the tools need to
differentiate behaviour depending on the kind of error that has happened, as is
the case in a few spots -- those can be found in the later patches, when this
functionality is used. In this particular patch, it is used for
FAILED_NOTFOUND.

Yours,
   Petr.

Tue Sep 30 23:40:34 CEST 2008  me mornfall net
  * Don't forget to pass EXISTENCE_CHECK to vg_read_for_update in vgsplit.
Sat Jul 12 14:33:07 CEST 2008  me mornfall net
  * Formatting.
Sat Jul 12 14:26:06 CEST 2008  me mornfall net
  * Use a special "failed" field in struct volume_group to indicate vg_read* failures. Used by vg_read_for_update now.
Sat Jul 12 13:26:28 CEST 2008  me mornfall net
  * Allow keeping lock even after a failure.
Sat Jul 12 12:02:57 CEST 2008  me mornfall net
  * Convert the existence-checking instances of vg_read_internal to vg_read_for_update.
Thu Oct 30 18:15:10 CET 2008  Petr Rockai <me mornfall net>
  tagged base 8-3
diff -rN -p -u old-lvmlib-b/lib/metadata/metadata.c new-lvmlib-b/lib/metadata/metadata.c
--- old-lvmlib-b/lib/metadata/metadata.c	2008-10-30 18:17:02.505837415 +0100
+++ new-lvmlib-b/lib/metadata/metadata.c	2008-10-30 18:17:02.577837348 +0100
@@ -2371,42 +2371,66 @@ int pv_analyze(struct cmd_context *cmd, 
  * 0 - fail
  * 1 - success
  */
-int vg_check_status(const struct volume_group *vg, uint32_t status)
+static int _vg_check_status(const struct volume_group *vg, uint32_t status)
 {
+	uint32_t ret = 0;
 	if ((status & CLUSTERED) &&
 	    (vg_is_clustered(vg)) && !locking_is_clustered() &&
 	    !lockingfailed()) {
 		log_error("Skipping clustered volume group %s", vg->name);
-		return 0;
+		ret |= FAILED_CLUSTERED;
 	}
 
 	if ((status & EXPORTED_VG) &&
 	    (vg->status & EXPORTED_VG)) {
 		log_error("Volume group %s is exported", vg->name);
-		return 0;
+		ret |= FAILED_EXPORTED;
 	}
 
 	if ((status & LVM_WRITE) &&
 	    !(vg->status & LVM_WRITE)) {
 		log_error("Volume group %s is read-only", vg->name);
-		return 0;
+		ret |= FAILED_READ_ONLY;
 	}
+
 	if ((status & RESIZEABLE_VG) &&
 	    !(vg->status & RESIZEABLE_VG)) {
 		log_error("Volume group %s is not resizeable.", vg->name);
-		return 0;
+		ret |= FAILED_RESIZEABLE;
 	}
 
-	return 1;
+	return ret;
+}
+
+int vg_check_status(const struct volume_group *vg, uint32_t status)
+{
+	return !_vg_check_status(vg, status);
+}
+
+static vg_t *_vg_read_failure(struct cmd_context *cmd, uint32_t flags)
+{
+	struct volume_group *vg;
+	if (!(vg = dm_pool_zalloc(cmd->mem, sizeof(*vg))))
+		return_NULL;
+	vg->failed = flags;
+	return vg;
 }
 
 /*
  * High-level read-for-update function.
+ * On failure:
+ *  - out of memory: NULL is returned
+ *  - other: a volume group with "failed" field set to nonzero is returned
+ *
  * Failures:
- *  - metadata inconsistent and automatic correction failed
- *  - VG is read-only
- *  - VG is EXPORTED, unless flags has ALLOW_EXPORTED
- *  - VG is not RESIZEABLE, unless flags has ALLOW_NONRESIZEABLE
+ *  - metadata inconsistent and automatic correction failed: FAILED_INCONSISTENT
+ *  - VG is read-only: FAILED_READ_ONLY
+ *  - VG is EXPORTED, unless flags has ALLOW_EXPORTED: FAILED_EXPORTED
+ *  - VG is not RESIZEABLE, unless flags has ALLOW_NONRESIZEABLE:
+ *    FAILED_RESIZEABLE
+ *  - locking failed: FAILED_LOCKING
+ *
+ * On failures, all locks are released, unless KEEP_LOCK has been supplied.
  *
  * If EXISTENCE_CHECK is set in flags, if the VG exists, a non-NULL struct
  * volume_group will be returned every time, but if it has INCONSISTENT_VG set,
@@ -2427,12 +2451,15 @@ vg_t *vg_read_for_update(struct cmd_cont
 	if (flags & REQUIRE_RESIZEABLE)
 		status |= RESIZEABLE_VG;
 
-	if (flags & LOCK_NONBLOCK)
+	if (flags & NONBLOCKING_LOCK)
 		lock |= LCK_NONBLOCK;
 
 	if (flags & EXISTENCE_CHECK)
 		misc |= EXISTENCE_CHECK;
 
+	if (flags & KEEP_LOCK)
+		misc |= KEEP_LOCK;
+
 	return vg_lock_and_read(cmd, vg_name, vgid, lock,
 				status, misc);
 }
@@ -2471,7 +2498,7 @@ vg_t *vg_lock_and_read(struct cmd_contex
 
 	if (!lock_vol(cmd, vg_name, lock_flags)) {
 		log_error("Can't get lock for %s", vg_name);
-		return NULL;
+		return _vg_read_failure(cmd, FAILED_LOCKING);
 	}
 
 	if (misc_flags & ORPHAN_LOCK)
@@ -2481,26 +2508,30 @@ vg_t *vg_lock_and_read(struct cmd_contex
 		consistent = 0;
 
 	consistent_in = consistent;
-	/* If consistent == 1, we get NULL if correction fails. */
+
+	/* FIXME those log_error's with vg_name rely on vg_name != NULL. */
+
+	/* If consistent == 1, we get NULL here if correction fails. */
 	if (!(vg = vg_read_internal(cmd, vg_name, vgid, &consistent))) {
-		if (((misc_flags & FAIL_INCONSISTENT) && !consistent)) {
-			if (!(misc_flags & EXISTENCE_CHECK))
-				log_error("Volume group \"%s\" inconsistent.",
-					  vg_name);
+		vg = _vg_read_failure(cmd, 0);
+		if (consistent_in && !consistent) {
+			log_error("Volume group \"%s\" inconsistent.", vg_name);
+			vg->failed |= FAILED_INCONSISTENT;
 			goto_bad;
 		}
-		log_error("Volume group \"%s\" not found", vg_name);
-		vg->status |= INCONSISTENT_VG;
+		if (!(misc_flags & EXISTENCE_CHECK))
+			log_error("Volume group \"%s\" not found", vg_name);
+		vg->failed |= FAILED_NOTFOUND;
 		goto_bad;
 	}
 
-	if (!vg_check_status(vg, status_flags))
-		goto_bad;
+	vg->failed |= _vg_check_status(vg, status_flags);
 
 	return vg;
  bad:
-	unlock_vg(cmd, vg_name);
-	return NULL;
+	if (!(misc_flags & KEEP_LOCK))
+		unlock_vg(cmd, vg_name);
+	return vg;
 }
 
 /*
diff -rN -p -u old-lvmlib-b/lib/metadata/metadata-exported.h new-lvmlib-b/lib/metadata/metadata-exported.h
--- old-lvmlib-b/lib/metadata/metadata-exported.h	2008-10-30 18:17:02.505837415 +0100
+++ new-lvmlib-b/lib/metadata/metadata-exported.h	2008-10-30 18:17:02.573837259 +0100
@@ -82,7 +82,6 @@ struct pv_segment;
 #define LVM_WRITE             	0x00000200U	/* LV VG */
 #define CLUSTERED         	0x00000400U	/* VG */
 //#define SHARED            	0x00000800U	/* VG */
-#define INCONSISTENT_VG         0x00001000U	/* VG */
 
 /* Format features flags */
 #define FMT_SEGMENTS		0x00000001U	/* Arbitrary segment params? */
@@ -100,11 +99,22 @@ struct pv_segment;
 #define CORRECT_INCONSISTENT    0x00000001U /* Correct inconsistent metadata */
 #define FAIL_INCONSISTENT       0x00000002U /* Fail if metadata inconsistent */
 
-/* vg_read_for_update flags */
+/* vg_read and vg_read_for_update flags */
 #define ALLOW_EXPORTED 0x1
 #define REQUIRE_RESIZEABLE 0x2
 #define EXISTENCE_CHECK 0x4
-#define LOCK_NONBLOCK 0x8
+#define NONBLOCKING_LOCK 0x8
+#define KEEP_LOCK 0x10
+
+/* vg's "failed" field */
+#define FAILED_INCONSISTENT 0x1
+#define FAILED_LOCKING 0x2
+#define FAILED_NOTFOUND 0x4
+
+#define FAILED_READ_ONLY 0x10
+#define FAILED_EXPORTED 0x20
+#define FAILED_RESIZEABLE 0x40
+#define FAILED_CLUSTERED 0x80
 
 /* Mirror conversion type flags */
 #define MIRROR_BY_SEG		0x00000001U	/* segment-by-segment mirror */
@@ -204,6 +214,8 @@ struct volume_group {
 	char *system_id;
 
 	uint32_t status;
+	uint32_t failed;
+
 	alloc_policy_t alloc;
 
 	uint32_t extent_size;
diff -rN -p -u old-lvmlib-b/tools/lvconvert.c new-lvmlib-b/tools/lvconvert.c
--- old-lvmlib-b/tools/lvconvert.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/lvconvert.c	2008-10-30 18:17:02.537836664 +0100
@@ -744,7 +744,8 @@ int lvconvert(struct cmd_context * cmd, 
 
 	log_verbose("Checking for existing volume group \"%s\"", lp.vg_name);
 
-	if (!(vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0)))
+	vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0);
+	if (!vg || vg->failed)
 		return ECMD_FAILED;
 
 	if (!(lvl = find_lv_in_vg(vg, lp.lv_name))) {
diff -rN -p -u old-lvmlib-b/tools/lvcreate.c new-lvmlib-b/tools/lvcreate.c
--- old-lvmlib-b/tools/lvcreate.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/lvcreate.c	2008-10-30 18:17:02.541837382 +0100
@@ -896,7 +896,8 @@ int lvcreate(struct cmd_context *cmd, in
 		return EINVALID_CMD_LINE;
 
 	log_verbose("Finding volume group \"%s\"", lp.vg_name);
-	if (!(vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0)))
+	vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0);
+	if (!vg || vg->failed)
 		return ECMD_FAILED;
 
 	if (!_lvcreate(cmd, vg, &lp))
diff -rN -p -u old-lvmlib-b/tools/lvrename.c new-lvmlib-b/tools/lvrename.c
--- old-lvmlib-b/tools/lvrename.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/lvrename.c	2008-10-30 18:17:02.541837382 +0100
@@ -101,7 +101,8 @@ int lvrename(struct cmd_context *cmd, in
 	}
 
 	log_verbose("Checking for existing volume group \"%s\"", vg_name);
-	if (!(vg = vg_read_for_update(cmd, vg_name, NULL, 0)))
+	vg = vg_read_for_update(cmd, vg_name, NULL, 0);
+	if (!vg || vg->failed)
 		return ECMD_FAILED;
 
 	if (!(lvl = find_lv_in_vg(vg, lv_name_old))) {
diff -rN -p -u old-lvmlib-b/tools/lvresize.c new-lvmlib-b/tools/lvresize.c
--- old-lvmlib-b/tools/lvresize.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/lvresize.c	2008-10-30 18:17:02.541837382 +0100
@@ -661,7 +661,8 @@ int lvresize(struct cmd_context *cmd, in
 		return EINVALID_CMD_LINE;
 
 	log_verbose("Finding volume group %s", lp.vg_name);
-	if (!(vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0))) {
+	vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0);
+	if (!vg || vg->failed) {
 		stack;
 		return ECMD_FAILED;
 	}
diff -rN -p -u old-lvmlib-b/tools/polldaemon.c new-lvmlib-b/tools/polldaemon.c
--- old-lvmlib-b/tools/polldaemon.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/polldaemon.c	2008-10-30 18:17:02.537836664 +0100
@@ -147,7 +147,8 @@ static int _wait_for_single_mirror(struc
 		}
 
 		/* Locks the (possibly renamed) VG again */
-		if (!(vg = parms->poll_fns->get_copy_vg(cmd, name))) {
+		vg = parms->poll_fns->get_copy_vg(cmd, name);
+		if (!vg || vg->failed) {
 			log_error("ABORTING: Can't reread VG for %s", name);
 			/* What more could we do here? */
 			return 0;
diff -rN -p -u old-lvmlib-b/tools/pvchange.c new-lvmlib-b/tools/pvchange.c
--- old-lvmlib-b/tools/pvchange.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/pvchange.c	2008-10-30 18:17:02.541837382 +0100
@@ -56,7 +56,8 @@ static int _pvchange_single(struct cmd_c
 
 		log_verbose("Finding volume group %s of physical volume %s",
 			    vg_name, pv_name);
-		if (!(vg = vg_read_for_update(cmd, vg_name, NULL, 0)))
+		vg = vg_read_for_update(cmd, vg_name, NULL, 0);
+		if (!vg || vg->failed)
 			return_0;
 
 		if (!(pvl = find_pv_in_vg(vg, pv_name))) {
diff -rN -p -u old-lvmlib-b/tools/pvmove.c new-lvmlib-b/tools/pvmove.c
--- old-lvmlib-b/tools/pvmove.c	2008-10-30 18:17:02.493835890 +0100
+++ new-lvmlib-b/tools/pvmove.c	2008-10-30 18:17:02.541837382 +0100
@@ -89,14 +89,9 @@ static const char *_extract_lvname(struc
 
 static struct volume_group *_get_vg(struct cmd_context *cmd, const char *vgname)
 {
-	struct volume_group *vg;
-
 	dev_close_all();
 
-	if (!(vg = vg_read_for_update(cmd, vgname, NULL, 0)))
-		 return NULL;
-
-	return vg;
+	return vg_read_for_update(cmd, vgname, NULL, 0);
 }
 
 /* Create list of PVs for allocation of replacement extents */
@@ -380,7 +375,8 @@ static int _set_up_pvmove(struct cmd_con
 	/* Read VG */
 	log_verbose("Finding volume group \"%s\"", pv_vg_name(pv));
 
-	if (!(vg = _get_vg(cmd, pv_vg_name(pv)))) {
+	vg = _get_vg(cmd, pv_vg_name(pv));
+	if (!vg || vg->failed) {
 		stack;
 		return ECMD_FAILED;
 	}
diff -rN -p -u old-lvmlib-b/tools/vgextend.c new-lvmlib-b/tools/vgextend.c
--- old-lvmlib-b/tools/vgextend.c	2008-10-30 18:17:02.505837415 +0100
+++ new-lvmlib-b/tools/vgextend.c	2008-10-30 18:17:02.541837382 +0100
@@ -41,8 +41,9 @@ int vgextend(struct cmd_context *cmd, in
 	}
 
 	log_verbose("Checking for volume group \"%s\"", vg_name);
-	if (!(vg = vg_read_for_update(cmd, vg_name, NULL,
-				      REQUIRE_RESIZEABLE | LOCK_NONBLOCK))) {
+	vg = vg_read_for_update(cmd, vg_name, NULL,
+				REQUIRE_RESIZEABLE | NONBLOCKING_LOCK);
+	if (!vg || vg->failed) {
 		unlock_vg(cmd, VG_ORPHANS);
 		return ECMD_FAILED;
 	 }
diff -rN -p -u old-lvmlib-b/tools/vgmerge.c new-lvmlib-b/tools/vgmerge.c
--- old-lvmlib-b/tools/vgmerge.c	2008-10-30 18:17:02.505837415 +0100
+++ new-lvmlib-b/tools/vgmerge.c	2008-10-30 18:17:02.541837382 +0100
@@ -27,12 +27,14 @@ static int _vgmerge_single(struct cmd_co
 	}
 
 	log_verbose("Checking for volume group \"%s\"", vg_name_to);
-	if (!(vg_to = vg_read_for_update(cmd, vg_name_to, NULL, 0)))
+	vg_to = vg_read_for_update(cmd, vg_name_to, NULL, 0);
+	if (!vg_to || vg_to->failed)
 		 return ECMD_FAILED;
 
 	log_verbose("Checking for volume group \"%s\"", vg_name_from);
-	if (!(vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
-					   LOCK_NONBLOCK))) {
+	vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
+				     NONBLOCKING_LOCK);
+	if (!vg_from || vg_from->failed) {
 		unlock_vg(cmd, vg_name_to);
 		return ECMD_FAILED;
 	}
diff -rN -p -u old-lvmlib-b/tools/vgrename.c new-lvmlib-b/tools/vgrename.c
--- old-lvmlib-b/tools/vgrename.c	2008-10-30 18:17:02.509834362 +0100
+++ new-lvmlib-b/tools/vgrename.c	2008-10-30 18:17:02.541837382 +0100
@@ -86,14 +86,13 @@ static int vg_rename_path(struct cmd_con
 
 	log_verbose("Checking for new volume group \"%s\"", vg_name_new);
 
-	if (!lock_vol(cmd, vg_name_new, LCK_VG_WRITE | LCK_NONBLOCK)) {
-		unlock_vg(cmd, vg_name_old);
-		log_error("Can't get lock for %s", vg_name_new);
-		return 0;
-	}
+	vg_new = vg_read_for_update(cmd, vg_name_new, NULL, KEEP_LOCK |
+				    EXISTENCE_CHECK | NONBLOCKING_LOCK);
+
+	if (!vg_new || (vg_new->failed && !(vg_new->failed & FAILED_NOTFOUND)))
+		goto error;
 
-	consistent = 0;
-	if ((vg_new = vg_read_internal(cmd, vg_name_new, NULL, &consistent))) {
+	if (!(vg_new->failed & FAILED_NOTFOUND)) {
 		log_error("New volume group \"%s\" already exists",
 			  vg_name_new);
 		goto error;
diff -rN -p -u old-lvmlib-b/tools/vgsplit.c new-lvmlib-b/tools/vgsplit.c
--- old-lvmlib-b/tools/vgsplit.c	2008-10-30 18:17:02.509834362 +0100
+++ new-lvmlib-b/tools/vgsplit.c	2008-10-30 18:17:02.541837382 +0100
@@ -320,19 +320,20 @@ int vgsplit(struct cmd_context *cmd, int
 	}
 
 	log_verbose("Checking for volume group \"%s\"", vg_name_from);
-	if (!(vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
-					   REQUIRE_RESIZEABLE)))
-		 return ECMD_FAILED;
+	vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
+				     REQUIRE_RESIZEABLE);
+	if (!vg_from || vg_from->failed)
+		return ECMD_FAILED;
 
 	log_verbose("Checking for new volume group \"%s\"", vg_name_to);
-	if (!lock_vol(cmd, vg_name_to, LCK_VG_WRITE | LCK_NONBLOCK)) {
-		log_error("Can't get lock for %s", vg_name_to);
-		unlock_vg(cmd, vg_name_from);
-		return ECMD_FAILED;
-	}
+	vg_to = vg_read_for_update(cmd, vg_name_to, NULL, REQUIRE_RESIZEABLE |
+				   NONBLOCKING_LOCK | KEEP_LOCK |
+				   EXISTENCE_CHECK);
+
+	if (!vg_to || (vg_to->failed && !(vg_to->failed & FAILED_NOTFOUND)))
+		goto_bad;
 
-	consistent = 0;
-	if ((vg_to = vg_read_internal(cmd, vg_name_to, NULL, &consistent))) {
+	if (!(vg_to->failed & FAILED_NOTFOUND)) {
 		existing_vg = 1;
 		if (new_vg_option_specified(cmd)) {
 			log_error("Volume group \"%s\" exists, but new VG "

-- 
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]