Improve handling of duplicate VG namesdev-dct-dupvgnames-A

If a VG name is specified on the command line, and
multiple VGs with that name exist on the system,
then lvm will refuse to process the ambiguous
name.  However, if all but one of the matching
VGs is foreign, then the one accessible VG is assumed.

For commands that use process_each_vg/lv, duplicate
VG name checking and resolution is done from process_each.
This is mostly common code for both lvmetad and non-lvmetad:

. _resolve_duplicate_vgnames uses lvmcache_vg_is_foreign
. lvmcache_vg_is_foreign uses lvmetad_vg_is_foreign when
  lvmetad is used, or vginfo structs in lvmcache for
  the non-lvmetad case.

For commands that do not use process_each_vg/lv,
duplicate VG name checking and resolution is done
from _vg_read().  Different code is needed for
lvmetad and non-lvmetad:

non-lvmetad:

. initial scan populates lvmcache vginfo for all VGs
. _vg_read -> lvmcache_vgname_to_vgid
. lvmcache_vgname_to_vgid sees multiple vginfo structs
  for the VG name
. if all but one of the vginfo's is foreign, then the
  one remaining VG is assumed and used
. if multiple vginfos match, then lvmcache_vgname_to_vgid
  prints an error and fails, causing _vg_read to fail.

lvmetad:

. there is no initial scan to populate lvmcache,
  so vginfos cannot be used like the non-lvmetad case
. _vg_read -> lvmcache_get_vg -> lvmetad_vg_lookup
. lvmetad returns an error from vg_lookup because it
  sees multiple VGs for the VG name
. lvmetad_vg_lookup looks up each of the VGs to check
  which are foreign
. if all but one of the VGs is foreign, then the one
  remaining VG is assumed and used
. if multiple VGs match, then lvmetad_vg_lookup prints
  an error and fails, causing _vg_read to fail.

9 files changed, 461 insertions, 117 deletions

diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 7af9bde1c..cab67448b 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -757,11 +757,11 @@ static response vg_lookup(lvmetad_state *s, request r)
 		/* FIXME: comment out these sanity checks when not testing */
 
 		if (!name_lookup || !uuid_lookup) {
-			ERROR(s, "vg_lookup vgid %s name %s found incomplete mapping uuid %s name %s",
+			ERROR(s, "vg_lookup vgid %s name %s found incomplete mapping uuid %.32s name %s",
 			      uuid, name, uuid_lookup ?: "none", name_lookup ?: "none");
 			return reply_unknown("VG mapping incomplete");
 		} else if (strcmp(name_lookup, name) || strcmp(uuid_lookup, uuid)) {
-			ERROR(s, "vg_lookup vgid %s name %s found inconsistent mapping uuid %s name %s",
+			ERROR(s, "vg_lookup vgid %s name %s found inconsistent mapping uuid %.32s name %s",
 			      uuid, name, uuid_lookup, name_lookup);
 			return reply_unknown("VG mapping inconsistent");
 		}
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 8ab1a3191..b772261de 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -56,6 +56,7 @@ struct lvmcache_vginfo {
 	char _padding[7];
 	struct lvmcache_vginfo *next; /* Another VG with same name? */
 	char *creation_host;
+	char *system_id;
 	char *lock_type;
 	uint32_t mda_checksum;
 	size_t mda_size;
@@ -613,6 +614,49 @@ const char *lvmcache_vgname_from_vgid(struct dm_pool *mem, const char *vgid)
 	return vgname;
 }
 
+const char *lvmcache_vgid_from_vgname(struct cmd_context *cmd, const char *vgname)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_vginfo *vginfo2;
+	struct lvmcache_vginfo *vginfo_use = NULL;
+
+	if (!(vginfo = dm_hash_lookup(_vgname_hash, vgname)))
+		return_NULL;
+
+	if (!vginfo->next)
+		return dm_pool_strdup(cmd->mem, vginfo->vgid);
+
+	/*
+	 * There are multiple VGs with this name to choose from.
+	 * If one VG is local and the rest are foreign, then use
+	 * the local VG.  Otherwise return an error because we
+	 * don't know which VG is intended.
+	 */
+
+	vginfo2 = vginfo;
+
+	while (vginfo2) {
+		if (!system_id_allowed(cmd, vginfo2->system_id))
+			goto next;
+
+		if (vginfo_use)
+			goto fail;
+		vginfo_use = vginfo2;
+next:
+		vginfo2 = vginfo2->next;
+	}
+
+	if (!vginfo_use)
+		vginfo_use = vginfo;
+
+	return dm_pool_strdup(cmd->mem, vginfo_use->vgid);
+
+ fail:
+	log_error("Multiple VGs found with the same name: %s.", vgname);
+	log_error("Use the --select option with VG UUID (vg_uuid).");
+	return NULL;
+}
+
 static int _info_is_valid(struct lvmcache_info *info)
 {
 	if (info->status & CACHE_INVALID)
@@ -741,6 +785,8 @@ int lvmcache_label_scan(struct cmd_context *cmd, int full_scan)
 	if (_scanning_in_progress)
 		return 0;
 
+	log_verbose("Scanning devices for internal cache");
+
 	_scanning_in_progress = 1;
 
 	if (!_vgname_hash && !lvmcache_init()) {
@@ -1241,6 +1287,15 @@ static int _insert_vginfo(struct lvmcache_vginfo *new_vginfo, const char *vgid,
 			return_0;
 
 		/*
+		 * vginfo is kept for each VG with the same name.
+		 * They are saved with the vginfo->next list.
+		 * These checks just decide the ordering of
+		 * that list.
+		 *
+		 * FIXME: it should no longer matter what order
+		 * the vginfo's are kept in, so we can probably
+		 * remove these comparisons and reordering entirely.
+		 *
 		 * If   Primary not exported, new exported => keep
 		 * Else Primary exported, new not exported => change
 		 * Else Primary has hostname for this machine => keep
@@ -1250,38 +1305,42 @@ static int _insert_vginfo(struct lvmcache_vginfo *new_vginfo, const char *vgid,
 		 */
 		if (!(primary_vginfo->status & EXPORTED_VG) &&
 		    (vgstatus & EXPORTED_VG))
-			log_warn("WARNING: Duplicate VG name %s: "
-				 "Existing %s takes precedence over "
-				 "exported %s", new_vginfo->vgname,
-				 uuid_primary, uuid_new);
+			log_verbose("Duplicate VG name %s: "
+				    "Existing %s takes precedence over "
+				    "exported %s", new_vginfo->vgname,
+				    uuid_primary, uuid_new);
 		else if ((primary_vginfo->status & EXPORTED_VG) &&
 			   !(vgstatus & EXPORTED_VG)) {
-			log_warn("WARNING: Duplicate VG name %s: "
-				 "%s takes precedence over exported %s",
-				 new_vginfo->vgname, uuid_new,
-				 uuid_primary);
+			log_verbose("Duplicate VG name %s: "
+				    "%s takes precedence over exported %s",
+				    new_vginfo->vgname, uuid_new,
+				    uuid_primary);
 			use_new = 1;
 		} else if (primary_vginfo->creation_host &&
 			   !strcmp(primary_vginfo->creation_host,
 				   primary_vginfo->fmt->cmd->hostname))
-			log_warn("WARNING: Duplicate VG name %s: "
-				 "Existing %s (created here) takes precedence "
-				 "over %s", new_vginfo->vgname, uuid_primary,
-				 uuid_new);
+			log_verbose("Duplicate VG name %s: "
+				    "Existing %s (created here) takes precedence "
+				    "over %s", new_vginfo->vgname, uuid_primary,
+				    uuid_new);
 		else if (!primary_vginfo->creation_host && creation_host) {
-			log_warn("WARNING: Duplicate VG name %s: "
-				 "%s (with creation_host) takes precedence over %s",
-				 new_vginfo->vgname, uuid_new,
-				 uuid_primary);
+			log_verbose("Duplicate VG name %s: "
+				    "%s (with creation_host) takes precedence over %s",
+				    new_vginfo->vgname, uuid_new,
+				    uuid_primary);
 			use_new = 1;
 		} else if (creation_host &&
 			   !strcmp(creation_host,
 				   primary_vginfo->fmt->cmd->hostname)) {
-			log_warn("WARNING: Duplicate VG name %s: "
-				 "%s (created here) takes precedence over %s",
-				 new_vginfo->vgname, uuid_new,
-				 uuid_primary);
+			log_verbose("Duplicate VG name %s: "
+				    "%s (created here) takes precedence over %s",
+				    new_vginfo->vgname, uuid_new,
+				    uuid_primary);
 			use_new = 1;
+		} else {
+			log_verbose("Duplicate VG name %s: "
+				    "Prefer existing %s vs new %s",
+				    new_vginfo->vgname, uuid_primary, uuid_new);
 		}
 
 		if (!use_new) {
@@ -1448,7 +1507,7 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
 }
 
 static int _lvmcache_update_vgstatus(struct lvmcache_info *info, uint32_t vgstatus,
-				     const char *creation_host, const char *lock_type)
+				     const char *creation_host, const char *lock_type, const char *system_id)
 {
 	if (!info || !info->vginfo)
 		return 1;
@@ -1482,10 +1541,10 @@ static int _lvmcache_update_vgstatus(struct lvmcache_info *info, uint32_t vgstat
 set_lock_type:
 
 	if (!lock_type)
-		goto out;
+		goto set_system_id;
 
 	if (info->vginfo->lock_type && !strcmp(lock_type, info->vginfo->lock_type))
-		goto out;
+		goto set_system_id;
 
 	if (info->vginfo->lock_type)
 		dm_free(info->vginfo->lock_type);
@@ -1496,6 +1555,28 @@ set_lock_type:
 		return 0;
 	}
 
+	log_debug_cache("lvmcache: %s: VG %s: Set lock_type to %s.",
+			dev_name(info->dev), info->vginfo->vgname, lock_type);
+
+set_system_id:
+
+	if (!system_id)
+		goto out;
+
+	if (info->vginfo->system_id && !strcmp(system_id, info->vginfo->system_id))
+		goto out;
+
+	if (info->vginfo->system_id)
+		dm_free(info->vginfo->system_id);
+
+	if (!(info->vginfo->system_id = dm_strdup(system_id))) {
+		log_error("cache creation host alloc failed for %s",
+			  system_id);
+		return 0;
+	}
+
+	log_debug_cache("lvmcache: %s: VG %s: Set system_id to %s.",
+			dev_name(info->dev), info->vginfo->vgname, system_id);
 out:
 	return 1;
 }
@@ -1561,7 +1642,7 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus,
 				     vgsummary->creation_host, info->fmt) ||
 	    !_lvmcache_update_vgid(info, info->vginfo, vgid) ||
-	    !_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host, vgsummary->lock_type) ||
+	    !_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host, vgsummary->lock_type, vgsummary->system_id) ||
 	    !_lvmcache_update_vg_mda_info(info, vgsummary->mda_checksum, vgsummary->mda_size))
 		return_0;
 
@@ -1577,6 +1658,7 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
 		.vgname = vg->name,
 		.vgstatus = vg->status,
 		.vgid = vg->id,
+		.system_id = vg->system_id,
 		.lock_type = vg->lock_type
 	};
 
@@ -2378,3 +2460,17 @@ void lvmcache_get_max_name_lengths(struct cmd_context *cmd,
 	}
 }
 
+int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const char *vgid)
+{
+	struct lvmcache_vginfo *vginfo;
+	int ret = 0;
+
+	if (lvmetad_active())
+		return lvmetad_vg_is_foreign(cmd, vgname, vgid);
+
+	if ((vginfo = lvmcache_vginfo_from_vgid(vgid)))
+		ret = !system_id_allowed(cmd, vginfo->system_id);
+
+	return ret;
+}
+
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index ccf3eb498..af01ca89d 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -55,6 +55,7 @@ struct lvmcache_vgsummary {
 	struct id vgid;
 	uint64_t vgstatus;
 	char *creation_host;
+	const char *system_id;
 	const char *lock_type;
 	uint32_t mda_checksum;
 	size_t mda_size;
@@ -193,5 +194,10 @@ int lvmcache_contains_lock_type_sanlock(struct cmd_context *cmd);
 void lvmcache_get_max_name_lengths(struct cmd_context *cmd,
 			unsigned *pv_max_name_len, unsigned *vg_max_name_len);
 
+int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname,
+			const char *vgid);
+
+const char *lvmcache_vgid_from_vgname(struct cmd_context *cmd,
+			const char *vgname);
 
 #endif
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index 78844c4c6..e180ef41b 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -547,9 +547,17 @@ struct volume_group *lvmetad_vg_lookup(struct cmd_context *cmd, const char *vgna
 
 	if (_lvmetad_handle_reply(reply, "lookup VG", diag_name, &found) && found) {
 
+		/*
+		 * FIXME: if only one VG is not foreign, then use it.
+		 * . lvmetad_get_vgnameids()
+		 * . for each list entry with a matching vgname,
+		 *   lvmetad_vg_is_foreign(vgname, vgid),
+		 * . if all but one are foreign, then assume that
+		 *   one was intended and use it.
+		 */
 		if ((found == 2) && vgname) {
 			log_error("Multiple VGs found with the same name: %s.", vgname);
-			log_error("See the --select option with VG UUID (vg_uuid).");
+			log_error("Use the --select option with VG UUID (vg_uuid).");
 			goto out;
 		}
 
@@ -1808,3 +1816,29 @@ void lvmetad_validate_global_cache(struct cmd_context *cmd, int force)
 		_update_changed_pvs_in_udev(cmd, &pvc_before, &pvc_after);
 	}
 }
+
+int lvmetad_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const char *vgid)
+{
+	daemon_reply reply;
+	struct dm_config_node *top;
+	const char *system_id = NULL;
+	char uuid[64];
+	int ret;
+
+	if (!id_write_format((const struct id*)vgid, uuid, sizeof(uuid)))
+		return_0;
+
+	reply = _lvmetad_send("vg_lookup",
+			      "uuid = %s", uuid,
+			      "name = %s", vgname,
+			       NULL);
+
+	if ((top = dm_config_find_node(reply.cft->root, "metadata")))
+		system_id = dm_config_find_str(top, "metadata/system_id", NULL);
+
+	ret = !system_id_allowed(cmd, system_id);
+
+	daemon_reply_destroy(reply);
+	return ret;
+}
+
diff --git a/lib/cache/lvmetad.h b/lib/cache/lvmetad.h
index af0d562fe..2c0b13464 100644
--- a/lib/cache/lvmetad.h
+++ b/lib/cache/lvmetad.h
@@ -169,6 +169,8 @@ int lvmetad_pvscan_foreign_vgs(struct cmd_context *cmd, activation_handler handl
 int lvmetad_vg_clear_outdated_pvs(struct volume_group *vg);
 void lvmetad_validate_global_cache(struct cmd_context *cmd, int force);
 
+int lvmetad_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const char *vgid);
+
 #  else		/* LVMETAD_SUPPORT */
 
 #    define lvmetad_init(cmd)	do { } while (0)
@@ -197,6 +199,7 @@ void lvmetad_validate_global_cache(struct cmd_context *cmd, int force);
 #    define lvmetad_pvscan_foreign_vgs(cmd, handler)	(0)
 #    define lvmetad_vg_clear_outdated_pvs(vg)           (1)
 #    define lvmetad_validate_global_cache(cmd, force)	do { } while (0)
+#    define lvmetad_vg_is_foreign(cmd, vgname, vgid) (0)
 
 #  endif	/* LVMETAD_SUPPORT */
 
diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index 879aae71b..2951349a4 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -1042,11 +1042,11 @@ static int _read_vgname(const struct format_type *fmt, const struct dm_config_tr
 {
 	const struct dm_config_node *vgn;
 	struct dm_pool *mem = fmt->cmd->mem;
+	const char *str;
 	int old_suppress;
 
 	old_suppress = log_suppress(2);
-	vgsummary->creation_host =
-	    dm_pool_strdup(mem, dm_config_find_str_allow_empty(cft->root, "creation_host", ""));
+	vgsummary->creation_host = dm_pool_strdup(mem, dm_config_find_str_allow_empty(cft->root, "creation_host", ""));
 	log_suppress(old_suppress);
 
 	/* skip any top-level values */
@@ -1073,7 +1073,11 @@ static int _read_vgname(const struct format_type *fmt, const struct dm_config_tr
 		return 0;
 	}
 
-	dm_config_get_str(vgn, "lock_type", &vgsummary->lock_type);
+	if (dm_config_get_str(vgn, "system_id", &str))
+		vgsummary->system_id = dm_pool_strdup(mem, str);
+
+	if (dm_config_get_str(vgn, "lock_type", &str))
+		vgsummary->lock_type = dm_pool_strdup(mem, str);
 
 	return 1;
 }
diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index f00968763..1d8e292ed 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -1238,4 +1238,6 @@ int validate_vg_rename_params(struct cmd_context *cmd,
 
 int is_lockd_type(const char *lock_type);
 
+int system_id_allowed(struct cmd_context *cmd, const char *system_id);
+
 #endif
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index b1ddb3440..ac009e7c0 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3556,6 +3556,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 	struct cached_vg_fmtdata *vg_fmtdata = NULL;	/* Additional format-specific data about the vg */
 	unsigned use_previous_vg;
 
+	log_very_verbose("Reading VG name %s vgid %s", vgname ?: "none", vgid ?: "none");
+
 	if (is_orphan_vg(vgname)) {
 		if (use_precommitted) {
 			log_error(INTERNAL_ERROR "vg_read_internal requires vgname "
@@ -3601,7 +3603,6 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 		correct_vg = NULL;
 	}
 
-
 	/* 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, 1))) {
@@ -3617,8 +3618,16 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 	}
 
 	/* Now determine the correct vgname if none was supplied */
-	if (!vgname && !(vgname = lvmcache_vgname_from_vgid(cmd->mem, vgid)))
+	if (!vgname && !(vgname = lvmcache_vgname_from_vgid(cmd->mem, vgid))) {
+		log_debug_metadata("Cache did not find VG name from vgid %.32s", vgid);
+		return_NULL;
+	}
+
+	/* Determine the correct vgid if none was supplied */
+	if (!vgid && !(vgid = lvmcache_vgid_from_vgname(cmd, vgname))) {
+		log_debug_metadata("Cache did not found vgid from VG name %s", vgname);
 		return_NULL;
+	}
 
 	if (use_precommitted && !(fmt->features & FMT_PRECOMMIT))
 		use_precommitted = 0;
@@ -4854,39 +4863,53 @@ static int _access_vg_lock_type(struct cmd_context *cmd, struct volume_group *vg
 	return 1;
 }
 
-static int _access_vg_systemid(struct cmd_context *cmd, struct volume_group *vg)
+int system_id_allowed(struct cmd_context *cmd, const char *system_id)
 {
 	/*
-	 * LVM1 VGs must not be accessed if a new-style LVM2 system ID is set.
+	 * A VG without a system_id can be accessed by anyone.
 	 */
-	if (cmd->system_id && systemid_on_pvs(vg)) {
-		log_error("Cannot access VG %s with LVM1 system ID %s when host system ID is set.",
-			  vg->name, vg->lvm1_system_id);
-		return 0;
-	}
+	if (!system_id || !system_id[0])
+		return 1;
 
 	/*
-	 * A VG without a system_id can be accessed by anyone.
+	 * Allowed if the host and VG system_id's match.
 	 */
-	if (!vg->system_id || !vg->system_id[0])
+	if (cmd->system_id && !strcmp(cmd->system_id, system_id))
 		return 1;
 
 	/*
-	 * A few commands allow read-only access to foreign VGs.
+	 * Allowed if a host's extra system_id matches.
 	 */
-	if (cmd->include_foreign_vgs)
+	if (cmd->system_id && _allow_extra_system_id(cmd, system_id))
 		return 1;
 
 	/*
-	 * A host can access a VG with a matching system_id.
+	 * Not allowed if the host does not have a system_id
+	 * and the VG does, or if the host and VG's system_id's
+	 * do not match.
 	 */
-	if (cmd->system_id && !strcmp(vg->system_id, cmd->system_id))
-		return 1;
+
+	return 0;
+}
+
+static int _access_vg_systemid(struct cmd_context *cmd, struct volume_group *vg)
+{
+	/*
+	 * LVM1 VGs must not be accessed if a new-style LVM2 system ID is set.
+	 */
+	if (cmd->system_id && systemid_on_pvs(vg)) {
+		log_error("Cannot access VG %s with LVM1 system ID %s when host system ID is set.",
+			  vg->name, vg->lvm1_system_id);
+		return 0;
+	}
 
 	/*
-	 * A host can access a VG if the VG's system_id is in extra_system_ids list.
+	 * A few commands allow read-only access to foreign VGs.
 	 */
-	if (cmd->system_id && _allow_extra_system_id(cmd, vg->system_id))
+	if (cmd->include_foreign_vgs)
+		return 1;
+
+	if (system_id_allowed(cmd, vg->system_id))
 		return 1;
 
 	/*
@@ -4901,7 +4924,8 @@ static int _access_vg_systemid(struct cmd_context *cmd, struct volume_group *vg)
 	}
 
 	/*
-	 * A host without a system_id cannot access a VG with a system_id.
+	 * Print an error when reading a VG that has a system_id
+	 * and the host system_id is unknown.
 	 */
 	if (!cmd->system_id || cmd->unknown_system_id) {
 		log_error("Cannot access VG %s with system ID %s with unknown local system ID.",
diff --git a/tools/toollib.c b/tools/toollib.c
index a52252e97..06ca69e41 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -1947,6 +1947,8 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 		skip = 0;
 		notfound = 0;
 
+		log_very_verbose("Process VG %s %.32s", vg_name, vg_uuid);
+
 		if (!lockd_vg(cmd, vg_name, NULL, 0, &lockd_state)) {
 			ret_max = ECMD_FAILED;
 			continue;
@@ -1966,6 +1968,9 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 		    (!dm_list_empty(arg_vgnames) && str_list_match_item(arg_vgnames, vg_name)) ||
 		    (!dm_list_empty(arg_tags) && str_list_match_list(arg_tags, &vg->tags, NULL))) &&
 		    select_match_vg(cmd, handle, vg, &selected) && selected) {
+
+			log_very_verbose("Process single VG %s", vg_name);
+
 			ret = process_single_vg(cmd, vg_name, vg, handle);
 			_update_selection_result(handle, &whole_selected);
 			if (ret != ECMD_PROCESSED)
@@ -1988,32 +1993,125 @@ endvg:
 }
 
 /*
- * Copy the contents of a str_list of VG names to a name list, filling
- * in the vgid with NULL (unknown).
+ * Check if a command line VG name is ambiguous, i.e. there are multiple VGs on
+ * the system that have the given name.  If *one* VG with the given name is
+ * local and the rest are foreign, then use the local VG (removing foreign VGs
+ * with the same name from the vgnameids_on_system list).  If multiple VGs with
+ * the given name are local, we don't know which VG is intended, so remove the
+ * ambiguous name from the list of args.
  */
-static int _copy_str_to_vgnameid_list(struct cmd_context *cmd, struct dm_list *sll,
-				      struct dm_list *vgnll)
-{
-	const char *vgname;
-	struct dm_str_list *sl;
-	struct vgnameid_list *vgnl;
+static int _resolve_duplicate_vgnames(struct cmd_context *cmd,
+				      struct dm_list *arg_vgnames,
+				      struct dm_list *vgnameids_on_system)
+{
+	struct dm_str_list *sl, *sl2;
+	struct vgnameid_list *vgnl, *vgnl2;
+	char uuid[64] __attribute__((aligned(8)));
+	int found;
+	int ret = ECMD_PROCESSED;
+
+	dm_list_iterate_items_safe(sl, sl2, arg_vgnames) {
+		found = 0;
+		dm_list_iterate_items(vgnl, vgnameids_on_system) {
+			if (strcmp(sl->str, vgnl->vg_name))
+				continue;
+			found++;
+		}
 
-	dm_list_iterate_items(sl, sll) {
-		vgname = sl->str;
+		if (found < 2)
+			continue;
 
-		vgnl = dm_pool_alloc(cmd->mem, sizeof(*vgnl));
-		if (!vgnl) {
-			log_error("vgnameid_list allocation failed.");
-			return ECMD_FAILED;
+		/*
+		 * More than one VG match the given name.
+		 * If only one is local, use that one.
+		 */
+
+		found = 0;
+		dm_list_iterate_items_safe(vgnl, vgnl2, vgnameids_on_system) {
+			if (strcmp(sl->str, vgnl->vg_name))
+				continue;
+
+			/*
+			 * Without lvmetad, a label scan has already populated
+			 * lvmcache vginfo with this information.
+			 * With lvmetad, this function does vg_lookup on this
+			 * name/vgid and checks system_id in the metadata.
+			 */
+			if (lvmcache_vg_is_foreign(cmd, vgnl->vg_name, vgnl->vgid)) {
+				id_write_format((const struct id*)vgnl->vgid, uuid, sizeof(uuid));
+				log_warn("WARNING: Ignoring foreign VG with matching name %s UUID %s.",
+					 vgnl->vg_name, uuid);
+				dm_list_del(&vgnl->list);
+			} else {
+				found++;
+			}
 		}
 
-		vgnl->vgid = NULL;
-		vgnl->vg_name = dm_pool_strdup(cmd->mem, vgname);
+		if (found < 2)
+			continue;
 
-		dm_list_add(vgnll, &vgnl->list);
+		/*
+		 * More than one VG with this name is local so the intended VG
+		 * is unknown.
+		 */
+		log_error("Multiple VGs found with the same name: %s", sl->str);
+		log_error("Use the --select option with VG UUID (vg_uuid).");
+		dm_list_del(&sl->list);
+		ret = ECMD_FAILED;
 	}
 
-	return ECMD_PROCESSED;
+	return ret;
+}
+
+/*
+ * For each arg_vgname, move the corresponding entry from
+ * vgnameids_on_system to vgnameids_to_process.  If an
+ * item in arg_vgnames doesn't exist in vgnameids_on_system,
+ * then add a new entry for it to vgnameids_to_process.
+ */
+static void _choose_vgs_to_process(struct cmd_context *cmd,
+				   struct dm_list *arg_vgnames,
+				   struct dm_list *vgnameids_on_system,
+				   struct dm_list *vgnameids_to_process)
+{
+	struct dm_str_list *sl, *sl2;
+	struct vgnameid_list *vgnl, *vgnl2;
+	int found;
+
+	dm_list_iterate_items_safe(sl, sl2, arg_vgnames) {
+		found = 0;
+		dm_list_iterate_items_safe(vgnl, vgnl2, vgnameids_on_system) {
+			if (strcmp(sl->str, vgnl->vg_name))
+				continue;
+
+			dm_list_del(&vgnl->list);
+			dm_list_add(vgnameids_to_process, &vgnl->list);
+			found = 1;
+			break;
+		}
+		
+		/*
+		 * If the name arg was not found in the list of all VGs, then
+		 * it probably doesn't exist, but we want the "VG not found"
+		 * failure to be handled by the existing vg_read() code for
+		 * that error.  So, create an entry with just the VG name so
+		 * that the processing loop will attempt to process it and use
+		 * the vg_read() error path.
+		 */
+		if (!found) {
+			log_verbose("VG name on command line not found in list of VGs: %s", sl->str);
+
+			if (!(vgnl = dm_pool_alloc(cmd->mem, sizeof(*vgnl))))
+				continue;
+
+			vgnl->vgid = NULL;
+
+			if (!(vgnl->vg_name = dm_pool_strdup(cmd->mem, sl->str)))
+				continue;
+
+			dm_list_add(vgnameids_to_process, &vgnl->list);
+		}
+	}
 }
 
 /*
@@ -2028,9 +2126,10 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
 	struct dm_list arg_vgnames;		/* str_list */
 	struct dm_list vgnameids_on_system;	/* vgnameid_list */
 	struct dm_list vgnameids_to_process;	/* vgnameid_list */
-
 	int enable_all_vgs = (cmd->command->flags & ALL_VGS_IS_DEFAULT);
 	int one_vgname_arg = (cmd->command->flags & ONE_VGNAME_ARG);
+	int process_all_vgs_on_system = 0;
+	int ret_max = ECMD_PROCESSED;
 	int ret;
 
 	/* Disable error in vg_read so we can print it from ignore_vg. */
@@ -2044,29 +2143,57 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
 	/*
 	 * Find any VGs or tags explicitly provided on the command line.
 	 */
-	if ((ret = _get_arg_vgnames(cmd, argc, argv, one_vgname_arg, &arg_vgnames, &arg_tags)) != ECMD_PROCESSED)
+	if ((ret = _get_arg_vgnames(cmd, argc, argv, one_vgname_arg, &arg_vgnames, &arg_tags)) != ECMD_PROCESSED) {
+		ret_max = ret;
 		goto_out;
+	}
 
 	/*
-	 * Obtain the complete list of VGs present on the system if it is needed because:
-	 *   any tags were supplied and need resolving; or
-	 *   no VG names were given and the command defaults to processing all VGs.
+	 * Process all VGs on the system when:
+	 * . tags are specified and all VGs need to be read to
+	 *   look for matching tags.
+	 * . no VG names are specified and the command defaults
+	 *   to processing all VGs when none are specified.
 	 */
-	if ((dm_list_empty(&arg_vgnames) && enable_all_vgs) || !dm_list_empty(&arg_tags)) {
-		/* Needed for a current listing of the global VG namespace. */
-		if (!lockd_gl(cmd, "sh", 0)) {
-			ret = ECMD_FAILED;
-			goto_out;
-		}
+	if ((dm_list_empty(&arg_vgnames) && enable_all_vgs) || !dm_list_empty(&arg_tags))
+		process_all_vgs_on_system = 1;
 
-		if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0))
-			goto_out;
+	/*
+	 * Needed for a current listing of the global VG namespace.
+	 */
+	if (process_all_vgs_on_system && !lockd_gl(cmd, "sh", 0)) {
+		ret_max = ECMD_FAILED;
+		goto_out;
+	}
+
+	/*
+	 * A list of all VGs on the system is needed when:
+	 * . processing all VGs on the system
+	 * . A VG name is specified which may refer to one
+	 *   of multiple VGs on the system with that name.
+	 */
+	log_very_verbose("Get list of VGs on system");
+
+	if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0)) {
+		ret_max = ECMD_FAILED;
+		goto_out;
+	}
+
+	if (!dm_list_empty(&arg_vgnames)) {
+		/* This may remove entries from arg_vgnames or vgnameids_on_system. */
+		ret = _resolve_duplicate_vgnames(cmd, &arg_vgnames, &vgnameids_on_system);
+		if (ret > ret_max)
+			ret_max = ret;
+		if (dm_list_empty(&arg_vgnames) && dm_list_empty(&arg_tags)) {
+			ret_max = ECMD_FAILED;
+			goto out;
+		}
 	}
 
 	if (dm_list_empty(&arg_vgnames) && dm_list_empty(&vgnameids_on_system)) {
 		/* FIXME Should be log_print, but suppressed for reporting cmds */
 		log_verbose("No volume groups found.");
-		ret = ECMD_PROCESSED;
+		ret_max = ECMD_PROCESSED;
 		goto out;
 	}
 
@@ -2074,28 +2201,37 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
 		read_flags |= READ_OK_NOTFOUND;
 
 	/*
-	 * If we obtained a full list of VGs on the system, we need to work through them all;
-	 * otherwise we can merely work through the VG names provided.
+	 * When processing all VGs, vgnameids_on_system simply becomes
+	 * vgnameids_to_process.
+	 * When processing only specified VGs, then for each item in
+	 * arg_vgnames, move the corresponding entry from
+	 * vgnameids_on_system to vgnameids_to_process.
 	 */
-	if (!dm_list_empty(&vgnameids_on_system))
+	if (process_all_vgs_on_system)
 		dm_list_splice(&vgnameids_to_process, &vgnameids_on_system);
-	else if ((ret = _copy_str_to_vgnameid_list(cmd, &arg_vgnames, &vgnameids_to_process)) != ECMD_PROCESSED)
-		goto_out;
+	else
+		_choose_vgs_to_process(cmd, &arg_vgnames, &vgnameids_on_system, &vgnameids_to_process);
 
-	if (!handle && !(handle = init_processing_handle(cmd)))
+	if (!handle && !(handle = init_processing_handle(cmd))) {
+		ret_max = ECMD_FAILED;
 		goto_out;
+	}
 
 	if (handle->internal_report_for_select && !handle->selection_handle &&
-	    !init_selection_handle(cmd, handle, VGS))
+	    !init_selection_handle(cmd, handle, VGS)) {
+		ret_max = ECMD_FAILED;
 		goto_out;
+	}
 
 	ret = _process_vgnameid_list(cmd, read_flags, &vgnameids_to_process,
 				     &arg_vgnames, &arg_tags, handle, process_single_vg);
+	if (ret > ret_max)
+		ret_max = ret;
 out:
 	if (!handle_supplied)
 		destroy_processing_handle(cmd, handle);
 
-	return ret;
+	return ret_max;
 }
 
 int process_each_lv_in_vg(struct cmd_context *cmd, struct volume_group *vg,
@@ -2440,6 +2576,8 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag
 			}
 		}
 
+		log_very_verbose("Process VG %s %.32s", vg_name, vg_uuid);
+
 		if (!lockd_vg(cmd, vg_name, NULL, 0, &lockd_state)) {
 			ret_max = ECMD_FAILED;
 			continue;
@@ -2483,9 +2621,9 @@ int process_each_lv(struct cmd_context *cmd, int argc, char **argv, uint32_t rea
 	struct dm_list arg_lvnames;		/* str_list */
 	struct dm_list vgnameids_on_system;	/* vgnameid_list */
 	struct dm_list vgnameids_to_process;	/* vgnameid_list */
-
 	int enable_all_vgs = (cmd->command->flags & ALL_VGS_IS_DEFAULT);
-	int need_vgnameids = 0;
+	int process_all_vgs_on_system = 0;
+	int ret_max = ECMD_PROCESSED;
 	int ret;
 
 	/* Disable error in vg_read so we can print it from ignore_vg. */
@@ -2500,44 +2638,74 @@ int process_each_lv(struct cmd_context *cmd, int argc, char **argv, uint32_t rea
 	/*
 	 * Find any LVs, VGs or tags explicitly provided on the command line.
 	 */
-	if ((ret = _get_arg_lvnames(cmd, argc, argv, &arg_vgnames, &arg_lvnames, &arg_tags) != ECMD_PROCESSED))
+	if ((ret = _get_arg_lvnames(cmd, argc, argv, &arg_vgnames, &arg_lvnames, &arg_tags) != ECMD_PROCESSED)) {
+		ret_max = ret;
 		goto_out;
+	}
 
-	if (!handle && !(handle = init_processing_handle(cmd)))
+	if (!handle && !(handle = init_processing_handle(cmd))) {
+		ret_max = ECMD_FAILED;
 		goto_out;
+	}
 
 	if (handle->internal_report_for_select && !handle->selection_handle &&
-	    !init_selection_handle(cmd, handle, LVS))
+	    !init_selection_handle(cmd, handle, LVS)) {
+		ret_max = ECMD_FAILED;
 		goto_out;
+	}
 
 	/*
-	 * Obtain the complete list of VGs present on the system if it is needed because:
-	 *   any tags were supplied and need resolving; or
-	 *   no VG names were given and the select option needs resolving; or
-	 *   no VG names were given and the command defaults to processing all VGs.
-	*/
+	 * Process all VGs on the system when:
+	 * . tags are specified and all VGs need to be read to
+	 *   look for matching tags.
+	 * . no VG names are specified and the command defaults
+	 *   to processing all VGs when none are specified.
+	 * . no VG names are specified and the select option needs
+	 *   resolving.
+	 */
 	if (!dm_list_empty(&arg_tags))
-		need_vgnameids = 1;
+		process_all_vgs_on_system = 1;
 	else if (dm_list_empty(&arg_vgnames) && enable_all_vgs)
-		need_vgnameids = 1;
+		process_all_vgs_on_system = 1;
 	else if (dm_list_empty(&arg_vgnames) && handle->internal_report_for_select)
-		need_vgnameids = 1;
+		process_all_vgs_on_system = 1;
 
-	if (need_vgnameids) {
-		/* Needed for a current listing of the global VG namespace. */
-		if (!lockd_gl(cmd, "sh", 0)) {
-			ret = ECMD_FAILED;
-			goto_out;
-		}
+	/*
+	 * Needed for a current listing of the global VG namespace.
+	 */
+	if (process_all_vgs_on_system && !lockd_gl(cmd, "sh", 0)) {
+		ret_max = ECMD_FAILED;
+		goto_out;
+	}
 
-		if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0))
-			goto_out;
+	/*
+	 * A list of all VGs on the system is needed when:
+	 * . processing all VGs on the system
+	 * . A VG name is specified which may refer to one
+	 *   of multiple VGs on the system with that name.
+	 */
+	log_very_verbose("Get list of VGs on system");
+
+	if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0)) {
+		ret_max = ECMD_FAILED;
+		goto_out;
+	}
+
+	if (!dm_list_empty(&arg_vgnames)) {
+		/* This may remove entries from arg_vgnames or vgnameids_on_system. */
+		ret = _resolve_duplicate_vgnames(cmd, &arg_vgnames, &vgnameids_on_system);
+		if (ret > ret_max)
+			ret_max = ret;
+		if (dm_list_empty(&arg_vgnames) && dm_list_empty(&arg_tags)) {
+			ret_max = ECMD_FAILED;
+			goto out;
+		}
 	}
 
 	if (dm_list_empty(&arg_vgnames) && dm_list_empty(&vgnameids_on_system)) {
 		/* FIXME Should be log_print, but suppressed for reporting cmds */
 		log_verbose("No volume groups found.");
-		ret = ECMD_PROCESSED;
+		ret_max = ECMD_PROCESSED;
 		goto out;
 	}
 
@@ -2545,20 +2713,27 @@ int process_each_lv(struct cmd_context *cmd, int argc, char **argv, uint32_t rea
 		read_flags |= READ_OK_NOTFOUND;
 
 	/*
-	 * If we obtained a full list of VGs on the system, we need to work through them all;
-	 * otherwise we can merely work through the VG names provided.
+	 * When processing all VGs, vgnameids_on_system simply becomes
+	 * vgnameids_to_process.
+	 * When processing only specified VGs, then for each item in
+	 * arg_vgnames, move the corresponding entry from
+	 * vgnameids_on_system to vgnameids_to_process.
 	 */
-	if (!dm_list_empty(&vgnameids_on_system))
+	if (process_all_vgs_on_system)
 		dm_list_splice(&vgnameids_to_process, &vgnameids_on_system);
-	else if ((ret = _copy_str_to_vgnameid_list(cmd, &arg_vgnames, &vgnameids_to_process)) != ECMD_PROCESSED)
-		goto_out;
+	else
+		_choose_vgs_to_process(cmd, &arg_vgnames, &vgnameids_on_system, &vgnameids_to_process);
 
 	ret = _process_lv_vgnameid_list(cmd, read_flags, &vgnameids_to_process, &arg_vgnames, &arg_lvnames,
 					&arg_tags, handle, process_single_lv);
+
+	if (ret > ret_max)
+		ret_max = ret;
 out:
 	if (!handle_supplied)
 		destroy_processing_handle(cmd, handle);
-	return ret;
+
+	return ret_max;
 }
 
 static int _get_arg_pvnames(struct cmd_context *cmd,