test: Some old systems do not have /etc/machine-id

IMHO it does not add much value running these tests on all the old systems.

WHATS_NEW

@@ -1,5 +1,7 @@
 Version 2.03.02 - 
 ===================================
+  Add pvck --dump option to extract metadata.
+  Fix signal delivery checking race in libdaemon (lvmetad).
   Add missing Before=shutdown.target to LVM2 services to fix shutdown ordering.
   Skip autoactivation for a PV when PV size does not match device size.
   Remove first-pvscan-initialization which should no longer be needed.

configure

@@ -6634,7 +6634,7 @@ $as_echo "#define _REENTRANT 1" >>confdefs.h
 
 ################################################################################
 for ac_func in ftruncate gethostname getpagesize gettimeofday localtime_r \
-  memchr memset mkdir mkfifo munmap nl_langinfo realpath rmdir setenv \
+  memchr memset mkdir mkfifo munmap nl_langinfo pselect realpath rmdir setenv \
   setlocale strcasecmp strchr strcspn strdup strerror strncasecmp strndup \
   strrchr strspn strstr strtol strtoul uname
 do :

configure.ac

@@ -153,7 +153,7 @@ AC_DEFINE([_REENTRANT], 1, [Define to use re-entrant thread safe versions])
 ################################################################################
 dnl -- Check for functions
 AC_CHECK_FUNCS([ftruncate gethostname getpagesize gettimeofday localtime_r \
-  memchr memset mkdir mkfifo munmap nl_langinfo realpath rmdir setenv \
+  memchr memset mkdir mkfifo munmap nl_langinfo pselect realpath rmdir setenv \
   setlocale strcasecmp strchr strcspn strdup strerror strncasecmp strndup \
   strrchr strspn strstr strtol strtoul uname], , [AC_MSG_ERROR(bailing out)])
 AC_FUNC_ALLOCA

daemons/lvmlockd/lvmlockd-core.c

@@ -2230,7 +2230,7 @@ static void *lockspace_thread_main(void *arg_in)
 	struct action *act_op_free = NULL;
 	struct list_head tmp_act;
 	struct list_head act_close;
-	char tmp_name[MAX_NAME+1];
+	char tmp_name[MAX_NAME+5];
 	int free_vg = 0;
 	int drop_vg = 0;
 	int error = 0;
@@ -2624,8 +2624,10 @@ out_act:
 	 * blank or fill it with garbage, but instead set it to REM:<name>
 	 * to make it easier to follow progress of freeing is via log_debug.
 	 */
-	dm_strncpy(tmp_name, ls->name, sizeof(tmp_name));
-	snprintf(ls->name, sizeof(ls->name), "REM:%s", tmp_name);
+	memset(tmp_name, 0, sizeof(tmp_name));
+	memcpy(tmp_name, "REM:", 4);
+	strncpy(tmp_name+4, ls->name, sizeof(tmp_name)-4);
+	memcpy(ls->name, tmp_name, sizeof(ls->name));
 	pthread_mutex_unlock(&lockspaces_mutex);
 
 	/* worker_thread will join this thread, and free the ls */

daemons/lvmlockd/lvmlockd-dlm.c

@@ -128,16 +128,18 @@ static int read_cluster_name(char *clustername)
 	return 0;
 }
 
+#define MAX_VERSION 16
+
 int lm_init_vg_dlm(char *ls_name, char *vg_name, uint32_t flags, char *vg_args)
 {
 	char clustername[MAX_ARGS+1];
-	char lock_args_version[MAX_ARGS+1];
+	char lock_args_version[MAX_VERSION+1];
 	int rv;
 
 	memset(clustername, 0, sizeof(clustername));
 	memset(lock_args_version, 0, sizeof(lock_args_version));
 
-	snprintf(lock_args_version, MAX_ARGS, "%u.%u.%u",
+	snprintf(lock_args_version, MAX_VERSION, "%u.%u.%u",
 		 VG_LOCK_ARGS_MAJOR, VG_LOCK_ARGS_MINOR, VG_LOCK_ARGS_PATCH);
 
 	rv = read_cluster_name(clustername);
@@ -149,7 +151,9 @@ int lm_init_vg_dlm(char *ls_name, char *vg_name, uint32_t flags, char *vg_args)
 		return -EARGS;
 	}
 
-	snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, clustername);
+	rv = snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, clustername);
+	if (rv >= MAX_ARGS)
+		log_debug("init_vg_dlm vg_args may be too long %d %s", rv, vg_args);
 	rv = 0;
 
 	log_debug("init_vg_dlm done %s vg_args %s", ls_name, vg_args);

daemons/lvmlockd/lvmlockd-sanlock.c

@@ -500,13 +500,15 @@ static int get_sizes_lockspace(char *path, int *sector_size, int *align_size)
  * version and lv name, and returns the real lock_args in vg_args.
  */
 
+#define MAX_VERSION 16
+
 int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_args)
 {
 	struct sanlk_lockspace ss;
 	struct sanlk_resourced rd;
 	struct sanlk_disk disk;
 	char lock_lv_name[MAX_ARGS+1];
-	char lock_args_version[MAX_ARGS+1];
+	char lock_args_version[MAX_VERSION+1];
 	const char *gl_name = NULL;
 	uint32_t daemon_version;
 	uint32_t daemon_proto;
@@ -526,7 +528,7 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
 		return -EARGS;
 	}
 
-	snprintf(lock_args_version, MAX_ARGS, "%u.%u.%u",
+	snprintf(lock_args_version, MAX_VERSION, "%u.%u.%u",
 		 VG_LOCK_ARGS_MAJOR, VG_LOCK_ARGS_MINOR, VG_LOCK_ARGS_PATCH);
 
 	/* see comment above about input vg_args being only lock_lv_name */
@@ -543,7 +545,9 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
 	if (daemon_test) {
 		if (!gl_lsname_sanlock[0])
 			strncpy(gl_lsname_sanlock, ls_name, MAX_NAME);
-		snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, lock_lv_name);
+		rv = snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, lock_lv_name);
+		if (rv >= MAX_ARGS)
+			log_debug("init_vg_san vg_args may be too long %d %s", rv, vg_args);
 		return 0;
 	}
 
@@ -635,7 +639,9 @@ int lm_init_vg_sanlock(char *ls_name, char *vg_name, uint32_t flags, char *vg_ar
 	if (!strcmp(gl_name, R_NAME_GL))
 		strncpy(gl_lsname_sanlock, ls_name, MAX_NAME);
  
-	snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, lock_lv_name);
+	rv = snprintf(vg_args, MAX_ARGS, "%s:%s", lock_args_version, lock_lv_name);
+	if (rv >= MAX_ARGS)
+		log_debug("init_vg_san vg_args may be too long %d %s", rv, vg_args);
 
 	log_debug("S %s init_vg_san done vg_args %s", ls_name, vg_args);
 
@@ -692,7 +698,7 @@ int lm_init_lv_sanlock(char *ls_name, char *vg_name, char *lv_name,
 {
 	struct sanlk_resourced rd;
 	char lock_lv_name[MAX_ARGS+1];
-	char lock_args_version[MAX_ARGS+1];
+	char lock_args_version[MAX_VERSION+1];
 	uint64_t offset;
 	int rv;
 
@@ -707,7 +713,7 @@ int lm_init_lv_sanlock(char *ls_name, char *vg_name, char *lv_name,
 		return rv;
 	}
 
-	snprintf(lock_args_version, MAX_ARGS, "%u.%u.%u",
+	snprintf(lock_args_version, MAX_VERSION, "%u.%u.%u",
 		 LV_LOCK_ARGS_MAJOR, LV_LOCK_ARGS_MINOR, LV_LOCK_ARGS_PATCH);
 
 	if (daemon_test) {

daemons/lvmpolld/lvmpolld-core.c

@@ -915,7 +915,7 @@ int main(int argc, char *argv[])
 	int option_index = 0;
 	int client = 0, server = 0;
 	unsigned action = ACTION_MAX;
-	struct timeval timeout;
+	struct timespec timeout;
 	daemon_idle di = { .ptimeout = &timeout };
 	struct lvmpolld_state ls = { .log_config = "" };
 	daemon_state s = {

device_mapper/libdm-config.c

@@ -51,6 +51,8 @@ struct parser {
 
 	struct dm_pool *mem;
 	int no_dup_node_check;	/* whether to disable dup node checking */
+	const char *key;        /* last obtained key */
+	unsigned ignored_creation_time;
 };
 
 struct config_output {
@@ -176,7 +178,7 @@ static int _do_dm_config_parse(struct dm_config_tree *cft, const char *start, co
 	/* TODO? if (start == end) return 1; */
 
 	struct parser *p;
-	if (!(p = dm_pool_alloc(cft->mem, sizeof(*p))))
+	if (!(p = dm_pool_zalloc(cft->mem, sizeof(*p))))
 		return_0;
 
 	p->mem = cft->mem;
@@ -615,6 +617,7 @@ static struct dm_config_node *_section(struct parser *p, struct dm_config_node *
 		match(TOK_SECTION_E);
 	} else {
 		match(TOK_EQ);
+		p->key = root->key;
 		if (!(value = _value(p)))
 			return_NULL;
 		if (root->v)
@@ -682,8 +685,17 @@ static struct dm_config_value *_type(struct parser *p)
 		errno = 0;
 		v->v.i = strtoll(p->tb, NULL, 0);	/* FIXME: check error */
 		if (errno) {
-			log_error("Failed to read int token.");
-			return NULL;
+			if (errno == ERANGE && p->key &&
+			    strcmp("creation_time", p->key) == 0) {
+				/* Due to a bug in some older 32bit builds (<2.02.169),
+				 * lvm was able to produce invalid creation_time string */
+				v->v.i = 1527120000; /* Pick 2018-05-24 day instead */
+				if (!p->ignored_creation_time++)
+					log_warn("WARNING: Invalid creation_time found in metadata (repaired with next metadata update).");
+			} else {
+				log_error("Failed to read int token.");
+				return NULL;
+			}
 		}
 		match(TOK_INT);
 		break;

include/configure.h.in

@@ -292,6 +292,12 @@
 /* Define to 1 if you have the <paths.h> header file. */
 #undef HAVE_PATHS_H
 
+/* Define to 1 if you have the `pselect' function. */
+#undef HAVE_PSELECT
+
+/* Define to 1 if you have the <pthread.h> header file. */
+#undef HAVE_PTHREAD_H
+
 /* Define to 1 if the system has the type `ptrdiff_t'. */
 #undef HAVE_PTRDIFF_T
 

lib/Makefile.in

@@ -70,7 +70,6 @@ SOURCES =\
 	metadata/lv_manip.c \
 	metadata/merge.c \
 	metadata/metadata.c \
-	metadata/read.c \
 	metadata/mirror.c \
 	metadata/pool_manip.c \
 	metadata/pv.c \

lib/cache/lvmcache.c

@@ -31,7 +31,6 @@ struct lvmcache_info {
 	struct dm_list mdas;	/* list head for metadata areas */
 	struct dm_list das;	/* list head for data areas */
 	struct dm_list bas;	/* list head for bootloader areas */
-	struct dm_list bad_mdas;/* list head for bad metadata areas */
 	struct lvmcache_vginfo *vginfo;	/* NULL == unknown */
 	struct label *label;
 	const struct format_type *fmt;
@@ -40,19 +39,12 @@ struct lvmcache_info {
 	uint32_t ext_version;   /* Extension version */
 	uint32_t ext_flags;	/* Extension flags */
 	uint32_t status;
-	int summary_seqno;      /* vg seqno found on this dev during scan */
-	int mda1_seqno;
-	int mda2_seqno;
-	unsigned summary_seqno_mismatch:1; /* two mdas on this dev has mismatching metadata */
-	unsigned mda1_bad:1;	/* label scan found bad metadata in mda1 */
-	unsigned mda2_bad:1;	/* label scan found bad metadata in mda2 */
 };
 
 /* One per VG */
 struct lvmcache_vginfo {
 	struct dm_list list;	/* Join these vginfos together */
 	struct dm_list infos;	/* List head for lvmcache_infos */
-	struct dm_list outdated_infos; /* vg_read moves info from infos to outdated_infos */
 	const struct format_type *fmt;
 	char *vgname;		/* "" == orphan */
 	uint32_t status;
@@ -74,7 +66,6 @@ static struct dm_hash_table *_vgname_hash = NULL;
 static DM_LIST_INIT(_vginfos);
 static DM_LIST_INIT(_found_duplicate_devs);
 static DM_LIST_INIT(_unused_duplicate_devs);
-static int _scanning_in_progress = 0;
 static int _vgs_locked = 0;
 static int _found_duplicate_pvs = 0;	/* If we never see a duplicate PV we can skip checking for them later. */
 static int _found_duplicate_vgnames = 0;
@@ -105,14 +96,13 @@ int lvmcache_init(struct cmd_context *cmd)
 
 void lvmcache_lock_vgname(const char *vgname, int read_only __attribute__((unused)))
 {
-	if (strcmp(vgname, VG_GLOBAL))
-		_vgs_locked++;
+	_vgs_locked++;
 }
 
 void lvmcache_unlock_vgname(const char *vgname)
 {
 	/* FIXME Do this per-VG */
-	if (strcmp(vgname, VG_GLOBAL) && !--_vgs_locked) {
+	if (!--_vgs_locked) {
 		dev_size_seqno_inc(); /* invalidate all cached dev sizes */
 	}
 }
@@ -183,51 +173,6 @@ static void _destroy_duplicate_device_list(struct dm_list *head)
 	dm_list_init(head);
 }
 
-int lvmcache_has_bad_metadata(struct device *dev)
-{
-	struct lvmcache_info *info;
-
-	if (!(info = lvmcache_info_from_pvid(dev->pvid, dev, 0))) {
-		/* shouldn't happen */
-		log_error("No lvmcache info for checking bad metadata on %s", dev_name(dev));
-		return 0;
-	}
-
-	if (info->mda1_bad || info->mda2_bad)
-		return 1;
-	return 0;
-}
-
-void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda)
-{
-	if (mda->mda_num == 1)
-		info->mda1_bad = 1;
-	else if (mda->mda_num == 2)
-		info->mda2_bad = 1;
-	dm_list_add(&info->bad_mdas, &mda->list);
-}
-
-void lvmcache_get_bad_mdas(struct cmd_context *cmd,
-			   const char *vgname, const char *vgid,
-                           struct dm_list *bad_mdas)
-{
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info;
-	struct metadata_area *mda, *mda2;
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-		log_error(INTERNAL_ERROR "lvmcache_get_bad_mdas no vginfo %s", vgname);
-		return;
-	}
-
-	dm_list_iterate_items(info, &vginfo->infos) {
-		dm_list_iterate_items_safe(mda, mda2, &info->bad_mdas) {
-			dm_list_del(&mda->list);
-			dm_list_add(bad_mdas, &mda->list);
-		}
-	}
-}
-
 static void _vginfo_attach_info(struct lvmcache_vginfo *vginfo,
 				struct lvmcache_info *info)
 {
@@ -909,12 +854,6 @@ int lvmcache_label_scan(struct cmd_context *cmd)
 
 	log_debug_cache("Finding VG info");
 
-	/* Avoid recursion when a PVID can't be found! */
-	if (_scanning_in_progress)
-		return 0;
-
-	_scanning_in_progress = 1;
-
 	/* FIXME: can this happen? */
 	if (!cmd->filter) {
 		log_error("label scan is missing filter");
@@ -995,8 +934,6 @@ int lvmcache_label_scan(struct cmd_context *cmd)
 	r = 1;
 
       out:
-	_scanning_in_progress = 0;
-
 	dm_list_iterate_items(vginfo, &_vginfos) {
 		if (is_orphan_vg(vginfo->vgname))
 			continue;
@@ -1396,7 +1333,6 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
 			return 0;
 		}
 		dm_list_init(&vginfo->infos);
-		dm_list_init(&vginfo->outdated_infos);
 
 		/*
 		 * A different VG (different uuid) can exist with the same name.
@@ -1521,9 +1457,12 @@ int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt)
 }
 
 /*
- * Returning 0 causes the caller to remove the info struct for this
- * device from lvmcache, which will make it look like a missing device.
+ * FIXME: get rid of other callers of this function which call it
+ * in odd cases to "fix up" some bit of lvmcache state.  Make those
+ * callers fix up what they need to directly, and leave this function
+ * with one purpose and caller.
  */
+
 int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vgsummary *vgsummary)
 {
 	const char *vgname = vgsummary->vgname;
@@ -1549,7 +1488,6 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	 * Puts the vginfo into the vgname hash table.
 	 */
 	if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus, vgsummary->creation_host, info->fmt)) {
-		/* shouldn't happen, internal error */
 		log_error("Failed to update VG %s info in lvmcache.", vgname);
 		return 0;
 	}
@@ -1558,7 +1496,6 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	 * Puts the vginfo into the vgid hash table.
 	 */
 	if (!_lvmcache_update_vgid(info, info->vginfo, vgid)) {
-		/* shouldn't happen, internal error */
 		log_error("Failed to update VG %s info in lvmcache.", vgname);
 		return 0;
 	}
@@ -1574,140 +1511,56 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
 	if (!vgsummary->seqno && !vgsummary->mda_size && !vgsummary->mda_checksum)
 		return 1;
 
-	/*
-	 * Keep track of which devs/mdas have old versions of the metadata.
-	 * The values we keep in vginfo are from the metadata with the largest
-	 * seqno.  One dev may have more recent metadata than another dev, and
-	 * one mda may have more recent metadata than the other mda on the same
-	 * device.
-	 *
-	 * When a device holds old metadata, the info struct for the device
-	 * remains in lvmcache, so the device is not treated as missing.
-	 * Also the mda struct containing the old metadata is kept on
-	 * info->mdas.  This means that vg_read will read metadata from
-	 * the mda again (and probably see the same old metadata).  It
-	 * also means that vg_write will use the mda to write new metadata
-	 * into the mda that currently has the old metadata.
-	 */
-	if (vgsummary->mda_num == 1)
-		info->mda1_seqno = vgsummary->seqno;
-	else if (vgsummary->mda_num == 2)
-		info->mda2_seqno = vgsummary->seqno;
-
-	if (!info->summary_seqno)
-		info->summary_seqno = vgsummary->seqno;
-	else {
-		if (info->summary_seqno == vgsummary->seqno) {
-			/* This mda has the same metadata as the prev mda on this dev. */
-			return 1;
-
-		} else if (info->summary_seqno > vgsummary->seqno) {
-			/* This mda has older metadata than the prev mda on this dev. */
-			info->summary_seqno_mismatch = 1;
-
-		} else if (info->summary_seqno < vgsummary->seqno) {
-			/* This mda has newer metadata than the prev mda on this dev. */
-			info->summary_seqno_mismatch = 1;
-			info->summary_seqno = vgsummary->seqno;
-		}
-	}
-
-	/* this shouldn't happen */
 	if (!(vginfo = info->vginfo))
 		return 1;
 
 	if (!vginfo->seqno) {
 		vginfo->seqno = vgsummary->seqno;
-		vginfo->mda_checksum = vgsummary->mda_checksum;
-		vginfo->mda_size = vgsummary->mda_size;
 
-		log_debug_cache("lvmcache %s mda%d VG %s set seqno %u checksum %x mda_size %zu",
-				dev_name(info->dev), vgsummary->mda_num, vgname,
-				vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size);
-		goto update_vginfo;
+		log_debug_cache("lvmcache %s: VG %s: set seqno to %d",
+				dev_name(info->dev), vginfo->vgname, vginfo->seqno);
 
-	} else if (vgsummary->seqno < vginfo->seqno) {
+	} else if (vgsummary->seqno != vginfo->seqno) {
+		log_warn("Scan of VG %s from %s found metadata seqno %d vs previous %d.",
+			 vgname, dev_name(info->dev), vgsummary->seqno, vginfo->seqno);
 		vginfo->scan_summary_mismatch = 1;
-
-		log_debug_cache("lvmcache %s mda%d VG %s older seqno %u checksum %x mda_size %zu",
-				dev_name(info->dev), vgsummary->mda_num, vgname,
-				vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size);
-		return 1;
-
-	} else if (vgsummary->seqno > vginfo->seqno) {
-		vginfo->scan_summary_mismatch = 1;
-
-		/* Replace vginfo values with values from newer metadata. */
-		vginfo->seqno = vgsummary->seqno;
-		vginfo->mda_checksum = vgsummary->mda_checksum;
-		vginfo->mda_size = vgsummary->mda_size;
-
-		log_debug_cache("lvmcache %s mda%d VG %s newer seqno %u checksum %x mda_size %zu",
-				dev_name(info->dev), vgsummary->mda_num, vgname,
-				vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size);
-
-		goto update_vginfo;
-	} else {
-		/*
-		 * Same seqno as previous metadata we saw for this VG.
-		 * If the metadata somehow has a different checksum or size,
-		 * even though it has the same seqno, something has gone wrong.
-		 * FIXME: test this case: VG has two PVs, first goes missing,
-		 * second updated to seqno 4, first comes back and second goes
-		 * missing, first updated to seqno 4, second comes back, now
-		 * both are present with same seqno but different checksums.
-		 */
-
-		if ((vginfo->mda_size != vgsummary->mda_size) || (vginfo->mda_checksum != vgsummary->mda_checksum)) {
-			log_warn("WARNING: scan of VG %s from %s mda%d found mda_checksum %x mda_size %zu vs %x %zu",
-				 vgname, dev_name(info->dev), vgsummary->mda_num,
-				 vgsummary->mda_checksum, vgsummary->mda_size,
-				 vginfo->mda_checksum, vginfo->mda_size);
-			vginfo->scan_summary_mismatch = 1;
-			return 0;
-		}
-
-		/*
-		 * The seqno and checksum matches what was previously seen;
-		 * the summary values have already been saved in vginfo.
-		 */
+		/* If we don't return success, this dev info will be removed from lvmcache,
+		   and then we won't be able to rescan it or repair it. */
 		return 1;
 	}
 
- update_vginfo:
+	if (!vginfo->mda_size) {
+		vginfo->mda_checksum = vgsummary->mda_checksum;
+		vginfo->mda_size = vgsummary->mda_size;
+
+		log_debug_cache("lvmcache %s: VG %s: set mda_checksum to %x mda_size to %zu",
+				dev_name(info->dev), vginfo->vgname,
+				vginfo->mda_checksum, vginfo->mda_size);
+
+	} else if ((vginfo->mda_size != vgsummary->mda_size) || (vginfo->mda_checksum != vgsummary->mda_checksum)) {
+		log_warn("Scan of VG %s from %s found mda_checksum %x mda_size %zu vs previous %x %zu",
+			 vgname, dev_name(info->dev), vgsummary->mda_checksum, vgsummary->mda_size,
+			 vginfo->mda_checksum, vginfo->mda_size);
+		vginfo->scan_summary_mismatch = 1;
+		/* If we don't return success, this dev info will be removed from lvmcache,
+		   and then we won't be able to rescan it or repair it. */
+		return 1;
+	}
+
+	/*
+	 * If a dev has an unmatching checksum, ignore the other
+	 * info from it, keeping the info we already saved.
+	 */
 	if (!_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host,
 				       vgsummary->lock_type, vgsummary->system_id)) {
-		/*
-		 * This shouldn't happen, it's an internal errror, and we can leave
-		 * the info in place without saving the summary values in vginfo.
-		 */
 		log_error("Failed to update VG %s info in lvmcache.", vgname);
+		return 0;
 	}
 
 	return 1;
 }
 
-/*
- * FIXME: quit trying to mirror changes that a command is making into lvmcache.
- *
- * First, it's complicated and hard to ensure it's done correctly in every case
- * (it would be much easier and safer to just toss out what's in lvmcache and
- * reread the info to recreate it from scratch instead of trying to make sure
- * every possible discrete state change is correct.)
- *
- * Second, it's unnecessary if commands just use the vg they are modifying
- * rather than also trying to get info from lvmcache.  The lvmcache state
- * should be populated by label_scan, used to perform vg_read's, and then
- * ignored (or dropped so it can't be used).
- *
- * lvmcache info is already used very little after a command begins its
- * operation.  The code that's supposed to keep the lvmcache in sync with
- * changes being made to disk could be half wrong and we wouldn't know it.
- * That creates a landmine for someone who might try to use a bit of it that
- * isn't being updated correctly.
- */
-
-int lvmcache_update_vg_from_write(struct volume_group *vg)
+int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
 {
 	struct pv_list *pvl;
 	struct lvmcache_info *info;
@@ -1731,110 +1584,6 @@ int lvmcache_update_vg_from_write(struct volume_group *vg)
 	return 1;
 }
 
-/*
- * The lvmcache representation of a VG after label_scan can be incorrect
- * because the label_scan does not use the full VG metadata to construct
- * vginfo/info.  PVs that don't hold VG metadata weren't attached to the vginfo
- * during label scan, and PVs with outdated metadata (claiming to be in the VG,
- * but not listed in the latest metadata) were attached to the vginfo, but
- * shouldn't be.  After vg_read() gets the full metdata in the form of a 'vg',
- * this function is called to fix up the lvmcache representation of the VG
- * using the 'vg'.
- */
-
-int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted)
-{
-	struct pv_list *pvl;
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info, *info2;
-	struct metadata_area *mda;
-	char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
-	struct lvmcache_vgsummary vgsummary = {
-		.vgname = vg->name,
-		.vgstatus = vg->status,
-		.vgid = vg->id,
-		.system_id = vg->system_id,
-		.lock_type = vg->lock_type
-	};
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vg->name, (const char *)&vg->id))) {
-		log_error(INTERNAL_ERROR "lvmcache_update_vg %s no vginfo", vg->name);
-		return 0;
-	}
-
-	/*
-	 * The label scan doesn't know when a PV with old metadata has been
-	 * removed from the VG.  Now with the vg we can tell, so remove the
-	 * info for a PV that has been removed from the VG with
-	 * vgreduce --removemissing.
-	 */
-	dm_list_iterate_items_safe(info, info2, &vginfo->infos) {
-		int found = 0;
-		dm_list_iterate_items(pvl, &vg->pvs) {
-			if (pvl->pv->dev != info->dev)
-				continue;
-			found = 1;
-			break;
-		}
-
-		if (found)
-			continue;
-
-		log_warn("WARNING: outdated PV %s seqno %u has been removed in current VG %s seqno %u.",
-			 dev_name(info->dev), info->summary_seqno, vg->name, vginfo->seqno);
-
-		_drop_vginfo(info, vginfo); /* remove from vginfo->infos */
-		dm_list_add(&vginfo->outdated_infos, &info->list);
-	}
-
-	dm_list_iterate_items(pvl, &vg->pvs) {
-		(void) dm_strncpy(pvid_s, (char *) &pvl->pv->id, sizeof(pvid_s));
-
-		if (!(info = lvmcache_info_from_pvid(pvid_s, pvl->pv->dev, 0))) {
-			log_debug_cache("lvmcache_update_vg %s no info for %s %s",
-					vg->name,
-					(char *) &pvl->pv->id,
-					pvl->pv->dev ? dev_name(pvl->pv->dev) : "missing");
-			continue;
-		}
-
-		log_debug_cache("lvmcache_update_vg %s for info %s",
-				vg->name, dev_name(info->dev));
-		
-		/*
-		 * FIXME: use a different function that just attaches info's that
-		 * had no metadata onto the correct vginfo.
-		 *
-		 * info's for PVs without metadata were not connected to the
-		 * vginfo by label_scan, so do it here.
-		 */
-		if (!lvmcache_update_vgname_and_id(info, &vgsummary)) {
-			log_debug_cache("lvmcache_update_vg %s failed to update info for %s",
-					vg->name, dev_name(info->dev));
-		}
-
-		/*
-		 * Ignored mdas were not copied from info->mdas to
-		 * fid->metadata_areas... when create_text_instance (at the
-		 * start of vg_read) called lvmcache_fid_add_mdas_vg because at
-		 * that point the info's were not connected to the vginfo
-		 * (since label_scan didn't know this without metadata.)
-		 */
-		dm_list_iterate_items(mda, &info->mdas) {
-			if (!mda_is_ignored(mda))
-				continue;
-			log_debug("lvmcache_update_vg %s copy ignored mdas for %s", vg->name, dev_name(info->dev));
-			if (!lvmcache_fid_add_mdas_pv(info, vg->fid)) {
-				log_debug_cache("lvmcache_update_vg %s failed to update mdas for %s",
-					        vg->name, dev_name(info->dev));
-			}
-			break;
-		}
-	}
-
-	return 1;
-}
-
 /*
  * We can see multiple different devices with the
  * same pvid, i.e. duplicates.
@@ -1886,7 +1635,7 @@ int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted)
  *   transient duplicate?
  */
 
-static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev, uint64_t label_sector)
+static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev)
 {
 	struct lvmcache_info *info;
 	struct label *label;
@@ -1899,9 +1648,6 @@ static struct lvmcache_info * _create_info(struct labeller *labeller, struct dev
 		return NULL;
 	}
 
-	label->dev = dev;
-	label->sector = label_sector;
-
 	info->dev = dev;
 	info->fmt = labeller->fmt;
 
@@ -1917,9 +1663,8 @@ static struct lvmcache_info * _create_info(struct labeller *labeller, struct dev
 }
 
 struct lvmcache_info *lvmcache_add(struct labeller *labeller,
-				   const char *pvid, struct device *dev, uint64_t label_sector,
-				   const char *vgname, const char *vgid, uint32_t vgstatus,
-				   int *is_duplicate)
+				   const char *pvid, struct device *dev,
+				   const char *vgname, const char *vgid, uint32_t vgstatus)
 {
 	char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
 	char uuid[64] __attribute__((aligned(8)));
@@ -1947,7 +1692,7 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller,
 		info = lvmcache_info_from_pvid(dev->pvid, NULL, 0);
 
 	if (!info) {
-		info = _create_info(labeller, dev, label_sector);
+		info = _create_info(labeller, dev);
 		created = 1;
 	}
 
@@ -1979,8 +1724,6 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller,
 
 			dm_list_add(&_found_duplicate_devs, &devl->list);
 			_found_duplicate_pvs = 1;
-			if (is_duplicate)
-				*is_duplicate = 1;
 			return NULL;
 		}
 
@@ -2124,14 +1867,6 @@ int lvmcache_fid_add_mdas_pv(struct lvmcache_info *info, struct format_instance
 	return lvmcache_fid_add_mdas(info, fid, info->dev->pvid, ID_LEN);
 }
 
-/*
- * This is the linkage where information is passed from
- * the label_scan to vg_read.
- *
- * Called by create_text_instance in vg_read to copy the
- * mda's found during label_scan and saved in info->mdas,
- * to fid->metadata_areas_in_use which is used by vg_read.
- */
 int lvmcache_fid_add_mdas_vg(struct lvmcache_vginfo *vginfo, struct format_instance *fid)
 {
 	struct lvmcache_info *info;
@@ -2205,10 +1940,6 @@ void lvmcache_del_mdas(struct lvmcache_info *info)
 	if (info->mdas.n)
 		del_mdas(&info->mdas);
 	dm_list_init(&info->mdas);
-
-	if (info->bad_mdas.n)
-		del_mdas(&info->bad_mdas);
-	dm_list_init(&info->bad_mdas);
 }
 
 void lvmcache_del_das(struct lvmcache_info *info)
@@ -2226,10 +1957,9 @@ void lvmcache_del_bas(struct lvmcache_info *info)
 }
 
 int lvmcache_add_mda(struct lvmcache_info *info, struct device *dev,
-		     uint64_t start, uint64_t size, unsigned ignored,
-		     struct metadata_area **mda_new)
+		     uint64_t start, uint64_t size, unsigned ignored)
 {
-	return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored, mda_new);
+	return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored);
 }
 
 int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size)
@@ -2572,117 +2302,33 @@ int lvmcache_vginfo_has_pvid(struct lvmcache_vginfo *vginfo, char *pvid)
 	return 0;
 }
 
-/*
- * This is used by the metadata repair command to check if
- * the metadata on a dev needs repair because it's old.
- */
-int lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev)
+struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd,
+				       const char *vgname,
+				       struct device *dev,
+				       int use_mda_num)
 {
 	struct lvmcache_vginfo *vginfo;
 	struct lvmcache_info *info;
+	struct metadata_area *mda;
 
-	/* shouldn't happen */
-	if (!vgname || !vgid)
-		return 0;
+	if (!use_mda_num)
+		use_mda_num = 1;
 
-	/* shouldn't happen */
-	if (!(vginfo = lvmcache_vginfo_from_vgid(vgid)))
-		return 0;
+	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, NULL)))
+		return NULL;
 
-	/* shouldn't happen */
-	if (!(info = lvmcache_info_from_pvid(dev->pvid, NULL, 0)))
-		return 0;
-
-	/* writing to a new PV */
-	if (!info->summary_seqno)
-		return 0;
-
-	/* on same dev, one mda has newer metadata than the other */
-	if (info->summary_seqno_mismatch)
-		return 1;
-
-	/* one or both mdas on this dev has older metadata than another dev */
-	if (vginfo->seqno > info->summary_seqno)
-		return 1;
-
-	return 0;
-}
-
-void lvmcache_get_outdated_devs(struct cmd_context *cmd,
-				const char *vgname, const char *vgid,
-				struct dm_list *devs)
-{
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info;
-	struct device_list *devl;
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-		log_error(INTERNAL_ERROR "lvmcache_get_outdated_devs no vginfo %s", vgname);
-		return;
-	}
-
-	dm_list_iterate_items(info, &vginfo->outdated_infos) {
-		if (!(devl = zalloc(sizeof(*devl))))
-			return;
-		devl->dev = info->dev;
-		dm_list_add(devs, &devl->list);
-	}
-}
-
-void lvmcache_del_outdated_devs(struct cmd_context *cmd,
-				const char *vgname, const char *vgid)
-{
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info, *info2;
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-		log_error(INTERNAL_ERROR "lvmcache_get_outdated_devs no vginfo");
-		return;
-	}
-
-	dm_list_iterate_items_safe(info, info2, &vginfo->outdated_infos)
-		lvmcache_del(info);
-}
-
-void lvmcache_get_outdated_mdas(struct cmd_context *cmd,
-				const char *vgname, const char *vgid,
-				struct device *dev,
-				struct dm_list **mdas)
-{
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info;
-
-	*mdas = NULL;
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-		log_error(INTERNAL_ERROR "lvmcache_get_outdated_mdas no vginfo");
-		return;
-	}
-
-	dm_list_iterate_items(info, &vginfo->outdated_infos) {
+	dm_list_iterate_items(info, &vginfo->infos) {
 		if (info->dev != dev)
 			continue;
-		*mdas = &info->mdas;
-		return;
+
+		dm_list_iterate_items(mda, &info->mdas) {
+			if ((use_mda_num == 1) && (mda->status & MDA_PRIMARY))
+				return mda;
+			if ((use_mda_num == 2) && !(mda->status & MDA_PRIMARY))
+				return mda;
+		}
+		return NULL;
 	}
+	return NULL;
 }
 
-int lvmcache_is_outdated_dev(struct cmd_context *cmd,
-			     const char *vgname, const char *vgid,
-			     struct device *dev)
-{
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info;
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-		log_error(INTERNAL_ERROR "lvmcache_get_outdated_mdas no vginfo");
-		return 0;
-	}
-
-	dm_list_iterate_items(info, &vginfo->outdated_infos) {
-		if (info->dev == dev)
-			return 1;
-	}
-
-	return 0;
-}

lib/cache/lvmcache.h

@@ -57,12 +57,10 @@ struct lvmcache_vgsummary {
 	char *creation_host;
 	const char *system_id;
 	const char *lock_type;
-	uint32_t seqno;
 	uint32_t mda_checksum;
 	size_t mda_size;
-	int mda_num; /* 1 = summary from mda1, 2 = summary from mda2 */
-	unsigned mda_ignored:1;
-	unsigned zero_offset:1;
+	int zero_offset;
+	int seqno;
 };
 
 int lvmcache_init(struct cmd_context *cmd);
@@ -74,9 +72,9 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const
 
 /* Add/delete a device */
 struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid,
-                                   struct device *dev, uint64_t label_sector,
-                                   const char *vgname, const char *vgid,
-                                   uint32_t vgstatus, int *is_duplicate);
+				   struct device *dev,
+				   const char *vgname, const char *vgid,
+				   uint32_t vgstatus);
 int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt);
 void lvmcache_del(struct lvmcache_info *info);
 void lvmcache_del_dev(struct device *dev);
@@ -84,8 +82,7 @@ void lvmcache_del_dev(struct device *dev);
 /* Update things */
 int lvmcache_update_vgname_and_id(struct lvmcache_info *info,
 				  struct lvmcache_vgsummary *vgsummary);
-int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted);
-int lvmcache_update_vg_from_write(struct volume_group *vg);
+int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted);
 
 void lvmcache_lock_vgname(const char *vgname, int read_only);
 void lvmcache_unlock_vgname(const char *vgname);
@@ -130,8 +127,7 @@ void lvmcache_del_mdas(struct lvmcache_info *info);
 void lvmcache_del_das(struct lvmcache_info *info);
 void lvmcache_del_bas(struct lvmcache_info *info);
 int lvmcache_add_mda(struct lvmcache_info *info, struct device *dev,
-		     uint64_t start, uint64_t size, unsigned ignored,
-		     struct metadata_area **mda_new);
+		     uint64_t start, uint64_t size, unsigned ignored);
 int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size);
 int lvmcache_add_ba(struct lvmcache_info *info, uint64_t start, uint64_t size);
 
@@ -172,6 +168,11 @@ unsigned lvmcache_mda_count(struct lvmcache_info *info);
 int lvmcache_vgid_is_cached(const char *vgid);
 uint64_t lvmcache_smallest_mda_size(struct lvmcache_info *info);
 
+struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd,
+                                      const char *vgname,
+                                      struct device *dev,
+                                      int use_mda_num);
+
 int lvmcache_found_duplicate_pvs(void);
 int lvmcache_found_duplicate_vgnames(void);
 
@@ -219,30 +220,4 @@ void lvmcache_save_metadata_size(uint64_t val);
 
 int dev_in_device_list(struct device *dev, struct dm_list *head);
 
-int lvmcache_has_bad_metadata(struct device *dev);
-
-int lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev);
-
-void lvmcache_get_outdated_devs(struct cmd_context *cmd,
-                                const char *vgname, const char *vgid,
-                                struct dm_list *devs);
-void lvmcache_get_outdated_mdas(struct cmd_context *cmd,
-                                const char *vgname, const char *vgid,
-                                struct device *dev,
-                                struct dm_list **mdas);
-
-int lvmcache_is_outdated_dev(struct cmd_context *cmd,
-                             const char *vgname, const char *vgid,
-                             struct device *dev);
-
-void lvmcache_del_outdated_devs(struct cmd_context *cmd,
-                                const char *vgname, const char *vgid);
-
-void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda);
-
-void lvmcache_get_bad_mdas(struct cmd_context *cmd,
-			   const char *vgname, const char *vgid,
-			   struct dm_list *bad_mdas);
-
-
 #endif

lib/commands/toolcontext.c

@@ -1821,7 +1821,7 @@ int refresh_toolcontext(struct cmd_context *cmd)
 	 */
 
 	activation_release();
-	hints_exit();
+	hints_exit(cmd);
 	lvmcache_destroy(cmd, 0, 0);
 	label_scan_destroy(cmd);
 	label_exit();
@@ -1941,7 +1941,7 @@ void destroy_toolcontext(struct cmd_context *cmd)
 
 	archive_exit(cmd);
 	backup_exit(cmd);
-	hints_exit();
+	hints_exit(cmd);
 	lvmcache_destroy(cmd, 0, 0);
 	label_scan_destroy(cmd);
 	label_exit();

lib/commands/toolcontext.h

@@ -160,6 +160,9 @@ struct cmd_context {
 	unsigned lockd_vg_default_sh:1;
 	unsigned lockd_vg_enforce_sh:1;
 	unsigned lockd_lv_sh_for_ex:1;
+	unsigned lockd_global_ex:1;		/* set while global lock held ex (lockd) */
+	unsigned lockf_global_ex:1;		/* set while global lock held ex (flock) */
+	unsigned nolocking:1;
 	unsigned vg_notify:1;
 	unsigned lv_notify:1;
 	unsigned pv_notify:1;
@@ -175,7 +178,6 @@ struct cmd_context {
 	unsigned use_hints:1;			/* if hints are enabled this cmd can use them */
 	unsigned pvscan_recreate_hints:1;	/* enable special case hint handling for pvscan --cache */
 	unsigned scan_lvs:1;
-	unsigned wipe_outdated_pvs:1;
 
 	/*
 	 * Devices and filtering.

lib/device/dev-md.c

@@ -88,8 +88,21 @@ static uint64_t _v1_sb_offset(uint64_t size, md_minor_version_t minor_version)
 	return sb_offset;
 }
 
+/*
+ * _udev_dev_is_md_component() only works if
+ *   external_device_info_source="udev"
+ *
+ * but
+ *
+ * udev_dev_is_md_component() in dev-type.c only works if
+ *   obtain_device_list_from_udev=1
+ *
+ * and neither of those config setting matches very well
+ * with what we're doing here.
+ */
+
 #ifdef UDEV_SYNC_SUPPORT
-static int _udev_dev_is_md(struct device *dev)
+static int _udev_dev_is_md_component(struct device *dev)
 {
 	const char *value;
 	struct dev_ext *ext;
@@ -103,7 +116,7 @@ static int _udev_dev_is_md(struct device *dev)
 	return !strcmp(value, DEV_EXT_UDEV_BLKID_TYPE_SW_RAID);
 }
 #else
-static int _udev_dev_is_md(struct device *dev)
+static int _udev_dev_is_md_component(struct device *dev)
 {
 	return 0;
 }
@@ -112,7 +125,7 @@ static int _udev_dev_is_md(struct device *dev)
 /*
  * Returns -1 on error
  */
-static int _native_dev_is_md(struct device *dev, uint64_t *offset_found, int full)
+static int _native_dev_is_md_component(struct device *dev, uint64_t *offset_found, int full)
 {
 	md_minor_version_t minor;
 	uint64_t size, sb_offset;
@@ -188,7 +201,7 @@ out:
 	return ret;
 }
 
-int dev_is_md(struct device *dev, uint64_t *offset_found, int full)
+int dev_is_md_component(struct device *dev, uint64_t *offset_found, int full)
 {
 	int ret;
 
@@ -198,19 +211,25 @@ int dev_is_md(struct device *dev, uint64_t *offset_found, int full)
 	 * information is not in udev db.
 	 */
 	if ((dev->ext.src == DEV_EXT_NONE) || offset_found) {
-		ret = _native_dev_is_md(dev, offset_found, full);
+		ret = _native_dev_is_md_component(dev, offset_found, full);
 
 		if (!full) {
 			if (!ret || (ret == -EAGAIN)) {
 				if (udev_dev_is_md_component(dev))
-					return 1;
+					ret = 1;
 			}
 		}
+		if (ret && (ret != -EAGAIN))
+			dev->flags |= DEV_IS_MD_COMPONENT;
 		return ret;
 	}
 
-	if (dev->ext.src == DEV_EXT_UDEV)
-		return _udev_dev_is_md(dev);
+	if (dev->ext.src == DEV_EXT_UDEV) {
+		ret = _udev_dev_is_md_component(dev);
+		if (ret && (ret != -EAGAIN))
+			dev->flags |= DEV_IS_MD_COMPONENT;
+		return ret;
+	}
 
 	log_error(INTERNAL_ERROR "Missing hook for MD device recognition "
 		  "using external device info source %s", dev_ext_name(dev));
@@ -439,7 +458,7 @@ int dev_is_md_with_end_superblock(struct dev_types *dt, struct device *dev)
 
 #else
 
-int dev_is_md(struct device *dev __attribute__((unused)),
+int dev_is_md_component(struct device *dev __attribute__((unused)),
 	      uint64_t *sb __attribute__((unused)))
 {
 	return 0;

lib/device/dev-type.c

@@ -866,7 +866,7 @@ static int _wipe_known_signatures_with_lvm(struct device *dev, const char *name,
 		wiped = &wiped_tmp;
 	*wiped = 0;
 
-	if (!_wipe_signature(dev, "software RAID md superblock", name, 4, yes, force, wiped, dev_is_md) ||
+	if (!_wipe_signature(dev, "software RAID md superblock", name, 4, yes, force, wiped, dev_is_md_component) ||
 	    !_wipe_signature(dev, "swap signature", name, 10, yes, force, wiped, dev_is_swap) ||
 	    !_wipe_signature(dev, "LUKS signature", name, 8, yes, force, wiped, dev_is_luks))
 		return 0;
@@ -1180,6 +1180,7 @@ int udev_dev_is_md_component(struct device *dev)
 	if (value && !strcmp(value, DEV_EXT_UDEV_BLKID_TYPE_SW_RAID)) {
 		log_debug("Device %s is md raid component based on blkid variable in udev db (%s=\"%s\").",
 			   dev_name(dev), DEV_EXT_UDEV_BLKID_TYPE, value);
+		dev->flags |= DEV_IS_MD_COMPONENT;
 		ret = 1;
 		goto out;
 	}

lib/device/dev-type.h

@@ -57,7 +57,7 @@ const char *dev_subsystem_name(struct dev_types *dt, struct device *dev);
 int major_is_scsi_device(struct dev_types *dt, int major);
 
 /* Signature/superblock recognition with position returned where found. */
-int dev_is_md(struct device *dev, uint64_t *sb, int full);
+int dev_is_md_component(struct device *dev, uint64_t *sb, int full);
 int dev_is_swap(struct device *dev, uint64_t *signature, int full);
 int dev_is_luks(struct device *dev, uint64_t *signature, int full);
 int dasd_is_cdl_formatted(struct device *dev);

lib/device/device.h

@@ -36,6 +36,7 @@
 #define DEV_FILTER_OUT_SCAN	0x00004000	/* filtered out during label scan */
 #define DEV_BCACHE_WRITE	0x00008000      /* bcache_fd is open with RDWR */
 #define DEV_SCAN_FOUND_LABEL	0x00010000      /* label scan read dev and found label */
+#define DEV_IS_MD_COMPONENT	0x00020000	/* device is an md component */
 
 /*
  * Support for external device info.

lib/filters/filter-md.c

@@ -93,7 +93,7 @@ static int _passes_md_filter(struct cmd_context *cmd, struct dev_filter *f __att
 	if (!md_filtering())
 		return 1;
 
-	ret = dev_is_md(dev, NULL, cmd->use_full_md_check);
+	ret = dev_is_md_component(dev, NULL, cmd->use_full_md_check);
 
 	if (ret == -EAGAIN) {
 		/* let pass, call again after scan */

lib/format_text/format-text.c

@@ -166,7 +166,6 @@ static int _pv_analyze_mda_raw (const struct format_type * fmt,
 	char *buf=NULL;
 	struct device_area *area;
 	struct mda_context *mdac;
-	uint32_t bad_fields = 0;
 	int r=0;
 
 	mdac = (struct mda_context *) mda->metadata_locn;
@@ -175,7 +174,7 @@ static int _pv_analyze_mda_raw (const struct format_type * fmt,
 		  FMTu64, mdac->area.start, mdac->area.size);
 	area = &mdac->area;
 
-	if (!(mdah = raw_read_mda_header(fmt, area, mda_is_primary(mda), 0, &bad_fields)))
+	if (!(mdah = raw_read_mda_header(fmt, area, mda_is_primary(mda))))
 		goto_out;
 
 	rlocn = mdah->raw_locns;
@@ -313,88 +312,61 @@ static void _xlate_mdah(struct mda_header *mdah)
 	}
 }
 
-static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev_area,
-				int primary_mda, uint32_t ignore_bad_fields, uint32_t *bad_fields)
+static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev_area, int primary_mda)
 {
-	int bad = 0;
-
 	log_debug_metadata("Reading mda header sector from %s at %llu",
 			   dev_name(dev_area->dev), (unsigned long long)dev_area->start);
 
 	if (!dev_read_bytes(dev_area->dev, dev_area->start, MDA_HEADER_SIZE, mdah)) {
 		log_error("Failed to read metadata area header on %s at %llu",
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		*bad_fields |= BAD_MDA_READ;
 		return 0;
 	}
 
 	if (mdah->checksum_xl != xlate32(calc_crc(INITIAL_CRC, (uint8_t *)mdah->magic,
 						  MDA_HEADER_SIZE -
 						  sizeof(mdah->checksum_xl)))) {
-		log_warn("WARNING: wrong checksum %x in mda header on %s at %llu",
-			  mdah->checksum_xl,
+		log_error("Incorrect checksum in metadata area header on %s at %llu",
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-
-		if (!(ignore_bad_fields & BAD_MDA_CHECKSUM)) {
-			*bad_fields |= BAD_MDA_CHECKSUM;
-			bad = 1;
-		}
+		return 0;
 	}
 
 	_xlate_mdah(mdah);
 
-	if (strncmp((char *)mdah->magic, FMTT_MAGIC, sizeof(mdah->magic))) {
-		log_warn("WARNING: wrong magic number %.8s in mda header on %s at %llu",
-			  mdah->magic,
+	if (memcmp(mdah->magic, FMTT_MAGIC, sizeof(mdah->magic))) {
+		log_error("Wrong magic number in metadata area header on %s at %llu",
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-
-		if (!(ignore_bad_fields & BAD_MDA_MAGIC)) {
-			*bad_fields |= BAD_MDA_MAGIC;
-			bad = 1;
-		}
+		return 0;
 	}
 
 	if (mdah->version != FMTT_VERSION) {
-		log_warn("WARNING: wrong version %u in mda header on %s at %llu",
+		log_error("Incompatible version %u metadata area header on %s at %llu",
 			  mdah->version,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-
-		if (!(ignore_bad_fields & BAD_MDA_VERSION)) {
-			*bad_fields |= BAD_MDA_VERSION;
-			bad = 1;
-		}
+		return 0;
 	}
 
 	if (mdah->start != dev_area->start) {
-		log_warn("WARNING: wrong start sector %llu in mda header on %s at %llu",
+		log_error("Incorrect start sector %llu in metadata area header on %s at %llu",
 			  (unsigned long long)mdah->start,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-
-		if (!(ignore_bad_fields & BAD_MDA_START)) {
-			*bad_fields |= BAD_MDA_START;
-			bad = 1;
-		}
-	}
-
-	if (bad)
 		return 0;
+	}
 
 	return 1;
 }
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area,
-				       int primary_mda, uint32_t ignore_bad_fields, uint32_t *bad_fields)
+				       struct device_area *dev_area, int primary_mda)
 {
 	struct mda_header *mdah;
 
 	if (!(mdah = dm_pool_alloc(fmt->cmd->mem, MDA_HEADER_SIZE))) {
 		log_error("struct mda_header allocation failed");
-		*bad_fields |= BAD_MDA_INTERNAL;
 		return NULL;
 	}
 
-	if (!_raw_read_mda_header(mdah, dev_area, primary_mda, ignore_bad_fields, bad_fields)) {
+	if (!_raw_read_mda_header(mdah, dev_area, primary_mda)) {
 		dm_pool_free(fmt->cmd->mem, mdah);
 		return NULL;
 	}
@@ -406,7 +378,7 @@ static int _raw_write_mda_header(const struct format_type *fmt,
 				 struct device *dev, int primary_mda,
 				 uint64_t start_byte, struct mda_header *mdah)
 {
-	strncpy((char *)mdah->magic, FMTT_MAGIC, sizeof(mdah->magic));
+	memcpy(mdah->magic, FMTT_MAGIC, sizeof(mdah->magic));
 	mdah->version = FMTT_VERSION;
 	mdah->start = start_byte;
 
@@ -592,9 +564,8 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid,
 	time_t when;
 	char *desc;
 	uint32_t wrap = 0;
-	uint32_t bad_fields = 0;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, area, primary_mda, 0, &bad_fields))) {
+	if (!(mdah = raw_read_mda_header(fid->fmt, area, primary_mda))) {
 		log_error("Failed to read vg %s from %s", vgname, dev_name(area->dev));
 		goto out;
 	}
@@ -715,7 +686,6 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
 	uint64_t old_start = 0, old_last = 0, old_size = 0, old_wrap = 0;
 	uint64_t new_start = 0, new_last = 0, new_size = 0, new_wrap = 0;
 	uint64_t max_size;
-	uint32_t bad_fields = 0;
 	char *new_buf = NULL;
 	int overlap;
 	int found = 0;
@@ -731,7 +701,7 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg,
 	if (!found)
 		return 1;
 
-	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda), mda->ignore_bad_fields, &bad_fields)))
+	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda))))
 		goto_out;
 
 	/*
@@ -1002,7 +972,6 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
 	struct raw_locn *rlocn_slot1;
 	struct raw_locn *rlocn_new;
 	struct pv_list *pvl;
-	uint32_t bad_fields = 0;
 	int r = 0;
 	int found = 0;
 
@@ -1023,7 +992,7 @@ static int _vg_commit_raw_rlocn(struct format_instance *fid,
 	 * mdah buffer, but the mdah buffer is not modified and mdac->rlocn is
 	 * modified.
 	 */
-	if (!(mdab = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda), mda->ignore_bad_fields, &bad_fields)))
+	if (!(mdab = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda))))
 		goto_out;
 
 	/*
@@ -1215,7 +1184,6 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
 	struct mda_header *mdah;
 	struct raw_locn *rlocn_slot0;
 	struct raw_locn *rlocn_slot1;
-	uint32_t bad_fields = 0;
 	int r = 0;
 
 	if (!(mdah = dm_pool_alloc(fid->fmt->cmd->mem, MDA_HEADER_SIZE))) {
@@ -1229,7 +1197,7 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg,
 	 * Just to print the warning?
 	 */
 
-	if (!_raw_read_mda_header(mdah, &mdac->area, mda_is_primary(mda), 0, &bad_fields))
+	if (!_raw_read_mda_header(mdah, &mdac->area, mda_is_primary(mda)))
 		log_warn("WARNING: Removing metadata location on %s with bad mda header.",
 			  dev_name(mdac->area.dev));
 
@@ -1526,7 +1494,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 	 * valid vg name.
 	 */
 	if (!validate_name(namebuf)) {
-		log_warn("WARNING: Metadata location on %s at %llu begins with invalid VG name.",
+		log_error("Metadata location on %s at %llu begins with invalid VG name.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1592,7 +1560,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 				(off_t) (dev_area->start + MDA_HEADER_SIZE),
 				wrap, calc_crc, vgsummary->vgname ? 1 : 0,
 				vgsummary)) {
-		log_warn("WARNING: metadata on %s at %llu has invalid summary for VG.",
+		log_error("Metadata location on %s at %llu has invalid summary for VG.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1600,7 +1568,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 
 	/* Ignore this entry if the characters aren't permissible */
 	if (!validate_name(vgsummary->vgname)) {
-		log_warn("WARNING: metadata on %s at %llu has invalid VG name.",
+		log_error("Metadata location on %s at %llu has invalid VG name.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1682,12 +1650,13 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume
 
 	/* Add a new cache entry with PV info or update existing one. */
 	if (!(info = lvmcache_add(fmt->labeller, (const char *) &pv->id,
-				  pv->dev,  pv->label_sector, pv->vg_name,
-				  is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0, NULL)))
+				  pv->dev, pv->vg_name,
+				  is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0)))
 		return_0;
 
-	/* lvmcache_add() creates info and info->label structs for the dev, get info->label. */
 	label = lvmcache_get_label(info);
+	label->sector = pv->label_sector;
+	label->dev = pv->dev;
 
 	lvmcache_update_pv(info, pv, fmt);
 
@@ -1715,7 +1684,7 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume
 		// if fmt is not the same as info->fmt we are in trouble
 		if (!lvmcache_add_mda(info, mdac->area.dev,
 				      mdac->area.start, mdac->area.size,
-				      mda_is_ignored(mda), NULL))
+				      mda_is_ignored(mda)))
 			return_0;
 	}
 
@@ -1769,16 +1738,12 @@ static int _text_pv_needs_rewrite(const struct format_type *fmt, struct physical
 {
 	struct lvmcache_info *info;
 	uint32_t ext_vsn;
-	uint32_t ext_flags;
 
 	*needs_rewrite = 0;
 
 	if (!pv->is_labelled)
 		return 1;
 
-	if (!pv->dev)
-		return 1;
-
 	if (!(info = lvmcache_info_from_pvid((const char *)&pv->id, pv->dev, 0))) {
 		log_error("Failed to find cached info for PV %s.", pv_dev_name(pv));
 		return 0;
@@ -1786,16 +1751,8 @@ static int _text_pv_needs_rewrite(const struct format_type *fmt, struct physical
 
 	ext_vsn = lvmcache_ext_version(info);
 
-	if (ext_vsn < PV_HEADER_EXTENSION_VSN) {
-		log_debug("PV %s header needs rewrite for new ext version", dev_name(pv->dev));
+	if (ext_vsn < PV_HEADER_EXTENSION_VSN)
 		*needs_rewrite = 1;
-	}
-
-	ext_flags = lvmcache_ext_flags(info);
-	if (!(ext_flags & PV_EXT_USED)) {
-		log_debug("PV %s header needs rewrite to set ext used", dev_name(pv->dev));
-		*needs_rewrite = 1;
-	}
 
 	return 1;
 }
@@ -2646,36 +2603,220 @@ bad:
 	return NULL;
 }
 
-int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev,
-			      struct metadata_area *mda)
+static char *_read_metadata_text(struct cmd_context *cmd, struct device *dev,
+				 uint64_t area_start, uint64_t area_size,
+				 uint32_t *len, uint64_t *disk_offset)
 {
-	struct mda_context *mdac = mda->metadata_locn;
-	uint64_t start_byte = mdac->area.start;
-	struct mda_header *mdab;
+	struct mda_header *mh;
 	struct raw_locn *rlocn_slot0;
-	struct raw_locn *rlocn_slot1;
-	uint32_t bad_fields = 0;
+	uint64_t text_offset, text_size;
+	char *area_buf;
+	char *text_buf;
 
-	if (!(mdab = raw_read_mda_header(cmd->fmt, &mdac->area, mda_is_primary(mda), 0, &bad_fields))) {
-		log_error("Failed to read outdated pv mda header on %s", dev_name(dev));
-		return 0;
+	/*
+	 * Read the entire metadata area, including mda_header and entire
+	 * circular buffer.
+	 */
+	if (!(area_buf = malloc(area_size)))
+		return_NULL;
+
+	if (!dev_read_bytes(dev, area_start, area_size, area_buf)) {
+		log_error("Failed to read device %s at %llu size %llu",
+			  dev_name(dev),
+			  (unsigned long long)area_start,
+			  (unsigned long long)area_size);
+		return NULL;
 	}
 
-	rlocn_slot0 = &mdab->raw_locns[0];
-	rlocn_slot1 = &mdab->raw_locns[1];
+	mh = (struct mda_header *)area_buf;
+	_xlate_mdah(mh);
 
-	rlocn_slot0->offset = 0;
-	rlocn_slot0->size = 0;
-	rlocn_slot0->checksum = 0;
-	rlocn_slot1->offset = 0;
-	rlocn_slot1->size = 0;
-	rlocn_slot1->checksum = 0;
-         
-	if (!_raw_write_mda_header(cmd->fmt, dev, mda_is_primary(mda), start_byte, mdab)) {
-		log_error("Failed to write outdated pv mda header on %s", dev_name(dev));
-		return 0;
-	}
+	rlocn_slot0 = &mh->raw_locns[0];
+	text_offset = rlocn_slot0->offset;
+	text_size = rlocn_slot0->size;
 
-	return 1;
+	/*
+	 * Copy and return the current metadata text out of the metadata area.
+	 */
+
+	if (!(text_buf = malloc(text_size)))
+		return_NULL;
+
+	memcpy(text_buf, area_buf + text_offset, text_size);
+
+	if (len)
+		*len = (uint32_t)text_size;
+	if (disk_offset)
+		*disk_offset = area_start + text_offset;
+
+	free(area_buf);
+
+	return text_buf;
 }
 
+int dump_metadata_text(struct cmd_context *cmd,
+		       const char *vgname,
+		       const char *vgid,
+		       struct device *dev,
+		       struct metadata_area *mda,
+		       const char *tofile)
+{
+	char *textbuf;
+	struct format_instance *fid;
+	struct format_instance_ctx fic;
+	struct mda_context *mdac;
+	struct volume_group *vg;
+	unsigned use_previous_vg = 0;
+	uint32_t textlen = 0;
+	uint32_t textcrc;
+	uint64_t text_disk_offset;
+	int ret = 0;
+
+	/*
+	 * Set up overhead/abstractions for reading a given vgname
+	 * (fmt/fid/fic/vgid).
+	 */
+
+	fic.type = FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
+	fic.context.vg_ref.vg_name = vgname;
+	fic.context.vg_ref.vg_id = vgid;
+
+	if (!(fid = _text_create_text_instance(cmd->fmt, &fic))) {
+		log_error("Failed to create format instance");
+		return 0;
+	}
+
+	mdac = mda->metadata_locn;
+
+	/*
+	 * Read the VG metadata from the device as a raw chunk of original text.
+	 */
+	textbuf = _read_metadata_text(cmd, dev,
+				      mdac->area.start, mdac->area.size,
+				      &textlen, &text_disk_offset);
+	if (!textbuf || !textlen) {
+		log_error("No metadata text found on %s", dev_name(dev));
+		_text_destroy_instance(fid);
+		return 0;
+	}
+
+	textcrc = calc_crc(INITIAL_CRC, (uint8_t *)textbuf, textlen);
+
+	/*
+	 * Read the same VG metadata, but imported/parsed into a vg struct
+	 * format so we know it's valid/parsable, and can look at values in it.
+	 */
+	if (!(vg = _vg_read_raw(fid, vgname, mda, NULL, &use_previous_vg))) {
+		log_warn("WARNING: parse error for metadata on %s.", dev_name(dev));
+		_text_destroy_instance(fid);
+	}
+
+	log_print("Metadata for %s from %s at %llu size %u with seqno %u checksum 0x%x.",
+		  vgname, dev_name(dev),
+		  (unsigned long long)text_disk_offset, textlen,
+		  vg ? vg->seqno : 0, textcrc);
+
+	if (!tofile) {
+		log_print("---");
+		printf("%s\n", textbuf);
+		log_print("---");
+	} else {
+		FILE *fp;
+		if (!(fp = fopen(tofile, "wx"))) {
+			log_error("Failed to create file %s", tofile);
+			goto out;
+		}
+
+		fprintf(fp, "%s", textbuf);
+
+		if (fflush(fp))
+			stack;
+		if (fclose(fp))
+			stack;
+	}
+
+	if (vg)
+		release_vg(vg);
+
+	free(textbuf);
+	ret = 1;
+out:
+	return ret;
+}
+
+static char *_read_metadata_area(struct cmd_context *cmd, struct device *dev,
+				 uint64_t area_start, uint64_t area_size)
+{
+	char *area_buf;
+
+	/*
+	 * Read the entire metadata area, including mda_header and entire
+	 * circular buffer.
+	 */
+	if (!(area_buf = malloc(area_size)))
+		return_NULL;
+
+	if (!dev_read_bytes(dev, area_start, area_size, area_buf)) {
+		log_error("Failed to read device %s at %llu size %llu",
+			  dev_name(dev),
+			  (unsigned long long)area_start,
+			  (unsigned long long)area_size);
+		return NULL;
+	}
+
+	return area_buf;
+}
+
+int dump_metadata_area(struct cmd_context *cmd,
+		       const char *vgname,
+		       const char *vgid,
+		       struct device *dev,
+		       struct metadata_area *mda,
+		       const char *tofile)
+{
+	char *areabuf;
+	char *textbuf;
+	struct mda_context *mdac;
+	int ret = 0;
+
+	mdac = mda->metadata_locn;
+
+	areabuf = _read_metadata_area(cmd, dev,
+				      mdac->area.start, mdac->area.size);
+	if (!areabuf) {
+		log_error("No metadata area found on %s", dev_name(dev));
+		return 0;
+	}
+
+	log_print("Metadata buffer for %s from %s in area at %llu size %llu offset 512.",
+		  vgname, dev_name(dev),
+		  (unsigned long long)mdac->area.start,
+		  (unsigned long long)mdac->area.size);
+
+	/* text starts after mda_header which uses 512 bytes */
+	textbuf = areabuf + 512;
+
+	if (!tofile) {
+		/* N.B. this will often include unprintable data */
+		log_print("---");
+		fwrite(textbuf, mdac->area.size - 512, 1, stdout);
+		log_print("---");
+	} else {
+		FILE *fp;
+		if (!(fp = fopen(tofile, "wx"))) {
+			log_error("Failed to create file %s", tofile);
+			goto out;
+		}
+
+		fwrite(textbuf, mdac->area.size - 512, 1, fp);
+
+		if (fflush(fp))
+			stack;
+		if (fclose(fp))
+			stack;
+	}
+	ret = 1;
+out:
+	free(areabuf);
+	return ret;
+}

lib/format_text/format-text.h

@@ -61,8 +61,7 @@ int add_ba(struct dm_pool *mem, struct dm_list *eas,
 	   uint64_t start, uint64_t size);
 void del_bas(struct dm_list *bas);
 int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *mdas,
-	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored,
-	    struct metadata_area **mda_new);
+	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored);
 void del_mdas(struct dm_list *mdas);
 
 /* On disk */
@@ -77,7 +76,18 @@ struct data_area_list {
 	struct disk_locn disk_locn;
 };
 
-int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev,
-                              struct metadata_area *mda);
+int dump_metadata_text(struct cmd_context *cmd,
+                       const char *vgname,
+                       const char *vgid,
+                       struct device *dev,
+                       struct metadata_area *mda,
+                       const char *tofile);
+
+int dump_metadata_area(struct cmd_context *cmd,
+                       const char *vgname,
+                       const char *vgid,
+                       struct device *dev,
+                       struct metadata_area *mda,
+                       const char *tofile);
 
 #endif

lib/format_text/import.c

@@ -61,13 +61,13 @@ int text_read_metadata_summary(const struct format_type *fmt,
 					 offset2, size2, checksum_fn,
 					 vgsummary->mda_checksum,
 					 checksum_only, 1)) {
-			log_warn("WARNING: invalid metadata text from %s at %llu.",
-				 dev_name(dev), (unsigned long long)offset);
+			/* FIXME: handle errors */
+			log_error("Couldn't read volume group metadata from %s.", dev_name(dev));
 			goto out;
 		}
 	} else {
 		if (!config_file_read(cft)) {
-			log_warn("WARNING: invalid metadata text from file.");
+			log_error("Couldn't read volume group metadata from file.");
 			goto out;
 		}
 	}

lib/format_text/layout.h

@@ -81,9 +81,7 @@ struct mda_header {
 } __attribute__ ((packed));
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area, int primary_mda,
-				       uint32_t ignore_bad_fields,
-				       uint32_t *bad_fields);
+				       struct device_area *dev_area, int primary_mda);
 
 struct mda_lists {
 	struct metadata_area_ops *file_ops;

lib/format_text/text_label.c

@@ -30,7 +30,7 @@ static int _text_can_handle(struct labeller *l __attribute__((unused)),
 {
 	struct label_header *lh = (struct label_header *) buf;
 
-	if (!strncmp((char *)lh->type, LVM2_LABEL, sizeof(lh->type)))
+	if (!memcmp(lh->type, LVM2_LABEL, sizeof(lh->type)))
 		return 1;
 
 	return 0;
@@ -95,9 +95,9 @@ static int _text_write(struct label *label, void *buf)
 	 * PV header base
 	 */
 	/* FIXME Move to where label is created */
-	strncpy(label->type, LVM2_LABEL, sizeof(label->type));
+	memcpy(label->type, LVM2_LABEL, sizeof(label->type));
 
-	strncpy((char *)lh->type, label->type, sizeof(label->type));
+	memcpy(lh->type, LVM2_LABEL, sizeof(lh->type));
 
 	pvhdr = (struct pv_header *) ((char *) buf + xlate32(lh->offset_xl));
 	info = (struct lvmcache_info *) label->info;
@@ -241,10 +241,11 @@ void del_bas(struct dm_list *bas)
 	del_das(bas);
 }
 
+/* FIXME: refactor this function with other mda constructor code */
 int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *mdas,
-	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored,
-	    struct metadata_area **mda_new)
+	    struct device *dev, uint64_t start, uint64_t size, unsigned ignored)
 {
+/* FIXME List size restricted by pv_header SECTOR_SIZE */
 	struct metadata_area *mdal, *mda;
 	struct mda_lists *mda_lists = (struct mda_lists *) fmt->private;
 	struct mda_context *mdac, *mdac2;
@@ -294,8 +295,6 @@ int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *
 	mda_set_ignored(mdal, ignored);
 
 	dm_list_add(mdas, &mdal->list);
-	if (mda_new)
-		*mda_new = mdal;
 	return 1;
 }
 
@@ -315,108 +314,83 @@ void del_mdas(struct dm_list *mdas)
 static int _text_initialise_label(struct labeller *l __attribute__((unused)),
 				  struct label *label)
 {
-	strncpy(label->type, LVM2_LABEL, sizeof(label->type));
+	memcpy(label->type, LVM2_LABEL, sizeof(label->type));
 
 	return 1;
 }
 
-static int _read_mda_header_and_metadata(const struct format_type *fmt,
-					 struct metadata_area *mda,
-					 struct lvmcache_vgsummary *vgsummary,
-					 uint32_t *bad_fields)
+struct _update_mda_baton {
+	struct lvmcache_info *info;
+	struct label *label;
+};
+
+static int _read_mda_header_and_metadata(struct metadata_area *mda, void *baton)
 {
+	struct _update_mda_baton *p = baton;
+	const struct format_type *fmt = p->label->labeller->fmt;
 	struct mda_context *mdac = (struct mda_context *) mda->metadata_locn;
 	struct mda_header *mdah;
+	struct lvmcache_vgsummary vgsummary = { 0 };
 
-	if (!(mdah = raw_read_mda_header(fmt, &mdac->area, (mda->mda_num == 1), 0, bad_fields))) {
-		log_warn("WARNING: bad metadata header on %s at %llu.",
-			 dev_name(mdac->area.dev),
-			 (unsigned long long)mdac->area.start);
-		if (mda)
-			mda->header_start = mdac->area.start;
-		*bad_fields |= BAD_MDA_HEADER;
-		return 0;
+	if (!(mdah = raw_read_mda_header(fmt, &mdac->area, mda_is_primary(mda)))) {
+		log_error("Failed to read mda header from %s", dev_name(mdac->area.dev));
+		goto fail;
 	}
 
-	if (mda)
-		mda->header_start = mdah->start;
-
 	mda_set_ignored(mda, rlocn_is_ignored(mdah->raw_locns));
 
 	if (mda_is_ignored(mda)) {
 		log_debug_metadata("Ignoring mda on device %s at offset " FMTu64,
 				   dev_name(mdac->area.dev),
 				   mdac->area.start);
-		vgsummary->mda_ignored = 1;
 		return 1;
 	}
 
 	if (!read_metadata_location_summary(fmt, mdah, mda_is_primary(mda), &mdac->area,
-					    vgsummary, &mdac->free_sectors)) {
-		if (vgsummary->zero_offset)
+					    &vgsummary, &mdac->free_sectors)) {
+		if (vgsummary.zero_offset)
 			return 1;
 
-		log_warn("WARNING: bad metadata text on %s in mda%d",
-			 dev_name(mdac->area.dev), mda->mda_num);
-		*bad_fields |= BAD_MDA_TEXT;
-		return 0;
+		log_error("Failed to read metadata summary from %s", dev_name(mdac->area.dev));
+		goto fail;
+	}
+
+	if (!lvmcache_update_vgname_and_id(p->info, &vgsummary)) {
+		log_error("Failed to save lvm summary for %s", dev_name(mdac->area.dev));
+		goto fail;
 	}
 
 	return 1;
+
+fail:
+	lvmcache_del(p->info);
+	return 0;
 }
 
-/*
- * Used by label_scan to get a summary of the VG that exists on this PV.  This
- * summary is stored in lvmcache vginfo/info/info->mdas and is used later by
- * vg_read which needs to know which PVs to read for a given VG name, and where
- * the metadata is at for those PVs.
- */
-
-static int _text_read(struct labeller *labeller, struct device *dev, void *label_buf,
-		      uint64_t label_sector, int *is_duplicate)
+static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
+		      struct label **label)
 {
-	struct lvmcache_vgsummary vgsummary;
-	struct lvmcache_info *info;
-	const struct format_type *fmt = labeller->fmt;
 	struct label_header *lh = (struct label_header *) label_buf;
 	struct pv_header *pvhdr;
 	struct pv_header_extension *pvhdr_ext;
-	struct metadata_area *mda;
-	struct metadata_area *mda1 = NULL;
-	struct metadata_area *mda2 = NULL;
+	struct lvmcache_info *info;
 	struct disk_locn *dlocn_xl;
 	uint64_t offset;
 	uint32_t ext_version;
-	uint32_t bad_fields;
-	int mda_count = 0;
-	int good_mda_count = 0;
-	int bad_mda_count = 0;
-	int rv1, rv2;
+	struct _update_mda_baton baton;
 
 	/*
 	 * PV header base
 	 */
 	pvhdr = (struct pv_header *) ((char *) label_buf + xlate32(lh->offset_xl));
 
-	/*
-	 * FIXME: stop adding the device to lvmcache initially as an orphan
-	 * (and then moving it later) and instead just add it when we know the
-	 * VG.
-	 *
-	 * If another device with this same PVID has already been seen,
-	 * lvmcache_add will put this device in the duplicates list in lvmcache
-	 * and return NULL.  At the end of label_scan, the duplicate devs are
-	 * compared, and if another dev is preferred for this PV, then the
-	 * existing dev is removed from lvmcache and _text_read is called again
-	 * for this dev, and lvmcache_add will add it.
-	 *
-	 * Other reasons for lvmcache_add to return NULL are internal errors.
-	 */
-	if (!(info = lvmcache_add(labeller, (char *)pvhdr->pv_uuid, dev, label_sector,
+	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev,
 				  FMT_TEXT_ORPHAN_VG_NAME,
-				  FMT_TEXT_ORPHAN_VG_NAME, 0, is_duplicate)))
+				  FMT_TEXT_ORPHAN_VG_NAME, 0)))
 		return_0;
 
+	*label = lvmcache_get_label(info);
+
 	lvmcache_set_device_size(info, xlate64(pvhdr->device_size_xl));
 
 	lvmcache_del_das(info);
@@ -430,27 +404,11 @@ static int _text_read(struct labeller *labeller, struct device *dev, void *label
 		dlocn_xl++;
 	}
 
+	/* Metadata area headers */
 	dlocn_xl++;
-
-	/* Metadata areas */
 	while ((offset = xlate64(dlocn_xl->offset))) {
-
-		/*
-		 * This just calls add_mda() above, replacing info with info->mdas.
-		 */
-		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0, &mda);
-
+		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0);
 		dlocn_xl++;
-		mda_count++;
-
-		if (mda_count == 1) {
-			mda1 = mda;
-			mda1->mda_num = 1;
-		}
-		else if (mda_count == 2) {
-			mda2 = mda;
-			mda2->mda_num = 2;
-		}
 	}
 
 	dlocn_xl++;
@@ -460,7 +418,7 @@ static int _text_read(struct labeller *labeller, struct device *dev, void *label
 	 */
 	pvhdr_ext = (struct pv_header_extension *) ((char *) dlocn_xl);
 	if (!(ext_version = xlate32(pvhdr_ext->version)))
-		goto scan_mdas;
+		goto out;
 
 	log_debug_metadata("%s: PV header extension version " FMTu32 " found",
 			   dev_name(dev), ext_version);
@@ -477,117 +435,22 @@ static int _text_read(struct labeller *labeller, struct device *dev, void *label
 		lvmcache_add_ba(info, offset, xlate64(dlocn_xl->size));
 		dlocn_xl++;
 	}
-
- scan_mdas:
-	if (!mda_count) {
-		log_debug_metadata("Scanning %s found no mdas.", dev_name(dev));
-		return 1;
-	}
+out:
+	baton.info = info;
+	baton.label = *label;
 
 	/*
-	 * Track which devs have bad metadata so repair can find them (even if
-	 * this dev also has good metadata that we are able to use).
+	 * In the vg_read phase, we compare all mdas and decide which to use
+	 * which are bad and need repair.
 	 *
-	 * When bad metadata is seen, the unusable mda struct is removed from
-	 * lvmcache info->mdas.  This means that vg_read and vg_write will skip
-	 * the bad mda not try to read or write the bad metadata.  The bad mdas
-	 * are saved in a separate bad_mdas list in lvmcache so that repair can
-	 * find them to repair.
+	 * FIXME: this quits if the first mda is bad, but we need something
+	 * smarter to be able to use the second mda if it's good.
 	 */
-
-	if (mda1) {
-		log_debug_metadata("Scanning %s mda1 summary.", dev_name(dev));
-		memset(&vgsummary, 0, sizeof(vgsummary));
-		bad_fields = 0;
-		vgsummary.mda_num = 1;
-
-		rv1 = _read_mda_header_and_metadata(fmt, mda1, &vgsummary, &bad_fields);
-
-		if (rv1 && !vgsummary.zero_offset && !vgsummary.mda_ignored) {
-			if (!lvmcache_update_vgname_and_id(info, &vgsummary)) {
-				/* I believe this is only an internal error. */
-				log_warn("WARNING: Scanning %s mda1 failed to save internal summary.", dev_name(dev));
-
-				dm_list_del(&mda1->list);
-				bad_fields |= BAD_MDA_INTERNAL;
-				mda1->bad_fields = bad_fields;
-				lvmcache_save_bad_mda(info, mda1);
-				mda1 = NULL;
-				bad_mda_count++;
-			} else {
-				/* The normal success path */
-				log_debug("Scanned %s mda1 seqno %u", dev_name(dev), vgsummary.seqno);
-				good_mda_count++;
-			}
-		}
-
-		if (!rv1) {
-			/*
-			 * Remove the bad mda from normal mda list so it's not
-			 * used by vg_read/vg_write, but keep track of it in
-			 * lvmcache for repair.
-			 */
-			log_warn("WARNING: scanning %s mda1 failed to read metadata summary.", dev_name(dev));
-			log_warn("WARNING: repair VG metadata on %s with vgck --updatemetadata.", dev_name(dev));
-
-			dm_list_del(&mda1->list);
-			mda1->bad_fields = bad_fields;
-			lvmcache_save_bad_mda(info, mda1);
-			mda1 = NULL;
-			bad_mda_count++;
-		}
-	}
-
-	if (mda2) {
-		log_debug_metadata("Scanning %s mda2 summary.", dev_name(dev));
-		memset(&vgsummary, 0, sizeof(vgsummary));
-		bad_fields = 0;
-		vgsummary.mda_num = 2;
-
-		rv2 = _read_mda_header_and_metadata(fmt, mda2, &vgsummary, &bad_fields);
-
-		if (rv2 && !vgsummary.zero_offset && !vgsummary.mda_ignored) {
-			if (!lvmcache_update_vgname_and_id(info, &vgsummary)) {
-				/* I believe this is only an internal error. */
-				log_warn("WARNING: Scanning %s mda2 failed to save internal summary.", dev_name(dev));
-
-				dm_list_del(&mda2->list);
-				bad_fields |= BAD_MDA_INTERNAL;
-				mda2->bad_fields = bad_fields;
-				lvmcache_save_bad_mda(info, mda2);
-				mda2 = NULL;
-				bad_mda_count++;
-			} else {
-				/* The normal success path */
-				log_debug("Scanned %s mda2 seqno %u", dev_name(dev), vgsummary.seqno);
-				good_mda_count++;
-			}
-		}
-
-		if (!rv2) {
-			/*
-			 * Remove the bad mda from normal mda list so it's not
-			 * used by vg_read/vg_write, but keep track of it in
-			 * lvmcache for repair.
-			 */
-			log_warn("WARNING: scanning %s mda2 failed to read metadata summary.", dev_name(dev));
-			log_warn("WARNING: repair VG metadata on %s with vgck --updatemetadata.", dev_name(dev));
-
-			dm_list_del(&mda2->list);
-			mda2->bad_fields = bad_fields;
-			lvmcache_save_bad_mda(info, mda2);
-			mda2 = NULL;
-			bad_mda_count++;
-		}
-	}
-
-	if (good_mda_count)
-		return 1;
-
-	if (bad_mda_count)
+	if (!lvmcache_foreach_mda(info, _read_mda_header_and_metadata, &baton)) {
+		log_error("Failed to scan VG from %s", dev_name(dev));
 		return 0;
+	}
 
-	/* no metadata in the mdas */
 	return 1;
 }
 

lib/label/hints.c

@@ -294,12 +294,15 @@ static int _clear_hints(struct cmd_context *cmd)
 	return 1;
 }
 
-static int _lock_hints(int mode, int nonblock)
+static int _lock_hints(struct cmd_context *cmd, int mode, int nonblock)
 {
 	int fd;
 	int op = mode;
 	int ret;
 
+	if (cmd->nolocking)
+		return 1;
+
 	if (nonblock)
 		op |= LOCK_NB;
 
@@ -326,10 +329,13 @@ static int _lock_hints(int mode, int nonblock)
 	return 0;
 }
 
-static void _unlock_hints(void)
+static void _unlock_hints(struct cmd_context *cmd)
 {
 	int ret;
 
+	if (cmd->nolocking)
+		return;
+
 	if (_hints_fd == -1) {
 		log_warn("unlock_hints no existing fd");
 		return;
@@ -344,11 +350,11 @@ static void _unlock_hints(void)
 	_hints_fd = -1;
 }
 
-void hints_exit(void)
+void hints_exit(struct cmd_context *cmd)
 {
 	if (_hints_fd == -1)
 		return;
-	return _unlock_hints();
+	return _unlock_hints(cmd);
 }
 
 static struct hint *_find_hint_name(struct dm_list *hints, const char *name)
@@ -895,6 +901,11 @@ int write_hint_file(struct cmd_context *cmd, int newhints)
 		if (!(dev->flags & DEV_SCAN_FOUND_LABEL))
 			continue;
 
+		if (dev->flags & DEV_IS_MD_COMPONENT) {
+			log_debug("exclude md component from hints %s", dev_name(dev));
+			continue;
+		}
+
 		/*
 		 * No vgname will be found here for a PV with no mdas,
 		 * in which case the vgname hint will be incomplete.
@@ -941,7 +952,7 @@ int write_hint_file(struct cmd_context *cmd, int newhints)
 
  out_unlock:
 	/* get_hints() took ex lock before returning with newhints set */
-	_unlock_hints();
+	_unlock_hints(cmd);
 
 	return ret;
 }
@@ -979,17 +990,13 @@ int write_hint_file(struct cmd_context *cmd, int newhints)
  * an issue we could easily write the pid in the nohints file, and
  * others could check if the pid is still around before obeying it.)
  *
- * The intention is to call this function after the global ex lock has been
+ * The function is meant to be called after the global ex lock has been
  * taken, which is the official lock serializing commands changing which
  * devs are PVs or not.  This means that a command should never block in
  * this function due to another command that has used this function --
  * they would be serialized by the official global lock first.
  * e.g. two pvcreates should never block each other from the hint lock,
- * but rather from the global lock...
- *
- * Unfortunately, the global(orphan) lock is not used consistently so it's not
- * quite doing its job right and needs some cleanup.  Until that's done,
- * concurrent commands like pvcreate may block each other on the hint lock.
+ * but rather from the global lock.
  */
 
 void clear_hint_file(struct cmd_context *cmd)
@@ -1010,7 +1017,7 @@ void clear_hint_file(struct cmd_context *cmd)
 	if (!_touch_nohints())
 		stack;
 
-	if (!_lock_hints(LOCK_EX, 0))
+	if (!_lock_hints(cmd, LOCK_EX, 0))
 		stack;
 
 	_unlink_nohints();
@@ -1046,7 +1053,7 @@ void pvscan_recreate_hints_begin(struct cmd_context *cmd)
 	if (!_touch_nohints())
 		stack;
 
-	if (!_lock_hints(LOCK_EX, 0))
+	if (!_lock_hints(cmd, LOCK_EX, 0))
 		stack;
 
 	_unlink_nohints();
@@ -1198,7 +1205,7 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints,
 		log_debug("get_hints: newhints file");
 		if (!_hints_exists())
 			_touch_hints();
-		if (!_lock_hints(LOCK_EX, NONBLOCK))
+		if (!_lock_hints(cmd, LOCK_EX, NONBLOCK))
 			return 0;
 		/* create new hints after scan */
 		*newhints = NEWHINTS_FILE;
@@ -1212,7 +1219,7 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints,
 		log_debug("get_hints: no file");
 		if (!_touch_hints())
 			return 0;
-		if (!_lock_hints(LOCK_EX, NONBLOCK))
+		if (!_lock_hints(cmd, LOCK_EX, NONBLOCK))
 			return 0;
 		/* create new hints after scan */
 		*newhints = NEWHINTS_INIT;
@@ -1225,7 +1232,7 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints,
 	 * We hold a sh lock on the hints file while reading it to prevent
 	 * another command from clearing it while we're reading
 	 */
-	if (!_lock_hints(LOCK_SH, NONBLOCK)) {
+	if (!_lock_hints(cmd, LOCK_SH, NONBLOCK)) {
 		log_debug("get_hints: lock fail");
 		return 0;
 	}
@@ -1235,11 +1242,11 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints,
 	 */
 	if (!_read_hint_file(cmd, &hints_list, &needs_refresh)) {
 		log_debug("get_hints: read fail");
-		_unlock_hints();
+		_unlock_hints(cmd);
 		return 0;
 	}
 
-	_unlock_hints();
+	_unlock_hints(cmd);
 
 	/*
 	 * The content of the hint file is invalid and should be refreshed,
@@ -1248,7 +1255,7 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints,
 	if (needs_refresh) {
 		log_debug("get_hints: needs refresh");
 
-		if (!_lock_hints(LOCK_EX, NONBLOCK))
+		if (!_lock_hints(cmd, LOCK_EX, NONBLOCK))
 			return 0;
 
 		/* create new hints after scan */
@@ -1265,7 +1272,7 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints,
 	if (dm_list_empty(&hints_list)) {
 		log_debug("get_hints: no entries");
 
-		if (!_lock_hints(LOCK_EX, NONBLOCK))
+		if (!_lock_hints(cmd, LOCK_EX, NONBLOCK))
 			return 0;
 
 		/* create new hints after scan */

lib/label/hints.h

@@ -35,7 +35,7 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints, int *newhints,
 
 int validate_hints(struct cmd_context *cmd, struct dm_list *hints);
 
-void hints_exit(void);
+void hints_exit(struct cmd_context *cmd);
 
 void pvscan_recreate_hints_begin(struct cmd_context *cmd);
 

lib/label/label.c

@@ -139,7 +139,7 @@ int label_remove(struct device *dev)
 
 		wipe = 0;
 
-		if (!strncmp((char *)lh->id, LABEL_ID, sizeof(lh->id))) {
+		if (!memcmp(lh->id, LABEL_ID, sizeof(lh->id))) {
 			if (xlate64(lh->sector_xl) == sector)
 				wipe = 1;
 		} else {
@@ -192,7 +192,7 @@ int label_write(struct device *dev, struct label *label)
 
 	memset(buf, 0, LABEL_SIZE);
 
-	strncpy((char *)lh->id, LABEL_ID, sizeof(lh->id));
+	memcpy(lh->id, LABEL_ID, sizeof(lh->id));
 	lh->sector_xl = xlate64(label->sector);
 	lh->offset_xl = xlate32(sizeof(*lh));
 
@@ -293,7 +293,7 @@ static struct labeller *_find_lvm_header(struct device *dev,
 
 		lh = (struct label_header *) (scan_buf + (sector << SECTOR_SHIFT));
 
-		if (!strncmp((char *)lh->id, LABEL_ID, sizeof(lh->id))) {
+		if (!memcmp(lh->id, LABEL_ID, sizeof(lh->id))) {
 			if (found) {
 				log_error("Ignoring additional label on %s at sector %llu",
 					  dev_name(dev), (unsigned long long)(block_sector + sector));
@@ -356,9 +356,9 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f,
 			  int *is_lvm_device)
 {
 	char label_buf[LABEL_SIZE] __attribute__((aligned(8)));
+	struct label *label = NULL;
 	struct labeller *labeller;
 	uint64_t sector = 0;
-	int is_duplicate = 0;
 	int ret = 0;
 	int pass;
 
@@ -423,38 +423,17 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f,
 
 	/*
 	 * This is the point where the scanning code dives into the rest of
-	 * lvm.  ops->read() is _text_read() which reads the pv_header, mda
-	 * locations, and metadata text.  All of the info it finds about the PV
-	 * and VG is stashed in lvmcache which saves it in the form of
-	 * info/vginfo structs.  That lvmcache info is used later when the
-	 * command wants to read the VG to do something to it.
+	 * lvm.  ops->read() is usually _text_read() which reads the pv_header,
+	 * mda locations, mda contents.  As these bits of data are read, they
+	 * are saved into lvmcache as info/vginfo structs.
 	 */
-	ret = labeller->ops->read(labeller, dev, label_buf, sector, &is_duplicate);
 
-	if (!ret) {
-		if (is_duplicate) {
-			/*
-			 * _text_read() called lvmcache_add() which found an
-			 * existing info struct for this PVID but for a
-			 * different dev.  lvmcache_add() did not add an info
-			 * struct for this dev, but added this dev to the list
-			 * of duplicate devs.
-			 */
-			log_warn("WARNING: scan found duplicate PVID %s on %s", dev->pvid, dev_name(dev));
-		} else {
-			/*
-			 * Leave the info in lvmcache because the device is
-			 * present and can still be used even if it has
-			 * metadata that we can't process (we can get metadata
-			 * from another PV/mda.) _text_read only saves mdas
-			 * with good metadata in lvmcache (this includes old
-			 * metadata), and if a PV has no mdas with good
-			 * metadata, then the info for the PV will be in
-			 * lvmcache with empty info->mdas, and it will behave
-			 * like a PV with no mdas (a common configuration.)
-			 */
-			log_warn("WARNING: scan failed to get metadata summary from %s PVID %s", dev_name(dev), dev->pvid);
-		}
+	if ((ret = (labeller->ops->read)(labeller, dev, label_buf, &label)) && label) {
+		label->dev = dev;
+		label->sector = sector;
+	} else {
+		/* FIXME: handle errors */
+		lvmcache_del_dev(dev);
 	}
  out:
 	return ret;
@@ -717,6 +696,7 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f,
 				scan_failed = 1;
 				scan_process_errors++;
 				scan_failed_count++;
+				lvmcache_del_dev(devl->dev);
 			}
 		}
 
@@ -867,8 +847,11 @@ static void _free_hints(struct dm_list *hints)
 }
 
 /*
- * Scan and cache lvm data from all devices on the system.
- * The cache should be empty/reset before calling this.
+ * Scan devices on the system to discover which are LVM devices.
+ * Info about the LVM devices (PVs) is saved in lvmcache in a
+ * basic/summary form (info/vginfo structs).  The vg_read phase
+ * uses this summary info to know which PVs to look at for
+ * processing a given VG.
  */
 
 int label_scan(struct cmd_context *cmd)
@@ -880,7 +863,8 @@ int label_scan(struct cmd_context *cmd)
 	struct device_list *devl, *devl2;
 	struct device *dev;
 	uint64_t max_metadata_size_bytes;
-	int newhints = 0;
+	int using_hints;
+	int create_hints = 0; /* NEWHINTS_NONE */
 
 	log_debug_devs("Finding devices to scan");
 
@@ -889,20 +873,37 @@ int label_scan(struct cmd_context *cmd)
 	dm_list_init(&hints_list);
 
 	/*
-	 * Iterate through all the devices in dev-cache (block devs that appear
-	 * under /dev that could possibly hold a PV and are not excluded by
-	 * filters).  Read each to see if it's an lvm device, and if so
-	 * populate lvmcache with some basic info about the device and the VG
-	 * on it.  This info will be used by the vg_read() phase of the
-	 * command.
+	 * dev_cache_scan() creates a list of devices on the system
+	 * (saved in in dev-cache) which we can iterate through to
+	 * search for LVM devs.  The dev cache list either comes from
+	 * looking at dev nodes under /dev, or from udev.
 	 */
 	dev_cache_scan();
 
+	/*
+	 * Set up the iterator that is needed to step through each device in
+	 * dev cache.
+	 */
 	if (!(iter = dev_iter_create(cmd->filter, 0))) {
 		log_error("Scanning failed to get devices.");
 		return 0;
 	}
 
+	log_debug_devs("Filtering devices to scan");
+
+	/*
+	 * Iterate through all devices in dev cache and apply filters
+	 * to exclude devs that we do not need to scan.  Those devs
+	 * that pass the filters are returned by the iterator and
+	 * saved in a list of devs that we will proceed to scan to
+	 * check if they are LVM devs.  IOW this loop is the
+	 * application of filters (those that do not require reading
+	 * the devs) to the list of all devices.  It does that because
+	 * the 'cmd->filter' is used above when setting up the iterator.
+	 * Unfortunately, it's not obvious that this is what's happening
+	 * here.  filters that require reading the device are not applied
+	 * here, but in process_block(), see DEV_FILTER_AFTER_SCAN.
+	 */
 	while ((dev = dev_iter_get(cmd, iter))) {
 		if (!(devl = zalloc(sizeof(*devl))))
 			continue;
@@ -921,13 +922,12 @@ int label_scan(struct cmd_context *cmd)
 		/*
 		 * When md devices exist that use the old superblock at the
 		 * end of the device, then in order to detect and filter out
-		 * the component devices of those md devs, we need to enable
-		 * the full md filter which scans both the start and the end
-		 * of every device.  This doubles the amount of scanning i/o,
-		 * which we want to avoid.  FIXME: it may not be worth the
-		 * cost of double i/o just to avoid displaying md component
-		 * devs in 'pvs', which is a pretty harmless effect from a
-		 * pretty uncommon situation.
+		 * the component devices of those md devs, we enable the full
+		 * md filter which scans both the start and the end of every
+		 * device.  This doubles the amount of scanning i/o, which we
+		 * want to avoid.  FIXME: this forces start+end scanning of
+		 * every device, but it would be more efficient to limit the
+		 * end scan only to PVs.
 		 */
 		if (dev_is_md_with_end_superblock(cmd->dev_types, dev))
 			cmd->use_full_md_check = 1;
@@ -940,7 +940,10 @@ int label_scan(struct cmd_context *cmd)
 	}
 
 	/*
-	 * In some common cases we can avoid scanning all devices.
+	 * In some common cases we can avoid scanning all devices
+	 * by using hints which tell us which devices are PVs, which
+	 * are the only devices we actually need to scan.  Without
+	 * hints we need to scan all devs to find which are PVs.
 	 *
 	 * TODO: if the command is using hints and a single vgname
 	 * arg, we can also take the vg lock here, prior to scanning.
@@ -950,10 +953,12 @@ int label_scan(struct cmd_context *cmd)
 	 * able to avoid rescan in vg_read, but locking early would
 	 * apply to more cases.)
 	 */
-	if (!get_hints(cmd, &hints_list, &newhints, &all_devs, &scan_devs)) {
+	if (!get_hints(cmd, &hints_list, &create_hints, &all_devs, &scan_devs)) {
 		dm_list_splice(&scan_devs, &all_devs);
 		dm_list_init(&hints_list);
-	}
+		using_hints = 0;
+	} else
+		using_hints = 1;
 
 	log_debug("Will scan %d devices skip %d", dm_list_size(&scan_devs), dm_list_size(&all_devs));
 
@@ -999,17 +1004,18 @@ int label_scan(struct cmd_context *cmd)
 
 	dm_list_init(&cmd->hints);
 
-	if (!dm_list_empty(&hints_list)) {
+	/*
+	 * If we're using hints to limit which devs we scanned, verify
+	 * that those hints were valid, and if not we need to scan the
+	 * rest of the devs.
+	 */
+	if (using_hints) {
 		if (!validate_hints(cmd, &hints_list)) {
-			/*
-			 * We scanned a subset of all devices based on hints.
-			 * With the results from the scan we may decide that
-			 * the hints are not valid, so scan all others.
-			 */
 			log_debug("Will scan %d remaining devices", dm_list_size(&all_devs));
 			_scan_list(cmd, cmd->filter, &all_devs, NULL);
 			_free_hints(&hints_list);
-			newhints = 0;
+			using_hints = 0;
+			create_hints = 0;
 		} else {
 			/* The hints may be used by another device iteration. */
 			dm_list_splice(&cmd->hints, &hints_list);
@@ -1026,8 +1032,15 @@ int label_scan(struct cmd_context *cmd)
 		free(devl);
 	}
 
-	if (newhints)
-		write_hint_file(cmd, newhints);
+	/*
+	 * If hints were not available/usable, then we scanned all devs,
+	 * and we now know which are PVs.  Save this list of PVs we've
+	 * identified as hints for the next command to use.
+	 * (create_hints variable has NEWHINTS_X value which indicates
+	 * the reason for creating the new hints.)
+	 */
+	if (create_hints)
+		write_hint_file(cmd, create_hints);
 
 	return 1;
 }
@@ -1044,8 +1057,6 @@ int label_scan_devs(struct cmd_context *cmd, struct dev_filter *f, struct dm_lis
 {
 	struct device_list *devl;
 
-	/* FIXME: get rid of this, it's only needed for lvmetad in which
-	   case we should be setting up bcache in one place. */
 	if (!scan_bcache) {
 		if (!_setup_bcache(0))
 			return 0;
@@ -1247,14 +1258,6 @@ out:
 	return ret;
 }
 
-/*
- * This is only needed when commands are using lvmetad, in which case they
- * don't do an initial label_scan, but may later need to rescan certain devs
- * from disk and call this function.  FIXME: is there some better number to
- * choose here?  How should we predict the number of devices that might need
- * scanning when using lvmetad?
- */
-
 int label_scan_setup_bcache(void)
 {
 	if (!scan_bcache) {
@@ -1292,6 +1295,18 @@ int label_scan_open_excl(struct device *dev)
 	return label_scan_open(dev);
 }
 
+int label_scan_open_rw(struct device *dev)
+{
+	if (_in_bcache(dev) && !(dev->flags & DEV_BCACHE_WRITE)) {
+		/* FIXME: avoid tossing out bcache blocks just to replace fd. */
+		log_debug("Close and reopen rw %s", dev_name(dev));
+		bcache_invalidate_fd(scan_bcache, dev->bcache_fd);
+		_scan_dev_close(dev);
+	}
+	dev->flags |= DEV_BCACHE_WRITE;
+	return label_scan_open(dev);
+}
+
 bool dev_read_bytes(struct device *dev, uint64_t start, size_t len, void *data)
 {
 	if (!scan_bcache) {
@@ -1301,7 +1316,7 @@ bool dev_read_bytes(struct device *dev, uint64_t start, size_t len, void *data)
 	}
 
 	if (dev->bcache_fd <= 0) {
-		/* This is not often needed, perhaps only with lvmetad. */
+		/* This is not often needed. */
 		if (!label_scan_open(dev)) {
 			log_error("Error opening device %s for reading at %llu length %u.",
 				  dev_name(dev), (unsigned long long)start, (uint32_t)len);
@@ -1341,7 +1356,7 @@ bool dev_write_bytes(struct device *dev, uint64_t start, size_t len, void *data)
 	}
 
 	if (dev->bcache_fd <= 0) {
-		/* This is not often needed, perhaps only with lvmetad. */
+		/* This is not often needed. */
 		dev->flags |= DEV_BCACHE_WRITE;
 		if (!label_scan_open(dev)) {
 			log_error("Error opening device %s for writing at %llu length %u.",
@@ -1387,7 +1402,7 @@ bool dev_write_zeros(struct device *dev, uint64_t start, size_t len)
 	}
 
 	if (dev->bcache_fd <= 0) {
-		/* This is not often needed, perhaps only with lvmetad. */
+		/* This is not often needed. */
 		dev->flags |= DEV_BCACHE_WRITE;
 		if (!label_scan_open(dev)) {
 			log_error("Error opening device %s for writing at %llu length %u.",
@@ -1436,7 +1451,7 @@ bool dev_set_bytes(struct device *dev, uint64_t start, size_t len, uint8_t val)
 	}
 
 	if (dev->bcache_fd <= 0) {
-		/* This is not often needed, perhaps only with lvmetad. */
+		/* This is not often needed. */
 		dev->flags |= DEV_BCACHE_WRITE;
 		if (!label_scan_open(dev)) {
 			log_error("Error opening device %s for writing at %llu length %u.",

lib/label/label.h

@@ -65,7 +65,7 @@ struct label_ops {
 	 * Read a label from a volume.
 	 */
 	int (*read) (struct labeller * l, struct device * dev,
-		     void *label_buf, uint64_t label_sector, int *is_duplicate);
+		     void *label_buf, struct label ** label);
 
 	/*
 	 * Populate label_type etc.
@@ -115,6 +115,7 @@ void label_scan_confirm(struct device *dev);
 int label_scan_setup_bcache(void);
 int label_scan_open(struct device *dev);
 int label_scan_open_excl(struct device *dev);
+int label_scan_open_rw(struct device *dev);
 
 /*
  * Wrappers around bcache equivalents.

lib/locking/file_locking.c

@@ -44,7 +44,7 @@ static int _file_lock_resource(struct cmd_context *cmd, const char *resource,
 {
 	char lockfile[PATH_MAX];
 
-	if (is_orphan_vg(resource) || is_global_vg(resource)) {
+	if (!strcmp(resource, VG_GLOBAL)) {
 		if (dm_snprintf(lockfile, sizeof(lockfile),
 				"%s/P_%s", _lock_dir, resource + 1) < 0) {
 			log_error("Too long locking filename %s/P_%s.", _lock_dir, resource + 1);

lib/locking/locking.c

@@ -15,6 +15,7 @@
 
 #include "lib/misc/lib.h"
 #include "lib/locking/locking.h"
+#include "lib/locking/lvmlockd.h"
 #include "locking_types.h"
 #include "lib/misc/lvm-string.h"
 #include "lib/activate/activate.h"
@@ -185,13 +186,14 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags, const str
 {
 	char resource[258] __attribute__((aligned(8)));
 	uint32_t lck_type = flags & LCK_TYPE_MASK;
+	int is_global = !strcmp(vol, VG_GLOBAL);
+
+	if (is_orphan_vg(vol))
+		return 1;
 
 	if (!_blocking_supported)
 		flags |= LCK_NONBLOCK;
 
-	if (is_orphan_vg(vol))
-		vol = VG_ORPHANS;
-
 	if (!dm_strncpy(resource, vol, sizeof(resource))) {
 		log_error(INTERNAL_ERROR "Resource name %s is too long.", vol);
 		return 0;
@@ -211,7 +213,7 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags, const str
 		if (lck_type != LCK_WRITE)
 			goto out_hold;
 
-		if (cmd->is_activating && strcmp(vol, VG_GLOBAL))
+		if (cmd->is_activating && !is_global)
 			goto out_hold;
 
 		goto out_fail;
@@ -252,7 +254,7 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags, const str
 	 * refuse write lock requests.
 	 */
 	if (cmd->metadata_read_only) {
-		if ((lck_type == LCK_WRITE) && strcmp(vol, VG_GLOBAL)) {
+		if (lck_type == LCK_WRITE) {
 			log_error("Operation prohibited while global/metadata_read_only is set.");
 			goto out_fail;
 		}
@@ -264,6 +266,9 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags, const str
 		goto out_fail;
 
 out_hold:
+	if (is_global)
+		return 1;
+
 	/*
 	 * FIXME: other parts of the code want to check if a VG is
 	 * locked by looking in lvmcache.  They shouldn't need to
@@ -279,6 +284,8 @@ out_hold:
 	return 1;
 
 out_fail:
+	if (is_global)
+		return 0;
 	if (lck_type == LCK_UNLOCK)
 		_update_vg_lock_count(resource, flags);
 	return 0;
@@ -318,3 +325,89 @@ int sync_local_dev_names(struct cmd_context* cmd)
 	return 1;
 }
 
+/*
+ * The lockf_global_ex flag is used to prevent changing
+ * an explicitly acquired ex global lock to sh in process_each.
+ */
+
+static int _lockf_global(struct cmd_context *cmd, const char *mode, int convert)
+{
+	uint32_t flags = 0;
+	int ret;
+
+	if (convert)
+		flags |= LCK_CONVERT;
+
+	if (!strcmp(mode, "ex")) {
+		flags |= LCK_WRITE;
+
+		if (cmd->lockf_global_ex) {
+			log_warn("global flock already held ex");
+			return 1;
+		}
+
+		ret = lock_vol(cmd, VG_GLOBAL, flags, NULL);
+		if (ret)
+			cmd->lockf_global_ex = 1;
+
+	} else if (!strcmp(mode, "sh")) {
+		if (cmd->lockf_global_ex)
+			return 1;
+
+		flags |= LCK_READ;
+
+		ret = lock_vol(cmd, VG_GLOBAL, flags, NULL);
+
+	} else if (!strcmp(mode, "un")) {
+		ret = lock_vol(cmd, VG_GLOBAL, LCK_UNLOCK, NULL);
+		cmd->lockf_global_ex = 0;
+	} else {
+		log_error(INTERNAL_ERROR "Unknown locking mode %s.", mode);
+		return 0;
+	}
+
+	return ret;
+}
+
+int lockf_global(struct cmd_context *cmd, const char *mode)
+{
+	return _lockf_global(cmd, mode, 0);
+}
+
+int lockf_global_convert(struct cmd_context *cmd, const char *mode)
+{
+	return _lockf_global(cmd, mode, 1);
+}
+
+int lock_global(struct cmd_context *cmd, const char *mode)
+{
+	if (!lockf_global(cmd, mode))
+		return 0;
+
+	if (!lockd_global(cmd, mode)) {
+		lockf_global(cmd, "un");
+		return 0;
+	}
+
+	return 1;
+}
+
+/*
+ * The global lock is already held, convert it to another mode.
+ *
+ * Currently only used for sh->ex.
+ *
+ * (The lockf_global_ex flag would need overriding
+ * to handle ex->sh.)
+ */
+
+int lock_global_convert(struct cmd_context *cmd, const char *mode)
+{
+	if (!lockf_global_convert(cmd, mode))
+		return 0;
+
+	if (!lockd_global(cmd, mode))
+		return 0;
+
+	return 1;
+}

lib/locking/locking.h

@@ -28,12 +28,10 @@ int vg_write_lock_held(void);
 
 /*
  *   Lock/unlock on-disk volume group data.
- *   Use VG_ORPHANS to lock all orphan PVs.
- *   Use VG_GLOBAL as a global lock and to wipe the internal cache.
+ *   Use VG_GLOBAL as a global lock.
  *   char *vol holds volume group name.
  *   If more than one lock needs to be held simultaneously, they must be
- *   acquired in alphabetical order of 'vol' (to avoid deadlocks), with
- *   VG_ORPHANS last.
+ *   acquired in alphabetical order of 'vol' (to avoid deadlocks).
  */
 int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags, const struct logical_volume *lv);
 
@@ -47,10 +45,8 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags, const str
  * Bottom 8 bits except LCK_LOCAL form args[0] in cluster comms.
  */
 #define LCK_NONBLOCK	0x00000010U	/* Don't block waiting for lock? */
+#define LCK_CONVERT	0x00000020U
 
-/*
- * Special cases of VG locks.
- */
 #define VG_ORPHANS	"#orphans"
 #define VG_GLOBAL	"#global"
 
@@ -77,4 +73,9 @@ int sync_local_dev_names(struct cmd_context* cmd);
 struct volume_group;
 int activate_lvs(struct cmd_context *cmd, struct dm_list *lvs, unsigned exclusive);
 
+int lockf_global(struct cmd_context *cmd, const char *mode);
+int lockf_global_convert(struct cmd_context *cmd, const char *mode);
+int lock_global(struct cmd_context *cmd, const char *mode);
+int lock_global_convert(struct cmd_context *cmd, const char *mode);
+
 #endif

lib/locking/lvmlockd.c

@@ -1315,7 +1315,7 @@ int lockd_start_wait(struct cmd_context *cmd)
  *    Future lockd_gl/lockd_gl_create calls will acquire the existing gl.
  */
 
-int lockd_gl_create(struct cmd_context *cmd, const char *def_mode, const char *vg_lock_type)
+int lockd_global_create(struct cmd_context *cmd, const char *def_mode, const char *vg_lock_type)
 {
 	const char *mode = NULL;
 	uint32_t lockd_flags;
@@ -1460,6 +1460,12 @@ int lockd_gl_create(struct cmd_context *cmd, const char *def_mode, const char *v
 	/* --shared with vgcreate does not mean include_shared_vgs */
 	cmd->include_shared_vgs = 0;
 
+	/*
+	 * This is done to prevent converting an explicitly acquired
+	 * ex lock to sh in process_each.
+	 */
+	cmd->lockd_global_ex = 1;
+
 	return 1;
 }
 
@@ -1542,7 +1548,7 @@ int lockd_gl_create(struct cmd_context *cmd, const char *def_mode, const char *v
  * are unprotected.
  */
 
-int lockd_gl(struct cmd_context *cmd, const char *def_mode, uint32_t flags)
+int lockd_global(struct cmd_context *cmd, const char *def_mode)
 {
 	const char *mode = NULL;
 	const char *opts = NULL;
@@ -1572,10 +1578,16 @@ int lockd_gl(struct cmd_context *cmd, const char *def_mode, uint32_t flags)
 	if (!mode)
 		mode = def_mode;
 	if (!mode) {
-		log_error("Unknown lock-gl mode");
+		log_error("Unknown lvmlockd global lock mode");
 		return 0;
 	}
 
+	if (!strcmp(mode, "sh") && cmd->lockd_global_ex)
+		return 1;
+
+	if (!strcmp(mode, "un") && cmd->lockd_global_ex)
+		cmd->lockd_global_ex = 0;
+
  req:
 	log_debug("lockd global mode %s", mode);
 
@@ -1719,6 +1731,14 @@ int lockd_gl(struct cmd_context *cmd, const char *def_mode, uint32_t flags)
 	}
 
  allow:
+
+	/*
+	 * This is done to prevent converting an explicitly acquired
+	 * ex lock to sh in process_each.
+	 */
+	if (!strcmp(mode, "ex"))
+		cmd->lockd_global_ex = 1;
+
 	return 1;
 }
 

lib/locking/lvmlockd.h

@@ -13,12 +13,10 @@
 
 #include "libdaemon/client/config-util.h"
 #include "libdaemon/client/daemon-client.h"
+#include "lib/metadata/metadata-exported.h" /* is_lockd_type() */
 
 #define LOCKD_SANLOCK_LV_NAME "lvmlock"
 
-/* lockd_gl flags */
-#define LDGL_UPDATE_NAMES         0x00000001
-
 /* lockd_lv flags */
 #define LDLV_MODE_NO_SH           0x00000001
 #define LDLV_PERSISTENT           0x00000002
@@ -43,6 +41,9 @@
 
 #ifdef LVMLOCKD_SUPPORT
 
+struct lvresize_params;
+struct lvcreate_params;
+
 /* lvmlockd connection and communication */
 
 void lvmlockd_set_socket(const char *sock);
@@ -71,8 +72,8 @@ int lockd_start_wait(struct cmd_context *cmd);
 
 /* locking */
 
-int lockd_gl_create(struct cmd_context *cmd, const char *def_mode, const char *vg_lock_type);
-int lockd_gl(struct cmd_context *cmd, const char *def_mode, uint32_t flags);
+int lockd_global_create(struct cmd_context *cmd, const char *def_mode, const char *vg_lock_type);
+int lockd_global(struct cmd_context *cmd, const char *def_mode);
 int lockd_vg(struct cmd_context *cmd, const char *vg_name, const char *def_mode,
 	     uint32_t flags, uint32_t *lockd_state);
 int lockd_vg_update(struct volume_group *vg);
@@ -169,7 +170,7 @@ static inline int lockd_start_wait(struct cmd_context *cmd)
 	return 0;
 }
 
-static inline int lockd_gl_create(struct cmd_context *cmd, const char *def_mode, const char *vg_lock_type)
+static inline int lockd_global_create(struct cmd_context *cmd, const char *def_mode, const char *vg_lock_type)
 {
 	/*
 	 * When lvm is built without lvmlockd support, creating a VG with
@@ -182,7 +183,7 @@ static inline int lockd_gl_create(struct cmd_context *cmd, const char *def_mode,
 	return 1;
 }
 
-static inline int lockd_gl(struct cmd_context *cmd, const char *def_mode, uint32_t flags)
+static inline int lockd_global(struct cmd_context *cmd, const char *def_mode)
 {
 	return 1;
 }

lib/metadata/lv_manip.c

@@ -7365,7 +7365,7 @@ int wipe_lv(struct logical_volume *lv, struct wipe_params wp)
 		return 0;
 	}
 
-	if (!label_scan_open(dev)) {
+	if (!label_scan_open_rw(dev)) {
 		log_error("Failed to open %s/%s for wiping and zeroing.", lv->vg->name, lv->name);
 		goto out;
 	}

lib/metadata/metadata-exported.h

@@ -181,15 +181,15 @@
 #define MIRROR_SKIP_INIT_SYNC	0x00000010U	/* skip initial sync */
 
 /* 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
 #define READ_FOR_UPDATE		0x00100000U /* A meta-flag, useful with toollib for_each_* functions. */
 #define PROCESS_SKIP_SCAN	 0x00200000U /* skip lvmcache_label_scan in process_each_pv */
-#define PROCESS_SKIP_ORPHAN_LOCK 0x00400000U /* skip lock_vol(VG_ORPHAN) in vg_read */
 
-/* vg_read returns these in error_flags */
-#define FAILED_NOT_ENABLED	0x00000001U
+/* vg's "read_status" field */
+#define FAILED_INCONSISTENT	0x00000001U
 #define FAILED_LOCKING		0x00000002U
 #define FAILED_NOTFOUND		0x00000004U
 #define FAILED_READ_ONLY	0x00000008U
@@ -202,7 +202,6 @@
 #define FAILED_SYSTEMID		0x00000400U
 #define FAILED_LOCK_TYPE	0x00000800U
 #define FAILED_LOCK_MODE	0x00001000U
-#define FAILED_INTERNAL_ERROR	0x00002000U
 #define SUCCESS			0x00000000U
 
 #define VGMETADATACOPIES_ALL UINT32_MAX
@@ -706,7 +705,6 @@ int move_pv(struct volume_group *vg_from, struct volume_group *vg_to,
 int move_pvs_used_by_lv(struct volume_group *vg_from,
 			struct volume_group *vg_to,
 			const char *lv_name);
-int is_global_vg(const char *vg_name);
 int is_orphan_vg(const char *vg_name);
 int is_real_vg(const char *vg_name);
 int vg_missing_pv_count(const struct volume_group *vg);
@@ -719,14 +717,24 @@ int lv_resize(struct logical_volume *lv,
 	      struct lvresize_params *lp,
 	      struct dm_list *pvh);
 
-struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const char *vgid,
-			     uint32_t read_flags, uint32_t lockd_state,
-			     uint32_t *error_flags, struct volume_group **error_vg);
+/*
+ * Return a handle to VG metadata.
+ */
+struct volume_group *vg_read_internal(struct cmd_context *cmd,
+                                      const char *vgname, const char *vgid,
+                                      uint32_t lockd_state, uint32_t warn_flags,
+                                      int enable_repair,
+                                      int *mdas_consistent);
+struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name,
+			     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 read_flags, uint32_t lockd_state);
-struct volume_group *vg_read_orphans(struct cmd_context *cmd, const char *orphan_vgname);
-
-/* this is historical and being removed, don't use */
+struct volume_group *vg_read_orphans(struct cmd_context *cmd,
+                                             uint32_t warn_flags,
+                                             const char *orphan_vgname);
+/* 
+ * Test validity of a VG handle.
+ */
 uint32_t vg_read_error(struct volume_group *vg_handle);
 
 /* pe_start and pe_end relate to any existing data so that new metadata
@@ -749,7 +757,7 @@ uint32_t pv_list_extents_free(const struct dm_list *pvh);
 int validate_new_vg_name(struct cmd_context *cmd, const char *vg_name);
 int vg_validate(struct volume_group *vg);
 struct volume_group *vg_create(struct cmd_context *cmd, const char *vg_name);
-struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name, int *exists);
+struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name);
 int vg_remove_mdas(struct volume_group *vg);
 int vg_remove_check(struct volume_group *vg);
 void vg_remove_pvs(struct volume_group *vg);
@@ -1373,6 +1381,4 @@ int lv_on_pmem(struct logical_volume *lv);
 
 int vg_is_foreign(struct volume_group *vg);
 
-void vg_write_commit_bad_mdas(struct cmd_context *cmd, struct volume_group *vg);
-
 #endif

lib/metadata/metadata.h

@@ -168,27 +168,11 @@ struct metadata_area_ops {
 #define MDA_CONTENT_REASON(primary_mda) ((primary_mda) ? DEV_IO_MDA_CONTENT : DEV_IO_MDA_EXTRA_CONTENT)
 #define MDA_HEADER_REASON(primary_mda)  ((primary_mda) ? DEV_IO_MDA_HEADER : DEV_IO_MDA_EXTRA_HEADER)
 
-/*
- * Flags describing errors found while reading.
- */
-#define BAD_MDA_INTERNAL	0x00000001 /* internal lvm error */
-#define BAD_MDA_READ		0x00000002 /* read io failed */
-#define BAD_MDA_HEADER		0x00000004 /* general problem with header */
-#define BAD_MDA_TEXT		0x00000008 /* general problem with text */
-#define BAD_MDA_CHECKSUM	0x00000010
-#define BAD_MDA_MAGIC		0x00000020
-#define BAD_MDA_VERSION		0x00000040
-#define BAD_MDA_START		0x00000080
-
 struct metadata_area {
 	struct dm_list list;
 	struct metadata_area_ops *ops;
 	void *metadata_locn;
 	uint32_t status;
-	uint64_t header_start; /* mda_header.start */
-	int mda_num;
-	uint32_t bad_fields; /* BAD_MDA_ flags are set to indicate errors found when reading */
-	uint32_t ignore_bad_fields; /* BAD_MDA_ flags are set to indicate errors to ignore */
 };
 struct metadata_area *mda_copy(struct dm_pool *mem,
 			       struct metadata_area *mda);

lib/metadata/pv.h

@@ -59,7 +59,6 @@ struct physical_volume {
 
         /* This is true whenever the represented PV has a label associated. */
         uint64_t is_labelled:1;
-        uint64_t unused_missing_cleared:1;
 
         /* NB. label_sector is valid whenever is_labelled is true */
 	uint64_t label_sector;

lib/metadata/raid_manip.c

@@ -6471,7 +6471,7 @@ int lv_raid_convert(struct logical_volume *lv,
 		return_0;
 
 	region_size = new_region_size ? : seg->region_size;
-	region_size = region_size ? : get_default_region_size(lv->vg->cmd);
+	region_size = region_size ? : (uint32_t)get_default_region_size(lv->vg->cmd);
 
 	/*
 	 * Check acceptible options mirrors, region_size,

lib/metadata/read.c

@@ -1,964 +0,0 @@
-/*
- * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2012 Red Hat, Inc. All rights reserved.
- *
- * This file is part of LVM2.
- *
- * This copyrighted material is made available to anyone wishing to use,
- * modify, copy, or redistribute it subject to the terms and conditions
- * of the GNU Lesser General Public License v.2.1.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include "lib/misc/lib.h"
-#include "lib/device/device.h"
-#include "lib/metadata/metadata.h"
-#include "lib/commands/toolcontext.h"
-#include "lib/misc/lvm-string.h"
-#include "lib/misc/lvm-file.h"
-#include "lib/cache/lvmcache.h"
-#include "lib/mm/memlock.h"
-#include "lib/datastruct/str_list.h"
-#include "lib/metadata/pv_alloc.h"
-#include "lib/metadata/segtype.h"
-#include "lib/activate/activate.h"
-#include "lib/display/display.h"
-#include "lib/locking/locking.h"
-#include "lib/format_text/archiver.h"
-#include "lib/format_text/format-text.h"
-#include "lib/format_text/layout.h"
-#include "lib/format_text/import-export.h"
-#include "lib/config/defaults.h"
-#include "lib/locking/lvmlockd.h"
-#include "lib/notify/lvmnotify.h"
-
-#include <time.h>
-#include <math.h>
-
-static int _check_pv_ext(struct cmd_context *cmd, struct volume_group *vg)
-{
-	struct lvmcache_info *info;
-	uint32_t ext_version, ext_flags;
-	struct pv_list *pvl;
-
-	if (vg_is_foreign(vg))
-		return 1;
-
-	if (vg_is_shared(vg))
-		return 1;
-
-	dm_list_iterate_items(pvl, &vg->pvs) {
-		if (is_missing_pv(pvl->pv))
-			continue;
-
-		/* is_missing_pv doesn't catch NULL dev */
-		if (!pvl->pv->dev)
-			continue;
-
-		if (!(info = lvmcache_info_from_pvid(pvl->pv->dev->pvid, pvl->pv->dev, 0)))
-			continue;
-
-		ext_version = lvmcache_ext_version(info);
-		if (ext_version < PV_HEADER_EXTENSION_VSN) {
-			log_warn("WARNING: PV %s in VG %s is using an old PV header, modify the VG to update.",
-				 dev_name(pvl->pv->dev), vg->name);
-			continue;
-		}
-
-		ext_flags = lvmcache_ext_flags(info);
-		if (!(ext_flags & PV_EXT_USED)) {
-			log_warn("WARNING: PV %s in VG %s is missing the used flag in PV header.",
-				 dev_name(pvl->pv->dev), vg->name);
-		}
-	}
-
-	return 1;
-}
-
-#define DEV_LIST_DELIM ", "
-
-static int _check_devs_used_correspond_with_lv(struct dm_pool *mem, struct dm_list *list, struct logical_volume *lv)
-{
-	struct device_list *dl;
-	int found_inconsistent = 0;
-	struct device *dev;
-	struct lv_segment *seg;
-	uint32_t s;
-	int warned_about_no_dev = 0;
-	char *used_devnames = NULL, *assumed_devnames = NULL;
-
-	if (!(list = dev_cache_get_dev_list_for_lvid(lv->lvid.s + ID_LEN)))
-		return 1;
-
-	dm_list_iterate_items(dl, list) {
-		dev = dl->dev;
-		if (!(dev->flags & DEV_ASSUMED_FOR_LV)) {
-			if (!found_inconsistent) {
-				if (!dm_pool_begin_object(mem, 32))
-					return_0;
-				found_inconsistent = 1;
-			} else {
-				if (!dm_pool_grow_object(mem, DEV_LIST_DELIM, sizeof(DEV_LIST_DELIM) - 1))
-					return_0;
-			}
-			if (!dm_pool_grow_object(mem, dev_name(dev), 0))
-				return_0;
-		}
-	}
-
-	if (!found_inconsistent)
-		return 1;
-
-	if (!dm_pool_grow_object(mem, "\0", 1))
-		return_0;
-	used_devnames = dm_pool_end_object(mem);
-
-	found_inconsistent = 0;
-	dm_list_iterate_items(seg, &lv->segments) {
-		for (s = 0; s < seg->area_count; s++) {
-			if (seg_type(seg, s) == AREA_PV) {
-				if (!(dev = seg_dev(seg, s))) {
-					if (!warned_about_no_dev) {
-						log_warn("WARNING: Couldn't find all devices for LV %s "
-							 "while checking used and assumed devices.",
-							  display_lvname(lv));
-						warned_about_no_dev = 1;
-					}
-					continue;
-				}
-				if (!(dev->flags & DEV_USED_FOR_LV)) {
-					if (!found_inconsistent) {
-						if (!dm_pool_begin_object(mem, 32))
-                                                        return_0;
-						found_inconsistent = 1;
-					} else {
-						if (!dm_pool_grow_object(mem, DEV_LIST_DELIM, sizeof(DEV_LIST_DELIM) - 1))
-							return_0;
-					}
-					if (!dm_pool_grow_object(mem, dev_name(dev), 0))
-						return_0;
-				}
-			}
-		}
-	}
-
-	if (found_inconsistent) {
-		if (!dm_pool_grow_object(mem, "\0", 1))
-			return_0;
-		assumed_devnames = dm_pool_end_object(mem);
-		log_warn("WARNING: Device mismatch detected for %s which is accessing %s instead of %s.",
-			 display_lvname(lv), used_devnames, assumed_devnames);
-	}
-
-	return 1;
-}
-
-static void _check_devs_used_correspond_with_vg(struct volume_group *vg)
-{
-	struct dm_pool *mem;
-	char vgid[ID_LEN + 1];
-	struct pv_list *pvl;
-	struct lv_list *lvl;
-	struct dm_list *list;
-	struct device_list *dl;
-	int found_inconsistent = 0;
-
-	strncpy(vgid, (const char *) vg->id.uuid, sizeof(vgid));
-	vgid[ID_LEN] = '\0';
-
-	/* Mark all PVs in VG as used. */
-	dm_list_iterate_items(pvl, &vg->pvs) {
-		/*
-		 * FIXME: It's not clear if the meaning
-		 * of "missing" should always include the
-		 * !pv->dev case, or if "missing" is the
-		 * more narrow case where VG metadata has
-		 * been written with the MISSING flag.
-		 */
-		if (!pvl->pv->dev)
-			continue;
-		if (is_missing_pv(pvl->pv))
-			continue;
-		pvl->pv->dev->flags |= DEV_ASSUMED_FOR_LV;
-	}
-
-	if (!(list = dev_cache_get_dev_list_for_vgid(vgid)))
-		return;
-
-	dm_list_iterate_items(dl, list) {
-		if (!(dl->dev->flags & DEV_OPEN_FAILURE) &&
-		    !(dl->dev->flags & DEV_ASSUMED_FOR_LV)) {
-			found_inconsistent = 1;
-			break;
-		}
-	}
-
-	if (found_inconsistent) {
-		if (!(mem = dm_pool_create("vg_devs_check", 1024)))
-			return;
-
-		dm_list_iterate_items(lvl, &vg->lvs) {
-			if (!_check_devs_used_correspond_with_lv(mem, list, lvl->lv)) {
-				dm_pool_destroy(mem);
-				return;
-			}
-		}
-
-		dm_pool_destroy(mem);
-	}
-
-	return;
-}
-
-static void _destroy_fid(struct format_instance **fid)
-{
-	if (*fid) {
-		(*fid)->fmt->ops->destroy_instance(*fid);
-		*fid = NULL;
-	}
-}
-
-static int _access_vg_clustered(struct cmd_context *cmd, const struct volume_group *vg)
-{
-	if (vg_is_clustered(vg)) {
-		/*
-		 * force_access_clustered is only set when forcibly
-		 * converting a clustered vg to lock type none.
-		 */
-		if (cmd->force_access_clustered) {
-			log_debug("Allowing forced access to clustered vg %s", vg->name);
-			return 1;
-		}
-
-		log_verbose("Skipping clustered VG %s.", vg->name);
-		return 0;
-	}
-
-	return 1;
-}
-
-static int _allow_extra_system_id(struct cmd_context *cmd, const char *system_id)
-{
-	const struct dm_config_node *cn;
-	const struct dm_config_value *cv;
-	const char *str;
-
-	if (!(cn = find_config_tree_array(cmd, local_extra_system_ids_CFG, NULL)))
-		return 0;
-
-	for (cv = cn->v; cv; cv = cv->next) {
-		if (cv->type == DM_CFG_EMPTY_ARRAY)
-			break;
-		/* Ignore invalid data: Warning message already issued by config.c */
-		if (cv->type != DM_CFG_STRING)
-			continue;
-		str = cv->v.str;
-		if (!*str)
-			continue;
-
-		if (!strcmp(str, system_id))
-			return 1;
-	}
-
-	return 0;
-}
-
-static int _access_vg_lock_type(struct cmd_context *cmd, struct volume_group *vg,
-				uint32_t lockd_state, uint32_t *failure)
-{
-	if (cmd->lockd_vg_disable)
-		return 1;
-
-	/*
-	 * Local VG requires no lock from lvmlockd.
-	 */
-	if (!vg_is_shared(vg))
-		return 1;
-
-	/*
-	 * 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()) {
-		/*
-	 	 * 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);
-		}
-
-		*failure |= FAILED_LOCK_TYPE;
-		return 0;
-	}
-
-	/*
-	 * 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) || cmd->lockd_vg_enforce_sh) {
-			log_error("Cannot access VG %s due to failed lock.", vg->name);
-			*failure |= FAILED_LOCK_MODE;
-			return 0;
-		}
-
-		log_warn("Reading VG %s without a lock.", vg->name);
-		return 1;
-	}
-
-	if (test_mode()) {
-		log_error("Test mode is not yet supported with lock type %s.", vg->lock_type);
-		return 0;
-	}
-
-	return 1;
-}
-
-int is_system_id_allowed(struct cmd_context *cmd, const char *system_id)
-{
-	/*
-	 * A VG without a system_id can be accessed by anyone.
-	 */
-	if (!system_id || !system_id[0])
-		return 1;
-
-	/*
-	 * Allowed if the host and VG system_id's match.
-	 */
-	if (cmd->system_id && !strcmp(cmd->system_id, system_id))
-		return 1;
-
-	/*
-	 * Allowed if a host's extra system_id matches.
-	 */
-	if (cmd->system_id && _allow_extra_system_id(cmd, system_id))
-		return 1;
-
-	/*
-	 * 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.
-	 */
-
-	return 0;
-}
-
-static int _access_vg_systemid(struct cmd_context *cmd, struct volume_group *vg)
-{
-	/*
-	 * A few commands allow read-only access to foreign VGs.
-	 */
-	if (cmd->include_foreign_vgs)
-		return 1;
-
-	if (is_system_id_allowed(cmd, vg->system_id))
-		return 1;
-
-	/*
-	 * Allow VG access if the local host has active LVs in it.
-	 */
-	if (lvs_in_vg_activated(vg)) {
-		log_warn("WARNING: Found LVs active in VG %s with foreign system ID %s.  Possible data corruption.",
-			  vg->name, vg->system_id);
-		if (cmd->include_active_foreign_vgs)
-			return 1;
-		return 0;
-	}
-
-	/*
-	 * 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.",
-			  vg->name, vg->system_id);
-		return 0;
-	}
-
-	/*
-	 * 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 system ID %s with local system ID %s.",
-			  vg->name, vg->system_id, cmd->system_id);
-		return 0;
-	}
-
-	/* Silently ignore foreign vgs. */
-
-	return 0;
-}
-
-static struct volume_group *_vg_read(struct cmd_context *cmd,
-				     const char *vgname,
-				     const char *vgid,
-				     unsigned precommitted)
-{
-	const struct format_type *fmt = cmd->fmt;
-	struct format_instance *fid = NULL;
-	struct format_instance_ctx fic;
-	struct volume_group *vg, *vg_ret = NULL;
-	struct metadata_area *mda, *mda2;
-	unsigned use_precommitted = precommitted;
-	struct device *mda_dev, *dev_ret;
-	struct cached_vg_fmtdata *vg_fmtdata = NULL;	/* Additional format-specific data about the vg */
-	int found_old_metadata = 0;
-	unsigned use_previous_vg;
-
-	log_debug_metadata("Reading VG %s %s", vgname ?: "<no name>", vgid ?: "<no vgid>");
-
-	/*
-	 * 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.
-	 *
-	 * Some commands (e.g. reporting) are fine reporting data read by
-	 * the label scan.  It doesn't matter if the devs changed between
-	 * the label scan and here, we can report what was seen in the
-	 * scan, even though it is the old state, since we will not be
-	 * making any modifications.  If the VG was being modified during
-	 * the scan, and caused us to see inconsistent metadata on the
-	 * different PVs in the VG, then we do want to rescan the devs
-	 * here to get a consistent view of the VG.  Note that we don't
-	 * know if the scan found all the PVs in the VG at this point.
-	 * We don't know that until vg_read looks at the list of PVs in
-	 * the metadata and compares that to the devices found by the scan.
-	 *
-	 * It's possible that a change made to the VG during scan was
-	 * adding or removing a PV from the VG.  In this case, the list
-	 * of devices associated with the VG in lvmcache would change
-	 * due to the rescan.
-	 *
-	 * The devs in the VG may be persistently inconsistent due to some
-	 * previous problem.  In this case, rescanning the labels here will
-	 * find the same inconsistency.  The VG repair (mistakenly done by
-	 * vg_read below) is supposed to fix that.
-	 *
-	 * FIXME: sort out the usage of the global lock (which is mixed up
-	 * with the orphan lock), and when we can tell that the global
-	 * lock is taken prior to the label scan, and still held here,
-	 * we can also skip the rescan in that case.
-	 */
-	if (!cmd->can_use_one_scan || lvmcache_scan_mismatch(cmd, vgname, vgid)) {
-		log_debug_metadata("Rescanning devices for %s", vgname);
-		lvmcache_label_rescan_vg(cmd, vgname, vgid);
-	} else {
-		log_debug_metadata("Skipped rescanning devices for %s", vgname);
-	}
-
-	/* Now determine the correct vgname if none was supplied */
-	if (!vgname && !(vgname = lvmcache_vgname_from_vgid(cmd->mem, vgid))) {
-		log_debug_metadata("Cache did not find VG name from vgid %s", 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 find VG vgid from name %s", vgname);
-		return NULL;
-	}
-
-	/*
-	 * A "format instance" is an abstraction for a VG location,
-	 * i.e. where a VG's metadata exists on disk.
-	 *
-	 * An fic (format_instance_ctx) is a temporary struct used
-	 * to create an fid (format_instance).  The fid hangs around
-	 * and is used to create a 'vg' to which it connected (vg->fid).
-	 *
-	 * The 'fic' describes a VG in terms of fmt/name/id.
-	 *
-	 * The 'fid' describes a VG in more detail than the fic,
-	 * holding information about where to find the VG metadata.
-	 *
-	 * The 'vg' describes the VG in the most detail representing
-	 * all the VG metadata.
-	 *
-	 * The fic and fid are set up by create_instance() to describe
-	 * the VG location.  This happens before the VG metadata is
-	 * assembled into the more familiar struct volume_group "vg".
-	 *
-	 * The fid has one main purpose: to keep track of the metadata
-	 * locations for a given VG.  It does this by putting 'mda'
-	 * structs on fid->metadata_areas_in_use, which specify where
-	 * metadata is located on disk.  It gets this information
-	 * (metadata locations for a specific VG) from the command's
-	 * initial label scan.  The info is passed indirectly via
-	 * lvmcache info/vginfo structs, which are created by the
-	 * label scan and then copied into fid by create_instance().
-	 *
-	 * FIXME: just use the vginfo/info->mdas lists directly instead
-	 * of copying them into the fid list.
-	 */
-
-	fic.type = FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
-	fic.context.vg_ref.vg_name = vgname;
-	fic.context.vg_ref.vg_id = vgid;
-
-	/*
-	 * Sets up the metadata areas that we need to read below.
-	 * For each info in vginfo->infos, for each mda in info->mdas,
-	 * (found during label_scan), copy the mda to fid->metadata_areas_in_use
-	 */
-	if (!(fid = fmt->ops->create_instance(fmt, &fic))) {
-		log_error("Failed to create format instance");
-		return NULL;
-	}
-
-	/*
-	 * We use the fid globally here so prevent the release_vg
-	 * call to destroy the fid - we may want to reuse it!
-	 */
-	fid->ref_count++;
-
-
-	/*
-	 * label_scan found PVs for this VG and set up lvmcache to describe the
-	 * VG/PVs that we use here to read the VG.  It created 'vginfo' for the
-	 * VG, and created an 'info' attached to vginfo for each PV.  It also
-	 * added a metadata_area struct to info->mdas for each metadata area it
-	 * found on the PV.  The info->mdas structs are copied to
-	 * fid->metadata_areas_in_use by create_instance above, and here we
-	 * read VG metadata from each of those mdas.
-	 */
-	dm_list_iterate_items(mda, &fid->metadata_areas_in_use) {
-		mda_dev = mda_get_device(mda);
-
-		/* I don't think this can happen */
-		if (!mda_dev) {
-			log_warn("Ignoring metadata for VG %s from missing dev.", vgname);
-			continue;
-		}
-
-		use_previous_vg = 0;
-
-		if (use_precommitted) {
-			log_debug_metadata("Reading VG %s precommit metadata from %s %llu",
-				 vgname, dev_name(mda_dev), (unsigned long long)mda->header_start);
-
-			vg = mda->ops->vg_read_precommit(fid, vgname, mda, &vg_fmtdata, &use_previous_vg);
-
-			if (!vg && !use_previous_vg) {
-				log_warn("WARNING: Reading VG %s precommit on %s failed.", vgname, dev_name(mda_dev));
-				vg_fmtdata = NULL;
-				continue;
-			}
-		} else {
-			log_debug_metadata("Reading VG %s metadata from %s %llu",
-				 vgname, dev_name(mda_dev), (unsigned long long)mda->header_start);
-
-			vg = mda->ops->vg_read(fid, vgname, mda, &vg_fmtdata, &use_previous_vg);
-
-			if (!vg && !use_previous_vg) {
-				log_warn("WARNING: Reading VG %s on %s failed.", vgname, dev_name(mda_dev));
-				vg_fmtdata = NULL;
-				continue;
-			}
-		}
-
-		if (!vg)
-			continue;
-
-		if (vg && !vg_ret) {
-			vg_ret = vg;
-			dev_ret = mda_dev;
-			continue;
-		}
-
-		/* 
-		 * Use the newest copy of the metadata found on any mdas.
-		 * Above, We could check if the scan found an old metadata
-		 * seqno in this mda and just skip reading it again; then these
-		 * seqno checks would just be sanity checks.
-		 */
-
-		if (vg->seqno == vg_ret->seqno) {
-			release_vg(vg);
-			continue;
-		}
-
-		if (vg->seqno > vg_ret->seqno) {
-			log_warn("WARNING: ignoring metadata seqno %u on %s for seqno %u on %s for VG %s.",
-				 vg_ret->seqno, dev_name(dev_ret),
-				 vg->seqno, dev_name(mda_dev), vg->name);
-			found_old_metadata = 1;
-			release_vg(vg_ret);
-			vg_ret = vg;
-			dev_ret = mda_dev;
-			vg_fmtdata = NULL;
-			continue;
-		}
-
-		if (vg_ret->seqno > vg->seqno) {
-			log_warn("WARNING: ignoring metadata seqno %u on %s for seqno %u on %s for VG %s.",
-				 vg->seqno, dev_name(mda_dev),
-				 vg_ret->seqno, dev_name(dev_ret), vg->name);
-			found_old_metadata = 1;
-			release_vg(vg);
-			vg_fmtdata = NULL;
-			continue;
-		}
-	}
-
-	if (found_old_metadata)
-		log_warn("WARNING: Inconsistent metadata found for VG %s", vgname);
-
-	vg = NULL;
-
-	if (vg_ret)
-		set_pv_devices(fid, vg_ret);
-
-	fid->ref_count--;
-
-	if (!vg_ret) {
-		_destroy_fid(&fid);
-		goto_out;
-	}
-
-	/*
-	 * Correct the lvmcache representation of the VG using the metadata
-	 * that we have chosen above (vg_ret).
-	 *
-	 * The vginfo/info representation created by label_scan was not
-	 * entirely correct since it did not use the full or final metadata.
-	 *
-	 * In lvmcache, PVs with no mdas were not attached to the vginfo during
-	 * label_scan because label_scan didn't know where they should go.  Now
-	 * that we have the VG metadata we can tell, so use that to attach those
-	 * info's to the vginfo.
-	 *
-	 * Also, outdated PVs that have been removed from the VG were incorrectly
-	 * attached to the vginfo during label_scan, and now need to be detached.
-	 */
-	lvmcache_update_vg_from_read(vg_ret, vg_ret->status & PRECOMMITTED);
-
-	/*
-	 * lvmcache_update_vg identified outdated mdas that we read above that
-	 * are not actually part of the VG.  Remove those outdated mdas from
-	 * the fid's list of mdas.
-	 */
-	dm_list_iterate_items_safe(mda, mda2, &fid->metadata_areas_in_use) {
-		mda_dev = mda_get_device(mda);
-		if (lvmcache_is_outdated_dev(cmd, vg_ret->name, (const char *)&vg_ret->id, mda_dev)) {
-			log_debug_metadata("vg_read %s ignore mda for outdated dev %s",
-					   vg_ret->name, dev_name(mda_dev));
-			dm_list_del(&mda->list);
-		}
-	}
-
-out:
-	return vg_ret;
-}
-
-struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const char *vgid,
-			     uint32_t read_flags, uint32_t lockd_state,
-			     uint32_t *error_flags, struct volume_group **error_vg)
-{
-	char uuidstr[64] __attribute__((aligned(8)));
-	struct volume_group *vg = NULL;
-	struct lv_list *lvl;
-	struct pv_list *pvl;
-	int missing_pv_dev = 0;
-	int missing_pv_flag = 0;
-	uint32_t failure = 0;
-	int writing = (read_flags & READ_FOR_UPDATE);
-
-	/*
-	 * FIXME: is this function still used to read orphans?
-	 * If so, replace any callers with vg_read_orphans.
-	 */
-	if (is_orphan_vg(vg_name)) {
-		int skip_lock = read_flags & PROCESS_SKIP_ORPHAN_LOCK;
-		log_very_verbose("Reading orphan VG %s", vg_name);
-
-		if (!skip_lock && !lock_vol(cmd, vg_name, LCK_VG_READ, NULL))
-			return_NULL;
-
-		vg = vg_read_orphans(cmd, vg_name);
-
-		if (!skip_lock)
-			unlock_vg(cmd, vg, vg_name);
-
-		*error_flags = 0;
-		*error_vg = NULL;
-		return vg;
-	}
-
-	if (!validate_name(vg_name)) {
-		log_error("Volume group name \"%s\" has invalid characters.", vg_name);
-		return NULL;
-	}
-
-	if (!lock_vol(cmd, vg_name, writing ? LCK_VG_WRITE : LCK_VG_READ, NULL)) {
-		log_error("Can't get lock for %s", vg_name);
-		failure |= FAILED_LOCKING;
-		goto_bad;
-	}
-
-	if (!(vg = _vg_read(cmd, vg_name, vgid, 0))) {
-		/* Some callers don't care if the VG doesn't exist and don't want an error message. */
-		if (!(read_flags & READ_OK_NOTFOUND))
-			log_error("Volume group \"%s\" not found", vg_name);
-		failure |= FAILED_NOTFOUND;
-		goto_bad;
-	}
-
-	/*
-	 * Check and warn if PV ext info is not in sync with VG metadata
-	 * (vg_write fixes.)
-	 */
-	_check_pv_ext(cmd, vg);
-
-	if (!vg_strip_outdated_historical_lvs(vg))
-		log_warn("WARNING: failed to strip outdated historical lvs.");
-
-	/*
-	 * Check for missing devices in the VG.  In most cases a VG cannot be
-	 * changed while it's missing devices.  This restriction is implemented
-	 * here in vg_read.  Below we return an error from vg_read if the
-	 * vg_read flag indicates that the command is going to modify the VG.
-	 * (We should probably implement this restriction elsewhere instead of
-	 * returning an error from vg_read.)
-	 *
-	 * The PV's device may be present while the PV for the device has the
-	 * MISSING_PV flag set in the metadata.  This happened because the VG
-	 * was written while this dev was missing, so the MISSING flag was
-	 * written in the metadata for PV.  Now the device has reappeared.
-	 * However, the VG has changed since the device was last present, and
-	 * if the device has outdated data it may not be safe to just start
-	 * using it again.
-	 *
-	 * If there were no PE's used on the PV, we can just clear the MISSING
-	 * flag, but if there were PE's used we need to continue to treat the
-	 * PV as if the device is missing, limiting operations like the VG has
-	 * a missing device, and requiring the user to remove the reappeared
-	 * device from the VG, like a missing device, with vgreduce
-	 * --removemissing.
-	 */
-	dm_list_iterate_items(pvl, &vg->pvs) {
-		if (!id_write_format(&pvl->pv->id, uuidstr, sizeof(uuidstr)))
-			uuidstr[0] = '\0';
-
-		if (!pvl->pv->dev) {
-			/* The obvious and common case of a missing device. */
-
-			log_warn("WARNING: VG %s is missing PV %s.", vg_name, uuidstr);
-			missing_pv_dev++;
-
-		} else if (pvl->pv->status & MISSING_PV) {
-			/* A device that was missing but has reappeared. */
-
-			if (pvl->pv->pe_alloc_count == 0) {
-				log_warn("WARNING: VG %s has unused reappeared PV %s %s.", vg_name, dev_name(pvl->pv->dev), uuidstr);
-				pvl->pv->status &= ~MISSING_PV;
-				/* tell vgextend restoremissing that MISSING flag was cleared here */
-				pvl->pv->unused_missing_cleared = 1;
-			} else {
-				log_warn("WARNING: VG %s was missing PV %s %s.", vg_name, dev_name(pvl->pv->dev), uuidstr);
-				missing_pv_flag++;
-			}
-		}
-	}
-
-	if (missing_pv_dev || missing_pv_flag)
-		vg_mark_partial_lvs(vg, 1);
-
-	if (!check_pv_segments(vg)) {
-		log_error(INTERNAL_ERROR "PV segments corrupted in %s.", vg->name);
-		failure |= FAILED_INTERNAL_ERROR;
-		goto_bad;
-	}
-
-	dm_list_iterate_items(lvl, &vg->lvs) {
-		if (!check_lv_segments(lvl->lv, 0)) {
-			log_error(INTERNAL_ERROR "LV segments corrupted in %s.", lvl->lv->name);
-			failure |= FAILED_INTERNAL_ERROR;
-			goto_bad;
-		}
-	}
-
-	dm_list_iterate_items(lvl, &vg->lvs) {
-		/* Checks that cross-reference other LVs. */
-		if (!check_lv_segments(lvl->lv, 1)) {
-			log_error(INTERNAL_ERROR "LV segments corrupted in %s.", lvl->lv->name);
-			failure |= FAILED_INTERNAL_ERROR;
-			goto_bad;
-		}
-	}
-
-	if (!check_pv_dev_sizes(vg))
-		log_warn("WARNING: One or more devices used as PVs in VG %s have changed sizes.", vg->name);
-
-	_check_devs_used_correspond_with_vg(vg);
-
-	if (!_access_vg_lock_type(cmd, vg, lockd_state, &failure)) {
-		/* Either FAILED_LOCK_TYPE or FAILED_LOCK_MODE were set. */
-		goto_bad;
-	}
-
-	if (!_access_vg_systemid(cmd, vg)) {
-		failure |= FAILED_SYSTEMID;
-		goto_bad;
-	}
-
-	if (!_access_vg_clustered(cmd, vg)) {
-		failure |= FAILED_CLUSTERED;
-		goto_bad;
-	}
-
-	if (writing && !(read_flags & READ_ALLOW_EXPORTED) && vg_is_exported(vg)) {
-		log_error("Volume group %s is exported", vg->name);
-		failure |= FAILED_EXPORTED;
-		goto_bad;
-	}
-
-	if (writing && !(vg->status & LVM_WRITE)) {
-		log_error("Volume group %s is read-only", vg->name);
-		failure |= FAILED_READ_ONLY;
-		goto_bad;
-	}
-
-	if (!cmd->handles_missing_pvs && (missing_pv_dev || missing_pv_flag) && writing) {
-		log_error("Cannot change VG %s while PVs are missing.", vg->name);
-		log_error("See vgreduce --removemissing and vgextend --restoremissing.");
-		failure |= FAILED_NOT_ENABLED;
-		goto_bad;
-	}
-
-	if (!cmd->handles_unknown_segments && vg_has_unknown_segments(vg) && writing) {
-		log_error("Cannot change VG %s with unknown segments in it!", vg->name);
-		failure |= FAILED_NOT_ENABLED; /* FIXME new failure code here? */
-		goto_bad;
-	}
-
-	/*
-	 * When we are reading the VG with the intention of writing it,
-	 * we save a second copy of the VG in vg->vg_committed.  This
-	 * copy remains unmodified by the command operation, and is used
-	 * later if there is an error and we want to reactivate LVs.
-	 * FIXME: be specific about exactly when this works correctly.
-	 */
-	if (writing) {
-		struct dm_config_tree *cft;
-
-		if (dm_pool_locked(vg->vgmem)) {
-			/* FIXME: can this happen? */
-			log_warn("WARNING: vg_read no vg copy: pool locked");
-			goto out;
-		}
-
-		if (vg->vg_committed) {
-			/* FIXME: can this happen? */
-			log_warn("WARNING: vg_read no vg copy: copy exists");
-			release_vg(vg->vg_committed);
-			vg->vg_committed = NULL;
-		}
-
-		if (vg->vg_precommitted) {
-			/* FIXME: can this happen? */
-			log_warn("WARNING: vg_read no vg copy: pre copy exists");
-			release_vg(vg->vg_precommitted);
-			vg->vg_precommitted = NULL;
-		}
-
-		if (!(cft = export_vg_to_config_tree(vg))) {
-			log_warn("WARNING: vg_read no vg copy: copy export failed");
-			goto out;
-		}
-
-		if (!(vg->vg_committed = import_vg_from_config_tree(cft, vg->fid)))
-			log_warn("WARNING: vg_read no vg copy: copy import failed");
-
-		dm_config_destroy(cft);
-	} else {
-		if (vg->vg_precommitted)
-			log_error(INTERNAL_ERROR "vg_read vg %p vg_precommitted %p", vg, vg->vg_precommitted);
-		if (vg->vg_committed)
-			log_error(INTERNAL_ERROR "vg_read vg %p vg_committed %p", vg, vg->vg_committed);
-	}
-out:
-	/* We return with the VG lock held when read is successful. */
-	*error_flags = SUCCESS;
-	if (error_vg)
-		*error_vg = NULL;
-	return vg;
-
-bad:
-	*error_flags = failure;
-
-	/*
-	 * FIXME: get rid of this case so we don't have to return the vg when
-	 * there's an error.  It is here for process_each_pv() which wants to
-	 * eliminate the VG's devs from the list of devs it is processing, even
-	 * when it can't access the VG because of wrong system id or similar.
-	 * This could be done by looking at lvmcache info structs intead of 'vg'.
-	 * It's also used by process_each_vg/process_each_lv which want to
-	 * include error_vg values (like system_id) in error messages.
-	 * These values could also be found from lvmcache vginfo.
-	 */
-	if (error_vg && vg) {
-		if (vg->vg_precommitted)
-			log_error(INTERNAL_ERROR "vg_read vg %p vg_precommitted %p", vg, vg->vg_precommitted);
-		if (vg->vg_committed)
-			log_error(INTERNAL_ERROR "vg_read vg %p vg_committed %p", vg, vg->vg_committed);
-
-		/* caller must unlock_vg and release_vg */
-		*error_vg = vg;
-		return_NULL;
-	}
-
-	if (vg) {
-		unlock_vg(cmd, vg, vg_name);
-		release_vg(vg);
-	}
-	if (error_vg)
-		*error_vg = NULL;
-	return_NULL;
-}
-
-/*
- * Simply a version of vg_read() that automatically sets the READ_FOR_UPDATE
- * flag, which means the caller intends to write the VG after reading it,
- * so vg_read should acquire an exclusive file lock on the vg.
- */
-struct volume_group *vg_read_for_update(struct cmd_context *cmd, const char *vg_name,
-			 const char *vgid, uint32_t read_flags, uint32_t lockd_state)
-{
-	struct volume_group *vg;
-	uint32_t error_flags = 0;
-
-	vg = vg_read(cmd, vg_name, vgid, read_flags | READ_FOR_UPDATE, lockd_state, &error_flags, NULL);
-
-	return vg;
-}

lib/metadata/vg.c

@@ -84,7 +84,7 @@ static void _free_vg(struct volume_group *vg)
 
 void release_vg(struct volume_group *vg)
 {
-	if (!vg || is_orphan_vg(vg->name))
+	if (!vg || (vg->fid && vg == vg->fid->fmt->orphan_vg))
 		return;
 
 	release_vg(vg->vg_committed);
@@ -711,9 +711,9 @@ int vgreduce_single(struct cmd_context *cmd, struct volume_group *vg,
 	vg->extent_count -= pv_pe_count(pv);
 
 	/* FIXME: we don't need to vg_read the orphan vg here */
-	orphan_vg = vg_read_orphans(cmd, vg->fid->fmt->orphan_vg_name);
+	orphan_vg = vg_read_orphans(cmd, 0, vg->fid->fmt->orphan_vg_name);
 
-	if (!orphan_vg)
+	if (vg_read_error(orphan_vg))
 		goto bad;
 
 	if (!vg_split_mdas(cmd, vg, orphan_vg) || !vg->pv_count) {

lib/metadata/vg.h

@@ -122,6 +122,11 @@ struct volume_group {
 	struct dm_list removed_pvs;
 	uint32_t open_mode; /* FIXME: read or write - check lock type? */
 
+	/*
+	 * Store result of the last vg_read().
+	 * 0 for success else appropriate FAILURE_* bits set.
+	 */
+	uint32_t read_status;
 	uint32_t mda_copies; /* target number of mdas for this VG */
 
 	struct dm_hash_table *hostnames; /* map of creation hostnames */

lib/misc/lvm-flock.c

@@ -56,6 +56,20 @@ static void _undo_flock(const char *file, int fd)
 		log_sys_debug("close", file);
 }
 
+static struct lock_list *_get_lock_list_entry(const char *file)
+{
+	struct lock_list *ll;
+	struct dm_list *llh;
+
+	dm_list_iterate(llh, &_lock_list) {
+		ll = dm_list_item(llh, struct lock_list);
+
+		if (!strcmp(ll->res, file))
+			return ll;
+	}
+	return NULL;
+}
+
 static int _release_lock(const char *file, int unlock)
 {
 	struct lock_list *ll;
@@ -167,8 +181,8 @@ int lock_file(const char *file, uint32_t flags)
 {
 	int operation;
 	uint32_t nonblock = flags & LCK_NONBLOCK;
+	uint32_t convert = flags & LCK_CONVERT;
 	int r;
-
 	struct lock_list *ll;
 	char state;
 
@@ -188,6 +202,20 @@ int lock_file(const char *file, uint32_t flags)
 		return 0;
 	}
 
+	if (convert) {
+		if (nonblock)
+			operation |= LOCK_NB;
+		if (!(ll = _get_lock_list_entry(file)))
+			return 0;
+		log_very_verbose("Locking %s %c%c convert", ll->res, state,
+			 	 nonblock ? ' ' : 'B');
+		r = flock(ll->lf, operation);
+		if (!r)
+			return 1;
+		log_error("Failed to convert flock on %s %d", file, errno);
+		return 0;
+	}
+
 	if (!(ll = malloc(sizeof(struct lock_list))))
 		return_0;
 

lib/writecache/writecache.c

@@ -26,8 +26,6 @@
 #include "lib/metadata/lv_alloc.h"
 #include "lib/config/defaults.h"
 
-static const char _writecache_module[] = "writecache";
-
 #define SEG_LOG_ERROR(t, p...) \
         log_error(t " segment %s of logical volume %s.", ## p,	\
                   dm_config_parent_name(sn), seg->lv->name), 0;

libdaemon/server/daemon-server.c

@@ -85,7 +85,7 @@ static int _is_idle(daemon_state s)
 	return s.idle && s.idle->is_idle && !s.threads->next;
 }
 
-static struct timeval *_get_timeout(daemon_state s)
+static struct timespec *_get_timeout(daemon_state s)
 {
 	return s.idle ? s.idle->ptimeout : NULL;
 }
@@ -94,7 +94,7 @@ static void _reset_timeout(daemon_state s)
 {
 	if (s.idle) {
 		s.idle->ptimeout->tv_sec = 1;
-		s.idle->ptimeout->tv_usec = 0;
+		s.idle->ptimeout->tv_nsec = 0;
 	}
 }
 
@@ -559,6 +559,8 @@ void daemon_start(daemon_state s)
 	thread_state _threads = { .next = NULL };
 	unsigned timeout_count = 0;
 	fd_set in;
+	sigset_t new_set, old_set;
+	int ret;
 
 	/*
 	 * Switch to C locale to avoid reading large locale-archive file used by
@@ -626,8 +628,7 @@ void daemon_start(daemon_state s)
 	if (!s.foreground)
 		kill(getppid(), SIGTERM);
 
-	/*
-	 * Use daemon_main for daemon-specific init and polling, or
+	/*                                                                               	 * Use daemon_main for daemon-specific init and polling, or
 	 * use daemon_init for daemon-specific init and generic lib polling.
 	 */
 
@@ -641,22 +642,39 @@ void daemon_start(daemon_state s)
 		if (!s.daemon_init(&s))
 			failed = 1;
 
+	if (s.socket_fd >= FD_SETSIZE)
+		failed = 1; /* FD out of available selectable set */
+
+	sigfillset(&new_set);
+	sigprocmask(SIG_SETMASK, NULL, &old_set);
+
 	while (!failed) {
 		_reset_timeout(s);
 		FD_ZERO(&in);
 		FD_SET(s.socket_fd, &in);
-		if (select(FD_SETSIZE, &in, NULL, NULL, _get_timeout(s)) < 0 && errno != EINTR)
-			perror("select error");
+
+		sigprocmask(SIG_SETMASK, &new_set, NULL);
+		if (_shutdown_requested && !s.threads->next) {
+			sigprocmask(SIG_SETMASK, &old_set, NULL);
+			INFO(&s, "%s shutdown requested", s.name);
+			break;
+		}
+		ret = pselect(s.socket_fd + 1, &in, NULL, NULL, _get_timeout(s), &old_set);
+		sigprocmask(SIG_SETMASK, &old_set, NULL);
+
+		if (ret < 0) {
+			if (errno != EINTR && errno != EAGAIN &&
+			    (EWOULDBLOCK == EAGAIN || errno != EWOULDBLOCK))
+				perror("select error");
+			continue;
+		}
+
 		if (FD_ISSET(s.socket_fd, &in)) {
 			timeout_count = 0;
 			_handle_connect(s);
 		}
-
 		_reap(s, 0);
 
-		if (_shutdown_requested && !s.threads->next)
-			break;
-
 		/* s.idle == NULL equals no shutdown on timeout */
 		if (_is_idle(s)) {
 			DEBUGLOG(&s, "timeout occured");

libdaemon/server/daemon-server.h

@@ -47,7 +47,7 @@ struct timeval;
 typedef struct {
 	volatile unsigned is_idle;
 	unsigned max_timeouts;
-	struct timeval *ptimeout;
+	struct timespec *ptimeout;
 } daemon_idle;
 
 struct daemon_state;

libdm/libdm-config.c

@@ -51,6 +51,8 @@ struct parser {
 
 	struct dm_pool *mem;
 	int no_dup_node_check;	/* whether to disable dup node checking */
+	const char *key;        /* last obtained key */
+	unsigned ignored_creation_time;
 };
 
 struct config_output {
@@ -176,7 +178,7 @@ static int _do_dm_config_parse(struct dm_config_tree *cft, const char *start, co
 	/* TODO? if (start == end) return 1; */
 
 	struct parser *p;
-	if (!(p = dm_pool_alloc(cft->mem, sizeof(*p))))
+	if (!(p = dm_pool_zalloc(cft->mem, sizeof(*p))))
 		return_0;
 
 	p->mem = cft->mem;
@@ -615,6 +617,7 @@ static struct dm_config_node *_section(struct parser *p, struct dm_config_node *
 		match(TOK_SECTION_E);
 	} else {
 		match(TOK_EQ);
+		p->key = root->key;
 		if (!(value = _value(p)))
 			return_NULL;
 		if (root->v)
@@ -682,8 +685,17 @@ static struct dm_config_value *_type(struct parser *p)
 		errno = 0;
 		v->v.i = strtoll(p->tb, NULL, 0);	/* FIXME: check error */
 		if (errno) {
-			log_error("Failed to read int token.");
-			return NULL;
+			if (errno == ERANGE && p->key &&
+			    strcmp("creation_time", p->key) == 0) {
+				/* Due to a bug in some older 32bit builds (<2.02.169),
+				 * lvm was able to produce invalid creation_time string */
+				v->v.i = 1527120000; /* Pick 2018-05-24 day instead */
+				if (!p->ignored_creation_time++)
+					log_warn("WARNING: Invalid creation_time found in metadata (repaired with next metadata update).");
+			} else {
+				log_error("Failed to read int token.");
+				return NULL;
+			}
 		}
 		match(TOK_INT);
 		break;

man/pvck.8_des

@@ -1 +1,8 @@
-pvck checks the LVM metadata for consistency on PVs.
+pvck checks LVM metadata on PVs.
+
+Use the --dump option to extract metadata from PVs for debugging.
+With dump, set --pvmetadatacopies 2 to extract metadata from a
+second metadata area at the end of the device. Use the --file
+option to save the raw metadata to a specified file. (The raw
+metadata is not usable with vgcfgbackup and vgcfgrestore.)
+

test/Makefile.in

@@ -49,9 +49,9 @@ include $(top_builddir)/make.tmpl
 T ?= .
 S ?= @ # never match anything by default
 VERBOSE ?= 0
-ALL := $(shell find -L $(srcdir) \( -path \*/shell/\*.sh -or -path \*/unit/\*.sh \) | $(SORT))
+ALL := $(shell find -L $(srcdir) \( -path \*/shell/\*.sh -or -path \*/api/\*.sh -or -path \*/unit/\*.sh \) | $(SORT))
 comma = ,
-RUN := $(shell find -L $(srcdir) -regextype posix-egrep \( -path \*/shell/\*.sh -or -path \*/unit/\*.sh \) -and -regex "$(srcdir)/.*($(subst $(comma),|,$(T))).*" -and -not -regex "$(srcdir)/.*($(subst $(comma),|,$(S))).*" | $(SORT))
+RUN := $(shell find -L $(srcdir) -regextype posix-egrep \( -path \*/shell/\*.sh -or -path \*/api/\*.sh -or -path \*/unit/\*.sh \) -and -regex "$(srcdir)/.*($(subst $(comma),|,$(T))).*" -and -not -regex "$(srcdir)/.*($(subst $(comma),|,$(S))).*" | $(SORT))
 RUN_BASE = $(subst $(srcdir)/,,$(RUN))
 
 ifeq ("@BUILD_LVMPOLLD@", "yes")
@@ -200,8 +200,9 @@ LIB_SHARED = check aux inittest utils get lvm-wrapper
 
 install: .tests-stamp lib/paths-installed
 	@echo $(srcdir)
-	$(Q) $(INSTALL_DIR) $(DATADIR)/{shell,unit,lib,dbus} $(EXECDIR)
+	$(Q) $(INSTALL_DIR) $(DATADIR)/{shell,api,unit,lib,dbus} $(EXECDIR)
 	$(Q) $(INSTALL_DATA) shell/*.sh $(DATADIR)/shell
+	$(INSTALL_DATA) api/*.sh $(DATADIR)/api
 	$(Q) $(INSTALL_DATA) unit/*.sh $(DATADIR)/unit
 	-$(Q) $(INSTALL_PROGRAM) unit/unit-test $(DATADIR)/unit
 	-$(Q) $(INSTALL_PROGRAM) dbus/*.py $(DATADIR)/dbus/
@@ -291,7 +292,7 @@ lib/paths-installed: lib/paths-common
 	$(Q) $(RM) $@-t
 	$(Q) cat lib/paths-common > $@-t
 	$(Q) echo 'installed_testsuite=1' >> $@-t
-	$(Q) echo 'export PATH=@libexecdir@/lvm2-testsuite:@datadir@/lvm2-testsuite/lib:$$PATH' >> $@-t
+	$(Q) echo 'export PATH=@libexecdir@/lvm2-testsuite:@datadir@/lvm2-testsuite/lib:@datadir@/lvm2-testsuite/api:$$PATH' >> $@-t
 	$(Q) mv $@-t $@
 
 lib/paths: lib/paths-common

test/shell/creation-time.sh

@@ -0,0 +1,40 @@
+#!/usr/bin/env bash
+
+# Copyright (C) 2019 Red Hat, Inc. All rights reserved.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Check we can read metadata with out-of-range creation time
+
+# Due to a bug in 32-bit version lvm2 <2.02.169  produced metadata
+# contained invalid number for creation_time
+
+SKIP_WITH_LVMPOLLD=1
+
+. lib/inittest
+
+aux prepare_vg 1
+
+lvcreate -an -L1 -n $lv1 $vg
+
+vgcfgbackup -f back $vg
+
+sed -e 's/creation_time = \(.*\)$/creation_time = 12029933779523993599/g' back >backnew
+
+vgcfgrestore -f backnew $vg  |& tee err
+
+# Check the time was spotted
+grep Invalid err
+
+vgcfgbackup -f back $vg |& tee err
+
+# Check the time is not a problem anymore
+not grep Invalid err
+
+vgremove -ff $vg

test/shell/fsadm-crypt.sh

@@ -46,7 +46,14 @@ PWD2="mymJeD8ivEhE"
 PWD3="ocMakf3fAcQO"
 SKIP_DETACHED=
 
-which cryptsetup || check_cryptsetup=${check_cryptsetup:-cryptsetup}
+if which cryptsetup ; then
+	# use older format luks1 - otherwise the test would need to pass password everywhere...
+	case $(cryptsetup --version) in
+	"cryptsetup 2"*)  FORMAT_PARAMS="$FORMAT_PARAMS --type luks1" ;;
+	esac
+else
+	check_cryptsetup=${check_cryptsetup:-cryptsetup}
+fi
 
 which mkfs.ext2 || check_ext2=${check_ext2:-mkfs.ext2}
 which mkfs.ext3 || check_ext3=${check_ext3:-mkfs.ext3}
@@ -130,7 +137,7 @@ get_crypt_kname() {
 # $1 device
 # $2 pass
 crypt_format() {
-	echo "$2" | cryptsetup luksFormat "$FORMAT_PARAMS" "$1"
+	echo "$2" | cryptsetup luksFormat $FORMAT_PARAMS "$1"
 }
 
 
@@ -261,7 +268,7 @@ test_ext3_small_shrink() {
 }
 
 test_xfs_resize() {
-	mkfs.xfs -l internal,size=1000b -f "$3"
+	mkfs.xfs -l internal,size=1536b -f "$3"
 
 	fsadm --lvresize resize $1 30M
 	# Fails - not enough space for 4M fs
@@ -276,7 +283,7 @@ test_xfs_resize() {
 }
 
 test_xfs_small_shrink() {
-	mkfs.xfs -l internal,size=1000b -f "$3"
+	mkfs.xfs -l internal,size=1536b -f "$3"
 
 	not lvresize -L-1 -r $1
 	fscheck_xfs "$3"
@@ -340,7 +347,7 @@ test_ext3_inactive() {
 
 test_xfs_inactive() {
 	crypt_open "$2" $PWD2 "$4"
-	mkfs.xfs -l internal,size=1000b -f "$3"
+	mkfs.xfs -l internal,size=1536b -f "$3"
 	crypt_close "$4"
 
 	not fsadm --lvresize resize $1 30M
@@ -421,7 +428,7 @@ test_ext3_plain() {
 }
 
 test_xfs_plain() {
-	mkfs.xfs -l internal,size=1000b -f "$3"
+	mkfs.xfs -l internal,size=1536b -f "$3"
 
 	not fsadm --lvresize resize $1 30M
 	not lvresize -L+10M -r $1
@@ -487,7 +494,7 @@ test_ext3_detached() {
 }
 
 test_xfs_detached() {
-	mkfs.xfs -l internal,size=1000b -f "$3"
+	mkfs.xfs -l internal,size=1536b -f "$3"
 
 	not fsadm --lvresize resize $1 30M
 	not lvresize -L+10M -r $1

test/shell/hints.sh

@@ -53,12 +53,15 @@ grep -v -E "$dev1|$dev2" $HINTS > tmptest
 not grep scan: tmptest
 
 # test that 'pvs' submits only two reads, one for each PV in hints
+
+if [ -e "/usr/bin/strace" ]; then
 strace -e io_submit pvs 2>&1|tee tmptest
 test "$(grep io_submit tmptest | wc -l)" -eq 2
 
 # test that 'pvs -a' submits six reads, one for each device
 strace -e io_submit pvs -a 2>&1|tee tmptest
 test "$(grep io_submit tmptest | wc -l)" -eq 6
+fi
 
 #
 # vg2 uses dev3,dev4

test/shell/inconsistent-metadata.sh

@@ -24,48 +24,53 @@ lvchange -a n $vg/mirror
 
 aux backup_dev "${DEVICES[@]}"
 
-makeold() {
-	# reset metadata on all devs to starting condition
+init() {
 	aux restore_dev "${DEVICES[@]}"
 	not check lv_field $vg/resized lv_size "8.00m"
-	# change the metadata on all devs
 	lvresize -L 8192K $vg/resized
-	# reset metadata on just dev1 to the previous version
 	aux restore_dev "$dev1"
 }
 
-# create old metadata
-makeold
-
-# reports old metadata
-vgs $vg 2>&1 | tee cmd.out
-grep "ignoring metadata" cmd.out
+init
+vgscan 2>&1 | tee cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
+vgscan 2>&1 | tee cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
 check lv_field $vg/resized lv_size "8.00m"
 
-# corrects old metadata
-lvcreate -l1 -an $vg
-
-# no old report
-vgs $vg 2>&1 | tee cmd.out
-not grep "ignoring metadata" cmd.out
+# vgdisplay fixes
+init
+vgdisplay $vg 2>&1 | tee cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
+vgdisplay $vg 2>&1 | tee cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
 check lv_field $vg/resized lv_size "8.00m"
 
+# lvs fixes up
+init
+lvs $vg 2>&1 | tee cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
+vgdisplay $vg 2>&1 | tee cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
+check lv_field $vg/resized lv_size "8.00m"
 
-echo Check auto-repair of failed vgextend
-echo - metadata written to original pv but not new pv
+# vgs fixes up as well
+init
+vgs $vg 2>&1 | tee cmd.out
+grep "Inconsistent metadata found for VG $vg" cmd.out
+vgs $vg 2>&1 | tee cmd.out
+not grep "Inconsistent metadata found for VG $vg" cmd.out
+check lv_field $vg/resized lv_size "8.00m"
 
+echo Check auto-repair of failed vgextend - metadata written to original pv but not new pv
 vgremove -f $vg
 pvremove -ff "${DEVICES[@]}"
 pvcreate "${DEVICES[@]}"
-
 aux backup_dev "$dev2"
 vgcreate $SHARED $vg "$dev1"
 vgextend $vg "$dev2"
 aux restore_dev "$dev2"
-
-vgs -o+vg_mda_count $vg
-pvs -o+vg_mda_count
-
+vgscan
 should check compare_fields vgs $vg vg_mda_count pvs "$dev2" vg_mda_count
 
 vgremove -ff $vg

test/shell/lock-blocking.sh

@@ -21,26 +21,26 @@ aux prepare_devs 3
 pvcreate "$dev1" "$dev2"
 vgcreate $SHARED $vg "$dev1" "$dev2"
 
-# if wait_for_locks set, vgremove should wait for orphan lock
+# if wait_for_locks set, vgremove should wait for global lock
 # flock process should have exited by the time first vgremove completes
-flock -w 5 "$TESTDIR/var/lock/lvm/P_orphans" sleep 10 &
-while ! test -f "$TESTDIR/var/lock/lvm/P_orphans" ; do sleep .1 ; done
+flock -w 5 "$TESTDIR/var/lock/lvm/P_global" sleep 10 &
+while ! test -f "$TESTDIR/var/lock/lvm/P_global" ; do sleep .1 ; done
 
 vgremove --config 'global { wait_for_locks = 1 }' $vg
 not vgremove --config 'global { wait_for_locks = 1 }' $vg
 
-test ! -f "$TESTDIR/var/lock/lvm/P_orphans"
+test ! -f "$TESTDIR/var/lock/lvm/P_global"
 
 # if wait_for_locks not set, vgremove should fail on non-blocking lock
 # we must wait for flock process at the end - vgremove won't wait
 vgcreate $SHARED $vg "$dev1" "$dev2"
-flock -w 5 "$TESTDIR/var/lock/lvm/P_orphans" sleep 10 &
+flock -w 5 "$TESTDIR/var/lock/lvm/P_global" sleep 10 &
 
-while ! test -f "$TESTDIR/var/lock/lvm/P_orphans" ; do sleep .1 ; done
+while ! test -f "$TESTDIR/var/lock/lvm/P_global" ; do sleep .1 ; done
 flock_pid=$(jobs -p)
 
 not vgremove --config 'global { wait_for_locks = 0 }' $vg
-test -f "$TESTDIR/var/lock/lvm/P_orphans" # still running
+test -f "$TESTDIR/var/lock/lvm/P_global" # still running
 kill "$flock_pid"
 
 vgremove -ff $vg

test/shell/lvconvert-raid-reshape-linear_to_raid6-single-type.sh

@@ -32,7 +32,8 @@ lvcreate -aey -L 16M -n $lv $vg
 check lv_field $vg/$lv segtype "linear"
 check lv_field $vg/$lv stripes 1
 check lv_field $vg/$lv data_stripes 1
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
+wipefs -a $DM_DEV_DIR/$vg/$lv
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
 fsck -fn $DM_DEV_DIR/$vg/$lv
 
 # Convert linear -> raid1 (takeover)

test/shell/lvconvert-raid-reshape-linear_to_striped-single-type.sh

@@ -31,7 +31,8 @@ lvcreate -aey -L 16M -n $lv $vg
 check lv_field $vg/$lv segtype "linear"
 check lv_field $vg/$lv stripes 1
 check lv_field $vg/$lv data_stripes 1
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
+wipefs -a $DM_DEV_DIR/$vg/$lv
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
 fsck -fn $DM_DEV_DIR/$vg/$lv
 
 # Convert linear -> raid1

test/shell/lvconvert-raid-reshape-linear_to_striped.sh

@@ -30,7 +30,8 @@ lvcreate -aey -L 16M -n $lv1 $vg
 check lv_field $vg/$lv1 segtype "linear"
 check lv_field $vg/$lv1 stripes 1
 check lv_field $vg/$lv1 data_stripes 1
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv1
+wipefs -a $DM_DEV_DIR/$vg/$lv1
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv1
 fsck -fn $DM_DEV_DIR/$vg/$lv1
 
 # Convert linear -> raid1

test/shell/lvconvert-raid-reshape-load.sh

@@ -44,7 +44,8 @@ lvcreate --yes --type raid5_ls --stripes 13 -L190M -n$lv1 $vg
 check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
 check lv_first_seg_field $vg/$lv1 data_stripes 13
 check lv_first_seg_field $vg/$lv1 stripes 14
-echo y|mkfs -t ext4 /dev/$vg/$lv1
+wipefs -a /dev/$vg/$lv1
+mkfs -t ext4 /dev/$vg/$lv1
 aux wait_for_sync $vg $lv1
 
 mkdir -p $mount_dir

test/shell/lvconvert-raid-reshape-striped_to_linear-single-type.sh

@@ -32,7 +32,8 @@ check lv_first_seg_field $vg/$lv segtype "striped"
 check lv_first_seg_field $vg/$lv stripes 4
 check lv_first_seg_field $vg/$lv data_stripes 4
 check lv_first_seg_field $vg/$lv stripesize "64.00k"
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
+wipefs -a $DM_DEV_DIR/$vg/$lv
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
 fsck -fn $DM_DEV_DIR/$vg/$lv
 lvextend -y -L64M $DM_DEV_DIR/$vg/$lv
 

test/shell/lvconvert-raid-reshape-striped_to_linear.sh

@@ -33,7 +33,8 @@ check lv_field $vg/$lv1 data_stripes 4
 check lv_field $vg/$lv1 stripes 4
 check lv_field $vg/$lv1 stripesize "32.00k"
 check lv_field $vg/$lv1 reshape_len_le ""
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv1
+wipefs -a $DM_DEV_DIR/$vg/$lv1
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv1
 fsck -fn $DM_DEV_DIR/$vg/$lv1
 
 # Convert striped -> raid5(_n)

test/shell/lvconvert-raid-reshape-stripes-load-fail.sh

@@ -45,7 +45,8 @@ check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
 check lv_first_seg_field $vg/$lv1 stripesize "64.00k"
 check lv_first_seg_field $vg/$lv1 data_stripes 10
 check lv_first_seg_field $vg/$lv1 stripes 11
-echo y|mkfs -t ext4 /dev/$vg/$lv1
+wipefs -a /dev/$vg/$lv1
+mkfs -t ext4 /dev/$vg/$lv1
 fsck -fn /dev/$vg/$lv1
 
 mkdir -p $mount_dir

test/shell/lvconvert-raid-reshape-stripes-load-reload.sh

@@ -48,7 +48,8 @@ check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
 check lv_first_seg_field $vg/$lv1 stripesize "64.00k"
 check lv_first_seg_field $vg/$lv1 data_stripes 10
 check lv_first_seg_field $vg/$lv1 stripes 11
-echo y|mkfs -t ext4 /dev/$vg/$lv1
+wipefs -a /dev/$vg/$lv1
+mkfs -t ext4 /dev/$vg/$lv1
 
 mkdir -p "$mount_dir"
 mount "$DM_DEV_DIR/$vg/$lv1" "$mount_dir"

test/shell/lvconvert-raid-reshape-stripes-load.sh

@@ -45,7 +45,8 @@ check lv_first_seg_field $vg/$lv1 segtype "raid5_ls"
 check lv_first_seg_field $vg/$lv1 stripesize "64.00k"
 check lv_first_seg_field $vg/$lv1 data_stripes 10
 check lv_first_seg_field $vg/$lv1 stripes 11
-echo y|mkfs -t ext4 /dev/$vg/$lv1
+wipefs -a /dev/$vg/$lv1
+mkfs -t ext4 /dev/$vg/$lv1
 
 mkdir -p $mount_dir
 mount "$DM_DEV_DIR/$vg/$lv1" $mount_dir

test/shell/lvconvert-raid-restripe-linear.sh

@@ -29,7 +29,8 @@ lvcreate -aey -L 16M -n $lv $vg
 check lv_field $vg/$lv segtype "linear"
 check lv_field $vg/$lv stripes 1
 check lv_field $vg/$lv data_stripes 1
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
+wipefs -a $DM_DEV_DIR/$vg/$lv
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv
 fsck -fn $DM_DEV_DIR/$vg/$lv
 
 # Convert linear -> raid1

test/shell/lvconvert-raid-takeover.sh

@@ -26,7 +26,7 @@ aux prepare_vg 8
 # FIXME: lvconvert leaks  'error' devices
 detect_error_leak_()
 {
-	dmsetup table -S "name=~^$vg-" | not grep "error" || \
+	dmsetup table -S "name=~^$vg-" | awk '{print $3}'| not grep error || \
 		die "Device(s) with error target should not be here."
 }
 

test/shell/lvconvert-raid5_to_raid10.sh

@@ -30,7 +30,8 @@ check lv_field $vg/$lv1 segtype "raid5"
 check lv_field $vg/$lv1 stripes 4
 check lv_field $vg/$lv1 data_stripes 3
 check lv_field $vg/$lv1 region_size "256.00k"
-echo y|mkfs -t ext4 $DM_DEV_DIR/$vg/$lv1
+wipefs -a $DM_DEV_DIR/$vg/$lv1
+mkfs -t ext4 $DM_DEV_DIR/$vg/$lv1
 fsck -fn $DM_DEV_DIR/$vg/$lv1
 aux wait_for_sync $vg $lv1
 fsck -fn $DM_DEV_DIR/$vg/$lv1

test/shell/lvconvert-repair-cache.sh

@@ -93,9 +93,6 @@ lvconvert --yes --uncache $vg/$lv1
 
 aux enable_dev "$dev2"
 
-# vg was changed while dev was missing
-vgextend --restoremissing $vg "$dev2"
-
 # FIXME: temporary workaround
 lvcreate -L1 -n $lv5 $vg
 lvremove -ff $vg

test/shell/lvconvert-repair-policy.sh

@@ -24,8 +24,6 @@ aux lvmconf 'allocation/maximise_cling = 0' \
 cleanup_() {
 	vgreduce --removemissing $vg
 	for d in "$@"; do aux enable_dev "$d"; done
-	# clear the outdated metadata on enabled devs before we can reuse them
-	vgck --updatemetadata $vg
 	for d in "$@"; do vgextend $vg "$d"; done
 	lvremove -ff $vg/mirror
 	lvcreate -aey --type mirror -m 1 --ignoremonitoring -l 2 -n mirror $vg "$dev1" "$dev2" "$dev3:0"

test/shell/lvconvert-repair-raid.sh

@@ -65,7 +65,6 @@ aux disable_dev "$dev2"
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev2"
-vgck --updatemetadata $vg
 vgextend $vg "$dev2"
 lvremove -ff $vg/$lv1
 
@@ -81,7 +80,6 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev2" "$dev3"
-vgck --updatemetadata $vg
 vgextend $vg "$dev2" "$dev3"
 lvremove -ff $vg/$lv1
 
@@ -98,7 +96,6 @@ aux disable_dev "$dev3"
 vgreduce --removemissing -f $vg
 lvconvert -y --repair $vg/$lv1
 aux enable_dev "$dev3"
-vgck --updatemetadata $vg
 pvcreate -yff "$dev3"
 vgextend $vg "$dev3"
 lvremove -ff $vg/$lv1
@@ -117,7 +114,6 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev3"
-vgck --updatemetadata $vg
 vgextend $vg "$dev3"
 lvremove -ff $vg/$lv1
 
@@ -132,7 +128,6 @@ aux disable_dev "$dev4" "$dev5"
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev4" "$dev5"
-vgck --updatemetadata $vg
 vgextend $vg "$dev4" "$dev5"
 lvremove -ff $vg/$lv1
 
@@ -150,7 +145,6 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev4"
-vgck --updatemetadata $vg
 vgextend $vg "$dev4"
 lvremove -ff $vg/$lv1
 
@@ -169,7 +163,6 @@ aux wait_for_sync $vg $lv1
 lvconvert -y --repair $vg/$lv1
 vgreduce --removemissing $vg
 aux enable_dev "$dev4"
-vgck --updatemetadata $vg
 vgextend $vg "$dev4"
 lvremove -ff $vg/$lv1
 

test/shell/lvconvert-repair.sh

@@ -106,23 +106,17 @@ lvconvert -y --repair $vg/mirror
 vgreduce --removemissing $vg
 
 aux enable_dev "$dev1"
-# clear the outdated dev before we can reuse it
-vgck --updatemetadata $vg
 vgextend $vg "$dev1"
 aux disable_dev "$dev2"
 lvconvert -y --repair $vg/mirror
 vgreduce --removemissing $vg
 
 aux enable_dev "$dev2"
-# clear the outdated dev before we can reuse it
-vgck --updatemetadata $vg
 vgextend $vg "$dev2"
 aux disable_dev "$dev3"
 lvconvert -y --repair $vg/mirror
 vgreduce --removemissing $vg
 aux enable_dev "$dev3"
-# clear the outdated dev before we can reuse it
-vgck --updatemetadata $vg
 vgextend $vg "$dev3"
 
 vgremove -ff $vg

test/shell/lvm-on-md.sh

@@ -12,12 +12,30 @@
 
 SKIP_WITH_LVMPOLLD=1
 
+RUNDIR="/run"
+test -d "$RUNDIR" || RUNDIR="/var/run"
+PVS_ONLINE_DIR="$RUNDIR/lvm/pvs_online"
+VGS_ONLINE_DIR="$RUNDIR/lvm/vgs_online"
+
+_clear_online_files() {
+        # wait till udev is finished
+        aux udev_wait
+        rm -f "$PVS_ONLINE_DIR"/*
+        rm -f "$VGS_ONLINE_DIR"/*
+}
+
 . lib/inittest
 
 test -f /proc/mdstat && grep -q raid1 /proc/mdstat || \
 	modprobe raid1 || skip
 
 aux lvmconf 'devices/md_component_detection = 1'
+
+# This stops lvm from asking udev if a dev is an md component.
+# LVM will ask udev if a dev is an md component, but we don't
+# want to rely on that ability in this test.
+aux lvmconf 'devices/obtain_device_list_from_udev = 0'
+
 aux extend_filter_LVMTEST "a|/dev/md|"
 
 aux prepare_devs 2
@@ -26,31 +44,31 @@ aux prepare_devs 2
 # by default using metadata format 1.0 with data at the end of device
 aux prepare_md_dev 1 64 2 "$dev1" "$dev2"
 
+cat /proc/mdstat
+
 mddev=$(< MD_DEV)
 pvdev=$(< MD_DEV_PV)
 
 vgcreate $vg "$mddev"
 
-lvs $vg
-
 lvcreate -n $lv1 -l 2 $vg
 lvcreate -n $lv2 -l 2 -an $vg
 
 lvchange -ay $vg/$lv2
+check lv_field $vg/$lv1 lv_active "active"
 
-lvs $vg
-
-pvs -vvvv 2>&1|tee pvs.out
-
-vgchange -an $vg
-
-vgchange -ay -vvvv $vg 2>&1| tee vgchange.out
-
-lvs $vg
-pvs
+# lvm does not show md components as PVs
+pvs "$mddev"
+not pvs "$dev1"
+not pvs "$dev2"
+sleep 1
 
 vgchange -an $vg
+sleep 1
 
+# When the md device is started, lvm will see that and know to
+# scan for md components, so stop the md device to remove this
+# advantage so we will test the fallback detection.
 mdadm --stop "$mddev"
 aux udev_wait
 
@@ -61,13 +79,26 @@ pvs 2>&1 |tee out
 cat out
 grep "prefers device" out
 
-pvs -vvvv 2>&1| tee pvs2.out
-
 # should not activate from the md legs
-not vgchange -ay -vvvv $vg 2>&1|tee vgchange-fail.out
+not vgchange -ay $vg
 
 # should not show an active lv
-lvs $vg
+rm out
+lvs -o active $vg |tee out || true
+not grep "active" out
+
+# should not allow updating vg
+not lvcreate -l1 $vg
+
+# should not activate from the md legs
+_clear_online_files
+pvscan --cache -aay "$dev1"
+pvscan --cache -aay "$dev2"
+
+# should not show an active lv
+rm out
+lvs -o active $vg |tee out || true
+not grep "active" out
 
 # start the md dev
 mdadm --assemble "$mddev" "$dev1" "$dev2"
@@ -84,9 +115,95 @@ vgchange -ay $vg 2>&1 |tee out
 cat out
 not grep "prefers device" out
 
+check lv_field $vg/$lv1 lv_active "active"
+
 vgchange -an $vg
 aux udev_wait
 
+vgremove -f $vg
+
+aux cleanup_md_dev
+
+# create 2 disk MD raid0 array
+# by default using metadata format 1.0 with data at the end of device
+# When a raid0 md array is stopped, the components will not look like
+# duplicate PVs as they do with raid1.
+aux prepare_md_dev 0 64 2 "$dev1" "$dev2"
+
+cat /proc/mdstat
+
+mddev=$(< MD_DEV)
+pvdev=$(< MD_DEV_PV)
+
+vgcreate $vg "$mddev"
+
+lvs $vg
+
+lvcreate -n $lv1 -l 2 $vg
+lvcreate -n $lv2 -l 2 -an $vg
+
+lvchange -ay $vg/$lv2
+check lv_field $vg/$lv1 lv_active "active"
+
+# lvm does not show md components as PVs
+pvs "$mddev"
+not pvs "$dev1"
+not pvs "$dev2"
+sleep 1
+
+vgchange -an $vg
+sleep 1
+
+# When the md device is started, lvm will see that and know to
+# scan for md components, so stop the md device to remove this
+# advantage so we will test the fallback detection.
+mdadm --stop "$mddev"
+aux udev_wait
+
+pvs 2>&1 |tee pvs.out
+
+# should not activate from the md legs
+not vgchange -ay $vg
+
+# should not show an active lv
+rm out
+lvs -o active $vg |tee out || true
+not grep "active" out
+
+# should not allow updating vg
+not lvcreate -l1 $vg
+
+# should not activate from the md legs
+_clear_online_files
+pvscan --cache -aay "$dev1"
+pvscan --cache -aay "$dev2"
+
+# should not show an active lv
+rm out
+lvs -o active $vg |tee out || true
+not grep "active" out
+
+# start the md dev
+mdadm --assemble "$mddev" "$dev1" "$dev2"
+aux udev_wait
+
+# Now that the md dev is online, pvs can see it and
+# ignore the two legs, so there's no duplicate warning
+
+pvs 2>&1 |tee out
+cat out
+not grep "prefers device" out
+
+vgchange -ay $vg 2>&1 |tee out
+cat out
+not grep "prefers device" out
+
+check lv_field $vg/$lv1 lv_active "active"
+
+vgchange -an $vg
+aux udev_wait
+
+vgremove -f $vg
+
 aux cleanup_md_dev
 
-# vgremove -f $vg

test/shell/lvmcache-exercise.sh

@@ -43,16 +43,14 @@ pvs "$dev1"
 lvcreate -aey -m2 --type mirror -l4 --alloc anywhere --corelog -n $lv1 $vg2
 
 aux disable_dev "$dev3"
-
-pvs 2>&1| tee out
-grep "is missing PV" out
-
 lvconvert --yes --repair $vg2/$lv1
-
 aux enable_dev "$dev3"
 
+# here it should fix any reappeared devices
+lvs
+
 lvs -a $vg2 -o+devices 2>&1 | tee out
-not grep "is missing PV" out
+not grep reappeared out
 
 # This removes the first "vg1" using its uuid
 vgremove -ff -S vg_uuid=$UUID1

test/shell/mirror-vgreduce-removemissing.sh

@@ -123,12 +123,7 @@ check_and_cleanup_lvs_()
 recover_vg_()
 {
 	aux enable_dev "$@"
-	# ensure hints are updated
-	pvscan --cache
-	# clear outdated metadata on PVs so they can be used again
-	vgck --updatemetadata $vg
 	pvcreate -ff "$@"
-	# wipefs -a "$@"
 	vgextend $vg "$@"
 	check_and_cleanup_lvs_
 }

test/shell/missing-pv.sh

@@ -1,141 +0,0 @@
-#!/usr/bin/env bash
-
-# Copyright (C) 2008-2013,2018 Red Hat, Inc. All rights reserved.
-#
-# This copyrighted material is made available to anyone wishing to use,
-# modify, copy, or redistribute it subject to the terms and conditions
-# of the GNU General Public License v.2.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software Foundation,
-# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-
-. lib/inittest
-
-aux prepare_devs 3
-get_devs
-
-vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
-
-lvcreate -n $lv1 -L8M --type mirror -m 1 $vg
-lvcreate -n $lv2 -L8M --type mirror -m 1 $vg
-
-vgchange -an $vg
-
-pvs
-vgs
-lvs -a -o+devices
-
-# Fail one leg of each mirror LV.
-aux disable_dev "$dev1"
-
-pvs
-vgs
-lvs -a -o+devices
-
-# Cannot do normal activate of either LV with a failed leg.
-not lvchange -ay $vg/$lv1
-not lvchange -ay $vg/$lv2
-
-# Can activate with partial option.
-lvchange -ay --activationmode partial $vg/$lv1
-lvchange -ay --activationmode partial $vg/$lv2
-
-pvs
-vgs
-lvs -a -o+devices
-
-# Repair lv1 so it no longer uses failed dev.
-lvconvert --repair --yes $vg/$lv1
-
-# TODO: check that MISSING flag is set in ondisk metadata
-# it should have been written by the lvconvert since the
-# missing PV is still used by lv2
-
-pvs
-vgs
-lvs -a -o+devices
-
-# Verify normal activation is possible of lv1 since it's
-# not using any failed devs, and partial activation is
-# required for lv2 since it's still using the failed dev.
-vgchange -an $vg
-lvchange -ay $vg/$lv1
-not lvchange -ay $vg/$lv2
-vgchange -an $vg
-
-aux enable_dev "$dev1"
-
-pvs
-vgs
-lvs -a -o+devices
-
-# TODO: check that lv2 has partial flag, lv1 does not
-# (there's no partial reporting option, only attr p.)
-
-# TODO: check that MISSING flag is still set in ondisk
-# metadata since the previously missing dev is still
-# used by lv2.
-
-# The missing pv restrictions still apply even after
-# the dev has reappeared since it has the MISSING flag.
-not lvchange -ay $vg/$lv2
-not lvcreate -l1 $vg
-
-# Update old metadata on the previously missing PV.
-# This should not clear the MISSING flag because the
-# previously missing PV is still used by lv2.
-# This would be done by any command that writes
-# metadata, e.g. lvcreate, but since we are in a
-# state with a missing pv, most commands that write
-# metadata are restricted, so use a command that
-# explicitly writes/fixes metadata.
-vgck --updatemetadata $vg
-
-pvs
-vgs
-lvs -a -o+devices
-
-# TODO: check that MISSING flag is still set in ondisk
-# metadata since the previously missing dev is still
-# used by lv2.
-
-# The missing pv restrictions still apply since it
-# has the MISSING flag.
-not lvchange -ay $vg/$lv2
-not lvcreate -l1 $vg
-
-lvchange -ay --activationmode partial  $vg/$lv2
-
-# After repair, no more LVs will be using the previously
-# missing PV.
-lvconvert --repair --yes $vg/$lv2
-
-pvs
-vgs
-lvs -a -o+devices
-
-vgchange -an $vg
-
-# The next write of the metadata will clear the MISSING
-# flag in ondisk metadata because the previously missing
-# PV is no longer used by any LVs.
-
-# Run a command to write ondisk metadata, which should clear
-# the MISSING flag, options include:
-# lvcreate -l1 $vg
-# vgck --updatemetadata vg
-# vgextend --restoremissing
-# vgreduce --removemissing
-vgreduce --removemissing $vg
-
-# TODO: check that the MISSING flag is no longer set
-# in the ondisk metadata.
-
-pvs
-vgs
-lvs -a -o+devices
-
-vgchange -an $vg
-vgremove -ff $vg
-

test/shell/pv-ext-flags.sh

@@ -39,6 +39,7 @@ check pv_field "$dev2" pv_in_use "used"
 # disable $dev2 and dev1 with 0 MDAs remains, but still
 # marked as used, so pvcreate/vgcreate/pvremove should fail
 aux disable_dev "$dev2"
+pvscan --cache
 
 check pv_field "$dev1" pv_in_use "used"
 not pvcreate "$dev1" 2>err
@@ -70,14 +71,20 @@ vgcreate $vg1 "$dev1" "$dev2"
 # disable $dev1, then repair the VG - $dev1 is removed from VG
 aux disable_dev "$dev1"
 vgreduce --removemissing $vg1
-
-# now, enable $dev1 and clear the old metadata from it
+# now, enable $dev1, automatic repair will happen on pvs call
+# (or any other lvm command that does vg_read with repair inside)
 aux enable_dev "$dev1"
-vgck --updatemetadata $vg1
+
+# FIXME: once persistent cache does not cause races with timestamps
+#        causing LVM tools to not see the VG inconsistency and once
+#        VG repair is always done, delete this line which removes
+#        persistent .cache as a workaround
+rm -f "$TESTDIR/etc/.cache"
 
 vgck $vg1
-
-# check $dev1 does not contain the PV_EXT_FLAG anymore
+# check $dev1 does not contain the PV_EXT_FLAG anymore - it
+# should be removed as part of the repaid during vg_read since
+# $dev1 is not part of $vg1 anymore
 check pv_field "$dev1" pv_in_use ""
 
 #############################################
@@ -98,6 +105,7 @@ check pv_field "$dev2" pv_in_use "used"
 
 pvchange --metadataignore y "$dev1"
 aux disable_dev "$dev2"
+pvscan --cache
 
 check pv_field "$dev1" pv_in_use "used"
 not pvcreate "$dev1" 2>err
@@ -128,14 +136,20 @@ vgcreate $vg1 "$dev1" "$dev2"
 # disable $dev1, then repair the VG - $dev1 is removed from VG
 aux disable_dev "$dev1"
 vgreduce --removemissing $vg1
-
-# now, enable $dev1 and clear the old metadata from it
+# now, enable $dev1, automatic repair will happen on pvs call
+# (or any other lvm command that does vg_read with repair inside)
 aux enable_dev "$dev1"
-vgck --updatemetadata $vg1
+
+# FIXME: once persistent cache does not cause races with timestamps
+#        causing LVM tools to not see the VG inconsistency and once
+#        VG repair is always done, delete this line which removes
+#        persistent .cache as a workaround
+rm -f "$TESTDIR/etc/.cache"
 
 vgck $vg1
-
-# check $dev1 does not contain the PV_EXT_FLAG anymore
+# check $dev1 does not contain the PV_EXT_FLAG anymore - it
+# should be removed as part of the repaid during vg_read since
+# $dev1 is not part of $vg1 anymore
 check pv_field "$dev1" pv_in_use ""
 
 ###########################

test/shell/pvmove-resume-multiseg.sh

@@ -33,9 +33,9 @@ test_pvmove_resume() {
 	# next LV on same VG and differetnt PV (we want to test 2 pvmoves per VG)
 	lvcreate -an -Zn -l30 -n $lv2 $vg "$dev3"
 
-	aux delay_dev "$dev4" 0 250 "$(get first_extent_sector "$dev4"):"
+	aux delay_dev "$dev4" 0 500 "$(get first_extent_sector "$dev4"):"
 	test -e HAVE_DM_DELAY || { lvremove -f $vg; return 0; }
-	aux delay_dev "$dev5" 0 250 "$(get first_extent_sector "$dev5"):"
+	aux delay_dev "$dev5" 0 500 "$(get first_extent_sector "$dev5"):"
 
 	pvmove -i5 "$dev1" "$dev4" &
 	PVMOVE=$!

test/shell/pvscan-autoactivate.sh

@@ -27,7 +27,7 @@ _clear_online_files() {
 
 . lib/inittest
 
-aux prepare_pvs 3
+aux prepare_pvs 8
 
 vgcreate $vg1 "$dev1" "$dev2"
 lvcreate -n $lv1 -l 4 -a n $vg1
@@ -59,6 +59,19 @@ pvscan --cache -aay "$dev1" "$dev2"
 check lv_field $vg1/$lv1 lv_active "active"
 lvchange -an $vg1
 
+# check that a cache command without aay will
+# just record online state, and that a following
+# pvscan cache aay that does not record any new
+# online files will activate the vg
+_clear_online_files
+pvscan --cache "$dev1"
+check lv_field $vg1/$lv1 lv_active ""
+pvscan --cache "$dev2"
+check lv_field $vg1/$lv1 lv_active ""
+pvscan --cache -aay
+check lv_field $vg1/$lv1 lv_active "active"
+lvchange -an $vg1
+
 # Set up tests where one dev has no metadata
 
 vgchange -an $vg1
@@ -96,18 +109,6 @@ lvchange -an $vg1
 
 _clear_online_files
 
-pvscan --cache "$dev1"
-check lv_field $vg1/$lv1 lv_active ""
-pvscan --cache "$dev2"
-check lv_field $vg1/$lv1 lv_active ""
-pvscan --cache -aay
-check lv_field $vg1/$lv1 lv_active "active"
-lvchange -an $vg1
-
-# like previous
-
-_clear_online_files
-
 pvscan --cache "$dev1"
 check lv_field $vg1/$lv1 lv_active ""
 pvscan --cache -aay "$dev2"
@@ -131,9 +132,11 @@ ls "$RUNDIR/lvm/pvs_online"
 not ls "$RUNDIR/lvm/pvs_online/$PVID3"
 
 
+# TODO: check this works on Lenny:
+if [[ -f /etc/machine-id ]]; then
 # pvscan cache ignores pv in a foreign vg
 
-aux lvmconf "global/system_id_source = uname"
+aux lvmconf "global/system_id_source = machineid"
 
 _clear_online_files
 
@@ -166,6 +169,41 @@ lvs --foreign $vg2 > tmp
 cat tmp
 grep $lv2 tmp
 check lv_field $vg2/$lv2 lv_active "" --foreign
+fi
 
+# Test the case where pvscan --cache -aay (with no devs)
+# gets the final PV to complete the VG, where that final PV
+# does not hold VG metadata.  In this case it needs to rely
+# on VG metadata that has been saved from a previously
+# scanned PV from the same VG.
+#
+# We can't control which order of devices pvscan will see,
+# so create several PVs without metadata surrounding one
+# PV with metadata, to make it likely that pvscan will
+# get a final PV without metadata.
 
+pvcreate --metadatacopies 0 "$dev4"
+pvcreate --metadatacopies 0 "$dev5"
+pvcreate --metadatacopies 1 "$dev6"
+pvcreate --metadatacopies 0 "$dev7"
+pvcreate --metadatacopies 0 "$dev8"
+vgcreate $vg3 "$dev4" "$dev5" "$dev6" "$dev7" "$dev8"
+lvcreate -n $lv1 -l 4 -a n $vg3
+
+_clear_online_files
+
+check lv_field $vg3/$lv1 lv_active ""
+pvscan --cache "$dev4"
+check lv_field $vg3/$lv1 lv_active ""
+pvscan --cache "$dev5"
+check lv_field $vg3/$lv1 lv_active ""
+pvscan --cache "$dev6"
+check lv_field $vg3/$lv1 lv_active ""
+pvscan --cache "$dev7"
+check lv_field $vg3/$lv1 lv_active ""
+pvscan --cache "$dev8"
+check lv_field $vg3/$lv1 lv_active ""
+pvscan --cache -aay
+check lv_field $vg3/$lv1 lv_active "active"
+lvchange -an $vg3
 

test/shell/pvscan-cache.sh

@@ -59,7 +59,9 @@ check lv_field $vg1/$lv1 lv_active ""
 pvscan --cache -aay "$dev1"
 pvscan --cache -aay "$dev2"
 check lv_field $vg1/$lv1 lv_active "active"
-lvchange -an $vg1/$lv1
+lvchange -an -vvvv $vg1/$lv1
+dmsetup ls
+lvremove $vg1/$lv1
 
 # When MDA is ignored on PV, do not read any VG
 # metadata from such PV as it may contain old

test/shell/relative-sign-options.sh

@@ -73,7 +73,7 @@ lvresize --poolmetadatasize 64 $vg/$lv7
 lvresize --poolmetadatasize +8 $vg/$lv7
 not lvresize -y --poolmetadatasize -8 $vg/$lv7
 
-lvextend --poolmetadatasize +8 $vg/$lv7
+lvextend --poolmetadatasize +4 $vg/$lv7
 not lvextend -y --poolmetadatasize -8 $vg/$lv7
 fi
 

test/shell/unlost-pv.sh

@@ -15,59 +15,47 @@ SKIP_WITH_LVMPOLLD=1
 
 . lib/inittest
 
+check_() {
+	local cache=""
+	# vgscan needs --cache option for direct scan if lvmetad is used
+	test -e LOCAL_LVMETAD && cache="--cache"
+	vgscan $cache 2>&1 | tee vgscan.out
+	"$@" grep "Inconsistent metadata found for VG $vg" vgscan.out
+}
+
 aux prepare_vg 3
 
 lvcreate -an -Zn --type mirror -m 1 -l 1 -n mirror $vg
+#lvchange -a n $vg
 
 # try orphaning a missing PV (bz45867)
 aux disable_dev "$dev1"
 vgreduce --removemissing --force $vg
 aux enable_dev "$dev1"
 
-vgscan 2>&1 | tee vgscan.out
-grep "Inconsistent metadata found for VG $vg" vgscan.out
-
-# erase outdated dev1
-vgck --updatemetadata $vg
-
-vgscan 2>&1 | tee vgscan.out
-not grep "Inconsistent metadata found for VG $vg" vgscan.out
-
+check_
+test -e LOCAL_LVMETAD && pvcreate -f "$dev1"
+check_ not
 
+# try to just change metadata; we expect the new version (with MISSING_PV set
+# on the reappeared volume) to be written out to the previously missing PV
 vgextend $vg "$dev1"
-
 lvcreate -l 1 -n boo -a n --zero n $vg
-
 aux disable_dev "$dev1"
-
 lvremove $vg/mirror
-
 aux enable_dev "$dev1"
-
-vgscan 2>&1 | tee vgscan.out
-grep "Inconsistent metadata found for VG $vg" vgscan.out
-
-# write the vg to update the metadata on dev1
-vgck --updatemetadata $vg
-
-vgscan 2>&1 | tee vgscan.out
-not grep "Inconsistent metadata found for VG $vg" vgscan.out
+check_
+test -e LOCAL_LVMETAD && lvremove $vg/boo # FIXME trigger a write :-(
+check_ not
 
 aux disable_dev "$dev1"
-
 vgreduce --removemissing --force $vg
-
 aux enable_dev "$dev1"
 
 vgscan 2>&1 | tee out
+grep 'Removing PV' out
 
-vgscan 2>&1 | tee vgscan.out
-grep "Inconsistent metadata found for VG $vg" vgscan.out
-
-# erase outdated dev1
-vgck --updatemetadata $vg
-
-vgscan 2>&1 | tee vgscan.out
-not grep "Inconsistent metadata found for VG $vg" vgscan.out
+vgs 2>&1 | tee out
+not grep 'Removing PV' out
 
 vgremove -ff $vg

test/shell/vgck.sh

@@ -24,11 +24,11 @@ dd if=/dev/urandom bs=512 seek=2 count=32 of="$dev2"
 
 vgscan 2>&1 | tee vgscan.out || true
 
-grep "checksum" vgscan.out
+grep "Failed" vgscan.out
 
 dd if=/dev/urandom bs=512 seek=2 count=32 of="$dev2"
 
 vgck $vg 2>&1 | tee vgck.out || true
-grep "checksum" vgck.out
+grep Incorrect vgck.out
 
 vgremove -ff $vg

tools/args.h

@@ -213,6 +213,11 @@ arg(driverloaded_ARG, '\0', "driverloaded", bool_VAL, 0, 0,
     "If set to no, the command will not attempt to use device-mapper.\n"
     "For testing and debugging.\n")
 
+arg(dump_ARG, '\0', "dump", string_VAL, 0, 0,
+    "Dump metadata from a PV. Option values include \\fBmetadata\\fP\n"
+    "to extract the current text metadata, and \\fBmetadata_area\\fP\n"
+    "to extract the entire text metadata area.\n")
+
 arg(errorwhenfull_ARG, '\0', "errorwhenfull", bool_VAL, 0, 0,
     "Specifies thin pool behavior when data space is exhausted.\n"
     "When yes, device-mapper will immediately return an error\n"
@@ -1386,9 +1391,6 @@ arg(thin_ARG, 'T', "thin", 0, 0, 0,
     "See --type thin, --type thin-pool, and --virtualsize.\n"
     "See \\fBlvmthin\\fP(7) for more information about LVM thin provisioning.\n")
 
-arg(updatemetadata_ARG, '\0', "updatemetadata", 0, 0, 0,
-    "Update VG metadata to correct problems.\n")
-
 arg(uuid_ARG, 'u', "uuid", 0, 0, 0,
     "#pvchange\n"
     "Generate new random UUID for specified PVs.\n"

tools/command-lines.in

@@ -1427,6 +1427,12 @@ ID: pvresize_general
 pvck PV ...
 OO: --labelsector Number
 ID: pvck_general
+DESC: Check for metadata on a device
+
+pvck --dump String PV
+OO: --file String, --pvmetadatacopies MetadataCopiesPV
+ID: pvck_dumpmetadata
+DESC: Dump raw metadata from a device
 
 ---
 
@@ -1618,11 +1624,6 @@ vgck
 OO: --reportformat ReportFmt
 OP: VG|Tag ...
 ID: vgck_general
-DESC: Read and display information about a VG.
-
-vgck --updatemetadata VG
-ID: vgck_update_metadata
-DESC: Rewrite VG metadata to correct problems.
 
 ---
 

tools/commands.h

@@ -114,7 +114,7 @@ xx(pvresize,
    0)
 
 xx(pvck,
-   "Check the consistency of physical volume(s)",
+   "Check metadata on physical volumes",
    LOCKD_VG_SH)
 
 xx(pvcreate,

tools/lvmcmdline.c

@@ -2766,6 +2766,22 @@ static int _init_lvmlockd(struct cmd_context *cmd)
 	return 1;
 }
 
+static void _init_md_checks(struct cmd_context *cmd)
+{
+	/*
+	 * use_full_md_check can also be set later.
+	 * These commands are chosen to always perform
+	 * a full md component check because they initialize
+	 * a new device that could be an md component,
+	 * and they are not run frequently during normal
+	 * operation.
+	 */
+	if (!strcmp(cmd->name, "pvcreate") ||
+	    !strcmp(cmd->name, "vgcreate") ||
+	    !strcmp(cmd->name, "vgextend"))
+		cmd->use_full_md_check = 1;
+}
+
 static int _cmd_no_meta_proc(struct cmd_context *cmd)
 {
 	return cmd->cname->flags & NO_METADATA_PROCESSING;
@@ -2958,7 +2974,9 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		log_warn("WARNING: locking_type (%d) is deprecated, using file locking.", locking_type);
 	}
 
-	if (arg_is_set(cmd, nolocking_ARG) || _cmd_no_meta_proc(cmd))
+	cmd->nolocking = arg_is_set(cmd, nolocking_ARG);
+
+	if (cmd->nolocking || _cmd_no_meta_proc(cmd))
 		nolocking = 1;
 
 	if (arg_is_set(cmd, sysinit_ARG))
@@ -2977,6 +2995,8 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		}
 	}
 
+	_init_md_checks(cmd);
+
 	if (!_cmd_no_meta_proc(cmd) && !_init_lvmlockd(cmd)) {
 		ret = ECMD_FAILED;
 		goto_out;
@@ -2997,7 +3017,7 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 
       out:
 
-	hints_exit();
+	hints_exit(cmd);
 	lvmcache_destroy(cmd, 1, 1);
 	label_scan_destroy(cmd);
 

tools/polldaemon.c

@@ -148,7 +148,6 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
 	struct logical_volume *lv;
 	int finished = 0;
 	uint32_t lockd_state = 0;
-	uint32_t error_flags = 0;
 	int ret;
 
 	if (!parms->wait_before_testing)
@@ -169,10 +168,12 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
 		}
 
 		/* Locks the (possibly renamed) VG again */
-		vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state, &error_flags, NULL);
-		if (!vg) {
+		vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state);
+		if (vg_read_error(vg)) {
 			/* What more could we do here? */
-			log_error("ABORTING: Can't reread VG for %s error flags %x.", id->display_name, error_flags);
+			log_error("ABORTING: Can't reread VG for %s.", id->display_name);
+			release_vg(vg);
+			vg = NULL;
 			ret = 0;
 			goto out;
 		}
@@ -375,6 +376,7 @@ static void _poll_for_all_vgs(struct cmd_context *cmd,
 	while (1) {
 		parms->outstanding_count = 0;
 		process_each_vg(cmd, 0, NULL, NULL, NULL, READ_FOR_UPDATE, 0, handle, _poll_vg);
+		lock_global(cmd, "un");
 		if (!parms->outstanding_count)
 			break;
 		_nanosleep(parms->interval, 1);
@@ -393,7 +395,6 @@ static int _report_progress(struct cmd_context *cmd, struct poll_operation_id *i
 	struct volume_group *vg;
 	struct logical_volume *lv;
 	uint32_t lockd_state = 0;
-	uint32_t error_flags = 0;
 	int ret;
 
 	/*
@@ -406,9 +407,10 @@ static int _report_progress(struct cmd_context *cmd, struct poll_operation_id *i
 	 * change done locally.
 	 */
 
-	vg = vg_read(cmd, id->vg_name, NULL, 0, lockd_state, &error_flags, NULL);
-	if (!vg) {
-		log_error("Can't reread VG for %s error flags %x", id->display_name, error_flags);
+	vg = vg_read(cmd, id->vg_name, NULL, 0, lockd_state);
+	if (vg_read_error(vg)) {
+		release_vg(vg);
+		log_error("Can't reread VG for %s", id->display_name);
 		ret = 0;
 		goto out_ret;
 	}

tools/pvchange.c

@@ -122,9 +122,8 @@ static int _pvchange_single(struct cmd_context *cmd, struct volume_group *vg,
 	 * Convert sh to ex.
 	 */
 	if (is_orphan(pv)) {
-		if (!lockd_gl(cmd, "ex", 0))
+		if (!lock_global_convert(cmd, "ex"))
 			return_ECMD_FAILED;
-		cmd->lockd_gl_disable = 1;
 	}
 
 	if (tagargs) {
@@ -236,29 +235,12 @@ int pvchange(struct cmd_context *cmd, int argc, char **argv)
 		goto out;
 	}
 
-	if (!argc) {
-		/*
-		 * Take the global lock here so the lvmcache remains
-		 * consistent across orphan/non-orphan vg locks.  If we don't
-		 * take the lock here, pvs with 0 mdas in a non-orphan VG will
-		 * be processed twice.
-		 */
-		if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_WRITE, NULL)) {
-			log_error("Unable to obtain global lock.");
-			ret = ECMD_FAILED;
-			goto out;
-		}
-	}
-
 	set_pv_notify(cmd);
 
 	clear_hint_file(cmd);
 
 	ret = process_each_pv(cmd, argc, argv, NULL, 0, READ_FOR_UPDATE | READ_ALLOW_EXPORTED, handle, _pvchange_single);
 
-	if (!argc)
-		unlock_vg(cmd, NULL, VG_GLOBAL);
-
 	log_print_unless_silent("%d physical volume%s changed / %d physical volume%s not changed",
 				params.done, params.done == 1 ? "" : "s",
 				params.total - params.done, (params.total - params.done) == 1 ? "" : "s");

tools/pvck.c

@@ -15,17 +15,115 @@
 
 #include "base/memory/zalloc.h"
 #include "tools.h"
+#include "lib/format_text/format-text.h"
+
+/*
+ * TODO: option to dump all copies of metadata that are found
+ *
+ * TODO: option to intelligently search for mda locations on
+ * disk in case the pv_header and/or mda_header are damaged.
+ */
+
+static int _dump_metadata(struct cmd_context *cmd, int argc, char **argv, int full_area)
+{
+	struct dm_list devs;
+	struct device_list *devl;
+	struct device *dev;
+	const char *pv_name;
+	const char *vgname;
+	const char *vgid;
+	struct lvmcache_info *info;
+	struct metadata_area *mda;
+	const char *tofile = NULL;
+	int mda_num = 1;
+	int ret;
+
+	dm_list_init(&devs);
+
+	if (arg_is_set(cmd, file_ARG)) {
+		if (!(tofile = arg_str_value(cmd, file_ARG, NULL)))
+			return ECMD_FAILED;
+	}
+
+	/* 1: dump metadata from first mda, 2: dump metadata from second mda */
+	if (arg_is_set(cmd, pvmetadatacopies_ARG))
+		mda_num = arg_int_value(cmd, pvmetadatacopies_ARG, 1);
+
+	pv_name = argv[0];
+
+	if (!(dev = dev_cache_get(cmd, pv_name, cmd->filter))) {
+		log_error("No device found for %s %s.", pv_name, dev_cache_filtered_reason(pv_name));
+		return ECMD_FAILED;
+	}
+
+	if (!(devl = zalloc(sizeof(*devl))))
+		return ECMD_FAILED;
+
+	devl->dev = dev;
+	dm_list_add(&devs, &devl->list);
+
+	label_scan_setup_bcache();
+	label_scan_devs(cmd, cmd->filter, &devs);
+
+	if (!dev->pvid[0]) {
+		log_error("No PV ID found for %s", dev_name(dev));
+		return ECMD_FAILED;
+	}
+
+	if (!(info = lvmcache_info_from_pvid(dev->pvid, dev, 0))) {
+		log_error("No VG info found for %s", dev_name(dev));
+		return ECMD_FAILED;
+	}
+
+	if (!(vgname = lvmcache_vgname_from_info(info))) {
+		log_error("No VG name found for %s", dev_name(dev));
+		return ECMD_FAILED;
+	}
+
+        if (!(vgid = lvmcache_vgid_from_vgname(cmd, vgname))) {
+		log_error("No VG ID found for %s", dev_name(dev));
+		return ECMD_FAILED;
+        }
+
+	if (!(mda = lvmcache_get_mda(cmd, vgname, dev, mda_num))) {
+		log_error("No mda %d found for %s", mda_num, dev_name(dev));
+		return ECMD_FAILED;
+	}
+
+	if (full_area)
+		ret = dump_metadata_area(cmd, vgname, vgid, dev, mda, tofile);
+	else
+		ret = dump_metadata_text(cmd, vgname, vgid, dev, mda, tofile);
+
+	if (!ret)
+		return ECMD_FAILED;
+	return ECMD_PROCESSED;
+}
 
 int pvck(struct cmd_context *cmd, int argc, char **argv)
 {
 	struct dm_list devs;
 	struct device_list *devl;
 	struct device *dev;
+	const char *dump;
 	const char *pv_name;
 	uint64_t labelsector;
 	int i;
 	int ret_max = ECMD_PROCESSED;
 
+	if (arg_is_set(cmd, dump_ARG)) {
+		dump = arg_str_value(cmd, dump_ARG, NULL);
+
+		if (!strcmp(dump, "metadata"))
+			return _dump_metadata(cmd, argc, argv, 0);
+
+		if (!strcmp(dump, "metadata_area"))
+			return _dump_metadata(cmd, argc, argv, 1);
+
+		log_error("Unknown dump value.");
+		return ECMD_FAILED;
+	}
+
 	labelsector = arg_uint64_value(cmd, labelsector_ARG, UINT64_C(0));
 
 	dm_list_init(&devs);
@@ -53,7 +151,6 @@ int pvck(struct cmd_context *cmd, int argc, char **argv)
 	label_scan_devs(cmd, cmd->filter, &devs);
 
 	dm_list_iterate_items(devl, &devs) {
-
 		/*
 		 * The scan above will populate lvmcache with any info from the
 		 * standard locations at the start of the device.  Now populate

tools/pvcreate.c

@@ -136,20 +136,14 @@ int pvcreate(struct cmd_context *cmd, int argc, char **argv)
 	pp.pv_count = argc;
 	pp.pv_names = argv;
 
-	/* Check for old md signatures at the end of devices. */
-	cmd->use_full_md_check = 1;
-
-	/*
-	 * Needed to change the set of orphan PVs.
-	 * (disable afterward to prevent process_each_pv from doing
-	 * a shared global lock since it's already acquired it ex.)
-	 */
-	if (!lockd_gl(cmd, "ex", 0))
+	/* Needed to change the set of orphan PVs. */
+	if (!lock_global(cmd, "ex"))
 		return_ECMD_FAILED;
-	cmd->lockd_gl_disable = 1;
 
 	clear_hint_file(cmd);
 
+	lvmcache_label_scan(cmd);
+
 	if (!(handle = init_processing_handle(cmd, NULL))) {
 		log_error("Failed to initialize processing handle.");
 		return ECMD_FAILED;
@@ -157,11 +151,8 @@ int pvcreate(struct cmd_context *cmd, int argc, char **argv)
 
 	if (!pvcreate_each_device(cmd, handle, &pp))
 		ret = ECMD_FAILED;
-	else {
-		/* pvcreate_each_device returns with orphans locked */
-		unlock_vg(cmd, NULL, VG_ORPHANS);
+	else
 		ret = ECMD_PROCESSED;
-	}
 
 	destroy_processing_handle(cmd, handle);
 	return ret;

tools/pvmove.c

@@ -913,10 +913,10 @@ int pvmove(struct cmd_context *cmd, int argc, char **argv)
 
 		/*
 		 * The command may sit and report progress for some time,
-		 * and we do not want or need the lockd locks held during
+		 * and we do not want or need the global lock held during
 		 * that time.
 		 */
-		lockd_gl(cmd, "un", 0);
+		lock_global(cmd, "un");
 	}
 
 	return pvmove_poll(cmd, pv_name, lvid ? lvid->s : NULL,

tools/pvremove.c

@@ -34,22 +34,19 @@ int pvremove(struct cmd_context *cmd, int argc, char **argv)
 	pp.pv_count = argc;
 	pp.pv_names = argv;
 
-	/*
-	 * Needed to change the set of orphan PVs.
-	 * (disable afterward to prevent process_each_pv from doing
-	 * a shared global lock since it's already acquired it ex.)
-	 */
-	if (!lockd_gl(cmd, "ex", 0)) {
+	/* Needed to change the set of orphan PVs. */
+	if (!lock_global(cmd, "ex")) {
 		/* Let pvremove -ff skip locks */
 		if (pp.force == DONT_PROMPT_OVERRIDE)
-			log_warn("WARNING: skipping global lock in lvmlockd for force.");
+			log_warn("WARNING: skipping global lock for force.");
 		else
 			return_ECMD_FAILED;
 	}
-	cmd->lockd_gl_disable = 1;
 
 	clear_hint_file(cmd);
 
+	lvmcache_label_scan(cmd);
+
 	/* When forcibly clearing a PV we don't care about a VG lock. */
 	if (pp.force == DONT_PROMPT_OVERRIDE)
 		cmd->lockd_vg_disable = 1;
@@ -67,11 +64,8 @@ int pvremove(struct cmd_context *cmd, int argc, char **argv)
 
 	if (!pvcreate_each_device(cmd, handle, &pp))
 		ret = ECMD_FAILED;
-	else {
-		/* pvcreate_each_device returns with orphans locked */
-		unlock_vg(cmd, NULL, VG_ORPHANS);
+	else
 		ret = ECMD_PROCESSED;
-	}
 
 	destroy_processing_handle(cmd, handle);
 	return ret;

tools/pvresize.c

@@ -44,12 +44,11 @@ static int _pvresize_single(struct cmd_context *cmd,
 	/*
 	 * Needed to change a property on an orphan PV.
 	 * i.e. the global lock is only needed for orphans.
-	 * Convert sh to ex.
+	 * Convert sh to ex.  (sh was taken by process_each)
 	 */
 	if (is_orphan(pv)) {
-		if (!lockd_gl(cmd, "ex", 0))
+		if (!lock_global_convert(cmd, "ex"))
 			return_ECMD_FAILED;
-		cmd->lockd_gl_disable = 1;
 	}
 
 	if (!pv_resize_single(cmd, vg, pv, params->new_size, arg_is_set(cmd, yes_ARG)))

tools/pvscan.c

@@ -148,11 +148,6 @@ int pvscan_display_cmd(struct cmd_context *cmd, int argc, char **argv)
 			  arg_is_set(cmd, exported_ARG) ?
 			  "of exported volume group(s)" : "in no volume group");
 
-	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_WRITE, NULL)) {
-		log_error("Unable to obtain global lock.");
-		return ECMD_FAILED;
-	}
-
 	if (!(handle = init_processing_handle(cmd, NULL))) {
 		log_error("Failed to initialize processing handle.");
 		ret = ECMD_FAILED;
@@ -174,7 +169,6 @@ int pvscan_display_cmd(struct cmd_context *cmd, int argc, char **argv)
 					params.new_pvs_found, display_size(cmd, params.size_new));
 
 out:
-	unlock_vg(cmd, NULL, VG_GLOBAL);
 	destroy_processing_handle(cmd, handle);
 
 	return ret;
@@ -517,6 +511,7 @@ struct _pvscan_baton {
 	struct cmd_context *cmd;
 	struct volume_group *vg;
 	struct format_instance *fid;
+	int mda_read_errors;
 };
 
 static int _online_pvscan_single(struct metadata_area *mda, void *baton)
@@ -524,9 +519,13 @@ static int _online_pvscan_single(struct metadata_area *mda, void *baton)
 	struct _pvscan_baton *b = baton;
 	struct volume_group *vg;
 
-	if (mda_is_ignored(mda) ||
-	    !(vg = mda->ops->vg_read(b->fid, "", mda, NULL, NULL)))
+	if (mda_is_ignored(mda))
 		return 1;
+	vg = mda->ops->vg_read(b->fid, "", mda, NULL, NULL);
+	if (!vg) {
+		b->mda_read_errors++;
+		return 1;
+	}
 
 	/* FIXME Also ensure contents match etc. */
 	if (!b->vg || vg->seqno > b->vg->seqno)
@@ -547,10 +546,12 @@ static int _online_pvscan_single(struct metadata_area *mda, void *baton)
 static int _online_pvscan_one(struct cmd_context *cmd, struct device *dev,
 			      struct dm_list *dev_args,
 			      struct dm_list *found_vgnames,
+			      struct dm_list *all_vgs,
 			      int disable_remove,
 			      const char **pvid_without_metadata)
 {
 	struct lvmcache_info *info;
+	struct vg_list *vgl;
 	struct _pvscan_baton baton;
 	const struct format_type *fmt;
 	/* Create a dummy instance. */
@@ -590,6 +591,7 @@ static int _online_pvscan_one(struct cmd_context *cmd, struct device *dev,
 
 	fmt = lvmcache_fmt(info);
 
+	memset(&baton, 0, sizeof(baton));
 	baton.cmd = cmd;
 	baton.vg = NULL;
 	baton.fid = fmt->ops->create_instance(fmt, &fic);
@@ -601,6 +603,16 @@ static int _online_pvscan_one(struct cmd_context *cmd, struct device *dev,
 
 	lvmcache_foreach_mda(info, _online_pvscan_single, &baton);
 
+	if (baton.mda_read_errors) {
+		/* Don't record the PV as online if there are errors reading the vg. */
+		log_print("pvscan[%d] PV %s ignore for metadata processing error.", getpid(), dev_name(dev));
+		if (baton.vg)
+			release_vg(baton.vg);
+		else
+			fmt->ops->destroy_instance(baton.fid);
+		return 1;
+	}
+
 	if (!baton.vg) {
 		if (pvid_without_metadata)
 			*pvid_without_metadata = dm_pool_strdup(cmd->mem, dev->pvid);
@@ -658,11 +670,88 @@ static int _online_pvscan_one(struct cmd_context *cmd, struct device *dev,
 
 	ret = _online_pv_found(cmd, dev, dev_args, baton.vg, found_vgnames);
 
-	release_vg(baton.vg);
+	/*
+	 * Save vg's in case they need to be used at the end for checking PVs
+	 * without metadata (in _check_vg_with_pvid_complete).
+	 */
+	if (all_vgs && baton.vg) {
+		int found = 0;
+		dm_list_iterate_items(vgl, all_vgs) {
+			if (!strcmp(baton.vg->name, vgl->vg->name)) {
+				found = 1;
+				break;
+			}
+		}
+		if (!found) {
+	       		if ((vgl = malloc(sizeof(struct vg_list)))) {
+				vgl->vg = baton.vg;
+				baton.vg = NULL;
+				dm_list_add(all_vgs, &vgl->list);
+			}
+		}
+	}
+
+	if (baton.vg)
+		release_vg(baton.vg);
 out:
 	return ret;
 }
 
+/*
+ * This is to handle the case where pvscan --cache -aay (with no dev args)
+ * gets to the final PV, completing the VG, but that final PV does not
+ * have VG metadata.  In this case, we need to use VG metadata from a
+ * previously scanned PV in the same VG, which we saved in the all_vgs
+ * list.  Using this saved metadata, we can find which VG this PVID
+ * belongs to, and then check if that VG is now complete, and if so
+ * add the VG name to the list of complete VGs to be autoactivated.
+ *
+ * The "pvid" arg here is the PVID of the PV that has just been scanned
+ * and didn't have metadata.  We look through previously scanned VG
+ * metadata to find the VG this PVID belongs to, and then check that VG
+ * metadata to see if all the PVs are now online.
+ */
+static void _check_vg_with_pvid_complete(struct cmd_context *cmd,
+				         struct dm_list *found_vgnames,
+					 struct dm_list *all_vgs,
+					 const char *pvid)
+{
+	struct vg_list *vgl;
+	struct pv_list *pvl;
+	struct volume_group *vg;
+	int pvids_not_online = 0;
+	int found;
+
+	dm_list_iterate_items(vgl, all_vgs) {
+		vg = vgl->vg;
+		found = 0;
+
+		dm_list_iterate_items(pvl, &vg->pvs) {
+			if (strcmp((const char *)&pvl->pv->id.uuid, pvid))
+				continue;
+			found = 1;
+			break;
+		}
+		if (!found)
+			continue;
+
+		dm_list_iterate_items(pvl, &vg->pvs) {
+			if (!_online_pvid_file_exists((const char *)&pvl->pv->id.uuid)) {
+				pvids_not_online++;
+				break;
+			}
+		}
+
+		if (!pvids_not_online) {
+			log_debug("pvid %s makes complete VG %s", pvid, vg->name);
+			if (!str_list_add(cmd->mem, found_vgnames, dm_pool_strdup(cmd->mem, vg->name)))
+				stack;
+		} else
+			log_debug("pvid %s incomplete VG %s", pvid, vg->name);
+		break;
+	}
+}
+
 /*
  * dev_args is the list of devices that were specified on the
  * pvscan command line.
@@ -672,14 +761,23 @@ out:
  *
  * . When dev_args is set, then complete VGs that that contain
  *   devs in dev_args will be returned in found_vgnames.
+ *
+ * found_vgnames is null for 'pvscan --cache' (without -aay)
+ * since the command does not need to keep track of complete
+ * vgs since it does not need to activate them.
  */
 
 static void _online_pvscan_all_devs(struct cmd_context *cmd,
 				    struct dm_list *found_vgnames,
 				    struct dm_list *dev_args)
 {
+	struct dm_list all_vgs;
 	struct dev_iter *iter;
+	struct vg_list *vgl;
 	struct device *dev;
+	const char *pvid_without_metadata;
+
+	dm_list_init(&all_vgs);
 
 	label_scan(cmd);
 
@@ -694,12 +792,21 @@ static void _online_pvscan_all_devs(struct cmd_context *cmd,
 			break;
 		}
 
-		if (!_online_pvscan_one(cmd, dev, dev_args, found_vgnames, 1, NULL)) {
+		pvid_without_metadata = NULL;
+
+		if (!_online_pvscan_one(cmd, dev, dev_args, found_vgnames, &all_vgs, 1, &pvid_without_metadata)) {
 			stack;
 			break;
 		}
+
+		/* This PV without metadata may complete a VG. */
+		if (pvid_without_metadata && found_vgnames)
+			_check_vg_with_pvid_complete(cmd, found_vgnames, &all_vgs, pvid_without_metadata);
 	}
 
+	dm_list_iterate_items(vgl, &all_vgs)
+		release_vg(vgl->vg);
+
 	dev_iter_destroy(iter);
 }
 
@@ -846,11 +953,6 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
 		all_devs = 1;
 	}
 
-	if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_READ, NULL)) {
-		log_error("Unable to obtain global lock.");
-		return ECMD_FAILED;
-	}
-
 	_online_dir_setup();
 
 	/* Creates a list of dev names from /dev, sysfs, etc; does not read any. */
@@ -1024,7 +1126,7 @@ int pvscan_cache_cmd(struct cmd_context *cmd, int argc, char **argv)
 
 			add_single_count++;
 
-			if (!_online_pvscan_one(cmd, dev, NULL, complete_vgnames, 0, &pvid_without_metadata))
+			if (!_online_pvscan_one(cmd, dev, NULL, complete_vgnames, NULL, 0, &pvid_without_metadata))
 				add_errors++;
 		}
 	}
@@ -1070,7 +1172,6 @@ activate:
 
 	if (!sync_local_dev_names(cmd))
 		stack;
-	unlock_vg(cmd, NULL, VG_GLOBAL);
 	return ret;
 }
 

tools/reporter.c

@@ -708,7 +708,6 @@ int report_for_selection(struct cmd_context *cmd,
 static void _check_pv_list(struct cmd_context *cmd, struct report_args *args, struct single_report_args *single_args)
 {
 	int i;
-	int rescan_done = 0;
 
 	if (!args->argv)
 		return;
@@ -719,12 +718,6 @@ static void _check_pv_list(struct cmd_context *cmd, struct report_args *args, st
 
 	if (single_args->args_are_pvs && args->argc) {
 		for (i = 0; i < args->argc; i++) {
-			if (!rescan_done && !dev_cache_get(cmd, args->argv[i], cmd->filter)) {
-				cmd->filter->wipe(cmd->filter);
-				/* FIXME scan only one device */
-				lvmcache_label_scan(cmd);
-				rescan_done = 1;
-			}
 			if (*args->argv[i] == '@') {
 				/*
 				 * Tags are metadata related, not label
@@ -732,13 +725,7 @@ static void _check_pv_list(struct cmd_context *cmd, struct report_args *args, st
 				 */
 				if (single_args->report_type == LABEL)
 					single_args->report_type = PVS;
-				/*
-				 * If we changed the report_type and we did rescan,
-				 * no need to iterate over dev list further - nothing
-				 * else would change.
-				 */
-				if (rescan_done)
-					break;
+				break;
 			}
 		}
 	}
@@ -1076,7 +1063,6 @@ static int _do_report(struct cmd_context *cmd, struct processing_handle *handle,
 	void *orig_custom_handle = handle->custom_handle;
 	report_type_t report_type = single_args->report_type;
 	void *report_handle = NULL;
-	int lock_global = 0;
 	int lv_info_needed;
 	int lv_segment_status_needed;
 	int report_in_group = 0;
@@ -1100,18 +1086,6 @@ static int _do_report(struct cmd_context *cmd, struct processing_handle *handle,
 		report_in_group = 1;
 	}
 
-	/*
-	 * We lock VG_GLOBAL to enable use of metadata cache.
-	 * This can pause alongide pvscan or vgscan process for a while.
-	 */
-	if (single_args->args_are_pvs && (report_type == PVS || report_type == PVSEGS)) {
-		lock_global = 1;
-		if (!lock_vol(cmd, VG_GLOBAL, LCK_VG_READ, NULL)) {
-			log_error("Unable to obtain global lock.");
-			goto out;
-		}
-	}
-
 	switch (report_type) {
 		case DEVTYPES:
 			r = _process_each_devtype(cmd, args->argc, handle);
@@ -1209,8 +1183,6 @@ static int _do_report(struct cmd_context *cmd, struct processing_handle *handle,
 	if (!(args->log_only && (single_args->report_type != CMDLOG)))
 		dm_report_output(report_handle);
 
-	if (lock_global)
-		unlock_vg(cmd, NULL, VG_GLOBAL);
 out:
 	if (report_handle) {
 		if (report_in_group && !dm_report_group_pop(cmd->cmd_report.report_group))

tools/toollib.c

@@ -189,12 +189,11 @@ static int _printed_clustered_vg_advice = 0;
  * Case c covers the other errors returned when reading the VG.
  *   If *skip is 1, it's OK for the caller to read the list of PVs in the VG.
  */
-static int _ignore_vg(struct cmd_context *cmd,
-		      uint32_t error_flags, struct volume_group *error_vg,
-		      const char *vg_name, struct dm_list *arg_vgnames,
-		      uint32_t read_flags, int *skip, int *notfound)
+static int _ignore_vg(struct volume_group *vg, const char *vg_name,
+		      struct dm_list *arg_vgnames, uint32_t read_flags,
+		      int *skip, int *notfound)
 {
-	uint32_t read_error = error_flags;
+	uint32_t read_error = vg_read_error(vg);
 
 	*skip = 0;
 	*notfound = 0;
@@ -204,9 +203,12 @@ static int _ignore_vg(struct cmd_context *cmd,
 		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) {
-		if (arg_vgnames && str_list_match_item(arg_vgnames, vg_name)) {
-			log_error("Cannot access clustered VG %s.", vg_name);
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
+			log_error("Cannot access clustered VG %s.", vg->name);
 			if (!_printed_clustered_vg_advice) {
 				_printed_clustered_vg_advice = 1;
 				log_error("See lvmlockd(8) for changing a clvm/clustered VG to a shared VG.");
@@ -231,13 +233,10 @@ static int _ignore_vg(struct cmd_context *cmd,
 	 * would expect to fail.
 	 */
 	if (read_error & FAILED_SYSTEMID) {
-		if (arg_vgnames && str_list_match_item(arg_vgnames, vg_name)) {
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
 			log_error("Cannot access VG %s with system ID %s with %slocal system ID%s%s.",
-				  vg_name,
-				  error_vg ? error_vg->system_id : "unknown ",
-				  cmd->system_id ? "" : "unknown ",
-				  cmd->system_id ? " " : "",
-				  cmd->system_id ? cmd->system_id : "");
+				  vg->name, vg->system_id, vg->cmd->system_id ? "" : "unknown ",
+				  vg->cmd->system_id ? " " : "", vg->cmd->system_id ? vg->cmd->system_id : "");
 			return 1;
 		} else {
 			read_error &= ~FAILED_SYSTEMID; /* Check for other errors */
@@ -256,11 +255,10 @@ static int _ignore_vg(struct cmd_context *cmd,
 	 * command failed to acquire the necessary lock.)
 	 */
 	if (read_error & (FAILED_LOCK_TYPE | FAILED_LOCK_MODE)) {
-		if (arg_vgnames && str_list_match_item(arg_vgnames, vg_name)) {
+		if (arg_vgnames && str_list_match_item(arg_vgnames, vg->name)) {
 			if (read_error & FAILED_LOCK_TYPE)
 				log_error("Cannot access VG %s with lock type %s that requires lvmlockd.",
-					  vg_name,
-					  error_vg ? error_vg->lock_type : "unknown");
+					  vg->name, vg->lock_type);
 			/* For FAILED_LOCK_MODE, the error is printed in vg_read. */
 			return 1;
 		} else {
@@ -1926,12 +1924,10 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 	log_report_t saved_log_report_state = log_get_report_state();
 	char uuid[64] __attribute__((aligned(8)));
 	struct volume_group *vg;
-	struct volume_group *error_vg = NULL;
 	struct vgnameid_list *vgnl;
 	const char *vg_name;
 	const char *vg_uuid;
 	uint32_t lockd_state = 0;
-	uint32_t error_flags = 0;
 	int whole_selected = 0;
 	int ret_max = ECMD_PROCESSED;
 	int ret;
@@ -1981,18 +1977,13 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 			continue;
 		}
 
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state, &error_flags, &error_vg);
-		if (_ignore_vg(cmd, error_flags, error_vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
-			if (error_vg)
-				unlock_and_release_vg(cmd, error_vg, vg_name);
 			goto endvg;
 		}
-		if (error_vg)
-			unlock_and_release_vg(cmd, error_vg, vg_name);
-
 		if (skip || notfound)
 			goto endvg;
 
@@ -2013,7 +2004,8 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 				ret_max = ret;
 		}
 
-		unlock_vg(cmd, vg, vg_name);
+		if (!vg_read_error(vg))
+			unlock_vg(cmd, vg, vg_name);
 endvg:
 		release_vg(vg);
 		if (!lockd_vg(cmd, vg_name, "un", 0, &lockd_state))
@@ -2242,7 +2234,7 @@ int process_each_vg(struct cmd_context *cmd,
 	/*
 	 * Needed for a current listing of the global VG namespace.
 	 */
-	if (process_all_vgs_on_system && !lockd_gl(cmd, "sh", 0)) {
+	if (process_all_vgs_on_system && !lock_global(cmd, "sh")) {
 		ret_max = ECMD_FAILED;
 		goto_out;
 	}
@@ -2251,7 +2243,8 @@ int process_each_vg(struct cmd_context *cmd,
 	 * Scan all devices to populate lvmcache with initial
 	 * list of PVs and VGs.
 	 */
-	lvmcache_label_scan(cmd);
+	if (!(read_flags & PROCESS_SKIP_SCAN))
+		lvmcache_label_scan(cmd);
 
 	/*
 	 * A list of all VGs on the system is needed when:
@@ -3597,13 +3590,11 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag
 	log_report_t saved_log_report_state = log_get_report_state();
 	char uuid[64] __attribute__((aligned(8)));
 	struct volume_group *vg;
-	struct volume_group *error_vg = NULL;
 	struct vgnameid_list *vgnl;
 	struct dm_str_list *sl;
 	struct dm_list *tags_arg;
 	struct dm_list lvnames;
 	uint32_t lockd_state = 0;
-	uint32_t error_flags = 0;
 	const char *vg_name;
 	const char *vg_uuid;
 	const char *vgn;
@@ -3672,18 +3663,13 @@ static int _process_lv_vgnameid_list(struct cmd_context *cmd, uint32_t read_flag
 			continue;
 		}
 
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state, &error_flags, &error_vg);
-		if (_ignore_vg(cmd, error_flags, error_vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
-			if (error_vg)
-				unlock_and_release_vg(cmd, error_vg, vg_name);
 			goto endvg;
 		}
-		if (error_vg)
-			unlock_and_release_vg(cmd, error_vg, vg_name);
-
 		if (skip || notfound)
 			goto endvg;
 
@@ -3787,7 +3773,7 @@ int process_each_lv(struct cmd_context *cmd,
 	/*
 	 * Needed for a current listing of the global VG namespace.
 	 */
-	if (process_all_vgs_on_system && !lockd_gl(cmd, "sh", 0)) {
+	if (process_all_vgs_on_system && !lock_global(cmd, "sh")) {
 		ret_max = ECMD_FAILED;
 		goto_out;
 	}
@@ -3947,14 +3933,14 @@ static int _get_all_devices(struct cmd_context *cmd,
 
 			if (!(dil = dm_pool_alloc(cmd->mem, sizeof(*dil)))) {
 				log_error("device_id_list alloc failed.");
-				goto out;
+				return ECMD_FAILED;
 			}
 
 			strncpy(dil->pvid, hint->pvid, ID_LEN);
 			dil->dev = dev;
 			dm_list_add(all_devices, &dil->list);
 		}
-		return 1;
+		return ECMD_PROCESSED;
 	}
 
 	log_debug("Getting list of all devices from system");
@@ -4007,59 +3993,6 @@ static struct device_id_list *_device_list_find_dev(struct dm_list *devices, str
 	return NULL;
 }
 
-static int _device_list_copy(struct cmd_context *cmd, struct dm_list *src, struct dm_list *dst)
-{
-	struct device_id_list *dil;
-	struct device_id_list *dil_new;
-
-	dm_list_iterate_items(dil, src) {
-		if (!(dil_new = dm_pool_alloc(cmd->mem, sizeof(*dil_new)))) {
-			log_error("device_id_list alloc failed.");
-			return ECMD_FAILED;
-		}
-
-		dil_new->dev = dil->dev;
-		strncpy(dil_new->pvid, dil->pvid, ID_LEN);
-		dm_list_add(dst, &dil_new->list);
-	}
-
-	return ECMD_PROCESSED;
-}
-
-/*
- * For each device in arg_devices or all_devices that has a pvid, add a copy of
- * that device to arg_missed.  All PVs (devices with a pvid) should have been
- * found while processing all VGs (including orphan VGs).  But, some may have
- * been missed if VGs were changing at the same time.  This function creates a
- * list of PVs that still remain in the given list, i.e. were missed the first
- * time.  A second iteration through VGs can look for these explicitly.
- * (arg_devices is used if specific PVs are being processed; all_devices is
- * used if all devs are being processed)
- */
-static int _get_missed_pvs(struct cmd_context *cmd,
-			   struct dm_list *devices,
-			   struct dm_list *arg_missed)
-{
-	struct device_id_list *dil;
-	struct device_id_list *dil_missed;
-
-	dm_list_iterate_items(dil, devices) {
-		if (!dil->pvid[0])
-			continue;
-
-		if (!(dil_missed = dm_pool_alloc(cmd->mem, sizeof(*dil_missed)))) {
-			log_error("device_id_list alloc failed.");
-			return ECMD_FAILED;
-		}
-
-		dil_missed->dev = dil->dev;
-		strncpy(dil_missed->pvid, dil->pvid, ID_LEN);
-		dm_list_add(arg_missed, &dil_missed->list);
-	}
-
-	return ECMD_PROCESSED;
-}
-
 static int _process_device_list(struct cmd_context *cmd, struct dm_list *all_devices,
 				struct processing_handle *handle,
 				process_single_pv_fn_t process_single_pv)
@@ -4223,16 +4156,12 @@ static int _process_pvs_in_vg(struct cmd_context *cmd,
 	struct physical_volume *pv;
 	struct pv_list *pvl;
 	struct device_id_list *dil;
-	struct device_list *devl;
-	struct dm_list outdated_devs;
 	const char *pv_name;
 	int process_pv;
 	int do_report_ret_code = 1;
 	int ret_max = ECMD_PROCESSED;
 	int ret = 0;
 
-	dm_list_init(&outdated_devs);
-
 	log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_PV);
 
 	vg_uuid[0] = '\0';
@@ -4318,12 +4247,6 @@ static int _process_pvs_in_vg(struct cmd_context *cmd,
 			break;
 		log_set_report_object_name_and_id(NULL, NULL);
 	}
-
-	if (!is_orphan_vg(vg->name))
-		lvmcache_get_outdated_devs(cmd, vg->name, (const char *)&vg->id, &outdated_devs);
-	dm_list_iterate_items(devl, &outdated_devs)
-		_device_list_remove(all_devices, devl->dev);
-
 	do_report_ret_code = 0;
 out:
 	if (do_report_ret_code)
@@ -4361,17 +4284,14 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 	log_report_t saved_log_report_state = log_get_report_state();
 	char uuid[64] __attribute__((aligned(8)));
 	struct volume_group *vg;
-	struct volume_group *error_vg;
 	struct vgnameid_list *vgnl;
 	const char *vg_name;
 	const char *vg_uuid;
 	uint32_t lockd_state = 0;
-	uint32_t error_flags = 0;
 	int ret_max = ECMD_PROCESSED;
 	int ret;
 	int skip;
 	int notfound;
-	int skip_lock;
 	int do_report_ret_code = 1;
 
 	log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_VG);
@@ -4405,10 +4325,8 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 
 		log_debug("Processing PVs in VG %s", vg_name);
 
-		skip_lock = is_orphan_vg(vg_name) && (read_flags & PROCESS_SKIP_ORPHAN_LOCK);
-
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state, &error_flags, &error_vg);
-		if (_ignore_vg(cmd, error_flags, error_vg, vg_name, NULL, read_flags, &skip, &notfound)) {
+		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
+		if (_ignore_vg(vg, vg_name, NULL, read_flags, &skip, &notfound)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
@@ -4420,26 +4338,22 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 			goto endvg;
 		
 		/*
-		 * Don't call "continue" when skip is set, because we need to remove
-		 * error_vg->pvs entries from devices list.
+		 * Don't continue when skip is set, because we need to remove
+		 * vg->pvs entries from devices list.
 		 */
 		
-		ret = _process_pvs_in_vg(cmd, vg ? vg : error_vg, all_devices, arg_devices, arg_tags,
+		ret = _process_pvs_in_vg(cmd, vg, all_devices, arg_devices, arg_tags,
 					 process_all_pvs, process_all_devices, skip,
 					 handle, process_single_pv);
 		if (ret != ECMD_PROCESSED)
 			stack;
-
 		report_log_ret_code(ret);
-
 		if (ret > ret_max)
 			ret_max = ret;
 
-		if (!skip && !skip_lock)
+		if (!skip)
 			unlock_vg(cmd, vg, vg->name);
 endvg:
-		if (error_vg)
-			unlock_and_release_vg(cmd, error_vg, vg_name);
 		release_vg(vg);
 		if (!lockd_vg(cmd, vg_name, "un", 0, &lockd_state))
 			stack;
@@ -4470,7 +4384,6 @@ int process_each_pv(struct cmd_context *cmd,
 	struct dm_list arg_tags;	/* str_list */
 	struct dm_list arg_pvnames;	/* str_list */
 	struct dm_list arg_devices;	/* device_id_list */
-	struct dm_list arg_missed;	/* device_id_list */
 	struct dm_list all_vgnameids;	/* vgnameid_list */
 	struct dm_list all_devices;	/* device_id_list */
 	struct device_id_list *dil;
@@ -4501,7 +4414,6 @@ int process_each_pv(struct cmd_context *cmd,
 	dm_list_init(&arg_tags);
 	dm_list_init(&arg_pvnames);
 	dm_list_init(&arg_devices);
-	dm_list_init(&arg_missed);
 	dm_list_init(&all_vgnameids);
 	dm_list_init(&all_devices);
 
@@ -4528,7 +4440,7 @@ int process_each_pv(struct cmd_context *cmd,
 	process_all_devices = process_all_pvs && (cmd->cname->flags & ENABLE_ALL_DEVS) && all_is_set;
 
 	/* Needed for a current listing of the global VG namespace. */
-	if (!only_this_vgname && !lockd_gl(cmd, "sh", 0)) {
+	if (!only_this_vgname && !lock_global(cmd, "sh")) {
 		ret_max = ECMD_FAILED;
 		goto_out;
 	}
@@ -4604,54 +4516,6 @@ int process_each_pv(struct cmd_context *cmd,
 	if (ret > ret_max)
 		ret_max = ret;
 
-	/*
-	 * If the orphans lock was held, there shouldn't be missed devices.
-	 */
-	if (read_flags & PROCESS_SKIP_ORPHAN_LOCK)
-		goto skip_missed;
-
-	/*
-	 * Some PVs may have been missed by the first search if another command
-	 * moved them at the same time.  Repeat the search for only the
-	 * specific PVs missed.  lvmcache needs clearing for a fresh search.
-	 *
-	 * If missed PVs are found in this repeated search, they are removed
-	 * from the arg_missed list, but they also need to be removed from the
-	 * arg_devices list, otherwise the check at the end will produce an
-	 * error, thinking they weren't found.  This is the reason for saving
-	 * and comparing the original arg_missed list.
-	 */
-	if (!process_all_pvs)
-		_get_missed_pvs(cmd, &arg_devices, &arg_missed);
-	else
-		_get_missed_pvs(cmd, &all_devices, &arg_missed);
-
-	if (!dm_list_empty(&arg_missed)) {
-		struct dm_list arg_missed_orig;
-
-		dm_list_init(&arg_missed_orig);
-		_device_list_copy(cmd, &arg_missed, &arg_missed_orig);
-
-		log_warn("WARNING: some PVs were not found in first search, retrying.");
-
-		lvmcache_label_scan(cmd);
-
-		ret = _process_pvs_in_vgs(cmd, read_flags, &all_vgnameids, &all_devices,
-					  &arg_missed, &arg_tags, 0, 0,
-					  handle, process_single_pv);
-		if (ret != ECMD_PROCESSED)
-			stack;
-		if (ret > ret_max)
-			ret_max = ret;
-
-		/* Devices removed from arg_missed are removed from arg_devices. */
-		dm_list_iterate_items(dil, &arg_missed_orig) {
-			if (!_device_list_find_dev(&arg_missed, dil->dev))
-				_device_list_remove(&arg_devices, dil->dev);
-		}
-	}
-
-skip_missed:
 	dm_list_iterate_items(dil, &arg_devices) {
 		log_error("Failed to find physical volume \"%s\".", dev_name(dil->dev));
 		ret_max = ECMD_FAILED;
@@ -5413,9 +5277,6 @@ static int _pvremove_check_single(struct cmd_context *cmd,
  * This function returns 1 (success) if the caller requires all specified
  * devices to be created, and all are created, or if the caller does not
  * require all specified devices to be created and one or more were created.
- *
- * When this function returns 1 (success), it returns to the caller with the
- * VG_ORPHANS write lock held.
  */
 
 int pvcreate_each_device(struct cmd_context *cmd,
@@ -5466,18 +5327,6 @@ int pvcreate_each_device(struct cmd_context *cmd,
 		dm_list_add(&pp->arg_devices, &pd->list);
 	}
 
-	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
-		log_error("Can't get lock for orphan PVs.");
-		return 0;
-	}
-
-	/*
-	 * Scan before calling process_each_pv so we can set up the PV args
-	 * first.  We can then skip the scan that would normally occur at the
-	 * beginning of process_each_pv.
-	 */
-	lvmcache_label_scan(cmd);
-
 	/*
 	 * Translate arg names into struct device's.
 	 */
@@ -5499,7 +5348,7 @@ int pvcreate_each_device(struct cmd_context *cmd,
 	 * If it's added to arg_process but needs a prompt or force option, then
 	 * a corresponding prompt entry is added to pp->prompts.
 	 */
-	process_each_pv(cmd, 0, NULL, NULL, 1, PROCESS_SKIP_SCAN | PROCESS_SKIP_ORPHAN_LOCK,
+	process_each_pv(cmd, 0, NULL, NULL, 1, PROCESS_SKIP_SCAN,
 			handle, pp->is_remove ? _pvremove_check_single : _pvcreate_check_single);
 
 	/*
@@ -5588,7 +5437,8 @@ int pvcreate_each_device(struct cmd_context *cmd,
 	 * from the user, reacquire the lock, verify that the PVs were not used
 	 * during the wait, then do the create steps.
 	 */
-	unlock_vg(cmd, NULL, VG_ORPHANS);
+
+	lockf_global(cmd, "un");
 
 	/*
 	 * Process prompts that require asking the user.  The orphans lock is
@@ -5626,9 +5476,10 @@ int pvcreate_each_device(struct cmd_context *cmd,
 	 * finds them changed, or can't find them any more, then they aren't
 	 * used.
 	 */
-	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
-		log_error("Can't get lock for orphan PVs.");
-		goto out;
+
+	if (!lockf_global(cmd, "ex")) {
+		log_error("Failed to reacquire global lock after prompt.");
+		goto_out;
 	}
 
 	lvmcache_label_scan(cmd);
@@ -5647,7 +5498,7 @@ int pvcreate_each_device(struct cmd_context *cmd,
 	 */
 	dm_list_splice(&pp->arg_confirm, &pp->arg_process);
 
-	process_each_pv(cmd, 0, NULL, NULL, 1, PROCESS_SKIP_SCAN | PROCESS_SKIP_ORPHAN_LOCK,
+	process_each_pv(cmd, 0, NULL, NULL, 1, PROCESS_SKIP_SCAN,
 			handle, _pv_confirm_single);
 
 	dm_list_iterate_items(pd, &pp->arg_confirm)
@@ -5723,7 +5574,7 @@ do_command:
 	if (pp->preserve_existing && pp->orphan_vg_name) {
 		log_debug("Using existing orphan PVs in %s.", pp->orphan_vg_name);
 
-		if (!(orphan_vg = vg_read_orphans(cmd, pp->orphan_vg_name))) {
+		if (!(orphan_vg = vg_read_orphans(cmd, 0, pp->orphan_vg_name))) {
 			log_error("Cannot read orphans VG %s.", pp->orphan_vg_name);
 			goto bad;
 		}
@@ -5865,16 +5716,10 @@ do_command:
 			  cmd->command->name, pd->name);
 
 	if (!dm_list_empty(&pp->arg_fail))
-		goto_bad;
+		goto_out;
 
-	/*
-	 * Returns with VG_ORPHANS write lock held because vgcreate and
-	 * vgextend want to use the newly created PVs.
-	 */
 	return 1;
-
 bad:
-	unlock_vg(cmd, NULL, VG_ORPHANS);
 out:
 	return 0;
 }

tools/vgcfgbackup.c

@@ -67,12 +67,9 @@ static int _vg_backup_single(struct cmd_context *cmd, const char *vg_name,
 		if (!backup_to_file(filename, vg->cmd->cmd_line, vg))
 			return_ECMD_FAILED;
 	} else {
-		if (vg_missing_pv_count(vg)) {
-			log_error("No backup taken: specify filename with -f to backup with missing PVs.");
-			return ECMD_FAILED;
-		}
-		if (vg_has_unknown_segments(vg)) {
-			log_error("No backup taken: specify filename with -f to backup with unknown segments.");
+		if (vg_read_error(vg) == FAILED_INCONSISTENT) {
+			log_error("No backup taken: specify filename with -f "
+				  "to backup an inconsistent VG");
 			return ECMD_FAILED;
 		}
 
@@ -100,17 +97,9 @@ int vgcfgbackup(struct cmd_context *cmd, int argc, char **argv)
 
 	handle->custom_handle = &last_filename;
 
-	/*
-	 * Just set so that we can do the check ourselves above and
-	 * report a helpful error message in place of the error message
-	 * that would be generated from vg_read.
-	 */
-	cmd->handles_missing_pvs = 1;
-	cmd->handles_unknown_segments = 1;
-
 	init_pvmove(1);
 
-	ret = process_each_vg(cmd, argc, argv, NULL, NULL, 0, 0,
+	ret = process_each_vg(cmd, argc, argv, NULL, NULL, READ_ALLOW_INCONSISTENT, 0,
 			      handle, &_vg_backup_single);
 
 	free(last_filename);

tools/vgcfgrestore.c

@@ -119,14 +119,11 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
 		}
 	}
 
-	if (!lock_vol(cmd, VG_ORPHANS, LCK_VG_WRITE, NULL)) {
-		log_error("Unable to lock orphans.");
+	if (!lock_global(cmd, "ex"))
 		return ECMD_FAILED;
-	}
 
 	if (!lock_vol(cmd, vg_name, LCK_VG_WRITE, NULL)) {
 		log_error("Unable to lock volume group %s.", vg_name);
-		unlock_vg(cmd, NULL, VG_ORPHANS);
 		return ECMD_FAILED;
 	}
 
@@ -142,7 +139,6 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
 				       arg_count(cmd, force_long_ARG)) :
 	      backup_restore(cmd, vg_name, arg_count(cmd, force_long_ARG)))) {
 		unlock_vg(cmd, NULL, vg_name);
-		unlock_vg(cmd, NULL, VG_ORPHANS);
 		log_error("Restore failed.");
 		ret = ECMD_FAILED;
 		goto out;
@@ -151,7 +147,6 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
 	ret = ECMD_PROCESSED;
 	log_print_unless_silent("Restored volume group %s.", vg_name);
 
-	unlock_vg(cmd, NULL, VG_ORPHANS);
 	unlock_vg(cmd, NULL, vg_name);
 out:
 	return ret;

tools/vgchange.c

@@ -1075,7 +1075,7 @@ int vgchange_locktype_cmd(struct cmd_context *cmd, int argc, char **argv)
 	 * on other hosts, to cause them to see the new system_id or
 	 * lock_type.
 	 */
-	if (!lockd_gl(cmd, "ex", LDGL_UPDATE_NAMES))
+	if (!lockd_global(cmd, "ex"))
 		return 0;
 
 process:
@@ -1138,7 +1138,7 @@ int vgchange_lock_start_stop_cmd(struct cmd_context *cmd, int argc, char **argv)
 	if (arg_is_set(cmd, lockstart_ARG)) {
 		cmd->lockd_vg_disable = 1;
 
-		if (!lockd_gl(cmd, "sh", 0))
+		if (!lockd_global(cmd, "sh"))
 			log_debug("No global lock for lock start");
 
 		/* Disable the lockd_gl in process_each_vg. */
@@ -1154,7 +1154,7 @@ int vgchange_lock_start_stop_cmd(struct cmd_context *cmd, int argc, char **argv)
 	if (arg_is_set(cmd, lockstart_ARG) && vp.lock_start_count) {
 		const char *start_opt = arg_str_value(cmd, lockopt_ARG, NULL);
 
-		if (!lockd_gl(cmd, "un", 0))
+		if (!lockd_global(cmd, "un"))
 			stack;
 
 		if (!start_opt || !strcmp(start_opt, "auto")) {
@@ -1210,7 +1210,7 @@ int vgchange_systemid_cmd(struct cmd_context *cmd, int argc, char **argv)
 	 * on other hosts, to cause them to see the new system_id or
 	 * lock_type.
 	 */
-	if (!lockd_gl(cmd, "ex", LDGL_UPDATE_NAMES))
+	if (!lockd_global(cmd, "ex"))
 		return 0;
 
 	if (!(handle = init_processing_handle(cmd, NULL))) {

tools/vgck.c

@@ -15,57 +15,6 @@
 
 #include "tools.h"
 
-/*
- * TODO: we cannot yet repair corruption in label_header, pv_header/locations,
- * or corruption of some mda_header fields.
- */
-
-static int _update_metadata_single(struct cmd_context *cmd __attribute__((unused)),
-		       const char *vg_name,
-		       struct volume_group *vg,
-		       struct processing_handle *handle __attribute__((unused)))
-{
-
-	/*
-	 * Simply calling vg_write can correct or clean up various things:
-	 * . some mda's have old versions of metdadata 
-	 * . wipe outdated PVs
-	 * . fix pv_header used flag and version
-	 * . strip historical lvs
-	 * . clear missing pv flag on unused PV
-	 */
-	if (!vg_write(vg)) {
-		log_error("Failed to write VG.");
-		return 0;
-	}
-
-	if (!vg_commit(vg)) {
-		log_error("Failed to commit VG.");
-		return 0;
-	}
-
-	/*
-	 * vg_write does not write to "bad" mdas (where "bad" is corrupt, can't
-	 * be processed when reading).  bad mdas are not kept in
-	 * fid->metadata_areas_in_use so vg_read and vg_write ignore them, but
-	 * they are saved in lvmcache.  this gets them from lvmcache and tries
-	 * to write this metadata to them.
-	 */
-	vg_write_commit_bad_mdas(cmd, vg);
-
-	return 1;
-}
-
-static int _update_metadata(struct cmd_context *cmd, int argc, char **argv)
-{
-	cmd->handles_missing_pvs = 1;
-	cmd->wipe_outdated_pvs = 1;
-	cmd->handles_unknown_segments = 1;
-
-	return process_each_vg(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, 0, NULL,
-			       &_update_metadata_single);
-}
-
 static int vgck_single(struct cmd_context *cmd __attribute__((unused)),
 		       const char *vg_name,
 		       struct volume_group *vg,
@@ -88,9 +37,6 @@ static int vgck_single(struct cmd_context *cmd __attribute__((unused)),
 
 int vgck(struct cmd_context *cmd, int argc, char **argv)
 {
-	if (arg_is_set(cmd, updatemetadata_ARG))
-		return _update_metadata(cmd, argc, argv);
-
 	return process_each_vg(cmd, argc, argv, NULL, NULL, 0, 0, NULL,
 			       &vgck_single);
 }

tools/vgcreate.c

@@ -56,19 +56,13 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 	if (!vgcreate_params_validate(cmd, &vp_new))
 		return EINVALID_CMD_LINE;
 
-	/*
-	 * Needed to change the global VG namespace,
-	 * and to change the set of orphan PVs.
-	 */
-	if (!lockd_gl_create(cmd, "ex", vp_new.lock_type))
+	if (!lockf_global(cmd, "ex"))
+		return_ECMD_FAILED;
+	if (!lockd_global_create(cmd, "ex", vp_new.lock_type))
 		return_ECMD_FAILED;
-	cmd->lockd_gl_disable = 1;
 
 	clear_hint_file(cmd);
 
-	/* Check for old md signatures at the end of devices. */
-	cmd->use_full_md_check = 1;
-
 	/*
 	 * Check if the VG name already exists.  This should be done before
 	 * creating PVs on any of the devices.
@@ -94,15 +88,6 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 		return ECMD_FAILED;
 	}
 
-	/*
-	 * FIXME: we have to unlock/relock the new VG name around the pvcreate
-	 * step because pvcreate needs to destroy lvmcache, which doesn't allow
-	 * any locks to be held.  There shouldn't be any reason to require this
-	 * VG lock to be released, so the lvmcache destroy rule about locks
-	 * seems to be unwarranted here.
-	 */
-	unlock_vg(cmd, NULL, vp_new.vg_name);
-
 	if (!(handle = init_processing_handle(cmd, NULL))) {
 		log_error("Failed to initialize processing handle.");
 		return ECMD_FAILED;
@@ -113,18 +98,6 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 		return_ECMD_FAILED;
 	}
 
-	/* Relock the new VG name, see comment above. */
-	if (!lock_vol(cmd, vp_new.vg_name, LCK_VG_WRITE, NULL)) {
-		destroy_processing_handle(cmd, handle);
-		return_ECMD_FAILED;
-	}
-
-	/*
-	 * pvcreate_each_device returns with the VG_ORPHANS write lock held,
-	 * which was used to do pvcreate.  Now to create the VG using those
-	 * PVs, the VG lock will be taken (with the orphan lock already held.)
-	 */
-
 	if (!(vg = vg_create(cmd, vp_new.vg_name)))
 		goto_bad;
 
@@ -186,7 +159,6 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 		goto_bad;
 	}
 
-	unlock_vg(cmd, NULL, VG_ORPHANS);
 	unlock_vg(cmd, vg, vp_new.vg_name);
 
 	backup(vg);
@@ -209,7 +181,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 			goto out;
 		}
 
-		lockd_gl(cmd, "un", 0);
+		lock_global(cmd, "un");
 
 		if (!start_opt || !strcmp(start_opt, "wait")) {
 			/* It is OK if the user does Ctrl-C to cancel the wait. */
@@ -228,7 +200,6 @@ out:
 
 bad:
 	unlock_vg(cmd, vg, vp_new.vg_name);
-	unlock_vg(cmd, NULL, VG_ORPHANS);
 	release_vg(vg);
 	destroy_processing_handle(cmd, handle);
 	return ECMD_FAILED;