[PATCH] Use (new) vg_read(_for_update) in client code.

[PATCH] Use (new) vg_read(_for_update) in client code.

[Petr Rockai]

Hi,

this patchset is rebased and updated version of the previous rebase of the
vg_read series. Most of the patches had lots of conflicts, mostly with Milan's
per-VG pool work. Some review wrt. pool behaviour is needed, since we may have
introduced some leaks again. The tests are passing though, and I have tried to
make sure that I am not breaking anything.

Anyway, I'd welcome a timely review, as the rebases are quite tedious (also not
made easier by the fact that part of the earlier patches changed semantics
between review and corresponding CVS checkin).

There are still further patches blocking on this, but I'll only rebase them
when this batch (which is relatively independent of the rest) is checked in.

Yours,
   Petr.

PS: I have upgraded git-core and git-email since the last batch, and it is
printing lots of perl errors in the terminal now... hopefully, it won't mangle
the e-mails too badly.

[PATCH 1/9] Convert the straight instances of vg_lock_and_read to new vg_read(_for_update).

[Petr Rockai]

Sun May  3 11:40:51 CEST 2009  Petr Rockai <me@mornfall.net>
  * Convert the straight instances of vg_lock_and_read to new vg_read(_for_update).
diff -rN -u -p old-temp.31080/tools/lvconvert.c new-temp.31080/tools/lvconvert.c
--- old-temp.31080/tools/lvconvert.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/lvconvert.c	2009-05-03 13:29:25.046236063 +0200
@@ -240,10 +240,8 @@ static struct volume_group *_get_lvconve
 {
 	dev_close_all();
 
-        return vg_lock_and_read(cmd, extract_vgname(cmd, lv_name),
-				NULL, LCK_VG_WRITE,
- 				CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				CORRECT_INCONSISTENT | FAIL_INCONSISTENT);
+        return vg_read_for_update(cmd, extract_vgname(cmd, lv_name),
+				  NULL, 0);
 }
 
 static struct logical_volume *_get_lvconvert_lv(struct cmd_context *cmd __attribute((unused)),
@@ -852,9 +850,8 @@ int lvconvert(struct cmd_context * cmd, 
 
 	log_verbose("Checking for existing volume group \"%s\"", lp.vg_name);
 
-	if (!(vg = vg_lock_and_read(cmd, lp.vg_name, NULL, LCK_VG_WRITE,
-				    CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				    CORRECT_INCONSISTENT)))
+	vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
 
 	if (!(lvl = find_lv_in_vg(vg, lp.lv_name))) {
diff -rN -u -p old-temp.31080/tools/lvcreate.c new-temp.31080/tools/lvcreate.c
--- old-temp.31080/tools/lvcreate.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/lvcreate.c	2009-05-03 13:29:25.046236063 +0200
@@ -984,9 +984,8 @@ int lvcreate(struct cmd_context *cmd, in
 		return EINVALID_CMD_LINE;
 
 	log_verbose("Finding volume group \"%s\"", lp.vg_name);
-	if (!(vg = vg_lock_and_read(cmd, lp.vg_name, NULL, LCK_VG_WRITE,
-				    CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				    CORRECT_INCONSISTENT)))
+	vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
 
 	if (!_lvcreate(cmd, vg, &lp))
diff -rN -u -p old-temp.31080/tools/lvrename.c new-temp.31080/tools/lvrename.c
--- old-temp.31080/tools/lvrename.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/lvrename.c	2009-05-03 13:29:25.050236572 +0200
@@ -102,9 +102,8 @@ int lvrename(struct cmd_context *cmd, in
 	}
 
 	log_verbose("Checking for existing volume group \"%s\"", vg_name);
-	if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE,
-				    CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				    CORRECT_INCONSISTENT)))
+	vg = vg_read_for_update(cmd, vg_name, NULL, 0);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
 
 	if (!(lvl = find_lv_in_vg(vg, lv_name_old))) {
diff -rN -u -p old-temp.31080/tools/lvresize.c new-temp.31080/tools/lvresize.c
--- old-temp.31080/tools/lvresize.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/lvresize.c	2009-05-03 13:29:25.050236572 +0200
@@ -677,9 +677,8 @@ int lvresize(struct cmd_context *cmd, in
 		return EINVALID_CMD_LINE;
 
 	log_verbose("Finding volume group %s", lp.vg_name);
-	if (!(vg = vg_lock_and_read(cmd, lp.vg_name, NULL, LCK_VG_WRITE,
-				    CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				    CORRECT_INCONSISTENT))) {
+	vg = vg_read_for_update(cmd, lp.vg_name, NULL, 0);
+	if (vg_read_error(vg)) {
 		stack;
 		return ECMD_FAILED;
 	}
diff -rN -u -p old-temp.31080/tools/polldaemon.c new-temp.31080/tools/polldaemon.c
--- old-temp.31080/tools/polldaemon.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/polldaemon.c	2009-05-03 13:29:25.050236572 +0200
@@ -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_read_error(vg)) {
 			log_error("ABORTING: Can't reread VG for %s", name);
 			/* What more could we do here? */
 			return 0;
diff -rN -u -p old-temp.31080/tools/pvchange.c new-temp.31080/tools/pvchange.c
--- old-temp.31080/tools/pvchange.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/pvchange.c	2009-05-03 13:29:25.050236572 +0200
@@ -57,9 +57,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_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE,
-					    CLUSTERED | EXPORTED_VG | LVM_WRITE,
-					    CORRECT_INCONSISTENT)))
+		vg = vg_read_for_update(cmd, vg_name, NULL, 0);
+		if (vg_read_error(vg))
 			return_0;
 
 		if (!(pvl = find_pv_in_vg(vg, pv_name))) {
diff -rN -u -p old-temp.31080/tools/pvdisplay.c new-temp.31080/tools/pvdisplay.c
--- old-temp.31080/tools/pvdisplay.c	2009-05-03 13:29:24.954235823 +0200
+++ new-temp.31080/tools/pvdisplay.c	2009-05-03 13:29:25.050236572 +0200
@@ -29,8 +29,8 @@ static int _pvdisplay_single(struct cmd_
 
 	if (!is_orphan(pv) && !vg) {
 		vg_name = pv_vg_name(pv);
-		if (!(vg = vg_lock_and_read(cmd, vg_name, (char *)&pv->vgid,
-					    LCK_VG_READ, CLUSTERED, 0))) {
+		vg = vg_read(cmd, vg_name, (char *)&pv->vgid, 0);
+		if (vg_read_error(vg)) {
 		 	log_error("Skipping volume group %s", vg_name);
 			/* FIXME If CLUSTERED should return ECMD_PROCESSED here */
 		 	return ECMD_FAILED;
diff -rN -u -p old-temp.31080/tools/pvmove.c new-temp.31080/tools/pvmove.c
--- old-temp.31080/tools/pvmove.c	2009-05-03 13:29:24.950234547 +0200
+++ new-temp.31080/tools/pvmove.c	2009-05-03 13:29:25.050236572 +0200
@@ -89,16 +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_lock_and_read(cmd, vgname, NULL, LCK_VG_WRITE,
-				    CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				    CORRECT_INCONSISTENT | FAIL_INCONSISTENT)))
-		 return NULL;
-
-	return vg;
+	return vg_read_for_update(cmd, vgname, NULL, 0);
 }
 
 /* Create list of PVs for allocation of replacement extents */
@@ -384,7 +377,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_read_error(vg)) {
 		stack;
 		return ECMD_FAILED;
 	}
diff -rN -u -p old-temp.31080/tools/pvresize.c new-temp.31080/tools/pvresize.c
--- old-temp.31080/tools/pvresize.c	2009-05-03 13:29:24.950234547 +0200
+++ new-temp.31080/tools/pvresize.c	2009-05-03 13:29:25.050236572 +0200
@@ -29,7 +29,6 @@ static int _pv_resize_single(struct cmd_
 			     const uint64_t new_size)
 {
 	struct pv_list *pvl;
-	int consistent = 1;
 	uint64_t size = 0;
 	uint32_t new_pe_count = 0;
 	int r = 0;
@@ -59,19 +58,9 @@ static int _pv_resize_single(struct cmd_
 	} else {
 		vg_name = pv_vg_name(pv);
 
-		if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) {
-			log_error("Can't get lock for %s", pv_vg_name(pv));
-			return 0;
-		}
-
-		if (!(vg = vg_read_internal(cmd, vg_name, NULL, &consistent))) {
-			unlock_vg(cmd, vg_name);
-			log_error("Unable to find volume group of \"%s\"",
-				  pv_name);
-			return 0;
-		}
+ 		vg = vg_read_for_update(cmd, vg_name, NULL, 0);
 
-		if (!vg_check_status(vg, CLUSTERED | EXPORTED_VG | LVM_WRITE))
+		if (vg_read_error(vg))
 			goto bad;
 
 		if (!(pvl = find_pv_in_vg(vg, pv_name))) {
diff -rN -u -p old-temp.31080/tools/reporter.c new-temp.31080/tools/reporter.c
--- old-temp.31080/tools/reporter.c	2009-05-03 13:29:24.950234547 +0200
+++ new-temp.31080/tools/reporter.c	2009-05-03 13:29:25.050236572 +0200
@@ -137,8 +137,8 @@ static int _pvs_single(struct cmd_contex
 	if (is_pv(pv) && !is_orphan(pv) && !vg) {
 		vg_name = pv_vg_name(pv);
 
-		if (!(vg = vg_lock_and_read(cmd, vg_name, (char *)&pv->vgid,
-					    LCK_VG_READ, CLUSTERED, 0))) {
+		vg = vg_read(cmd, vg_name, (char *)&pv->vgid, 0);
+		if (vg_read_error(vg)) {
 			log_error("Skipping volume group %s", vg_name);
 			return ECMD_FAILED;
 		}
diff -rN -u -p old-temp.31080/tools/vgextend.c new-temp.31080/tools/vgextend.c
--- old-temp.31080/tools/vgextend.c	2009-05-03 13:29:24.946235086 +0200
+++ new-temp.31080/tools/vgextend.c	2009-05-03 13:29:25.054237779 +0200
@@ -42,13 +42,12 @@ int vgextend(struct cmd_context *cmd, in
 	}
 
 	log_verbose("Checking for volume group \"%s\"", vg_name);
-	if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_WRITE | LCK_NONBLOCK,
-				    CLUSTERED | EXPORTED_VG |
-				    LVM_WRITE | RESIZEABLE_VG,
-				    CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) {
-		unlock_vg(cmd, VG_ORPHANS);
+ 	vg = vg_read_for_update(cmd, vg_name, NULL,
+ 				READ_REQUIRE_RESIZEABLE | LOCK_NONBLOCKING);
+ 	if (vg_read_error(vg)) {
+ 		unlock_vg(cmd, VG_ORPHANS);
 		return ECMD_FAILED;
-	 }
+	}
 /********** FIXME
 	log_print("maximum logical volume size is %s",
 		  (dummy = lvm_show_size(LVM_LV_SIZE_MAX(vg) / 2, LONG)));
diff -rN -u -p old-temp.31080/tools/vgmerge.c new-temp.31080/tools/vgmerge.c
--- old-temp.31080/tools/vgmerge.c	2009-05-03 13:29:24.946235086 +0200
+++ new-temp.31080/tools/vgmerge.c	2009-05-03 13:29:25.054237779 +0200
@@ -28,16 +28,14 @@ static int _vgmerge_single(struct cmd_co
 	}
 
 	log_verbose("Checking for volume group \"%s\"", vg_name_to);
-	if (!(vg_to = vg_lock_and_read(cmd, vg_name_to, NULL, LCK_VG_WRITE,
-				       CLUSTERED | EXPORTED_VG | LVM_WRITE,
-				       CORRECT_INCONSISTENT | FAIL_INCONSISTENT)))
+ 	vg_to = vg_read_for_update(cmd, vg_name_to, NULL, 0);
+ 	if (vg_read_error(vg_to))
 		 return ECMD_FAILED;
 
 	log_verbose("Checking for volume group \"%s\"", vg_name_from);
-	if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, NULL,
-					 LCK_VG_WRITE | LCK_NONBLOCK,
-					 CLUSTERED | EXPORTED_VG | LVM_WRITE,
-					 CORRECT_INCONSISTENT | FAIL_INCONSISTENT))) {
+ 	vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
+ 				     LOCK_NONBLOCKING);
+ 	if (vg_read_error(vg_from)) {
 		unlock_release_vg(cmd, vg_to, vg_name_to);
 		return ECMD_FAILED;
 	}

[PATCH 2/9] Rework the toollib interface (process_each_*) on top of new vg_read.

[Petr Rockai]

Sun May  3 12:32:30 CEST 2009  Petr Rockai <me@mornfall.net>
  * Rework the toollib interface (process_each_*) on top of new vg_read.
diff -rN -u -p old-temp.31080/lib/metadata/metadata.c new-temp.31080/lib/metadata/metadata.c
--- old-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:27.826236802 +0200
@@ -357,7 +357,7 @@ static int remove_lvs_in_vg(struct cmd_c
 
 /* FIXME: remove redundant vg_name */
 int vg_remove_single(struct cmd_context *cmd, const char *vg_name,
-		     struct volume_group *vg, int consistent,
+		     struct volume_group *vg,
 		     force_t force __attribute((unused)))
 {
 	struct physical_volume *pv;
@@ -365,7 +365,7 @@ int vg_remove_single(struct cmd_context 
 	unsigned lv_count;
 	int ret = 1;
 
-	if (!vg || !consistent || vg_missing_pv_count(vg)) {
+	if (vg_read_error(vg) || vg_missing_pv_count(vg)) {
 		log_error("Volume group \"%s\" not found, is inconsistent "
 			  "or has PVs missing.", vg_name);
 		log_error("Consider vgreduce --removemissing if metadata "
diff -rN -u -p old-temp.31080/lib/metadata/metadata-exported.h new-temp.31080/lib/metadata/metadata-exported.h
--- old-temp.31080/lib/metadata/metadata-exported.h	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/lib/metadata/metadata-exported.h	2009-05-03 13:29:27.826236802 +0200
@@ -429,7 +429,7 @@ struct volume_group *vg_create(struct cm
 			       int pv_count, char **pv_names);
 int vg_remove(struct volume_group *vg);
 int vg_remove_single(struct cmd_context *cmd, const char *vg_name,
-		     struct volume_group *vg, int consistent,
+		     struct volume_group *vg,
 		     force_t force);
 int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
 	      const char *new_name);
diff -rN -u -p old-temp.31080/tools/lvchange.c new-temp.31080/tools/lvchange.c
--- old-temp.31080/tools/lvchange.c	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/tools/lvchange.c	2009-05-03 13:29:27.866236369 +0200
@@ -725,6 +725,6 @@ int lvchange(struct cmd_context *cmd, in
 		return EINVALID_CMD_LINE;
 	}
 
-	return process_each_lv(cmd, argc, argv, LCK_VG_WRITE, NULL,
+	return process_each_lv(cmd, argc, argv, READ_FOR_UPDATE, NULL,
 			       &lvchange_single);
 }
diff -rN -u -p old-temp.31080/tools/lvdisplay.c new-temp.31080/tools/lvdisplay.c
--- old-temp.31080/tools/lvdisplay.c	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/tools/lvdisplay.c	2009-05-03 13:29:27.866236369 +0200
@@ -54,6 +54,6 @@ int lvdisplay(struct cmd_context *cmd, i
 		return EINVALID_CMD_LINE;
 	}
 
-	return process_each_lv(cmd, argc, argv, LCK_VG_READ, NULL,
+	return process_each_lv(cmd, argc, argv, 0, NULL,
 			       &_lvdisplay_single);
 }
diff -rN -u -p old-temp.31080/tools/lvremove.c new-temp.31080/tools/lvremove.c
--- old-temp.31080/tools/lvremove.c	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/tools/lvremove.c	2009-05-03 13:29:27.870237506 +0200
@@ -41,6 +41,6 @@ int lvremove(struct cmd_context *cmd, in
 
 	cmd->handles_missing_pvs = 1;
 
-	return process_each_lv(cmd, argc, argv, LCK_VG_WRITE, NULL,
+	return process_each_lv(cmd, argc, argv, READ_FOR_UPDATE, NULL,
 			       &lvremove_single);
 }
diff -rN -u -p old-temp.31080/tools/lvscan.c new-temp.31080/tools/lvscan.c
--- old-temp.31080/tools/lvscan.c	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/tools/lvscan.c	2009-05-03 13:29:27.870237506 +0200
@@ -80,6 +80,6 @@ int lvscan(struct cmd_context *cmd, int 
 		return EINVALID_CMD_LINE;
 	}
 
-	return process_each_lv(cmd, argc, argv, LCK_VG_READ, NULL,
+	return process_each_lv(cmd, argc, argv, 0, NULL,
 			       &lvscan_single);
 }
diff -rN -u -p old-temp.31080/tools/polldaemon.c new-temp.31080/tools/polldaemon.c
--- old-temp.31080/tools/polldaemon.c	2009-05-03 13:29:27.774236967 +0200
+++ new-temp.31080/tools/polldaemon.c	2009-05-03 13:29:27.870237506 +0200
@@ -175,7 +175,7 @@ static int _wait_for_single_mirror(struc
 }
 
 static int _poll_vg(struct cmd_context *cmd, const char *vgname,
-		    struct volume_group *vg, int consistent, void *handle)
+		    struct volume_group *vg, void *handle)
 {
 	struct daemon_parms *parms = (struct daemon_parms *) handle;
 	struct lv_list *lvl;
@@ -183,18 +183,7 @@ static int _poll_vg(struct cmd_context *
 	const char *name;
 	int finished;
 
-	if (!vg) {
-		log_error("Couldn't read volume group %s", vgname);
-		return ECMD_FAILED;
-	}
-
-	if (!consistent) {
-		log_error("Volume Group %s inconsistent - skipping", vgname);
-		/* FIXME Should we silently recover it here or not? */
-		return ECMD_FAILED;
-	}
-
-	if (!vg_check_status(vg, EXPORTED_VG))
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
 
 	dm_list_iterate_items(lvl, &vg->lvs) {
@@ -219,7 +208,7 @@ static void _poll_for_all_vgs(struct cmd
 {
 	while (1) {
 		parms->outstanding_count = 0;
-		process_each_vg(cmd, 0, NULL, LCK_VG_WRITE, 1, parms, _poll_vg);
+		process_each_vg(cmd, 0, NULL, READ_FOR_UPDATE, parms, _poll_vg);
 		if (!parms->outstanding_count)
 			break;
 		sleep(parms->interval);
diff -rN -u -p old-temp.31080/tools/reporter.c new-temp.31080/tools/reporter.c
--- old-temp.31080/tools/reporter.c	2009-05-03 13:29:27.770236808 +0200
+++ new-temp.31080/tools/reporter.c	2009-05-03 13:29:27.870237506 +0200
@@ -18,12 +18,10 @@
 
 static int _vgs_single(struct cmd_context *cmd __attribute((unused)),
 		       const char *vg_name, struct volume_group *vg,
-		       int consistent __attribute((unused)), void *handle)
+		       void *handle)
 {
-	if (!vg) {
-		log_error("Volume group %s not found", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
 
 	if (!report_object(handle, vg, NULL, NULL, NULL, NULL))
 		return ECMD_FAILED;
@@ -181,26 +179,20 @@ static int _label_single(struct cmd_cont
 
 static int _pvs_in_vg(struct cmd_context *cmd, const char *vg_name,
 		      struct volume_group *vg,
-		      int consistent __attribute((unused)),
 		      void *handle)
 {
-	if (!vg) {
-		log_error("Volume group %s not found", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
 
 	return process_each_pv_in_vg(cmd, vg, NULL, handle, &_pvs_single);
 }
 
 static int _pvsegs_in_vg(struct cmd_context *cmd, const char *vg_name,
 			 struct volume_group *vg,
-			 int consistent __attribute((unused)),
 			 void *handle)
 {
-	if (!vg) {
-		log_error("Volume group %s not found", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
 
 	return process_each_pv_in_vg(cmd, vg, NULL, handle, &_pvsegs_single);
 }
@@ -378,12 +370,12 @@ static int _report(struct cmd_context *c
 
 	switch (report_type) {
 	case LVS:
-		r = process_each_lv(cmd, argc, argv, LCK_VG_READ, report_handle,
+		r = process_each_lv(cmd, argc, argv, 0, report_handle,
 				    &_lvs_single);
 		break;
 	case VGS:
-		r = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0,
-				    report_handle, &_vgs_single);
+		r = process_each_vg(cmd, argc, argv, 0, report_handle,
+				    &_vgs_single);
 		break;
 	case LABEL:
 		r = process_each_pv(cmd, argc, argv, NULL, LCK_NONE,
@@ -394,11 +386,11 @@ static int _report(struct cmd_context *c
 			r = process_each_pv(cmd, argc, argv, NULL, LCK_VG_READ,
 					    0, report_handle, &_pvs_single);
 		else
-			r = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0,
+			r = process_each_vg(cmd, argc, argv, 0,
 					    report_handle, &_pvs_in_vg);
 		break;
 	case SEGS:
-		r = process_each_lv(cmd, argc, argv, LCK_VG_READ, report_handle,
+		r = process_each_lv(cmd, argc, argv, 0, report_handle,
 				    &_lvsegs_single);
 		break;
 	case PVSEGS:
@@ -406,7 +398,7 @@ static int _report(struct cmd_context *c
 			r = process_each_pv(cmd, argc, argv, NULL, LCK_VG_READ,
 					    0, report_handle, &_pvsegs_single);
 		else
-			r = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0,
+			r = process_each_vg(cmd, argc, argv, 0,
 					    report_handle, &_pvsegs_in_vg);
 		break;
 	}
diff -rN -u -p old-temp.31080/tools/toollib.c new-temp.31080/tools/toollib.c
--- old-temp.31080/tools/toollib.c	2009-05-03 13:29:27.770236808 +0200
+++ new-temp.31080/tools/toollib.c	2009-05-03 13:29:27.870237506 +0200
@@ -161,7 +161,7 @@ int process_each_lv_in_vg(struct cmd_con
 }
 
 int process_each_lv(struct cmd_context *cmd, int argc, char **argv,
-		    uint32_t lock_type, void *handle,
+		    uint32_t flags, void *handle,
 		    int (*process_single) (struct cmd_context * cmd,
 					   struct logical_volume * lv,
 					   void *handle))
@@ -169,7 +169,6 @@ int process_each_lv(struct cmd_context *
 	int opt = 0;
 	int ret_max = ECMD_PROCESSED;
 	int ret = 0;
-	int consistent;
 
 	struct dm_list *tags_arg;
 	struct dm_list *vgnames;	/* VGs to process */
@@ -284,42 +283,10 @@ int process_each_lv(struct cmd_context *
 		vgname = strl->str;
 		if (is_orphan_vg(vgname))
 			continue;	/* FIXME Unnecessary? */
-		if (!lock_vol(cmd, vgname, lock_type)) {
-			log_error("Can't lock %s: skipping", vgname);
-			ret_max = ECMD_FAILED;
-			continue;
-		}
-		if (lock_type & LCK_WRITE)
-			consistent = 1;
-		else
-			consistent = 0;
-		if (!(vg = vg_read_internal(cmd, vgname, NULL, &consistent)) || !consistent) {
-			unlock_vg(cmd, vgname);
-			if (!vg)
-				log_error("Volume group \"%s\" "
-					  "not found", vgname);
-			else {
-				if (!vg_check_status(vg, CLUSTERED)) {
-					if (ret_max < ECMD_FAILED)
-						ret_max = ECMD_FAILED;
-					vg_release(vg);
-					continue;
-				}
-				log_error("Volume group \"%s\" "
-					  "inconsistent", vgname);
-			}
+		vg = vg_read(cmd, vgname, NULL, flags);
 
+		if (vg_read_error(vg)) {
 			vg_release(vg);
-			if (!vg || !(vg = recover_vg(cmd, vgname, lock_type))) {
-				if (ret_max < ECMD_FAILED)
-					ret_max = ECMD_FAILED;
-				vg_release(vg);
-				continue;
-			}
-		}
-
-		if (!vg_check_status(vg, CLUSTERED)) {
-			unlock_release_vg(cmd, vg, vgname);
 			if (ret_max < ECMD_FAILED)
 				ret_max = ECMD_FAILED;
 			continue;
@@ -380,8 +347,8 @@ int process_each_segment_in_pv(struct cm
 	if (is_pv(pv) && !vg && !is_orphan(pv)) {
 		vg_name = pv_vg_name(pv);
 
-		if (!(vg = vg_lock_and_read(cmd, vg_name, NULL, LCK_VG_READ,
-					    CLUSTERED, 0))) {
+		vg = vg_read(cmd, vg_name, NULL, 0);
+		if (vg_read_error(vg)) {
 			log_error("Skipping volume group %s", vg_name);
 			return ECMD_FAILED;
 		}
@@ -446,32 +413,19 @@ int process_each_segment_in_lv(struct cm
 static int _process_one_vg(struct cmd_context *cmd, const char *vg_name,
 			   const char *vgid,
 			   struct dm_list *tags, struct dm_list *arg_vgnames,
-			   uint32_t lock_type, int consistent, void *handle,
+			   uint32_t flags, void *handle,
 			   int ret_max,
 			   int (*process_single) (struct cmd_context * cmd,
 						  const char *vg_name,
 						  struct volume_group * vg,
-						  int consistent, void *handle))
+						  void *handle))
 {
 	struct volume_group *vg;
 	int ret = 0;
 
-	if (!lock_vol(cmd, vg_name, lock_type)) {
-		log_error("Can't lock volume group %s: skipping", vg_name);
-		return ECMD_FAILED;
-	}
-
 	log_verbose("Finding volume group \"%s\"", vg_name);
-	if (!(vg = vg_read_internal(cmd, vg_name, vgid, &consistent))) {
-		log_error("Volume group \"%s\" not found", vg_name);
-		unlock_vg(cmd, vg_name);
-		return ECMD_FAILED;
-	}
 
-	if (!vg_check_status(vg, CLUSTERED)) {
-		unlock_release_vg(cmd, vg, vg_name);
-		return ECMD_FAILED;
-	}
+	vg = vg_read(cmd, vg_name, vgid, flags);
 
 	if (!dm_list_empty(tags)) {
 		/* Only process if a tag matches or it's on arg_vgnames */
@@ -482,7 +436,7 @@ static int _process_one_vg(struct cmd_co
 		}
 	}
 
-	if ((ret = process_single(cmd, vg_name, vg, consistent,
+	if ((ret = process_single(cmd, vg_name, vg,
 				  handle)) > ret_max) {
 		ret_max = ret;
 	}
@@ -493,11 +447,11 @@ static int _process_one_vg(struct cmd_co
 }
 
 int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
-		    uint32_t lock_type, int consistent, void *handle,
+		    uint32_t flags, void *handle,
 		    int (*process_single) (struct cmd_context * cmd,
 					   const char *vg_name,
 					   struct volume_group * vg,
-					   int consistent, void *handle))
+					   void *handle))
 {
 	int opt = 0;
 	int ret_max = ECMD_PROCESSED;
@@ -560,7 +514,7 @@ int process_each_vg(struct cmd_context *
 				continue;
 			ret_max = _process_one_vg(cmd, vg_name, vgid, &tags,
 						  &arg_vgnames,
-					  	  lock_type, consistent, handle,
+					  	  flags, handle,
 					  	  ret_max, process_single);
 			if (sigint_caught())
 				return ret_max;
@@ -572,7 +526,7 @@ int process_each_vg(struct cmd_context *
 				continue;	/* FIXME Unnecessary? */
 			ret_max = _process_one_vg(cmd, vg_name, NULL, &tags,
 						  &arg_vgnames,
-					  	  lock_type, consistent, handle,
+					  	  flags, handle,
 					  	  ret_max, process_single);
 			if (sigint_caught())
 				return ret_max;
@@ -1179,30 +1133,6 @@ struct dm_list *clone_pv_list(struct dm_
 	return r;
 }
 
-/*
- * Attempt metadata recovery
- */
-struct volume_group *recover_vg(struct cmd_context *cmd, const char *vgname,
-				uint32_t lock_type)
-{
-	int consistent = 1;
-
-	/* Don't attempt automatic recovery without proper locking */
-	if (lockingfailed())
-		return NULL;
-
-	lock_type &= ~LCK_TYPE_MASK;
-	lock_type |= LCK_WRITE;
-
-	if (!lock_vol(cmd, vgname, lock_type)) {
-		log_error("Can't lock %s for metadata recovery: skipping",
-			  vgname);
-		return NULL;
-	}
-
-	return vg_read_internal(cmd, vgname, NULL, &consistent);
-}
-
 int apply_lvname_restrictions(const char *name)
 {
 	if (!strncmp(name, "snapshot", 8)) {
diff -rN -u -p old-temp.31080/tools/toollib.h new-temp.31080/tools/toollib.h
--- old-temp.31080/tools/toollib.h	2009-05-03 13:29:27.766236090 +0200
+++ new-temp.31080/tools/toollib.h	2009-05-03 13:29:27.870237506 +0200
@@ -27,11 +27,11 @@ struct volume_group *recover_vg(struct c
 				uint32_t lock_type);
 
 int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
-		    uint32_t lock_type, int consistent, void *handle,
+		    uint32_t flags, void *handle,
 		    int (*process_single) (struct cmd_context * cmd,
 					   const char *vg_name,
 					   struct volume_group * vg,
-					   int consistent, void *handle));
+					   void *handle));
 
 int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
 		    struct volume_group *vg, uint32_t lock_type,
@@ -51,7 +51,7 @@ int process_each_segment_in_pv(struct cm
 						      void *handle));
 
 int process_each_lv(struct cmd_context *cmd, int argc, char **argv,
-		    uint32_t lock_type, void *handle,
+		    uint32_t flags, void *handle,
 		    int (*process_single) (struct cmd_context * cmd,
 					   struct logical_volume * lv,
 					   void *handle));
diff -rN -u -p old-temp.31080/tools/vgcfgbackup.c new-temp.31080/tools/vgcfgbackup.c
--- old-temp.31080/tools/vgcfgbackup.c	2009-05-03 13:29:27.766236090 +0200
+++ new-temp.31080/tools/vgcfgbackup.c	2009-05-03 13:29:27.870237506 +0200
@@ -48,19 +48,14 @@ static char *_expand_filename(const char
 }
 
 static int vg_backup_single(struct cmd_context *cmd, const char *vg_name,
-			    struct volume_group *vg, int consistent,
+			    struct volume_group *vg,
 			    void *handle)
 {
 	char **last_filename = (char **)handle;
 	char *filename;
 
-	if (!vg) {
-		log_error("Volume group \"%s\" not found", vg_name);
+	if (vg_read_error(vg) && !vg_read_error(vg) == FAILED_INCONSISTENT)
 		return ECMD_FAILED;
-	}
-
-	if (!consistent)
-		log_error("WARNING: Volume group \"%s\" inconsistent", vg_name);
 
 	if (arg_count(cmd, file_ARG)) {
 		if (!(filename = _expand_filename(arg_value(cmd, file_ARG),
@@ -72,7 +67,7 @@ static int vg_backup_single(struct cmd_c
 		if (!backup_to_file(filename, vg->cmd->cmd_line, vg))
 			return ECMD_FAILED;
 	} else {
-		if (!consistent) {
+		if (vg_read_error(vg) == FAILED_INCONSISTENT) {
 			log_error("No backup taken: specify filename with -f "
 				  "to backup an inconsistent VG");
 			stack;
@@ -98,7 +93,7 @@ int vgcfgbackup(struct cmd_context *cmd,
 
 	init_pvmove(1);
 
-	ret = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0, &last_filename,
+	ret = process_each_vg(cmd, argc, argv, 0, &last_filename,
 			      &vg_backup_single);
 
 	dm_free(last_filename);
diff -rN -u -p old-temp.31080/tools/vgchange.c new-temp.31080/tools/vgchange.c
--- old-temp.31080/tools/vgchange.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgchange.c	2009-05-03 13:29:27.870237506 +0200
@@ -521,28 +521,13 @@ static int _vgchange_refresh(struct cmd_
 }
 
 static int vgchange_single(struct cmd_context *cmd, const char *vg_name,
-			   struct volume_group *vg, int consistent,
+			   struct volume_group *vg,
 			   void *handle __attribute((unused)))
 {
 	int r = ECMD_FAILED;
 
-	if (!vg) {
-		log_error("Unable to find volume group \"%s\"", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
-
-	if (!consistent) {
-		unlock_release_vg(cmd, vg, vg_name);
-		dev_close_all();
-		log_error("Volume group \"%s\" inconsistent", vg_name);
-		if (!(vg = recover_vg(cmd, vg_name, LCK_VG_WRITE)))
-			return ECMD_FAILED;
-	}
-
-	if (!(vg_status(vg) & LVM_WRITE) && !arg_count(cmd, available_ARG)) {
-		log_error("Volume group \"%s\" is read-only", vg->name);
-		return ECMD_FAILED;
-	}
 
 	if (vg_status(vg) & EXPORTED_VG) {
 		log_error("Volume group \"%s\" is exported", vg_name);
@@ -633,6 +618,6 @@ int vgchange(struct cmd_context *cmd, in
 
 	return process_each_vg(cmd, argc, argv,
 			       (arg_count(cmd, available_ARG)) ?
-			       LCK_VG_READ : LCK_VG_WRITE, 0, NULL,
+			       0 : READ_FOR_UPDATE, NULL,
 			       &vgchange_single);
 }
diff -rN -u -p old-temp.31080/tools/vgck.c new-temp.31080/tools/vgck.c
--- old-temp.31080/tools/vgck.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgck.c	2009-05-03 13:29:27.870237506 +0200
@@ -18,30 +18,14 @@
 
 static int vgck_single(struct cmd_context *cmd __attribute((unused)),
 		       const char *vg_name,
-		       struct volume_group *vg, int consistent,
+		       struct volume_group *vg,
 		       void *handle __attribute((unused)))
 {
-	if (!vg) {
-		log_error("Volume group \"%s\" not found", vg_name);
-		return ECMD_FAILED;
-	}
-
-	if (!consistent) {
-		log_error("Volume group \"%s\" inconsistent", vg_name);
-		return ECMD_FAILED;
-	}
-
-	if (!vg_check_status(vg, EXPORTED_VG))
-		return ECMD_FAILED;
-
-	if (!vg_validate(vg))
-		return ECMD_FAILED;
-
 	return ECMD_PROCESSED;
 }
 
 int vgck(struct cmd_context *cmd, int argc, char **argv)
 {
-	return process_each_vg(cmd, argc, argv, LCK_VG_READ, 0, NULL,
+	return process_each_vg(cmd, argc, argv, 0, NULL,
 			       &vgck_single);
 }
diff -rN -u -p old-temp.31080/tools/vgconvert.c new-temp.31080/tools/vgconvert.c
--- old-temp.31080/tools/vgconvert.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgconvert.c	2009-05-03 13:29:27.870237506 +0200
@@ -16,7 +16,7 @@
 #include "tools.h"
 
 static int vgconvert_single(struct cmd_context *cmd, const char *vg_name,
-			    struct volume_group *vg, int consistent,
+			    struct volume_group *vg,
 			    void *handle __attribute((unused)))
 {
 	struct physical_volume *pv, *existing_pv;
@@ -32,18 +32,8 @@ static int vgconvert_single(struct cmd_c
 	struct lvinfo info;
 	int active = 0;
 
-	if (!vg) {
-		log_error("Unable to find volume group \"%s\"", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
-
-	if (!consistent) {
-		unlock_release_vg(cmd, vg, vg_name);
-		dev_close_all();
-		log_error("Volume group \"%s\" inconsistent", vg_name);
-		if (!(vg = recover_vg(cmd, vg_name, LCK_VG_WRITE)))
-			return ECMD_FAILED;
-	}
 
 	if (!vg_check_status(vg, LVM_WRITE | EXPORTED_VG))
 		return ECMD_FAILED;
@@ -233,6 +223,6 @@ int vgconvert(struct cmd_context *cmd, i
 		return EINVALID_CMD_LINE;
 	}
 
-	return process_each_vg(cmd, argc, argv, LCK_VG_WRITE, 0, NULL,
+	return process_each_vg(cmd, argc, argv, READ_FOR_UPDATE, NULL,
 			       &vgconvert_single);
 }
diff -rN -u -p old-temp.31080/tools/vgdisplay.c new-temp.31080/tools/vgdisplay.c
--- old-temp.31080/tools/vgdisplay.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgdisplay.c	2009-05-03 13:29:27.870237506 +0200
@@ -16,17 +16,12 @@
 #include "tools.h"
 
 static int vgdisplay_single(struct cmd_context *cmd, const char *vg_name,
-			    struct volume_group *vg, int consistent,
+			    struct volume_group *vg,
 			    void *handle __attribute((unused)))
 {
 	/* FIXME Do the active check here if activevolumegroups_ARG ? */
-	if (!vg) {
-		log_error("Volume group \"%s\" doesn't exist", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
-
-	if (!consistent)
-		log_error("WARNING: Volume group \"%s\" inconsistent", vg_name);
 
 	vg_check_status(vg, EXPORTED_VG);
 
@@ -98,8 +93,7 @@ int vgdisplay(struct cmd_context *cmd, i
 	}
 **********/
 
-	return process_each_vg(cmd, argc, argv, LCK_VG_READ, 0, NULL,
-			       vgdisplay_single);
+	return process_each_vg(cmd, argc, argv, 0, NULL, vgdisplay_single);
 
 /******** FIXME Need to count number processed
 	  Add this to process_each_vg if arg_count(cmd,activevolumegroups_ARG) ?
diff -rN -u -p old-temp.31080/tools/vgexport.c new-temp.31080/tools/vgexport.c
--- old-temp.31080/tools/vgexport.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgexport.c	2009-05-03 13:29:27.870237506 +0200
@@ -17,25 +17,14 @@
 
 static int vgexport_single(struct cmd_context *cmd __attribute((unused)),
 			   const char *vg_name,
-			   struct volume_group *vg, int consistent,
+			   struct volume_group *vg,
 			   void *handle __attribute((unused)))
 {
 	struct pv_list *pvl;
 	struct physical_volume *pv;
 
-	if (!vg) {
-		log_error("Unable to find volume group \"%s\"", vg_name);
+	if (vg_read_error(vg))
 		goto error;
-	}
-
-	if (!consistent) {
-		log_error("Volume group %s inconsistent", vg_name);
-		goto error;
-	}
-
-	if (!vg_check_status(vg, EXPORTED_VG | LVM_WRITE)) {
-		goto error;
-	}
 
 	if (lvs_in_vg_activated(vg)) {
 		log_error("Volume group \"%s\" has active logical volumes",
@@ -78,6 +67,6 @@ int vgexport(struct cmd_context *cmd, in
 		return ECMD_FAILED;
 	}
 
-	return process_each_vg(cmd, argc, argv, LCK_VG_WRITE, 1, NULL,
+	return process_each_vg(cmd, argc, argv, READ_FOR_UPDATE, NULL,
 			       &vgexport_single);
 }
diff -rN -u -p old-temp.31080/tools/vgimport.c new-temp.31080/tools/vgimport.c
--- old-temp.31080/tools/vgimport.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgimport.c	2009-05-03 13:29:27.870237506 +0200
@@ -17,17 +17,14 @@
 
 static int vgimport_single(struct cmd_context *cmd __attribute((unused)),
 			   const char *vg_name,
-			   struct volume_group *vg, int consistent,
+			   struct volume_group *vg,
 			   void *handle __attribute((unused)))
 {
 	struct pv_list *pvl;
 	struct physical_volume *pv;
 
-	if (!vg || !consistent) {
-		log_error("Unable to find exported volume group \"%s\"",
-			  vg_name);
+	if (vg_read_error(vg))
 		goto error;
-	}
 
 	if (!(vg_status(vg) & EXPORTED_VG)) {
 		log_error("Volume group \"%s\" is not exported", vg_name);
@@ -74,6 +71,7 @@ int vgimport(struct cmd_context *cmd, in
 		return ECMD_FAILED;
 	}
 
-	return process_each_vg(cmd, argc, argv, LCK_VG_WRITE, 1, NULL,
+	return process_each_vg(cmd, argc, argv,
+			       READ_FOR_UPDATE | READ_ALLOW_EXPORTED, NULL,
 			       &vgimport_single);
 }
diff -rN -u -p old-temp.31080/tools/vgremove.c new-temp.31080/tools/vgremove.c
--- old-temp.31080/tools/vgremove.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgremove.c	2009-05-03 13:29:27.874238783 +0200
@@ -16,10 +16,10 @@
 #include "tools.h"
 
 static int vgremove_single(struct cmd_context *cmd, const char *vg_name,
-			   struct volume_group *vg, int consistent,
+			   struct volume_group *vg,
 			   void *handle __attribute((unused)))
 {
-	if (!vg_remove_single(cmd, vg_name, vg, consistent,
+	if (!vg_remove_single(cmd, vg_name, vg,
 			      arg_count(cmd, force_ARG)))
 		return ECMD_FAILED;
 
@@ -41,7 +41,7 @@ int vgremove(struct cmd_context *cmd, in
 	}
 
 	ret = process_each_vg(cmd, argc, argv,
-			      LCK_VG_WRITE | LCK_NONBLOCK, 1,
+			      READ_FOR_UPDATE | LOCK_NONBLOCKING,
 			      NULL, &vgremove_single);
 
 	unlock_vg(cmd, VG_ORPHANS);
diff -rN -u -p old-temp.31080/tools/vgscan.c new-temp.31080/tools/vgscan.c
--- old-temp.31080/tools/vgscan.c	2009-05-03 13:29:27.762235512 +0200
+++ new-temp.31080/tools/vgscan.c	2009-05-03 13:29:27.874238783 +0200
@@ -16,22 +16,11 @@
 #include "tools.h"
 
 static int vgscan_single(struct cmd_context *cmd, const char *vg_name,
-			 struct volume_group *vg, int consistent,
+			 struct volume_group *vg,
 			 void *handle __attribute((unused)))
 {
-	if (!vg) {
-		log_error("Volume group \"%s\" not found", vg_name);
+	if (vg_read_error(vg))
 		return ECMD_FAILED;
-	}
-
-	if (!consistent) {
-		unlock_release_vg(cmd, vg, vg_name);
-		dev_close_all();
-		log_error("Volume group \"%s\" inconsistent", vg_name);
-		/* Don't allow partial switch to this program */
-		if (!(vg = recover_vg(cmd, vg_name, LCK_VG_WRITE)))
-			return ECMD_FAILED;
-	}
 
 	log_print("Found %svolume group \"%s\" using metadata type %s",
 		  (vg_status(vg) & EXPORTED_VG) ? "exported " : "", vg_name,
@@ -61,8 +50,7 @@ int vgscan(struct cmd_context *cmd, int 
 
 	log_print("Reading all physical volumes.  This may take a while...");
 
-	maxret = process_each_vg(cmd, argc, argv, LCK_VG_READ, 0, NULL,
-				 &vgscan_single);
+	maxret = process_each_vg(cmd, argc, argv, 0, NULL, &vgscan_single);
 
 	if (arg_count(cmd, mknodes_ARG)) {
 		ret = vgmknodes(cmd, argc, argv);

[PATCH 3/9] Convert vgreduce to use vg_read_for_update.

[Petr Rockai]

Sun May  3 12:50:58 CEST 2009  Petr Rockai <me@mornfall.net>
  * Convert vgreduce to use vg_read_for_update.
diff -rN -u -p old-temp.31080/tools/vgreduce.c new-temp.31080/tools/vgreduce.c
--- old-temp.31080/tools/vgreduce.c	2009-05-03 13:29:29.730232352 +0200
+++ new-temp.31080/tools/vgreduce.c	2009-05-03 13:29:29.830234657 +0200
@@ -382,7 +382,6 @@ static int _vgreduce_single(struct cmd_c
 {
 	struct pv_list *pvl;
 	struct volume_group *orphan_vg = NULL;
-	int consistent = 1;
 	int r = ECMD_FAILED;
 	const char *name = pv_dev_name(pv);
 
@@ -397,15 +396,10 @@ static int _vgreduce_single(struct cmd_c
 		return ECMD_FAILED;
 	}
 
-	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE | LCK_NONBLOCK)) {
-		log_error("Can't get lock for orphan PVs");
-		return ECMD_FAILED;
-	}
-
 	pvl = find_pv_in_vg(vg, name);
 
 	if (!archive(vg))
-		goto bad;
+		return ECMD_FAILED;
 
 	log_verbose("Removing \"%s\" from volume group \"%s\"", name, vg->name);
 
@@ -424,11 +418,11 @@ static int _vgreduce_single(struct cmd_c
 	vg->free_count -= pv_pe_count(pv) - pv_pe_alloc_count(pv);
 	vg->extent_count -= pv_pe_count(pv);
 
-	if(!(orphan_vg = vg_read_internal(cmd, vg->fid->fmt->orphan_vg_name, NULL, &consistent)) ||
-	   !consistent) {
-		log_error("Unable to read existing orphan PVs");
+ 	orphan_vg = vg_read_for_update(cmd, vg->fid->fmt->orphan_vg_name,
+ 				       NULL, LOCK_NONBLOCKING);
+
+ 	if (vg_read_error(orphan_vg))
 		goto bad;
-	}
 
 	if (!vg_split_mdas(cmd, vg, orphan_vg) || !vg->pv_count) {
 		log_error("Cannot remove final metadata area on \"%s\" from \"%s\"",
@@ -463,7 +457,6 @@ int vgreduce(struct cmd_context *cmd, in
 	struct volume_group *vg;
 	char *vg_name;
 	int ret = ECMD_FAILED;
-	int consistent = 1;
 	int fixed = 1;
 	int repairing = arg_count(cmd, removemissing_ARG);
 
@@ -510,22 +503,19 @@ int vgreduce(struct cmd_context *cmd, in
 	}
 
 	log_verbose("Finding volume group \"%s\"", vg_name);
-	if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) {
-		log_error("Can't get lock for %s", vg_name);
-		return ECMD_FAILED;
-	}
 
-	if ((!(vg = vg_read_internal(cmd, vg_name, NULL, &consistent)) || !consistent)
-	    && !repairing) {
-		log_error("Volume group \"%s\" doesn't exist", vg_name);
+	vg = vg_read_for_update(cmd, vg_name, NULL, READ_ALLOW_EXPORTED);
+	if (vg_read_error(vg) == FAILED_ALLOCATION ||
+	    vg_read_error(vg) == FAILED_NOTFOUND)
 		goto out;
-	}
 
-	if (vg && !vg_check_status(vg, CLUSTERED))
+	/* FIXME We want to allow read-only VGs to be changed here? */
+	if (vg_read_error(vg) && vg_read_error(vg) != FAILED_READ_ONLY
+	    && !arg_count(cmd, removemissing_ARG))
 		goto out;
 
 	if (repairing) {
-		if (vg && consistent && !vg_missing_pv_count(vg)) {
+		if (!vg_read_error(vg) && !vg_missing_pv_count(vg)) {
 			log_error("Volume group \"%s\" is already consistent",
 				  vg_name);
 			ret = ECMD_PROCESSED;
@@ -533,13 +523,16 @@ int vgreduce(struct cmd_context *cmd, in
 		}
 
 		vg_release(vg);
-		consistent = !arg_count(cmd, force_ARG);
-		if (!(vg = vg_read_internal(cmd, vg_name, NULL, &consistent))) {
-			log_error("Volume group \"%s\" not found", vg_name);
-			goto out;
-		}
-		if (!vg_check_status(vg, CLUSTERED))
+		log_verbose("Trying to open VG %s for recovery...", vg_name);
+
+		vg = vg_read_for_update(cmd, vg_name, NULL,
+					READ_ALLOW_INCONSISTENT
+					| READ_ALLOW_EXPORTED);
+
+		if (vg_read_error(vg) && vg_read_error(vg) != FAILED_READ_ONLY
+		    && vg_read_error(vg) != FAILED_INCONSISTENT)
 			goto out;
+
 		if (!archive(vg))
 			goto out;
 

[PATCH 4/9] Convert vgrename to vg_read_for_update.

[Petr Rockai]

Sun May  3 12:54:28 CEST 2009  Petr Rockai <me@mornfall.net>
  * Convert vgrename to vg_read_for_update.
diff -rN -u -p old-temp.31080/tools/vgrename.c new-temp.31080/tools/vgrename.c
--- old-temp.31080/tools/vgrename.c	2009-05-03 13:29:31.734236424 +0200
+++ new-temp.31080/tools/vgrename.c	2009-05-03 13:29:31.834237541 +0200
@@ -20,7 +20,6 @@ static int vg_rename_path(struct cmd_con
 {
 	char *dev_dir;
 	struct id id;
-	int consistent = 1;
 	int match = 0;
 	int found_id = 0;
 	struct dm_list *vgids;
@@ -70,25 +69,11 @@ static int vg_rename_path(struct cmd_con
 	} else
 		vgid = NULL;
 
-	if (!lock_vol(cmd, vg_name_old, LCK_VG_WRITE)) {
-		log_error("Can't get lock for %s", vg_name_old);
-		return 0;
-	}
-
-	if (!(vg = vg_read_internal(cmd, vg_name_old, vgid, &consistent)) || !consistent) {
-		log_error("Volume group %s %s%s%snot found.", vg_name_old,
-		vgid ? "(" : "", vgid ? vgid : "", vgid ? ") " : "");
-		unlock_vg(cmd, vg_name_old);
-		return 0;
-	}
-
-	if (!vg_check_status(vg, CLUSTERED | LVM_WRITE)) {
-		unlock_release_vg(cmd, vg, vg_name_old);
-		return 0;
-	}
-
-	/* Don't return failure for EXPORTED_VG */
-	vg_check_status(vg, EXPORTED_VG);
+	/* FIXME we used to print an error about EXPORTED, but proceeded
+	   nevertheless. */
+	vg = vg_read_for_update(cmd, vg_name_old, vgid, READ_ALLOW_EXPORTED);
+	if (vg_read_error(vg))
+		return_0;
 
 	if (lvs_in_vg_activated_by_uuid_only(vg)) {
 		unlock_release_vg(cmd, vg, vg_name_old);
@@ -100,14 +85,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_release_vg(cmd, vg, 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, LOCK_KEEP |
+				    READ_CHECK_EXISTENCE | LOCK_NONBLOCKING);
+
+	if (vg_read_error(vg_new))
+		goto error;
 
-	consistent = 0;
-	if ((vg_new = vg_read_internal(cmd, vg_name_new, NULL, &consistent))) {
+	if (vg_might_exist(vg_new)) {
 		log_error("New volume group \"%s\" already exists",
 			  vg_name_new);
 		goto error;

[PATCH 5/9] Don't segfault in vg_release when vg->cmd is NULL.

[Petr Rockai]

Sun May  3 13:06:14 CEST 2009  Petr Rockai <me@mornfall.net>
  * Don't segfault in vg_release when vg->cmd is NULL.
diff -rN -u -p old-temp.31080/lib/metadata/metadata.c new-temp.31080/lib/metadata/metadata.c
--- old-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:33.682235890 +0200
+++ new-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:33.734236563 +0200
@@ -2091,7 +2091,7 @@ void vg_release(struct volume_group *vg)
 	if (!vg || !vg->vgmem)
 		return;
 
-	if (vg->vgmem == vg->cmd->mem)
+	if (vg->cmd && vg->vgmem == vg->cmd->mem)
 		log_error("Internal error: global memory pool used for VG %s",
 			  vg->name);
 

[PATCH 6/9] Convert vgsplit to use vg_read_for_update.

[Petr Rockai]

Sun May  3 13:12:28 CEST 2009  Petr Rockai <me@mornfall.net>
  * Convert vgsplit to use vg_read_for_update.
diff -rN -u -p old-temp.31080/tools/vgsplit.c new-temp.31080/tools/vgsplit.c
--- old-temp.31080/tools/vgsplit.c	2009-05-03 13:29:35.650236640 +0200
+++ new-temp.31080/tools/vgsplit.c	2009-05-03 13:29:35.758243454 +0200
@@ -289,7 +289,6 @@ int vgsplit(struct cmd_context *cmd, int
 	struct volume_group *vg_to = NULL, *vg_from = NULL;
 	int opt;
 	int existing_vg;
-	int consistent;
 	int r = ECMD_FAILED;
 	const char *lv_name;
 
@@ -321,21 +320,22 @@ int vgsplit(struct cmd_context *cmd, int
 	}
 
 	log_verbose("Checking for volume group \"%s\"", vg_name_from);
-	if (!(vg_from = vg_lock_and_read(cmd, vg_name_from, NULL, LCK_VG_WRITE,
-				       CLUSTERED | EXPORTED_VG |
-				       RESIZEABLE_VG | LVM_WRITE,
-				       CORRECT_INCONSISTENT | FAIL_INCONSISTENT)))
-		 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_release_vg(cmd, vg_from, vg_name_from);
+	vg_from = vg_read_for_update(cmd, vg_name_from, NULL,
+				     READ_REQUIRE_RESIZEABLE);
+	if (vg_read_error(vg_from))
 		return ECMD_FAILED;
-	}
 
-	consistent = 0;
-	if ((vg_to = vg_read_internal(cmd, vg_name_to, NULL, &consistent))) {
+	log_verbose("Checking for new volume group \"%s\"", vg_name_to);
+	vg_to = vg_read_for_update(cmd, vg_name_to, NULL,
+				   READ_REQUIRE_RESIZEABLE |
+				   LOCK_NONBLOCKING | LOCK_KEEP |
+				   READ_CHECK_EXISTENCE);
+
+	if (vg_read_error(vg_to))
+		goto_bad;
+
+	if (vg_might_exist(vg_to)) {
 		existing_vg = 1;
 		if (new_vg_option_specified(cmd)) {
 			log_error("Volume group \"%s\" exists, but new VG "
@@ -448,13 +448,12 @@ int vgsplit(struct cmd_context *cmd, int
 	/*
 	 * Finally, remove the EXPORTED flag from the new VG and write it out.
 	 */
-	consistent = 1;
 	if (!test_mode()) {
-		vg_release(vg_to);
-		if (!(vg_to = vg_read_internal(cmd, vg_name_to, NULL, &consistent)) ||
-		    !consistent) {
-			log_error("Volume group \"%s\" became inconsistent: please "
-				  "fix manually", vg_name_to);
+		vg_to = vg_read_for_update(cmd, vg_name_to, NULL,
+					   READ_ALLOW_EXPORTED);
+		if (vg_read_error(vg_to)) {
+			log_error("Volume group \"%s\" became inconsistent: "
+				  "please fix manually", vg_name_to);
 			goto_bad;
 		}
 	}

[PATCH 7/9] Remove now-unused vg_lock_and_read.

[Petr Rockai]

Sun May  3 13:14:51 CEST 2009  Petr Rockai <me@mornfall.net>
  * Remove now-unused vg_lock_and_read.
diff -rN -u -p old-temp.31080/lib/metadata/metadata.c new-temp.31080/lib/metadata/metadata.c
--- old-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:37.630236334 +0200
+++ new-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:37.682236658 +0200
@@ -2541,50 +2541,6 @@ int vg_check_status(const struct volume_
 }
 
 /*
- * vg_lock_and_read - consolidate vg locking, reading, and status flag checking
- *
- * Returns:
- * NULL - failure
- * non-NULL - success; volume group handle
- */
-vg_t *vg_lock_and_read(struct cmd_context *cmd, const char *vg_name,
-		       const char *vgid,
-		       uint32_t lock_flags, uint32_t status_flags,
-		       uint32_t misc_flags)
-{
-	struct volume_group *vg;
-	int consistent = 1;
-
-	if (!(misc_flags & CORRECT_INCONSISTENT))
-		consistent = 0;
-
-	if (!validate_name(vg_name)) {
-		log_error("Volume group name %s has invalid characters",
-			  vg_name);
-		return NULL;
-	}
-
-	if (!lock_vol(cmd, vg_name, lock_flags)) {
-		log_error("Can't get lock for %s", vg_name);
-		return NULL;
-	}
-
-	if (!(vg = vg_read_internal(cmd, vg_name, vgid, &consistent)) ||
-	    ((misc_flags & FAIL_INCONSISTENT) && !consistent)) {
-		log_error("Volume group \"%s\" not found", vg_name);
-		unlock_release_vg(cmd, vg, vg_name);
-		return NULL;
-	}
-
-	if (!vg_check_status(vg, status_flags)) {
-		unlock_release_vg(cmd, vg, vg_name);
-		return NULL;
-	}
-
-	return vg;
-}
-
-/*
  * Create a (vg_t) volume group handle from a struct volume_group pointer and a
  * possible failure code or zero for success.
  */
diff -rN -u -p old-temp.31080/lib/metadata/metadata-exported.h new-temp.31080/lib/metadata/metadata-exported.h
--- old-temp.31080/lib/metadata/metadata-exported.h	2009-05-03 13:29:37.630236334 +0200
+++ new-temp.31080/lib/metadata/metadata-exported.h	2009-05-03 13:29:37.682236658 +0200
@@ -96,10 +96,6 @@ struct pv_segment;
 #define FMT_UNLIMITED_STRIPESIZE 0x00000100U	/* Unlimited stripe size? */
 #define FMT_RESTRICTED_READAHEAD 0x00000200U	/* Readahead restricted to 2-120? */
 
-/* LVM2 external library flags */
-#define CORRECT_INCONSISTENT    0x00000001U /* Correct inconsistent metadata */
-#define FAIL_INCONSISTENT       0x00000002U /* Fail if metadata inconsistent */
-
 /* Mirror conversion type flags */
 #define MIRROR_BY_SEG		0x00000001U	/* segment-by-segment mirror */
 #define MIRROR_BY_LV		0x00000002U	/* mirror using whole mimage LVs */

[PATCH 8/9] Un-export vg_read_internal.

[Petr Rockai]

Sun May  3 13:17:06 CEST 2009  Petr Rockai <me@mornfall.net>
  * Un-export vg_read_internal.
diff -rN -u -p old-temp.31080/lib/metadata/metadata.c new-temp.31080/lib/metadata/metadata.c
--- old-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:39.590237605 +0200
+++ new-temp.31080/lib/metadata/metadata.c	2009-05-03 13:29:39.638236373 +0200
@@ -31,6 +31,9 @@
 
 #include <sys/param.h>
 
+static struct volume_group *_vg_read_internal(struct cmd_context *cmd,
+					      const char *vgname,
+					      const char *vgid, int *consistent);
 /*
  * FIXME: Check for valid handle before dereferencing field or log error?
  */
@@ -244,8 +247,8 @@ int get_pv_from_vg_by_id(const struct fo
 	struct pv_list *pvl;
 	int r = 0, consistent = 0;
 
-	if (!(vg = vg_read_internal(fmt->cmd, vg_name, vgid, &consistent))) {
-		log_error("get_pv_from_vg_by_id: vg_read_internal failed to read VG %s",
+	if (!(vg = _vg_read_internal(fmt->cmd, vg_name, vgid, &consistent))) {
+		log_error("get_pv_from_vg_by_id: _vg_read_internal failed to read VG %s",
 			  vg_name);
 		return 0;
 	}
@@ -525,7 +528,7 @@ struct volume_group *vg_create(struct cm
 	struct dm_pool *mem;
 
 	/* is this vg name already in use ? */
-	if ((vg = vg_read_internal(cmd, vg_name, NULL, &consistent))) {
+	if ((vg = _vg_read_internal(cmd, vg_name, NULL, &consistent))) {
 		log_err("A volume group called '%s' already exists.", vg_name);
 		vg_release(vg);
 		return NULL;
@@ -1772,7 +1775,7 @@ static struct volume_group *_vg_read(str
 
 	if (is_orphan_vg(vgname)) {
 		if (use_precommitted) {
-			log_error("Internal error: vg_read_internal requires vgname "
+			log_error("Internal error: _vg_read_internal requires vgname "
 				  "with pre-commit.");
 			return NULL;
 		}
@@ -2058,8 +2061,9 @@ static struct volume_group *_vg_read(str
 	return correct_vg;
 }
 
-struct volume_group *vg_read_internal(struct cmd_context *cmd, const char *vgname,
-			     const char *vgid, int *consistent)
+static struct volume_group *_vg_read_internal(struct cmd_context *cmd,
+					      const char *vgname,
+					      const char *vgid, int *consistent)
 {
 	struct volume_group *vg;
 	struct lv_list *lvl;
@@ -2100,7 +2104,7 @@ void vg_release(struct volume_group *vg)
 
 /* This is only called by lv_from_lvid, which is only called from
  * activate.c so we know the appropriate VG lock is already held and
- * the vg_read_internal is therefore safe.
+ * the _vg_read_internal is therefore safe.
  */
 static struct volume_group *_vg_read_by_vgid(struct cmd_context *cmd,
 					    const char *vgid,
@@ -2133,7 +2137,7 @@ static struct volume_group *_vg_read_by_
 	if (memlock())
 		goto out;
 
-	/* FIXME Need a genuine read by ID here - don't vg_read_internal by name! */
+	/* FIXME Need a genuine read by ID here - don't _vg_read_internal by name! */
 	/* FIXME Disabled vgrenames while active for now because we aren't
 	 *       allowed to do a full scan here any more. */
 
@@ -2324,7 +2328,7 @@ static int _get_pvs(struct cmd_context *
 			stack;
 			continue;
 		}
-		if (!(vg = vg_read_internal(cmd, vgname, vgid, &consistent))) {
+		if (!(vg = _vg_read_internal(cmd, vgname, vgid, &consistent))) {
 			stack;
 			continue;
 		}
@@ -2583,7 +2587,7 @@ static vg_t *_recover_vg(struct cmd_cont
 	if (!lock_vol(cmd, lock_name, lock_flags))
 		return_NULL;
 
-	if (!(vg = vg_read_internal(cmd, vg_name, vgid, &consistent)))
+	if (!(vg = _vg_read_internal(cmd, vg_name, vgid, &consistent)))
 		return_NULL;
 
 	if (!consistent) {
@@ -2642,7 +2646,7 @@ static vg_t *_vg_lock_and_read(struct cm
 	consistent_in = consistent;
 
 	/* If consistent == 1, we get NULL here if correction fails. */
-	if (!(vg = vg_read_internal(cmd, vg_name, vgid, &consistent))) {
+	if (!(vg = _vg_read_internal(cmd, vg_name, vgid, &consistent))) {
 		if (consistent_in && !consistent) {
 			log_error("Volume group \"%s\" inconsistent.", vg_name);
 			failure |= FAILED_INCONSISTENT;
diff -rN -u -p old-temp.31080/lib/metadata/metadata-exported.h new-temp.31080/lib/metadata/metadata-exported.h
--- old-temp.31080/lib/metadata/metadata-exported.h	2009-05-03 13:29:39.590237605 +0200
+++ new-temp.31080/lib/metadata/metadata-exported.h	2009-05-03 13:29:39.638236373 +0200
@@ -360,8 +360,6 @@ struct lv_list {
 int vg_write(struct volume_group *vg);
 int vg_commit(struct volume_group *vg);
 int vg_revert(struct volume_group *vg);
-struct volume_group *vg_read_internal(struct cmd_context *cmd, const char *vg_name,
-			     const char *vgid, int *consistent);
 struct physical_volume *pv_read(struct cmd_context *cmd, const char *pv_name,
 				struct dm_list *mdas, uint64_t *label_sector,
 				int warnings, int scan_label_only);

[PATCH 9/9] All tools now handle inconsistent metadata the same way.

[Petr Rockai]

Sun May  3 13:25:10 CEST 2009  Petr Rockai <me@mornfall.net>
  * All tools now handle inconsistent metadata the same way.
diff -rN -u -p old-temp.31080/test/t-inconsistent-metadata.sh new-temp.31080/test/t-inconsistent-metadata.sh
--- old-temp.31080/test/t-inconsistent-metadata.sh	2009-05-03 13:29:41.594232457 +0200
+++ new-temp.31080/test/t-inconsistent-metadata.sh	2009-05-03 13:29:41.682234353 +0200
@@ -40,23 +40,26 @@ vgscan 2>&1 | tee cmd.out
 not grep "Inconsistent metadata found for VG $vg" cmd.out
 check
 
-# vgdisplay doesn't change anything
+# vgdisplay fixes up metadata
 init
 vgdisplay 2>&1 | tee cmd.out
-grep "Volume group \"$vg\" inconsistent" cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
 vgdisplay 2>&1 | tee cmd.out
-grep "Volume group \"$vg\" inconsistent" cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
+check
 
 # lvs fixes up
 init
 lvs 2>&1 | tee cmd.out
-grep "Inconsistent metadata found for VG $vg - updating" cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
 vgdisplay 2>&1 | tee cmd.out
-not grep "Volume group \"$vg\" inconsistent" cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
 check
 
-# vgs doesn't fix up... (why?)
+# vgs fixes up as well
 init
 vgs 2>&1 | tee cmd.out
-vgdisplay 2>&1 | tee cmd.out
-grep "Volume group \"$vg\" inconsistent" cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
+vgs 2>&1 | tee cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
+check

[PATCH 2/9] Rework the toollib interface (process_each_*) on top of new vg_read.

[Petr Rockai]

Dave Wysochanski <dwysocha@redhat.com> writes:

> Did you miss process_each_pv?  It looks to me like you missed a
> hunk/patch somewhere since process_each_pv still calls the old
> vg_read_internal().

In fact, I have mis-ordered the patches -- I have only sent a prefix to speed
up the process of getting at least those in. Please ignore the "Un-export
vg_read_internal" patch for now, it was supposed to go later in sequence. There
are still a few patches depending on those that need to be rebased, but it
doesn't really make sense to do until at least the groundwork is
in. (Especially since it may, sadly, take yet another rebase before these get
in, as it looks now...)

Yours,
   Petr.

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