lvmcache: add defective device for bad pv header

If label scan finds bad data in the pv header of
an LVM device, add the device to the defective list.
Previously, lvm ignored the bad data and acted as if
the device was unformatted.

8 files changed, 168 insertions, 41 deletions

diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index df49882af..d1a15e41c 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -27,6 +27,8 @@
 
 #define FAILED_LABEL_CHECKSUM		0x00000001
 #define FAILED_LABEL_SECTOR_NUMBER	0x00000002
+#define FAILED_INTERNAL			0x00000004
+#define FAILED_PV_HEADER		0x00000008
 
 /* LVM specific per-volume info */
 /* Eventual replacement for struct physical_volume perhaps? */
diff --git a/lib/format1/lvm1-label.c b/lib/format1/lvm1-label.c
index a9ccaaf6b..5303e740e 100644
--- a/lib/format1/lvm1-label.c
+++ b/lib/format1/lvm1-label.c
@@ -56,7 +56,7 @@ static int _lvm1_write(struct label *label __attribute__((unused)), void *buf __
 
 static int _lvm1_read(struct labeller *l, struct device *dev, void *buf,
 		 struct label_read_data *ld,
-		 struct label **label)
+		 struct label **label, uint32_t *failed_flags)
 {
 	struct pv_disk *pvd = (struct pv_disk *) buf;
 	struct vg_disk vgd;
diff --git a/lib/format_pool/pool_label.c b/lib/format_pool/pool_label.c
index fa34c2fe1..5c703a130 100644
--- a/lib/format_pool/pool_label.c
+++ b/lib/format_pool/pool_label.c
@@ -57,7 +57,7 @@ static int _pool_write(struct label *label __attribute__((unused)), void *buf __
 
 static int _pool_read(struct labeller *l, struct device *dev, void *buf,
 		 struct label_read_data *ld,
-		 struct label **label)
+		 struct label **label, uint32_t *failed_flags)
 {
 	struct pool_list pl;
 
diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c
index 17be60bd6..6cf23ff06 100644
--- a/lib/format_text/text_label.c
+++ b/lib/format_text/text_label.c
@@ -374,74 +374,166 @@ close_dev:
  */
 
 static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
-		      struct label_read_data *ld, struct label **label)
+		      struct label_read_data *ld, struct label **label,
+		      uint32_t *failed_flags)
 {
+	char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
+	char uuid[64] __attribute__((aligned(8)));
+	struct id pv_id_check;
 	struct label_header *lh = (struct label_header *) label_buf;
 	struct pv_header *pvhdr;
 	struct pv_header_extension *pvhdr_ext;
 	struct lvmcache_info *info;
 	struct disk_locn *dlocn_xl;
 	uint64_t offset;
+	uint64_t device_size;
 	uint32_t ext_version;
-	struct _mda_baton baton;
+	uint32_t ext_flags;
+	unsigned int data_area_count = 0;
+	unsigned int meta_area_count = 0;
+	int add_errors = 0;
+	struct _mda_baton baton = { 0 };
 
 	/*
-	 * PV header base
+	 * pv_header has uuid and device_size
+	 * pv_header.disk_areas are two variable sequences of disk_locn's:
+	 * . first null terminated sequence of disk_locn's are data areas
+	 * . second null terminated sequence of disk_locn's are meta areas
+	 * pv_header_extension has version and flags
+	 * pv_header_extension.bootloader_areas is one set of disk_locn's:
+	 * . null terminated sequence of disk_locn's are bootloader areas
+	 *
+	 * Step 1: look through structs to summarize for log message.
 	 */
 	pvhdr = (struct pv_header *) ((char *) label_buf + xlate32(lh->offset_xl));
 
-	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev,
-				  FMT_TEXT_ORPHAN_VG_NAME,
-				  FMT_TEXT_ORPHAN_VG_NAME, 0)))
+	strncpy(pvid_s, (char *)pvhdr->pv_uuid, sizeof(pvid_s) - 1);
+	pvid_s[sizeof(pvid_s) - 1] = '\0';
+
+	if (!id_read_format_try(&pv_id_check, pvid_s)) {
+		log_debug_metadata("PV header on %s uuid cannot be read.", dev_name(dev));
+		*failed_flags |= FAILED_PV_HEADER;
 		return_0;
+	}
 
-	*label = lvmcache_get_label(info);
+	if (!id_write_format((const struct id *)&pvid_s, uuid, sizeof(uuid))) {
+		log_debug_metadata("PV header on %s uuid cannot be written.", dev_name(dev));
+		*failed_flags |= FAILED_INTERNAL;
+		return_0;
+	}
 
-	lvmcache_set_device_size(info, xlate64(pvhdr->device_size_xl));
+	/*
+	 * FIXME: check for invalid values of other pv_header fields.
+	 */
 
-	lvmcache_del_das(info);
-	lvmcache_del_mdas(info);
-	lvmcache_del_bas(info);
+	device_size = xlate64(pvhdr->device_size_xl);
 
 	/* Data areas holding the PEs */
 	dlocn_xl = pvhdr->disk_areas_xl;
 	while ((offset = xlate64(dlocn_xl->offset))) {
-		lvmcache_add_da(info, offset, xlate64(dlocn_xl->size));
 		dlocn_xl++;
+		data_area_count++;
 	}
 
 	/* Metadata area headers */
 	dlocn_xl++;
 	while ((offset = xlate64(dlocn_xl->offset))) {
-		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0);
 		dlocn_xl++;
+		meta_area_count++;
 	}
 
+	/* PV header extension */
 	dlocn_xl++;
+	pvhdr_ext = (struct pv_header_extension *) ((char *) dlocn_xl);
+	ext_version = xlate32(pvhdr_ext->version);
+	ext_flags = xlate32(pvhdr_ext->flags);
+
+	log_debug_metadata("PV header on %s has device_size %llu uuid %s",
+			   dev_name(dev), (unsigned long long)device_size, uuid);
+
+	log_debug_metadata("PV header on %s has data areas %d metadata areas %d",
+			   dev_name(dev), data_area_count, meta_area_count);
+
+	log_debug_metadata("PV header on %s has extension version %u flags %x",
+			   dev_name(dev), ext_version, ext_flags);
 
 	/*
-	 * PV header extension
+	 * Step 2: look through structs to populate lvmcache
+	 * with pv_header/extension info for this device.
+	 *
+	 * An "info" struct represents a device in lvmcache
+	 * and is created by lvmcache_add().  The info struct
+	 * in lvmcache is not associated with any vginfo
+	 * struct until the VG name is known from the summary.
+	 *
+	 * lvmcache_add() calls _create_info() which creates
+	 * the label struct, saved at info->label.
+	 * lvmcache_get_label(info) then returns info->label.
 	 */
+	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev, FMT_TEXT_ORPHAN_VG_NAME, FMT_TEXT_ORPHAN_VG_NAME, 0))) {
+		log_error("PV %s info cannot be saved in cache.", dev_name(dev));
+		*failed_flags |= FAILED_INTERNAL;
+		return 0;
+	}
+
+	/* get the label that lvmcache_add() created */
+	if (!(*label = lvmcache_get_label(info))) {
+		*failed_flags |= FAILED_INTERNAL;
+		return_0;
+	}
+
+	lvmcache_set_device_size(info, device_size);
+
+	lvmcache_del_das(info);
+	lvmcache_del_mdas(info);
+	lvmcache_del_bas(info);
+
+	/* Data areas holding the PEs */
+	dlocn_xl = pvhdr->disk_areas_xl;
+	while ((offset = xlate64(dlocn_xl->offset))) {
+		if (!lvmcache_add_da(info, offset, xlate64(dlocn_xl->size)))
+			add_errors++;
+		dlocn_xl++;
+	}
+
+	/* Metadata area headers */
+	dlocn_xl++;
+	while ((offset = xlate64(dlocn_xl->offset))) {
+		/* this is just a roundabout call to add_mda() */
+		if (!lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0))
+			add_errors++;
+		dlocn_xl++;
+	}
+
+	/* PV header extension */
+	dlocn_xl++;
 	pvhdr_ext = (struct pv_header_extension *) ((char *) dlocn_xl);
-	if (!(ext_version = xlate32(pvhdr_ext->version)))
-		goto out;
 
-	log_debug_metadata("%s: PV header extension version %" PRIu32 " found",
-			   dev_name(dev), ext_version);
+	/* version 0 doesn't support extension */
+	if (!ext_version)
+		goto mda_read;
 
 	/* Extension version */
-	lvmcache_set_ext_version(info, xlate32(pvhdr_ext->version));
+	lvmcache_set_ext_version(info, ext_version);
 
 	/* Extension flags */
-	lvmcache_set_ext_flags(info, xlate32(pvhdr_ext->flags));
+	lvmcache_set_ext_flags(info, ext_flags);
 
 	/* Bootloader areas */
 	dlocn_xl = pvhdr_ext->bootloader_areas_xl;
 	while ((offset = xlate64(dlocn_xl->offset))) {
-		lvmcache_add_ba(info, offset, xlate64(dlocn_xl->size));
+		if (!lvmcache_add_ba(info, offset, xlate64(dlocn_xl->size)))
+			add_errors++;
 		dlocn_xl++;
 	}
-out:
+
+	if (add_errors) {
+		log_error("PV %s disk area info cannot be saved in cache.", dev_name(dev));
+		*failed_flags |= FAILED_INTERNAL;
+		return 0;
+	}
+
+mda_read:
 	baton.info = info;
 	baton.label = *label;
 	baton.ld = ld;
diff --git a/lib/label/label.c b/lib/label/label.c
index eab028258..18513fd32 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -413,7 +413,8 @@ int label_read(struct device *dev, struct label **labelp, uint64_t scan_sector)
 	 */
 	if (!(l = _find_label_header(dev, scanbuf, label_buf, &sector, scan_sector, &failed_flags))) {
 		if (failed_flags) {
-			log_warn("Found defective device %s: bad LVM label header.", dev_name(dev));
+			log_warn("Found defective device %s: bad %s.",
+				 dev_name(dev), failed_flags_str(failed_flags));
 			lvmcache_add_defective_dev(dev);
 		}
 		goto_out;
@@ -424,11 +425,15 @@ int label_read(struct device *dev, struct label **labelp, uint64_t scan_sector)
 	 * the pv_header, mda locations, mda contents.
 	 * It saves the info it finds into lvmcache info/vginfo structs.
 	 */
-	if ((r = (l->ops->read)(l, dev, label_buf, NULL, labelp)) && *labelp) {
+	if ((r = (l->ops->read)(l, dev, label_buf, NULL, labelp, &failed_flags)) && *labelp) {
 		(*labelp)->dev = dev;
 		(*labelp)->sector = sector;
 	} else {
-		/* FIXME: handle errors */
+		if (failed_flags) {
+			log_warn("Found defective device %s: bad %s.",
+				 dev_name(dev), failed_flags_str(failed_flags));
+			lvmcache_add_defective_dev(dev);
+		}
 	}
  out:
 	if (!dev_close(dev))
@@ -445,6 +450,7 @@ static int _label_read_sync(struct cmd_context *cmd, struct device *dev)
 	struct label *label = NULL;
 	struct labeller *l;
 	uint64_t sector;
+	uint32_t failed_flags;
 	int r = 0;
 
 	memset(scanbuf, 0, sizeof(scanbuf));
@@ -475,11 +481,15 @@ static int _label_read_sync(struct cmd_context *cmd, struct device *dev)
 	 * the pv_header, mda locations, mda contents.
 	 * It saves the info it finds into lvmcache info/vginfo structs.
 	 */
-	if ((r = (l->ops->read)(l, dev, label_buf, NULL, &label)) && label) {
+	if ((r = (l->ops->read)(l, dev, label_buf, NULL, &label, &failed_flags)) && label) {
 		label->dev = dev;
 		label->sector = sector;
 	} else {
-		/* FIXME: handle errors */
+		if (failed_flags) {
+			log_warn("Found defective device %s: bad %s.",
+				 dev_name(dev), failed_flags_str(failed_flags));
+			lvmcache_add_defective_dev(dev);
+		}
 	}
  out:
 	return r;
@@ -560,7 +570,8 @@ int label_verify(struct device *dev)
 
 	if (!(l = _find_label_header(dev, scanbuf, label_buf, &sector, UINT64_C(0), &failed_flags))) {
 		if (failed_flags) {
-			log_warn("Found defective device %s: bad LVM label header.", dev_name(dev));
+			log_warn("Found defective device %s: bad %s.",
+				 dev_name(dev), failed_flags_str(failed_flags));
 			lvmcache_add_defective_dev(dev);
 		}
 		goto out;
@@ -692,7 +703,8 @@ static int _label_read_data_process(struct cmd_context *cmd, struct label_read_d
 		/* Non-PVs exit here when no LVM label is found on the device. */
 
 		if (failed_flags) {
-			log_warn("Found defective device %s: bad LVM label header.", dev_name(ld->dev));
+			log_warn("Found defective device %s: bad %s.",
+				 dev_name(ld->dev), failed_flags_str(failed_flags));
 			lvmcache_add_defective_dev(ld->dev);
 		}
 		goto_out;
@@ -703,11 +715,15 @@ static int _label_read_data_process(struct cmd_context *cmd, struct label_read_d
 	 * the pv_header, mda locations, mda contents.
 	 * It saves the info it finds into lvmcache info/vginfo structs.
 	 */
-	if ((r = (l->ops->read)(l, ld->dev, label_buf, ld, &label)) && label) {
+	if ((r = (l->ops->read)(l, ld->dev, label_buf, ld, &label, &failed_flags)) && label) {
 		label->dev = ld->dev;
 		label->sector = sector;
 	} else {
-		/* FIXME: handle errors */
+		if (failed_flags) {
+			log_warn("Found defective device %s: bad %s.",
+				 dev_name(ld->dev), failed_flags_str(failed_flags));
+			lvmcache_add_defective_dev(ld->dev);
+		}
 	}
  out:
 	return r;
diff --git a/lib/label/label.h b/lib/label/label.h
index 0f88511dd..d3e47b1c3 100644
--- a/lib/label/label.h
+++ b/lib/label/label.h
@@ -76,7 +76,8 @@ struct label_ops {
 	 */
 	int (*read) (struct labeller * l, struct device * dev,
 		     void *label_buf,
-		     struct label_read_data *ld, struct label ** label);
+		     struct label_read_data *ld, struct label ** label,
+		     uint32_t *failed_flags);
 
 	/*
 	 * Additional consistency checks for the paranoid.
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 85e5838f1..81632e359 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -1289,4 +1289,6 @@ int is_system_id_allowed(struct cmd_context *cmd, const char *system_id);
 
 int vg_strip_outdated_historical_lvs(struct volume_group *vg);
 
+const char *failed_flags_str(uint32_t failed_flags);
+
 #endif
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 80d1e3821..c2011f35e 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -40,6 +40,7 @@
 #include <sys/param.h>
 
 static struct physical_volume *_pv_read(struct cmd_context *cmd,
+					struct format_type *fmt,
 					struct dm_pool *pvmem,
 					const char *pv_name,
 					struct format_instance *fid,
@@ -3253,6 +3254,7 @@ static int _check_mda_in_use(struct metadata_area *mda, void *_in_use)
 
 struct _vg_read_orphan_baton {
 	struct cmd_context *cmd;
+	struct format_type *fmt;
 	struct volume_group *vg;
 	uint32_t warn_flags;
 	int consistent;
@@ -3353,7 +3355,7 @@ static int _vg_read_orphan_pv(struct lvmcache_info *info, void *baton)
 	uint32_t ext_version;
 	uint32_t ext_flags;
 
-	if (!(pv = _pv_read(b->vg->cmd, b->vg->vgmem, dev_name(lvmcache_device(info)),
+	if (!(pv = _pv_read(b->vg->cmd, b->fmt, b->vg->vgmem, dev_name(lvmcache_device(info)),
 			    b->vg->fid, b->warn_flags, 0))) {
 		stack;
 		return 1;
@@ -3461,6 +3463,7 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
 	vg->free_count = 0;
 
 	baton.cmd = cmd;
+	baton.fmt = fmt;
 	baton.warn_flags = warn_flags;
 	baton.vg = vg;
 	baton.consistent = 1;
@@ -4643,6 +4646,7 @@ const char *find_vgname_from_pvname(struct cmd_context *cmd,
 
 /* FIXME Use label functions instead of PV functions */
 static struct physical_volume *_pv_read(struct cmd_context *cmd,
+					struct format_type *fmt,
 					struct dm_pool *pvmem,
 					const char *pv_name,
 					struct format_instance *fid,
@@ -4652,7 +4656,6 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
 	struct label *label;
 	struct lvmcache_info *info;
 	struct device *dev;
-	const struct format_type *fmt;
 	int found;
 
 	if (!(dev = dev_cache_get(pv_name, cmd->filter)))
@@ -4687,8 +4690,6 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
 		info = (struct lvmcache_info *) label->info;
 	}
 
-	fmt = lvmcache_fmt(info);
-
 	pv = _alloc_pv(pvmem, dev);
 	if (!pv) {
 		log_error("pv allocation for '%s' failed", pv_name);
@@ -4698,9 +4699,8 @@ static struct physical_volume *_pv_read(struct cmd_context *cmd,
 	pv->label_sector = label->sector;
 
 	/* FIXME Move more common code up here */
-	if (!(lvmcache_fmt(info)->ops->pv_read(lvmcache_fmt(info), pv_name, pv, scan_label_only))) {
-		log_error("Failed to read existing physical volume '%s'",
-			  pv_name);
+	if (!(fmt->ops->pv_read(fmt, pv_name, pv, scan_label_only))) {
+		log_error("Failed to read existing physical volume '%s'", pv_name);
 		goto bad;
 	}
 
@@ -6055,3 +6055,17 @@ int vg_strip_outdated_historical_lvs(struct volume_group *vg) {
 
 	return 1;
 }
+
+const char *failed_flags_str(uint32_t failed_flags)
+{
+	if (failed_flags & FAILED_LABEL_CHECKSUM)
+		return "LVM label header checksum";
+	if (failed_flags & FAILED_LABEL_SECTOR_NUMBER)
+		return "LVM label header number";
+	if (failed_flags & FAILED_INTERNAL)
+		return "LVM internal error";
+	if (failed_flags & FAILED_PV_HEADER)
+		return "PV header";
+	return "unknown";
+}
+