pre-release

The man page generation for pvchange/lvchange/vgchange was
incorrect (leaving out some option listings) as a result of
commit e225bf5 "fix command definition for pvchange -a"

VERSION

@@ -1 +1 @@
-2.03.02(2)-git (2018-10-31)
+2.03.05(2) (2019-06-15)

VERSION_DM

@@ -1 +1 @@
-1.02.155-git (2018-10-31)
+1.02.163 (2019-06-15)

WHATS_NEW

@@ -1,5 +1,20 @@
-Version 2.03.02 - 
-===================================
+Version 2.03.05 - 15th June 2019
+================================
+  Fix command definition for pvchange -a.
+  Add vgck --updatemetadata command that will repair metadata problems.
+  Improve VG reading to work if one good copy of metadata is found.
+  Report/display/scan commands that read VGs will no longer write/repair.
+  Move metadata repairs from VG reading to VG writing.
+  Add config setting md_component_checks to control MD component checks.
+  Add end of device MD component checks when dev has no udev info.
+
+Version 2.03.04 - 10th June 2019
+================================
+  Remove unused_duplicate_devs from cmd causing segfault in dmeventd.
+
+Version 2.03.03 - 07th June 2019
+================================
+  Report no_discard_passdown for cache LVs with lvs -o+kernel_discards.
   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.
@@ -34,10 +49,14 @@ Version 2.03.02 -
   Fix missing unlock on lvm2 dmeventd plugin error path initialization.
   Improve Makefile dependency tracking.
   Move VDO support towards V2 target (6.2) support.
+
+Version 2.03.02 - 18th December 2018
+====================================
   Fix missing proper initialization of pv_list struct when adding pv.
-  Fix (de)activation of RaidLVs with visible SubLVs
-  Prohibit mirrored 'mirror' log via lvcreate and lvconvert
+  Fix (de)activation of RaidLVs with visible SubLVs.
+  Prohibit mirrored 'mirror' log via lvcreate and lvconvert.
   Use sync io if async io_setup fails, or use_aio=0 is set in config.
+  Fix more issues reported by coverity scan.
 
 Version 2.03.01 - 31st October 2018
 ===================================

WHATS_NEW_DM

@@ -1,10 +1,21 @@
-Version 1.02.155 - 
-====================================
+Version 1.02.163 - 15th June 2019
+=================================
+
+Version 1.02.161 - 10th June 2019
+=================================
+
+Version 1.02.159 - 07th June 2019
+=================================
+  Parsing of cache status understand no_discard_passdown.
   Ensure migration_threshold for cache is at least 8 chunks.
+
+Version 1.02.155 - 18th December 2018
+=====================================
   Include correct internal header inside libdm list.c.
   Enhance ioctl flattening and add parameters only when needed.
   Add DM_DEVICE_ARM_POLL for API completness matching kernel.
   Do not add parameters for RESUME with DM_DEVICE_CREATE dm task.
+  Fix dmstats report printing no output.
 
 Version 1.02.153 - 31st October 2018
 ====================================

conf/example.conf.in

@@ -88,6 +88,22 @@ devices {
 	# 
 	external_device_info_source = "none"
 
+	# Configuration option devices/hints.
+	# Use a local file to remember which devices have PVs on them.
+	# Some commands will use this as an optimization to reduce device
+	# scanning, and will only scan the listed PVs. Removing the hint file
+	# will cause lvm to generate a new one. Disable hints if PVs will
+	# be copied onto devices using non-lvm commands, like dd.
+	# 
+	# Accepted values:
+	#   all
+	#     Use all hints.
+	#   none
+	#     Use no hints.
+	# 
+	# This configuration option has an automatic default value.
+	# hints = "all"
+
 	# Configuration option devices/preferred_names.
 	# Select which path name to display for a block device.
 	# If multiple path names exist for a block device, and LVM needs to
@@ -167,17 +183,52 @@ devices {
 	sysfs_scan = 1
 
 	# Configuration option devices/scan_lvs.
-	# Scan LVM LVs for layered PVs.
-	scan_lvs = 1
+	# Scan LVM LVs for layered PVs, allowing LVs to be used as PVs.
+	# When 1, LVM will detect PVs layered on LVs, and caution must be
+	# taken to avoid a host accessing a layered VG that may not belong
+	# to it, e.g. from a guest image. This generally requires excluding
+	# the LVs with device filters. Also, when this setting is enabled,
+	# every LVM command will scan every active LV on the system (unless
+	# filtered), which can cause performance problems on systems with
+	# many active LVs. When this setting is 0, LVM will not detect or
+	# use PVs that exist on LVs, and will not allow a PV to be created on
+	# an LV. The LVs are ignored using a built in device filter that
+	# identifies and excludes LVs.
+	scan_lvs = 0
 
 	# Configuration option devices/multipath_component_detection.
 	# Ignore devices that are components of DM multipath devices.
 	multipath_component_detection = 1
 
 	# Configuration option devices/md_component_detection.
-	# Ignore devices that are components of software RAID (md) devices.
+	# Enable detection and exclusion of MD component devices.
+	# An MD component device is a block device that MD uses as part
+	# of a software RAID virtual device. When an LVM PV is created
+	# on an MD device, LVM must only use the top level MD device as
+	# the PV, and should ignore the underlying component devices.
+	# In cases where the MD superblock is located at the end of the
+	# component devices, it is more difficult for LVM to consistently
+	# identify an MD component, see the md_component_checks setting.
 	md_component_detection = 1
 
+	# Configuration option devices/md_component_checks.
+	# The checks LVM should use to detect MD component devices.
+	# MD component devices are block devices used by MD software RAID.
+	# 
+	# Accepted values:
+	#   auto
+	#     LVM will skip scanning the end of devices when it has other
+	#     indications that the device is not an MD component.
+	#   start
+	#     LVM will only scan the start of devices for MD superblocks.
+	#     This does not incur extra I/O by LVM.
+	#   full
+	#     LVM will scan the start and end of devices for MD superblocks.
+	#     This requires an extra read at the end of devices.
+	# 
+	# This configuration option has an automatic default value.
+	# md_component_checks = "auto"
+
 	# Configuration option devices/fw_raid_component_detection.
 	# Ignore devices that are components of firmware RAID devices.
 	# LVM must use an external_device_info_source other than none for this
@@ -720,7 +771,8 @@ log {
 
 	# Configuration option log/indent.
 	# Indent messages according to their severity.
-	indent = 1
+	# This configuration option has an automatic default value.
+	# indent = 0
 
 	# Configuration option log/command_names.
 	# Display the command name on each line of output.
@@ -746,6 +798,20 @@ log {
 	# available: memory, devices, io, activation, allocation,
 	# metadata, cache, locking, lvmpolld. Use "all" to see everything.
 	debug_classes = [ "memory", "devices", "io", "activation", "allocation", "metadata", "cache", "locking", "lvmpolld", "dbus" ]
+
+	# Configuration option log/debug_file_fields.
+	# The fields included in debug output written to log file.
+	# Use "all" to include everything (the default).
+	# This configuration option is advanced.
+	# This configuration option has an automatic default value.
+	# debug_file_fields = [ "time", "command", "fileline", "message" ]
+
+	# Configuration option log/debug_output_fields.
+	# The fields included in debug output written to stderr.
+	# Use "all" to include everything (the default).
+	# This configuration option is advanced.
+	# This configuration option has an automatic default value.
+	# debug_output_fields = [ "time", "command", "fileline", "message" ]
 }
 
 # Configuration section backup.
@@ -1174,6 +1240,16 @@ global {
 	# When enabled, an LVM command that changes PVs, changes VG metadata,
 	# or changes the activation state of an LV will send a notification.
 	notify_dbus = 1
+
+	# Configuration option global/io_memory_size.
+	# The amount of memory in KiB that LVM allocates to perform disk io.
+	# LVM performance may benefit from more io memory when there are many
+	# disks or VG metadata is large. Increasing this size may be necessary
+	# when a single copy of VG metadata is larger than the current setting.
+	# This value should usually not be decreased from the default; setting
+	# it too low can result in lvm failing to read VGs.
+	# This configuration option has an automatic default value.
+	# io_memory_size = 8192
 }
 
 # Configuration section activation.

configure

@@ -742,6 +742,7 @@ CLDNOWHOLEARCHIVE
 CLDFLAGS
 CACHE
 BUILD_DMFILEMAPD
+BUILD_LOCKDDLM_CONTROL
 BUILD_LOCKDDLM
 BUILD_LOCKDSANLOCK
 BUILD_LVMLOCKD
@@ -771,6 +772,8 @@ BLKID_LIBS
 BLKID_CFLAGS
 NOTIFY_DBUS_LIBS
 NOTIFY_DBUS_CFLAGS
+LOCKD_DLM_CONTROL_LIBS
+LOCKD_DLM_CONTROL_CFLAGS
 LOCKD_DLM_LIBS
 LOCKD_DLM_CFLAGS
 LOCKD_SANLOCK_LIBS
@@ -932,6 +935,7 @@ enable_devmapper
 enable_lvmpolld
 enable_lvmlockd_sanlock
 enable_lvmlockd_dlm
+enable_lvmlockd_dlmcontrol
 enable_use_lvmlockd
 with_lvmlockd_pidfile
 enable_use_lvmpolld
@@ -1000,6 +1004,8 @@ LOCKD_SANLOCK_CFLAGS
 LOCKD_SANLOCK_LIBS
 LOCKD_DLM_CFLAGS
 LOCKD_DLM_LIBS
+LOCKD_DLM_CONTROL_CFLAGS
+LOCKD_DLM_CONTROL_LIBS
 NOTIFY_DBUS_CFLAGS
 NOTIFY_DBUS_LIBS
 BLKID_CFLAGS
@@ -1643,6 +1649,8 @@ Optional Features:
   --enable-lvmlockd-sanlock
                           enable the LVM lock daemon using sanlock
   --enable-lvmlockd-dlm   enable the LVM lock daemon using dlm
+  --enable-lvmlockd-dlmcontrol
+                          enable lvmlockd remote refresh using libdlmcontrol
   --disable-use-lvmlockd  disable usage of LVM lock daemon
   --disable-use-lvmpolld  disable usage of LVM Poll Daemon
   --enable-dmfilemapd     enable the dmstats filemap daemon
@@ -1788,6 +1796,10 @@ Some influential environment variables:
               C compiler flags for LOCKD_DLM, overriding pkg-config
   LOCKD_DLM_LIBS
               linker flags for LOCKD_DLM, overriding pkg-config
+  LOCKD_DLM_CONTROL_CFLAGS
+              C compiler flags for LOCKD_DLM_CONTROL, overriding pkg-config
+  LOCKD_DLM_CONTROL_LIBS
+              linker flags for LOCKD_DLM_CONTROL, overriding pkg-config
   NOTIFY_DBUS_CFLAGS
               C compiler flags for NOTIFY_DBUS, overriding pkg-config
   NOTIFY_DBUS_LIBS
@@ -3077,6 +3089,7 @@ case "$host_os" in
 		BUILD_LVMPOLLD=no
 		LOCKDSANLOCK=no
 		LOCKDDLM=no
+		LOCKDDLM_CONTROL=no
 		ODIRECT=yes
 		DM_IOCTLS=yes
 		SELINUX=yes
@@ -10958,6 +10971,97 @@ $as_echo "#define LOCKDDLM_SUPPORT 1" >>confdefs.h
 	BUILD_LVMLOCKD=yes
 fi
 
+################################################################################
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build lvmlockddlmcontrol" >&5
+$as_echo_n "checking whether to build lvmlockddlmcontrol... " >&6; }
+# Check whether --enable-lvmlockd-dlmcontrol was given.
+if test "${enable_lvmlockd_dlmcontrol+set}" = set; then :
+  enableval=$enable_lvmlockd_dlmcontrol; LOCKDDLM_CONTROL=$enableval
+fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $LOCKDDLM_CONTROL" >&5
+$as_echo "$LOCKDDLM_CONTROL" >&6; }
+
+BUILD_LOCKDDLM_CONTROL=$LOCKDDLM_CONTROL
+
+if test "$BUILD_LOCKDDLM_CONTROL" = yes; then
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for LOCKD_DLM_CONTROL" >&5
+$as_echo_n "checking for LOCKD_DLM_CONTROL... " >&6; }
+
+if test -n "$LOCKD_DLM_CONTROL_CFLAGS"; then
+    pkg_cv_LOCKD_DLM_CONTROL_CFLAGS="$LOCKD_DLM_CONTROL_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+    if test -n "$PKG_CONFIG" && \
+    { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libdlmcontrol >= 3.2\""; } >&5
+  ($PKG_CONFIG --exists --print-errors "libdlmcontrol >= 3.2") 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; then
+  pkg_cv_LOCKD_DLM_CONTROL_CFLAGS=`$PKG_CONFIG --cflags "libdlmcontrol >= 3.2" 2>/dev/null`
+		      test "x$?" != "x0" && pkg_failed=yes
+else
+  pkg_failed=yes
+fi
+ else
+    pkg_failed=untried
+fi
+if test -n "$LOCKD_DLM_CONTROL_LIBS"; then
+    pkg_cv_LOCKD_DLM_CONTROL_LIBS="$LOCKD_DLM_CONTROL_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+    if test -n "$PKG_CONFIG" && \
+    { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libdlmcontrol >= 3.2\""; } >&5
+  ($PKG_CONFIG --exists --print-errors "libdlmcontrol >= 3.2") 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; then
+  pkg_cv_LOCKD_DLM_CONTROL_LIBS=`$PKG_CONFIG --libs "libdlmcontrol >= 3.2" 2>/dev/null`
+		      test "x$?" != "x0" && pkg_failed=yes
+else
+  pkg_failed=yes
+fi
+ else
+    pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+   	{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+        _pkg_short_errors_supported=yes
+else
+        _pkg_short_errors_supported=no
+fi
+        if test $_pkg_short_errors_supported = yes; then
+	        LOCKD_DLM_CONTROL_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libdlmcontrol >= 3.2" 2>&1`
+        else
+	        LOCKD_DLM_CONTROL_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libdlmcontrol >= 3.2" 2>&1`
+        fi
+	# Put the nasty error message in config.log where it belongs
+	echo "$LOCKD_DLM_CONTROL_PKG_ERRORS" >&5
+
+	$bailout
+elif test $pkg_failed = untried; then
+     	{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+	$bailout
+else
+	LOCKD_DLM_CONTROL_CFLAGS=$pkg_cv_LOCKD_DLM_CONTROL_CFLAGS
+	LOCKD_DLM_CONTROL_LIBS=$pkg_cv_LOCKD_DLM_CONTROL_LIBS
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+	HAVE_LOCKD_DLM_CONTROL=yes
+fi
+
+$as_echo "#define LOCKDDLM_CONTROL_SUPPORT 1" >>confdefs.h
+
+	BUILD_LVMLOCKD=yes
+fi
+
 ################################################################################
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build lvmlockd" >&5
 $as_echo_n "checking whether to build lvmlockd... " >&6; }
@@ -13784,6 +13888,7 @@ _ACEOF
 
 
 
+
 
 
 ################################################################################

device_mapper/all.h

@@ -903,6 +903,7 @@ int dm_tree_node_add_raid_target_with_params_v2(struct dm_tree_node *node,
 #define DM_CACHE_FEATURE_WRITETHROUGH 0x00000002
 #define DM_CACHE_FEATURE_PASSTHROUGH  0x00000004
 #define DM_CACHE_FEATURE_METADATA2    0x00000008 /* cache v1.10 */
+#define DM_CACHE_FEATURE_NO_DISCARD_PASSDOWN 0x00000010
 
 struct dm_config_node;
 /*

device_mapper/libdm-targets.c

@@ -296,6 +296,8 @@ int dm_get_status_cache(struct dm_pool *mem, const char *params,
 			s->feature_flags |= DM_CACHE_FEATURE_PASSTHROUGH;
 		else if (!strncmp(p, "metadata2 ", 10))
 			s->feature_flags |= DM_CACHE_FEATURE_METADATA2;
+		else if (!strncmp(p, "no_discard_passdown ", 20))
+			s->feature_flags |= DM_CACHE_FEATURE_NO_DISCARD_PASSDOWN;
 		else
 			log_error("Unknown feature in status: %s", params);
 

include/configure.h.in

@@ -295,9 +295,6 @@
 /* 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
 
@@ -537,6 +534,9 @@
 /* Locale-dependent data */
 #undef LOCALEDIR
 
+/* Define to 1 to include code that uses lvmlockd dlm control option. */
+#undef LOCKDDLM_CONTROL_SUPPORT
+
 /* Define to 1 to include code that uses lvmlockd dlm option. */
 #undef LOCKDDLM_SUPPORT
 

lib/cache/lvmcache.c

@@ -31,6 +31,7 @@ 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;
@@ -39,12 +40,19 @@ struct lvmcache_info {
 	uint32_t ext_version;   /* Extension version */
 	uint32_t ext_flags;	/* Extension flags */
 	uint32_t status;
+	bool mda1_bad;		/* label scan found bad metadata in mda1 */
+	bool mda2_bad;		/* label scan found bad metadata in mda2 */
+	bool summary_seqno_mismatch; /* two mdas on this dev has mismatching metadata */
+	int summary_seqno;      /* vg seqno found on this dev during scan */
+	int mda1_seqno;
+	int mda2_seqno;
 };
 
 /* 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;
@@ -57,7 +65,7 @@ struct lvmcache_vginfo {
 	uint32_t mda_checksum;
 	size_t mda_size;
 	int seqno;
-	int scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */
+	bool scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */
 };
 
 static struct dm_hash_table *_pvid_hash = NULL;
@@ -66,6 +74,7 @@ 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 DM_LIST_INIT(_prev_unused_duplicate_devs);
 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;
@@ -81,6 +90,7 @@ int lvmcache_init(struct cmd_context *cmd)
 	dm_list_init(&_vginfos);
 	dm_list_init(&_found_duplicate_devs);
 	dm_list_init(&_unused_duplicate_devs);
+	dm_list_init(&_prev_unused_duplicate_devs);
 
 	if (!(_vgname_hash = dm_hash_create(128)))
 		return 0;
@@ -107,11 +117,6 @@ void lvmcache_unlock_vgname(const char *vgname)
 	}
 }
 
-int lvmcache_vgs_locked(void)
-{
-	return _vgs_locked;
-}
-
 /*
  * When lvmcache sees a duplicate PV, this is set.
  * process_each_pv() can avoid searching for duplicates
@@ -173,6 +178,54 @@ static void _destroy_duplicate_device_list(struct dm_list *head)
 	dm_list_init(head);
 }
 
+bool 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 false;
+	}
+
+	if (info->mda1_bad || info->mda2_bad)
+		return true;
+	return false;
+}
+
+void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda)
+{
+	if (mda->mda_num == 1)
+		info->mda1_bad = true;
+	else if (mda->mda_num == 2)
+		info->mda2_bad = true;
+	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_mda_list)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+	struct mda_list *mdal;
+	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) {
+			if (!(mdal = zalloc(sizeof(*mdal))))
+				continue;
+			mdal->mda = mda;
+			dm_list_add(bad_mda_list, &mdal->list);
+		}
+	}
+}
+
 static void _vginfo_attach_info(struct lvmcache_vginfo *vginfo,
 				struct lvmcache_info *info)
 {
@@ -225,60 +278,6 @@ struct lvmcache_vginfo *lvmcache_vginfo_from_vgname(const char *vgname, const ch
 	return vginfo;
 }
 
-const struct format_type *lvmcache_fmt_from_vgname(struct cmd_context *cmd,
-						   const char *vgname, const char *vgid,
-						   unsigned revalidate_labels)
-{
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info;
-	struct dm_list *devh, *tmp;
-	struct dm_list devs;
-	struct device_list *devl;
-	char vgid_found[ID_LEN + 1] __attribute__((aligned(8)));
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-		stack;
-		return NULL;
-	}
-
-	/*
-	 * If this function is called repeatedly, only the first one needs to revalidate.
-	 */
-	if (!revalidate_labels)
-		goto out;
-
-	/*
-	 * This function is normally called before reading metadata so
- 	 * we check cached labels here. Unfortunately vginfo is volatile.
- 	 */
-	dm_list_init(&devs);
-	dm_list_iterate_items(info, &vginfo->infos) {
-		if (!(devl = malloc(sizeof(*devl)))) {
-			log_error("device_list element allocation failed");
-			return NULL;
-		}
-		devl->dev = info->dev;
-		dm_list_add(&devs, &devl->list);
-	}
-
-	memcpy(vgid_found, vginfo->vgid, sizeof(vgid_found));
-
-	dm_list_iterate_safe(devh, tmp, &devs) {
-		devl = dm_list_item(devh, struct device_list);
-		label_read(devl->dev);
-		dm_list_del(&devl->list);
-		free(devl);
-	}
-
-	/* If vginfo changed, caller needs to rescan */
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid_found)) ||
-	    strncmp(vginfo->vgid, vgid_found, ID_LEN))
-		return NULL;
-
-out:
-	return vginfo->fmt;
-}
-
 struct lvmcache_vginfo *lvmcache_vginfo_from_vgid(const char *vgid)
 {
 	struct lvmcache_vginfo *vginfo;
@@ -553,8 +552,11 @@ next:
 	 */
 
 	if (!(info = lvmcache_info_from_pvid(alt->dev->pvid, NULL, 0))) {
-		/* This shouldn't happen */
-		log_warn("WARNING: PV %s on duplicate device %s not found in cache.",
+		/*
+		 * This will happen if a duplicate dev has been dropped already,
+		 * e.g. it was found to be an md component.
+		 */
+		log_debug("PVID %s on duplicate device %s not found in cache.",
 			 alt->dev->pvid, dev_name(alt->dev));
 		goto next;
 	}
@@ -581,14 +583,14 @@ next:
 
 		if (!prev_unchosen1 && !prev_unchosen2) {
 			/*
-			 * The cmd list saves the unchosen preference across
+			 * The prev list saves the unchosen preference across
 			 * lvmcache_destroy.  Sometimes a single command will
 			 * fill lvmcache, destroy it, and refill it, and we
 			 * want the same duplicate preference to be preserved
 			 * in each instance of lvmcache for a single command.
 			 */
-			prev_unchosen1 = dev_in_device_list(dev1, &cmd->unused_duplicate_devs);
-			prev_unchosen2 = dev_in_device_list(dev2, &cmd->unused_duplicate_devs);
+			prev_unchosen1 = dev_in_device_list(dev1, &_prev_unused_duplicate_devs);
+			prev_unchosen2 = dev_in_device_list(dev2, &_prev_unused_duplicate_devs);
 		}
 
 		dev1_major = MAJOR(dev1->dev);
@@ -945,49 +947,26 @@ int lvmcache_label_scan(struct cmd_context *cmd)
 	return r;
 }
 
-/*
- * lvmcache_label_scan() detects duplicates in the basic label_scan(), then
- * filters out some dups, and chooses preferred duplicates to use.
- */
-
-void lvmcache_pvscan_duplicate_check(struct cmd_context *cmd)
-{
-	struct device_list *devl;
-
-	/* Check if label_scan() detected any dups. */
-	if (!_found_duplicate_pvs)
-		return;
-
-	/*
-	 * Once all the dups are identified, they are moved from the
-	 * "found" list to the "unused" list to sort out.
-	 */
-	dm_list_splice(&_unused_duplicate_devs, &_found_duplicate_devs);
-
-	/*
-	 * Remove items from the dups list that we know are the same
-	 * underlying dev, e.g. md components, that we want to just ignore.
-	 */
-	_filter_duplicate_devs(cmd);
-
-	/*
-	 * no more dups after ignoring some
-	 */
-	if (!_found_duplicate_pvs)
-		return;
-
-	/* Duplicates are found where we would have to pick one. */
-
-	dm_list_iterate_items(devl, &_unused_duplicate_devs)
-		log_warn("WARNING: found device with duplicate %s", dev_name(devl->dev));
-}
-
-int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal,
-			   struct dm_list *vgnameids)
+int lvmcache_get_vgnameids(struct cmd_context *cmd,
+			   struct dm_list *vgnameids,
+			   const char *only_this_vgname,
+			   int include_internal)
 {
 	struct vgnameid_list *vgnl;
 	struct lvmcache_vginfo *vginfo;
 
+	if (only_this_vgname) {
+		if (!(vgnl = dm_pool_alloc(cmd->mem, sizeof(*vgnl)))) {
+			log_error("vgnameid_list allocation failed.");
+			return 0;
+		}
+
+		vgnl->vg_name = dm_pool_strdup(cmd->mem, only_this_vgname);
+		vgnl->vgid = NULL;
+		dm_list_add(vgnameids, &vgnl->list);
+		return 1;
+	}
+
 	dm_list_iterate_items(vginfo, &_vginfos) {
 		if (!include_internal && is_orphan_vg(vginfo->vgname))
 			continue;
@@ -1011,49 +990,6 @@ int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal,
 	return 1;
 }
 
-struct dm_list *lvmcache_get_pvids(struct cmd_context *cmd, const char *vgname,
-				const char *vgid)
-{
-	struct dm_list *pvids;
-	struct lvmcache_vginfo *vginfo;
-	struct lvmcache_info *info;
-
-	if (!(pvids = str_list_create(cmd->mem))) {
-		log_error("pvids list allocation failed");
-		return NULL;
-	}
-
-	if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid)))
-		return pvids;
-
-	dm_list_iterate_items(info, &vginfo->infos) {
-		if (!str_list_add(cmd->mem, pvids,
-				  dm_pool_strdup(cmd->mem, info->dev->pvid))) {
-			log_error("strlist allocation failed");
-			return NULL;
-		}
-	}
-
-	return pvids;
-}
-
-int lvmcache_get_vg_devs(struct cmd_context *cmd,
-			 struct lvmcache_vginfo *vginfo,
-			 struct dm_list *devs)
-{
-	struct lvmcache_info *info;
-	struct device_list *devl;
-
-	dm_list_iterate_items(info, &vginfo->infos) {
-		if (!(devl = dm_pool_zalloc(cmd->mem, sizeof(*devl))))
-			return_0;
-
-		devl->dev = info->dev;
-		dm_list_add(devs, &devl->list);
-	}
-	return 1;
-}
-
 static struct device *_device_from_pvid(const struct id *pvid, uint64_t *label_sector)
 {
 	struct lvmcache_info *info;
@@ -1333,6 +1269,7 @@ 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.
@@ -1457,12 +1394,9 @@ int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt)
 }
 
 /*
- * 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.
+ * Returning 0 causes the caller to remove the info struct for this
+ * device from lvmcache, which will make it look like a missing device.
  */
-
 int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vgsummary *vgsummary)
 {
 	const char *vgname = vgsummary->vgname;
@@ -1488,6 +1422,7 @@ 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;
 	}
@@ -1496,6 +1431,7 @@ 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;
 	}
@@ -1511,56 +1447,140 @@ 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 = true;
+
+		} else if (info->summary_seqno < vgsummary->seqno) {
+			/* This mda has newer metadata than the prev mda on this dev. */
+			info->summary_seqno_mismatch = true;
+			info->summary_seqno = vgsummary->seqno;
+		}
+	}
+
+	/* this shouldn't happen */
 	if (!(vginfo = info->vginfo))
 		return 1;
 
 	if (!vginfo->seqno) {
 		vginfo->seqno = vgsummary->seqno;
-
-		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) {
-		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;
-		/* 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 (!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);
+		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;
 
-	} 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. */
+	} else if (vgsummary->seqno < vginfo->seqno) {
+		vginfo->scan_summary_mismatch = true;
+
+		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 = true;
+
+		/* 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 = true;
+			return 0;
+		}
+
+		/*
+		 * The seqno and checksum matches what was previously seen;
+		 * the summary values have already been saved in vginfo.
+		 */
 		return 1;
 	}
 
-	/*
-	 * If a dev has an unmatching checksum, ignore the other
-	 * info from it, keeping the info we already saved.
-	 */
+ update_vginfo:
 	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;
 }
 
-int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
+/*
+ * 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)
 {
 	struct pv_list *pvl;
 	struct lvmcache_info *info;
@@ -1584,6 +1604,110 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
 	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.
@@ -1635,7 +1759,7 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted)
  *   transient duplicate?
  */
 
-static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev)
+static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev, uint64_t label_sector)
 {
 	struct lvmcache_info *info;
 	struct label *label;
@@ -1648,6 +1772,9 @@ 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;
 
@@ -1663,8 +1790,9 @@ 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,
-				   const char *vgname, const char *vgid, uint32_t vgstatus)
+				   const char *pvid, struct device *dev, uint64_t label_sector,
+				   const char *vgname, const char *vgid, uint32_t vgstatus,
+				   int *is_duplicate)
 {
 	char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
 	char uuid[64] __attribute__((aligned(8)));
@@ -1692,7 +1820,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);
+		info = _create_info(labeller, dev, label_sector);
 		created = 1;
 	}
 
@@ -1724,6 +1852,8 @@ 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;
 		}
 
@@ -1828,8 +1958,8 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset)
 	dm_list_init(&_vginfos);
 
 	/*
-	 * Copy the current _unused_duplicate_devs into a cmd list before
-	 * destroying _unused_duplicate_devs.
+	 * Move the current _unused_duplicate_devs to _prev_unused_duplicate_devs
+	 * before destroying _unused_duplicate_devs.
 	 *
 	 * One command can init/populate/destroy lvmcache multiple times.  Each
 	 * time it will encounter duplicates and choose the preferrred devs.
@@ -1837,8 +1967,8 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset)
 	 * the unpreferred devs here so that _choose_preferred_devs can use
 	 * this to make the same choice each time.
 	 */
-	dm_list_init(&cmd->unused_duplicate_devs);
-	lvmcache_get_unused_duplicate_devs(cmd, &cmd->unused_duplicate_devs);
+	_destroy_duplicate_device_list(&_prev_unused_duplicate_devs);
+	dm_list_splice(&_prev_unused_duplicate_devs, &_unused_duplicate_devs);
 	_destroy_duplicate_device_list(&_unused_duplicate_devs);
 	_destroy_duplicate_device_list(&_found_duplicate_devs); /* should be empty anyway */
 	_found_duplicate_pvs = 0;
@@ -1867,6 +1997,14 @@ 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;
@@ -1940,6 +2078,10 @@ 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)
@@ -1957,9 +2099,10 @@ 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)
+		     uint64_t start, uint64_t size, unsigned ignored,
+		     struct metadata_area **mda_new)
 {
-	return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored);
+	return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored, mda_new);
 }
 
 int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size)
@@ -2127,26 +2270,6 @@ uint32_t lvmcache_ext_flags(struct lvmcache_info *info) {
 	return info->ext_flags;
 }
 
-int lvmcache_is_orphan(struct lvmcache_info *info) {
-	if (!info->vginfo)
-		return 1; /* FIXME? */
-	return is_orphan_vg(info->vginfo->vgname);
-}
-
-int lvmcache_vgid_is_cached(const char *vgid) {
-	struct lvmcache_vginfo *vginfo;
-
-	vginfo = lvmcache_vginfo_from_vgid(vgid);
-
-	if (!vginfo || !vginfo->vgname)
-		return 0;
-
-	if (is_orphan_vg(vginfo->vgname))
-		return 0;
-
-	return 1;
-}
-
 uint64_t lvmcache_smallest_mda_size(struct lvmcache_info *info)
 {
 	if (!info)
@@ -2263,17 +2386,17 @@ int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const ch
  *    lvmcache: info for dev5 is deleted, FIXME: use a defective state
  */
 
-int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid)
+bool lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid)
 {
 	struct lvmcache_vginfo *vginfo;
 
 	if (!vgname || !vgid)
-		return 1;
+		return true;
 
 	if ((vginfo = lvmcache_vginfo_from_vgid(vgid)))
 		return vginfo->scan_summary_mismatch;
 
-	return 1;
+	return true;
 }
 
 static uint64_t _max_metadata_size;
@@ -2332,3 +2455,117 @@ struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd,
 	return NULL;
 }
 
+/*
+ * This is used by the metadata repair command to check if
+ * the metadata on a dev needs repair because it's old.
+ */
+bool lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev)
+{
+	struct lvmcache_vginfo *vginfo;
+	struct lvmcache_info *info;
+
+	/* shouldn't happen */
+	if (!vgname || !vgid)
+		return false;
+
+	/* shouldn't happen */
+	if (!(vginfo = lvmcache_vginfo_from_vgid(vgid)))
+		return false;
+
+	/* shouldn't happen */
+	if (!(info = lvmcache_info_from_pvid(dev->pvid, NULL, 0)))
+		return false;
+
+	/* writing to a new PV */
+	if (!info->summary_seqno)
+		return false;
+
+	/* on same dev, one mda has newer metadata than the other */
+	if (info->summary_seqno_mismatch)
+		return true;
+
+	/* one or both mdas on this dev has older metadata than another dev */
+	if (vginfo->seqno > info->summary_seqno)
+		return true;
+
+	return false;
+}
+
+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_del_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) {
+		if (info->dev != dev)
+			continue;
+		*mdas = &info->mdas;
+		return;
+	}
+}
+
+bool 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 false;
+	}
+
+	dm_list_iterate_items(info, &vginfo->outdated_infos) {
+		if (info->dev == dev)
+			return true;
+	}
+
+	return false;
+}

lib/cache/lvmcache.h

@@ -57,10 +57,12 @@ 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 zero_offset;
-	int seqno;
+	int mda_num; /* 1 = summary from mda1, 2 = summary from mda2 */
+	unsigned mda_ignored:1;
+	unsigned zero_offset:1;
 };
 
 int lvmcache_init(struct cmd_context *cmd);
@@ -72,9 +74,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,
-				   const char *vgname, const char *vgid,
-				   uint32_t vgstatus);
+                                   struct device *dev, uint64_t label_sector,
+                                   const char *vgname, const char *vgid,
+                                   uint32_t vgstatus, int *is_duplicate);
 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);
@@ -82,18 +84,15 @@ 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(struct volume_group *vg, unsigned precommitted);
+int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted);
+int lvmcache_update_vg_from_write(struct volume_group *vg);
 
 void lvmcache_lock_vgname(const char *vgname, int read_only);
 void lvmcache_unlock_vgname(const char *vgname);
 
 /* Queries */
-const struct format_type *lvmcache_fmt_from_vgname(struct cmd_context *cmd, const char *vgname, const char *vgid, unsigned revalidate_labels);
 int lvmcache_lookup_mda(struct lvmcache_vgsummary *vgsummary);
 
-/* Decrement and test if there are still vg holders in vginfo. */
-int lvmcache_vginfo_holders_dec_and_test_for_zero(struct lvmcache_vginfo *vginfo);
-
 struct lvmcache_vginfo *lvmcache_vginfo_from_vgname(const char *vgname,
 					   const char *vgid);
 struct lvmcache_vginfo *lvmcache_vginfo_from_vgid(const char *vgid);
@@ -103,14 +102,11 @@ const char *lvmcache_vgid_from_vgname(struct cmd_context *cmd, const char *vgnam
 struct device *lvmcache_device_from_pvid(struct cmd_context *cmd, const struct id *pvid, uint64_t *label_sector);
 const char *lvmcache_vgname_from_info(struct lvmcache_info *info);
 const struct format_type *lvmcache_fmt_from_info(struct lvmcache_info *info);
-int lvmcache_vgs_locked(void);
 
-int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal,
-                          struct dm_list *vgnameids);
-
-/* Returns list of struct dm_str_list containing pool-allocated copy of pvids */
-struct dm_list *lvmcache_get_pvids(struct cmd_context *cmd, const char *vgname,
-				const char *vgid);
+int lvmcache_get_vgnameids(struct cmd_context *cmd,
+                           struct dm_list *vgnameids,
+                           const char *only_this_vgname,
+                           int include_internal);
 
 void lvmcache_drop_metadata(const char *vgname, int drop_precommitted);
 void lvmcache_commit_metadata(const char *vgname);
@@ -127,7 +123,8 @@ 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);
+		     uint64_t start, uint64_t size, unsigned ignored,
+		     struct metadata_area **mda_new);
 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);
 
@@ -163,9 +160,7 @@ int lvmcache_foreach_pv(struct lvmcache_vginfo *vginfo,
 uint64_t lvmcache_device_size(struct lvmcache_info *info);
 void lvmcache_set_device_size(struct lvmcache_info *info, uint64_t size);
 struct device *lvmcache_device(struct lvmcache_info *info);
-int lvmcache_is_orphan(struct lvmcache_info *info);
 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,
@@ -176,8 +171,6 @@ struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd,
 int lvmcache_found_duplicate_pvs(void);
 int lvmcache_found_duplicate_vgnames(void);
 
-void lvmcache_pvscan_duplicate_check(struct cmd_context *cmd);
-
 int lvmcache_get_unused_duplicate_devs(struct cmd_context *cmd, struct dm_list *head);
 
 int vg_has_duplicate_pvs(struct volume_group *vg);
@@ -189,35 +182,44 @@ void lvmcache_get_max_name_lengths(struct cmd_context *cmd,
 
 int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const char *vgid);
 
-void lvmcache_lock_ordering(int enable);
-
 int lvmcache_dev_is_unchosen_duplicate(struct device *dev);
 
 void lvmcache_remove_unchosen_duplicate(struct device *dev);
 
 int lvmcache_pvid_in_unchosen_duplicates(const char *pvid);
 
-int lvmcache_get_vg_devs(struct cmd_context *cmd,
-			 struct lvmcache_vginfo *vginfo,
-			 struct dm_list *devs);
-void lvmcache_set_independent_location(const char *vgname);
-
-int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid);
+bool lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid);
 
 int lvmcache_vginfo_has_pvid(struct lvmcache_vginfo *vginfo, char *pvid);
 
-/*
- * These are clvmd-specific functions and are not related to lvmcache.
- * FIXME: rename these with a clvm_ prefix in place of lvmcache_
- */
-void lvmcache_save_vg(struct volume_group *vg, int precommitted);
-struct volume_group *lvmcache_get_saved_vg(const char *vgid, int precommitted);
-struct volume_group *lvmcache_get_saved_vg_latest(const char *vgid);
-void lvmcache_drop_saved_vgid(const char *vgid);
-
 uint64_t lvmcache_max_metadata_size(void);
 void lvmcache_save_metadata_size(uint64_t val);
 
 int dev_in_device_list(struct device *dev, struct dm_list *head);
 
+bool lvmcache_has_bad_metadata(struct device *dev);
+
+bool 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);
+
+bool 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_mda_list);
+
 #endif

lib/commands/toolcontext.c

@@ -1715,8 +1715,6 @@ struct cmd_context *create_toolcontext(unsigned is_clvmd,
 	if (!init_lvmcache_orphans(cmd))
 		goto_out;
 
-	dm_list_init(&cmd->unused_duplicate_devs);
-
 	if (!_init_segtypes(cmd))
 		goto_out;
 

lib/commands/toolcontext.h

@@ -172,18 +172,21 @@ struct cmd_context {
 	unsigned pvscan_cache_single:1;
 	unsigned can_use_one_scan:1;
 	unsigned is_clvmd:1;
+	unsigned md_component_detection:1;
 	unsigned use_full_md_check:1;
 	unsigned is_activating:1;
 	unsigned enable_hints:1;		/* hints are enabled for cmds in general */
 	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.
 	 */
 	struct dev_filter *filter;
 	struct dm_list hints;
+	const char *md_component_checks;
 
 	/*
 	 * Configuration.
@@ -234,7 +237,6 @@ struct cmd_context {
 	const char *report_list_item_separator;
 	const char *time_format;
 	unsigned rand_seed;
-	struct dm_list unused_duplicate_devs; /* save preferences between lvmcache instances */
 };
 
 /*

lib/config/config_settings.h

@@ -368,7 +368,30 @@ cfg(devices_multipath_component_detection_CFG, "multipath_component_detection",
 	"Ignore devices that are components of DM multipath devices.\n")
 
 cfg(devices_md_component_detection_CFG, "md_component_detection", devices_CFG_SECTION, 0, CFG_TYPE_BOOL, DEFAULT_MD_COMPONENT_DETECTION, vsn(1, 0, 18), NULL, 0, NULL,
-	"Ignore devices that are components of software RAID (md) devices.\n")
+	"Enable detection and exclusion of MD component devices.\n"
+	"An MD component device is a block device that MD uses as part\n"
+	"of a software RAID virtual device. When an LVM PV is created\n"
+	"on an MD device, LVM must only use the top level MD device as\n"
+	"the PV, and should ignore the underlying component devices.\n"
+	"In cases where the MD superblock is located at the end of the\n"
+	"component devices, it is more difficult for LVM to consistently\n"
+	"identify an MD component, see the md_component_checks setting.\n")
+
+cfg(devices_md_component_checks_CFG, "md_component_checks", devices_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_STRING, DEFAULT_MD_COMPONENT_CHECKS, vsn(2, 3, 2), NULL, 0, NULL,
+	"The checks LVM should use to detect MD component devices.\n"
+	"MD component devices are block devices used by MD software RAID.\n"
+	"#\n"
+	"Accepted values:\n"
+	"  auto\n"
+	"    LVM will skip scanning the end of devices when it has other\n"
+	"    indications that the device is not an MD component.\n"
+	"  start\n"
+	"    LVM will only scan the start of devices for MD superblocks.\n"
+	"    This does not incur extra I/O by LVM.\n"
+	"  full\n"
+	"    LVM will scan the start and end of devices for MD superblocks.\n"
+	"    This requires an extra read at the end of devices.\n"
+	"#\n")
 
 cfg(devices_fw_raid_component_detection_CFG, "fw_raid_component_detection", devices_CFG_SECTION, 0, CFG_TYPE_BOOL, DEFAULT_FW_RAID_COMPONENT_DETECTION, vsn(2, 2, 112), NULL, 0, NULL,
 	"Ignore devices that are components of firmware RAID devices.\n"

lib/config/defaults.h

@@ -318,4 +318,6 @@
 
 #define DEFAULT_IO_MEMORY_SIZE_KB 8192
 
+#define DEFAULT_MD_COMPONENT_CHECKS "auto"
+
 #endif				/* _LVM_DEFAULTS_H */

lib/device/dev-md.c

@@ -110,14 +110,17 @@ static int _udev_dev_is_md_component(struct device *dev)
 	if (!(ext = dev_ext_get(dev)))
 		return_0;
 
-	if (!(value = udev_device_get_property_value((struct udev_device *)ext->handle, DEV_EXT_UDEV_BLKID_TYPE)))
+	if (!(value = udev_device_get_property_value((struct udev_device *)ext->handle, DEV_EXT_UDEV_BLKID_TYPE))) {
+		dev->flags |= DEV_UDEV_INFO_MISSING;
 		return 0;
+	}
 
 	return !strcmp(value, DEV_EXT_UDEV_BLKID_TYPE_SW_RAID);
 }
 #else
 static int _udev_dev_is_md_component(struct device *dev)
 {
+	dev->flags |= DEV_UDEV_INFO_MISSING;
 	return 0;
 }
 #endif

lib/device/dev-type.c

@@ -1170,8 +1170,10 @@ int udev_dev_is_md_component(struct device *dev)
 	const char *value;
 	int ret = 0;
 
-	if (!obtain_device_list_from_udev())
+	if (!obtain_device_list_from_udev()) {
+		dev->flags |= DEV_UDEV_INFO_MISSING;
 		return 0;
+	}
 
 	if (!(udev_device = _udev_get_dev(dev)))
 		return 0;
@@ -1198,6 +1200,7 @@ int udev_dev_is_mpath_component(struct device *dev)
 
 int udev_dev_is_md_component(struct device *dev)
 {
+	dev->flags |= DEV_UDEV_INFO_MISSING;
 	return 0;
 }
 

lib/device/device.h

@@ -37,6 +37,7 @@
 #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 */
+#define DEV_UDEV_INFO_MISSING   0x00040000	/* we have no udev info for this device */
 
 /*
  * Support for external device info.

lib/format_text/format-text.c

@@ -121,157 +121,6 @@ static struct device *_mda_get_device_raw(struct metadata_area *mda)
 	return mdac->area.dev;
 }
 
-/*
- * For circular region between region_start and region_start + region_size,
- * back up one SECTOR_SIZE from 'region_ptr' and return the value.
- * This allows reverse traversal through text metadata area to find old
- * metadata.
- *
- * Parameters:
- *   region_start: start of the region (bytes)
- *   region_size: size of the region (bytes)
- *   region_ptr: pointer within the region (bytes)
- *   NOTE: region_start <= region_ptr <= region_start + region_size
- */
-static uint64_t _get_prev_sector_circular(uint64_t region_start,
-					  uint64_t region_size,
-					  uint64_t region_ptr)
-{
-	if (region_ptr >= region_start + SECTOR_SIZE)
-		return region_ptr - SECTOR_SIZE;
-
-	return (region_start + region_size - SECTOR_SIZE);
-}
-
-/*
- * Analyze a metadata area for old metadata records in the circular buffer.
- * This function just looks through and makes a first pass at the data in
- * the sectors for particular things.
- * FIXME: do something with each metadata area (try to extract vg, write
- * raw data to file, etc)
- */
-static int _pv_analyze_mda_raw (const struct format_type * fmt,
-				struct metadata_area *mda)
-{
-	struct mda_header *mdah;
-	struct raw_locn *rlocn;
-	uint64_t area_start;
-	uint64_t area_size;
-	uint64_t prev_sector, prev_sector2;
-	uint64_t latest_mrec_offset;
-	uint64_t offset;
-	uint64_t offset2;
-	size_t size;
-	size_t size2;
-	char *buf=NULL;
-	struct device_area *area;
-	struct mda_context *mdac;
-	int r=0;
-
-	mdac = (struct mda_context *) mda->metadata_locn;
-
-	log_print("Found text metadata area: offset=" FMTu64 ", size="
-		  FMTu64, mdac->area.start, mdac->area.size);
-	area = &mdac->area;
-
-	if (!(mdah = raw_read_mda_header(fmt, area, mda_is_primary(mda))))
-		goto_out;
-
-	rlocn = mdah->raw_locns;
-
-	/*
-	 * The device area includes the metadata header as well as the
-	 * records, so remove the metadata header from the start and size
-	 */
-	area_start = area->start + MDA_HEADER_SIZE;
-	area_size = area->size - MDA_HEADER_SIZE;
-	latest_mrec_offset = rlocn->offset + area->start;
-
-	/*
-	 * Start searching at rlocn (point of live metadata) and go
-	 * backwards.
-	 */
-	prev_sector = _get_prev_sector_circular(area_start, area_size,
-					       latest_mrec_offset);
-	offset = prev_sector;
-	size = SECTOR_SIZE;
-	offset2 = size2 = 0;
-
-	while (prev_sector != latest_mrec_offset) {
-		prev_sector2 = prev_sector;
-		prev_sector = _get_prev_sector_circular(area_start, area_size,
-							prev_sector);
-		if (prev_sector > prev_sector2)
-			goto_out;
-		/*
-		 * FIXME: for some reason, the whole metadata region from
-		 * area->start to area->start+area->size is not used.
-		 * Only ~32KB seems to contain valid metadata records
-		 * (LVM2 format - format_text).  As a result, I end up with
-		 * "dm_config_maybe_section" returning true when there's no valid
-		 * metadata in a sector (sectors with all nulls).
-		 */
-		if (!(buf = malloc(size + size2)))
-			goto_out;
-
-		if (!dev_read_bytes(area->dev, offset, size, buf)) {
-			log_error("Failed to read dev %s offset %llu size %llu",
-				  dev_name(area->dev),
-				  (unsigned long long)offset,
-				  (unsigned long long)size);
-			goto out;
-		}
-
-		if (size2) {
-			if (!dev_read_bytes(area->dev, offset2, size2, buf + size)) {
-				log_error("Failed to read dev %s offset %llu size %llu",
-				  	  dev_name(area->dev),
-					  (unsigned long long)offset2,
-				          (unsigned long long)size2);
-				goto out;
-			}
-		}
-
-		/*
-		 * FIXME: We could add more sophisticated metadata detection
-		 */
-		if (dm_config_maybe_section(buf, size + size2)) {
-			/* FIXME: Validate region, pull out timestamp?, etc */
-			/* FIXME: Do something with this region */
-			log_verbose ("Found LVM2 metadata record at "
-				     "offset=" FMTu64 ", size=" FMTsize_t ", "
-				     "offset2=" FMTu64 " size2=" FMTsize_t,
-				     offset, size, offset2, size2);
-			offset = prev_sector;
-			size = SECTOR_SIZE;
-			offset2 = size2 = 0;
-		} else {
-			/*
-			 * Not a complete metadata record, assume we have
-			 * metadata and just increase the size and offset.
-			 * Start the second region if the previous sector is
-			 * wrapping around towards the end of the disk.
-			 */
-			if (prev_sector > offset) {
-				offset2 = prev_sector;
-				size2 += SECTOR_SIZE;
-			} else {
-				offset = prev_sector;
-				size += SECTOR_SIZE;
-			}
-		}
-		free(buf);
-		buf = NULL;
-	}
-
-	r = 1;
- out:
-	free(buf);
-	return r;
-}
-
-
-
 static int _text_lv_setup(struct format_instance *fid __attribute__((unused)),
 			  struct logical_volume *lv)
 {
@@ -312,7 +161,8 @@ 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)
+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)
 {
 	log_debug_metadata("Reading mda header sector from %s at %llu",
 			   dev_name(dev_area->dev), (unsigned long long)dev_area->start);
@@ -320,53 +170,62 @@ static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev
 	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_error("Incorrect checksum in metadata area header on %s at %llu",
+		log_warn("WARNING: wrong checksum %x in mda header on %s at %llu",
+			  mdah->checksum_xl,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_CHECKSUM;
 	}
 
 	_xlate_mdah(mdah);
 
 	if (memcmp(mdah->magic, FMTT_MAGIC, sizeof(mdah->magic))) {
-		log_error("Wrong magic number in metadata area header on %s at %llu",
+		log_warn("WARNING: wrong magic number in mda header on %s at %llu",
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_MAGIC;
 	}
 
 	if (mdah->version != FMTT_VERSION) {
-		log_error("Incompatible version %u metadata area header on %s at %llu",
+		log_warn("WARNING: wrong version %u in mda header on %s at %llu",
 			  mdah->version,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_VERSION;
 	}
 
 	if (mdah->start != dev_area->start) {
-		log_error("Incorrect start sector %llu in metadata area header on %s at %llu",
+		log_warn("WARNING: wrong start sector %llu in mda header on %s at %llu",
 			  (unsigned long long)mdah->start,
 			  dev_name(dev_area->dev), (unsigned long long)dev_area->start);
-		return 0;
+		*bad_fields |= BAD_MDA_START;
 	}
 
+	*bad_fields &= ~ignore_bad_fields;
+
+	if (*bad_fields)
+		return 0;
+
 	return 1;
 }
 
 struct mda_header *raw_read_mda_header(const struct format_type *fmt,
-				       struct device_area *dev_area, int primary_mda)
+				       struct device_area *dev_area,
+				       int primary_mda, uint32_t ignore_bad_fields, uint32_t *bad_fields)
 {
 	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)) {
+	if (!_raw_read_mda_header(mdah, dev_area, primary_mda, ignore_bad_fields, bad_fields)) {
 		dm_pool_free(fmt->cmd->mem, mdah);
 		return NULL;
 	}
@@ -564,8 +423,9 @@ 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))) {
+	if (!(mdah = raw_read_mda_header(fid->fmt, area, primary_mda, 0, &bad_fields))) {
 		log_error("Failed to read vg %s from %s", vgname, dev_name(area->dev));
 		goto out;
 	}
@@ -686,6 +546,7 @@ 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;
@@ -701,7 +562,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))))
+	if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda), mda->ignore_bad_fields, &bad_fields)))
 		goto_out;
 
 	/*
@@ -972,6 +833,7 @@ 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;
 
@@ -992,7 +854,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))))
+	if (!(mdab = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda), mda->ignore_bad_fields, &bad_fields)))
 		goto_out;
 
 	/*
@@ -1184,6 +1046,7 @@ 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))) {
@@ -1197,7 +1060,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)))
+	if (!_raw_read_mda_header(mdah, &mdac->area, mda_is_primary(mda), 0, &bad_fields))
 		log_warn("WARNING: Removing metadata location on %s with bad mda header.",
 			  dev_name(mdac->area.dev));
 
@@ -1494,7 +1357,7 @@ int read_metadata_location_summary(const struct format_type *fmt,
 	 * valid vg name.
 	 */
 	if (!validate_name(namebuf)) {
-		log_error("Metadata location on %s at %llu begins with invalid VG name.",
+		log_warn("WARNING: 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;
@@ -1560,7 +1423,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_error("Metadata location on %s at %llu has invalid summary for VG.",
+		log_warn("WARNING: metadata on %s at %llu has invalid summary for VG.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1568,7 +1431,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_error("Metadata location on %s at %llu has invalid VG name.",
+		log_warn("WARNING: metadata on %s at %llu has invalid VG name.",
 			  dev_name(dev_area->dev),
 			  (unsigned long long)(dev_area->start + rlocn->offset));
 		return 0;
@@ -1650,13 +1513,12 @@ 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->vg_name,
-				  is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0)))
+				  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)))
 		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);
 
@@ -1684,7 +1546,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)))
+				      mda_is_ignored(mda), NULL))
 			return_0;
 	}
 
@@ -1738,12 +1600,16 @@ 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;
@@ -1751,8 +1617,16 @@ 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)
+	if (ext_vsn < PV_HEADER_EXTENSION_VSN) {
+		log_debug("PV %s header needs rewrite for new ext version", dev_name(pv->dev));
 		*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;
 }
@@ -1962,7 +1836,6 @@ static struct metadata_area_ops _metadata_text_raw_ops = {
 	.mda_free_sectors = _mda_free_sectors_raw,
 	.mda_total_sectors = _mda_total_sectors_raw,
 	.mda_in_vg = _mda_in_vg_raw,
-	.pv_analyze_mda = _pv_analyze_mda_raw,
 	.mda_locns_match = _mda_locns_match_raw,
 	.mda_get_device = _mda_get_device_raw,
 };
@@ -2603,220 +2476,36 @@ bad:
 	return NULL;
 }
 
-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)
+int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev,
+			      struct metadata_area *mda)
 {
-	struct mda_header *mh;
+	struct mda_context *mdac = mda->metadata_locn;
+	uint64_t start_byte = mdac->area.start;
+	struct mda_header *mdab;
 	struct raw_locn *rlocn_slot0;
-	uint64_t text_offset, text_size;
-	char *area_buf;
-	char *text_buf;
+	struct raw_locn *rlocn_slot1;
+	uint32_t bad_fields = 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;
-	}
-
-	mh = (struct mda_header *)area_buf;
-	_xlate_mdah(mh);
-
-	rlocn_slot0 = &mh->raw_locns[0];
-	text_offset = rlocn_slot0->offset;
-	text_size = rlocn_slot0->size;
-
-	/*
-	 * 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");
+	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;
 	}
 
-	mdac = mda->metadata_locn;
+	rlocn_slot0 = &mdab->raw_locns[0];
+	rlocn_slot1 = &mdab->raw_locns[1];
 
-	/*
-	 * 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);
+	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;
 	}
 
-	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;
+	return 1;
 }
 
-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,7 +61,8 @@ 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 device *dev, uint64_t start, uint64_t size, unsigned ignored,
+	    struct metadata_area **mda_new);
 void del_mdas(struct dm_list *mdas);
 
 /* On disk */
@@ -76,18 +77,7 @@ struct data_area_list {
 	struct disk_locn disk_locn;
 };
 
-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);
+int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev,
+                              struct metadata_area *mda);
 
 #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)) {
-			/* FIXME: handle errors */
-			log_error("Couldn't read volume group metadata from %s.", dev_name(dev));
+			log_warn("WARNING: invalid metadata text from %s at %llu.",
+				 dev_name(dev), (unsigned long long)offset);
 			goto out;
 		}
 	} else {
 		if (!config_file_read(cft)) {
-			log_error("Couldn't read volume group metadata from file.");
+			log_warn("WARNING: invalid metadata text from file.");
 			goto out;
 		}
 	}

lib/format_text/layout.h

@@ -81,7 +81,9 @@ 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);
+				       struct device_area *dev_area, int primary_mda,
+				       uint32_t ignore_bad_fields,
+				       uint32_t *bad_fields);
 
 struct mda_lists {
 	struct metadata_area_ops *file_ops;

lib/format_text/text_label.c

@@ -241,11 +241,10 @@ 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 device *dev, uint64_t start, uint64_t size, unsigned ignored,
+	    struct metadata_area **mda_new)
 {
-/* 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;
@@ -295,6 +294,8 @@ 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;
 }
 
@@ -319,78 +320,103 @@ static int _text_initialise_label(struct labeller *l __attribute__((unused)),
 	return 1;
 }
 
-struct _update_mda_baton {
-	struct lvmcache_info *info;
-	struct label *label;
-};
-
-static int _read_mda_header_and_metadata(struct metadata_area *mda, void *baton)
+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 *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_is_primary(mda)))) {
-		log_error("Failed to read mda header from %s", dev_name(mdac->area.dev));
-		goto fail;
+	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 (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_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;
+		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;
 	}
 
 	return 1;
-
-fail:
-	lvmcache_del(p->info);
-	return 0;
 }
 
-static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
-		      struct label **label)
+/*
+ * 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)
 {
+	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 lvmcache_info *info;
+	struct metadata_area *mda;
+	struct metadata_area *mda1 = NULL;
+	struct metadata_area *mda2 = NULL;
 	struct disk_locn *dlocn_xl;
 	uint64_t offset;
 	uint32_t ext_version;
-	struct _update_mda_baton baton;
+	uint32_t bad_fields;
+	int mda_count = 0;
+	int good_mda_count = 0;
+	int bad_mda_count = 0;
+	int rv1, rv2;
 
 	/*
 	 * PV header base
 	 */
 	pvhdr = (struct pv_header *) ((char *) label_buf + xlate32(lh->offset_xl));
 
-	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev,
+	/*
+	 * 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,
 				  FMT_TEXT_ORPHAN_VG_NAME,
-				  FMT_TEXT_ORPHAN_VG_NAME, 0)))
+				  FMT_TEXT_ORPHAN_VG_NAME, 0, is_duplicate)))
 		return_0;
 
-	*label = lvmcache_get_label(info);
-
 	lvmcache_set_device_size(info, xlate64(pvhdr->device_size_xl));
 
 	lvmcache_del_das(info);
@@ -404,11 +430,27 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
 		dlocn_xl++;
 	}
 
-	/* Metadata area headers */
 	dlocn_xl++;
+
+	/* Metadata areas */
 	while ((offset = xlate64(dlocn_xl->offset))) {
-		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0);
+
+		/*
+		 * This just calls add_mda() above, replacing info with info->mdas.
+		 */
+		lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0, &mda);
+
 		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++;
@@ -418,7 +460,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
 	 */
 	pvhdr_ext = (struct pv_header_extension *) ((char *) dlocn_xl);
 	if (!(ext_version = xlate32(pvhdr_ext->version)))
-		goto out;
+		goto scan_mdas;
 
 	log_debug_metadata("%s: PV header extension version " FMTu32 " found",
 			   dev_name(dev), ext_version);
@@ -435,22 +477,117 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf,
 		lvmcache_add_ba(info, offset, xlate64(dlocn_xl->size));
 		dlocn_xl++;
 	}
-out:
-	baton.info = info;
-	baton.label = *label;
 
-	/*
-	 * In the vg_read phase, we compare all mdas and decide which to use
-	 * which are bad and need 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 (!lvmcache_foreach_mda(info, _read_mda_header_and_metadata, &baton)) {
-		log_error("Failed to scan VG from %s", dev_name(dev));
-		return 0;
+ scan_mdas:
+	if (!mda_count) {
+		log_debug_metadata("Scanning %s found no mdas.", dev_name(dev));
+		return 1;
 	}
 
+	/*
+	 * 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).
+	 *
+	 * 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.
+	 */
+
+	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)
+		return 0;
+
+	/* no metadata in the mdas */
 	return 1;
 }
 

lib/label/label.c

@@ -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,17 +423,38 @@ 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 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.
+	 * 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.
 	 */
+	ret = labeller->ops->read(labeller, dev, label_buf, sector, &is_duplicate);
 
-	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);
+	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);
+		}
 	}
  out:
 	return ret;
@@ -696,7 +717,6 @@ 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);
 			}
 		}
 
@@ -1022,6 +1042,33 @@ int label_scan(struct cmd_context *cmd)
 		}
 	}
 
+	/*
+	 * Stronger exclusion of md components that might have been
+	 * misidentified as PVs due to having an end-of-device md superblock.
+	 * If we're not using hints, and are not already doing a full md check
+	 * on devs being scanned, then if udev info is missing for a PV, scan
+	 * the end of the PV to verify it's not an md component.  The full
+	 * dev_is_md_component call will do new reads at the end of the dev.
+	 */
+	if (cmd->md_component_detection && !cmd->use_full_md_check && !using_hints &&
+	    !strcmp(cmd->md_component_checks, "auto")) {
+		int once = 0;
+		dm_list_iterate_items(devl, &scan_devs) {
+			if (!(devl->dev->flags & DEV_SCAN_FOUND_LABEL))
+				continue;
+			if (!(devl->dev->flags & DEV_UDEV_INFO_MISSING))
+				continue;
+			if (!once++)
+				log_debug_devs("Scanning end of PVs with no udev info for MD components");
+
+			if (dev_is_md_component(devl->dev, NULL, 1)) {
+				log_debug_devs("Drop PV from MD component %s", dev_name(devl->dev));
+				devl->dev->flags &= ~DEV_SCAN_FOUND_LABEL;
+				lvmcache_del_dev(devl->dev);
+			}
+		}
+	}
+
 	dm_list_iterate_items_safe(devl, devl2, &all_devs) {
 		dm_list_del(&devl->list);
 		free(devl);
@@ -1198,66 +1245,6 @@ int label_read(struct device *dev)
 	return 1;
 }
 
-/*
- * Read a label from a specfic, non-zero sector.  This is used in only
- * one place: pvck/pv_analyze.
- */
-
-int label_read_sector(struct device *dev, uint64_t read_sector)
-{
-	struct block *bb = NULL;
-	uint64_t block_num;
-	uint64_t block_sector;
-	uint64_t start_sector;
-	int is_lvm_device = 0;
-	int result;
-	int ret;
-
-	block_num = read_sector / BCACHE_BLOCK_SIZE_IN_SECTORS;
-	block_sector = block_num * BCACHE_BLOCK_SIZE_IN_SECTORS;
-	start_sector = read_sector % BCACHE_BLOCK_SIZE_IN_SECTORS;
-
-	if (!label_scan_open(dev)) {
-		log_error("Error opening device %s for prefetch %llu sector.",
-			  dev_name(dev), (unsigned long long)block_num);
-		return false;
-	}
-
-	bcache_prefetch(scan_bcache, dev->bcache_fd, block_num);
-
-	if (!bcache_get(scan_bcache, dev->bcache_fd, block_num, 0, &bb)) {
-		log_error("Scan failed to read %s at %llu",
-			  dev_name(dev), (unsigned long long)block_num);
-		ret = 0;
-		goto out;
-	}
-
-	/*
-	 * TODO: check if scan_sector is larger than the bcache block size.
-	 * If it is, we need to fetch a later block from bcache.
-	 */
-
-	result = _process_block(NULL, NULL, dev, bb, block_sector, start_sector, &is_lvm_device);
-
-	if (!result && is_lvm_device) {
-		log_error("Scan failed to process %s", dev_name(dev));
-		ret = 0;
-		goto out;
-	}
-
-	if (!result || !is_lvm_device) {
-		log_error("Could not find LVM label on %s", dev_name(dev));
-		ret = 0;
-		goto out;
-	}
-
-	ret = 1;
-out:
-	if (bb)
-		bcache_put(bb);
-	return ret;
-}
-
 int label_scan_setup_bcache(void)
 {
 	if (!scan_bcache) {

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, struct label ** label);
+		     void *label_buf, uint64_t label_sector, int *is_duplicate);
 
 	/*
 	 * Populate label_type etc.

lib/locking/locking.c

@@ -156,12 +156,20 @@ int init_locking(struct cmd_context *cmd,
 	return 1;
 }
 
-void fin_locking(void)
+void fin_locking(struct cmd_context *cmd)
 {
 	/* file locking disabled */
 	if (!_locking.flags)
 		return;
 
+	/*
+	 * These may be automatically released when the
+	 * command ends, without an explicit unlock call,
+	 * in which case these flags would not be cleared.
+	 */
+	cmd->lockf_global_ex = 0;
+	cmd->lockd_global_ex = 0;
+
 	_locking.fin_locking();
 }
 
@@ -376,6 +384,10 @@ int lockf_global(struct cmd_context *cmd, const char *mode)
 
 int lockf_global_convert(struct cmd_context *cmd, const char *mode)
 {
+	/* some uncommon cases like pvchange -a can call this multiple times */
+	if (cmd->lockf_global_ex && !strcmp(mode, "ex"))
+		return 1;
+
 	return _lockf_global(cmd, mode, 1);
 }
 

lib/locking/locking.h

@@ -22,7 +22,7 @@
 struct logical_volume;
 
 int init_locking(struct cmd_context *cmd, int file_locking_sysinit, int file_locking_readonly, int file_locking_ignorefail);
-void fin_locking(void);
+void fin_locking(struct cmd_context *cmd);
 void reset_locking(void);
 int vg_write_lock_held(void);
 

lib/metadata/lv.c

@@ -245,6 +245,11 @@ char *lvseg_kernel_discards_dup_with_info_and_seg_status(struct dm_pool *mem, co
 			return 0;
 		}
 		s = get_pool_discards_name(d);
+	} else if (lvdm->seg_status.type == SEG_STATUS_CACHE) {
+		if (lvdm->seg_status.cache->feature_flags &
+		    DM_CACHE_FEATURE_NO_DISCARD_PASSDOWN) {
+			s = "nopassdown";
+		}
 	}
 
 	if (!(ret = dm_pool_strdup(mem, s))) {

lib/metadata/metadata-exported.h

@@ -181,15 +181,14 @@
 #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 */
 
-/* vg's "read_status" field */
-#define FAILED_INCONSISTENT	0x00000001U
+/* vg_read returns these in error_flags */
+#define FAILED_NOT_ENABLED	0x00000001U
 #define FAILED_LOCKING		0x00000002U
 #define FAILED_NOTFOUND		0x00000004U
 #define FAILED_READ_ONLY	0x00000008U
@@ -202,6 +201,7 @@
 #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
@@ -696,9 +696,6 @@ int unlink_lv_from_vg(struct logical_volume *lv);
 void lv_set_visible(struct logical_volume *lv);
 void lv_set_hidden(struct logical_volume *lv);
 
-int get_vgnameids(struct cmd_context *cmd, struct dm_list *vgnameids,
-		  const char *only_this_vgname, int include_internal);
-
 int pv_write(struct cmd_context *cmd, struct physical_volume *pv, int allow_non_orphan);
 int move_pv(struct volume_group *vg_from, struct volume_group *vg_to,
 	    const char *pv_name);
@@ -717,24 +714,14 @@ int lv_resize(struct logical_volume *lv,
 	      struct lvresize_params *lp,
 	      struct dm_list *pvh);
 
-/*
- * 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(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);
 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,
-                                             uint32_t warn_flags,
-                                             const char *orphan_vgname);
-/* 
- * Test validity of a VG handle.
- */
+struct volume_group *vg_read_orphans(struct cmd_context *cmd, const char *orphan_vgname);
+
+/* this is historical and being removed, don't use */
 uint32_t vg_read_error(struct volume_group *vg_handle);
 
 /* pe_start and pe_end relate to any existing data so that new metadata
@@ -757,7 +744,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);
+struct volume_group *vg_lock_and_create(struct cmd_context *cmd, const char *vg_name, int *exists);
 int vg_remove_mdas(struct volume_group *vg);
 int vg_remove_check(struct volume_group *vg);
 void vg_remove_pvs(struct volume_group *vg);
@@ -1381,4 +1368,6 @@ 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,11 +168,27 @@ 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);
@@ -234,7 +250,7 @@ struct name_list {
 
 struct mda_list {
 	struct dm_list list;
-	struct device_area mda;
+	struct metadata_area *mda;
 };
 
 struct peg_list {

lib/metadata/pv.h

@@ -59,6 +59,7 @@ 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/vg.c

@@ -84,7 +84,7 @@ static void _free_vg(struct volume_group *vg)
 
 void release_vg(struct volume_group *vg)
 {
-	if (!vg || (vg->fid && vg == vg->fid->fmt->orphan_vg))
+	if (!vg || is_orphan_vg(vg->name))
 		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, 0, vg->fid->fmt->orphan_vg_name);
+	orphan_vg = vg_read_orphans(cmd, vg->fid->fmt->orphan_vg_name);
 
-	if (vg_read_error(orphan_vg))
+	if (!orphan_vg)
 		goto bad;
 
 	if (!vg_split_mdas(cmd, vg, orphan_vg) || !vg->pv_count) {

lib/metadata/vg.h

@@ -122,11 +122,6 @@ 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-globals.c

@@ -31,7 +31,6 @@ static int _fwraid_filtering = 0;
 static int _pvmove = 0;
 static int _obtain_device_list_from_udev = DEFAULT_OBTAIN_DEVICE_LIST_FROM_UDEV;
 static enum dev_ext_e _external_device_info_source = DEV_EXT_NONE;
-static int _trust_cache = 0; /* Don't scan when incomplete VGs encountered */
 static int _debug_level = 0;
 static int _debug_classes_logged = 0;
 static int _security_level = SECURITY_LEVEL;
@@ -107,11 +106,6 @@ void init_external_device_info_source(enum dev_ext_e src)
 	_external_device_info_source = src;
 }
 
-void init_trust_cache(int trustcache)
-{
-	_trust_cache = trustcache;
-}
-
 void init_security_level(int level)
 {
 	_security_level = level;
@@ -285,11 +279,6 @@ enum dev_ext_e external_device_info_source(void)
 	return _external_device_info_source;
 }
 
-int trust_cache(void)
-{
-	return _trust_cache;
-}
-
 int background_polling(void)
 {
 	return _background_polling;

lib/misc/lvm-globals.h

@@ -32,7 +32,6 @@ void init_fwraid_filtering(int level);
 void init_pvmove(int level);
 void init_external_device_info_source(enum dev_ext_e src);
 void init_obtain_device_list_from_udev(int device_list_from_udev);
-void init_trust_cache(int trustcache);
 void init_debug(int level);
 void init_debug_classes_logged(int classes);
 void init_cmd_name(int status);
@@ -65,7 +64,6 @@ int fwraid_filtering(void);
 int pvmove_mode(void);
 int obtain_device_list_from_udev(void);
 enum dev_ext_e external_device_info_source(void);
-int trust_cache(void);
 int verbose_level(void);
 int silent_mode(void);
 int debug_level(void);

libdm/libdevmapper.h

@@ -1893,6 +1893,7 @@ int dm_tree_node_add_raid_target_with_params_v2(struct dm_tree_node *node,
 #define DM_CACHE_FEATURE_WRITETHROUGH 0x00000002
 #define DM_CACHE_FEATURE_PASSTHROUGH  0x00000004
 #define DM_CACHE_FEATURE_METADATA2    0x00000008 /* cache v1.10 */
+#define DM_CACHE_FEATURE_NO_DISCARD_PASSDOWN 0x00000010
 
 struct dm_config_node;
 /*

libdm/libdm-targets.c

@@ -296,6 +296,8 @@ int dm_get_status_cache(struct dm_pool *mem, const char *params,
 			s->feature_flags |= DM_CACHE_FEATURE_PASSTHROUGH;
 		else if (!strncmp(p, "metadata2 ", 10))
 			s->feature_flags |= DM_CACHE_FEATURE_METADATA2;
+		else if (!strncmp(p, "no_discard_passdown ", 20))
+			s->feature_flags |= DM_CACHE_FEATURE_NO_DISCARD_PASSDOWN;
 		else
 			log_error("Unknown feature in status: %s", params);
 

man/lvconvert.8_pregen

@@ -42,6 +42,10 @@ lvconvert - Change logical volume layout
     \fB--cachesettings\fP \fIString\fP
 .ad b
 .br
+.ad l
+    \fB--cachevol\fP \fILV\fP
+.ad b
+.br
 .ad l
  \fB-c\fP|\fB--chunksize\fP \fISize\fP[k|UNIT]
 .ad b
@@ -567,7 +571,7 @@ Convert LV to a thin LV, using the original LV as an external origin.
 .br
 -
 
-Attach a cache to an LV, converts the LV to type cache.
+Attach a cache pool to an LV, converts the LV to type cache.
 .br
 .P
 \fBlvconvert\fP \fB--type\fP \fBcache\fP \fB--cachepool\fP \fILV\fP \fILV\fP\fI_linear_striped_thinpool_vdo_vdopool_vdopooldata_raid\fP
@@ -629,7 +633,7 @@ Attach a cache to an LV, converts the LV to type cache.
 Attach a writecache to an LV, converts the LV to type writecache.
 .br
 .P
-\fBlvconvert\fP \fB--type\fP \fBwritecache\fP \fB--cachepool\fP \fILV\fP \fILV\fP\fI_linear_striped_raid\fP
+\fBlvconvert\fP \fB--type\fP \fBwritecache\fP \fB--cachevol\fP \fILV\fP \fILV\fP\fI_linear_striped_raid\fP
 .br
 .RS 4
 .ad l
@@ -641,6 +645,49 @@ Attach a writecache to an LV, converts the LV to type writecache.
 .br
 -
 
+Attach a cache to an LV, converts the LV to type cache.
+.br
+.P
+\fBlvconvert\fP \fB--type\fP \fBcache\fP \fB--cachevol\fP \fILV\fP \fILV\fP\fI_linear_striped_thinpool_raid\fP
+.br
+.RS 4
+.ad l
+[ \fB-H\fP|\fB--cache\fP ]
+.ad b
+.br
+.ad l
+[ \fB-Z\fP|\fB--zero\fP \fBy\fP|\fBn\fP ]
+.ad b
+.br
+.ad l
+[ \fB-c\fP|\fB--chunksize\fP \fISize\fP[k|UNIT] ]
+.ad b
+.br
+.ad l
+[    \fB--cachemetadataformat\fP \fBauto\fP|\fB1\fP|\fB2\fP ]
+.ad b
+.br
+.ad l
+[    \fB--cachemode\fP \fBwritethrough\fP|\fBwriteback\fP|\fBpassthrough\fP ]
+.ad b
+.br
+.ad l
+[    \fB--cachepolicy\fP \fIString\fP ]
+.ad b
+.br
+.ad l
+[    \fB--cachesettings\fP \fIString\fP ]
+.ad b
+.br
+.ad l
+[    \fB--poolmetadatasize\fP \fISize\fP[m|UNIT] ]
+.ad b
+.br
+[ COMMON_OPTIONS ]
+.RE
+.br
+-
+
 Convert LV to type thin-pool.
 .br
 .P
@@ -1047,7 +1094,7 @@ See \fBlvmcache\fP(7) for more information.
 .ad l
 \fB--cachepool\fP \fILV\fP
 .br
-The name of a cache pool LV.
+The name of a cache pool.
 .ad b
 .HP
 .ad l
@@ -1063,6 +1110,12 @@ See \fBlvmcache\fP(7) for more information.
 .ad b
 .HP
 .ad l
+\fB--cachevol\fP \fILV\fP
+.br
+The name of a cache volume.
+.ad b
+.HP
+.ad l
 \fB-c\fP|\fB--chunksize\fP \fISize\fP[k|UNIT]
 .br
 The size of chunks in a snapshot, cache pool or thin pool.
@@ -1689,7 +1742,7 @@ Convert LV to a thin LV, using the original LV as an external origin
 .br
 -
 
-Attach a cache to an LV (infers --type cache).
+Attach a cache pool to an LV (infers --type cache).
 .br
 .P
 \fBlvconvert\fP \fB-H\fP|\fB--cache\fP \fB--cachepool\fP \fILV\fP \fILV\fP\fI_linear_striped_thinpool_vdo_vdopool_vdopooldata_raid\fP

man/lvcreate.8_pregen

@@ -309,7 +309,8 @@ numeric suffix.
 In the usage section below, when creating a pool and the name is omitted
 the new LV pool name is generated with the
 "vpool" for vdo-pools  for prefix and a unique numeric suffix.
-Also pool name can be specified together with \fIVG\fP name i.e.:
+
+Pool name can be specified together with \fIVG\fP name i.e.:
 vg00/mythinpool.
 .SH USAGE
 Create a linear LV.
@@ -1097,7 +1098,7 @@ See \fBlvmcache\fP(7) for more information.
 .ad l
 \fB--cachepool\fP \fILV\fP
 .br
-The name of a cache pool LV.
+The name of a cache pool.
 .ad b
 .HP
 .ad l

man/lvm-fullreport.8_pregen

@@ -92,10 +92,6 @@ if information changes between commands.
 .ad b
 .br
 .ad l
-[    \fB--trustcache\fP ]
-.ad b
-.br
-.ad l
 [    \fB--unbuffered\fP ]
 .ad b
 .br
@@ -406,12 +402,6 @@ back metadata it believes has changed but hasn't.
 .ad b
 .HP
 .ad l
-\fB--trustcache\fP
-.br
-Avoids certain device scanning during command processing. Do not use.
-.ad b
-.HP
-.ad l
 \fB--unbuffered\fP
 .br
 Produce output immediately without sorting or aligning the columns properly.

man/lvs.8_pregen

@@ -96,10 +96,6 @@ lvs produces formatted output about LVs.
 .ad b
 .br
 .ad l
-[    \fB--trustcache\fP ]
-.ad b
-.br
-.ad l
 [    \fB--unbuffered\fP ]
 .ad b
 .br
@@ -427,12 +423,6 @@ back metadata it believes has changed but hasn't.
 .ad b
 .HP
 .ad l
-\fB--trustcache\fP
-.br
-Avoids certain device scanning during command processing. Do not use.
-.ad b
-.HP
-.ad l
 \fB--unbuffered\fP
 .br
 Produce output immediately without sorting or aligning the columns properly.

man/pvchange.8_pregen

@@ -16,7 +16,7 @@ others are optional.
 Change properties of all PVs.
 .br
 .P
-\fBpvchange\fP
+\fBpvchange\fP \fB-a\fP|\fB--all\fP
 .RS 4
 ( \fB-x\fP|\fB--allocatable\fP \fBy\fP|\fBn\fP,
 .ad b
@@ -25,10 +25,6 @@ Change properties of all PVs.
   \fB-u\fP|\fB--uuid\fP,
 .ad b
 .br
-.ad l
-  \fB-a\fP|\fB--all\fP,
-.ad b
-.br
 .ad l
      \fB--addtag\fP \fITag\fP,
 .ad b

man/pvck.8_pregen

@@ -1,26 +1,58 @@
 .TH PVCK 8 "LVM TOOLS #VERSION#" "Red Hat, Inc."
 .SH NAME
-pvck - Check the consistency of physical volume(s)
+pvck - Check metadata on physical volumes
 .
 .SH SYNOPSIS
-\fBpvck\fP \fIposition_args\fP
+\fBpvck\fP \fIoption_args\fP \fIposition_args\fP
 .br
     [ \fIoption_args\fP ]
 .br
 .SH DESCRIPTION
-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.)
+
 .SH USAGE
+Check for metadata on a device
+.br
+.P
 \fBpvck\fP \fIPV\fP ...
 .br
 .RS 4
+[ COMMON_OPTIONS ]
+.RE
+.br
+
+Print metadata from a device
+.br
+.P
+\fBpvck\fP \fB--dump\fP \fIString\fP \fIPV\fP
+.br
+.RS 4
 .ad l
-[    \fB--labelsector\fP \fINumber\fP ]
+[ \fB-f\fP|\fB--file\fP \fIString\fP ]
+.ad b
+.br
+.ad l
+[    \fB--[pv]metadatacopies\fP \fB0\fP|\fB1\fP|\fB2\fP ]
 .ad b
 .br
 [ COMMON_OPTIONS ]
 .RE
 .br
 
+Common options for command:
+.
+.RS 4
+.ad l
+[    \fB--labelsector\fP \fINumber\fP ]
+.ad b
+.RE
+
 Common options for lvm:
 .
 .RS 4
@@ -113,6 +145,21 @@ For testing and debugging.
 .ad b
 .HP
 .ad l
+\fB--dump\fP \fIString\fP
+.br
+Dump metadata from a PV. Option values include \fBmetadata\fP
+to print or save the current text metadata, \fBmetadata_area\fP
+to save the entire text metadata area to a file, \fBmetadata_all\fP
+to save the current and any previous complete versions of metadata
+to a file, and \fBheaders\fP to print and check LVM headers.
+.ad b
+.HP
+.ad l
+\fB-f\fP|\fB--file\fP \fIString\fP
+.br
+.ad b
+.HP
+.ad l
 \fB-h\fP|\fB--help\fP
 .br
 Display help text.
@@ -154,6 +201,18 @@ on the command.
 .ad b
 .HP
 .ad l
+\fB--[pv]metadatacopies\fP \fB0\fP|\fB1\fP|\fB2\fP
+.br
+The number of metadata areas to set aside on a PV for storing VG metadata.
+When 2, one copy of the VG metadata is stored at the front of the PV
+and a second copy is stored at the end.
+When 1, one copy of the VG metadata is stored at the front of the PV.
+When 0, no copies of the VG metadata are stored on the given PV.
+This may be useful in VGs containing many PVs (this places limitations
+on the ability to use vgsplit later.)
+.ad b
+.HP
+.ad l
 \fB-q\fP|\fB--quiet\fP ...
 .br
 Suppress output and log messages. Overrides --debug and --verbose.

man/pvs.8_pregen

@@ -92,10 +92,6 @@ pvs produces formatted output about PVs.
 .ad b
 .br
 .ad l
-[    \fB--trustcache\fP ]
-.ad b
-.br
-.ad l
 [    \fB--unbuffered\fP ]
 .ad b
 .br
@@ -416,12 +412,6 @@ back metadata it believes has changed but hasn't.
 .ad b
 .HP
 .ad l
-\fB--trustcache\fP
-.br
-Avoids certain device scanning during command processing. Do not use.
-.ad b
-.HP
-.ad l
 \fB--unbuffered\fP
 .br
 Produce output immediately without sorting or aligning the columns properly.

man/vgck.8_pregen

@@ -3,7 +3,7 @@
 vgck - Check the consistency of volume group(s)
 .
 .SH SYNOPSIS
-\fBvgck\fP
+\fBvgck\fP \fIoption_args\fP \fIposition_args\fP
 .br
     [ \fIoption_args\fP ]
 .br
@@ -12,6 +12,9 @@ vgck - Check the consistency of volume group(s)
 .SH DESCRIPTION
 vgck checks LVM metadata for consistency.
 .SH USAGE
+Read and display information about a VG.
+.br
+.P
 \fBvgck\fP
 .br
 .RS 4
@@ -26,6 +29,21 @@ vgck checks LVM metadata for consistency.
 [ \fIVG\fP|\fITag\fP ... ]
 .RE
 
+Rewrite VG metadata to correct problems.
+.br
+.P
+\fBvgck\fP \fB--updatemetadata\fP \fIVG\fP
+.br
+.RS 4
+[ COMMON_OPTIONS ]
+.RE
+.br
+
+Common options for command:
+.
+.RS 4
+.RE
+
 Common options for lvm:
 .
 .RS 4
@@ -178,6 +196,12 @@ back metadata it believes has changed but hasn't.
 .ad b
 .HP
 .ad l
+\fB--updatemetadata\fP
+.br
+Update VG metadata to correct problems.
+.ad b
+.HP
+.ad l
 \fB-v\fP|\fB--verbose\fP ...
 .br
 Set verbose level. Repeat from 1 to 4 times to increase the detail

man/vgs.8_pregen

@@ -88,10 +88,6 @@ vgs produces formatted output about VGs.
 .ad b
 .br
 .ad l
-[    \fB--trustcache\fP ]
-.ad b
-.br
-.ad l
 [    \fB--unbuffered\fP ]
 .ad b
 .br
@@ -403,12 +399,6 @@ back metadata it believes has changed but hasn't.
 .ad b
 .HP
 .ad l
-\fB--trustcache\fP
-.br
-Avoids certain device scanning during command processing. Do not use.
-.ad b
-.HP
-.ad l
 \fB--unbuffered\fP
 .br
 Produce output immediately without sorting or aligning the columns properly.

test/lib/aux.sh

@@ -1090,7 +1090,7 @@ extend_filter() {
 	for rx in "$@"; do
 		filter=$(echo "$filter" | sed -e "s:\\[:[ \"$rx\", :")
 	done
-	lvmconf "$filter"
+	lvmconf "$filter" "devices/scan_lvs = 1"
 }
 
 extend_filter_LVMTEST() {
@@ -1421,7 +1421,7 @@ target_at_least() {
 	fi
 
 	local version
-	version=$(dmsetup targets 2>/dev/null | grep "${1##dm-} " 2>/dev/null)
+	version=$(dmsetup targets 2>/dev/null | grep "^${1##dm-} " 2>/dev/null)
 	version=${version##* v}
 
 	version_at_least "$version" "${@:2}" || {
@@ -1478,6 +1478,14 @@ have_vdo() {
 	target_at_least dm-vdo "$@"
 }
 
+have_writecache() {
+	lvm segtypes 2>/dev/null | grep -q writecache$ || {
+		echo "writecache is not built-in." >&2
+		return 1
+	}
+	target_at_least dm-writecache "$@"
+}
+
 have_raid() {
 	target_at_least dm-raid "$@"
 

test/shell/cache-no-discard.sh

@@ -0,0 +1,45 @@
+#!/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 reporting of no_discard_passdown
+
+SKIP_WITH_LVMPOLLD=1
+
+# Until new version of cache_check tools - no integrity validation
+LVM_TEST_CACHE_CHECK_CMD=""
+
+. lib/inittest
+
+aux kernel_at_least 5 1 || skip
+
+aux have_cache 2 0
+
+aux prepare_vg 1
+
+# Create thinLV without discard
+lvcreate --discards=ignore -T -V20 -L20 -n $lv $vg/pool
+
+aux extend_filter_LVMTEST
+
+# Use discard-less LV as PV for $vg1
+pvcreate "$DM_DEV_DIR/$vg/$lv"
+vgcreate -s 128K $vg1 "$DM_DEV_DIR/$vg/$lv"
+
+# Create simple cache LV
+lvcreate -L2 -n $lv1 $vg1
+lvcreate -H -L2 $vg1/$lv1
+
+#lvs -ao+kernel_discards $vg1
+check lv_field $vg1/$lv1 kernel_discards "nopassdown"
+
+vgremove -f $vg1
+vgremove -f $vg

test/shell/inconsistent-metadata.sh

@@ -24,53 +24,48 @@ lvchange -a n $vg/mirror
 
 aux backup_dev "${DEVICES[@]}"
 
-init() {
+makeold() {
+	# reset metadata on all devs to starting condition
 	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"
 }
 
-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"
+# create old metadata
+makeold
 
-# 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"
-
-# vgs fixes up as well
-init
+# reports old metadata
 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
+grep "ignoring metadata" 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
+# corrects old metadata
+lvcreate -l1 -an $vg
+
+# no old report
+vgs $vg 2>&1 | tee cmd.out
+not grep "ignoring metadata" 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
+
 vgremove -f $vg
 pvremove -ff "${DEVICES[@]}"
 pvcreate "${DEVICES[@]}"
+
 aux backup_dev "$dev2"
 vgcreate $SHARED $vg "$dev1"
 vgextend $vg "$dev2"
 aux restore_dev "$dev2"
-vgscan
+
+vgs -o+vg_mda_count $vg
+pvs -o+vg_mda_count
+
 should check compare_fields vgs $vg vg_mda_count pvs "$dev2" vg_mda_count
 
 vgremove -ff $vg

test/shell/lvconvert-repair-cache.sh

@@ -93,6 +93,9 @@ 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,6 +24,8 @@ 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,6 +65,7 @@ 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
 
@@ -80,6 +81,7 @@ 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
 
@@ -96,6 +98,7 @@ 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
@@ -114,6 +117,7 @@ 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
 
@@ -128,6 +132,7 @@ 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
 
@@ -145,6 +150,7 @@ 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
 
@@ -163,6 +169,7 @@ 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,17 +106,23 @@ 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

@@ -31,6 +31,10 @@ test -f /proc/mdstat && grep -q raid1 /proc/mdstat || \
 
 aux lvmconf 'devices/md_component_detection = 1'
 
+# This stops lvm from taking advantage of hints which
+# will have already excluded md components.
+aux lvmconf 'devices/hints = "none"'
+
 # 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.
@@ -61,6 +65,10 @@ check lv_field $vg/$lv1 lv_active "active"
 pvs "$mddev"
 not pvs "$dev1"
 not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
 sleep 1
 
 vgchange -an $vg
@@ -72,12 +80,14 @@ sleep 1
 mdadm --stop "$mddev"
 aux udev_wait
 
-# with md superblock 1.0 this pvs will report duplicates
-# for the two md legs since the md device itself is not
-# started
-pvs 2>&1 |tee out
-cat out
-grep "prefers device" out
+# The md components should still be detected and excluded.
+not pvs "$dev1"
+not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
+pvs -vvvv
 
 # should not activate from the md legs
 not vgchange -ay $vg
@@ -104,16 +114,20 @@ not grep "active" out
 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
+# Now that the md dev is online, pvs can see it
+# and check for components even if
+# md_component_checks is "start" (which disables
+# most default end-of-device scans)
+aux lvmconf 'devices/md_component_checks = "start"'
 
-pvs 2>&1 |tee out
-cat out
-not grep "prefers device" out
+not pvs "$dev1"
+not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
 
-vgchange -ay $vg 2>&1 |tee out
-cat out
-not grep "prefers device" out
+
+vgchange -ay $vg
 
 check lv_field $vg/$lv1 lv_active "active"
 
@@ -124,6 +138,9 @@ vgremove -f $vg
 
 aux cleanup_md_dev
 
+# Put this setting back to the default
+aux lvmconf 'devices/md_component_checks = "auto"'
+
 # 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
@@ -149,6 +166,10 @@ check lv_field $vg/$lv1 lv_active "active"
 pvs "$mddev"
 not pvs "$dev1"
 not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
 sleep 1
 
 vgchange -an $vg
@@ -160,7 +181,14 @@ sleep 1
 mdadm --stop "$mddev"
 aux udev_wait
 
-pvs 2>&1 |tee pvs.out
+# The md components should still be detected and excluded.
+not pvs "$dev1"
+not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
+
+pvs -vvvv
 
 # should not activate from the md legs
 not vgchange -ay $vg
@@ -187,16 +215,19 @@ not grep "active" out
 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
+# Now that the md dev is online, pvs can see it
+# and check for components even if
+# md_component_checks is "start" (which disables
+# most default end-of-device scans)
+aux lvmconf 'devices/md_component_checks = "start"'
 
-pvs 2>&1 |tee out
-cat out
-not grep "prefers device" out
+not pvs "$dev1"
+not pvs "$dev2"
+pvs > out
+not grep "$dev1" out
+not grep "$dev2" out
 
 vgchange -ay $vg 2>&1 |tee out
-cat out
-not grep "prefers device" out
 
 check lv_field $vg/$lv1 lv_active "active"
 

test/shell/lvmcache-exercise.sh

@@ -43,14 +43,16 @@ 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 reappeared out
+not grep "is missing PV" out
 
 # This removes the first "vg1" using its uuid
 vgremove -ff -S vg_uuid=$UUID1

test/shell/metadata-bad-mdaheader.sh

@@ -0,0 +1,80 @@
+#!/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
+
+#
+# Test corrupted mda_header.version field, which also
+# causes the mda_header checksum to be bad.
+#
+# FIXME: if a VG has only a single PV, this repair
+# doesn't work since there's no good PV to get
+# metadata from.  A more advanced repair capability
+# is needed.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+if [ -e "/usr/bin/xxd" ]; then
+
+# read mda_header which is 4k from start of disk
+dd if="$dev1" of=meta1 bs=4k count=1 skip=1
+
+# convert binary to text
+xxd meta1 > meta1.txt
+
+# Corrupt mda_header by changing the version field from 0100 to 0200
+sed 's/0000010:\ 304e\ 2a3e\ 0100\ 0000\ 0010\ 0000\ 0000\ 0000/0000010:\ 304e\ 2a3e\ 0200\ 0000\ 0010\ 0000\ 0000\ 0000/' meta1.txt > meta1-bad.txt
+
+# convert text to binary
+xxd -r meta1-bad.txt > meta1-bad
+
+# write bad mda_header back to disk
+dd if=meta1-bad of="$dev1" bs=4k seek=1
+
+# pvs reports bad metadata header
+pvs 2>&1 | tee out
+grep "bad metadata header" out
+
+fi
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in one mda doesn't prevent using
+# the VG since other mdas are fine and usable
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata header" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+

test/shell/metadata-bad-text.sh

@@ -0,0 +1,236 @@
+#!/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
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+dd if="$dev1" of=meta1 bs=4k count=2
+
+sed 's/flags =/flagx =/' meta1 > meta1.bad
+
+dd if=meta1.bad of="$dev1"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in one mda doesn't prevent using
+# the VG since other mdas are fine and usable
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+
+#
+# Same test as above, but corrupt metadata text
+# on two of the three PVs, leaving one good
+# copy of the metadata.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+dd if="$dev1" of=meta1 bs=4k count=2
+dd if="$dev2" of=meta2 bs=4k count=2
+
+sed 's/READ/RRRR/' meta1 > meta1.bad
+sed 's/seqno =/sss =/' meta2 > meta2.bad
+
+dd if=meta1.bad of="$dev1"
+dd if=meta2.bad of="$dev2"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out > out2
+grep "$dev1" out2
+grep "$dev2" out2
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in one mda doesn't prevent using
+# the VG since other mdas are fine
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+#
+# Three PVs where two have one mda, and the third
+# has two mdas.  The first mda is corrupted on all
+# thee PVs, but the second mda on the third PV
+# makes the VG usable.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvs
+
+dd if="$dev1" of=meta1 bs=4k count=2
+dd if="$dev2" of=meta2 bs=4k count=2
+dd if="$dev3" of=meta3 bs=4k count=2
+
+sed 's/READ/RRRR/' meta1 > meta1.bad
+sed 's/seqno =/sss =/' meta2 > meta2.bad
+sed 's/id =/id/' meta3 > meta3.bad
+
+dd if=meta1.bad of="$dev1"
+dd if=meta2.bad of="$dev2"
+dd if=meta3.bad of="$dev3"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out > out2
+grep "$dev1" out2
+grep "$dev2" out2
+grep "$dev3" out2
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# bad metadata in some mdas doesn't prevent using
+# the VG if there's a good mda found
+lvcreate -l1 $vg
+
+
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "bad metadata text" out
+
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+vgchange -an $vg
+vgremove -ff $vg
+
+#
+# Test that vgck --updatemetadata will update old metadata
+# and repair bad metadata text at the same time from different
+# devices.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+# Put bad metadata onto dev1
+dd if="$dev1" of=meta1 bs=4k count=2
+sed 's/READ/RRRR/' meta1 > meta1.bad
+dd if=meta1.bad of="$dev1"
+
+pvs 2>&1 | tee out
+grep "bad metadata text" out > out2
+grep "$dev1" out2
+
+# We can still use the VG with other available
+# mdas, skipping the bad mda.
+
+lvcreate -n $lv1 -l1 -an $vg "$dev1"
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+# Put old metadata onto dev2 by updating
+# the VG while dev2 is disabled.
+aux disable_dev "$dev2"
+
+pvs
+pvs "$dev1"
+not pvs "$dev2"
+pvs "$dev3"
+lvs $vg/$lv1
+lvs $vg/$lv2
+
+lvremove $vg/$lv2
+
+aux enable_dev "$dev2"
+
+# Both old and bad metadata are reported.
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+grep "bad metadata text" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+# fixes the bad metadata on dev1, and
+# fixes the old metadata on dev2.
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+not grep "bad metadata text" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+
+vgchange -an $vg
+vgremove -ff $vg
+

test/shell/metadata-old.sh

@@ -0,0 +1,177 @@
+#!/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
+
+#
+# Test "old metadata" repair which occurs when the VG is written
+# and one of the PVs in the VG does not get written to, and then
+# the PV reappears with the old metadata.  This can happen if
+# a command is killed or crashes after writing new metadata to
+# only some of the PVs in the VG, or if a PV is temporarily
+# inaccessible while a VG is written.
+#
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+#
+# Test that vgck --updatemetadata will update old metadata.
+#
+
+lvcreate -n $lv1 -l1 -an $vg "$dev1"
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+aux disable_dev "$dev2"
+
+pvs
+pvs "$dev1"
+not pvs "$dev2"
+pvs "$dev3"
+lvs $vg/$lv1
+lvs $vg/$lv2
+
+lvremove $vg/$lv2
+
+aux enable_dev "$dev2"
+
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+# fixes the old metadata on dev1
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+#
+# Test that any writing command will also update the
+# old metadata.
+#
+
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+aux disable_dev "$dev2"
+
+pvs
+pvs "$dev1"
+not pvs "$dev2"
+pvs "$dev3"
+lvs $vg/$lv1
+lvs $vg/$lv2
+
+lvremove $vg/$lv2
+
+aux enable_dev "$dev2"
+
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+# fixes the old metadata on dev1
+lvcreate -n $lv3 -l1 -an $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+not lvs $vg/$lv2
+
+vgremove -ff $vg
+
+#
+# First two PVs with one mda, where both have old metadata.
+# Third PV with two mdas, where the first mda has old
+# metadata, and the second mda has current metadata.
+#
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+lvcreate -n $lv1 -l1 -an $vg "$dev3"
+lvcreate -n $lv2 -l1 -an $vg "$dev3"
+
+# Save the metadata at this point...
+dd if="$dev1" of=meta1 bs=4k count=4
+dd if="$dev2" of=meta2 bs=4k count=4
+dd if="$dev3" of=meta3 bs=4k count=4
+
+# and now change metadata so the saved copies are old
+lvcreate -n $lv3 -l1 -an $vg "$dev3"
+
+# Copy the saved metadata back to the three
+# devs first mda, leaving the second mda on
+# dev3 as the only latest copy of the metadata.
+
+dd if=meta1 of="$dev1"
+dd if=meta2 of="$dev2"
+dd if=meta3 of="$dev3"
+
+pvs 2>&1 | tee out
+grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+# We still see the three LVs since we are using
+# the latest copy of metadata from dev3:mda2
+
+lvs $vg/$lv1
+lvs $vg/$lv2
+lvs $vg/$lv3
+
+# This command which writes the VG should update
+# all of the old copies.
+lvcreate -n $lv4 -l1 -an $vg
+
+pvs 2>&1 | tee out
+not grep "ignoring metadata seqno" out
+pvs "$dev1"
+pvs "$dev2"
+pvs "$dev3"
+
+lvs $vg/$lv1
+lvs $vg/$lv2
+lvs $vg/$lv3
+lvs $vg/$lv4
+
+vgchange -an $vg
+vgremove -ff $vg

test/shell/mirror-vgreduce-removemissing.sh

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

test/shell/missing-pv-unused.sh

@@ -0,0 +1,86 @@
+#!/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 $vg "$dev2"
+lvcreate -n $lv2 -L8M $vg "$dev3"
+lvcreate -n $lv3 -L8M $vg "$dev2"
+lvcreate -n $lv4 -L8M $vg "$dev3"
+
+vgchange -an $vg
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Fail device that is not used by any LVs.
+aux disable_dev "$dev1"
+
+pvs
+vgs
+lvs -a -o+devices
+
+# Cannot do normal activation of LVs not using failed PV.
+lvchange -ay $vg/$lv1
+lvchange -ay $vg/$lv2
+
+vgchange -an $vg
+
+# Check that MISSING flag is not set in ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+not grep MISSING meta
+rm meta
+
+pvs
+vgs
+lvs -a -o+devices
+
+# lvremove is one of the few commands that is allowed to run
+# when PVs are missing.  The vg_write from this command sets
+# the MISSING flag on the PV in the ondisk metadata.
+# (this could be changed, the MISSING flag wouldn't need
+# to be set in the first place since the PV isn't used.)
+lvremove $vg/$lv1
+
+# Check that MISSING flag is set in ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+# with MISSING flag in metadata, restrictions apply
+not lvcreate -l1 $vg
+
+aux enable_dev "$dev1"
+
+# No LVs are using the PV with MISSING flag, so no restrictions
+# are applied, and the vg_write here clears the MISSING flag on disk.
+lvcreate -l1 $vg
+
+# Check that MISSING flag is not set in ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+not grep MISSING meta
+rm meta
+
+
+pvs
+vgs
+lvs -a -o+devices
+
+vgchange -an $vg
+vgremove -ff $vg
+

test/shell/missing-pv.sh

@@ -0,0 +1,152 @@
+#!/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
+
+# 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.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+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.)
+
+# Check that MISSING flag is still set in ondisk
+# metadata since the previously missing dev is still
+# used by lv2.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+
+# 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
+
+# Check that MISSING flag is still set in ondisk
+# metadata since the previously missing dev is still
+# used by lv2.
+pvck --dump metadata "$dev2" > meta
+grep MISSING meta
+rm meta
+
+
+# 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, could also use vgck --updatemetadata vg.
+lvcreate -l1 $vg
+
+# Check that the MISSING flag is no longer set
+# in the ondisk metadata.
+pvck --dump metadata "$dev2" > meta
+not grep MISSING meta
+rm meta
+
+
+pvs
+vgs
+lvs -a -o+devices
+
+vgchange -an $vg
+vgremove -ff $vg
+

test/shell/outdated-pv.sh

@@ -0,0 +1,66 @@
+#!/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
+
+#
+# Test handling of "outdated PV" which occurs when a PV goes missing
+# from a VG, and while it's missing the PV is removed from the VG.
+# Then the PV reappears with the old VG metadata that shows it is a
+# member.  That outdated metadata needs to be cleared.
+#
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+lvcreate -n $lv1 -l1 -an $vg "$dev1"
+lvcreate -n $lv2 -l1 -an $vg "$dev1"
+
+aux disable_dev "$dev2"
+
+vgreduce --removemissing $vg
+
+pvs
+
+aux enable_dev "$dev2"
+
+pvs 2>&1 | tee out
+grep "outdated" out
+
+not pvs "$dev2"
+
+# The VG can still be used with the outdated PV around
+lvcreate -n $lv3 -l1 $vg
+lvchange -ay $vg
+lvs $vg
+lvchange -an $vg
+
+# Clears the outdated PV
+vgck --updatemetadata $vg
+
+pvs 2>&1 | tee out
+not grep "outdated" out
+
+# The PV is no longer in the VG
+pvs "$dev2" | tee out
+not grep "$vg" out
+
+# The cleared PV can be added back to the VG
+vgextend $vg "$dev2"
+
+pvs "$dev2" | tee out
+grep "$vg" out
+
+vgremove -ff $vg
+

test/shell/pv-ext-flags.sh

@@ -39,7 +39,6 @@ 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
@@ -71,20 +70,14 @@ 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, automatic repair will happen on pvs call
-# (or any other lvm command that does vg_read with repair inside)
-aux enable_dev "$dev1"
 
-# 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"
+# now, enable $dev1 and clear the old metadata from it
+aux enable_dev "$dev1"
+vgck --updatemetadata $vg1
 
 vgck $vg1
-# 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 $dev1 does not contain the PV_EXT_FLAG anymore
 check pv_field "$dev1" pv_in_use ""
 
 #############################################
@@ -105,7 +98,6 @@ 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
@@ -136,20 +128,14 @@ 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, automatic repair will happen on pvs call
-# (or any other lvm command that does vg_read with repair inside)
-aux enable_dev "$dev1"
 
-# 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"
+# now, enable $dev1 and clear the old metadata from it
+aux enable_dev "$dev1"
+vgck --updatemetadata $vg1
 
 vgck $vg1
-# 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 $dev1 does not contain the PV_EXT_FLAG anymore
 check pv_field "$dev1" pv_in_use ""
 
 ###########################

test/shell/pvck-dump.sh

@@ -0,0 +1,119 @@
+#!/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
+
+dd if=/dev/zero of="$dev1" || true
+dd if=/dev/zero of="$dev2" || true
+dd if=/dev/zero of="$dev3" || true
+
+pvcreate "$dev1"
+pvcreate "$dev2"
+pvcreate --pvmetadatacopies 2 "$dev3"
+
+vgcreate $SHARED $vg "$dev1" "$dev2" "$dev3"
+
+pvck --dump headers "$dev1" > h1
+pvck --dump headers "$dev2" > h2
+pvck --dump headers "$dev3" > h3
+
+grep "label_header at 512" h1
+grep "label_header at 512" h2
+grep "label_header at 512" h3
+
+grep "pv_header at 544" h1
+grep "pv_header at 544" h2
+grep "pv_header at 544" h3
+
+grep "pv_header.disk_locn\[0\].offset 1048576" h1
+grep "pv_header.disk_locn\[0\].offset 1048576" h2
+grep "pv_header.disk_locn\[0\].offset 1048576" h3
+
+grep "pv_header.disk_locn\[2\].offset 4096" h1
+grep "pv_header.disk_locn\[2\].offset 4096" h2
+grep "pv_header.disk_locn\[2\].offset 4096" h3
+
+grep "pv_header.disk_locn\[2\].size 1044480" h1
+grep "pv_header.disk_locn\[2\].size 1044480" h2
+grep "pv_header.disk_locn\[2\].size 1044480" h3
+
+grep "mda_header_1 at 4096" h1
+grep "mda_header_1 at 4096" h2
+grep "mda_header_1 at 4096" h3
+
+grep "mda_header_1.start 4096" h1
+grep "mda_header_1.start 4096" h2
+grep "mda_header_1.start 4096" h3
+
+grep "mda_header_1.size 1044480" h1
+grep "mda_header_1.size 1044480" h2
+grep "mda_header_1.size 1044480" h3
+
+grep "mda_header_2 at " h3
+grep "mda_header_2.start " h3
+
+grep "metadata text at " h1
+grep "metadata text at " h2
+grep "metadata text at " h3
+
+not grep CHECK h1
+not grep CHECK h2
+not grep CHECK h3
+
+pvck --dump metadata "$dev1" > m1
+pvck --dump metadata "$dev2" > m2
+pvck --dump metadata "$dev3" > m3
+pvck --dump metadata --pvmetadatacopies 2 "$dev3" > m3b
+
+diff m1 m2
+diff m1 m3
+
+not diff m1 m3b > tmp
+grep "metadata text at" tmp
+
+lvcreate -an -l1 $vg
+
+pvck --dump metadata_all -f all1 "$dev1" > out1
+pvck --dump metadata_all -f all2 "$dev2" > out2
+pvck --dump metadata_all -f all3 "$dev3" > out3
+pvck --dump metadata_all --pvmetadatacopies 2 -f all3b "$dev3" > out3b
+
+diff out1 out2
+diff out1 out3
+
+grep "seqno 1 with" out1
+grep "seqno 1 with" out3b
+grep "seqno 2 with" out1
+grep "seqno 2 with" out3b
+
+diff all1 all2
+diff all1 all3
+diff all1 all3b
+
+grep "seqno = 1" all1
+grep "seqno = 2" all1
+
+
+pvck --dump metadata_area -f area1 "$dev1"
+pvck --dump metadata_area -f area2 "$dev2"
+pvck --dump metadata_area -f area3 "$dev3"
+pvck --dump metadata_area -f area3b "$dev3"
+
+diff area1 area2
+diff area1 area3
+diff area1 area3b
+
+vgremove -ff $vg
+

test/shell/pvscan-autoactivate.sh

@@ -134,6 +134,8 @@ not ls "$RUNDIR/lvm/pvs_online/$PVID3"
 
 # pvscan cache ignores pv in a foreign vg
 
+if [ -e "/etc/machine-id" ]; then
+
 aux lvmconf "global/system_id_source = machineid"
 
 _clear_online_files
@@ -168,6 +170,8 @@ 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

test/shell/unlost-pv.sh

@@ -15,47 +15,59 @@ 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"
 
-check_
-test -e LOCAL_LVMETAD && pvcreate -f "$dev1"
-check_ not
+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
+
 
-# 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"
-check_
-test -e LOCAL_LVMETAD && lvremove $vg/boo # FIXME trigger a write :-(
-check_ not
 
 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
+
+aux disable_dev "$dev1"
+
 vgreduce --removemissing --force $vg
+
 aux enable_dev "$dev1"
 
 vgscan 2>&1 | tee out
-grep 'Removing PV' out
 
-vgs 2>&1 | tee out
-not 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
 
 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 "Failed" vgscan.out
+grep "checksum" vgscan.out
 
 dd if=/dev/urandom bs=512 seek=2 count=32 of="$dev2"
 
 vgck $vg 2>&1 | tee vgck.out || true
-grep Incorrect vgck.out
+grep "checksum" vgck.out
 
 vgremove -ff $vg

tools/args.h

@@ -215,8 +215,10 @@ arg(driverloaded_ARG, '\0', "driverloaded", bool_VAL, 0, 0,
 
 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")
+    "to print or save the current text metadata, \\fBmetadata_area\\fP\n"
+    "to save the entire text metadata area to a file, \\fBmetadata_all\\fP\n"
+    "to save the current and any previous complete versions of metadata\n"
+    "to a file, and \\fBheaders\\fP to print and check LVM headers.\n")
 
 arg(errorwhenfull_ARG, '\0', "errorwhenfull", bool_VAL, 0, 0,
     "Specifies thin pool behavior when data space is exhausted.\n"
@@ -711,9 +713,8 @@ arg(trackchanges_ARG, '\0', "trackchanges", 0, 0, 0,
     "merging the split image (see --mergemirrors) or permanently splitting\n"
     "the image (see --splitmirrors with --name.\n")
 
-/* TODO: hide this? */
 arg(trustcache_ARG, '\0', "trustcache", 0, 0, 0,
-    "Avoids certain device scanning during command processing. Do not use.\n")
+    "No longer used.\n")
 
 arg(type_ARG, '\0', "type", segtype_VAL, 0, 0,
     "The LV type, also known as \"segment type\" or \"segtype\".\n"
@@ -1391,6 +1392,9 @@ 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

@@ -198,7 +198,7 @@ OO_REPORT: --aligned, --all, --binary, --configreport ConfigReport, --foreign,
 --nameprefixes, --noheadings, --nosuffix,
 --options String, --readonly, --reportformat ReportFmt, --rows,
 --select String, --separator String, --shared, --sort String,
---trustcache, --unbuffered, --units Units, --unquoted
+--unbuffered, --units Units, --unquoted
 
 #
 # options for config, dumpconfig, lvmconfig
@@ -1384,7 +1384,7 @@ DESC: Resize a pool metadata SubLV by a specified size.
 lvs
 OO: --history, --segments, OO_REPORT
 OP: VG|LV|Tag ...
-IO: --partial, --ignoreskippedcluster
+IO: --partial, --ignoreskippedcluster, --trustcache
 ID: lvs_general
 
 ---
@@ -1404,7 +1404,7 @@ OO_PVCHANGE: --autobackup Bool, --force, --reportformat ReportFmt, --uuid
 OO_PVCHANGE_META: --allocatable Bool, --addtag Tag, --deltag Tag,
 --uuid, --metadataignore Bool
 
-pvchange OO_PVCHANGE_META --all
+pvchange --all OO_PVCHANGE_META
 OO: OO_PVCHANGE
 IO: --ignoreskippedcluster
 ID: pvchange_properties_all
@@ -1430,9 +1430,9 @@ 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
+OO: --file String, --pvmetadatacopies MetadataCopiesPV, --labelsector Number
+ID: pvck_dump
+DESC: Print metadata from a device
 
 ---
 
@@ -1486,7 +1486,7 @@ ID: pvremove_general
 pvs
 OO: --segments, OO_REPORT
 OP: PV|Tag ...
-IO: --partial, --ignoreskippedcluster
+IO: --partial, --ignoreskippedcluster, --trustcache
 ID: pvs_general
 
 ---
@@ -1624,6 +1624,11 @@ 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.
 
 ---
 
@@ -1747,7 +1752,7 @@ DESC: Rename a VG by specifying the VG UUID.
 vgs
 OO: OO_REPORT
 OP: VG|Tag ...
-IO: --partial, --ignoreskippedcluster
+IO: --partial, --ignoreskippedcluster, --trustcache
 ID: vgs_general
 
 ---
@@ -1802,7 +1807,7 @@ ID: devtypes_general
 fullreport
 OO: OO_REPORT
 OP: VG ...
-IO: --partial, --ignoreskippedcluster
+IO: --partial, --ignoreskippedcluster, --trustcache
 ID: fullreport_general
 
 lastlog

tools/command.c

@@ -1070,7 +1070,7 @@ static void _add_required_opt_line(struct cmd_context *cmdtool, struct command *
  * (This is the special case of vgchange/lvchange where one
  * optional option is required, and others are then optional.)
  * The set of options from OO_FOO are saved in required_opt_args,
- * and flag CMD_FLAG_ONE_REQUIRED_OPT is set on the cmd indicating
+ * and flag CMD_FLAG_ANY_REQUIRED_OPT is set on the cmd indicating
  * this special case.
  */
  
@@ -1104,6 +1104,7 @@ static void _add_required_line(struct cmd_context *cmdtool, struct command *cmd,
 	int i;
 	int takes_arg;
 	int prev_was_opt = 0, prev_was_pos = 0;
+	int orig_ro_count = 0;
 
 	/* argv[0] is command name */
 
@@ -1128,10 +1129,20 @@ static void _add_required_line(struct cmd_context *cmdtool, struct command *cmd,
 			prev_was_pos = 1;
 
 		} else if (!strncmp(argv[i], "OO_", 3)) {
-			/* one required_opt_arg is required, special case lv/vgchange */
-			cmd->cmd_flags |= CMD_FLAG_ONE_REQUIRED_OPT;
+			/*
+			 * the first ro_count entries in required_opt_arg required,
+			 * after which one or more of the next any_ro_count entries
+			 * in required_opt_arg are required.  required_opt_arg
+			 * has a total of ro_count+any_ro_count entries.
+			 */
+			cmd->cmd_flags |= CMD_FLAG_ANY_REQUIRED_OPT;
+			orig_ro_count = cmd->ro_count;
+
 			_include_required_opt_args(cmdtool, cmd, argv[i]);
 
+			cmd->any_ro_count = cmd->ro_count - orig_ro_count;
+			cmd->ro_count = orig_ro_count;
+
 		} else if (prev_was_pos) {
 			/* set property for previous required_pos_arg */
 			_update_prev_pos_arg(cmd, argv[i], REQUIRED);
@@ -1281,7 +1292,7 @@ void factor_common_options(void)
 				if (strcmp(cmd->name, command_names[cn].name))
 					continue;
 
-				if (cmd->ro_count)
+				if (cmd->ro_count || cmd->any_ro_count)
 					command_names[cn].variant_has_ro = 1;
 				if (cmd->rp_count)
 					command_names[cn].variant_has_rp = 1;
@@ -1290,7 +1301,7 @@ void factor_common_options(void)
 				if (cmd->op_count)
 					command_names[cn].variant_has_op = 1;
 
-				for (ro = 0; ro < cmd->ro_count; ro++) {
+				for (ro = 0; ro < cmd->ro_count + cmd->any_ro_count; ro++) {
 					command_names[cn].all_options[cmd->required_opt_args[ro].opt] = 1;
 
 					if ((cmd->required_opt_args[ro].opt == size_ARG) && !strncmp(cmd->name, "lv", 2))
@@ -1773,7 +1784,7 @@ static void _print_usage_def(struct command *cmd, int opt_enum, struct arg_def *
 void print_usage(struct command *cmd, int longhelp, int desc_first)
 {
 	struct command_name *cname = _find_command_name(cmd->name);
-	int onereq = (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) ? 1 : 0;
+	int any_req = (cmd->cmd_flags & CMD_FLAG_ANY_REQUIRED_OPT) ? 1 : 0;
 	int include_extents = 0;
 	int ro, rp, oo, op, opt_enum, first;
 
@@ -1788,20 +1799,31 @@ void print_usage(struct command *cmd, int longhelp, int desc_first)
 
 	printf("  %s", cmd->name);
 
-	if (onereq && cmd->ro_count) {
+	if (any_req) {
+		for (ro = 0; ro < cmd->ro_count; ro++) {
+			opt_enum = cmd->required_opt_args[ro].opt;
+
+			if (opt_names[opt_enum].short_opt)
+				printf(" -%c|%s", opt_names[opt_enum].short_opt, opt_names[opt_enum].long_opt);
+			else
+				printf(" %s", opt_names[opt_enum].long_opt);
+
+			if (cmd->required_opt_args[ro].def.val_bits) {
+				printf(" ");
+				_print_usage_def(cmd, opt_enum, &cmd->required_opt_args[ro].def);
+			}
+		}
+
 		/* one required option in a set */
 		first = 1;
 
 		/* options with short and long */
-		for (ro = 0; ro < cmd->ro_count; ro++) {
+		for (ro = cmd->ro_count; ro < cmd->ro_count + cmd->any_ro_count; ro++) {
 			opt_enum = cmd->required_opt_args[ro].opt;
 
 			if (!opt_names[opt_enum].short_opt)
 				continue;
 
-			if ((opt_enum == size_ARG) && command_has_alternate_extents(cmd->name))
-				include_extents = 1;
-
 			if (first)
 				printf("\n\t(");
 			else
@@ -1817,7 +1839,7 @@ void print_usage(struct command *cmd, int longhelp, int desc_first)
 		}
 
 		/* options with only long */
-		for (ro = 0; ro < cmd->ro_count; ro++) {
+		for (ro = cmd->ro_count; ro < cmd->ro_count + cmd->any_ro_count; ro++) {
 			opt_enum = cmd->required_opt_args[ro].opt;
 
 			if (opt_names[opt_enum].short_opt)
@@ -1843,7 +1865,7 @@ void print_usage(struct command *cmd, int longhelp, int desc_first)
 		printf(" )\n");
 	}
 
-	if (!onereq && cmd->ro_count) {
+	if (!any_req && cmd->ro_count) {
 		for (ro = 0; ro < cmd->ro_count; ro++) {
 			opt_enum = cmd->required_opt_args[ro].opt;
 
@@ -1863,7 +1885,7 @@ void print_usage(struct command *cmd, int longhelp, int desc_first)
 	}
 
 	if (cmd->rp_count) {
-		if (onereq)
+		if (any_req)
 			printf("\t");
 		for (rp = 0; rp < cmd->rp_count; rp++) {
 			if (cmd->required_pos_args[rp].def.val_bits) {
@@ -2403,7 +2425,7 @@ static const char *_man_long_opt_name(const char *cmdname, int opt_enum)
 static void _print_man_usage(char *lvmname, struct command *cmd)
 {
 	struct command_name *cname;
-	int onereq = (cmd->cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) ? 1 : 0;
+	int any_req = (cmd->cmd_flags & CMD_FLAG_ANY_REQUIRED_OPT) ? 1 : 0;
 	int sep, ro, rp, oo, op, opt_enum;
 	int need_ro_indent_end = 0;
 	int include_extents = 0;
@@ -2413,9 +2435,44 @@ static void _print_man_usage(char *lvmname, struct command *cmd)
 
 	printf("\\fB%s\\fP", lvmname);
 
-	if (!onereq)
+	if (!any_req)
 		goto ro_normal;
 
+	/*
+	 * required options that follow command name, all required
+	 */
+	if (cmd->ro_count) {
+		sep = 0;
+
+		for (ro = 0; ro < cmd->ro_count; ro++) {
+
+			/* avoid long line wrapping */
+			if ((cmd->ro_count > 2) && (sep == 2)) {
+				printf("\n.RS 5\n");
+				need_ro_indent_end = 1;
+			}
+
+			opt_enum = cmd->required_opt_args[ro].opt;
+
+			if ((opt_enum == size_ARG) && command_has_alternate_extents(cmd->name))
+				include_extents = 1;
+
+			if (opt_names[opt_enum].short_opt) {
+				printf(" \\fB-%c\\fP|\\fB%s\\fP",
+				       opt_names[opt_enum].short_opt,
+				       _man_long_opt_name(cmd->name, opt_enum));
+			} else
+				printf(" \\fB%s\\fP", opt_names[cmd->required_opt_args[ro].opt].long_opt);
+
+			if (cmd->required_opt_args[ro].def.val_bits) {
+				printf(" ");
+				_print_def_man(cname, opt_enum, &cmd->required_opt_args[ro].def, 1);
+			}
+
+			sep++;
+		}
+	}
+
 	/*
 	 * one required option in a set, print as:
 	 * ( -a|--a,
@@ -2427,7 +2484,7 @@ static void _print_man_usage(char *lvmname, struct command *cmd)
 	 * and the second loop prints those without short opts.
 	 */
 
-	if (cmd->ro_count) {
+	if (cmd->any_ro_count) {
 		printf("\n");
 		printf(".RS 4\n");
 		printf("(");
@@ -2435,7 +2492,7 @@ static void _print_man_usage(char *lvmname, struct command *cmd)
 		sep = 0;
 
 		/* print required options with a short opt */
-		for (ro = 0; ro < cmd->ro_count; ro++) {
+		for (ro = cmd->ro_count; ro < cmd->ro_count + cmd->any_ro_count; ro++) {
 			opt_enum = cmd->required_opt_args[ro].opt;
 
 			if (!opt_names[opt_enum].short_opt)
@@ -2467,7 +2524,7 @@ static void _print_man_usage(char *lvmname, struct command *cmd)
 		}
 
 		/* print required options without a short opt */
-		for (ro = 0; ro < cmd->ro_count; ro++) {
+		for (ro = cmd->ro_count; ro < cmd->ro_count + cmd->any_ro_count; ro++) {
 			opt_enum = cmd->required_opt_args[ro].opt;
 
 			if (opt_names[opt_enum].short_opt)
@@ -2497,7 +2554,7 @@ static void _print_man_usage(char *lvmname, struct command *cmd)
 		printf(".RE\n");
 	}
 
-	/* print required position args on a new line after the onereq set */
+	/* print required position args on a new line after the any_req set */
 	if (cmd->rp_count) {
 		printf(".RS 4\n");
 		for (rp = 0; rp < cmd->rp_count; rp++) {

tools/command.h

@@ -164,8 +164,16 @@ struct cmd_rule {
 /*
  * one or more from required_opt_args is required,
  * then the rest are optional.
+ *
+ * CMD_FLAG_ANY_REQUIRED_OPT: for lvchange/vgchange special case.
+ * The first ro_count entries of required_opt_args must be met
+ * (ro_count may be 0.)  After this, one or more options must be
+ * set from the remaining required_opt_args.  So, the first
+ * ro_count options in required_opt_args must match, and after
+ * that one or more of the remaining options in required_opt_args
+ * must match.
  */
-#define CMD_FLAG_ONE_REQUIRED_OPT   1  /* lvchange/vgchage require one item from required_opt_args */
+#define CMD_FLAG_ANY_REQUIRED_OPT   1
 #define CMD_FLAG_SECONDARY_SYNTAX   2  /* allows syntax variants to be suppressed in certain output */
 #define CMD_FLAG_PREVIOUS_SYNTAX    4  /* allows syntax variants to not be advertised in output */
 #define CMD_FLAG_PARSE_ERROR        8  /* error parsing command-lines.in def */
@@ -202,6 +210,8 @@ struct command {
 
 	struct cmd_rule rules[CMD_MAX_RULES];
 
+	int any_ro_count;
+
 	int ro_count;
 	int oo_count;
 	int rp_count;

tools/commands.h

@@ -177,7 +177,7 @@ xx(vgchange,
 
 xx(vgck,
    "Check the consistency of volume group(s)",
-   ALL_VGS_IS_DEFAULT | LOCKD_VG_SH | ALLOW_HINTS)
+   ALL_VGS_IS_DEFAULT | LOCKD_VG_SH)
 
 xx(vgconvert,
    "Change volume group metadata format",

tools/lvchange.c

@@ -1084,7 +1084,7 @@ static int _lvchange_properties_single(struct cmd_context *cmd,
 	 */
 
 	/* First group of options which allow for one metadata commit/update for the whole group */
-	for (i = 0; i < cmd->command->ro_count; i++) {
+	for (i = 0; i < cmd->command->ro_count + cmd->command->any_ro_count; i++) {
 		opt_enum = cmd->command->required_opt_args[i].opt;
 
 		if (!arg_is_set(cmd, opt_enum))
@@ -1206,7 +1206,7 @@ static int _lvchange_properties_single(struct cmd_context *cmd,
 		return_ECMD_FAILED;
 
 	/* Second group of options which need per option metadata commit+reload(s) */
-	for (i = 0; i < cmd->command->ro_count; i++) {
+	for (i = 0; i < cmd->command->ro_count + cmd->command->any_ro_count; i++) {
 		opt_enum = cmd->command->required_opt_args[i].opt;
 
 		if (!arg_is_set(cmd, opt_enum))

tools/lvmcmdline.c

@@ -1192,7 +1192,7 @@ static void _set_valid_args_for_command_name(int ci)
 		if (strcmp(commands[i].name, command_names[ci].name))
 			continue;
 
-		for (ro = 0; ro < commands[i].ro_count; ro++) {
+		for (ro = 0; ro < (commands[i].ro_count + commands[i].any_ro_count); ro++) {
 			opt_enum = commands[i].required_opt_args[ro].opt;
 			all_args[opt_enum] = 1;
 
@@ -1533,7 +1533,7 @@ static int _command_required_pos_matches(struct cmd_context *cmd, int ci, int rp
  * For each command[i], check how many required opt/pos args cmd matches.
  * Save the command[i] that matches the most.
  *
- * commands[i].cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT means
+ * commands[i].cmd_flags & CMD_FLAG_ANY_REQUIRED_OPT means
  * any one item from commands[i].required_opt_args needs to be
  * set to match.
  *
@@ -1551,7 +1551,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 	const char *name;
 	char opts_msg[MAX_OPTS_MSG];
 	char check_opts_msg[MAX_OPTS_MSG];
-	int match_required, match_ro, match_rp, match_type, match_unused, mismatch_required;
+	int match_required, match_ro, match_rp, match_any_ro, match_type, match_unused, mismatch_required;
 	int best_i = 0, best_required = 0, best_type = 0, best_unused = 0;
 	int close_i = 0, close_ro = 0, close_type = 0;
 	int only_i = 0;
@@ -1600,6 +1600,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 		match_required = 0;	/* required parameters that match */
 		match_ro = 0;		/* required opt_args that match */
 		match_rp = 0;		/* required pos_args that match */
+		match_any_ro = 0;
 		match_type = 0;		/* type arg matches */
 		match_unused = 0;	/* options set that are not accepted by command */
 		mismatch_required = 0;	/* required parameters that do not match */
@@ -1628,20 +1629,19 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			}
 		}
 
-		/*
-		 * Special case where missing required_opt_arg's does not matter
-		 * if one required_opt_arg did match.
-		 */
-		if (commands[i].cmd_flags & CMD_FLAG_ONE_REQUIRED_OPT) {
-			if (match_ro) {
-				/* one or more of the required_opt_args is used */
-				mismatch_required = 0;
-			} else {
-				/* not even one of the required_opt_args is used */
-				mismatch_required = 1;
+		for (ro = commands[i].ro_count; ro < commands[i].ro_count + commands[i].any_ro_count; ro++) {
+			if (_command_required_opt_matches(cmd, i, ro)) {
+				/* log_warn("match %d any ro opt %d", i, commands[i].required_opt_args[ro].opt); */
+				match_any_ro++;
 			}
 		}
 
+		if ((commands[i].cmd_flags & CMD_FLAG_ANY_REQUIRED_OPT) && !match_any_ro) {
+			/* not even one of the any ro is used */
+			/* log_warn("match %d not one from any", i); */
+			mismatch_required = 1;
+		}
+
 		/* match required_pos_args */
 
 		for (rp = 0; rp < commands[i].rp_count; rp++) {
@@ -1697,7 +1697,7 @@ static struct command *_find_command(struct cmd_context *cmd, const char *path,
 			accepted = 0;
 
 			/* NB in some cases required_opt_args are optional */
-			for (j = 0; j < commands[i].ro_count; j++) {
+			for (j = 0; j < commands[i].ro_count + commands[i].any_ro_count; j++) {
 				if (commands[i].required_opt_args[j].opt == opt_enum) {
 					accepted = 1;
 					break;
@@ -2396,17 +2396,6 @@ static int _get_current_settings(struct cmd_context *cmd)
 	if (arg_is_set(cmd, binary_ARG))
 		cmd->report_binary_values_as_numeric = 1;
 
-	if (arg_is_set(cmd, trustcache_ARG)) {
-		if (arg_is_set(cmd, all_ARG)) {
-			log_error("--trustcache is incompatible with --all");
-			return EINVALID_CMD_LINE;
-		}
-		init_trust_cache(1);
-		log_warn("WARNING: Cache file of PVs will be trusted.  "
-			  "New devices holding PVs may get ignored.");
-	} else
-		init_trust_cache(0);
-
 	if (arg_is_set(cmd, noudevsync_ARG))
 		cmd->current_settings.udev_sync = 0;
 
@@ -2766,20 +2755,53 @@ static int _init_lvmlockd(struct cmd_context *cmd)
 	return 1;
 }
 
+/*
+ * md_component_check full: always set use_full_md_check
+ * which causes filter-md to read the start+end of every
+ * device on the system (this could be optimized to only
+ * read the end of PVs.)
+ *
+ * md_component_check start: the end of devices will
+ * not generally be read to check for an md superblock
+ * (lvm may still scan for end-of-device md superblocks
+ * if it knows that some exists.)
+ *
+ * md_component_check auto: lvm will use some built-in
+ * heuristics to decide when it should scan the end of
+ * devices to look for md superblocks, e.g. commands
+ * like pvcreate that could clobber a component, or if
+ * udev info is not available and hints are not available.
+ */
 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"))
+	const char *md_check;
+
+	cmd->md_component_detection = find_config_tree_bool(cmd, devices_md_component_detection_CFG, NULL);
+
+	md_check = find_config_tree_str(cmd, devices_md_component_checks_CFG, NULL);
+	if (!md_check)
+		cmd->md_component_checks = "auto";
+	else if (!strcmp(md_check, "auto") ||
+	         !strcmp(md_check, "start") ||
+	         !strcmp(md_check, "full"))
+		cmd->md_component_checks = md_check;
+	else {
+		log_warn("Ignoring unknown md_component_checks setting, using auto.");
+		cmd->md_component_checks = "auto";
+	}
+
+	if (!strcmp(cmd->md_component_checks, "full"))
 		cmd->use_full_md_check = 1;
+	else if (!strcmp(cmd->md_component_checks, "auto")) {
+		/* use_full_md_check can also be set later */
+		if (!strcmp(cmd->name, "pvcreate") ||
+		    !strcmp(cmd->name, "vgcreate") ||
+		    !strcmp(cmd->name, "vgextend"))
+			cmd->use_full_md_check = 1;
+	}
+
+	log_debug("Using md_component_checks %s use_full_md_check %d",
+		  cmd->md_component_checks, cmd->use_full_md_check);
 }
 
 static int _cmd_no_meta_proc(struct cmd_context *cmd)
@@ -2909,12 +2931,12 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 	    !init_filters(cmd, !refresh_done))
 		return_ECMD_FAILED;
 
-	if (arg_is_set(cmd, readonly_ARG))
-		cmd->metadata_read_only = 1;
+	cmd->metadata_read_only = arg_is_set(cmd, readonly_ARG);
 
-	if ((cmd->command->command_enum == vgchange_activate_CMD) ||
-	    (cmd->command->command_enum == lvchange_activate_CMD))
-		cmd->is_activating = 1;
+	cmd->is_activating = (cmd->command->command_enum == vgchange_activate_CMD) ||
+			     (cmd->command->command_enum == lvchange_activate_CMD);
+
+	cmd->wipe_outdated_pvs = 0;
 
 	/*
 	 * Now that all configs, profiles and command lines args are available,
@@ -3010,7 +3032,7 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv)
 		ret = cmd->command->fn(cmd, argc, argv);
 
 	lvmlockd_disconnect();
-	fin_locking();
+	fin_locking(cmd);
 
 	if (!_cmd_no_meta_proc(cmd) && find_config_tree_bool(cmd, global_notify_dbus_CFG, NULL))
 		lvmnotify_send(cmd);

tools/polldaemon.c

@@ -148,6 +148,7 @@ 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)
@@ -168,12 +169,10 @@ 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);
-		if (vg_read_error(vg)) {
+		vg = vg_read(cmd, id->vg_name, NULL, READ_FOR_UPDATE, lockd_state, &error_flags, NULL);
+		if (!vg) {
 			/* What more could we do here? */
-			log_error("ABORTING: Can't reread VG for %s.", id->display_name);
-			release_vg(vg);
-			vg = NULL;
+			log_error("ABORTING: Can't reread VG for %s error flags %x.", id->display_name, error_flags);
 			ret = 0;
 			goto out;
 		}
@@ -395,6 +394,7 @@ 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;
 
 	/*
@@ -407,10 +407,9 @@ 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);
-	if (vg_read_error(vg)) {
-		release_vg(vg);
-		log_error("Can't reread VG for %s", id->display_name);
+	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);
 		ret = 0;
 		goto out_ret;
 	}

tools/toollib.c

@@ -189,11 +189,12 @@ 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 volume_group *vg, const char *vg_name,
-		      struct dm_list *arg_vgnames, uint32_t read_flags,
-		      int *skip, int *notfound)
+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)
 {
-	uint32_t read_error = vg_read_error(vg);
+	uint32_t read_error = error_flags;
 
 	*skip = 0;
 	*notfound = 0;
@@ -203,12 +204,9 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 		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.");
@@ -233,10 +231,13 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 	 * 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, vg->system_id, vg->cmd->system_id ? "" : "unknown ",
-				  vg->cmd->system_id ? " " : "", vg->cmd->system_id ? vg->cmd->system_id : "");
+				  vg_name,
+				  error_vg ? error_vg->system_id : "unknown ",
+				  cmd->system_id ? "" : "unknown ",
+				  cmd->system_id ? " " : "",
+				  cmd->system_id ? cmd->system_id : "");
 			return 1;
 		} else {
 			read_error &= ~FAILED_SYSTEMID; /* Check for other errors */
@@ -255,10 +256,11 @@ static int _ignore_vg(struct volume_group *vg, const char *vg_name,
 	 * 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, vg->lock_type);
+					  vg_name,
+					  error_vg ? error_vg->lock_type : "unknown");
 			/* For FAILED_LOCK_MODE, the error is printed in vg_read. */
 			return 1;
 		} else {
@@ -1924,10 +1926,12 @@ 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;
@@ -1977,13 +1981,18 @@ 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);
-		if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
+		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)) {
 			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;
 
@@ -2004,8 +2013,7 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
 				ret_max = ret;
 		}
 
-		if (!vg_read_error(vg))
-			unlock_vg(cmd, vg, vg_name);
+		unlock_vg(cmd, vg, vg_name);
 endvg:
 		release_vg(vg);
 		if (!lockd_vg(cmd, vg_name, "un", 0, &lockd_state))
@@ -2254,7 +2262,7 @@ int process_each_vg(struct cmd_context *cmd,
 	 */
 	log_very_verbose("Obtaining the complete list of VGs to process");
 
-	if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, include_internal)) {
+	if (!lvmcache_get_vgnameids(cmd, &vgnameids_on_system, NULL, include_internal)) {
 		ret_max = ECMD_FAILED;
 		goto_out;
 	}
@@ -3590,11 +3598,13 @@ 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;
@@ -3663,13 +3673,18 @@ 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);
-		if (_ignore_vg(vg, vg_name, arg_vgnames, read_flags, &skip, &notfound)) {
+		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)) {
 			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;
 
@@ -3792,7 +3807,7 @@ int process_each_lv(struct cmd_context *cmd,
 	 */
 	log_very_verbose("Obtaining the complete list of VGs before processing their LVs");
 
-	if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0)) {
+	if (!lvmcache_get_vgnameids(cmd, &vgnameids_on_system, NULL, 0)) {
 		ret_max = ECMD_FAILED;
 		goto_out;
 	}
@@ -4156,12 +4171,16 @@ 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';
@@ -4247,6 +4266,12 @@ 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)
@@ -4284,10 +4309,12 @@ 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;
@@ -4325,8 +4352,8 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 
 		log_debug("Processing PVs in VG %s", vg_name);
 
-		vg = vg_read(cmd, vg_name, vg_uuid, read_flags, lockd_state);
-		if (_ignore_vg(vg, vg_name, NULL, read_flags, &skip, &notfound)) {
+		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)) {
 			stack;
 			ret_max = ECMD_FAILED;
 			report_log_ret_code(ret_max);
@@ -4338,22 +4365,26 @@ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t read_flags,
 			goto endvg;
 		
 		/*
-		 * Don't continue when skip is set, because we need to remove
-		 * vg->pvs entries from devices list.
+		 * Don't call "continue" when skip is set, because we need to remove
+		 * error_vg->pvs entries from devices list.
 		 */
 		
-		ret = _process_pvs_in_vg(cmd, vg, all_devices, arg_devices, arg_tags,
+		ret = _process_pvs_in_vg(cmd, vg ? vg : error_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)
 			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;
@@ -4448,7 +4479,7 @@ int process_each_pv(struct cmd_context *cmd,
 	if (!(read_flags & PROCESS_SKIP_SCAN))
 		lvmcache_label_scan(cmd);
 
-	if (!get_vgnameids(cmd, &all_vgnameids, only_this_vgname, 1)) {
+	if (!lvmcache_get_vgnameids(cmd, &all_vgnameids, only_this_vgname, 1)) {
 		ret_max = ret;
 		goto_out;
 	}
@@ -5574,7 +5605,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, 0, pp->orphan_vg_name))) {
+		if (!(orphan_vg = vg_read_orphans(cmd, pp->orphan_vg_name))) {
 			log_error("Cannot read orphans VG %s.", pp->orphan_vg_name);
 			goto bad;
 		}

tools/vgcfgbackup.c

@@ -67,9 +67,12 @@ 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_read_error(vg) == FAILED_INCONSISTENT) {
-			log_error("No backup taken: specify filename with -f "
-				  "to backup an inconsistent VG");
+		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.");
 			return ECMD_FAILED;
 		}
 
@@ -97,9 +100,17 @@ 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, READ_ALLOW_INCONSISTENT, 0,
+	ret = process_each_vg(cmd, argc, argv, NULL, NULL, 0, 0,
 			      handle, &_vg_backup_single);
 
 	free(last_filename);

tools/vgchange.c

@@ -407,8 +407,8 @@ static int _vgchange_metadata_copies(struct cmd_context *cmd,
 {
 	uint32_t mda_copies = arg_uint_value(cmd, vgmetadatacopies_ARG, DEFAULT_VGMETADATACOPIES);
 
-	log_warn("vgchange_metadata_copies new %u vg_mda_copies %u D %u",
-		 mda_copies, vg_mda_copies(vg), DEFAULT_VGMETADATACOPIES);
+	log_debug("vgchange_metadata_copies new %u vg_mda_copies %u D %u",
+		  mda_copies, vg_mda_copies(vg), DEFAULT_VGMETADATACOPIES);
 
 	if (mda_copies == vg_mda_copies(vg)) {
 		if (vg_mda_copies(vg) == VGMETADATACOPIES_UNMANAGED)

tools/vgck.c

@@ -15,6 +15,57 @@
 
 #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,
@@ -37,6 +88,9 @@ 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

@@ -71,7 +71,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 	 * then do the initial label scan which reads all devices and
 	 * populates lvmcache with any VG name it finds.  If the VG name
 	 * we want to use exists, then the label scan will find it,
-	 * and the fmt_from_vgname call (used to check if the name exists)
+	 * and the vginfo_from_vgname call (used to check if the name exists)
 	 * will return non-NULL.
 	 */
 
@@ -82,7 +82,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 
 	lvmcache_label_scan(cmd);
 
-	if (lvmcache_fmt_from_vgname(cmd, vp_new.vg_name, NULL, 0)) {
+	if (lvmcache_vginfo_from_vgname(vp_new.vg_name, NULL)) {
 		unlock_vg(cmd, NULL, vp_new.vg_name);
 		log_error("A volume group called %s already exists.", vp_new.vg_name);
 		return ECMD_FAILED;

tools/vgextend.c

@@ -28,16 +28,25 @@ static int _restore_pv(struct volume_group *vg, const char *pv_name)
 		return 0;
 	}
 
-	if (!(pvl->pv->status & MISSING_PV)) {
-		log_warn("WARNING: PV %s was not missing in VG %s", pv_name, vg->name);
-		return 0;
-	}
-
 	if (!pvl->pv->dev) {
 		log_warn("WARNING: The PV %s is still missing.", pv_name);
 		return 0;
 	}
 
+	if (pvl->pv->status & MISSING_PV)
+		goto clear_flag;
+
+	/*
+	 * when the PV has no used PE's vg_read clears the MISSING_PV flag
+	 * and sets this so we know.
+	 */
+	if (pvl->pv->unused_missing_cleared)
+		goto clear_flag;
+
+	log_warn("WARNING: PV %s was not missing in VG %s", pv_name, vg->name);
+	return 0;
+
+clear_flag:
 	pvl->pv->status &= ~MISSING_PV;
 	return 1;
 }

tools/vgimportclone.c

@@ -296,7 +296,7 @@ int vgimportclone(struct cmd_context *cmd, int argc, char **argv)
 		vgname_count = 1;
 	}
 
-	if (!get_vgnameids(cmd, &vgnameids_on_system, NULL, 0))
+	if (!lvmcache_get_vgnameids(cmd, &vgnameids_on_system, NULL, 0))
 		goto_out;
 
 retry_name:

tools/vgremove.c

@@ -99,6 +99,8 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv)
 
 	clear_hint_file(cmd);
 
+	cmd->wipe_outdated_pvs = 1;
+
 	cmd->handles_missing_pvs = 1;
 	ret = process_each_vg(cmd, argc, argv, NULL, NULL,
 			      READ_FOR_UPDATE, 0,

tools/vgsplit.c

@@ -445,79 +445,6 @@ static int _move_cache(struct volume_group *vg_from,
 	return 1;
 }
 
-/*
- * Create or open the destination of the vgsplit operation.
- * Returns
- * - non-NULL: VG handle w/VG lock held
- * - NULL: no VG lock held
- */
-static struct volume_group *_vgsplit_to(struct cmd_context *cmd,
-					const char *vg_name_to,
-					int *existing_vg)
-{
-	struct volume_group *vg_to = NULL;
-
-	log_verbose("Checking for new volume group \"%s\"", vg_name_to);
-	/*
-	 * First try to create a new VG.  If we cannot create it,
-	 * and we get FAILED_EXIST (we will not be holding a lock),
-	 * a VG must already exist with this name.  We then try to
-	 * read the existing VG - the vgsplit will be into an existing VG.
-	 *
-	 * Otherwise, if the lock was successful, it must be the case that
-	 * we obtained a WRITE lock and could not find the vgname in the
-	 * system.  Thus, the split will be into a new VG.
-	 */
-	vg_to = vg_lock_and_create(cmd, vg_name_to);
-	if (vg_read_error(vg_to) == FAILED_LOCKING) {
-		log_error("Can't get lock for %s", vg_name_to);
-		release_vg(vg_to);
-		return NULL;
-	}
-	if (vg_read_error(vg_to) == FAILED_EXIST) {
-		*existing_vg = 1;
-		release_vg(vg_to);
-		vg_to = vg_read_for_update(cmd, vg_name_to, NULL, 0, 0);
-
-		if (vg_read_error(vg_to)) {
-			release_vg(vg_to);
-			return_NULL;
-		}
-
-	} else if (vg_read_error(vg_to) == SUCCESS) {
-		*existing_vg = 0;
-	}
-	return vg_to;
-}
-
-/*
- * Open the source of the vgsplit operation.
- * Returns
- * - non-NULL: VG handle w/VG lock held
- * - NULL: no VG lock held
- */
-static struct volume_group *_vgsplit_from(struct cmd_context *cmd,
-					  const char *vg_name_from)
-{
-	struct volume_group *vg_from;
-
-	log_verbose("Checking for volume group \"%s\"", vg_name_from);
-
-	vg_from = vg_read_for_update(cmd, vg_name_from, NULL, 0, 0);
-	if (vg_read_error(vg_from)) {
-		release_vg(vg_from);
-		return NULL;
-	}
-
-	if (vg_is_shared(vg_from)) {
-		log_error("vgsplit not allowed for lock_type %s", vg_from->lock_type);
-		unlock_and_release_vg(cmd, vg_from, vg_name_from);
-		return NULL;
-	}
-
-	return vg_from;
-}
-
 /*
  * Has the user given an option related to a new vg as the split destination?
  */
@@ -536,11 +463,11 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
 	struct vgcreate_params vp_def;
 	const char *vg_name_from, *vg_name_to;
 	struct volume_group *vg_to = NULL, *vg_from = NULL;
+	struct lvmcache_vginfo *vginfo_to;
 	int opt;
 	int existing_vg = 0;
 	int r = ECMD_FAILED;
 	const char *lv_name;
-	int lock_vg_from_first = 1;
 
 	if ((arg_is_set(cmd, name_ARG) + argc) < 3) {
 		log_error("Existing VG, new VG and either physical volumes "
@@ -576,47 +503,44 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
 
 	lvmcache_label_scan(cmd);
 
-	if (strcmp(vg_name_to, vg_name_from) < 0)
-		lock_vg_from_first = 0;
+	if (!(vginfo_to = lvmcache_vginfo_from_vgname(vg_name_to, NULL))) {
+		if (!validate_name(vg_name_to)) {
+			log_error("Invalid vg name %s.", vg_name_to);
+			return ECMD_FAILED;
+		}
 
-	if (lock_vg_from_first) {
-		if (!(vg_from = _vgsplit_from(cmd, vg_name_from)))
-			return_ECMD_FAILED;
-		/*
-		 * Set metadata format of original VG.
-		 * NOTE: We must set the format before calling vg_lock_and_create()
-		 * since vg_lock_and_create() calls the per-format constructor.
-		 */
-		cmd->fmt = vg_from->fid->fmt;
+		if (!lock_vol(cmd, vg_name_to, LCK_VG_WRITE, NULL)) {
+			log_error("Failed to lock new VG name %s.", vg_name_to);
+			return ECMD_FAILED;
+		}
 
-		if (!(vg_to = _vgsplit_to(cmd, vg_name_to, &existing_vg))) {
-			unlock_and_release_vg(cmd, vg_from, vg_name_from);
-			return_ECMD_FAILED;
+		if (!(vg_to = vg_create(cmd, vg_name_to))) {
+			log_error("Failed to create new VG %s.", vg_name_to);
+			unlock_vg(cmd, NULL, vg_name_to);
+			return ECMD_FAILED;
 		}
 	} else {
-		if (!(vg_to = _vgsplit_to(cmd, vg_name_to, &existing_vg)))
-			return_ECMD_FAILED;
-
-		if (!(vg_from = _vgsplit_from(cmd, vg_name_from))) {
-			unlock_and_release_vg(cmd, vg_to, vg_name_to);
-			return_ECMD_FAILED;
-		}
-
-		if (cmd->fmt != vg_from->fid->fmt) {
-			/* In this case we don't know the vg_from->fid->fmt */
-			log_error("Unable to set new VG metadata type based on "
-				  "source VG format - use -M option.");
-			goto bad;
+		if (!(vg_to = vg_read_for_update(cmd, vg_name_to, NULL, 0, 0))) {
+			log_error("Failed to read VG %s.", vg_name_to);
+			return ECMD_FAILED;
 		}
+		existing_vg = 1;
 	}
 
+	if (!(vg_from = vg_read_for_update(cmd, vg_name_from, NULL, 0, 0))) {
+		log_error("Failed to read VG %s.", vg_name_to);
+		unlock_and_release_vg(cmd, vg_to, vg_name_to);
+		return ECMD_FAILED;
+	}
+
+	cmd->fmt = vg_from->fid->fmt;
+
 	if (existing_vg) {
 		if (_new_vg_option_specified(cmd)) {
-			log_error("Volume group \"%s\" exists, but new VG "
-				    "option specified", vg_name_to);
+			log_error("Volume group \"%s\" exists, but new VG option specified", vg_name_to);
 			goto bad;
 		}
-		if (!vgs_are_compatible(cmd, vg_from,vg_to))
+		if (!vgs_are_compatible(cmd, vg_from, vg_to))
 			goto_bad;
 	} else {
 		if (!vgcreate_params_set_defaults(cmd, &vp_def, vg_from)) {