vg_read: improve VG not found handlingdev-dct-vg-read

Also fix the ONE_VGNAME_ARG flag which was passed and checked
as a function arg, but is actually a struct flag.

6 files changed, 61 insertions, 34 deletions

diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h
index 25320fdd9..41838a8ad 100644
--- a/lib/metadata/metadata-exported.h
+++ b/lib/metadata/metadata-exported.h
@@ -163,10 +163,9 @@
 /* vg_read and vg_read_for_update flags */
 #define READ_ALLOW_INCONSISTENT	0x00010000U
 #define READ_ALLOW_EXPORTED	0x00020000U
+#define READ_OK_NOTFOUND	0x00040000U
 #define READ_WARN_INCONSISTENT	0x00080000U
-
-/* A meta-flag, useful with toollib for_each_* functions. */
-#define READ_FOR_UPDATE		0x00100000U
+#define READ_FOR_UPDATE		0x00100000U /* A meta-flag, useful with toollib for_each_* functions. */
 
 /* vg's "read_status" field */
 #define FAILED_INCONSISTENT	0x00000001U
@@ -650,9 +649,9 @@ int lv_resize(struct cmd_context *cmd, struct logical_volume *lv,
  * Return a handle to VG metadata.
  */
 struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
-			     const char *vgid, uint32_t flags, uint32_t lockd_state);
+			     const char *vgid, uint32_t read_flags, uint32_t lockd_state);
 struct volume_group *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
-			 const char *vgid, uint32_t flags, uint32_t lockd_state);
+			 const char *vgid, uint32_t read_flags, uint32_t lockd_state);
 
 /* 
  * Test validity of a VG handle.
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 9bd42c91b..1db8f581e 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -4925,7 +4925,7 @@ static int _vg_access_permitted(struct cmd_context *cmd, struct volume_group *vg
  * Consolidated locking, reading, and status flag checking.
  *
  * If the metadata is inconsistent, setting READ_ALLOW_INCONSISTENT in
- * misc_flags will return it with FAILED_INCONSISTENT set instead of 
+ * read_flags will return it with FAILED_INCONSISTENT set instead of 
  * giving you nothing.
  *
  * Use vg_read_error(vg) to determine the result.  Nonzero means there were
@@ -4933,8 +4933,10 @@ static int _vg_access_permitted(struct cmd_context *cmd, struct volume_group *vg
  * Zero value means that the VG is open and appropriate locks are held.
  */
 static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const char *vg_name,
-			       const char *vgid, uint32_t lock_flags,
-			       uint64_t status_flags, uint32_t misc_flags,
+			       const char *vgid,
+			       uint32_t lock_flags,
+			       uint64_t status_flags,
+			       uint32_t read_flags,
 			       uint32_t lockd_state)
 {
 	struct volume_group *vg = NULL;
@@ -4944,7 +4946,7 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha
 	uint32_t warn_flags = 0;
 	int already_locked;
 
-	if (misc_flags & READ_ALLOW_INCONSISTENT || lock_flags != LCK_VG_WRITE)
+	if ((read_flags & READ_ALLOW_INCONSISTENT) || (lock_flags != LCK_VG_WRITE))
 		consistent = 0;
 
 	if (!validate_name(vg_name) && !is_orphan_vg(vg_name)) {
@@ -4967,7 +4969,7 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha
 	consistent_in = consistent;
 
 	warn_flags = WARN_PV_READ;
-	if (consistent || (misc_flags & READ_WARN_INCONSISTENT))
+	if (consistent || (read_flags & READ_WARN_INCONSISTENT))
 		warn_flags |= WARN_INCONSISTENT;
 
 	/* If consistent == 1, we get NULL here if correction fails. */
@@ -4976,7 +4978,8 @@ static struct volume_group *_vg_lock_and_read(struct cmd_context *cmd, const cha
 			failure |= FAILED_INCONSISTENT;
 			goto bad;
 		}
-		log_error("Volume group \"%s\" not found", vg_name);
+		if (!(read_flags & READ_OK_NOTFOUND))
+			log_error("Volume group \"%s\" not found", vg_name);
 		failure |= FAILED_NOTFOUND;
 		goto bad;
 	}
@@ -5057,20 +5060,20 @@ bad_no_unlock:
  * *consistent = 1.
  */
 struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
-			     const char *vgid, uint32_t flags, uint32_t lockd_state)
+			     const char *vgid, uint32_t read_flags, uint32_t lockd_state)
 {
-	uint64_t status = UINT64_C(0);
+	uint64_t status_flags = UINT64_C(0);
 	uint32_t lock_flags = LCK_VG_READ;
 
-	if (flags & READ_FOR_UPDATE) {
-		status |= EXPORTED_VG | LVM_WRITE;
+	if (read_flags & READ_FOR_UPDATE) {
+		status_flags |= EXPORTED_VG | LVM_WRITE;
 		lock_flags = LCK_VG_WRITE;
 	}
 
-	if (flags & READ_ALLOW_EXPORTED)
-		status &= ~EXPORTED_VG;
+	if (read_flags & READ_ALLOW_EXPORTED)
+		status_flags &= ~EXPORTED_VG;
 
-	return _vg_lock_and_read(cmd, vg_name, vgid, lock_flags, status, flags, lockd_state);
+	return _vg_lock_and_read(cmd, vg_name, vgid, lock_flags, status_flags, read_flags, lockd_state);
 }
 
 /*
@@ -5079,9 +5082,9 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
  * request the new metadata to be written and committed).
  */
 struct volume_group *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
-			 const char *vgid, uint32_t flags, uint32_t lockd_state)
+			 const char *vgid, uint32_t read_flags, uint32_t lockd_state)
 {
-	return vg_read(cmd, vg_name, vgid, flags | READ_FOR_UPDATE, lockd_state);
+	return vg_read(cmd, vg_name, vgid, read_flags | READ_FOR_UPDATE, lockd_state);
 }
 
 /*
diff --git a/tools/commands.h b/tools/commands.h
index 92d13fdf0..ae5dff7fd 100644
--- a/tools/commands.h
+++ b/tools/commands.h
@@ -1211,7 +1211,7 @@ xx(vgexport,
 
 xx(vgextend,
    "Add physical volumes to a volume group",
-   0,
+   ONE_VGNAME_ARG,
    "vgextend\n"
    "\t[-A|--autobackup y|n]\n"
    "\t[--restoremissing]\n"
diff --git a/tools/toollib.c b/tools/toollib.c
index dfb2b87e2..39bf0ac12 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -184,12 +184,24 @@ const char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
  *   If *skip is 1, it's OK for the caller to read the list of PVs in the VG.
  */
 static int _ignore_vg(struct volume_group *vg, const char *vg_name,
-		      struct dm_list *arg_vgnames, int allow_inconsistent, int *skip)
+		      struct dm_list *arg_vgnames, uint32_t read_flags, int *skip)
 {
 	uint32_t read_error = vg_read_error(vg);
 	*skip = 0;
 
-	if ((read_error & FAILED_INCONSISTENT) && allow_inconsistent)
+	if ((read_error & FAILED_NOTFOUND) && (read_flags & READ_OK_NOTFOUND)) {
+		read_error &= ~FAILED_NOTFOUND;
+		*skip = 1;
+		return 0;
+	}
+
+	if ((read_error & FAILED_INCONSISTENT) && (read_flags & READ_OK_NOTFOUND)) {
+		read_error &= ~FAILED_INCONSISTENT;
+		*skip = 1;
+		return 0;
+	}
+
+	if ((read_error & FAILED_INCONSISTENT) && (read_flags & READ_ALLOW_INCONSISTENT))
 		read_error &= ~FAILED_INCONSISTENT; /* Check for other errors */
 
 	if ((read_error & FAILED_CLUSTERED) && vg->cmd->ignore_clustered_vgs) {
@@ -1943,7 +1955,7 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t flags,
 		}
 
 		vg = vg_read(cmd, vg_name, vg_uuid, flags, lockd_state);
-		if (_ignore_vg(vg, vg_name, arg_vgnames, flags & READ_ALLOW_INCONSISTENT, &skip)) {
+		if (_ignore_vg(vg, vg_name, arg_vgnames, flags, &skip)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			goto endvg;
@@ -2020,7 +2032,7 @@ int process_each_vg(struct cmd_context *cmd, int argc, char **argv,
 	struct dm_list vgnameids_to_process;	/* vgnameid_list */
 
 	int enable_all_vgs = (cmd->command->flags & ALL_VGS_IS_DEFAULT);
-	unsigned one_vgname_arg = (flags & ONE_VGNAME_ARG);
+	int one_vgname_arg = (cmd->command->flags & ONE_VGNAME_ARG);
 	int ret;
 
 	/* Disable error in vg_read so we can print it from ignore_vg. */
@@ -2431,7 +2443,7 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t flags,
 		}
 
 		vg = vg_read(cmd, vg_name, vg_uuid, flags, lockd_state);
-		if (_ignore_vg(vg, vg_name, arg_vgnames, flags & READ_ALLOW_INCONSISTENT, &skip)) {
+		if (_ignore_vg(vg, vg_name, arg_vgnames, flags, &skip)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			goto endvg;
@@ -2897,7 +2909,7 @@ out:
  * should produce an error.  Any devices remaining in all_devices were
  * not found and should be processed by process_device_list().
  */
-static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t flags,
+static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 			       struct dm_list *all_vgnameids,
 			       struct dm_list *all_devices,
 			       struct dm_list *arg_devices,
@@ -2929,8 +2941,8 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t flags,
 			continue;
 		}
 
-		vg = vg_read(cmd, vg_name, vg_uuid, flags | READ_WARN_INCONSISTENT, lockd_state);
-		if (_ignore_vg(vg, vg_name, NULL, flags & READ_ALLOW_INCONSISTENT, &skip)) {
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		if (_ignore_vg(vg, vg_name, NULL, read_flags, &skip)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			if (!skip)
@@ -2969,7 +2981,7 @@ endvg:
 int process_each_pv(struct cmd_context *cmd,
 		    int argc, char **argv,
 		    const char *only_this_vgname,
-		    uint32_t flags,
+		    uint32_t read_flags,
 		    struct processing_handle *handle,
 		    process_single_pv_fn_t process_single_pv)
 {
@@ -2984,6 +2996,19 @@ int process_each_pv(struct cmd_context *cmd,
 	int ret_max = ECMD_PROCESSED;
 	int ret;
 
+	/*
+	 * When processing a specific VG name, warn if it's inconsistent and
+	 * print an error if it's not found.  Otherwise we're processing all
+	 * VGs, in which case the command doesn't care if the VG is inconsisent
+	 * or not found; it just wants to skip that VG.  (It may be not found
+	 * if it was removed between creating the list of all VGs and then
+	 * processing each VG.
+	 */
+	if (only_this_vgname)
+		read_flags |= READ_WARN_INCONSISTENT;
+	else
+		read_flags |= READ_OK_NOTFOUND;
+
 	/* Disable error in vg_read so we can print it from ignore_vg. */
 	cmd->vg_read_print_access_error = 0;
 
@@ -3040,7 +3065,7 @@ int process_each_pv(struct cmd_context *cmd,
 		/* get_arg_devices reports the error for any PV names not found. */
 		ret_max = ECMD_FAILED;
 
-	ret = _process_pvs_in_vgs(cmd, flags, &all_vgnameids, &all_devices,
+	ret = _process_pvs_in_vgs(cmd, read_flags, &all_vgnameids, &all_devices,
 				  &arg_devices, &arg_tags,
 				  process_all_pvs, process_all_devices,
 				  handle, process_single_pv);
diff --git a/tools/tools.h b/tools/tools.h
index 4ed893fc4..9857466de 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -99,12 +99,12 @@ struct arg_value_group_list {
 #define ALL_VGS_IS_DEFAULT	0x00000004
 /* Process all devices with --all if none are specified on the command line. */
 #define ENABLE_ALL_DEVS		0x00000008	
-/* Exactly one VG name argument required. */
-#define ONE_VGNAME_ARG		0x00000010
 /* Command needs a shared lock on a VG; it only reads the VG. */
 #define LOCKD_VG_SH		0x00000020
 /* Command does not process any metadata. */
 #define NO_METADATA_PROCESSING	0x00000040
+/* Command processes only a single VG name. */
+#define ONE_VGNAME_ARG		0x00000080
  
 /* a register of the lvm commands */
 struct command {
diff --git a/tools/vgextend.c b/tools/vgextend.c
index 581c21127..bddc22f92 100644
--- a/tools/vgextend.c
+++ b/tools/vgextend.c
@@ -170,7 +170,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv)
 		return_ECMD_FAILED;
 
 	ret = process_each_vg(cmd, argc, argv,
-			      READ_FOR_UPDATE | ONE_VGNAME_ARG, handle,
+			      READ_FOR_UPDATE, handle,
 			      restoremissing ? &_vgextend_restoremissing : &_vgextend_single);
 
 	destroy_processing_handle(cmd, handle);