From ea1f434f26c362b07b8d8dcad1f9acff021313ff Mon Sep 17 00:00:00 2001
From: David Teigland <teigland@redhat.com>
Date: Wed, 10 Apr 2019 16:02:02 -0500
Subject: improve some warning messages

---
 lib/metadata/metadata.c | 16 +++++++++++++++-
 lib/metadata/read.c     | 14 +++++++++-----
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 88fa9b481..c55995ce6 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3440,6 +3440,15 @@ bad:
  * FIXME: we only want to print the warnings when this is called from
  * vg_read, not from import_vg_from_metadata, so do the warnings elsewhere
  * or avoid calling this from import_vg_from.
+ *
+ * FIXME: the handling of the MISSING_PV flag should be improved because
+ * it's not clear when MISSING_PV is set because it is set in the ondisk
+ * metadata, or if MISSING_PV set because there's no dev found on the
+ * system while it's not written on disk, or if the dev is missing and
+ * the flag is written on disk.  In this function those cases are
+ * distinguishable, but not outside.  This could easily lead to bugs
+ * in cases where the code should be distinguishing between these
+ * variations.
  */
 static void _set_pv_device(struct format_instance *fid,
 			   struct volume_group *vg,
@@ -3452,8 +3461,13 @@ static void _set_pv_device(struct format_instance *fid,
 		if (!id_write_format(&pv->id, buffer, sizeof(buffer)))
 			buffer[0] = '\0';
 
+		/*
+		 * FIXME: include the device name hint in the VG metadata
+		 * in the message so the user has a better idea which
+		 * device is missing.
+		 */
 		if (fid->fmt->cmd && !fid->fmt->cmd->pvscan_cache_single)
-			log_error_once("Couldn't find device with uuid %s.", buffer);
+			log_warn("WARNING: Couldn't find device with uuid %s.", buffer);
 		else
 			log_debug_metadata("Couldn't find device with uuid %s.", buffer);
 	}
diff --git a/lib/metadata/read.c b/lib/metadata/read.c
index e0c882ca8..c08f66fb6 100644
--- a/lib/metadata/read.c
+++ b/lib/metadata/read.c
@@ -609,7 +609,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 		}
 
 		if (vg->seqno > vg_ret->seqno) {
-			log_warn("WARNING: ignoring old metadata seqno %u on %s vs new metadata seqno %u on %s for VG %s.",
+			log_warn("WARNING: ignoring metadata seqno %u on %s for seqno %u on %s for VG %s.",
 				 vg_ret->seqno, dev_name(dev_ret),
 				 vg->seqno, dev_name(mda_dev), vg->name);
 			found_old_metadata = 1;
@@ -621,7 +621,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 		}
 
 		if (vg_ret->seqno > vg->seqno) {
-			log_warn("WARNING: ignoring old metadata seqno %u on %s vs new metadata seqno %u on %s for VG %s.",
+			log_warn("WARNING: ignoring metadata seqno %u on %s for seqno %u on %s for VG %s.",
 				 vg->seqno, dev_name(mda_dev),
 				 vg_ret->seqno, dev_name(dev_ret), vg->name);
 			found_old_metadata = 1;
@@ -685,6 +685,7 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const
 			     uint32_t read_flags, uint32_t lockd_state,
 			     uint32_t *error_flags, struct volume_group **error_vg)
 {
+	char uuidstr[64] __attribute__((aligned(8)));
 	struct volume_group *vg = NULL;
 	struct lv_list *lvl;
 	struct pv_list *pvl;
@@ -766,22 +767,25 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const
 	 * --removemissing.
 	 */
 	dm_list_iterate_items(pvl, &vg->pvs) {
+		if (!id_write_format(&pvl->pv->id, uuidstr, sizeof(uuidstr)))
+			uuidstr[0] = '\0';
+
 		if (!pvl->pv->dev) {
 			/* The obvious and common case of a missing device. */
 
-			log_warn("WARNING: VG %s is missing PVID %s.", vg_name, (const char *)&pvl->pv->id);
+			log_warn("WARNING: VG %s is missing PV %s.", vg_name, uuidstr);
 			missing_pv_dev++;
 
 		} else if (pvl->pv->status & MISSING_PV) {
 			/* A device that was missing but has reappeared. */
 
 			if (pvl->pv->pe_alloc_count == 0) {
-				log_warn("WARNING: VG %s has unused reappeared PV %s.", vg_name, dev_name(pvl->pv->dev));
+				log_warn("WARNING: VG %s has unused reappeared PV %s %s.", vg_name, dev_name(pvl->pv->dev), uuidstr);
 				pvl->pv->status &= ~MISSING_PV;
 				/* tell vgextend restoremissing that MISSING flag was cleared here */
 				pvl->pv->unused_missing_cleared = 1;
 			} else {
-				log_warn("WARNING: VG %s was missing PV %s.", vg_name, dev_name(pvl->pv->dev));
+				log_warn("WARNING: VG %s was missing PV %s %s.", vg_name, dev_name(pvl->pv->dev), uuidstr);
 				missing_pv_flag++;
 			}
 		}
-- 
cgit v1.2.1