summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Teigland <teigland@redhat.com>2017-10-25 12:02:22 -0500
committerDavid Teigland <teigland@redhat.com>2017-10-26 11:40:03 -0500
commita873fd1704a72a46fd04ce69f6c95cd41cf0cbb8 (patch)
tree03ce9e79a7ca0a9421fed085e7c5e87c7f0a4116
parent11ba9f38ece78a24b33225cba38d90fa342b9336 (diff)
downloadlvm2-a873fd1704a72a46fd04ce69f6c95cd41cf0cbb8.tar.gz
label_scan: move to start of command
LVM's general design for scanning/reading of metadata from disks is that a command begins with a discovery phase, called "label scan", in which it discovers which devices belong to lvm, what VGs exist on those devices, and which devices are associated with each VG. After this comes the processing phase, which is based around processing specific VGs. In this phase, lvm acquires a lock on the VG, and rescans the devices associated with that VG, i.e. it repeats the label scan steps on the devices in the VG in case something has changed between the initial label scan and taking the VG lock. This ensures that the command is processing the lastest, unchanging data on disk. This commit moves the location of these label scans to make them clearer and avoid unnecessary repeated calls to them. Previously, the initial label scan was called as a side effect from various utility functions. This would lead to it being called unnecessarily. It is an expensive operation, and should only be called when necessary. Also, this is a primary step in the function of the command, and as such it should be called prominently at the top level of command processing, not as a hidden side effect of a utility function. lvm knows exactly where and when the label scan needs to be done. Because of this, move the label scan calls from the internal functions to the top level of processing. Other specific instances of lvmcache_label_scan() are still called unnecessarily or unclearly by specific commands that do not use the common process_each functions. These will be improved in future commits. During the processing phase, rescanning labels for devices in a VG needs to be done after the VG lock is acquired in case things have changed since the initial label scan. This was being done by way of rescanning devices that had the INVALID flag set in lvmcache. This usually approximated the right set of devices, but it was not exact, and obfuscated the real requirement. Correct this by using a new function that rescans the devices in the VG: lvmcache_label_rescan_vg(). Apart from being inexact, the rescanning was extremely well hidden. _vg_read() would call ->create_instance(), _text_create_text_instance(), _create_vg_text_instance() which would call lvmcache_label_scan() which would call _scan_invalid() which repeats the label scan on devices flagged INVALID. lvmcache_label_rescan_vg() is now called prominently by _vg_read() directly.
-rw-r--r--lib/cache/lvmcache.c19
-rw-r--r--lib/format_text/format-text.c16
-rw-r--r--lib/metadata/metadata.c62
-rw-r--r--tools/toollib.c27
-rw-r--r--tools/vgcfgrestore.c2
5 files changed, 71 insertions, 55 deletions
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 477361196..8253fd237 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -1438,8 +1438,6 @@ int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal,
struct vgnameid_list *vgnl;
struct lvmcache_vginfo *vginfo;
- lvmcache_label_scan(cmd);
-
dm_list_iterate_items(vginfo, &_vginfos) {
if (!include_internal && is_orphan_vg(vginfo->vgname))
continue;
@@ -1582,23 +1580,6 @@ struct device *lvmcache_device_from_pvid(struct cmd_context *cmd, const struct i
if (dev)
return dev;
- lvmcache_label_scan(cmd);
-
- /* Try again */
-
- dev = _device_from_pvid(pvid, label_sector);
- if (dev)
- return dev;
-
- lvmcache_force_next_label_scan();
- lvmcache_label_scan(cmd);
-
- /* Try again */
-
- dev = _device_from_pvid(pvid, label_sector);
- if (dev)
- return dev;
-
log_debug_devs("No device with uuid %s.", (const char *)pvid);
return NULL;
}
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 8f7883ef0..45460024a 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1995,22 +1995,6 @@ static int _create_vg_text_instance(struct format_instance *fid,
}
if (type & FMT_INSTANCE_MDAS) {
- /*
- * TODO in theory, this function should be never reached
- * while in critical_section(), because lvmcache's
- * cached_vg should be valid. However, this assumption
- * sometimes fails (possibly due to inconsistent
- * (precommit) metadata and/or missing devices), and
- * calling lvmcache_label_scan inside the critical
- * section may be fatal (i.e. deadlock).
- */
- if (!critical_section())
- /* Scan PVs in VG for any further MDAs */
- /*
- * FIXME Only scan PVs believed to be in the VG.
- */
- lvmcache_label_scan(fid->fmt->cmd);
-
if (!(vginfo = lvmcache_vginfo_from_vgname(vg_name, vg_id)))
goto_out;
if (!lvmcache_fid_add_mdas_vg(vginfo, fid))
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index bcc23bc1c..d3a547650 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3856,20 +3856,28 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
correct_vg = NULL;
}
+ /*
+ * Rescan the devices that are associated with this vg in lvmcache.
+ * This repeats what was done by the command's initial label scan,
+ * but only the devices associated with this VG.
+ *
+ * The lvmcache info about these devs is from the initial label scan
+ * performed by the command before the vg lock was held. Now the VG
+ * lock is held, so we rescan all the info from the devs in case
+ * something changed between the initial scan and now that the lock
+ * is held.
+ */
+ log_debug_metadata("Reading VG rereading labels for %s", vgname);
- /* Find the vgname in the cache */
- /* If it's not there we must do full scan to be completely sure */
- if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 1))) {
+ if (!lvmcache_label_rescan_vg(cmd, vgname, vgid)) {
+ /* The VG wasn't found, so force a full label scan. */
+ lvmcache_force_next_label_scan();
lvmcache_label_scan(cmd);
- if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 1))) {
- /* Independent MDAs aren't supported under low memory */
- if (!cmd->independent_metadata_areas && critical_section())
- return_NULL;
- lvmcache_force_next_label_scan();
- lvmcache_label_scan(cmd);
- if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0)))
- return_NULL;
- }
+ }
+
+ if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0))) {
+ log_debug_metadata("Cache did not find fmt for vgname %s", vgname);
+ return_NULL;
}
/* Now determine the correct vgname if none was supplied */
@@ -3887,6 +3895,36 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
if (use_precommitted && !(fmt->features & FMT_PRECOMMIT))
use_precommitted = 0;
+ /*
+ * A "format instance" is an abstraction for a VG location,
+ * i.e. where a VG's metadata exists on disk.
+ *
+ * An fic (format_instance_ctx) is a temporary struct used
+ * to create an fid (format_instance). The fid hangs around
+ * and is used to create a 'vg' to which it connected (vg->fid).
+ *
+ * The 'fic' describes a VG in terms of fmt/name/id.
+ *
+ * The 'fid' describes a VG in more detail than the fic,
+ * holding information about where to find the VG metadata.
+ *
+ * The 'vg' describes the VG in the most detail representing
+ * all the VG metadata.
+ *
+ * The fic and fid are set up by create_instance() to describe
+ * the VG location. This happens before the VG metadata is
+ * assembled into the more familiar struct volume_group "vg".
+ *
+ * The fid has one main purpose: to keep track of the metadata
+ * locations for a given VG. It does this by putting 'mda'
+ * structs on fid->metadata_areas_in_use, which specify where
+ * metadata is located on disk. It gets this information
+ * (metadata locations for a specific VG) from the command's
+ * initial label scan. The info is passed indirectly via
+ * lvmcache info/vginfo structs, which are created by the
+ * label scan and then copied into fid by create_instance().
+ */
+
/* create format instance with appropriate metadata area */
fic.type = FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
fic.context.vg_ref.vg_name = vgname;
diff --git a/tools/toollib.c b/tools/toollib.c
index 7eb0beda3..c16cc5a14 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -2216,14 +2216,10 @@ int process_each_vg(struct cmd_context *cmd,
}
/*
- * First rescan for available devices, then force the next
- * label scan to be done. get_vgnameids() will scan labels
- * (when not using lvmetad).
+ * Scan all devices to populate lvmcache with initial
+ * list of PVs and VGs.
*/
- if (cmd->cname->flags & REQUIRES_FULL_LABEL_SCAN) {
- dev_cache_full_scan(cmd->full_filter);
- lvmcache_force_next_label_scan();
- }
+ lvmcache_label_scan(cmd);
/*
* A list of all VGs on the system is needed when:
@@ -3728,6 +3724,12 @@ int process_each_lv(struct cmd_context *cmd,
}
/*
+ * Scan all devices to populate lvmcache with initial
+ * list of PVs and VGs.
+ */
+ lvmcache_label_scan(cmd);
+
+ /*
* A list of all VGs on the system is needed when:
* . processing all VGs on the system
* . A VG name is specified which may refer to one
@@ -4436,7 +4438,12 @@ int process_each_pv(struct cmd_context *cmd,
if (!trust_cache() && !orphans_locked) {
log_debug("Scanning for available devices");
lvmcache_destroy(cmd, 1, 0);
- dev_cache_full_scan(cmd->full_filter);
+
+ /*
+ * Scan all devices to populate lvmcache with initial
+ * list of PVs and VGs.
+ */
+ lvmcache_label_scan(cmd);
}
if (!get_vgnameids(cmd, &all_vgnameids, only_this_vgname, 1)) {
@@ -5450,6 +5457,8 @@ int pvcreate_each_device(struct cmd_context *cmd,
dev_cache_full_scan(cmd->full_filter);
+ lvmcache_label_scan(cmd);
+
/*
* Translate arg names into struct device's.
*/
@@ -5604,6 +5613,8 @@ int pvcreate_each_device(struct cmd_context *cmd,
goto out;
}
+ lvmcache_label_scan(cmd);
+
/*
* The device args began on the arg_devices list, then the first check
* loop moved those entries to arg_process as they were found. Devices
diff --git a/tools/vgcfgrestore.c b/tools/vgcfgrestore.c
index b5a2add12..e9f1a4c34 100644
--- a/tools/vgcfgrestore.c
+++ b/tools/vgcfgrestore.c
@@ -74,6 +74,8 @@ int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
return ECMD_FAILED;
}
+ lvmcache_label_scan(cmd);
+
cmd->handles_unknown_segments = 1;
if (!(arg_is_set(cmd, file_ARG) ?