LVM2/tools pvchange.c

[wysochanski@sourceware.org <wysochanski@sourceware.org>]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	wysochanski at sourceware.org	2010-05-19 11:53:00

Modified files:
	tools          : pvchange.c 

Log message:
	Update pvchange to always obtain a vg handle for each pv to process.
	
	Earlier patches added some infrastructure to lookup a vgname from
	a pvname.  We now can cleanup some of the pvchange and other code
	by requiring callers that want to modify some pv property:
	1) lookup the vgname by the pvname
	2) use the vgname to obtain a vg handle
	3) get the pv handle from the vg handle
	
	This should work going forward and be a much cleaner interface,
	as we move away from pvs as standalone objects.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/tools/pvchange.c.diff?cvsroot=lvm2&r1=1.73&r2=1.74

--- LVM2/tools/pvchange.c	2009/09/14 22:47:49	1.73
+++ LVM2/tools/pvchange.c	2010/05/19 11:53:00	1.74
@@ -17,13 +17,10 @@
 
 /* FIXME Locking.  PVs in VG. */
 
-static int _pvchange_single(struct cmd_context *cmd, struct physical_volume *pv,
+static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,
+			    struct physical_volume *pv,
 			    void *handle __attribute((unused)))
 {
-	struct volume_group *vg = NULL;
-	const char *vg_name = NULL;
-	struct pv_list *pvl;
-	uint64_t sector;
 	uint32_t orig_pe_alloc_count;
 	/* FIXME Next three only required for format1. */
 	uint32_t orig_pe_count, orig_pe_size;
@@ -53,21 +50,6 @@
 
 	/* If in a VG, must change using volume group. */
 	if (!is_orphan(pv)) {
-		vg_name = pv_vg_name(pv);
-
-		log_verbose("Finding volume group %s of physical volume %s",
-			    vg_name, pv_name);
-		vg = vg_read_for_update(cmd, vg_name, NULL, 0);
-		if (vg_read_error(vg)) {
-			vg_release(vg);
-			return_0;
-		}
-
-		if (!(pvl = find_pv_in_vg(vg, pv_name))) {
-			log_error("Unable to find \"%s\" in volume group \"%s\"",
-				  pv_name, vg->name);
-			goto out;
-		}
 		if (tagarg && !(vg->fid->fmt->features & FMT_TAGS)) {
 			log_error("Volume group containing %s does not "
 				  "support tags", pv_name);
@@ -78,7 +60,6 @@
 				  "logical volumes", pv_name);
 			goto out;
 		}
-		pv = pvl->pv;
 		if (!archive(vg))
 			goto out;
 	} else {
@@ -87,19 +68,6 @@
 				  "in volume group", pv_name);
 			return 0;
 		}
-
-		vg_name = VG_ORPHANS;
-
-		if (!lock_vol(cmd, vg_name, LCK_VG_WRITE)) {
-			log_error("Can't get lock for orphans");
-			return 0;
-		}
-
-		if (!(pv = pv_read(cmd, pv_name, NULL, &sector, 1, 0))) {
-			unlock_vg(cmd, vg_name);
-			log_error("Unable to read PV \"%s\"", pv_name);
-			return 0;
-		}
 	}
 
 	if (arg_count(cmd, allocatable_ARG)) {
@@ -201,7 +169,6 @@
 	log_print("Physical volume \"%s\" changed", pv_name);
 	r = 1;
 out:
-	unlock_and_release_vg(cmd, vg, vg_name);
 	return r;
 
 }
@@ -212,12 +179,12 @@
 	int done = 0;
 	int total = 0;
 
-	struct physical_volume *pv;
-	char *pv_name;
+	struct volume_group *vg;
+	const char *pv_name, *vg_name;
 
 	struct pv_list *pvl;
-	struct dm_list *pvslist;
-	struct dm_list mdas;
+	struct dm_list *vgnames;
+	struct str_list *sll;
 
 	if (arg_count(cmd, allocatable_ARG) + arg_count(cmd, addtag_ARG) +
 	    arg_count(cmd, deltag_ARG) + arg_count(cmd, uuid_ARG) != 1) {
@@ -240,50 +207,71 @@
 		log_verbose("Using physical volume(s) on command line");
 		for (; opt < argc; opt++) {
 			pv_name = argv[opt];
-			dm_list_init(&mdas);
-			if (!(pv = pv_read(cmd, pv_name, &mdas, NULL, 1, 0))) {
+			vg_name = find_vgname_from_pvname(cmd, pv_name);
+			if (!vg_name) {
 				log_error("Failed to read physical volume %s",
 					  pv_name);
 				continue;
 			}
-			/*
-			 * If a PV has no MDAs it may appear to be an
-			 * orphan until the metadata is read off
-			 * another PV in the same VG.  Detecting this
-			 * means checking every VG by scanning every
-			 * PV on the system.
-			 */
-			if (is_orphan(pv) && !dm_list_size(&mdas)) {
-				if (!scan_vgs_for_pvs(cmd)) {
-					log_error("Rescan for PVs without "
-						  "metadata areas failed.");
-					continue;
-				}
-				if (!(pv = pv_read(cmd, pv_name,
-						   NULL, NULL, 1, 0))) {
-					log_error("Failed to read "
-						  "physical volume %s",
-						  pv_name);
-					continue;
-				}
+			vg = vg_read_for_update(cmd, vg_name, NULL, 0);
+			if (vg_read_error(vg)) {
+				vg_release(vg);
+				stack;
+				continue;
+			}
+			pvl = find_pv_in_vg(vg, pv_name);
+			if (!pvl || !pvl->pv) {
+				log_error("Unable to find %s in %s",
+					  pv_name, vg_name);
+				continue;
 			}
 
 			total++;
-			done += _pvchange_single(cmd, pv, NULL);
+			done += _pvchange_single(cmd, vg,
+						 pvl->pv, NULL);
+			/* FIXME: we should be using #orphans_lvm2 everwhere */
+			if (is_orphan_vg(vg->name))
+				vg_name = VG_ORPHANS;
+			unlock_and_release_vg(cmd, vg, vg_name);
 		}
 	} else {
 		log_verbose("Scanning for physical volume names");
-		if (!(pvslist = get_pvs(cmd))) {
-			stack;
+		/* FIXME: share code with toollib */
+		/*
+		 * Take the global lock here so the lvmcache remains
+		 * consistent across orphan/non-orphan vg locks.  If we don't
+		 * take the lock here, pvs with 0 mdas in a non-orphan VG will
+		 * be processed twice.
+		 */
+		if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_WRITE)) {
+			log_error("Unable to obtain global lock.");
 			return ECMD_FAILED;
 		}
 
-		dm_list_iterate_items(pvl, pvslist) {
-			total++;
-			done += _pvchange_single(cmd, pvl->pv, NULL);
+		if ((vgnames = get_vgnames(cmd, 1)) &&
+		    !dm_list_empty(vgnames)) {
+			dm_list_iterate_items(sll, vgnames) {
+				vg = vg_read_for_update(cmd, sll->str, NULL, 0);
+				if (vg_read_error(vg)) {
+					vg_release(vg);
+					stack;
+					continue;
+				}
+				dm_list_iterate_items(pvl, &vg->pvs) {
+					total++;
+					done += _pvchange_single(cmd, vg,
+								 pvl->pv,
+								 NULL);
+				}
+				/* FIXME: we should be using #orphans_lvm2 everwhere */
+				if (is_orphan_vg(vg->name))
+					sll->str = VG_ORPHANS;
+				unlock_and_release_vg(cmd, vg, sll->str);
+			}
 		}
 	}
 
+	unlock_vg(cmd, VG_GLOBAL);
 	log_print("%d physical volume%s changed / %d physical volume%s "
 		  "not changed",
 		  done, done == 1 ? "" : "s",