[PATCH 1/9] metadata: Do not try to check vg_name of a NULL PV.

[PATCH 1/9] metadata: Do not try to check vg_name of a NULL PV.

[Petr Rockai]

---
 lib/metadata/metadata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 8a98aa6..a4a2aaa 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -1842,7 +1842,7 @@ struct physical_volume *find_pv_by_name(struct cmd_context *cmd,
 		}
 	}
 
-	if (!allow_orphan && is_orphan_vg(pv->vg_name)) {
+	if (pv && !allow_orphan && is_orphan_vg(pv->vg_name)) {
 		log_error("Physical volume %s not in a volume group", pv_name);
 		goto bad;
 	}
-- 
1.8.2

[PATCH 2/9] toollib: Drop the bogus pv_read optimisation.

[Petr Rockai]

The toollib code for processing PVs given on commandline tried to be smart and
use pv_read, avoiding real metadata scanning. The premise of the optimisation
was that if there is an active MDA on the PV, it can be reliably used to
establish VG membership. This assumption is however wrong, since it's entirely
possible that this particular MDA is simply out of date, and the PV is not
actually in any VG. While this is a subtle and rare bug, it's still a bug. One
way to trigger it would be to hotplug an old disk which at some point was a PV
in an existing VG, having a seemingly valid MDA but with an old sequence
number. All commands that use normal scanning realize that this PV does not
belong into the VG anymore, but anything based on pv_read alone will believe it
does.
---
 tools/toollib.c | 91 ++++++++++++++++++++++++++-------------------------------
 1 file changed, 42 insertions(+), 49 deletions(-)

diff --git a/tools/toollib.c b/tools/toollib.c
index b238a61..5ce40b4 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -646,6 +646,8 @@ int process_each_pv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
 static int _process_all_devs(struct cmd_context *cmd, void *handle,
 			     process_single_pv_fn_t process_single_pv)
 {
+	struct pv_list *pvl;
+	struct dm_list *pvslist;
 	struct physical_volume *pv;
 	struct physical_volume pv_dummy;
 	struct dev_iter *iter;
@@ -654,7 +656,8 @@ static int _process_all_devs(struct cmd_context *cmd, void *handle,
 	int ret_max = ECMD_PROCESSED;
 	int ret = 0;
 
-	if (!scan_vgs_for_pvs(cmd, 1)) {
+	lvmcache_seed_infos_from_lvmetad(cmd);
+	if (!(pvslist = get_pvs(cmd))) {
 		stack;
 		return ECMD_FAILED;
 	}
@@ -664,14 +667,19 @@ static int _process_all_devs(struct cmd_context *cmd, void *handle,
 		return ECMD_FAILED;
 	}
 
-	while ((dev = dev_iter_get(iter))) {
-		if (!(pv = pv_read(cmd, dev_name(dev), 0, 0))) {
-			memset(&pv_dummy, 0, sizeof(pv_dummy));
-			dm_list_init(&pv_dummy.tags);
-			dm_list_init(&pv_dummy.segments);
-			pv_dummy.dev = dev;
-			pv = &pv_dummy;
-		}
+	while ((dev = dev_iter_get(iter)))
+	{
+		memset(&pv_dummy, 0, sizeof(pv_dummy));
+		dm_list_init(&pv_dummy.tags);
+		dm_list_init(&pv_dummy.segments);
+		pv_dummy.dev = dev;
+		pv = &pv_dummy;
+
+		/* TODO use a device-indexed hash here */
+		dm_list_iterate_items(pvl, pvslist)
+			if (pvl->pv->dev == dev)
+				pv = pvl->pv;
+
 		ret = process_single_pv(cmd, NULL, pv, handle);
 
 		free_pv_fid(pv);
@@ -704,11 +712,11 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
 
 	struct pv_list *pvl;
 	struct physical_volume *pv;
-	struct dm_list *pvslist, *vgnames;
+	struct dm_list *pvslist = NULL, *vgnames;
 	struct dm_list tags;
 	struct str_list *sll;
 	char *at_sign, *tagname;
-	int scanned = 0;
+	struct device *dev;
 
 	dm_list_init(&tags);
 
@@ -750,52 +758,34 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
 				}
 				pv = pvl->pv;
 			} else {
-				if (!(pv = pv_read(cmd, argv[opt],
-						   1, scan_label_only))) {
-					log_error("Failed to read physical "
-						  "volume \"%s\"", argv[opt]);
+				if (!pvslist) {
+					lvmcache_seed_infos_from_lvmetad(cmd);
+					if (!(pvslist = get_pvs(cmd)))
+						goto bad;
+				}
+
+				if (!(dev = dev_cache_get(argv[opt], cmd->filter))) {
+					log_error("Failed to find device "
+						  "\"%s\"", argv[opt]);
 					ret_max = ECMD_FAILED;
 					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 (!scanned && is_orphan(pv) &&
-				    !dm_list_size(&pv->fid->metadata_areas_in_use)) {
-					if (!scan_label_only &&
-					    !scan_vgs_for_pvs(cmd, 1)) {
-						stack;
-						ret_max = ECMD_FAILED;
-						continue;
-					}
-					scanned = 1;
-					free_pv_fid(pv);
-					if (!(pv = pv_read(cmd, argv[opt],
-							   1,
-							   scan_label_only))) {
-						log_error("Failed to read "
-							  "physical volume "
-							  "\"%s\"", argv[opt]);
-						ret_max = ECMD_FAILED;
-						continue;
-					}
+				pv = NULL;
+				dm_list_iterate_items(pvl, pvslist)
+					if (pvl->pv->dev == dev)
+						pv = pvl->pv;
+
+				if (!pv) {
+					log_error("Failed to find physical volume "
+						  "\"%s\"", argv[opt]);
+					ret_max = ECMD_FAILED;
+					continue;
 				}
 			}
 
 			ret = process_single_pv(cmd, vg, pv, handle);
 
-			/*
-			 * Free PV only if we called pv_read before,
-			 * otherwise the PV structure is part of the VG.
-			 */
-			if (!vg)
-				free_pv_fid(pv);
-
 			if (ret > ret_max)
 				ret_max = ret;
 			if (sigint_caught())
@@ -850,7 +840,6 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
 			dm_list_iterate_items(pvl, pvslist) {
 				ret = process_single_pv(cmd, NULL, pvl->pv,
 						     handle);
-				free_pv_fid(pvl->pv);
 				if (ret > ret_max)
 					ret_max = ret;
 				if (sigint_caught())
@@ -859,6 +848,10 @@ int process_each_pv(struct cmd_context *cmd, int argc, char **argv,
 		}
 	}
 out:
+	if (pvslist)
+		dm_list_iterate_items(pvl, pvslist)
+			free_pv_fid(pvl->pv);
+
 	if (lock_global)
 		unlock_vg(cmd, VG_GLOBAL);
 	return ret_max;
-- 
1.8.2

[PATCH 3/9] test: Add a test for the failing pv_read optimisation.

[Petr Rockai]

---
 test/shell/pvread-bogosity.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 test/shell/pvread-bogosity.sh

diff --git a/test/shell/pvread-bogosity.sh b/test/shell/pvread-bogosity.sh
new file mode 100644
index 0000000..d47eb8a
--- /dev/null
+++ b/test/shell/pvread-bogosity.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+# Copyright (C) 2013 Red Hat, Inc. All rights reserved.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+. lib/test
+
+aux prepare_devs 3
+
+vgcreate --metadatasize 128k $vg1 "$dev1" "$dev2" "$dev3"
+
+vgreduce $vg1 $dev1
+dd if="$dev1" of=badmda bs=256K count=1
+vgextend $vg1 $dev1
+
+dd if=badmda of="$dev1" bs=256K count=1
+
+# dev1 is part of vg1 (as witnessed by metadata on dev2 and dev3), but its mda
+# was corrupt (written over by a backup from time dev1 was an orphan)
+check pv_field $dev1 vg_name $vg1
-- 
1.8.2

[PATCH 4/9] pvresize: Do not use pv_read (get the PV from orphan VG).

[Petr Rockai]

---
 lib/metadata/pv_manip.c |  2 +-
 tools/pvresize.c        | 45 +++++++++++++++------------------------------
 2 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index dea49f8..6dacf24 100644
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -513,7 +513,7 @@ int pv_resize(struct physical_volume *pv,
 	/* pv->pe_count is 0 now! We need to recalculate! */
 
 	/* If there's a VG, calculate new PE count value. */
-	if (vg) {
+	if (vg && !is_orphan_vg(vg->name)) {
 		/* FIXME: Maybe PE calculation should go into pv->fmt->resize?
 		          (like it is for pv->fmt->setup) */
 		if (!(new_pe_count = pv_size(pv) / vg->extent_size)) {
diff --git a/tools/pvresize.c b/tools/pvresize.c
index 26959e8..0a3d55a 100644
--- a/tools/pvresize.c
+++ b/tools/pvresize.c
@@ -37,38 +37,25 @@ static int _pv_resize_single(struct cmd_context *cmd,
 	struct volume_group *old_vg = vg;
 	int vg_needs_pv_write = 0;
 
-	if (is_orphan_vg(vg_name)) {
-		if (!lock_vol(cmd, vg_name, LCK_VG_WRITE, NULL)) {
-			log_error("Can't get lock for orphans");
-			return 0;
-		}
+	vg = vg_read_for_update(cmd, vg_name, NULL, 0);
 
-		if (!(pv = pv_read(cmd, pv_name, 1, 0))) {
-			unlock_vg(cmd, vg_name);
-			log_error("Unable to read PV \"%s\"", pv_name);
-			return 0;
-		}
-	} else {
-		vg = vg_read_for_update(cmd, vg_name, NULL, 0);
-
-		if (vg_read_error(vg)) {
-			release_vg(vg);
-			log_error("Unable to read volume group \"%s\".",
-				  vg_name);
-			return 0;
-		}
+	if (vg_read_error(vg)) {
+		release_vg(vg);
+		log_error("Unable to read volume group \"%s\".",
+			  vg_name);
+		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 (!(pvl = find_pv_in_vg(vg, pv_name))) {
+		log_error("Unable to find \"%s\" in volume group \"%s\"",
+			  pv_name, vg->name);
+		goto out;
+	}
 
-		pv = pvl->pv;
+	pv = pvl->pv;
 
-		if (!archive(vg))
-			goto out;
-	}
+	if (!archive(vg))
+		goto out;
 
 	if (!(pv->fmt->features & FMT_RESIZE_PV)) {
 		log_error("Physical volume %s format does not support resizing.",
@@ -126,8 +113,6 @@ out:
 		log_error("Use pvcreate and vgcfgrestore "
 			  "to repair from archived metadata.");
 	unlock_vg(cmd, vg_name);
-	if (is_orphan_vg(vg_name))
-		free_pv_fid(pv);
 	if (!old_vg)
 		release_vg(vg);
 	return r;
-- 
1.8.2

[PATCH 5/9] test: Fix fallout from pv_read changes.

[Petr Rockai]

---
 test/shell/pv-duplicate.sh          |  2 +-
 test/shell/pvcreate-operation-md.sh | 35 +++++++++++++++++++++--------------
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/test/shell/pv-duplicate.sh b/test/shell/pv-duplicate.sh
index 6b9f309..40d1f68 100644
--- a/test/shell/pv-duplicate.sh
+++ b/test/shell/pv-duplicate.sh
@@ -21,5 +21,5 @@ vgcreate --metadatasize 128k $vg1 "$dev1"
 dd if="$dev1" of="$dev2" bs=256K count=1
 dd if="$dev1" of="$dev3" bs=256K count=1
 
-pvs "$dev1"
+pvs "$dev3"
 vgs $vg1
diff --git a/test/shell/pvcreate-operation-md.sh b/test/shell/pvcreate-operation-md.sh
index 5e2a28d..9cad745 100644
--- a/test/shell/pvcreate-operation-md.sh
+++ b/test/shell/pvcreate-operation-md.sh
@@ -58,21 +58,24 @@ test -b "$mddev" && skip
 mdadm --create --metadata=1.0 "$mddev" --auto=md --level 0 --raid-devices=2 --chunk 64 "$dev1" "$dev2"
 trap 'cleanup_md_and_teardown' EXIT # cleanup this MD device at the end of the test
 test -b "$mddev" || skip
+cp -LR "$mddev" "$DM_DEV_DIR" # so that LVM/DM can see the device
+lvmdev="$DM_DEV_DIR/md_lvm_test0"
 
 # Test alignment of PV on MD without any MD-aware or topology-aware detection
 # - should treat $mddev just like any other block device
 pv_align="1.00m"
 pvcreate --metadatasize 128k \
     --config 'devices {md_chunk_alignment=0 data_alignment_detection=0 data_alignment_offset_detection=0}' \
-    "$mddev"
-check pv_field "$mddev" pe_start $pv_align
+    "$lvmdev"
+
+check pv_field "$lvmdev" pe_start $pv_align
 
 # Test md_chunk_alignment independent of topology-aware detection
 pv_align="1.00m"
 pvcreate --metadatasize 128k \
     --config 'devices {data_alignment_detection=0 data_alignment_offset_detection=0}' \
-    "$mddev"
-check pv_field "$mddev" pe_start $pv_align
+    "$lvmdev"
+check pv_field "$lvmdev" pe_start $pv_align
 
 
 # Test newer topology-aware alignment detection
@@ -81,8 +84,8 @@ if kernel_at_least 2 6 33 ; then
     pv_align="1.00m"
     # optimal_io_size=131072, minimum_io_size=65536
     pvcreate --metadatasize 128k \
-	--config 'devices { md_chunk_alignment=0 }' "$mddev"
-    check pv_field "$mddev" pe_start $pv_align
+	--config 'devices { md_chunk_alignment=0 }' "$lvmdev"
+    check pv_field "$lvmdev" pe_start $pv_align
 fi
 
 # partition MD array directly, depends on blkext in Linux >= 2.6.28
@@ -91,9 +94,10 @@ if kernel_at_least 2 6 28 ; then
     sfdisk "$mddev" <<EOF
 ,,83
 EOF
+    pvscan
     # make sure partition on MD is _not_ removed
     # - tests partition -> parent lookup via sysfs paths
-    not pvcreate --metadatasize 128k "$mddev"
+    not pvcreate --metadatasize 128k "$lvmdev"
 
     # verify alignment_offset is accounted for in pe_start
     # - topology infrastructure is available in Linux >= 2.6.31
@@ -109,6 +113,8 @@ EOF
     # wait here for created device node on tmpfs
     aux udev_wait "$mddev_p"
     test -b "$mddev_p" || skip
+    cp -LR "$mddev_p" "$DM_DEV_DIR"
+    lvmdev_p="$DM_DEV_DIR/$base_mddev_p"
 
     # Checking for 'alignment_offset' in sysfs implies Linux >= 2.6.31
     # but reliable alignment_offset support requires kernel.org Linux >= 2.6.33
@@ -120,9 +126,9 @@ EOF
     if [ $alignment_offset -gt 0 ]; then
         # default alignment is 1M, add alignment_offset
 	pv_align=$((1048576+$alignment_offset))B
-	pvcreate --metadatasize 128k "$mddev_p"
-	check pv_field "$mddev_p" pe_start $pv_align --units b
-	pvremove "$mddev_p"
+	pvcreate --metadatasize 128k "$lvmdev_p"
+	check pv_field "$lvmdev_p" pe_start $pv_align --units b
+	pvremove "$lvmdev_p"
     fi
 fi
 
@@ -140,12 +146,13 @@ if kernel_at_least 2 6 33 ; then
     # optimal_io_size=2097152, minimum_io_size=1048576
     pv_align="2.00m"
     pvcreate --metadatasize 128k \
-	--config 'devices { md_chunk_alignment=0 }' "$mddev"
-    check pv_field "$mddev" pe_start $pv_align
+	--config 'devices { md_chunk_alignment=0 }' "$lvmdev"
+    pvscan # Something is seriously broken.
+    check pv_field "$lvmdev" pe_start $pv_align
 
     # now verify pe_start alignment override using --dataalignment
     pv_align="192.00k"
     pvcreate --dataalignment 64k --metadatasize 128k \
-	--config 'devices { md_chunk_alignment=0 }' "$mddev"
-    check pv_field "$mddev" pe_start $pv_align
+	--config 'devices { md_chunk_alignment=0 }' "$lvmdev"
+    check pv_field "$lvmdev" pe_start $pv_align
 fi
-- 
1.8.2

[PATCH 6/9] pvremove: Avoid using pv_read in favour of scanning.

[Petr Rockai]

---
 tools/pvremove.c | 69 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/tools/pvremove.c b/tools/pvremove.c
index 2c8f5c6..1d5fe18 100644
--- a/tools/pvremove.c
+++ b/tools/pvremove.c
@@ -21,52 +21,53 @@
  */
 static int pvremove_check(struct cmd_context *cmd, const char *name)
 {
-	struct physical_volume *pv;
+	struct device *dev;
+	struct label *label;
+	struct pv_list *pvl;
+	struct dm_list *pvslist;
+
+	struct physical_volume *pv = NULL;
+	int r = 0;
 
 	/* FIXME Check partition type is LVM unless --force is given */
 
+	if (!(dev = dev_cache_get(name, cmd->filter))) {
+		log_error("Device %s not found", name);
+		return 0;
+	}
+
 	/* Is there a pv here already? */
 	/* If not, this is an error unless you used -f. */
-	if (!(pv = pv_read(cmd, name, 1, 0))) {
+	if (!label_read(dev, &label, 0)) {
 		if (arg_count(cmd, force_ARG))
 			return 1;
-		log_error("Physical Volume %s not found", name);
+		log_error("No PV label found on %s.", name);
 		return 0;
 	}
 
-	/*
-	 * 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(&pv->fid->metadata_areas_in_use) &&
-	    !dm_list_size(&pv->fid->metadata_areas_ignored)) {
-		if (!scan_vgs_for_pvs(cmd, 0)) {
-			log_error("Rescan for PVs without metadata areas "
-				  "failed.");
-			goto bad;
-		}
-		free_pv_fid(pv);
-		if (!(pv = pv_read(cmd, name, 1, 0))) {
-			log_error("Failed to read physical volume %s", name);
-			goto bad;
-		}
+	lvmcache_seed_infos_from_lvmetad(cmd);
+	if (!(pvslist = get_pvs(cmd)))
+		return_0;
+
+	dm_list_iterate_items(pvl, pvslist)
+		if (pvl->pv->dev == dev)
+			pv = pvl->pv;
+
+	if (!pv) {
+		log_error("Physical Volume %s not found through scanning.", name);
+		goto out; /* better safe than sorry */
 	}
 
-	/* orphan ? */
 	if (is_orphan(pv)) {
-		free_pv_fid(pv);
-		return 1;
+		r = 1;
+		goto out;
 	}
 
-	/* Allow partial & exported VGs to be destroyed. */
 	/* we must have -ff to overwrite a non orphan */
 	if (arg_count(cmd, force_ARG) < 2) {
 		log_error("PV %s belongs to Volume Group %s so please use vgreduce first.", name, pv_vg_name(pv));
 		log_error("(If you are certain you need pvremove, then confirm by using --force twice.)");
-		goto bad;
+		goto out;
 	}
 
 	/* prompt */
@@ -75,7 +76,7 @@ static int pvremove_check(struct cmd_context *cmd, const char *name)
 			  "of volume group \"%s\" [y/n]? ",
 			  name, pv_vg_name(pv)) == 'n') {
 		log_error("%s: physical volume label not removed", name);
-		goto bad;
+		goto out;
 	}
 
 	if (arg_count(cmd, force_ARG)) {
@@ -86,12 +87,12 @@ static int pvremove_check(struct cmd_context *cmd, const char *name)
 			  !is_orphan(pv) ? "\"" : "");
 	}
 
-	free_pv_fid(pv);
-	return 1;
-
-bad:
-	free_pv_fid(pv);
-	return 0;
+	r = 1;
+out:
+	if (pvslist)
+		dm_list_iterate_items(pvl, pvslist)
+			free_pv_fid(pvl->pv);
+	return r;
 }
 
 static int pvremove_single(struct cmd_context *cmd, const char *pv_name,
-- 
1.8.2

[PATCH 7/9] metadata: Avoid pv_read in find_pv_by_name.

[Petr Rockai]

---
 lib/metadata/metadata.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index a4a2aaa..f4942a5 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -1328,7 +1328,7 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
 	/* FIXME Check partition type is LVM unless --force is given */
 
 	/* Is there a pv here already? */
-	if (!(pv = pv_read(cmd, name, 0, 0)))
+	if (!(pv = find_pv_by_name(cmd, name, 1)))
 		stack;
 
 	/*
@@ -1824,23 +1824,29 @@ struct physical_volume *find_pv_by_name(struct cmd_context *cmd,
 					const char *pv_name,
 					int allow_orphan)
 {
-	struct physical_volume *pv;
+	struct device *dev;
+	struct pv_list *pvl;
+	struct dm_list *pvslist;
+	struct physical_volume *pv = NULL;
+
+	lvmcache_seed_infos_from_lvmetad(cmd);
 
-	if (!(pv = _pv_read(cmd, cmd->mem, pv_name, NULL, 1, 0))) {
+	if (!(dev = dev_cache_get(pv_name, cmd->filter))) {
 		log_error("Physical volume %s not found", pv_name);
-		goto bad;
+		return_NULL;
 	}
 
-	if (is_orphan_vg(pv->vg_name) && !dm_list_size(&pv->fid->metadata_areas_in_use)) {
-		/* If a PV has no MDAs - need to search all VGs for it */
-		if (!scan_vgs_for_pvs(cmd, 1))
-			goto_bad;
-		free_pv_fid(pv);
-		if (!(pv = _pv_read(cmd, cmd->mem, pv_name, NULL, 1, 0))) {
-			log_error("Physical volume %s not found", pv_name);
-			goto bad;
-		}
-	}
+	if (!(pvslist = get_pvs(cmd)))
+		return_NULL;
+
+	dm_list_iterate_items(pvl, pvslist)
+		if (pvl->pv->dev == dev)
+			pv = pvl->pv;
+		else
+			free_pv_fid(pvl->pv);
+
+	if (!pv)
+		log_error("Physical volume %s not found", pv_name);
 
 	if (pv && !allow_orphan && is_orphan_vg(pv->vg_name)) {
 		log_error("Physical volume %s not in a volume group", pv_name);
-- 
1.8.2

[PATCH 8/9] metadata: Fix handling of orphan PV linking & re-linking.

[Petr Rockai]

---
 lib/metadata/metadata.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index f4942a5..f28b067 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -269,6 +269,12 @@ int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
 	vg->extent_count += pv->pe_count;
 	vg->free_count += pv->pe_count;
 
+	dm_list_iterate_items(pvl, &fid->fmt->orphan_vg->pvs)
+		if (pv == pvl->pv) { /* unlink from orphan */
+			dm_list_del(&pvl->list);
+			break;
+		}
+
 	if (pv->status & UNLABELLED_PV) {
 		if (!(pvc = dm_pool_zalloc(mem, sizeof(*pvc)))) {
 			log_error("pv_to_create allocation for '%s' failed", pv_name);
@@ -2789,8 +2795,10 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
 	struct lvmcache_vginfo *vginfo;
 	struct volume_group *vg = NULL;
 	struct _vg_read_orphan_baton baton;
-	struct pv_list *pvl;
+	struct pv_list *pvl, *pvl_;
+	struct pv_list head;
 
+	dm_list_init(&head.list);
 	lvmcache_label_scan(cmd, 0);
 	lvmcache_seed_infos_from_lvmetad(cmd);
 
@@ -2801,14 +2809,31 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
 		return_NULL;
 
 	vg = fmt->orphan_vg;
-	dm_list_iterate_items(pvl, &vg->pvs)
-		pv_set_fid(pvl->pv, NULL);
+restart:
+	dm_list_iterate_items(pvl, &vg->pvs) {
+		if (pvl->pv->status & UNLABELLED_PV ) {
+			dm_list_del(&pvl->list);
+			dm_list_add(&head.list, &pvl->list);
+			goto restart;
+		} else
+			pv_set_fid(pvl->pv, NULL);
+	}
 	dm_list_init(&vg->pvs);
 	vg->pv_count = 0;
+	vg->extent_count = 0;
+	vg->free_count = 0;
 
 	baton.warnings = warnings;
 	baton.vg = vg;
 
+	while (!dm_list_empty(&head.list)) {
+		pvl = (struct pv_list *) dm_list_first(&head.list);
+		dm_list_del(&pvl->list);
+		add_pvl_to_vgs(vg, pvl);
+		vg->extent_count += pvl->pv->pe_count;
+		vg->free_count += pvl->pv->pe_count;
+	}
+
 	if (!lvmcache_foreach_pv(vginfo, _vg_read_orphan_pv, &baton))
 		return_NULL;
 
-- 
1.8.2

[PATCH 9/9] metadata: Nuke the exported "pv_read" function.

[Petr Rockai]

---
 lib/metadata/metadata-exported.h |  3 ---
 lib/metadata/metadata.c          | 36 ------------------------------------
 2 files changed, 39 deletions(-)

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 10aea9e..b08ef7c 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -438,9 +438,6 @@ int vg_commit(struct volume_group *vg);
 void vg_revert(struct volume_group *vg);
 struct volume_group *vg_read_internal(struct cmd_context *cmd, const char *vg_name,
 				      const char *vgid, int warnings, int *consistent);
-struct physical_volume *pv_read(struct cmd_context *cmd, const char *pv_name,
-				int warnings,
-				int scan_label_only);
 struct dm_list *get_pvs(struct cmd_context *cmd);
 
 /*
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index f28b067..0690903 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -1337,20 +1337,6 @@ static int pvcreate_check(struct cmd_context *cmd, const char *name,
 	if (!(pv = find_pv_by_name(cmd, name, 1)))
 		stack;
 
-	/*
-	 * 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 (pv && is_orphan(pv) && !dm_list_size(&pv->fid->metadata_areas_in_use)) {
-		free_pv_fid(pv);
-		if (!scan_vgs_for_pvs(cmd, 0))
-			return_0;
-		if (!(pv = pv_read(cmd, name, 0, 0)))
-			stack;
-	}
-
 	/* Allow partial & exported VGs to be destroyed. */
 	/* We must have -ff to overwrite a non orphan */
 	if (pv && !is_orphan(pv) && pp->force != DONT_PROMPT_OVERRIDE) {
@@ -3607,28 +3593,6 @@ const char *find_vgname_from_pvname(struct cmd_context *cmd,
 	return find_vgname_from_pvid(cmd, pvid);
 }
 
-/**
- * pv_read - read and return a handle to a physical volume
- * @cmd: LVM command initiating the pv_read
- * @pv_name: full device name of the PV, including the path
- * @mdas: list of metadata areas of the PV
- * @label_sector: sector number where the PV label is stored on @pv_name
- * @warnings:
- *
- * Returns:
- *   PV handle - valid pv_name and successful read of the PV, or
- *   NULL - invalid parameter or error in reading the PV
- *
- * Note:
- *   FIXME - liblvm todo - make into function that returns handle
- */
-struct physical_volume *pv_read(struct cmd_context *cmd, const char *pv_name,
-				int warnings,
-				int scan_label_only)
-{
-	return _pv_read(cmd, cmd->mem, pv_name, NULL, warnings, scan_label_only);
-}
-
 /* FIXME Use label functions instead of PV functions */
 static struct physical_volume *_pv_read(struct cmd_context *cmd,
 					struct dm_pool *pvmem,
-- 
1.8.2