vg_read: improve messages and add comments

comments added where future error path handling should go

2 files changed, 42 insertions, 33 deletions

diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 797ac37b3..86ee888ac 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -543,28 +543,25 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
 		goto out;
 	}
 
-	/* FIXME 64-bit */
-	if (!(vg = text_read_metadata(fid, NULL, vg_fmtdata, use_previous_vg, area->dev,
-				     (off_t) (area->start + rlocn->offset),
-				     (uint32_t) (rlocn->size - wrap),
-				     (off_t) (area->start + MDA_HEADER_SIZE),
-				     wrap,
-				     calc_crc,
-				     rlocn->checksum,
-				     &when, &desc))
-			&& (!use_previous_vg || !*use_previous_vg))
-		goto_out;
+	vg = text_read_metadata(fid, NULL, vg_fmtdata, use_previous_vg, area->dev,
+				(off_t) (area->start + rlocn->offset),
+				(uint32_t) (rlocn->size - wrap),
+				(off_t) (area->start + MDA_HEADER_SIZE),
+				wrap,
+				calc_crc,
+				rlocn->checksum,
+				&when, &desc);
 
-	if (vg)
-		log_debug_metadata("Read %s %smetadata (%u) from %s at %" PRIu64 " size %"
-				   PRIu64, vg->name, precommitted ? "pre-commit " : "",
-				   vg->seqno, dev_name(area->dev),
-				   area->start + rlocn->offset, rlocn->size);
-	else
-		log_debug_metadata("Skipped reading %smetadata from %s at %" PRIu64 " size %"
-				   PRIu64 " with matching checksum.", precommitted ? "pre-commit " : "",
-				   dev_name(area->dev),
-				   area->start + rlocn->offset, rlocn->size);
+	if (!vg) {
+		/* FIXME: detect and handle errors, and distinguish from the optimization
+		   that skips parsing the metadata which also returns NULL. */
+	}
+
+	log_debug_metadata("Read metadata from %s at %"PRIu64" size %"PRIu64" for VG %s",
+			   dev_name(area->dev),
+			   area->start + rlocn->offset,
+			   rlocn->size,
+			   vgname);
 
 	if (vg && precommitted)
 		vg->status |= PRECOMMITTED;
@@ -1104,6 +1101,8 @@ static int _vg_remove_file(struct format_instance *fid __attribute__((unused)),
 	return 1;
 }
 
+/* used for independent_metadata_areas */
+
 static int _scan_file(const struct format_type *fmt, const char *vgname)
 {
 	struct dirent *dirent;
@@ -1176,7 +1175,6 @@ int read_metadata_location(const struct format_type *fmt,
 	unsigned int len = 0;
 	char buf[NAME_LEN + 1] __attribute__((aligned(8)));
 	uint64_t buffer_size, current_usage;
-	unsigned used_cached_metadata = 0;
 
 	if (mda_free_sectors)
 		*mda_free_sectors = ((dev_area->size - MDA_HEADER_SIZE) / 2) >> SECTOR_SHIFT;
@@ -1230,30 +1228,25 @@ int read_metadata_location(const struct format_type *fmt,
 	vgsummary->mda_checksum = rlocn->checksum;
 	vgsummary->mda_size = rlocn->size;
 
-	if (lvmcache_lookup_mda(vgsummary))
-		used_cached_metadata = 1;
-
-	/* FIXME 64-bit */
 	if (!text_read_metadata_summary(fmt, dev_area->dev, ld,
 				(off_t) (dev_area->start + rlocn->offset),
 				(uint32_t) (rlocn->size - wrap),
 				(off_t) (dev_area->start + MDA_HEADER_SIZE),
 				wrap, calc_crc, vgsummary->vgname ? 1 : 0,
-				vgsummary))
+				vgsummary)) {
+		/* FIXME: detect and handle errors */
 		return_0;
+	}
 
 	/* Ignore this entry if the characters aren't permissible */
 	if (!validate_name(vgsummary->vgname))
 		return_0;
 
-	log_debug_metadata("%s: %s metadata at %" PRIu64 " size %" PRIu64
-			   " (in area at %" PRIu64 " size %" PRIu64
-			   ") for %s (" FMTVGID ")",
+	log_debug_metadata("Read metadata summary from %s at %"PRIu64" size %"PRIu64" for VG %s",
 			   dev_name(dev_area->dev),
-			   used_cached_metadata ? "Using cached" : "Found",
 			   dev_area->start + rlocn->offset,
-			   rlocn->size, dev_area->start, dev_area->size, vgsummary->vgname,
-			   (char *)&vgsummary->vgid);
+			   rlocn->size,
+			   vgsummary->vgname);
 
 	if (mda_free_sectors) {
 		current_usage = (rlocn->size + SECTOR_SIZE - UINT64_C(1)) -
@@ -1269,6 +1262,8 @@ int read_metadata_location(const struct format_type *fmt,
 	return 1;
 }
 
+/* used for independent_metadata_areas */
+
 static int _scan_raw(const struct format_type *fmt, const char *vgname __attribute__((unused)))
 {
 	struct raw_list *rl;
@@ -1310,6 +1305,8 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu
 	return 1;
 }
 
+/* used for independent_metadata_areas */
+
 static int _text_scan(const struct format_type *fmt, const char *vgname)
 {
 	return (_scan_file(fmt, vgname) & _scan_raw(fmt, vgname));
diff --git a/lib/format_text/import.c b/lib/format_text/import.c
index bfad9e0c4..d7d707aa1 100644
--- a/lib/format_text/import.c
+++ b/lib/format_text/import.c
@@ -83,6 +83,7 @@ int text_read_metadata_summary(const struct format_type *fmt,
 					 offset2, size2, checksum_fn,
 					 vgsummary->mda_checksum,
 					 checksum_only, 1)) {
+			/* FIXME: handle errors */
 			log_error("Couldn't read volume group metadata from %s.", dev_name(dev));
 			goto out;
 		}
@@ -139,6 +140,15 @@ struct volume_group *text_read_metadata(struct format_instance *fid,
 	struct text_vg_version_ops **vsn;
 	int skip_parse;
 
+	/*
+	 * This struct holds the checksum and size of the VG metadata
+	 * that was read from a previous device.  When we read the VG
+	 * metadata from this device, we can skip parsing it into a
+	 * cft (saving time) if the checksum of the metadata buffer
+	 * we read from this device matches the size/checksum saved in
+	 * the mda_header/rlocn struct on this device, and matches the
+	 * size/checksum from the previous device.
+	 */
 	if (vg_fmtdata && !*vg_fmtdata &&
 	    !(*vg_fmtdata = dm_pool_zalloc(fid->mem, sizeof(**vg_fmtdata)))) {
 		log_error("Failed to allocate VG fmtdata for text format.");
@@ -166,6 +176,7 @@ struct volume_group *text_read_metadata(struct format_instance *fid,
 		if (!config_file_read_fd(cft, dev, NULL, offset, size,
 					 offset2, size2, checksum_fn, checksum,
 					 skip_parse, 1)) {
+			/* FIXME: handle errors */
 			log_error("Couldn't read volume group metadata from %s.", dev_name(dev));
 			goto out;
 		}
@@ -179,6 +190,7 @@ struct volume_group *text_read_metadata(struct format_instance *fid,
 	if (skip_parse) {
 		if (use_previous_vg)
 			*use_previous_vg = 1;
+		log_debug_metadata("Skipped parsing metadata on %s", dev_name(dev));
 		goto out;
 	}