Merge remote-tracking branch 'origin/master'

* origin/master: (22 commits)
  tests: add metadata-bad-mdaheader.sh
  tests: add metadata-bad-text.sh
  tests: add outdated-pv.sh
  tests: add metadata-old.sh
  tests: add missing-pv missing-pv-unused
  metadata.c: removed unused code
  improve reading and repairing vg metadata
  add a warning message when updating old metadata
  vgcfgbackup add error messages
  vgck --updatemetadata is a new command
  move pv header repairs to vg_write
  process_each_pv handle outdated pvs
  move wipe_outdated_pvs to vg_write
  create separate lvmcache update functions for read and write
  fix vg_commit return value
  change args for text label read function
  add mda arg to add_mda
  keep track of which mdas have old metadata in lvmcache
  ability to keep track of outdated pvs in lvmcache
  ability to keep track of bad mdas in lvmcache
  ...

lib/cache/lvmcache.c

@@ -31,6 +31,7 @@ struct lvmcache_info {
 	struct dm_list mdas;	/* list head for metadata areas */
 	struct dm_list das;	/* list head for data areas */
 	struct dm_list bas;	/* list head for bootloader areas */
+	struct dm_list bad_mdas;/* list head for bad metadata areas */
 	struct lvmcache_vginfo *vginfo;	/* NULL == unknown */
 	struct label *label;
 	const struct format_type *fmt;
@@ -39,12 +40,19 @@ struct lvmcache_info {
 	uint32_t ext_version;   /* Extension version */
 	uint32_t ext_flags;	/* Extension flags */
 	uint32_t status;
+	bool mda1_bad;		/* label scan found bad metadata in mda1 */
+	bool mda2_bad;		/* label scan found bad metadata in mda2 */
+	bool summary_seqno_mismatch; /* two mdas on this dev has mismatching metadata */
+	int summary_seqno;      /* vg seqno found on this dev during scan */
+	int mda1_seqno;
+	int mda2_seqno;
 };
 
 /* One per VG */
 struct lvmcache_vginfo {
 	struct dm_list list;	/* Join these vginfos together */
 	struct dm_list infos;	/* List head for lvmcache_infos */
+	struct dm_list outdated_infos; /* vg_read moves info from infos to outdated_infos */
 	const struct format_type *fmt;
 	char *vgname;		/* "" == orphan */
 	uint32_t status;
@@ -57,7 +65,7 @@ struct lvmcache_vginfo {
 	uint32_t mda_checksum;
 	size_t mda_size;
 	int seqno;
-	int scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */
+	bool scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */
 };
 
 static struct dm_hash_table *_pvid_hash = NULL;
@@ -175,6 +183,54 @@ static void _destroy_duplicate_device_list(struct dm_list *head)
 	dm_list_init(head);
 }
 
+bool lvmcache_has_bad_metadata(struct device *dev)
+{
+	struct lvmcache_info *info;
+
+	if (!(info = lvmcache_info_from_pvid(dev->pvid, dev, 0))) {
+		/* shouldn't happen */
+		log_error("No lvmcache info for checking bad metadata on %s", dev_name(dev));
+		return false;
+	}
+
+	if (info->mda1_bad || info->mda2_bad)
+		return true;
+	return false;
+}
+
+void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda)
+{
+	if (mda->mda_num == 1)
+		info->mda1_bad = true;
+	else if (mda->mda_num == 2)
+		info->mda2_bad = true;
+	dm_list_add(&info->bad_mdas, &mda->list);
+}
+
+void lvmcache_get_bad_mdas(struct cmd_context *cmd,
+			   const char *vgname, const char *vgid,
+                           struct dm_list *bad_mda_list)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+	struct mda_list *mdal;
+	struct metadata_area *mda, *mda2;
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
+		log_error(INTERNAL_ERROR "lvmcache_get_bad_mdas no vginfo %s", vgname);
+		return;
+	}
+
+	dm_list_iterate_items(info, &vginfo->infos) {
+		dm_list_iterate_items_safe(mda, mda2, &info->bad_mdas) {
+			if (!(mdal = zalloc(sizeof(*mdal))))
+				continue;
+			mdal->mda = mda;
+			dm_list_add(bad_mda_list, &mdal->list);
+		}
+	}
+}
+
 static void _vginfo_attach_info(struct lvmcache_vginfo *vginfo,
 				struct lvmcache_info *info)
 {
@@ -555,8 +611,11 @@ next:
 	 */
 
 	if (!(info = lvmcache_info_from_pvid(alt->dev->pvid, NULL, 0))) {
-		/* This shouldn't happen */
+		/*
-		log_warn("WARNING: PV %s on duplicate device %s not found in cache.",
+		 * This will happen if a duplicate dev has been dropped already,
+		 * e.g. it was found to be an md component.
+		 */
+		log_debug("PVID %s on duplicate device %s not found in cache.",
 			 alt->dev->pvid, dev_name(alt->dev));
 		goto next;
 	}
@@ -1335,6 +1394,7 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
 			return 0;
 		}
 		dm_list_init(&vginfo->infos);
+		dm_list_init(&vginfo->outdated_infos);
 
 		/*
 		 * A different VG (different uuid) can exist with the same name.
@@ -1459,12 +1519,9 @@ int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt)
 }
 
 /*
- * FIXME: get rid of other callers of this function which call it
+ * Returning 0 causes the caller to remove the info struct for this
- * in odd cases to "fix up" some bit of lvmcache state.  Make those
+ * device from lvmcache, which will make it look like a missing device.
- * callers fix up what they need to directly, and leave this function
- * with one purpose and caller.
  */
-
 int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vgsummary *vgsummary)
 {
 	const char *vgname = vgsummary->vgname;
@@ -1490,6 +1547,7 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	 * Puts the vginfo into the vgname hash table.
 	 */
 	if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus, vgsummary->creation_host, info->fmt)) {
+		/* shouldn't happen, internal error */
 		log_error("Failed to update VG %s info in lvmcache.", vgname);
 		return 0;
 	}
@@ -1498,6 +1556,7 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	 * Puts the vginfo into the vgid hash table.
 	 */
 	if (!_lvmcache_update_vgid(info, info->vginfo, vgid)) {
+		/* shouldn't happen, internal error */
 		log_error("Failed to update VG %s info in lvmcache.", vgname);
 		return 0;
 	}
@@ -1513,56 +1572,140 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	if (!vgsummary->seqno && !vgsummary->mda_size && !vgsummary->mda_checksum)
 		return 1;
 
+	/*
+	 * Keep track of which devs/mdas have old versions of the metadata.
+	 * The values we keep in vginfo are from the metadata with the largest
+	 * seqno.  One dev may have more recent metadata than another dev, and
+	 * one mda may have more recent metadata than the other mda on the same
+	 * device.
+	 *
+	 * When a device holds old metadata, the info struct for the device
+	 * remains in lvmcache, so the device is not treated as missing.
+	 * Also the mda struct containing the old metadata is kept on
+	 * info->mdas.  This means that vg_read will read metadata from
+	 * the mda again (and probably see the same old metadata).  It
+	 * also means that vg_write will use the mda to write new metadata
+	 * into the mda that currently has the old metadata.
+	 */
+	if (vgsummary->mda_num == 1)
+		info->mda1_seqno = vgsummary->seqno;
+	else if (vgsummary->mda_num == 2)
+		info->mda2_seqno = vgsummary->seqno;
+
+	if (!info->summary_seqno)
+		info->summary_seqno = vgsummary->seqno;
+	else {
+		if (info->summary_seqno == vgsummary->seqno) {
+			/* This mda has the same metadata as the prev mda on this dev. */
+			return 1;
+
+		} else if (info->summary_seqno > vgsummary->seqno) {
+			/* This mda has older metadata than the prev mda on this dev. */
+			info->summary_seqno_mismatch = true;
+
+		} else if (info->summary_seqno < vgsummary->seqno) {
+			/* This mda has newer metadata than the prev mda on this dev. */
+			info->summary_seqno_mismatch = true;
+			info->summary_seqno = vgsummary->seqno;
+		}
+	}
+
+	/* this shouldn't happen */
 	if (!(vginfo = info->vginfo))
 		return 1;
 
 	if (!vginfo->seqno) {
 		vginfo->seqno = vgsummary->seqno;
-
-		log_debug_cache("lvmcache %s: VG %s: set seqno to %d",
-				dev_name(info->dev), vginfo->vgname, vginfo->seqno);
-
-	} else if (vgsummary->seqno != vginfo->seqno) {
-		log_warn("Scan of VG %s from %s found metadata seqno %d vs previous %d.",
-			 vgname, dev_name(info->dev), vgsummary->seqno, vginfo->seqno);
-		vginfo->scan_summary_mismatch = 1;
-		/* If we don't return success, this dev info will be removed from lvmcache,
-		   and then we won't be able to rescan it or repair it. */
-		return 1;
-	}
-
-	if (!vginfo->mda_size) {
 		vginfo->mda_checksum = vgsummary->mda_checksum;
 		vginfo->mda_size = vgsummary->mda_size;
 
-		log_debug_cache("lvmcache %s: VG %s: set mda_checksum to %x mda_size to %zu",
+		log_debug_cache("lvmcache %s mda%d VG %s set seqno %u checksum %x mda_size %zu",
-				dev_name(info->dev), vginfo->vgname,
+				dev_name(info->dev), vgsummary->mda_num, vgname,
-				vginfo->mda_checksum, vginfo->mda_size);
+				vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size);
+		goto update_vginfo;
 
-	} else if ((vginfo->mda_size != vgsummary->mda_size) || (vginfo->mda_checksum != vgsummary->mda_checksum)) {
+	} else if (vgsummary->seqno < vginfo->seqno) {
-		log_warn("Scan of VG %s from %s found mda_checksum %x mda_size %zu vs previous %x %zu",
+		vginfo->scan_summary_mismatch = true;
-			 vgname, dev_name(info->dev), vgsummary->mda_checksum, vgsummary->mda_size,
+
-			 vginfo->mda_checksum, vginfo->mda_size);
+		log_debug_cache("lvmcache %s mda%d VG %s older seqno %u checksum %x mda_size %zu",
-		vginfo->scan_summary_mismatch = 1;
+				dev_name(info->dev), vgsummary->mda_num, vgname,
-		/* If we don't return success, this dev info will be removed from lvmcache,
+				vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size);
-		   and then we won't be able to rescan it or repair it. */
+		return 1;
+
+	} else if (vgsummary->seqno > vginfo->seqno) {
+		vginfo->scan_summary_mismatch = true;
+
+		/* Replace vginfo values with values from newer metadata. */
+		vginfo->seqno = vgsummary->seqno;
+		vginfo->mda_checksum = vgsummary->mda_checksum;
+		vginfo->mda_size = vgsummary->mda_size;
+
+		log_debug_cache("lvmcache %s mda%d VG %s newer seqno %u checksum %x mda_size %zu",
+				dev_name(info->dev), vgsummary->mda_num, vgname,
+				vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size);
+
+		goto update_vginfo;
+	} else {
+		/*
+		 * Same seqno as previous metadata we saw for this VG.
+		 * If the metadata somehow has a different checksum or size,
+		 * even though it has the same seqno, something has gone wrong.
+		 * FIXME: test this case: VG has two PVs, first goes missing,
+		 * second updated to seqno 4, first comes back and second goes
+		 * missing, first updated to seqno 4, second comes back, now
+		 * both are present with same seqno but different checksums.
+		 */
+
+		if ((vginfo->mda_size != vgsummary->mda_size) || (vginfo->mda_checksum != vgsummary->mda_checksum)) {
+			log_warn("WARNING: scan of VG %s from %s mda%d found mda_checksum %x mda_size %zu vs %x %zu",
+				 vgname, dev_name(info->dev), vgsummary->mda_num,
+				 vgsummary->mda_checksum, vgsummary->mda_size,
+				 vginfo->mda_checksum, vginfo->mda_size);
+			vginfo->scan_summary_mismatch = true;
+			return 0;
+		}
+
+		/*
+		 * The seqno and checksum matches what was previously seen;
+		 * the summary values have already been saved in vginfo.
+		 */
 		return 1;
 	}
 
-	/*
+ update_vginfo:
-	 * If a dev has an unmatching checksum, ignore the other
-	 * info from it, keeping the info we already saved.
-	 */
 	if (!_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host,
 				       vgsummary->lock_type, vgsummary->system_id)) {
+		/*
+		 * This shouldn't happen, it's an internal errror, and we can leave
+		 * the info in place without saving the summary values in vginfo.
+		 */
 		log_error("Failed to update VG %s info in lvmcache.", vgname);
-		return 0;
 	}
 
 	return 1;
 }
 
-int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
+/*
+ * FIXME: quit trying to mirror changes that a command is making into lvmcache.
+ *
+ * First, it's complicated and hard to ensure it's done correctly in every case
+ * (it would be much easier and safer to just toss out what's in lvmcache and
+ * reread the info to recreate it from scratch instead of trying to make sure
+ * every possible discrete state change is correct.)
+ *
+ * Second, it's unnecessary if commands just use the vg they are modifying
+ * rather than also trying to get info from lvmcache.  The lvmcache state
+ * should be populated by label_scan, used to perform vg_read's, and then
+ * ignored (or dropped so it can't be used).
+ *
+ * lvmcache info is already used very little after a command begins its
+ * operation.  The code that's supposed to keep the lvmcache in sync with
+ * changes being made to disk could be half wrong and we wouldn't know it.
+ * That creates a landmine for someone who might try to use a bit of it that
+ * isn't being updated correctly.
+ */
+
+int lvmcache_update_vg_from_write(struct volume_group *vg)
 {
 	struct pv_list *pvl;
 	struct lvmcache_info *info;
@@ -1586,6 +1729,110 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
 	return 1;
 }
 
+/*
+ * The lvmcache representation of a VG after label_scan can be incorrect
+ * because the label_scan does not use the full VG metadata to construct
+ * vginfo/info.  PVs that don't hold VG metadata weren't attached to the vginfo
+ * during label scan, and PVs with outdated metadata (claiming to be in the VG,
+ * but not listed in the latest metadata) were attached to the vginfo, but
+ * shouldn't be.  After vg_read() gets the full metdata in the form of a 'vg',
+ * this function is called to fix up the lvmcache representation of the VG
+ * using the 'vg'.
+ */
+
+int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted)
+{
+	struct pv_list *pvl;
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info, *info2;
+	struct metadata_area *mda;
+	char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
+	struct lvmcache_vgsummary vgsummary = {
+		.vgname = vg->name,
+		.vgstatus = vg->status,
+		.vgid = vg->id,
+		.system_id = vg->system_id,
+		.lock_type = vg->lock_type
+	};
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vg->name, (const char *)&vg->id))) {
+		log_error(INTERNAL_ERROR "lvmcache_update_vg %s no vginfo", vg->name);
+		return 0;
+	}
+
+	/*
+	 * The label scan doesn't know when a PV with old metadata has been
+	 * removed from the VG.  Now with the vg we can tell, so remove the
+	 * info for a PV that has been removed from the VG with
+	 * vgreduce --removemissing.
+	 */
+	dm_list_iterate_items_safe(info, info2, &vginfo->infos) {
+		int found = 0;
+		dm_list_iterate_items(pvl, &vg->pvs) {
+			if (pvl->pv->dev != info->dev)
+				continue;
+			found = 1;
+			break;
+		}
+
+		if (found)
+			continue;
+
+		log_warn("WARNING: outdated PV %s seqno %u has been removed in current VG %s seqno %u.",
+			 dev_name(info->dev), info->summary_seqno, vg->name, vginfo->seqno);
+
+		_drop_vginfo(info, vginfo); /* remove from vginfo->infos */
+		dm_list_add(&vginfo->outdated_infos, &info->list);
+	}
+
+	dm_list_iterate_items(pvl, &vg->pvs) {
+		(void) dm_strncpy(pvid_s, (char *) &pvl->pv->id, sizeof(pvid_s));
+
+		if (!(info = lvmcache_info_from_pvid(pvid_s, pvl->pv->dev, 0))) {
+			log_debug_cache("lvmcache_update_vg %s no info for %s %s",
+					vg->name,
+					(char *) &pvl->pv->id,
+					pvl->pv->dev ? dev_name(pvl->pv->dev) : "missing");
+			continue;
+		}
+
+		log_debug_cache("lvmcache_update_vg %s for info %s",
+				vg->name, dev_name(info->dev));
+		
+		/*
+		 * FIXME: use a different function that just attaches info's that
+		 * had no metadata onto the correct vginfo.
+		 *
+		 * info's for PVs without metadata were not connected to the
+		 * vginfo by label_scan, so do it here.
+		 */
+		if (!lvmcache_update_vgname_and_id(info, &vgsummary)) {
+			log_debug_cache("lvmcache_update_vg %s failed to update info for %s",
+					vg->name, dev_name(info->dev));
+		}
+
+		/*
+		 * Ignored mdas were not copied from info->mdas to
+		 * fid->metadata_areas... when create_text_instance (at the
+		 * start of vg_read) called lvmcache_fid_add_mdas_vg because at
+		 * that point the info's were not connected to the vginfo
+		 * (since label_scan didn't know this without metadata.)
+		 */
+		dm_list_iterate_items(mda, &info->mdas) {
+			if (!mda_is_ignored(mda))
+				continue;
+			log_debug("lvmcache_update_vg %s copy ignored mdas for %s", vg->name, dev_name(info->dev));
+			if (!lvmcache_fid_add_mdas_pv(info, vg->fid)) {
+				log_debug_cache("lvmcache_update_vg %s failed to update mdas for %s",
+					        vg->name, dev_name(info->dev));
+			}
+			break;
+		}
+	}
+
+	return 1;
+}
+
 /*
  * We can see multiple different devices with the
  * same pvid, i.e. duplicates.
@@ -1637,7 +1884,7 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
  *   transient duplicate?
  */
 
-static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev)
+static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev, uint64_t label_sector)
 {
 	struct lvmcache_info *info;
 	struct label *label;
@@ -1650,6 +1897,9 @@ static struct lvmcache_info * _create_info(struct labeller *labeller, struct dev
 		return NULL;
 	}
 
+	label->dev = dev;
+	label->sector = label_sector;
+
 	info->dev = dev;
 	info->fmt = labeller->fmt;
 
@@ -1665,8 +1915,9 @@ static struct lvmcache_info * _create_info(struct labeller *labeller, struct dev
 }
 
 struct lvmcache_info *lvmcache_add(struct labeller *labeller,
-				   const char *pvid, struct device *dev,
+				   const char *pvid, struct device *dev, uint64_t label_sector,
-				   const char *vgname, const char *vgid, uint32_t vgstatus)
+				   const char *vgname, const char *vgid, uint32_t vgstatus,
+				   int *is_duplicate)
 {
 	char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
 	char uuid[64] __attribute__((aligned(8)));
@@ -1694,7 +1945,7 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller,
 		info = lvmcache_info_from_pvid(dev->pvid, NULL, 0);
 
 	if (!info) {
-		info = _create_info(labeller, dev);
+		info = _create_info(labeller, dev, label_sector);
 		created = 1;
 	}
 
@@ -1726,6 +1977,8 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller,
 
 			dm_list_add(&_found_duplicate_devs, &devl->list);
 			_found_duplicate_pvs = 1;
+			if (is_duplicate)
+				*is_duplicate = 1;
 			return NULL;
 		}
 
@@ -1869,6 +2122,14 @@ int lvmcache_fid_add_mdas_pv(struct lvmcache_info *info, struct format_instance
 	return lvmcache_fid_add_mdas(info, fid, info->dev->pvid, ID_LEN);
 }
 
+/*
+ * This is the linkage where information is passed from
+ * the label_scan to vg_read.
+ *
+ * Called by create_text_instance in vg_read to copy the
+ * mda's found during label_scan and saved in info->mdas,
+ * to fid->metadata_areas_in_use which is used by vg_read.
+ */
 int lvmcache_fid_add_mdas_vg(struct lvmcache_vginfo *vginfo, struct format_instance *fid)
 {
 	struct lvmcache_info *info;
@@ -1942,6 +2203,10 @@ void lvmcache_del_mdas(struct lvmcache_info *info)
 	if (info->mdas.n)
 		del_mdas(&info->mdas);
 	dm_list_init(&info->mdas);
+
+	if (info->bad_mdas.n)
+		del_mdas(&info->bad_mdas);
+	dm_list_init(&info->bad_mdas);
 }
 
 void lvmcache_del_das(struct lvmcache_info *info)
@@ -1959,9 +2224,10 @@ void lvmcache_del_bas(struct lvmcache_info *info)
 }
 
 int lvmcache_add_mda(struct lvmcache_info *info, struct device *dev,
-		     uint64_t start, uint64_t size, unsigned ignored)
+		     uint64_t start, uint64_t size, unsigned ignored,
+		     struct metadata_area **mda_new)
 {
-	return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored);
+	return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored, mda_new);
 }
 
 int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size)
@@ -2265,17 +2531,17 @@ int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const ch
  *    lvmcache: info for dev5 is deleted, FIXME: use a defective state
  */
 
-int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid)
+bool lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid)
 {
 	struct lvmcache_vginfo *vginfo;
 
 	if (!vgname || !vgid)
-		return 1;
+		return true;
 
 	if ((vginfo = lvmcache_vginfo_from_vgid(vgid)))
 		return vginfo->scan_summary_mismatch;
 
-	return 1;
+	return true;
 }
 
 static uint64_t _max_metadata_size;
@@ -2334,3 +2600,117 @@ struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd,
 	return NULL;
 }
 
+/*
+ * This is used by the metadata repair command to check if
+ * the metadata on a dev needs repair because it's old.
+ */
+bool lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+
+	/* shouldn't happen */
+	if (!vgname || !vgid)
+		return false;
+
+	/* shouldn't happen */
+	if (!(vginfo = lvmcache_vginfo_from_vgid(vgid)))
+		return false;
+
+	/* shouldn't happen */
+	if (!(info = lvmcache_info_from_pvid(dev->pvid, NULL, 0)))
+		return false;
+
+	/* writing to a new PV */
+	if (!info->summary_seqno)
+		return false;
+
+	/* on same dev, one mda has newer metadata than the other */
+	if (info->summary_seqno_mismatch)
+		return true;
+
+	/* one or both mdas on this dev has older metadata than another dev */
+	if (vginfo->seqno > info->summary_seqno)
+		return true;
+
+	return false;
+}
+
+void lvmcache_get_outdated_devs(struct cmd_context *cmd,
+				const char *vgname, const char *vgid,
+				struct dm_list *devs)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+	struct device_list *devl;
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
+		log_error(INTERNAL_ERROR "lvmcache_get_outdated_devs no vginfo %s", vgname);
+		return;
+	}
+
+	dm_list_iterate_items(info, &vginfo->outdated_infos) {
+		if (!(devl = zalloc(sizeof(*devl))))
+			return;
+		devl->dev = info->dev;
+		dm_list_add(devs, &devl->list);
+	}
+}
+
+void lvmcache_del_outdated_devs(struct cmd_context *cmd,
+				const char *vgname, const char *vgid)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info, *info2;
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
+		log_error(INTERNAL_ERROR "lvmcache_get_outdated_devs no vginfo");
+		return;
+	}
+
+	dm_list_iterate_items_safe(info, info2, &vginfo->outdated_infos)
+		lvmcache_del(info);
+}
+
+void lvmcache_get_outdated_mdas(struct cmd_context *cmd,
+				const char *vgname, const char *vgid,
+				struct device *dev,
+				struct dm_list **mdas)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+
+	*mdas = NULL;
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
+		log_error(INTERNAL_ERROR "lvmcache_get_outdated_mdas no vginfo");
+		return;
+	}
+
+	dm_list_iterate_items(info, &vginfo->outdated_infos) {
+		if (info->dev != dev)
+			continue;
+		*mdas = &info->mdas;
+		return;
+	}
+}
+
+bool lvmcache_is_outdated_dev(struct cmd_context *cmd,
+			     const char *vgname, const char *vgid,
+			     struct device *dev)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
+		log_error(INTERNAL_ERROR "lvmcache_get_outdated_mdas no vginfo");
+		return false;
+	}
+
+	dm_list_iterate_items(info, &vginfo->outdated_infos) {
+		if (info->dev == dev)
+			return true;
+	}
+
+	return false;
+}

lib/cache/lvmcache.h

@@ -57,10 +57,12 @@ struct lvmcache_vgsummary {
 	char *creation_host;
 	const char *system_id;
 	const char *lock_type;
+	uint32_t seqno;
 	uint32_t mda_checksum;
 	size_t mda_size;
-	int zero_offset;
+	int mda_num; /* 1 = summary from mda1, 2 = summary from mda2 */
-	int seqno;
+	unsigned mda_ignored:1;
+	unsigned zero_offset:1;
 };
 
 int lvmcache_init(struct cmd_context *cmd);
@@ -72,9 +74,9 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const
 
 /* Add/delete a device */
 struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid,
-				   struct device *dev,
+                                   struct device *dev, uint64_t label_sector,
-				   const char *vgname, const char *vgid,
+                                   const char *vgname, const char *vgid,
-				   uint32_t vgstatus);
+                                   uint32_t vgstatus, int *is_duplicate);
 int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt);
 void lvmcache_del(struct lvmcache_info *info);
 void lvmcache_del_dev(struct device *dev);
@@ -82,7 +84,8 @@ void lvmcache_del_dev(struct device *dev);
 /* Update things */
 int lvmcache_update_vgname_and_id(struct lvmcache_info *info,
 				  struct lvmcache_vgsummary *vgsummary);
-int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted);
+int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted);
+int lvmcache_update_vg_from_write(struct volume_group *vg);
 
 void lvmcache_lock_vgname(const char *vgname, int read_only);
 void lvmcache_unlock_vgname(const char *vgname);
@@ -127,7 +130,8 @@ void lvmcache_del_mdas(struct lvmcache_info *info);
 void lvmcache_del_das(struct lvmcache_info *info);
 void lvmcache_del_bas(struct lvmcache_info *info);
 int lvmcache_add_mda(struct lvmcache_info *info, struct device *dev,
-		     uint64_t start, uint64_t size, unsigned ignored);
+		     uint64_t start, uint64_t size, unsigned ignored,
+		     struct metadata_area **mda_new);
 int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size);
 int lvmcache_add_ba(struct lvmcache_info *info, uint64_t start, uint64_t size);
 
@@ -202,7 +206,7 @@ int lvmcache_get_vg_devs(struct cmd_context *cmd,
 			 struct dm_list *devs);
 void lvmcache_set_independent_location(const char *vgname);
 
-int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid);
+bool lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid);
 
 int lvmcache_vginfo_has_pvid(struct lvmcache_vginfo *vginfo, char *pvid);
 
@@ -220,4 +224,29 @@ void lvmcache_save_metadata_size(uint64_t val);
 
 int dev_in_device_list(struct device *dev, struct dm_list *head);
 
+bool lvmcache_has_bad_metadata(struct device *dev);
+
+bool lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev);
+
+void lvmcache_get_outdated_devs(struct cmd_context *cmd,
+                                const char *vgname, const char *vgid,
+                                struct dm_list *devs);
+void lvmcache_get_outdated_mdas(struct cmd_context *cmd,
+                                const char *vgname, const char *vgid,
+                                struct device *dev,
+                                struct dm_list **mdas);
+
+bool lvmcache_is_outdated_dev(struct cmd_context *cmd,
+                              const char *vgname, const char *vgid,
+                              struct device *dev);
+
+void lvmcache_del_outdated_devs(struct cmd_context *cmd,
+                                const char *vgname, const char *vgid);
+
+void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda);
+
+void lvmcache_get_bad_mdas(struct cmd_context *cmd,
+                           const char *vgname, const char *vgid,
+                           struct dm_list *bad_mda_list);
+
 #endif

lib/commands/toolcontext.h

@@ -172,18 +172,21 @@ struct cmd_context {
 	unsigned pvscan_cache_single:1;
 	unsigned can_use_one_scan:1;
 	unsigned is_clvmd:1;
+	unsigned md_component_detection:1;
 	unsigned use_full_md_check:1;
 	unsigned is_activating:1;
 	unsigned enable_hints:1;		/* hints are enabled for cmds in general */
 	unsigned use_hints:1;			/* if hints are enabled this cmd can use them */
 	unsigned pvscan_recreate_hints:1;	/* enable special case hint handling for pvscan --cache */
 	unsigned scan_lvs:1;
+	unsigned wipe_outdated_pvs:1;
 
 	/*
 	 * Devices and filtering.
 	 */
 	struct dev_filter *filter;
 	struct dm_list hints;
+	const char *md_component_checks;
 
 	/*
 	 * Configuration.

lib/config/config_settings.h

@@ -368,7 +368,30 @@ cfg(devices_multipath_component_detection_CFG, "multipath_component_detection",
 	"Ignore devices that are components of DM multipath devices.\n")
 
 cfg(devices_md_component_detection_CFG, "md_component_detection", devices_CFG_SECTION, 0, CFG_TYPE_BOOL, DEFAULT_MD_COMPONENT_DETECTION, vsn(1, 0, 18), NULL, 0, NULL,
-	"Ignore devices that are components of software RAID (md) devices.\n")
+	"Enable detection and exclusion of MD component devices.\n"
+	"An MD component device is a block device that MD uses as part\n"
+	"of a software RAID virtual device. When an LVM PV is created\n"
+	"on an MD device, LVM must only use the top level MD device as\n"
+	"the PV, and should ignore the underlying component devices.\n"
+	"In cases where the MD superblock is located at the end of the\n"
+	"component devices, it is more difficult for LVM to consistently\n"
+	"identify an MD component, see the md_component_checks setting.\n")
+
+cfg(devices_md_component_checks_CFG, "md_component_checks", devices_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_STRING, DEFAULT_MD_COMPONENT_CHECKS, vsn(2, 3, 2), NULL, 0, NULL,
+	"The checks LVM should use to detect MD component devices.\n"
+	"MD component devices are block devices used by MD software RAID.\n"
+	"#\n"
+	"Accepted values:\n"
+	"  auto\n"
+	"    LVM will skip scanning the end of devices when it has other\n"
+	"    indications that the device is not an MD component.\n"
+	"  start\n"
+	"    LVM will only scan the start of devices for MD superblocks.\n"
+	"    This does not incur extra I/O by LVM.\n"
+	"  full\n"
+	"    LVM will scan the start and end of devices for MD superblocks.\n"
+	"    This requires an extra read at the end of devices.\n"
+	"#\n")
 
 cfg(devices_fw_raid_component_detection_CFG, "fw_raid_component_detection", devices_CFG_SECTION, 0, CFG_TYPE_BOOL, DEFAULT_FW_RAID_COMPONENT_DETECTION, vsn(2, 2, 112), NULL, 0, NULL,
 	"Ignore devices that are components of firmware RAID devices.\n"

lib/config/defaults.h

@@ -318,4 +318,6 @@
 
 #define DEFAULT_IO_MEMORY_SIZE_KB 8192
 
+#define DEFAULT_MD_COMPONENT_CHECKS "auto"
+
 #endif				/* _LVM_DEFAULTS_H */

lib/device/dev-md.c

@@ -110,14 +110,17 @@ static int _udev_dev_is_md_component(struct device *dev)
 	if (!(ext = dev_ext_get(dev)))
 		return_0;
 
-	if (!(value = udev_device_get_property_value((struct udev_device *)ext->handle, DEV_EXT_UDEV_BLKID_TYPE)))
+	if (!(value = udev_device_get_property_value((struct udev_device *)ext->handle, DEV_EXT_UDEV_BLKID_TYPE))) {
+		dev->flags |= DEV_UDEV_INFO_MISSING;
 		return 0;
+	}
 
 	return !strcmp(value, DEV_EXT_UDEV_BLKID_TYPE_SW_RAID);
 }
 #else
 static int _udev_dev_is_md_component(struct device *dev)
 {
+	dev->flags |= DEV_UDEV_INFO_MISSING;
 	return 0;
 }
 #endif

lib/device/dev-type.c

@@ -1170,8 +1170,10 @@ int udev_dev_is_md_component(struct device *dev)
 	const char *value;
 	int ret = 0;
 
-	if (!obtain_device_list_from_udev())
+	if (!obtain_device_list_from_udev()) {
+		dev->flags |= DEV_UDEV_INFO_MISSING;
 		return 0;
+	}
 
 	if (!(udev_device = _udev_get_dev(dev)))
 		return 0;
@@ -1198,6 +1200,7 @@ int udev_dev_is_mpath_component(struct device *dev)
 
 int udev_dev_is_md_component(struct device *dev)
 {
+	dev->flags |= DEV_UDEV_INFO_MISSING;
 	return 0;
 }
 

lib/device/device.h

@@ -37,6 +37,7 @@
 #define DEV_BCACHE_WRITE	0x00008000      /* bcache_fd is open with RDWR */
 #define DEV_SCAN_FOUND_LABEL	0x00010000      /* label scan read dev and found label */
 #define DEV_IS_MD_COMPONENT	0x00020000	/* device is an md component */
+#define DEV_UDEV_INFO_MISSING   0x00040000	/* we have no udev info for this device */
 
 /*
  * Support for external device info.

lib/format_text/format-text.c

@@ -161,7 +161,8 @@ static void _xlate_mdah(struct mda_header *mdah)
 	}
 }
 
-static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev_area, int primary_mda)
+static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev_area,
+				int primary_mda, uint32_t ignore_bad_fields, uint32_t *bad_fields)
 {
 	log_debug_metadata("Reading mda header sector from %s at %llu",
 			   dev_name(dev_area->dev), (unsigned long long)dev_area->start);
@@ -169,53 +170,62 @@ static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev
 	if (!dev_read_bytes(dev_area->dev, dev_area->start, MDA_HEADER_SIZE, mdah)) {
 		log_error("Failed to read metadata area header on %s at %llu",
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
+		*bad_fields |= BAD_MDA_READ;
 		return 0;
 	}
 
 	if (mdah->checksum_xl != xlate32(calc_crc(INITIAL_CRC, (uint8_t *)mdah->magic,
 						  MDA_HEADER_SIZE -
 						  sizeof(mdah->checksum_xl)))) {
-		log_error("Incorrect checksum in metadata area header on %s at %llu",
+		log_warn("WARNING: wrong checksum %x in mda header on %s at %llu",
+			  mdah->checksum_xl,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_CHECKSUM;
 	}
 
 	_xlate_mdah(mdah);
 
 	if (memcmp(mdah->magic, FMTT_MAGIC, sizeof(mdah->magic))) {
-		log_error("Wrong magic number in metadata area header on %s at %llu",
+		log_warn("WARNING: wrong magic number in mda header on %s at %llu",
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_MAGIC;
 	}
 
 	if (mdah->version != FMTT_VERSION) {
-		log_error("Incompatible version %u metadata area header on %s at %llu",
+		log_warn("WARNING: wrong version %u in mda header on %s at %llu",
 			  mdah->version,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_VERSION;
 	}
 
 	if (mdah->start != dev_area->start) {
-		log_error("Incorrect start sector %llu in metadata area header on %s at %llu",
+		log_warn("WARNING: wrong start sector %llu in mda header on %s at %llu",
 			  (unsigned long long)mdah->start,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_START;
 	}
 
+	*bad_fields &= ~ignore_bad_fields;
+
+	if (*bad_fields)
+		return 0;
+
 	return 1;
 }
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area, int primary_mda)
+				       struct device_area *dev_area,
+				       int primary_mda, uint32_t ignore_bad_fields, uint32_t *bad_fields)
 {
 	struct mda_header *mdah;
 
 	if (!(mdah = dm_pool_alloc(fmt->cmd->mem, MDA_HEADER_SIZE))) {
 		log_error("struct mda_header allocation failed");
+		*bad_fields |= BAD_MDA_INTERNAL;
 		return NULL;
 	}
 
-	if (!_raw_read_mda_header(mdah, dev_area, primary_mda)) {
+	if (!_raw_read_mda_header(mdah, dev_area, primary_mda, ignore_bad_fields, bad_fields)) {
 		dm_pool_free(fmt->cmd->mem, mdah);
 		return NULL;
 	}
@@ -413,8 +423,9 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
 	time_t when;
 	char *desc;
 	uint32_t wrap = 0;
+	uint32_t bad_fields = 0;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, area, primary_mda))) {
+	if (!(mdah = raw_read_mda_header(fid->fmt, area, primary_mda, 0, &bad_fields))) {
 		log_error("Failed to read vg %s from %s", vgname, dev_name(area->dev));
 		goto out;
 	}
@@ -535,6 +546,7 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
 	uint64_t old_start = 0, old_last = 0, old_size = 0, old_wrap = 0;
 	uint64_t new_start = 0, new_last = 0, new_size = 0, new_wrap = 0;
 	uint64_t max_size;
+	uint32_t bad_fields = 0;
 	char *new_buf = NULL;
 	int overlap;
 	int found = 0;
@@ -550,7 +562,7 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
 	if (!found)
 		return 1;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda))))
+	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda), mda->ignore_bad_fields, &bad_fields)))
 		goto_out;
 
 	/*
@@ -821,6 +833,7 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
 	struct raw_locn *rlocn_slot1;
 	struct raw_locn *rlocn_new;
 	struct pv_list *pvl;
+	uint32_t bad_fields = 0;
 	int r = 0;
 	int found = 0;
 
@@ -841,7 +854,7 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
 	 * mdah buffer, but the mdah buffer is not modified and mdac->rlocn is
 	 * modified.
 	 */
-	if (!(mdab = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda))))
+	if (!(mdab = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda), mda->ignore_bad_fields, &bad_fields)))
 		goto_out;
 
 	/*
@@ -1033,6 +1046,7 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
 	struct mda_header *mdah;
 	struct raw_locn *rlocn_slot0;
 	struct raw_locn *rlocn_slot1;
+	uint32_t bad_fields = 0;
 	int r = 0;
 
 	if (!(mdah = dm_pool_alloc(fid->fmt->cmd->mem, MDA_HEADER_SIZE))) {
@@ -1046,7 +1060,7 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
 	 * Just to print the warning?
 	 */
 
-	if (!_raw_read_mda_header(mdah, &mdac->area, mda_is_primary(mda)))
+	if (!_raw_read_mda_header(mdah, &mdac->area, mda_is_primary(mda), 0, &bad_fields))
 		log_warn("WARNING: Removing metadata location on %s with bad mda header.",
 			  dev_name(mdac->area.dev));
 
@@ -1343,7 +1357,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 	 * valid vg name.
 	 */
 	if (!validate_name(namebuf)) {
-		log_error("Metadata location on %s at %llu begins with invalid VG name.",
+		log_warn("WARNING: Metadata location on %s at %llu begins with invalid VG name.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1409,7 +1423,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 				(off_t) (dev_area->start + MDA_HEADER_SIZE),
 				wrap, calc_crc, vgsummary->vgname ? 1 : 0,
 				vgsummary)) {
-		log_error("Metadata location on %s at %llu has invalid summary for VG.",
+		log_warn("WARNING: metadata on %s at %llu has invalid summary for VG.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1417,7 +1431,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 
 	/* Ignore this entry if the characters aren't permissible */
 	if (!validate_name(vgsummary->vgname)) {
-		log_error("Metadata location on %s at %llu has invalid VG name.",
+		log_warn("WARNING: metadata on %s at %llu has invalid VG name.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1499,13 +1513,12 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume
 
 	/* Add a new cache entry with PV info or update existing one. */
 	if (!(info = lvmcache_add(fmt->labeller, (const char *) &pv->id,
-				  pv->dev, pv->vg_name,
+				  pv->dev,  pv->label_sector, pv->vg_name,
-				  is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0)))
+				  is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0, NULL)))
 		return_0;
 
+	/* lvmcache_add() creates info and info->label structs for the dev, get info->label. */
 	label = lvmcache_get_label(info);
-	label->sector = pv->label_sector;
-	label->dev = pv->dev;
 
 	lvmcache_update_pv(info, pv, fmt);
 
@@ -1533,7 +1546,7 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume
 		// if fmt is not the same as info->fmt we are in trouble
 		if (!lvmcache_add_mda(info, mdac->area.dev,
 				      mdac->area.start, mdac->area.size,
-				      mda_is_ignored(mda)))
+				      mda_is_ignored(mda), NULL))
 			return_0;
 	}
 
@@ -1587,12 +1600,16 @@ static int _text_pv_needs_rewrite(const struct format_type *fmt, struct physical
 {
 	struct lvmcache_info *info;
 	uint32_t ext_vsn;
+	uint32_t ext_flags;
 
 	*needs_rewrite = 0;
 
 	if (!pv->is_labelled)
 		return 1;
 
+	if (!pv->dev)
+		return 1;
+
 	if (!(info = lvmcache_info_from_pvid((const char *)&pv->id, pv->dev, 0))) {
 		log_error("Failed to find cached info for PV %s.", pv_dev_name(pv));
 		return 0;
@@ -1600,8 +1617,16 @@ static int _text_pv_needs_rewrite(const struct format_type *fmt, struct physical
 
 	ext_vsn = lvmcache_ext_version(info);
 
-	if (ext_vsn < PV_HEADER_EXTENSION_VSN)
+	if (ext_vsn < PV_HEADER_EXTENSION_VSN) {
+		log_debug("PV %s header needs rewrite for new ext version", dev_name(pv->dev));
 		*needs_rewrite = 1;
+	}
+
+	ext_flags = lvmcache_ext_flags(info);
+	if (!(ext_flags & PV_EXT_USED)) {
+		log_debug("PV %s header needs rewrite to set ext used", dev_name(pv->dev));
+		*needs_rewrite = 1;
+	}
 
 	return 1;
 }
@@ -2450,3 +2475,37 @@ bad:
 
 	return NULL;
 }
+
+int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev,
+			      struct metadata_area *mda)
+{
+	struct mda_context *mdac = mda->metadata_locn;
+	uint64_t start_byte = mdac->area.start;
+	struct mda_header *mdab;
+	struct raw_locn *rlocn_slot0;
+	struct raw_locn *rlocn_slot1;
+	uint32_t bad_fields = 0;
+
+	if (!(mdab = raw_read_mda_header(cmd->fmt, &mdac->area, mda_is_primary(mda), 0, &bad_fields))) {
+		log_error("Failed to read outdated pv mda header on %s", dev_name(dev));
+		return 0;
+	}
+
+	rlocn_slot0 = &mdab->raw_locns[0];
+	rlocn_slot1 = &mdab->raw_locns[1];
+
+	rlocn_slot0->offset = 0;
+	rlocn_slot0->size = 0;
+	rlocn_slot0->checksum = 0;
+	rlocn_slot1->offset = 0;
+	rlocn_slot1->size = 0;
+	rlocn_slot1->checksum = 0;
+         
+	if (!_raw_write_mda_header(cmd->fmt, dev, mda_is_primary(mda), start_byte, mdab)) {
+		log_error("Failed to write outdated pv mda header on %s", dev_name(dev));
+		return 0;
+	}
+
+	return 1;
+}
+

lib/format_text/format-text.h

@@ -61,7 +61,8 @@ int add_ba(struct dm_pool *mem, struct dm_list *eas,
 	   uint64_t start, uint64_t size);
 void del_bas(struct dm_list *bas);
 int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *mdas,
-	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored);
+	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored,
+	    struct metadata_area **mda_new);
 void del_mdas(struct dm_list *mdas);
 
 /* On disk */
@@ -76,4 +77,7 @@ struct data_area_list {
 	struct disk_locn disk_locn;
 };
 
+int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev,
+                              struct metadata_area *mda);
+
 #endif

lib/format_text/import.c

@@ -61,13 +61,13 @@ int text_read_metadata_summary(const struct format_type *fmt,
 					 offset2, size2, checksum_fn,
 					 vgsummary->mda_checksum,
 					 checksum_only, 1)) {
-			/* FIXME: handle errors */
+			log_warn("WARNING: invalid metadata text from %s at %llu.",
-			log_error("Couldn't read volume group metadata from %s.", dev_name(dev));
+				 dev_name(dev), (unsigned long long)offset);
 			goto out;
 		}
 	} else {
 		if (!config_file_read(cft)) {
-			log_error("Couldn't read volume group metadata from file.");
+			log_warn("WARNING: invalid metadata text from file.");
 			goto out;
 		}
 	}

lib/format_text/layout.h

@@ -81,7 +81,9 @@ struct mda_header {
 } __attribute__ ((packed));
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area, int primary_mda);
+				       struct device_area *dev_area, int primary_mda,
+				       uint32_t ignore_bad_fields,
+				       uint32_t *bad_fields);
 
 struct mda_lists {
 	struct metadata_area_ops *file_ops;

lib/format_text/text_label.c

@@ -241,11 +241,10 @@ void del_bas(struct dm_list *bas)
 	del_das(bas);
 }
 
-/* FIXME: refactor this function with other mda constructor code */
 int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *mdas,
-	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored)
+	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored,
+	    struct metadata_area **mda_new)
 {
-/* FIXME List size restricted by pv_header SECTOR_SIZE */
 	struct metadata_area *mdal, *mda;
 	struct mda_lists *mda_lists = (struct mda_lists *) fmt->private;
 	struct mda_context *mdac, *mdac2;
@@ -295,6 +294,8 @@ int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *
 	mda_set_ignored(mdal, ignored);
 
 	dm_list_add(mdas, &mdal->list);
+	if (mda_new)
+		*mda_new = mdal;
 	return 1;
 }
 
@@ -319,78 +320,103 @@ static int _text_initialise_label(struct labeller *l __attribute__((unused)),
 	return 1;
 }
 
-struct _update_mda_baton {
+static int _read_mda_header_and_metadata(const struct format_type *fmt,
-	struct lvmcache_info *info;
+					 struct metadata_area *mda,
-	struct label *label;
+					 struct lvmcache_vgsummary *vgsummary,
-};
+					 uint32_t *bad_fields)
-
-static int _read_mda_header_and_metadata(struct metadata_area *mda, void *baton)
 {
-	struct _update_mda_baton *p = baton;
-	const struct format_type *fmt = p->label->labeller->fmt;
 	struct mda_context *mdac = (struct mda_context *) mda->metadata_locn;
 	struct mda_header *mdah;
-	struct lvmcache_vgsummary vgsummary = { 0 };
 
-	if (!(mdah = raw_read_mda_header(fmt, &mdac->area, mda_is_primary(mda)))) {
+	if (!(mdah = raw_read_mda_header(fmt, &mdac->area, (mda->mda_num == 1), 0, bad_fields))) {
-		log_error("Failed to read mda header from %s", dev_name(mdac->area.dev));
+		log_warn("WARNING: bad metadata header on %s at %llu.",
-		goto fail;
+			 dev_name(mdac->area.dev),
+			 (unsigned long long)mdac->area.start);
+		if (mda)
+			mda->header_start = mdac->area.start;
+		*bad_fields |= BAD_MDA_HEADER;
+		return 0;
 	}
 
+	if (mda)
+		mda->header_start = mdah->start;
+
 	mda_set_ignored(mda, rlocn_is_ignored(mdah->raw_locns));
 
 	if (mda_is_ignored(mda)) {
 		log_debug_metadata("Ignoring mda on device %s at offset " FMTu64,
 				   dev_name(mdac->area.dev),
 				   mdac->area.start);
+		vgsummary->mda_ignored = 1;
 		return 1;
 	}
 
 	if (!read_metadata_location_summary(fmt, mdah, mda_is_primary(mda), &mdac->area,
-					    &vgsummary, &mdac->free_sectors)) {
+					    vgsummary, &mdac->free_sectors)) {
-		if (vgsummary.zero_offset)
+		if (vgsummary->zero_offset)
 			return 1;
 
-		log_error("Failed to read metadata summary from %s", dev_name(mdac->area.dev));
+		log_warn("WARNING: bad metadata text on %s in mda%d",
-		goto fail;
+			 dev_name(mdac->area.dev), mda->mda_num);
-	}
+		*bad_fields |= BAD_MDA_TEXT;
-
+		return 0;
-	if (!lvmcache_update_vgname_and_id(p->info, &vgsummary)) {
-		log_error("Failed to save lvm summary for %s", dev_name(mdac->area.dev));
-		goto fail;
 	}
 
 	return 1;
-
-fail:
-	lvmcache_del(p->info);
-	return 0;
 }
 
-static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
+/*
-		      struct label **label)
+ * Used by label_scan to get a summary of the VG that exists on this PV.  This
+ * summary is stored in lvmcache vginfo/info/info->mdas and is used later by
+ * vg_read which needs to know which PVs to read for a given VG name, and where
+ * the metadata is at for those PVs.
+ */
+
+static int _text_read(struct labeller *labeller, struct device *dev, void *label_buf,
+		      uint64_t label_sector, int *is_duplicate)
 {
+	struct lvmcache_vgsummary vgsummary;
+	struct lvmcache_info *info;
+	const struct format_type *fmt = labeller->fmt;
 	struct label_header *lh = (struct label_header *) label_buf;
 	struct pv_header *pvhdr;
 	struct pv_header_extension *pvhdr_ext;
-	struct lvmcache_info *info;
+	struct metadata_area *mda;
+	struct metadata_area *mda1 = NULL;
+	struct metadata_area *mda2 = NULL;
 	struct disk_locn *dlocn_xl;
 	uint64_t offset;
 	uint32_t ext_version;
-	struct _update_mda_baton baton;
+	uint32_t bad_fields;
+	int mda_count = 0;
+	int good_mda_count = 0;
+	int bad_mda_count = 0;
+	int rv1, rv2;
 
 	/*
 	 * PV header base
 	 */
 	pvhdr = (struct pv_header *) ((char *) label_buf + xlate32(lh->offset_xl));
 
-	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev,
+	/*
+	 * FIXME: stop adding the device to lvmcache initially as an orphan
+	 * (and then moving it later) and instead just add it when we know the
+	 * VG.
+	 *
+	 * If another device with this same PVID has already been seen,
+	 * lvmcache_add will put this device in the duplicates list in lvmcache
+	 * and return NULL.  At the end of label_scan, the duplicate devs are
+	 * compared, and if another dev is preferred for this PV, then the
+	 * existing dev is removed from lvmcache and _text_read is called again
+	 * for this dev, and lvmcache_add will add it.
+	 *
+	 * Other reasons for lvmcache_add to return NULL are internal errors.
+	 */
+	if (!(info = lvmcache_add(labeller, (char *)pvhdr->pv_uuid, dev, label_sector,
 				  FMT_TEXT_ORPHAN_VG_NAME,
-				  FMT_TEXT_ORPHAN_VG_NAME, 0)))
+				  FMT_TEXT_ORPHAN_VG_NAME, 0, is_duplicate)))
 		return_0;
 
-	*label = lvmcache_get_label(info);
-
 	lvmcache_set_device_size(info, xlate64(pvhdr->device_size_xl));
 
 	lvmcache_del_das(info);
@@ -404,11 +430,27 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
 		dlocn_xl++;
 	}
 
-	/* Metadata area headers */
 	dlocn_xl++;
+
+	/* Metadata areas */
 	while ((offset = xlate64(dlocn_xl->offset))) {
-		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0);
+
+		/*
+		 * This just calls add_mda() above, replacing info with info->mdas.
+		 */
+		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0, &mda);
+
 		dlocn_xl++;
+		mda_count++;
+
+		if (mda_count == 1) {
+			mda1 = mda;
+			mda1->mda_num = 1;
+		}
+		else if (mda_count == 2) {
+			mda2 = mda;
+			mda2->mda_num = 2;
+		}
 	}
 
 	dlocn_xl++;
@@ -418,7 +460,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
 	 */
 	pvhdr_ext = (struct pv_header_extension *) ((char *) dlocn_xl);
 	if (!(ext_version = xlate32(pvhdr_ext->version)))
-		goto out;
+		goto scan_mdas;
 
 	log_debug_metadata("%s: PV header extension version " FMTu32 " found",
 			   dev_name(dev), ext_version);
@@ -435,22 +477,117 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
 		lvmcache_add_ba(info, offset, xlate64(dlocn_xl->size));
 		dlocn_xl++;
 	}
-out:
-	baton.info = info;
-	baton.label = *label;
 
-	/*
+ scan_mdas:
-	 * In the vg_read phase, we compare all mdas and decide which to use
+	if (!mda_count) {
-	 * which are bad and need repair.
+		log_debug_metadata("Scanning %s found no mdas.", dev_name(dev));
-	 *
+		return 1;
-	 * FIXME: this quits if the first mda is bad, but we need something
-	 * smarter to be able to use the second mda if it's good.
-	 */
-	if (!lvmcache_foreach_mda(info, _read_mda_header_and_metadata, &baton)) {
-		log_error("Failed to scan VG from %s", dev_name(dev));
-		return 0;
 	}
 
+	/*
+	 * Track which devs have bad metadata so repair can find them (even if
+	 * this dev also has good metadata that we are able to use).
+	 *
+	 * When bad metadata is seen, the unusable mda struct is removed from
+	 * lvmcache info->mdas.  This means that vg_read and vg_write will skip
+	 * the bad mda not try to read or write the bad metadata.  The bad mdas
+	 * are saved in a separate bad_mdas list in lvmcache so that repair can
+	 * find them to repair.
+	 */
+
+	if (mda1) {
+		log_debug_metadata("Scanning %s mda1 summary.", dev_name(dev));
+		memset(&vgsummary, 0, sizeof(vgsummary));
+		bad_fields = 0;
+		vgsummary.mda_num = 1;
+
+		rv1 = _read_mda_header_and_metadata(fmt, mda1, &vgsummary, &bad_fields);
+
+		if (rv1 && !vgsummary.zero_offset && !vgsummary.mda_ignored) {
+			if (!lvmcache_update_vgname_and_id(info, &vgsummary)) {
+				/* I believe this is only an internal error. */
+				log_warn("WARNING: Scanning %s mda1 failed to save internal summary.", dev_name(dev));
+
+				dm_list_del(&mda1->list);
+				bad_fields |= BAD_MDA_INTERNAL;
+				mda1->bad_fields = bad_fields;
+				lvmcache_save_bad_mda(info, mda1);
+				mda1 = NULL;
+				bad_mda_count++;
+			} else {
+				/* The normal success path */
+				log_debug("Scanned %s mda1 seqno %u", dev_name(dev), vgsummary.seqno);
+				good_mda_count++;
+			}
+		}
+
+		if (!rv1) {
+			/*
+			 * Remove the bad mda from normal mda list so it's not
+			 * used by vg_read/vg_write, but keep track of it in
+			 * lvmcache for repair.
+			 */
+			log_warn("WARNING: scanning %s mda1 failed to read metadata summary.", dev_name(dev));
+			log_warn("WARNING: repair VG metadata on %s with vgck --updatemetadata.", dev_name(dev));
+
+			dm_list_del(&mda1->list);
+			mda1->bad_fields = bad_fields;
+			lvmcache_save_bad_mda(info, mda1);
+			mda1 = NULL;
+			bad_mda_count++;
+		}
+	}
+
+	if (mda2) {
+		log_debug_metadata("Scanning %s mda2 summary.", dev_name(dev));
+		memset(&vgsummary, 0, sizeof(vgsummary));
+		bad_fields = 0;
+		vgsummary.mda_num = 2;
+
+		rv2 = _read_mda_header_and_metadata(fmt, mda2, &vgsummary, &bad_fields);
+
+		if (rv2 && !vgsummary.zero_offset && !vgsummary.mda_ignored) {
+			if (!lvmcache_update_vgname_and_id(info, &vgsummary)) {
+				/* I believe this is only an internal error. */
+				log_warn("WARNING: Scanning %s mda2 failed to save internal summary.", dev_name(dev));
+
+				dm_list_del(&mda2->list);
+				bad_fields |= BAD_MDA_INTERNAL;
+				mda2->bad_fields = bad_fields;
+				lvmcache_save_bad_mda(info, mda2);
+				mda2 = NULL;
+				bad_mda_count++;
+			} else {
+				/* The normal success path */
+				log_debug("Scanned %s mda2 seqno %u", dev_name(dev), vgsummary.seqno);
+				good_mda_count++;
+			}
+		}
+
+		if (!rv2) {
+			/*
+			 * Remove the bad mda from normal mda list so it's not
+			 * used by vg_read/vg_write, but keep track of it in
+			 * lvmcache for repair.
+			 */
+			log_warn("WARNING: scanning %s mda2 failed to read metadata summary.", dev_name(dev));
+			log_warn("WARNING: repair VG metadata on %s with vgck --updatemetadata.", dev_name(dev));
+
+			dm_list_del(&mda2->list);
+			mda2->bad_fields = bad_fields;
+			lvmcache_save_bad_mda(info, mda2);
+			mda2 = NULL;
+			bad_mda_count++;
+		}
+	}
+
+	if (good_mda_count)
+		return 1;
+
+	if (bad_mda_count)
+		return 0;
+
+	/* no metadata in the mdas */
 	return 1;
 }
 

lib/label/label.c

@@ -356,9 +356,9 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f,
 			  int *is_lvm_device)
 {
 	char label_buf[LABEL_SIZE] __attribute__((aligned(8)));
-	struct label *label = NULL;
 	struct labeller *labeller;
 	uint64_t sector = 0;
+	int is_duplicate = 0;
 	int ret = 0;
 	int pass;
 
@@ -423,17 +423,38 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f,
 
 	/*
 	 * This is the point where the scanning code dives into the rest of
-	 * lvm.  ops->read() is usually _text_read() which reads the pv_header,
+	 * lvm.  ops->read() is _text_read() which reads the pv_header, mda
-	 * mda locations, mda contents.  As these bits of data are read, they
+	 * locations, and metadata text.  All of the info it finds about the PV
-	 * are saved into lvmcache as info/vginfo structs.
+	 * and VG is stashed in lvmcache which saves it in the form of
+	 * info/vginfo structs.  That lvmcache info is used later when the
+	 * command wants to read the VG to do something to it.
 	 */
+	ret = labeller->ops->read(labeller, dev, label_buf, sector, &is_duplicate);
 
-	if ((ret = (labeller->ops->read)(labeller, dev, label_buf, &label)) && label) {
+	if (!ret) {
-		label->dev = dev;
+		if (is_duplicate) {
-		label->sector = sector;
+			/*
-	} else {
+			 * _text_read() called lvmcache_add() which found an
-		/* FIXME: handle errors */
+			 * existing info struct for this PVID but for a
-		lvmcache_del_dev(dev);
+			 * different dev.  lvmcache_add() did not add an info
+			 * struct for this dev, but added this dev to the list
+			 * of duplicate devs.
+			 */
+			log_warn("WARNING: scan found duplicate PVID %s on %s", dev->pvid, dev_name(dev));
+		} else {
+			/*
+			 * Leave the info in lvmcache because the device is
+			 * present and can still be used even if it has
+			 * metadata that we can't process (we can get metadata
+			 * from another PV/mda.) _text_read only saves mdas
+			 * with good metadata in lvmcache (this includes old
+			 * metadata), and if a PV has no mdas with good
+			 * metadata, then the info for the PV will be in
+			 * lvmcache with empty info->mdas, and it will behave
+			 * like a PV with no mdas (a common configuration.)
+			 */
+			log_warn("WARNING: scan failed to get metadata summary from %s PVID %s", dev_name(dev), dev->pvid);
+		}
 	}
  out:
 	return ret;
@@ -696,7 +717,6 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f,
 				scan_failed = 1;
 				scan_process_errors++;
 				scan_failed_count++;
-				lvmcache_del_dev(devl->dev);
 			}
 		}
 
@@ -1022,6 +1042,33 @@ int label_scan(struct cmd_context *cmd)
 		}
 	}
 
+	/*
+	 * Stronger exclusion of md components that might have been
+	 * misidentified as PVs due to having an end-of-device md superblock.
+	 * If we're not using hints, and are not already doing a full md check
+	 * on devs being scanned, then if udev info is missing for a PV, scan
+	 * the end of the PV to verify it's not an md component.  The full
+	 * dev_is_md_component call will do new reads at the end of the dev.
+	 */
+	if (cmd->md_component_detection && !cmd->use_full_md_check && !using_hints &&
+	    !strcmp(cmd->md_component_checks, "auto")) {
+		int once = 0;
+		dm_list_iterate_items(devl, &scan_devs) {
+			if (!(devl->dev->flags & DEV_SCAN_FOUND_LABEL))
+				continue;
+			if (!(devl->dev->flags & DEV_UDEV_INFO_MISSING))
+				continue;
+			if (!once++)
+				log_debug_devs("Scanning end of PVs with no udev info for MD components");
+
+			if (dev_is_md_component(devl->dev, NULL, 1)) {
+				log_debug_devs("Drop PV from MD component %s", dev_name(devl->dev));
+				devl->dev->flags &= ~DEV_SCAN_FOUND_LABEL;
+				lvmcache_del_dev(devl->dev);
+			}
+		}
+	}
+
 	dm_list_iterate_items_safe(devl, devl2, &all_devs) {
 		dm_list_del(&devl->list);
 		free(devl);

lib/label/label.h

@@ -65,7 +65,7 @@ struct label_ops {
 	 * Read a label from a volume.
 	 */
 	int (*read) (struct labeller * l, struct device * dev,
-		     void *label_buf, struct label ** label);
+		     void *label_buf, uint64_t label_sector, int *is_duplicate);
 
 	/*
 	 * Populate label_type etc.

lib/metadata/metadata-exported.h

@@ -181,15 +181,14 @@
 #define MIRROR_SKIP_INIT_SYNC	0x00000010U	/* skip initial sync */
 
 /* vg_read and vg_read_for_update flags */
-#define READ_ALLOW_INCONSISTENT	0x00010000U
 #define READ_ALLOW_EXPORTED	0x00020000U
 #define READ_OK_NOTFOUND	0x00040000U
 #define READ_WARN_INCONSISTENT	0x00080000U
 #define READ_FOR_UPDATE		0x00100000U /* A meta-flag, useful with toollib for_each_* functions. */
 #define PROCESS_SKIP_SCAN	 0x00200000U /* skip lvmcache_label_scan in process_each_pv */
 
-/* vg's "read_status" field */
+/* vg_read returns these in error_flags */
-#define FAILED_INCONSISTENT	0x00000001U
+#define FAILED_NOT_ENABLED	0x00000001U
 #define FAILED_LOCKING		0x00000002U
 #define FAILED_NOTFOUND		0x00000004U
 #define FAILED_READ_ONLY	0x00000008U
@@ -202,6 +201,7 @@
 #define FAILED_SYSTEMID		0x00000400U
 #define FAILED_LOCK_TYPE	0x00000800U
 #define FAILED_LOCK_MODE	0x00001000U
+#define FAILED_INTERNAL_ERROR	0x00002000U
 #define SUCCESS			0x00000000U
 
 #define VGMETADATACOPIES_ALL UINT32_MAX
@@ -717,24 +717,14 @@ int lv_resize(struct logical_volume *lv,
 	      struct lvresize_params *lp,
 	      struct dm_list *pvh);
 
-/*
+struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const char *vgid,
- * Return a handle to VG metadata.
+			     uint32_t read_flags, uint32_t lockd_state,
- */
+			     uint32_t *error_flags, struct volume_group **error_vg);
-struct volume_group *vg_read_internal(struct cmd_context *cmd,
-                                      const char *vgname, const char *vgid,
-                                      uint32_t lockd_state, uint32_t warn_flags,
-                                      int enable_repair,
-                                      int *mdas_consistent);
-struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
-			     const char *vgid, uint32_t read_flags, uint32_t lockd_state);
 struct volume_group *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
 			 const char *vgid, uint32_t read_flags, uint32_t lockd_state);
-struct volume_group *vg_read_orphans(struct cmd_context *cmd,
+struct volume_group *vg_read_orphans(struct cmd_context *cmd, const char *orphan_vgname);
-                                             uint32_t warn_flags,
+
-                                             const char *orphan_vgname);
+/* this is historical and being removed, don't use */
-/* 
- * Test validity of a VG handle.
- */
 uint32_t vg_read_error(struct volume_group *vg_handle);
 
 /* pe_start and pe_end relate to any existing data so that new metadata
@@ -757,7 +747,7 @@ uint32_t pv_list_extents_free(const struct dm_list *pvh);
 int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name);
 int vg_validate(struct volume_group *vg);
 struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name);
-struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name);
+struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name, int *exists);
 int vg_remove_mdas(struct volume_group *vg);
 int vg_remove_check(struct volume_group *vg);
 void vg_remove_pvs(struct volume_group *vg);
@@ -1381,4 +1371,6 @@ int lv_on_pmem(struct logical_volume *lv);
 
 int vg_is_foreign(struct volume_group *vg);
 
+void vg_write_commit_bad_mdas(struct cmd_context *cmd, struct volume_group *vg);
+
 #endif

lib/metadata/metadata.h

@@ -168,11 +168,27 @@ struct metadata_area_ops {
 #define MDA_CONTENT_REASON(primary_mda) ((primary_mda) ? DEV_IO_MDA_CONTENT : DEV_IO_MDA_EXTRA_CONTENT)
 #define MDA_HEADER_REASON(primary_mda)  ((primary_mda) ? DEV_IO_MDA_HEADER : DEV_IO_MDA_EXTRA_HEADER)
 
+/*
+ * Flags describing errors found while reading.
+ */
+#define BAD_MDA_INTERNAL	0x00000001 /* internal lvm error */
+#define BAD_MDA_READ		0x00000002 /* read io failed */
+#define BAD_MDA_HEADER		0x00000004 /* general problem with header */
+#define BAD_MDA_TEXT		0x00000008 /* general problem with text */
+#define BAD_MDA_CHECKSUM	0x00000010
+#define BAD_MDA_MAGIC		0x00000020
+#define BAD_MDA_VERSION		0x00000040
+#define BAD_MDA_START		0x00000080
+
 struct metadata_area {
 	struct dm_list list;
 	struct metadata_area_ops *ops;
 	void *metadata_locn;
 	uint32_t status;
+	uint64_t header_start; /* mda_header.start */
+	int mda_num;
+	uint32_t bad_fields; /* BAD_MDA_ flags are set to indicate errors found when reading */
+	uint32_t ignore_bad_fields; /* BAD_MDA_ flags are set to indicate errors to ignore */
 };
 struct metadata_area *mda_copy(struct dm_pool *mem,
 			       struct metadata_area *mda);
@@ -234,7 +250,7 @@ struct name_list {
 
 struct mda_list {
 	struct dm_list list;
-	struct device_area mda;
+	struct metadata_area *mda;
 };
 
 struct peg_list {

lib/metadata/pv.h

@@ -59,6 +59,7 @@ struct physical_volume {
 
         /* This is true whenever the represented PV has a label associated. */
         uint64_t is_labelled:1;
+        uint64_t unused_missing_cleared:1;
 
         /* NB. label_sector is valid whenever is_labelled is true */
 	uint64_t label_sector;

lib/metadata/vg.c

@@ -84,7 +84,7 @@ static void _free_vg(struct volume_group *vg)
 
 void release_vg(struct volume_group *vg)
 {
-	if (!vg || (vg->fid && vg == vg->fid->fmt->orphan_vg))
+	if (!vg || is_orphan_vg(vg->name))
 		return;
 
 	release_vg(vg->vg_committed);
@@ -711,9 +711,9 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
 	vg->extent_count -= pv_pe_count(pv);
 
 	/* FIXME: we don't need to vg_read the orphan vg here */
-	orphan_vg = vg_read_orphans(cmd, 0, vg->fid->fmt->orphan_vg_name);
+	orphan_vg = vg_read_orphans(cmd, vg->fid->fmt->orphan_vg_name);
 
-	if (vg_read_error(orphan_vg))
+	if (!orphan_vg)
 		goto bad;
 
 	if (!vg_split_mdas(cmd, vg, orphan_vg) || !vg->pv_count) {

lib/metadata/vg.h

@@ -122,11 +122,6 @@ struct volume_group {
 	struct dm_list removed_pvs;
 	uint32_t open_mode; /* FIXME: read or write - check lock type? */
 
-	/*
-	 * Store result of the last vg_read().
-	 * 0 for success else appropriate FAILURE_* bits set.
-	 */
-	uint32_t read_status;
 	uint32_t mda_copies; /* target number of mdas for this VG */
 
 	struct dm_hash_table *hostnames; /* map of creation hostnames */

test/shell/inconsistent-metadata.sh

@@ -24,53 +24,48 @@ lvchange -a n $vg/mirror
 
 aux backup_dev "${DEVICES[@]}"
 
-init() {
+makeold() {
+	# reset metadata on all devs to starting condition
 	aux restore_dev "${DEVICES[@]}"
 	not check lv_field $vg/resized lv_size "8.00m"
+	# change the metadata on all devs
 	lvresize -L 8192K $vg/resized
+	# reset metadata on just dev1 to the previous version
 	aux restore_dev "$dev1"
 }
 
-init
+# create old metadata
-vgscan 2>&1 | tee cmd.out
+makeold
-grep "Inconsistent metadata found for VG $vg" cmd.out
-vgscan 2>&1 | tee cmd.out
-not grep "Inconsistent metadata found for VG $vg" cmd.out
-check lv_field $vg/resized lv_size "8.00m"
 
-# vgdisplay fixes
+# reports old metadata
-init
-vgdisplay $vg 2>&1 | tee cmd.out
-grep "Inconsistent metadata found for VG $vg" cmd.out
-vgdisplay $vg 2>&1 | tee cmd.out
-not grep "Inconsistent metadata found for VG $vg" cmd.out
-check lv_field $vg/resized lv_size "8.00m"
-
-# lvs fixes up
-init
-lvs $vg 2>&1 | tee cmd.out
-grep "Inconsistent metadata found for VG $vg" cmd.out
-vgdisplay $vg 2>&1 | tee cmd.out
-not grep "Inconsistent metadata found for VG $vg" cmd.out
-check lv_field $vg/resized lv_size "8.00m"
-
-# vgs fixes up as well
-init
 vgs $vg 2>&1 | tee cmd.out
-grep "Inconsistent metadata found for VG $vg" cmd.out
+grep "ignoring metadata" cmd.out
-vgs $vg 2>&1 | tee cmd.out
-not grep "Inconsistent metadata found for VG $vg" cmd.out
 check lv_field $vg/resized lv_size "8.00m"
 
-echo Check auto-repair of failed vgextend - metadata written to original pv but not new pv
+# corrects old metadata
+lvcreate -l1 -an $vg
+
+# no old report
+vgs $vg 2>&1 | tee cmd.out
+not grep "ignoring metadata" cmd.out
+check lv_field $vg/resized lv_size "8.00m"
+
+
+echo Check auto-repair of failed vgextend
+echo - metadata written to original pv but not new pv
+
 vgremove -f $vg
 pvremove -ff "${DEVICES[@]}"
 pvcreate "${DEVICES[@]}"
+
 aux backup_dev "$dev2"
 vgcreate $SHARED $vg "$dev1"
 vgextend $vg "$dev2"
 aux restore_dev "$dev2"
-vgscan
+
+vgs -o+vg_mda_count $vg
+pvs -o+vg_mda_count
+
 should check compare_fields vgs $vg vg_mda_count pvs "$dev2" vg_mda_count
 
 vgremove -ff $vg

test/shell/lvconvert-repair-cache.sh

@@ -93,6 +93,9 @@ lvconvert --yes --uncache $vg/$lv1
 
 aux enable_dev "$dev2"
 
+# vg was changed while dev was missing
+vgextend --restoremissing $vg "$dev2"
+
 # FIXME: temporary workaround
 lvcreate -L1 -n $lv5 $vg
 lvremove -ff $vg

test/shell/lvconvert-repair-policy.sh

@@ -24,6 +24,8 @@ aux lvmconf 'allocation/maximise_cling = 0' \
 cleanup_() {
 	vgreduce --removemissing $vg
 	for d in "$@"; do aux enable_dev "$d"; done
+	# clear the outdated metadata on enabled devs before we can reuse them
+	vgck --updatemetadata $vg
 	for d in "$@"; do vgextend $vg "$d"; done
 	lvremove -ff $vg/mirror
 	lvcreate -aey --type mirror -m 1 --ignoremonitoring -l 2 -n mirror $vg "$dev1" "$dev2" "$dev3:0"

test/shell/lvconvert-repair-raid.sh

@@ -65,6 +65,7 @@ aux disable_dev "$dev2"
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev2"
+vgck --updatemetadata $vg
 vgextend $vg "$dev2"
 lvremove -ff $vg/$lv1
 
@@ -80,6 +81,7 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev2" "$dev3"
+vgck --updatemetadata $vg
 vgextend $vg "$dev2" "$dev3"
 lvremove -ff $vg/$lv1
 
@@ -96,6 +98,7 @@ aux disable_dev "$dev3"
 vgreduce --removemissing -f $vg
 lvconvert -y --repair $vg/$lv1
 aux enable_dev "$dev3"
+vgck --updatemetadata $vg
 pvcreate -yff "$dev3"
 vgextend $vg "$dev3"
 lvremove -ff $vg/$lv1
@@ -114,6 +117,7 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev3"
+vgck --updatemetadata $vg
 vgextend $vg "$dev3"
 lvremove -ff $vg/$lv1
 
@@ -128,6 +132,7 @@ aux disable_dev "$dev4" "$dev5"
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev4" "$dev5"
+vgck --updatemetadata $vg
 vgextend $vg "$dev4" "$dev5"
 lvremove -ff $vg/$lv1
 
@@ -145,6 +150,7 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev4"
+vgck --updatemetadata $vg
 vgextend $vg "$dev4"
 lvremove -ff $vg/$lv1
 
@@ -163,6 +169,7 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev4"
+vgck --updatemetadata $vg
 vgextend $vg "$dev4"
 lvremove -ff $vg/$lv1
 

test/shell/lvconvert-repair.sh

@@ -106,17 +106,23 @@ lvconvert -y --repair $vg/mirror
 vgreduce --removemissing $vg
 
 aux enable_dev "$dev1"
+# clear the outdated dev before we can reuse it
+vgck --updatemetadata $vg
 vgextend $vg "$dev1"
 aux disable_dev "$dev2"
 lvconvert -y --repair $vg/mirror
 vgreduce --removemissing $vg
 
 aux enable_dev "$dev2"
+# clear the outdated dev before we can reuse it
+vgck --updatemetadata $vg
 vgextend $vg "$dev2"
 aux disable_dev "$dev3"
 lvconvert -y --repair $vg/mirror
 vgreduce --removemissing $vg
 aux enable_dev "$dev3"
+# clear the outdated dev before we can reuse it
+vgck --updatemetadata $vg
 vgextend $vg "$dev3"
 
 vgremove -ff $vg

test/shell/lvm-on-md.sh

@@ -31,6 +31,10 @@ test -f /proc/mdstat && grep -q raid1 /proc/mdstat || \
 
 aux lvmconf 'devices/md_component_detection = 1'
 
+# This stops lvm from taking advantage of hints which
+# will have already excluded md components.
+aux lvmconf 'devices/hints = "none"'
+
 # This stops lvm from asking udev if a dev is an md component.
 # LVM will ask udev if a dev is an md component, but we don't
 # want to rely on that ability in this test.
@@ -61,6 +65,10 @@ check lv_field $vg/$lv1 lv_active "active"
 pvs "$mddev"
 not pvs "$dev1"
 not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
 sleep 1
 
 vgchange -an $vg
@@ -72,12 +80,14 @@ sleep 1
 mdadm --stop "$mddev"
 aux udev_wait
 
-# with md superblock 1.0 this pvs will report duplicates
+# The md components should still be detected and excluded.
-# for the two md legs since the md device itself is not
+not pvs "$dev1"
-# started
+not pvs "$dev2"
-pvs 2>&1 |tee out
+pvs > out
-cat out
+not grep "$dev1" out
-grep "prefers device" out
+not grep "$dev2" out
+
+pvs -vvvv
 
 # should not activate from the md legs
 not vgchange -ay $vg
@@ -104,16 +114,20 @@ not grep "active" out
 mdadm --assemble "$mddev" "$dev1" "$dev2"
 aux udev_wait
 
-# Now that the md dev is online, pvs can see it and
+# Now that the md dev is online, pvs can see it
-# ignore the two legs, so there's no duplicate warning
+# and check for components even if
+# md_component_checks is "start" (which disables
+# most default end-of-device scans)
+aux lvmconf 'devices/md_component_checks = "start"'
 
-pvs 2>&1 |tee out
+not pvs "$dev1"
-cat out
+not pvs "$dev2"
-not grep "prefers device" out
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
 
-vgchange -ay $vg 2>&1 |tee out
+
-cat out
+vgchange -ay $vg
-not grep "prefers device" out
 
 check lv_field $vg/$lv1 lv_active "active"
 
@@ -124,6 +138,9 @@ vgremove -f $vg
 
 aux cleanup_md_dev
 
+# Put this setting back to the default
+aux lvmconf 'devices/md_component_checks = "auto"'
+
 # create 2 disk MD raid0 array
 # by default using metadata format 1.0 with data at the end of device
 # When a raid0 md array is stopped, the components will not look like
@@ -149,6 +166,10 @@ check lv_field $vg/$lv1 lv_active "active"
 pvs "$mddev"
 not pvs "$dev1"
 not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
 sleep 1
 
 vgchange -an $vg
@@ -160,7 +181,14 @@ sleep 1
 mdadm --stop "$mddev"
 aux udev_wait
 
-pvs 2>&1 |tee pvs.out
+# The md components should still be detected and excluded.
+not pvs "$dev1"
+not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
+pvs -vvvv
 
 # should not activate from the md legs
 not vgchange -ay $vg
@@ -187,16 +215,19 @@ not grep "active" out
 mdadm --assemble "$mddev" "$dev1" "$dev2"
 aux udev_wait
 
-# Now that the md dev is online, pvs can see it and
+# Now that the md dev is online, pvs can see it
-# ignore the two legs, so there's no duplicate warning
+# and check for components even if
+# md_component_checks is "start" (which disables
+# most default end-of-device scans)
+aux lvmconf 'devices/md_component_checks = "start"'
 
-pvs 2>&1 |tee out
+not pvs "$dev1"
-cat out
+not pvs "$dev2"
-not grep "prefers device" out
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
 
 vgchange -ay $vg 2>&1 |tee out
-cat out
-not grep "prefers device" out
 
 check lv_field $vg/$lv1 lv_active "active"
 

test/shell/lvmcache-exercise.sh

@@ -43,14 +43,16 @@ pvs "$dev1"
 lvcreate -aey -m2 --type mirror -l4 --alloc anywhere --corelog -n $lv1 $vg2
 
 aux disable_dev "$dev3"
+
+pvs 2>&1| tee out
+grep "is missing PV" out
+
 lvconvert --yes --repair $vg2/$lv1
+
 aux enable_dev "$dev3"
 
-# here it should fix any reappeared devices
-lvs
-
 lvs -a $vg2 -o+devices 2>&1 | tee out
-not grep reappeared out
+not grep "is missing PV" out
 
 # This removes the first "vg1" using its uuid
 vgremove -ff -S vg_uuid=$UUID1

test/shell/metadata-bad-mdaheader.sh

@@ -0,0 +1,80 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2008-2013,2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. lib/inittest
+
+aux prepare_devs 3
+get_devs
+
+#
+# Test corrupted mda_header.version field, which also
+# causes the mda_header checksum to be bad.
+#
+# FIXME: if a VG has only a single PV, this repair
+# doesn't work since there's no good PV to get
+# metadata from.  A more advanced repair capability
+# is needed.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+if [ -e "/usr/bin/xxd" ]; then
+
+# read mda_header which is 4k from start of disk
+dd if="$dev1" of=meta1 bs=4k count=1 skip=1
+
+# convert binary to text
+xxd meta1 > meta1.txt
+
+# Corrupt mda_header by changing the version field from 0100 to 0200
+sed 's/0000010:\ 304e\ 2a3e\ 0100\ 0000\ 0010\ 0000\ 0000\ 0000/0000010:\ 304e\ 2a3e\ 0200\ 0000\ 0010\ 0000\ 0000\ 0000/' meta1.txt > meta1-bad.txt
+
+# convert text to binary
+xxd -r meta1-bad.txt > meta1-bad
+
+# write bad mda_header back to disk
+dd if=meta1-bad of="$dev1" bs=4k seek=1
+
+# pvs reports bad metadata header
+pvs 2>&1 | tee out
+grep "bad metadata header" out
+
+fi
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in one mda doesn't prevent using
+# the VG since other mdas are fine and usable
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata header" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+

test/shell/metadata-bad-text.sh

@@ -0,0 +1,236 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2008-2013,2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. lib/inittest
+
+aux prepare_devs 3
+get_devs
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+dd if="$dev1" of=meta1 bs=4k count=2
+
+sed 's/flags =/flagx =/' meta1 > meta1.bad
+
+dd if=meta1.bad of="$dev1"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in one mda doesn't prevent using
+# the VG since other mdas are fine and usable
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+
+#
+# Same test as above, but corrupt metadata text
+# on two of the three PVs, leaving one good
+# copy of the metadata.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+dd if="$dev1" of=meta1 bs=4k count=2
+dd if="$dev2" of=meta2 bs=4k count=2
+
+sed 's/READ/RRRR/' meta1 > meta1.bad
+sed 's/seqno =/sss =/' meta2 > meta2.bad
+
+dd if=meta1.bad of="$dev1"
+dd if=meta2.bad of="$dev2"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out > out2
+grep "$dev1" out2
+grep "$dev2" out2
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in one mda doesn't prevent using
+# the VG since other mdas are fine
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+#
+# Three PVs where two have one mda, and the third
+# has two mdas.  The first mda is corrupted on all
+# thee PVs, but the second mda on the third PV
+# makes the VG usable.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+dd if="$dev1" of=meta1 bs=4k count=2
+dd if="$dev2" of=meta2 bs=4k count=2
+dd if="$dev3" of=meta3 bs=4k count=2
+
+sed 's/READ/RRRR/' meta1 > meta1.bad
+sed 's/seqno =/sss =/' meta2 > meta2.bad
+sed 's/id =/id/' meta3 > meta3.bad
+
+dd if=meta1.bad of="$dev1"
+dd if=meta2.bad of="$dev2"
+dd if=meta3.bad of="$dev3"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out > out2
+grep "$dev1" out2
+grep "$dev2" out2
+grep "$dev3" out2
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in some mdas doesn't prevent using
+# the VG if there's a good mda found
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+#
+# Test that vgck --updatemetadata will update old metadata
+# and repair bad metadata text at the same time from different
+# devices.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+# Put bad metadata onto dev1
+dd if="$dev1" of=meta1 bs=4k count=2
+sed 's/READ/RRRR/' meta1 > meta1.bad
+dd if=meta1.bad of="$dev1"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out > out2
+grep "$dev1" out2
+
+# We can still use the VG with other available
+# mdas, skipping the bad mda.
+
+lvcreate -n $lv1 -l1 -an $vg "$dev1"
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+# Put old metadata onto dev2 by updating
+# the VG while dev2 is disabled.
+aux disable_dev "$dev2"
+
+pvs
+pvs "$dev1"
+not pvs "$dev2"
+pvs "$dev3"
+lvs $vg/$lv1
+lvs $vg/$lv2
+
+lvremove $vg/$lv2
+
+aux enable_dev "$dev2"
+
+# Both old and bad metadata are reported.
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+grep "bad metadata text" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+# fixes the bad metadata on dev1, and
+# fixes the old metadata on dev2.
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+not grep "bad metadata text" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+
+vgchange -an $vg
+vgremove -ff $vg
+

test/shell/metadata-old.sh

@@ -0,0 +1,177 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2008-2013,2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. lib/inittest
+
+aux prepare_devs 3
+get_devs
+
+#
+# Test "old metadata" repair which occurs when the VG is written
+# and one of the PVs in the VG does not get written to, and then
+# the PV reappears with the old metadata.  This can happen if
+# a command is killed or crashes after writing new metadata to
+# only some of the PVs in the VG, or if a PV is temporarily
+# inaccessible while a VG is written.
+#
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+#
+# Test that vgck --updatemetadata will update old metadata.
+#
+
+lvcreate -n $lv1 -l1 -an $vg "$dev1"
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+aux disable_dev "$dev2"
+
+pvs
+pvs "$dev1"
+not pvs "$dev2"
+pvs "$dev3"
+lvs $vg/$lv1
+lvs $vg/$lv2
+
+lvremove $vg/$lv2
+
+aux enable_dev "$dev2"
+
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+# fixes the old metadata on dev1
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+#
+# Test that any writing command will also update the
+# old metadata.
+#
+
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+aux disable_dev "$dev2"
+
+pvs
+pvs "$dev1"
+not pvs "$dev2"
+pvs "$dev3"
+lvs $vg/$lv1
+lvs $vg/$lv2
+
+lvremove $vg/$lv2
+
+aux enable_dev "$dev2"
+
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+# fixes the old metadata on dev1
+lvcreate -n $lv3 -l1 -an $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+vgremove -ff $vg
+
+#
+# First two PVs with one mda, where both have old metadata.
+# Third PV with two mdas, where the first mda has old
+# metadata, and the second mda has current metadata.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+lvcreate -n $lv1 -l1 -an $vg "$dev3"
+lvcreate -n $lv2 -l1 -an $vg "$dev3"
+
+# Save the metadata at this point...
+dd if="$dev1" of=meta1 bs=4k count=4
+dd if="$dev2" of=meta2 bs=4k count=4
+dd if="$dev3" of=meta3 bs=4k count=4
+
+# and now change metadata so the saved copies are old
+lvcreate -n $lv3 -l1 -an $vg "$dev3"
+
+# Copy the saved metadata back to the three
+# devs first mda, leaving the second mda on
+# dev3 as the only latest copy of the metadata.
+
+dd if=meta1 of="$dev1"
+dd if=meta2 of="$dev2"
+dd if=meta3 of="$dev3"
+
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# We still see the three LVs since we are using
+# the latest copy of metadata from dev3:mda2
+
+lvs $vg/$lv1
+lvs $vg/$lv2
+lvs $vg/$lv3
+
+# This command which writes the VG should update
+# all of the old copies.
+lvcreate -n $lv4 -l1 -an $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+lvs $vg/$lv2
+lvs $vg/$lv3
+lvs $vg/$lv4
+
+vgchange -an $vg
+vgremove -ff $vg

test/shell/mirror-vgreduce-removemissing.sh

@@ -123,8 +123,16 @@ check_and_cleanup_lvs_()
 recover_vg_()
 {
 	aux enable_dev "$@"
+
+	# clear outdated metadata on PVs so they can be used again
+	vgck --updatemetadata $vg
+
+	pvscan --cache
+
 	pvcreate -ff "$@"
+	# wipefs -a "$@"
 	vgextend $vg "$@"
+
 	check_and_cleanup_lvs_
 }
 

test/shell/missing-pv-unused.sh

@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2008-2013,2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. lib/inittest
+
+aux prepare_devs 3
+get_devs
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+lvcreate -n $lv1 -L8M $vg "$dev2"
+lvcreate -n $lv2 -L8M $vg "$dev3"
+lvcreate -n $lv3 -L8M $vg "$dev2"
+lvcreate -n $lv4 -L8M $vg "$dev3"
+
+vgchange -an $vg
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Fail device that is not used by any LVs.
+aux disable_dev "$dev1"
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Cannot do normal activation of LVs not using failed PV.
+lvchange -ay $vg/$lv1
+lvchange -ay $vg/$lv2
+
+vgchange -an $vg
+
+# Check that MISSING flag is not set in ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+not grep MISSING meta
+rm meta
+
+pvs
+vgs
+lvs -a -o+devices
+
+# lvremove is one of the few commands that is allowed to run
+# when PVs are missing.  The vg_write from this command sets
+# the MISSING flag on the PV in the ondisk metadata.
+# (this could be changed, the MISSING flag wouldn't need
+# to be set in the first place since the PV isn't used.)
+lvremove $vg/$lv1
+
+# Check that MISSING flag is set in ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+# with MISSING flag in metadata, restrictions apply
+not lvcreate -l1 $vg
+
+aux enable_dev "$dev1"
+
+# No LVs are using the PV with MISSING flag, so no restrictions
+# are applied, and the vg_write here clears the MISSING flag on disk.
+lvcreate -l1 $vg
+
+# Check that MISSING flag is not set in ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+not grep MISSING meta
+rm meta
+
+
+pvs
+vgs
+lvs -a -o+devices
+
+vgchange -an $vg
+vgremove -ff $vg
+

test/shell/missing-pv.sh

@@ -0,0 +1,152 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2008-2013,2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. lib/inittest
+
+aux prepare_devs 3
+get_devs
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+lvcreate -n $lv1 -L8M --type mirror -m 1 $vg
+lvcreate -n $lv2 -L8M --type mirror -m 1 $vg
+
+vgchange -an $vg
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Fail one leg of each mirror LV.
+aux disable_dev "$dev1"
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Cannot do normal activate of either LV with a failed leg.
+not lvchange -ay $vg/$lv1
+not lvchange -ay $vg/$lv2
+
+# Can activate with partial option.
+lvchange -ay --activationmode partial $vg/$lv1
+lvchange -ay --activationmode partial $vg/$lv2
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Repair lv1 so it no longer uses failed dev.
+lvconvert --repair --yes $vg/$lv1
+
+# Check that MISSING flag is set in ondisk metadata,
+# it should have been written by the lvconvert since the
+# missing PV is still used by lv2.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Verify normal activation is possible of lv1 since it's
+# not using any failed devs, and partial activation is
+# required for lv2 since it's still using the failed dev.
+vgchange -an $vg
+lvchange -ay $vg/$lv1
+not lvchange -ay $vg/$lv2
+vgchange -an $vg
+
+aux enable_dev "$dev1"
+
+pvs
+vgs
+lvs -a -o+devices
+
+# TODO: check that lv2 has partial flag, lv1 does not
+# (there's no partial reporting option, only attr p.)
+
+# Check that MISSING flag is still set in ondisk
+# metadata since the previously missing dev is still
+# used by lv2.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+
+# The missing pv restrictions still apply even after
+# the dev has reappeared since it has the MISSING flag.
+not lvchange -ay $vg/$lv2
+not lvcreate -l1 $vg
+
+# Update old metadata on the previously missing PV.
+# This should not clear the MISSING flag because the
+# previously missing PV is still used by lv2.
+# This would be done by any command that writes
+# metadata, e.g. lvcreate, but since we are in a
+# state with a missing pv, most commands that write
+# metadata are restricted, so use a command that
+# explicitly writes/fixes metadata.
+vgck --updatemetadata $vg
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Check that MISSING flag is still set in ondisk
+# metadata since the previously missing dev is still
+# used by lv2.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+
+# The missing pv restrictions still apply since it
+# has the MISSING flag.
+not lvchange -ay $vg/$lv2
+not lvcreate -l1 $vg
+
+lvchange -ay --activationmode partial  $vg/$lv2
+
+# After repair, no more LVs will be using the previously
+# missing PV.
+lvconvert --repair --yes $vg/$lv2
+
+pvs
+vgs
+lvs -a -o+devices
+
+vgchange -an $vg
+
+# The next write of the metadata will clear the MISSING
+# flag in ondisk metadata because the previously missing
+# PV is no longer used by any LVs.
+
+# Run a command to write ondisk metadata, which should clear
+# the MISSING flag, could also use vgck --updatemetadata vg.
+lvcreate -l1 $vg
+
+# Check that the MISSING flag is no longer set
+# in the ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+not grep MISSING meta
+rm meta
+
+
+pvs
+vgs
+lvs -a -o+devices
+
+vgchange -an $vg
+vgremove -ff $vg
+

test/shell/outdated-pv.sh

@@ -0,0 +1,66 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2008-2013,2018 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. lib/inittest
+
+aux prepare_devs 3
+get_devs
+
+#
+# Test handling of "outdated PV" which occurs when a PV goes missing
+# from a VG, and while it's missing the PV is removed from the VG.
+# Then the PV reappears with the old VG metadata that shows it is a
+# member.  That outdated metadata needs to be cleared.
+#
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+lvcreate -n $lv1 -l1 -an $vg "$dev1"
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+aux disable_dev "$dev2"
+
+vgreduce --removemissing $vg
+
+pvs
+
+aux enable_dev "$dev2"
+
+pvs 2>&1 | tee out
+grep "outdated" out
+
+not pvs "$dev2"
+
+# The VG can still be used with the outdated PV around
+lvcreate -n $lv3 -l1 $vg
+lvchange -ay $vg
+lvs $vg
+lvchange -an $vg
+
+# Clears the outdated PV
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "outdated" out
+
+# The PV is no longer in the VG
+pvs "$dev2" | tee out
+not grep "$vg" out
+
+# The cleared PV can be added back to the VG
+vgextend $vg "$dev2"
+
+pvs "$dev2" | tee out
+grep "$vg" out
+
+vgremove -ff $vg
+

test/shell/pv-ext-flags.sh

@@ -39,7 +39,6 @@ check pv_field "$dev2" pv_in_use "used"
 # disable $dev2 and dev1 with 0 MDAs remains, but still
 # marked as used, so pvcreate/vgcreate/pvremove should fail
 aux disable_dev "$dev2"
-pvscan --cache
 
 check pv_field "$dev1" pv_in_use "used"
 not pvcreate "$dev1" 2>err
@@ -71,20 +70,14 @@ vgcreate $vg1 "$dev1" "$dev2"
 # disable $dev1, then repair the VG - $dev1 is removed from VG
 aux disable_dev "$dev1"
 vgreduce --removemissing $vg1
-# now, enable $dev1, automatic repair will happen on pvs call
-# (or any other lvm command that does vg_read with repair inside)
-aux enable_dev "$dev1"
 
-# FIXME: once persistent cache does not cause races with timestamps
+# now, enable $dev1 and clear the old metadata from it
-#        causing LVM tools to not see the VG inconsistency and once
+aux enable_dev "$dev1"
-#        VG repair is always done, delete this line which removes
+vgck --updatemetadata $vg1
-#        persistent .cache as a workaround
-rm -f "$TESTDIR/etc/.cache"
 
 vgck $vg1
-# check $dev1 does not contain the PV_EXT_FLAG anymore - it
+
-# should be removed as part of the repaid during vg_read since
+# check $dev1 does not contain the PV_EXT_FLAG anymore
-# $dev1 is not part of $vg1 anymore
 check pv_field "$dev1" pv_in_use ""
 
 #############################################
@@ -105,7 +98,6 @@ check pv_field "$dev2" pv_in_use "used"
 
 pvchange --metadataignore y "$dev1"
 aux disable_dev "$dev2"
-pvscan --cache
 
 check pv_field "$dev1" pv_in_use "used"
 not pvcreate "$dev1" 2>err
@@ -136,20 +128,14 @@ vgcreate $vg1 "$dev1" "$dev2"
 # disable $dev1, then repair the VG - $dev1 is removed from VG
 aux disable_dev "$dev1"
 vgreduce --removemissing $vg1
-# now, enable $dev1, automatic repair will happen on pvs call
-# (or any other lvm command that does vg_read with repair inside)
-aux enable_dev "$dev1"
 
-# FIXME: once persistent cache does not cause races with timestamps
+# now, enable $dev1 and clear the old metadata from it
-#        causing LVM tools to not see the VG inconsistency and once
+aux enable_dev "$dev1"
-#        VG repair is always done, delete this line which removes
+vgck --updatemetadata $vg1
-#        persistent .cache as a workaround
-rm -f "$TESTDIR/etc/.cache"
 
 vgck $vg1
-# check $dev1 does not contain the PV_EXT_FLAG anymore - it
+
-# should be removed as part of the repaid during vg_read since
+# check $dev1 does not contain the PV_EXT_FLAG anymore
-# $dev1 is not part of $vg1 anymore
 check pv_field "$dev1" pv_in_use ""
 
 ###########################

test/shell/unlost-pv.sh

@@ -15,47 +15,59 @@ SKIP_WITH_LVMPOLLD=1
 
 . lib/inittest
 
-check_() {
-	local cache=""
-	# vgscan needs --cache option for direct scan if lvmetad is used
-	test -e LOCAL_LVMETAD && cache="--cache"
-	vgscan $cache 2>&1 | tee vgscan.out
-	"$@" grep "Inconsistent metadata found for VG $vg" vgscan.out
-}
-
 aux prepare_vg 3
 
 lvcreate -an -Zn --type mirror -m 1 -l 1 -n mirror $vg
-#lvchange -a n $vg
 
 # try orphaning a missing PV (bz45867)
 aux disable_dev "$dev1"
 vgreduce --removemissing --force $vg
 aux enable_dev "$dev1"
 
-check_
+vgscan 2>&1 | tee vgscan.out
-test -e LOCAL_LVMETAD && pvcreate -f "$dev1"
+grep "Inconsistent metadata found for VG $vg" vgscan.out
-check_ not
+
+# erase outdated dev1
+vgck --updatemetadata $vg
+
+vgscan 2>&1 | tee vgscan.out
+not grep "Inconsistent metadata found for VG $vg" vgscan.out
+
 
-# try to just change metadata; we expect the new version (with MISSING_PV set
-# on the reappeared volume) to be written out to the previously missing PV
 vgextend $vg "$dev1"
+
 lvcreate -l 1 -n boo -a n --zero n $vg
-aux disable_dev "$dev1"
-lvremove $vg/mirror
-aux enable_dev "$dev1"
-check_
-test -e LOCAL_LVMETAD && lvremove $vg/boo # FIXME trigger a write :-(
-check_ not
 
 aux disable_dev "$dev1"
+
+lvremove $vg/mirror
+
+aux enable_dev "$dev1"
+
+vgscan 2>&1 | tee vgscan.out
+grep "Inconsistent metadata found for VG $vg" vgscan.out
+
+# write the vg to update the metadata on dev1
+vgck --updatemetadata $vg
+
+vgscan 2>&1 | tee vgscan.out
+not grep "Inconsistent metadata found for VG $vg" vgscan.out
+
+aux disable_dev "$dev1"
+
 vgreduce --removemissing --force $vg
+
 aux enable_dev "$dev1"
 
 vgscan 2>&1 | tee out
-grep 'Removing PV' out
 
-vgs 2>&1 | tee out
+vgscan 2>&1 | tee vgscan.out
-not grep 'Removing PV' out
+grep "Inconsistent metadata found for VG $vg" vgscan.out
+
+# erase outdated dev1
+vgck --updatemetadata $vg
+
+vgscan 2>&1 | tee vgscan.out
+not grep "Inconsistent metadata found for VG $vg" vgscan.out
 
 vgremove -ff $vg

test/shell/vgck.sh

@@ -24,11 +24,11 @@ dd if=/dev/urandom bs=512 seek=2 count=32 of="$dev2"
 
 vgscan 2>&1 | tee vgscan.out || true
 
-grep "Failed" vgscan.out
+grep "checksum" vgscan.out
 
 dd if=/dev/urandom bs=512 seek=2 count=32 of="$dev2"
 
 vgck $vg 2>&1 | tee vgck.out || true
-grep Incorrect vgck.out
+grep "checksum" vgck.out
 
 vgremove -ff $vg

tools/args.h

@@ -1393,6 +1393,9 @@ arg(thin_ARG, 'T', "thin", 0, 0, 0,
     "See --type thin, --type thin-pool, and --virtualsize.\n"
     "See \\fBlvmthin\\fP(7) for more information about LVM thin provisioning.\n")
 
+arg(updatemetadata_ARG, '\0', "updatemetadata", 0, 0, 0,
+    "Update VG metadata to correct problems.\n")
+
 arg(uuid_ARG, 'u', "uuid", 0, 0, 0,
     "#pvchange\n"
     "Generate new random UUID for specified PVs.\n"

tools/command-lines.in

@@ -1624,6 +1624,11 @@ vgck
 OO: --reportformat ReportFmt
 OP: VG|Tag ...
 ID: vgck_general
+DESC: Read and display information about a VG.
+
+vgck --updatemetadata VG
+ID: vgck_update_metadata
+DESC: Rewrite VG metadata to correct problems.
 
 ---
 

tools/commands.h

@@ -177,7 +177,7 @@ xx(vgchange,
 
 xx(vgck,
    "Check the consistency of volume group(s)",
-   ALL_VGS_IS_DEFAULT | LOCKD_VG_SH | ALLOW_HINTS)
+   ALL_VGS_IS_DEFAULT | LOCKD_VG_SH)
 
 xx(vgconvert,
    "Change volume group metadata format",

tools/lvmcmdline.c

@@ -2766,20 +2766,53 @@ static int _init_lvmlockd(struct cmd_context *cmd)
 	return 1;
 }
 
+/*
+ * md_component_check full: always set use_full_md_check
+ * which causes filter-md to read the start+end of every
+ * device on the system (this could be optimized to only
+ * read the end of PVs.)
+ *
+ * md_component_check start: the end of devices will
+ * not generally be read to check for an md superblock
+ * (lvm may still scan for end-of-device md superblocks
+ * if it knows that some exists.)
+ *
+ * md_component_check auto: lvm will use some built-in
+ * heuristics to decide when it should scan the end of
+ * devices to look for md superblocks, e.g. commands
+ * like pvcreate that could clobber a component, or if
+ * udev info is not available and hints are not available.
+ */
 static void _init_md_checks(struct cmd_context *cmd)
 {
-	/*
+	const char *md_check;
-	 * use_full_md_check can also be set later.
+
-	 * These commands are chosen to always perform
+	cmd->md_component_detection = find_config_tree_bool(cmd, devices_md_component_detection_CFG, NULL);
-	 * a full md component check because they initialize
+
-	 * a new device that could be an md component,
+	md_check = find_config_tree_str(cmd, devices_md_component_checks_CFG, NULL);
-	 * and they are not run frequently during normal
+	if (!md_check)
-	 * operation.
+		cmd->md_component_checks = "auto";
-	 */
+	else if (!strcmp(md_check, "auto") ||
-	if (!strcmp(cmd->name, "pvcreate") ||
+	         !strcmp(md_check, "start") ||
-	    !strcmp(cmd->name, "vgcreate") ||
+	         !strcmp(md_check, "full"))
-	    !strcmp(cmd->name, "vgextend"))
+		cmd->md_component_checks = md_check;
+	else {
+		log_warn("Ignoring unknown md_component_checks setting, using auto.");
+		cmd->md_component_checks = "auto";
+	}
+
+	if (!strcmp(cmd->md_component_checks, "full"))
 		cmd->use_full_md_check = 1;
+	else if (!strcmp(cmd->md_component_checks, "auto")) {
+		/* use_full_md_check can also be set later */
+		if (!strcmp(cmd->name, "pvcreate") ||
+		    !strcmp(cmd->name, "vgcreate") ||
+		    !strcmp(cmd->name, "vgextend"))
+			cmd->use_full_md_check = 1;
+	}
+
+	log_debug("Using md_component_checks %s use_full_md_check %d",
+		  cmd->md_component_checks, cmd->use_full_md_check);
 }
 
 static int _cmd_no_meta_proc(struct cmd_context *cmd)

tools/polldaemon.c

@@ -148,6 +148,7 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
 	struct logical_volume *lv;
 	int finished = 0;
 	uint32_t lockd_state = 0;
+	uint32_t error_flags = 0;
 	int ret;
 
 	if (!parms->wait_before_testing)
@@ -168,12 +169,10 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
 		}
 
 		/* Locks the (possibly renamed) VG again */
-		vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state);
+		vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state, &error_flags, NULL);
-		if (vg_read_error(vg)) {
+		if (!vg) {
 			/* What more could we do here? */
-			log_error("ABORTING: Can't reread VG for %s.", id->display_name);
+			log_error("ABORTING: Can't reread VG for %s error flags %x.", id->display_name, error_flags);
-			release_vg(vg);
-			vg = NULL;
 			ret = 0;
 			goto out;
 		}
@@ -395,6 +394,7 @@ static int _report_progress(struct cmd_context *cmd, struct poll_operation_id *i
 	struct volume_group *vg;
 	struct logical_volume *lv;
 	uint32_t lockd_state = 0;
+	uint32_t error_flags = 0;
 	int ret;
 
 	/*
@@ -407,10 +407,9 @@ static int _report_progress(struct cmd_context *cmd, struct poll_operation_id *i
 	 * change done locally.
 	 */
 
-	vg = vg_read(cmd, id->vg_name, NULL, 0, lockd_state);
+	vg = vg_read(cmd, id->vg_name, NULL, 0, lockd_state, &error_flags, NULL);
-	if (vg_read_error(vg)) {
+	if (!vg) {
-		release_vg(vg);
+		log_error("Can't reread VG for %s error flags %x", id->display_name, error_flags);
-		log_error("Can't reread VG for %s", id->display_name);
 		ret = 0;
 		goto out_ret;
 	}

tools/toollib.c

@@ -189,11 +189,12 @@ static int _printed_clustered_vg_advice = 0;
  * Case c covers the other errors returned when reading the VG.
  *   If *skip is 1, it's OK for the caller to read the list of PVs in the VG.
  */
-static int _ignore_vg(struct volume_group *vg, const char *vg_name,
+static int _ignore_vg(struct cmd_context *cmd,
-		      struct dm_list *arg_vgnames, uint32_t read_flags,
+		      uint32_t error_flags, struct volume_group *error_vg,
-		      int *skip, int *notfound)
+		      const char *vg_name, struct dm_list *arg_vgnames,
+		      uint32_t read_flags, int *skip, int *notfound)
 {
-	uint32_t read_error = vg_read_error(vg);
+	uint32_t read_error = error_flags;
 
 	*skip = 0;
 	*notfound = 0;
@@ -203,12 +204,9 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 		return 0;
 	}
 
-	if ((read_error & FAILED_INCONSISTENT) && (read_flags & READ_ALLOW_INCONSISTENT))
-		read_error &= ~FAILED_INCONSISTENT; /* Check for other errors */
-
 	if (read_error & FAILED_CLUSTERED) {
-		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg_name)) {
-			log_error("Cannot access clustered VG %s.", vg->name);
+			log_error("Cannot access clustered VG %s.", vg_name);
 			if (!_printed_clustered_vg_advice) {
 				_printed_clustered_vg_advice = 1;
 				log_error("See lvmlockd(8) for changing a clvm/clustered VG to a shared VG.");
@@ -233,10 +231,13 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 	 * would expect to fail.
 	 */
 	if (read_error & FAILED_SYSTEMID) {
-		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg_name)) {
 			log_error("Cannot access VG %s with system ID %s with %slocal system ID%s%s.",
-				  vg->name, vg->system_id, vg->cmd->system_id ? "" : "unknown ",
+				  vg_name,
-				  vg->cmd->system_id ? " " : "", vg->cmd->system_id ? vg->cmd->system_id : "");
+				  error_vg ? error_vg->system_id : "unknown ",
+				  cmd->system_id ? "" : "unknown ",
+				  cmd->system_id ? " " : "",
+				  cmd->system_id ? cmd->system_id : "");
 			return 1;
 		} else {
 			read_error &= ~FAILED_SYSTEMID; /* Check for other errors */
@@ -255,10 +256,11 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 	 * command failed to acquire the necessary lock.)
 	 */
 	if (read_error & (FAILED_LOCK_TYPE | FAILED_LOCK_MODE)) {
-		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg_name)) {
 			if (read_error & FAILED_LOCK_TYPE)
 				log_error("Cannot access VG %s with lock type %s that requires lvmlockd.",
-					  vg->name, vg->lock_type);
+					  vg_name,
+					  error_vg ? error_vg->lock_type : "unknown");
 			/* For FAILED_LOCK_MODE, the error is printed in vg_read. */
 			return 1;
 		} else {
@@ -1924,10 +1926,12 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 	log_report_t saved_log_report_state = log_get_report_state();
 	char uuid[64] __attribute__((aligned(8)));
 	struct volume_group *vg;
+	struct volume_group *error_vg = NULL;
 	struct vgnameid_list *vgnl;
 	const char *vg_name;
 	const char *vg_uuid;
 	uint32_t lockd_state = 0;
+	uint32_t error_flags = 0;
 	int whole_selected = 0;
 	int ret_max = ECMD_PROCESSED;
 	int ret;
@@ -1977,13 +1981,18 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 			continue;
 		}
 
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state, &error_flags, &error_vg);
-		if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
+		if (_ignore_vg(cmd, error_flags, error_vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
+			if (error_vg)
+				unlock_and_release_vg(cmd, error_vg, vg_name);
 			goto endvg;
 		}
+		if (error_vg)
+			unlock_and_release_vg(cmd, error_vg, vg_name);
+
 		if (skip || notfound)
 			goto endvg;
 
@@ -2004,8 +2013,7 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 				ret_max = ret;
 		}
 
-		if (!vg_read_error(vg))
+		unlock_vg(cmd, vg, vg_name);
-			unlock_vg(cmd, vg, vg_name);
 endvg:
 		release_vg(vg);
 		if (!lockd_vg(cmd, vg_name, "un", 0, &lockd_state))
@@ -3590,11 +3598,13 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag
 	log_report_t saved_log_report_state = log_get_report_state();
 	char uuid[64] __attribute__((aligned(8)));
 	struct volume_group *vg;
+	struct volume_group *error_vg = NULL;
 	struct vgnameid_list *vgnl;
 	struct dm_str_list *sl;
 	struct dm_list *tags_arg;
 	struct dm_list lvnames;
 	uint32_t lockd_state = 0;
+	uint32_t error_flags = 0;
 	const char *vg_name;
 	const char *vg_uuid;
 	const char *vgn;
@@ -3663,13 +3673,18 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag
 			continue;
 		}
 
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state, &error_flags, &error_vg);
-		if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
+		if (_ignore_vg(cmd, error_flags, error_vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
+			if (error_vg)
+				unlock_and_release_vg(cmd, error_vg, vg_name);
 			goto endvg;
 		}
+		if (error_vg)
+			unlock_and_release_vg(cmd, error_vg, vg_name);
+
 		if (skip || notfound)
 			goto endvg;
 
@@ -4156,12 +4171,16 @@ static int _process_pvs_in_vg(struct cmd_context *cmd,
 	struct physical_volume *pv;
 	struct pv_list *pvl;
 	struct device_id_list *dil;
+	struct device_list *devl;
+	struct dm_list outdated_devs;
 	const char *pv_name;
 	int process_pv;
 	int do_report_ret_code = 1;
 	int ret_max = ECMD_PROCESSED;
 	int ret = 0;
 
+	dm_list_init(&outdated_devs);
+
 	log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_PV);
 
 	vg_uuid[0] = '\0';
@@ -4247,6 +4266,12 @@ static int _process_pvs_in_vg(struct cmd_context *cmd,
 			break;
 		log_set_report_object_name_and_id(NULL, NULL);
 	}
+
+	if (!is_orphan_vg(vg->name))
+		lvmcache_get_outdated_devs(cmd, vg->name, (const char *)&vg->id, &outdated_devs);
+	dm_list_iterate_items(devl, &outdated_devs)
+		_device_list_remove(all_devices, devl->dev);
+
 	do_report_ret_code = 0;
 out:
 	if (do_report_ret_code)
@@ -4284,10 +4309,12 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 	log_report_t saved_log_report_state = log_get_report_state();
 	char uuid[64] __attribute__((aligned(8)));
 	struct volume_group *vg;
+	struct volume_group *error_vg;
 	struct vgnameid_list *vgnl;
 	const char *vg_name;
 	const char *vg_uuid;
 	uint32_t lockd_state = 0;
+	uint32_t error_flags = 0;
 	int ret_max = ECMD_PROCESSED;
 	int ret;
 	int skip;
@@ -4325,8 +4352,8 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 
 		log_debug("Processing PVs in VG %s", vg_name);
 
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state, &error_flags, &error_vg);
-		if (_ignore_vg(vg, vg_name, NULL, read_flags, &skip, &notfound)) {
+		if (_ignore_vg(cmd, error_flags, error_vg, vg_name, NULL, read_flags, &skip, &notfound)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
@@ -4338,22 +4365,26 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 			goto endvg;
 		
 		/*
-		 * Don't continue when skip is set, because we need to remove
+		 * Don't call "continue" when skip is set, because we need to remove
-		 * vg->pvs entries from devices list.
+		 * error_vg->pvs entries from devices list.
 		 */
 		
-		ret = _process_pvs_in_vg(cmd, vg, all_devices, arg_devices, arg_tags,
+		ret = _process_pvs_in_vg(cmd, vg ? vg : error_vg, all_devices, arg_devices, arg_tags,
 					 process_all_pvs, process_all_devices, skip,
 					 handle, process_single_pv);
 		if (ret != ECMD_PROCESSED)
 			stack;
+
 		report_log_ret_code(ret);
+
 		if (ret > ret_max)
 			ret_max = ret;
 
 		if (!skip)
 			unlock_vg(cmd, vg, vg->name);
 endvg:
+		if (error_vg)
+			unlock_and_release_vg(cmd, error_vg, vg_name);
 		release_vg(vg);
 		if (!lockd_vg(cmd, vg_name, "un", 0, &lockd_state))
 			stack;
@@ -5574,7 +5605,7 @@ do_command:
 	if (pp->preserve_existing && pp->orphan_vg_name) {
 		log_debug("Using existing orphan PVs in %s.", pp->orphan_vg_name);
 
-		if (!(orphan_vg = vg_read_orphans(cmd, 0, pp->orphan_vg_name))) {
+		if (!(orphan_vg = vg_read_orphans(cmd, pp->orphan_vg_name))) {
 			log_error("Cannot read orphans VG %s.", pp->orphan_vg_name);
 			goto bad;
 		}

tools/vgcfgbackup.c

@@ -67,9 +67,12 @@ static int _vg_backup_single(struct cmd_context *cmd, const char *vg_name,
 		if (!backup_to_file(filename, vg->cmd->cmd_line, vg))
 			return_ECMD_FAILED;
 	} else {
-		if (vg_read_error(vg) == FAILED_INCONSISTENT) {
+		if (vg_missing_pv_count(vg)) {
-			log_error("No backup taken: specify filename with -f "
+			log_error("No backup taken: specify filename with -f to backup with missing PVs.");
-				  "to backup an inconsistent VG");
+			return ECMD_FAILED;
+		}
+		if (vg_has_unknown_segments(vg)) {
+			log_error("No backup taken: specify filename with -f to backup with unknown segments.");
 			return ECMD_FAILED;
 		}
 
@@ -97,9 +100,17 @@ int vgcfgbackup(struct cmd_context *cmd, int argc, char **argv)
 
 	handle->custom_handle = &last_filename;
 
+	/*
+	 * Just set so that we can do the check ourselves above and
+	 * report a helpful error message in place of the error message
+	 * that would be generated from vg_read.
+	 */
+	cmd->handles_missing_pvs = 1;
+	cmd->handles_unknown_segments = 1;
+
 	init_pvmove(1);
 
-	ret = process_each_vg(cmd, argc, argv, NULL, NULL, READ_ALLOW_INCONSISTENT, 0,
+	ret = process_each_vg(cmd, argc, argv, NULL, NULL, 0, 0,
 			      handle, &_vg_backup_single);
 
 	free(last_filename);

tools/vgck.c

@@ -15,6 +15,57 @@
 
 #include "tools.h"
 
+/*
+ * TODO: we cannot yet repair corruption in label_header, pv_header/locations,
+ * or corruption of some mda_header fields.
+ */
+
+static int _update_metadata_single(struct cmd_context *cmd __attribute__((unused)),
+		       const char *vg_name,
+		       struct volume_group *vg,
+		       struct processing_handle *handle __attribute__((unused)))
+{
+
+	/*
+	 * Simply calling vg_write can correct or clean up various things:
+	 * . some mda's have old versions of metdadata 
+	 * . wipe outdated PVs
+	 * . fix pv_header used flag and version
+	 * . strip historical lvs
+	 * . clear missing pv flag on unused PV
+	 */
+	if (!vg_write(vg)) {
+		log_error("Failed to write VG.");
+		return 0;
+	}
+
+	if (!vg_commit(vg)) {
+		log_error("Failed to commit VG.");
+		return 0;
+	}
+
+	/*
+	 * vg_write does not write to "bad" mdas (where "bad" is corrupt, can't
+	 * be processed when reading).  bad mdas are not kept in
+	 * fid->metadata_areas_in_use so vg_read and vg_write ignore them, but
+	 * they are saved in lvmcache.  this gets them from lvmcache and tries
+	 * to write this metadata to them.
+	 */
+	vg_write_commit_bad_mdas(cmd, vg);
+
+	return 1;
+}
+
+static int _update_metadata(struct cmd_context *cmd, int argc, char **argv)
+{
+	cmd->handles_missing_pvs = 1;
+	cmd->wipe_outdated_pvs = 1;
+	cmd->handles_unknown_segments = 1;
+
+	return process_each_vg(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, 0, NULL,
+			       &_update_metadata_single);
+}
+
 static int vgck_single(struct cmd_context *cmd __attribute__((unused)),
 		       const char *vg_name,
 		       struct volume_group *vg,
@@ -37,6 +88,9 @@ static int vgck_single(struct cmd_context *cmd __attribute__((unused)),
 
 int vgck(struct cmd_context *cmd, int argc, char **argv)
 {
+	if (arg_is_set(cmd, updatemetadata_ARG))
+		return _update_metadata(cmd, argc, argv);
+
 	return process_each_vg(cmd, argc, argv, NULL, NULL, 0, 0, NULL,
 			       &vgck_single);
 }

tools/vgextend.c

@@ -28,16 +28,25 @@ static int _restore_pv(struct volume_group *vg, const char *pv_name)
 		return 0;
 	}
 
-	if (!(pvl->pv->status & MISSING_PV)) {
-		log_warn("WARNING: PV %s was not missing in VG %s", pv_name, vg->name);
-		return 0;
-	}
-
 	if (!pvl->pv->dev) {
 		log_warn("WARNING: The PV %s is still missing.", pv_name);
 		return 0;
 	}
 
+	if (pvl->pv->status & MISSING_PV)
+		goto clear_flag;
+
+	/*
+	 * when the PV has no used PE's vg_read clears the MISSING_PV flag
+	 * and sets this so we know.
+	 */
+	if (pvl->pv->unused_missing_cleared)
+		goto clear_flag;
+
+	log_warn("WARNING: PV %s was not missing in VG %s", pv_name, vg->name);
+	return 0;
+
+clear_flag:
 	pvl->pv->status &= ~MISSING_PV;
 	return 1;
 }

tools/vgremove.c

@@ -99,6 +99,8 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv)
 
 	clear_hint_file(cmd);
 
+	cmd->wipe_outdated_pvs = 1;
+
 	cmd->handles_missing_pvs = 1;
 	ret = process_each_vg(cmd, argc, argv, NULL, NULL,
 			      READ_FOR_UPDATE, 0,

tools/vgsplit.c

@@ -456,6 +456,7 @@ static struct volume_group *_vgsplit_to(struct cmd_context *cmd,
 					int *existing_vg)
 {
 	struct volume_group *vg_to = NULL;
+	int exists = 0;
 
 	log_verbose("Checking for new volume group \"%s\"", vg_name_to);
 	/*
@@ -468,13 +469,13 @@ static struct volume_group *_vgsplit_to(struct cmd_context *cmd,
 	 * we obtained a WRITE lock and could not find the vgname in the
 	 * system.  Thus, the split will be into a new VG.
 	 */
-	vg_to = vg_lock_and_create(cmd, vg_name_to);
+	vg_to = vg_lock_and_create(cmd, vg_name_to, &exists);
-	if (vg_read_error(vg_to) == FAILED_LOCKING) {
+	if (!vg_to && !exists) {
 		log_error("Can't get lock for %s", vg_name_to);
 		release_vg(vg_to);
 		return NULL;
 	}
-	if (vg_read_error(vg_to) == FAILED_EXIST) {
+	if (!vg_to && exists) {
 		*existing_vg = 1;
 		release_vg(vg_to);
 		vg_to = vg_read_for_update(cmd, vg_name_to, NULL, 0, 0);