labels: move the label scan at the start of each vg_read

This moves a low level label scan to the start of vg_read.

4 files changed, 61 insertions, 28 deletions

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index e23b8f0da..0602d8e8f 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -1136,6 +1136,45 @@ next:
 	goto next;
 }
 
+int lvmcache_label_refresh_for_vg(struct cmd_context *cmd, const char *vgname, const char *vgid)
+{
+	struct dm_list devs;
+	struct device_list *devl;
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+
+	if (lvmetad_used())
+		return 1;
+
+	dm_list_init(&devs);
+
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid)))
+		return_0;
+
+	dm_list_iterate_items(info, &vginfo->infos) {
+		if (!(devl = dm_malloc(sizeof(*devl)))) {
+			log_error("device_list element allocation failed");
+			return 0;
+		}
+		devl->dev = info->dev;
+		dm_list_add(&devs, &devl->list);
+	}
+
+	dm_list_iterate_items(devl, &devs) {
+		log_debug_cache("Reading label on %s for VG %s", dev_name(devl->dev), vgname);
+		label_read(devl->dev, NULL, 0);
+	}
+
+	/*
+	 * TODO: grab vginfo again, and compare vginfo->infos
+	 * to what was found above before rereading labels.
+	 * If there are any info->devs now that were not in the
+	 * first devs list, then do label_read on those also.
+	 */
+
+	return 1;
+}
+
 int lvmcache_label_scan(struct cmd_context *cmd)
 {
 	struct dm_list del_cache_devs;
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 8ed182462..f46fdfd12 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -74,6 +74,7 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset);
  */
 void lvmcache_force_next_label_scan(void);
 int lvmcache_label_scan(struct cmd_context *cmd);
+int lvmcache_label_refresh_for_vg(struct cmd_context *cmd, const char *vgname, const char *vgid);
 
 /* Add/delete a device */
 struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid,
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index b05fca934..1f95bdde0 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -2011,22 +2011,6 @@ static int _create_vg_text_instance(struct format_instance *fid,
 		}
 
 		if (type & FMT_INSTANCE_MDAS) {
-			/*
-			 * TODO in theory, this function should be never reached
-			 * while in critical_section(), because lvmcache's
-			 * cached_vg should be valid. However, this assumption
-			 * sometimes fails (possibly due to inconsistent
-			 * (precommit) metadata and/or missing devices), and
-			 * calling lvmcache_label_scan inside the critical
-			 * section may be fatal (i.e. deadlock).
-			 */
-			if (!critical_section())
-				/* Scan PVs in VG for any further MDAs */
-				/*
-				 * FIXME Only scan PVs believed to be in the VG.
- 				 */
-				lvmcache_label_scan(fid->fmt->cmd);
-
 			if (!(vginfo = lvmcache_vginfo_from_vgname(vg_name, vg_id)))
 				goto_out;
 			if (!lvmcache_fid_add_mdas_vg(vginfo, fid))
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 779fb8cb8..95593a274 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -4301,19 +4301,28 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 		return correct_vg;
 	}
 
-	/* Find the vgname in the cache */
-	/* If it's not there we must do full scan to be completely sure */
-	if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0))) {
+	/*
+	 * Rescan the devices that are associated with this vg in lvmcache.
+	 * This repeats what was done by the command's initial label scan,
+	 * but only the devices associated with this VG.
+	 *
+	 * The lvmcache info about these devs is from the initial label scan
+	 * performed by the command before the vg lock was held.  Now the VG
+	 * lock is held, so we rescan all the info from the devs in case
+	 * something changed between the initial scan and now that the lock
+	 * is held.
+	 */
+	log_debug_metadata("Reading VG rereading labels for %s", vgname);
+
+	if (!lvmcache_label_refresh_for_vg(cmd, vgname, vgid)) {
+		/* The VG wasn't found, so force a full label scan. */
+		lvmcache_force_next_label_scan();
 		lvmcache_label_scan(cmd);
-		if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 1))) {
-			/* Independent MDAs aren't supported under low memory */
-			if (!cmd->independent_metadata_areas && critical_section())
-				return_NULL;
-			lvmcache_force_next_label_scan();
-			lvmcache_label_scan(cmd);
-			if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0)))
-				return_NULL;
-		}
+	}
+
+	if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0))) {
+		log_debug_metadata("Cache did not find fmt for vgname %s", vgname);
+		return_NULL;
 	}
 
 	/* Now determine the correct vgname if none was supplied */