shared and foreign VG access

9 files changed, 102 insertions, 47 deletions

diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h
index 5d1faaea3..33c4706cc 100644
--- a/lib/commands/toolcontext.h
+++ b/lib/commands/toolcontext.h
@@ -99,9 +99,10 @@ struct cmd_context {
 
 	unsigned independent_metadata_areas:1;	/* Active formats have MDAs outside PVs */
 	unsigned unknown_system_id:1;
-	unsigned include_foreign_vgs:1;
-	unsigned include_active_foreign_vgs:1;
-	unsigned error_foreign_vgs:1;
+	unsigned include_foreign_vgs:1; /* report/display cmds can reveal foreign VGs */
+	unsigned include_shared_vgs:1;  /* report/display cmds can reveal lockd VGs */
+	unsigned include_active_foreign_vgs:1; /* cmd should process foreign VGs with active LVs */
+	unsigned vg_read_print_access_error:1; /* print access errors from vg_read */
 	unsigned lockd_vg_disable:1;
 	unsigned lockd_lv_disable:1;
 	unsigned lockd_vg_default_sh:1;
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index b041cee65..ff6b0d36d 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -4651,22 +4651,44 @@ static int _access_vg_lock_type(struct cmd_context *cmd, struct volume_group *vg
 		return 1;
 
 	/*
-	 * When lvmlockd is not used, only allow read access to the VG.
+	 * When lvmlockd is not used, lockd VGs are ignored by lvm
+	 * and cannot be used, with two exceptions:
+	 *
+	 * . The --shared option allows them to be revealed with
+	 *   reporting/display commands.
+	 *
+	 * . If a command asks to operate on one specifically
+	 *   by name, then an error is printed.
 	 */
 	if (!lvmlockd_use()) {
-		if (lockd_state & LDST_EX) {
-			log_error("Cannot access VG %s which requires lvmlockd for lock_type %s.",
-				  vg->name, vg->lock_type);
-			return 0;
-		} else {
-			log_warn("Reading VG %s without a lock.", vg->name);
+		/*
+	 	 * Some reporting/display commands have the --shared option
+		 * (like --foreign) to allow them to reveal lockd VGs that
+		 * are otherwise ignored.  The --shared option must only be
+		 * permitted in commands that read the VG for report or display,
+		 * not any that write the VG or activate LVs.
+	 	 */
+		if (cmd->include_shared_vgs)
 			return 1;
+
+		/*
+		 * Some commands want the error printed by vg_read, others by ignore_vg.
+		 * Those using ignore_vg may choose to skip the error.
+		 */
+		if (cmd->vg_read_print_access_error) {
+			log_error("Cannot access VG %s with lock type %s that requires lvmlockd.",
+				  vg->name, vg->lock_type);
 		}
+
+		return 0;
 	}
 
 	/*
-	 * The lock failed.  If the lock was ex, we cannot continue.
-	 * If the lock was sh, we can allow reading.
+	 * The lock request from lvmlockd failed.  If the lock was ex,
+	 * we cannot continue.  If the lock was sh, we could also fail
+	 * to continue but since the lock was sh, it means the VG is
+	 * only being read, and it doesn't hurt to allow reading with
+	 * no lock.
 	 */
 	if (lockd_state & LDST_FAIL) {
 		if (lockd_state & LDST_EX) {
@@ -4737,18 +4759,16 @@ static int _access_vg_systemid(struct cmd_context *cmd, struct volume_group *vg)
 	}
 
 	/*
-	 * Some commands always produce an error when accessing foreign VG.
+	 * Some commands want the error printed by vg_read, others by ignore_vg.
+	 * Those using ignore_vg may choose to skip the error.
 	 */
-	if (cmd->error_foreign_vgs) {
+	if (cmd->vg_read_print_access_error) {
 		log_error("Cannot access VG %s with system ID %s with local system ID %s.",
 			  vg->name, vg->system_id, cmd->system_id);
 		return 0;
 	}
 
-	/*
-	 * When include_foreign_vgs is 0 and error_foreign_vgs is 0,
-	 * the result is to silently ignore foreign vgs.
-	 */
+	/* Silently ignore foreign vgs. */
 
 	return 0;
 }
diff --git a/tools/commands.h b/tools/commands.h
index 5be0622e3..43d5c80d4 100644
--- a/tools/commands.h
+++ b/tools/commands.h
@@ -394,7 +394,7 @@ xx(lvcreate,
 
 xx(lvdisplay,
    "Display information about a logical volume",
-   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH,
    "lvdisplay\n"
    "\t[-a|--all]\n"
    "\t[-c|--colon]\n"
@@ -442,7 +442,7 @@ xx(lvdisplay,
     aligned_ARG, all_ARG, binary_ARG, colon_ARG, columns_ARG, foreign_ARG,
     ignorelockingfailure_ARG, ignoreskippedcluster_ARG, maps_ARG,
     noheadings_ARG, nosuffix_ARG, options_ARG, sort_ARG, partial_ARG,
-    readonly_ARG, segments_ARG, select_ARG, separator_ARG,
+    readonly_ARG, segments_ARG, select_ARG, separator_ARG, shared_ARG,
     unbuffered_ARG, units_ARG)
 
 xx(lvextend,
@@ -646,7 +646,7 @@ xx(lvresize,
 
 xx(lvs,
    "Display information about logical volumes",
-   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH,
    "lvs\n"
    "\t[-a|--all]\n"
    "\t[--aligned]\n"
@@ -679,7 +679,7 @@ xx(lvs,
    aligned_ARG, all_ARG, binary_ARG, foreign_ARG, ignorelockingfailure_ARG,
    ignoreskippedcluster_ARG, nameprefixes_ARG, noheadings_ARG,
    nolocking_ARG, nosuffix_ARG, options_ARG, partial_ARG,
-   readonly_ARG, rows_ARG, segments_ARG, select_ARG, separator_ARG,
+   readonly_ARG, rows_ARG, segments_ARG, select_ARG, separator_ARG, shared_ARG,
    sort_ARG, trustcache_ARG, unbuffered_ARG, units_ARG, unquoted_ARG)
 
 xx(lvscan,
@@ -810,7 +810,7 @@ xx(pvdata,
 
 xx(pvdisplay,
    "Display various attributes of physical volume(s)",
-   CACHE_VGMETADATA | PERMITTED_READ_ONLY | ENABLE_ALL_DEVS | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   CACHE_VGMETADATA | PERMITTED_READ_ONLY | ENABLE_ALL_DEVS | LOCKD_VG_SH,
    "pvdisplay\n"
    "\t[-c|--colon]\n"
    "\t[--commandprofile ProfileName]\n"
@@ -855,7 +855,7 @@ xx(pvdisplay,
    aligned_ARG, all_ARG, binary_ARG, colon_ARG, columns_ARG, foreign_ARG,
    ignorelockingfailure_ARG, ignoreskippedcluster_ARG, maps_ARG,
    noheadings_ARG, nosuffix_ARG, options_ARG, readonly_ARG,
-   select_ARG, separator_ARG, short_ARG, sort_ARG, unbuffered_ARG,
+   select_ARG, separator_ARG, shared_ARG, short_ARG, sort_ARG, unbuffered_ARG,
    units_ARG)
 
 xx(pvmove,
@@ -919,7 +919,7 @@ xx(pvremove,
 
 xx(pvs,
    "Display information about physical volumes",
-   CACHE_VGMETADATA | PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_ALL_DEVS | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   CACHE_VGMETADATA | PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_ALL_DEVS | LOCKD_VG_SH,
    "pvs\n"
    "\t[-a|--all]\n"
    "\t[--aligned]\n"
@@ -952,12 +952,12 @@ xx(pvs,
    aligned_ARG, all_ARG, binary_ARG, foreign_ARG, ignorelockingfailure_ARG,
    ignoreskippedcluster_ARG, nameprefixes_ARG, noheadings_ARG, nolocking_ARG,
    nosuffix_ARG, options_ARG, partial_ARG, readonly_ARG, rows_ARG,
-   segments_ARG, select_ARG, separator_ARG, sort_ARG, trustcache_ARG,
+   segments_ARG, select_ARG, separator_ARG, shared_ARG, sort_ARG, trustcache_ARG,
    unbuffered_ARG, units_ARG, unquoted_ARG)
 
 xx(pvscan,
    "List all physical volumes",
-   PERMITTED_READ_ONLY | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | LOCKD_VG_SH,
    "pvscan\n"
    "\t[-b|--background]\n"
    "\t[--cache [-a|--activate ay] [ DevicePath | -j|--major major --minor minor]...]\n"
@@ -994,7 +994,7 @@ xx(tags,
 
 xx(vgcfgbackup,
    "Backup volume group configuration(s)",
-   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH,
    "vgcfgbackup\n"
    "\t[--commandprofile ProfileName]\n"
    "\t[-d|--debug]\n"
@@ -1143,7 +1143,7 @@ xx(vgcreate,
 
 xx(vgdisplay,
    "Display volume group information",
-   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH,
    "vgdisplay\n"
    "\t[-A|--activevolumegroups]\n"
    "\t[-c|--colon | -s|--short | -v|--verbose]\n"
@@ -1187,7 +1187,7 @@ xx(vgdisplay,
    activevolumegroups_ARG, aligned_ARG, binary_ARG, colon_ARG, columns_ARG,
    foreign_ARG, ignorelockingfailure_ARG, ignoreskippedcluster_ARG,
    noheadings_ARG, nosuffix_ARG, options_ARG, partial_ARG, readonly_ARG,
-   select_ARG, short_ARG, separator_ARG, sort_ARG, unbuffered_ARG, units_ARG)
+   select_ARG, shared_ARG, short_ARG, separator_ARG, sort_ARG, unbuffered_ARG, units_ARG)
 
 xx(vgexport,
    "Unregister volume group(s) from the system",
@@ -1331,7 +1331,7 @@ xx(vgrename,
 
 xx(vgs,
    "Display information about volume groups",
-   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH,
    "vgs\n"
    "\t[--aligned]\n"
    "\t[--binary]\n"
@@ -1363,12 +1363,12 @@ xx(vgs,
    aligned_ARG, all_ARG, binary_ARG, foreign_ARG, ignorelockingfailure_ARG,
    ignoreskippedcluster_ARG, nameprefixes_ARG, noheadings_ARG,
    nolocking_ARG, nosuffix_ARG, options_ARG, partial_ARG,
-   readonly_ARG, rows_ARG, select_ARG, separator_ARG, sort_ARG,
+   readonly_ARG, rows_ARG, select_ARG, separator_ARG, shared_ARG, sort_ARG,
    trustcache_ARG, unbuffered_ARG, units_ARG, unquoted_ARG)
 
 xx(vgscan,
    "Search for all volume groups",
-   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_FOREIGN_VGS | LOCKD_VG_SH,
+   PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH,
    "vgscan "
    "\t[--cache]\n"
    "\t[--commandprofile ProfileName]\n"
diff --git a/tools/lvchange.c b/tools/lvchange.c
index e97644e23..2a57abb16 100644
--- a/tools/lvchange.c
+++ b/tools/lvchange.c
@@ -1278,6 +1278,11 @@ int lvchange(struct cmd_context *cmd, int argc, char **argv)
 		}
 	}
 
+	/*
+	 * Include foreign VGs that contain active LVs.
+	 * That shouldn't happen in general, but if it does by some
+	 * mistake, then we want to allow those LVs to be deactivated.
+	 */
 	if (arg_is_set(cmd, activate_ARG))
 		cmd->include_active_foreign_vgs = 1;
 
diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c
index 1025b953f..f280954fc 100644
--- a/tools/lvmcmdline.c
+++ b/tools/lvmcmdline.c
@@ -1105,9 +1105,14 @@ static int _get_settings(struct cmd_context *cmd)
 		init_ignorelockingfailure(0);
 
 	cmd->ignore_clustered_vgs = arg_is_set(cmd, ignoreskippedcluster_ARG);
-	cmd->error_foreign_vgs = cmd->command->flags & ENABLE_FOREIGN_VGS ? 0 : 1;
 	cmd->include_foreign_vgs = arg_is_set(cmd, foreign_ARG) ? 1 : 0;
-	cmd->include_active_foreign_vgs = cmd->command->flags & ENABLE_FOREIGN_VGS ? 1 : 0;
+	cmd->include_shared_vgs = arg_is_set(cmd, shared_ARG) ? 1 : 0;
+
+	/*
+	 * This is set to zero by process_each which wants to print errors
+	 * itself rather than having them printed in vg_read.
+	 */
+	cmd->vg_read_print_access_error = 1;
 		
 	if (!arg_count(cmd, sysinit_ARG))
 		lvmetad_connect_or_warn();
diff --git a/tools/reporter.c b/tools/reporter.c
index eb6a4ed4e..c7a927b46 100644
--- a/tools/reporter.c
+++ b/tools/reporter.c
@@ -626,6 +626,14 @@ static int _report(struct cmd_context *cmd, int argc, char **argv,
 	quoted = find_config_tree_bool(cmd, report_quoted_CFG, NULL);
 	columns_as_rows = find_config_tree_bool(cmd, report_colums_as_rows_CFG, NULL);
 
+	/*
+	 * Include foreign VGs that contain active LVs.
+	 * That shouldn't happen in general, but if it does by some
+	 * mistake, then we want to display those VGs and allow the
+	 * LVs to be deactivated.
+	 */
+	cmd->include_active_foreign_vgs = 1;
+
 	/* Check PV specifics and do extra changes/actions if needed. */
 	_check_pv_list(cmd, argc, argv, &report_type, &args_are_pvs);
 
diff --git a/tools/toollib.c b/tools/toollib.c
index c01a8a9d0..6ef6080d4 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -217,18 +217,28 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 		}
 	}
 
+	/*
+	 * Accessing a lockd VG when lvmlockd is not used is similar
+	 * to accessing a foreign VG.
+	 */
+	if (read_error & FAILED_LOCK_TYPE) {
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
+			log_error("Cannot access VG %s with lock_type %s that requires lvmlockd.",
+				  vg->name, vg->lock_type);
+			return 1;
+		} else {
+			read_error &= ~FAILED_LOCK_TYPE; /* Check for other errors */
+			log_verbose("Skipping volume group %s", vg_name);
+			*skip = 1;
+		}
+	}
+
 	if (read_error == FAILED_CLUSTERED) {
 		*skip = 1;
 		stack;	/* Error already logged */
 		return 1;
 	}
 
-	if (read_error & FAILED_LOCK_TYPE) {
-		read_error &= ~FAILED_LOCK_TYPE; /* Check for other errors */
-		log_verbose("Skipping volume group %s", vg_name);
-		*skip = 1;
-	}
-
 	if (read_error != SUCCESS) {
 		*skip = 0;
 		log_error("Cannot process volume group %s", vg_name);
@@ -1986,7 +1996,8 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
 	unsigned one_vgname_arg = (flags & ONE_VGNAME_ARG);
 	int ret;
 
-	cmd->error_foreign_vgs = 0;
+	/* Disable error in vg_read so we can print it from ignore_vg. */
+	cmd->vg_read_print_access_error = 0;
 
 	dm_list_init(&arg_tags);
 	dm_list_init(&arg_vgnames);
@@ -2418,7 +2429,8 @@ int process_each_lv(struct cmd_context *cmd, int argc, char **argv, uint32_t fla
 	int need_vgnameids = 0;
 	int ret;
 
-	cmd->error_foreign_vgs = 0;
+	/* Disable error in vg_read so we can print it from ignore_vg. */
+	cmd->vg_read_print_access_error = 0;
 
 	dm_list_init(&arg_tags);
 	dm_list_init(&arg_vgnames);
@@ -2922,7 +2934,8 @@ int process_each_pv(struct cmd_context *cmd,
 	int ret_max = ECMD_PROCESSED;
 	int ret;
 
-	cmd->error_foreign_vgs = 0;
+	/* Disable error in vg_read so we can print it from ignore_vg. */
+	cmd->vg_read_print_access_error = 0;
 
 	dm_list_init(&arg_tags);
 	dm_list_init(&arg_pvnames);
diff --git a/tools/tools.h b/tools/tools.h
index 640aa1544..675867de2 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -109,10 +109,8 @@ struct arg_value_group_list {
 #define ENABLE_ALL_DEVS		0x00000008	
 /* Exactly one VG name argument required. */
 #define ONE_VGNAME_ARG		0x00000010
-/* Command is allowed to read foreign VGs. */
-#define ENABLE_FOREIGN_VGS	0x00000020
 /* Command needs a shared lock on a VG; it only reads the VG. */
-#define LOCKD_VG_SH		0x00000040
+#define LOCKD_VG_SH		0x00000020
  
 /* a register of the lvm commands */
 struct command {
diff --git a/tools/vgchange.c b/tools/vgchange.c
index 7a556e12d..7c710a11f 100644
--- a/tools/vgchange.c
+++ b/tools/vgchange.c
@@ -1195,6 +1195,11 @@ int vgchange(struct cmd_context *cmd, int argc, char **argv)
 	if (!update || !update_partial_unsafe)
 		cmd->handles_missing_pvs = 1;
 
+	/*
+	 * Include foreign VGs that contain active LVs.
+	 * That shouldn't happen in general, but if it does by some
+	 * mistake, then we want to allow those LVs to be deactivated.
+	 */
 	if (arg_is_set(cmd, activate_ARG))
 		cmd->include_active_foreign_vgs = 1;