pvscan: --cache should scan all devices

When running pvscan --cache, the intention is to scan
all devices, i.e. the equivalent of running
'pvscan --cache dev' for each device.  But, some devices
holding duplicate PVs were being skipped, and lvmetad
was not being told about them.

This was a side effect of the fact that the lvmcache
layer is used internally during scanning (it's used
to transfer info between the different stages of the
command: label reading, vg reading, sending the vg.)

An unwanted side effect of using lvmcache is that it
eliminates duplicate PVs if it sees them.  This
prevented the command from sending some duplicate PVs
to lvmetad (when the first duplicate was preferred
over the second.)

The fix is to drop the lvmcache state between scanning
each PV.  There is no need to keep any cache state between
each PV being scanned (there's nothing being reused), and
the unneeded state is causing lvmcache to eliminate
duplicates which is harmful for pvscan --cache.

Also, remove clvm locking from pvscan_lvmetad.  This is
not needed since lvmetad cannot be used with clvm.  It
was interfering with clearing the cache.

4 files changed, 46 insertions, 8 deletions

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index c495fa744..f8b44b08b 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -2003,6 +2003,36 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset)
 			stack;
 }
 
+void lvmcache_pvscan_clear(struct cmd_context *cmd)
+{
+	log_debug_cache("clear lvmcache for pvscan.");
+
+	_has_scanned = 0;
+
+	if (_vgid_hash) {
+		dm_hash_destroy(_vgid_hash);
+		_vgid_hash = NULL;
+	}
+
+	if (_pvid_hash) {
+		dm_hash_iter(_pvid_hash, (dm_hash_iterate_fn) _lvmcache_destroy_entry);
+		dm_hash_destroy(_pvid_hash);
+		_pvid_hash = NULL;
+	}
+
+	if (_vgname_hash) {
+		dm_hash_iter(_vgname_hash,
+			  (dm_hash_iterate_fn) _lvmcache_destroy_vgnamelist);
+		dm_hash_destroy(_vgname_hash);
+		_vgname_hash = NULL;
+	}
+
+	if (!dm_list_empty(&_vginfos))
+		log_error(INTERNAL_ERROR "_vginfos list should be empty");
+
+	dm_list_init(&_vginfos);
+}
+
 int lvmcache_pvid_is_locked(const char *pvid) {
 	struct lvmcache_info *info;
 	info = lvmcache_info_from_pvid(pvid, 0);
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 76b9b10ae..7206434cd 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -190,4 +190,6 @@ void lvmcache_set_preferred_duplicates(const char *vgid);
 
 int lvmcache_contains_lock_type_sanlock(struct cmd_context *cmd);
 
+void lvmcache_pvscan_clear(struct cmd_context *cmd);
+
 #endif
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index b24e3f59d..55bf0eecd 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -1434,6 +1434,20 @@ static int _lvmetad_pvscan_all_devs(struct cmd_context *cmd, activation_handler
 		}
 		if (!lvmetad_pvscan_single(cmd, dev, handler, ignore_obsolete))
 			r = 0;
+
+		/*
+		 * The fact that lvmcache is used to transport data between
+		 * steps within pvscan_single is an implementation detail;
+		 * we don't really need or want any caching since we're just
+		 * scanning each device once to populate lvmetad.  We need
+		 * to clear lvmcache after scanning each PV to avoid a side
+		 * effect of lvmcache, which is that it eliminates duplicate
+		 * PVs and prevents duplicate PVs from being sent to lvmetad.
+		 * lvmetad wants to know about duplicate PVs.  So, we just
+		 * drop the lvmcache data after scanning each PV so that
+		 * state from scanning one PV will not affect the next one.
+		 */
+		lvmcache_pvscan_clear(cmd);
 	}
 
 	init_silent(was_silent);
diff --git a/tools/pvscan.c b/tools/pvscan.c
index 3adb91c63..150ad72ed 100644
--- a/tools/pvscan.c
+++ b/tools/pvscan.c
@@ -221,11 +221,6 @@ static int _pvscan_lvmetad(struct cmd_context *cmd, int argc, char **argv)
 		return EINVALID_CMD_LINE;
 	}
 	
-	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_READ, NULL)) {
-		log_error("Unable to obtain global lock.");
-		return ECMD_FAILED;
-	}
-
 	/* Scan everything? */
 	if (!argc && !devno_args) {
 		if (!lvmetad_pvscan_all_devs(cmd, handler))
@@ -319,9 +314,6 @@ static int _pvscan_lvmetad(struct cmd_context *cmd, int argc, char **argv)
 	}
 
 out:
-	if (!sync_local_dev_names(cmd))
-		stack;
-	unlock_vg(cmd, VG_GLOBAL);
 	return ret;
 }