master - liblvm2app: missed the addition of lvmcache_label_scan
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=c42a18d372f314aa51d...
Commit: c42a18d372f314aa51d9d7877342f1d78fd68973
Parent: aee27dc7bad5734012885fe9f174def0a3f26771
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Apr 20 12:00:49 2018 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 12:00:49 2018 -0500
liblvm2app: missed the addition of lvmcache_label_scan
---
liblvm/lvm_vg.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/liblvm/lvm_vg.c b/liblvm/lvm_vg.c
index 0678bdc..616c78f 100644
--- a/liblvm/lvm_vg.c
+++ b/liblvm/lvm_vg.c
@@ -219,6 +219,8 @@ static vg_t _lvm_vg_open(lvm_t libh, const char *vgname, const char *mode,
return NULL;
}
+ lvmcache_label_scan((struct cmd_context *)libh);
+
vg = vg_read((struct cmd_context *)libh, vgname, NULL, internal_flags, 0);
if (vg_read_error(vg)) {
/* FIXME: use log_errno either here in inside vg_read */
6 years
- 1
- 0
master - scan: skip device rescan in vg_read
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=aee27dc7bad57340128...
Commit: aee27dc7bad5734012885fe9f174def0a3f26771
Parent: 7b0a8f47be7df13aab0552599aa2dc2233cc223c
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Apr 18 16:29:42 2018 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:23:14 2018 -0500
scan: skip device rescan in vg_read
For reporting commands (pvs,vgs,lvs,pvdisplay,vgdisplay,lvdisplay)
we do not need to repeat the label scan of devices in vg_read if
they all had matching metadata in the initial label scan. The
data read by label scan can just be reused for the vg_read.
This cuts the amount of device i/o in half, from two reads of
each device to one. We have to be careful to avoid repairing
the VG if we've skipped rescanning. (The VG repair code is very
poor, and will be redone soon.)
---
lib/cache/lvmcache.c | 207 +++++++++++++++++++++++++++++++---------
lib/cache/lvmcache.h | 3 +
lib/commands/toolcontext.h | 2 +
lib/config/config.c | 6 +
lib/format_text/format-text.c | 19 ++++
lib/format_text/import_vsn1.c | 6 +
lib/format_text/text_label.c | 7 ++
lib/metadata/metadata.c | 88 +++++++++++++++---
test/shell/mda-rollback.sh | 3 +
tools/command.c | 1 +
tools/commands.h | 12 +-
tools/lvmcmdline.c | 3 +
tools/toollib.c | 2 +-
tools/tools.h | 4 +
14 files changed, 295 insertions(+), 68 deletions(-)
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index c8046f7..f1fd683 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -62,7 +62,9 @@ struct lvmcache_vginfo {
char *lock_type;
uint32_t mda_checksum;
size_t mda_size;
+ int seqno;
int independent_metadata_location; /* metadata read from independent areas */
+ int scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */
/*
* The following are not related to lvmcache or vginfo,
@@ -1057,25 +1059,34 @@ next:
* the labels/metadata for each device in the VG now that we hold the
* lock, and use this for processing the VG.
*
- * FIXME: In some cases, the data read by label_scan may be fine, and not
- * need to be reread here. e.g. a reporting command, possibly with a
- * special option, could skip this second reread. Or, we could look
- * at the VG seqno in each copy of the metadata read in the first label
- * scan, and if they all match, consider it good enough to use for
- * reporting without rereading it. (A command modifying the VG would
- * always want to reread while the lock is held before modifying.)
- *
* A label scan is ultimately creating associations between devices
* and VGs so that when vg_read wants to get VG metadata, it knows
- * which devices to read. In the special case where VG metadata is
- * stored in files on the file system (configured in lvm.conf), the
+ * which devices to read.
+ *
+ * It's possible that a VG is being modified during the first label
+ * scan, causing the scan to see inconsistent metadata on different
+ * devs in the VG. It's possible that those modifications are
+ * adding/removing devs from the VG, in which case the device/VG
+ * associations in lvmcache after the scan are not correct.
+ * NB. It's even possible the VG was removed completely between
+ * label scan and here, in which case we'd not find the VG in
+ * lvmcache after this rescan.
+ *
+ * A scan will also create in incorrect/incomplete picture of a VG
+ * when devices have no metadata areas. The scan does not use
+ * VG metadata to figure out that a dev with no metadata belongs
+ * to a particular VG, so a device with no mdas will not be linked
+ * to that VG after a scan.
+ *
+ * (In the special case where VG metadata is stored in files on the
+ * file system (configured in lvm.conf), the
* vginfo->independent_metadata_location flag is set during label scan.
* When we get here to rescan, we are revalidating the device to VG
* mapping from label scan by repeating the label scan on a subset of
* devices. If we see independent_metadata_location is set from the
* initial label scan, we know that there is nothing to do because
* there is no device to VG mapping to revalidate, since the VG metadata
- * comes directly from files.
+ * comes directly from files.)
*/
int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid)
@@ -1083,7 +1094,7 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const
struct dm_list devs;
struct device_list *devl;
struct lvmcache_vginfo *vginfo;
- struct lvmcache_info *info;
+ struct lvmcache_info *info, *info2;
if (lvmetad_used())
return 1;
@@ -1112,14 +1123,17 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const
dm_list_add(&devs, &devl->list);
}
- label_scan_devs(cmd, &devs);
+ dm_list_iterate_items_safe(info, info2, &vginfo->infos)
+ lvmcache_del(info);
- /*
- * TODO: grab vginfo again, and compare vginfo->infos
- * to what was found above before rereading labels.
- * If there are any info->devs now that were not in the
- * first devs list, then do label_read on those also.
- */
+ /* Dropping the last info struct is supposed to drop vginfo. */
+ if ((vginfo = lvmcache_vginfo_from_vgname(vgname, vgid)))
+ log_warn("VG info not dropped before rescan of %s", vgname);
+
+ /* FIXME: should we also rescan unused_duplicate_devs for devs
+ being rescanned here and then repeat resolving the duplicates? */
+
+ label_scan_devs(cmd, &devs);
return 1;
}
@@ -1803,28 +1817,6 @@ out:
return 1;
}
-static int _lvmcache_update_vg_mda_info(struct lvmcache_info *info, uint32_t mda_checksum,
- size_t mda_size)
-{
- if (!info || !info->vginfo || !mda_size)
- return 1;
-
- if (info->vginfo->mda_checksum == mda_checksum || info->vginfo->mda_size == mda_size)
- return 1;
-
- info->vginfo->mda_checksum = mda_checksum;
- info->vginfo->mda_size = mda_size;
-
- /* FIXME Add checksum index */
-
- log_debug_cache("lvmcache %s: VG %s: stored metadata checksum 0x%08"
- PRIx32 " with size %" PRIsize_t ".",
- dev_name(info->dev), info->vginfo->vgname,
- mda_checksum, mda_size);
-
- return 1;
-}
-
int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt)
{
if (!_lock_hash && !lvmcache_init()) {
@@ -1835,10 +1827,18 @@ int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt)
return _lvmcache_update_vgname(NULL, vgname, vgname, 0, "", 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.
+ */
+
int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vgsummary *vgsummary)
{
const char *vgname = vgsummary->vgname;
const char *vgid = (char *)&vgsummary->vgid;
+ struct lvmcache_vginfo *vginfo;
if (!vgname && !info->vginfo) {
log_error(INTERNAL_ERROR "NULL vgname handed to cache");
@@ -1853,12 +1853,80 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
!is_orphan_vg(info->vginfo->vgname) && critical_section())
return 1;
- if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus,
- vgsummary->creation_host, info->fmt) ||
- !_lvmcache_update_vgid(info, info->vginfo, vgid) ||
- !_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host, vgsummary->lock_type, vgsummary->system_id) ||
- !_lvmcache_update_vg_mda_info(info, vgsummary->mda_checksum, vgsummary->mda_size))
- return_0;
+ /*
+ * Creates a new vginfo struct for this vgname/vgid if none exists,
+ * and attaches the info struct for the dev to the vginfo.
+ * Puts the vginfo into the vgname hash table.
+ */
+ if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus, vgsummary->creation_host, info->fmt)) {
+ log_error("Failed to update VG %s info in lvmcache.", vgname);
+ return 0;
+ }
+
+ /*
+ * Puts the vginfo into the vgid hash table.
+ */
+ if (!_lvmcache_update_vgid(info, info->vginfo, vgid)) {
+ log_error("Failed to update VG %s info in lvmcache.", vgname);
+ return 0;
+ }
+
+ /*
+ * FIXME: identify which case this is and why this is needed, then
+ * change that so it doesn't use this function and we can remove
+ * this special case.
+ * (I think this distinguishes the scan path, where these things
+ * are set from the vg_read path where lvmcache_update_vg() is
+ * called which calls this function without seqno/mda_size/mda_checksum.)
+ */
+ if (!vgsummary->seqno && !vgsummary->mda_size && !vgsummary->mda_checksum)
+ return 1;
+
+ 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);
+
+ } else if ((vginfo->mda_size != vgsummary->mda_size) || (vginfo->mda_checksum != vgsummary->mda_checksum)) {
+ log_warn("Scan of VG %s from %s found mda_checksum %x mda_size %zu vs previous %x %zu",
+ vgname, dev_name(info->dev), vgsummary->mda_checksum, vgsummary->mda_size,
+ vginfo->mda_checksum, vginfo->mda_size);
+ vginfo->scan_summary_mismatch = 1;
+ /* If we don't return success, this dev info will be removed from lvmcache,
+ and then we won't be able to rescan it or repair it. */
+ return 1;
+ }
+
+ /*
+ * If a dev has an unmatching checksum, ignore the other
+ * info from it, keeping the info we already saved.
+ */
+ if (!_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host,
+ vgsummary->lock_type, vgsummary->system_id)) {
+ log_error("Failed to update VG %s info in lvmcache.", vgname);
+ return 0;
+ }
return 1;
}
@@ -2532,6 +2600,7 @@ int lvmcache_lookup_mda(struct lvmcache_vgsummary *vgsummary)
vgsummary->vgname = vginfo->vgname;
vgsummary->creation_host = vginfo->creation_host;
vgsummary->vgstatus = vginfo->status;
+ vgsummary->seqno = vginfo->seqno;
/* vginfo->vgid has 1 extra byte then vgsummary->vgid */
memcpy(&vgsummary->vgid, vginfo->vgid, sizeof(vgsummary->vgid));
@@ -2592,3 +2661,47 @@ int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const ch
return ret;
}
+/*
+ * Example of reading four devs in sequence from the same VG:
+ *
+ * dev1:
+ * lvmcache: creates vginfo with initial values
+ *
+ * dev2: all checksums match.
+ * mda_header checksum matches vginfo from dev1
+ * metadata checksum matches vginfo from dev1
+ * metadata is not parsed, and the vgsummary values copied
+ * from lvmcache from dev1 and passed back to lvmcache for dev2.
+ * lvmcache: attach info for dev2 to existing vginfo
+ *
+ * dev3: mda_header and metadata have unmatching checksums.
+ * mda_header checksum matches vginfo from dev1
+ * metadata checksum doesn't match vginfo from dev1
+ * produces read error in config.c
+ * lvmcache: info for dev3 is deleted, FIXME: use a defective state
+ *
+ * dev4: mda_header and metadata have matching checksums, but
+ * does not match checksum in lvmcache from prev dev.
+ * mda_header checksum doesn't match vginfo from dev1
+ * lvmcache_lookup_mda returns 0, no vgname, no checksum_only
+ * lvmcache: update_vgname_and_id sees checksum from dev4 does not
+ * match vginfo from dev1, so vginfo->scan_summary_mismatch is set.
+ * attach info for dev4 to existing vginfo
+ *
+ * dev5: config parsing error.
+ * 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)
+{
+ struct lvmcache_vginfo *vginfo;
+
+ if (!vgname || !vgid)
+ return 1;
+
+ if ((vginfo = lvmcache_vginfo_from_vgid(vgid)))
+ return vginfo->scan_summary_mismatch;
+
+ return 1;
+}
+
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 107c993..ad478bd 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -60,6 +60,7 @@ struct lvmcache_vgsummary {
uint32_t mda_checksum;
size_t mda_size;
int zero_offset;
+ int seqno;
};
int lvmcache_init(void);
@@ -216,4 +217,6 @@ void lvmcache_save_suspended_vg(struct volume_group *vg, int precommitted);
struct volume_group *lvmcache_get_suspended_vg(const char *vgid);
void lvmcache_drop_suspended_vg(struct volume_group *vg);
+int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid);
+
#endif
diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h
index d20cef1..2474264 100644
--- a/lib/commands/toolcontext.h
+++ b/lib/commands/toolcontext.h
@@ -169,6 +169,8 @@ struct cmd_context {
unsigned process_component_lvs:1; /* command processes also component LVs */
unsigned mirror_warn_printed:1; /* command already printed warning about non-monitored mirrors */
unsigned pvscan_cache_single:1;
+ unsigned can_use_one_scan:1;
+
/*
* Filtering.
*/
diff --git a/lib/config/config.c b/lib/config/config.c
index d07c173..ad816c2 100644
--- a/lib/config/config.c
+++ b/lib/config/config.c
@@ -545,6 +545,12 @@ int config_file_read_fd(struct dm_config_tree *cft, struct device *dev, dev_io_r
fb = buf;
}
+ /*
+ * The checksum passed in is the checksum from the mda_header
+ * preceding this metadata. They should always match.
+ * FIXME: handle case where mda_header checksum is bad,
+ * but the checksum calculated here is correct.
+ */
if (checksum_fn && checksum !=
(checksum_fn(checksum_fn(INITIAL_CRC, (const uint8_t *)fb, size),
(const uint8_t *)(fb + size), size2))) {
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 4146e7c..792d75a 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1261,8 +1261,27 @@ int read_metadata_location_summary(const struct format_type *fmt,
* which also matches the checksum saved in vginfo from
* another device, then it skips parsing the metadata into
* a config tree, which saves considerable cpu time.
+ *
+ * (NB. there can be different VGs with different metadata
+ * and checksums, but with the same name.)
+ *
+ * FIXME: handle the case where mda_header checksum is bad
+ * but metadata checksum is good.
*/
+ /*
+ * If the checksum we compute of the metadata differs from
+ * the checksum from mda_header that we save here, then we
+ * ignore the device. FIXME: we need to classify a device
+ * with errors like this as defective.
+ *
+ * If the checksum from mda_header and computed from metadata
+ * does not match the checksum saved in lvmcache from a prev
+ * device, then we do not skip parsing/saving metadata from
+ * this dev. It's parsed, fields saved in vgsummary, which
+ * is passed into lvmcache (update_vgname_and_id), and
+ * there we'll see a checksum mismatch.
+ */
vgsummary->mda_checksum = rlocn->checksum;
vgsummary->mda_size = rlocn->size;
lvmcache_lookup_mda(vgsummary);
diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index dee5379..e038a27 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -1292,6 +1292,12 @@ static int _read_vgsummary(const struct format_type *fmt, const struct dm_config
(!(vgsummary->lock_type = dm_pool_strdup(mem, str))))
return_0;
+ if (!_read_int32(vgn, "seqno", &vgsummary->seqno)) {
+ log_error("Couldn't read seqno for volume group %s.",
+ vgsummary->vgname);
+ return 0;
+ }
+
return 1;
}
diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c
index c47a35a..7d10e06 100644
--- a/lib/format_text/text_label.c
+++ b/lib/format_text/text_label.c
@@ -438,6 +438,13 @@ 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;
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 8c8ce25..685c589 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3761,6 +3761,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
struct pv_list *pvl;
struct dm_list all_pvs;
char uuid[64] __attribute__((aligned(8)));
+ int skipped_rescan = 0;
int reappeared = 0;
struct cached_vg_fmtdata *vg_fmtdata = NULL; /* Additional format-specific data about the vg */
@@ -3834,10 +3835,42 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
* lock is held, so we rescan all the info from the devs in case
* something changed between the initial scan and now that the lock
* is held.
+ *
+ * Some commands (e.g. reporting) are fine reporting data read by
+ * the label scan. It doesn't matter if the devs changed between
+ * the label scan and here, we can report what was seen in the
+ * scan, even though it is the old state, since we will not be
+ * making any modifications. If the VG was being modified during
+ * the scan, and caused us to see inconsistent metadata on the
+ * different PVs in the VG, then we do want to rescan the devs
+ * here to get a consistent view of the VG. Note that we don't
+ * know if the scan found all the PVs in the VG at this point.
+ * We don't know that until vg_read looks at the list of PVs in
+ * the metadata and compares that to the devices found by the scan.
+ *
+ * It's possible that a change made to the VG during scan was
+ * adding or removing a PV from the VG. In this case, the list
+ * of devices associated with the VG in lvmcache would change
+ * due to the rescan.
+ *
+ * The devs in the VG may be persistently inconsistent due to some
+ * previous problem. In this case, rescanning the labels here will
+ * find the same inconsistency. The VG repair (mistakenly done by
+ * vg_read below) is supposed to fix that.
+ *
+ * FIXME: sort out the usage of the global lock (which is mixed up
+ * with the orphan lock), and when we can tell that the global
+ * lock is taken prior to the label scan, and still held here,
+ * we can also skip the rescan in that case.
*/
- log_debug_metadata("Reading VG rereading labels for %s", vgname);
-
- lvmcache_label_rescan_vg(cmd, vgname, vgid);
+ if (!cmd->can_use_one_scan || lvmcache_scan_mismatch(cmd, vgname, vgid)) {
+ skipped_rescan = 0;
+ log_debug_metadata("Rescanning devices for for %s", vgname);
+ lvmcache_label_rescan_vg(cmd, vgname, vgid);
+ } else {
+ log_debug_metadata("Skipped rescanning devices for %s", vgname);
+ skipped_rescan = 1;
+ }
if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0))) {
log_debug_metadata("Cache did not find fmt for vgname %s", vgname);
@@ -3940,10 +3973,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
/* FIXME Also ensure contents same - checksum compare? */
if (correct_vg->seqno != vg->seqno) {
- if (cmd->metadata_read_only)
- log_very_verbose("Not repairing VG %s metadata seqno (%d != %d) "
- "as global/metadata_read_only is set.",
- vgname, vg->seqno, correct_vg->seqno);
+ if (cmd->metadata_read_only || skipped_rescan)
+ log_warn("Not repairing metadata for VG %s.", vgname);
else
inconsistent = 1;
@@ -4004,7 +4035,29 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
return_NULL;
}
- log_debug_metadata("Empty mda found for VG %s.", vgname);
+ log_debug_metadata("Empty mda found for VG %s on %s.",
+ vgname, dev_name(pvl->pv->dev));
+
+#if 0
+ /*
+ * If we are going to do any repair we have to be using
+ * the latest metadata on disk, so we have to rescan devs
+ * if we skipped that at the start of the vg_read. We'll
+ * likely come back through here, but without having
+ * skipped_rescan.
+ *
+ * FIXME: in some cases we don't want to do this.
+ */
+ if (skipped_rescan && cmd->can_use_one_scan) {
+ log_debug_metadata("Restarting read to rescan devs.");
+ cmd->can_use_one_scan = 0;
+ release_vg(correct_vg);
+ correct_vg = NULL;
+ lvmcache_del(info);
+ label_read(pvl->pv->dev, NULL, 0);
+ goto restart_scan;
+ }
+#endif
if (inconsistent_mdas)
continue;
@@ -4142,10 +4195,8 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
/* FIXME Also ensure contents same - checksums same? */
if (correct_vg->seqno != vg->seqno) {
/* Ignore inconsistent seqno if told to skip repair logic */
- if (cmd->metadata_read_only)
- log_very_verbose("Not repairing VG %s metadata seqno (%d != %d) "
- "as global/metadata_read_only is set.",
- vgname, vg->seqno, correct_vg->seqno);
+ if (cmd->metadata_read_only || skipped_rescan)
+ log_warn("Not repairing metadata for VG %s.", vgname);
else
inconsistent = 1;
@@ -4225,6 +4276,13 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
return correct_vg;
}
+ if (skipped_rescan) {
+ log_warn("Not repairing metadata for VG %s.", vgname);
+ _free_pv_list(&all_pvs);
+ release_vg(correct_vg);
+ return_NULL;
+ }
+
/* Don't touch if vgids didn't match */
if (inconsistent_vgid) {
log_warn("WARNING: Inconsistent metadata UUIDs found for "
@@ -4271,14 +4329,16 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
}
/* We have the VG now finally, check if PV ext info is in sync with VG metadata. */
- if (!_check_or_repair_pv_ext(cmd, correct_vg, *consistent, &inconsistent_pvs)) {
+ if (!_check_or_repair_pv_ext(cmd, correct_vg,
+ skipped_rescan ? 0 : *consistent,
+ &inconsistent_pvs)) {
release_vg(correct_vg);
return_NULL;
}
*consistent = !inconsistent_pvs;
- if (correct_vg && *consistent) {
+ if (correct_vg && *consistent && !skipped_rescan) {
if (update_old_pv_ext && !_vg_update_old_pv_ext_if_needed(correct_vg)) {
release_vg(correct_vg);
return_NULL;
diff --git a/test/shell/mda-rollback.sh b/test/shell/mda-rollback.sh
index dbfdc7d..34080fa 100644
--- a/test/shell/mda-rollback.sh
+++ b/test/shell/mda-rollback.sh
@@ -25,6 +25,9 @@ vgextend $vg1 "$dev1"
dd if=badmda of="$dev1" bs=256K count=1
+# the vg_read in vgck (and other commands) will repair the metadata
+vgck $vg1
+
# dev1 is part of vg1 (as witnessed by metadata on dev2 and dev3), but its mda
# was corrupt (written over by a backup from time dev1 was an orphan)
check pv_field "$dev1" vg_name $vg1
diff --git a/tools/command.c b/tools/command.c
index f3b5d82..377d03f 100644
--- a/tools/command.c
+++ b/tools/command.c
@@ -140,6 +140,7 @@ static inline int configtype_arg(struct cmd_context *cmd __attribute__((unused))
#define ENABLE_DUPLICATE_DEVS 0x00000400
#define DISALLOW_TAG_ARGS 0x00000800
#define GET_VGNAME_FROM_OPTIONS 0x00001000
+#define CAN_USE_ONE_SCAN 0x00002000
/* create foo_CMD enums for command def ID's in command-lines.in */
diff --git a/tools/commands.h b/tools/commands.h
index 3d142c3..4af92c8 100644
--- a/tools/commands.h
+++ b/tools/commands.h
@@ -55,7 +55,7 @@ xx(lvcreate,
xx(lvdisplay,
"Display information about a logical volume",
- PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH)
+ PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH | CAN_USE_ONE_SCAN)
xx(lvextend,
"Add space to a logical volume",
@@ -99,7 +99,7 @@ xx(lvresize,
xx(lvs,
"Display information about logical volumes",
- PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH)
+ PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH | CAN_USE_ONE_SCAN)
xx(lvscan,
"List all logical volumes in all volume groups",
@@ -127,7 +127,7 @@ xx(pvdata,
xx(pvdisplay,
"Display various attributes of physical volume(s)",
- PERMITTED_READ_ONLY | ENABLE_ALL_DEVS | ENABLE_DUPLICATE_DEVS | LOCKD_VG_SH)
+ PERMITTED_READ_ONLY | ENABLE_ALL_DEVS | ENABLE_DUPLICATE_DEVS | LOCKD_VG_SH | CAN_USE_ONE_SCAN)
/* ALL_VGS_IS_DEFAULT is for polldaemon to find pvmoves in-progress using process_each_vg. */
@@ -145,7 +145,7 @@ xx(pvremove,
xx(pvs,
"Display information about physical volumes",
- PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_ALL_DEVS | ENABLE_DUPLICATE_DEVS | LOCKD_VG_SH)
+ PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | ENABLE_ALL_DEVS | ENABLE_DUPLICATE_DEVS | LOCKD_VG_SH | CAN_USE_ONE_SCAN)
xx(pvscan,
"List all physical volumes",
@@ -189,7 +189,7 @@ xx(vgcreate,
xx(vgdisplay,
"Display volume group information",
- PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH)
+ PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH | CAN_USE_ONE_SCAN)
xx(vgexport,
"Unregister volume group(s) from the system",
@@ -229,7 +229,7 @@ xx(vgrename,
xx(vgs,
"Display information about volume groups",
- PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH)
+ PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT | LOCKD_VG_SH | CAN_USE_ONE_SCAN)
xx(vgscan,
"Search for all volume groups",
diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c
index c7ac4b6..0600b1c 100644
--- a/tools/lvmcmdline.c
+++ b/tools/lvmcmdline.c
@@ -2291,6 +2291,9 @@ static int _get_current_settings(struct cmd_context *cmd)
if (cmd->cname->flags & LOCKD_VG_SH)
cmd->lockd_vg_default_sh = 1;
+ if (cmd->cname->flags & CAN_USE_ONE_SCAN)
+ cmd->can_use_one_scan = 1;
+
cmd->partial_activation = 0;
cmd->degraded_activation = 0;
activation_mode = find_config_tree_str(cmd, activation_mode_CFG, NULL);
diff --git a/tools/toollib.c b/tools/toollib.c
index e887f65..6b71f2d 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -2032,7 +2032,7 @@ static int _process_vgnameid_list(struct cmd_context *cmd, uint32_t read_flags,
(!dm_list_empty(arg_tags) && str_list_match_list(arg_tags, &vg->tags, NULL))) &&
select_match_vg(cmd, handle, vg) && _select_matches(handle)) {
- log_very_verbose("Processing VG %s %s", vg_name, vg_uuid ? uuid : "");
+ log_very_verbose("Running command for VG %s %s", vg_name, vg_uuid ? uuid : "");
ret = process_single_vg(cmd, vg_name, vg, handle);
_update_selection_result(handle, &whole_selected);
diff --git a/tools/tools.h b/tools/tools.h
index d4d2fb2..5fe3ba8 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -136,6 +136,10 @@ struct arg_value_group_list {
#define DISALLOW_TAG_ARGS 0x00000800
/* Command may need to find VG name in an option value. */
#define GET_VGNAME_FROM_OPTIONS 0x00001000
+/* The data read from disk by label scan can be used for vg_read. */
+#define CAN_USE_ONE_SCAN 0x00002000
+
+
void usage(const char *name);
/* the argument verify/normalise functions */
6 years
- 1
- 0
master - label_scan: remove extra label scan and read for orphan PVs
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=79c4971210a6337563f...
Commit: 79c4971210a6337563ffa2fca08fb636423d93d4
Parent: 5f138f36040297d092977a3b547cdefffb5ac4e8
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Nov 6 12:09:52 2017 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:22:45 2018 -0500
label_scan: remove extra label scan and read for orphan PVs
When process_each_pv() calls vg_read() on the orphan VG, the
internal implementation was doing an unnecessary
lvmcache_label_scan() and two unnecessary label_read() calls
on each orphan. Some of those unnecessary label scans/reads
would sometimes be skipped due to caching, but the code was
always doing at least one unnecessary read on each orphan.
The common format_text case was also unecessarily calling into
the format-specific pv_read() function which actually did nothing.
By analyzing each case in which vg_read() was being called on
the orphan VG, we can say that all of the label scans/reads
in vg_read_orphans are unnecessary:
1. reporting commands: the information saved in lvmcache by
the original label scan can be reported. There is no advantage
to repeating the label scan on the orphans a second time before
reporting it.
2. pvcreate/vgcreate/vgextend: these all share a common
implementation in pvcreate_each_device(). That function
already rescans labels after acquiring the orphan VG lock,
which ensures that the command is using valid lvmcache
information.
---
lib/cache/lvmcache.c | 51 +++-----------
lib/cache/lvmcache.h | 4 +-
lib/format_text/format-text.c | 31 ---------
lib/metadata/metadata.c | 150 +++++++++++------------------------------
lib/metadata/metadata.h | 6 --
5 files changed, 54 insertions(+), 188 deletions(-)
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 47058cc..8e119b5 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -2454,56 +2454,29 @@ int lvmcache_fid_add_mdas_vg(struct lvmcache_vginfo *vginfo, struct format_insta
return 1;
}
-static int _get_pv_if_in_vg(struct lvmcache_info *info,
- struct physical_volume *pv)
-{
- char vgname[NAME_LEN + 1];
- char vgid[ID_LEN + 1];
-
- if (info->vginfo && info->vginfo->vgname &&
- !is_orphan_vg(info->vginfo->vgname)) {
- /*
- * get_pv_from_vg_by_id() may call
- * lvmcache_label_scan() and drop cached
- * vginfo so make a local copy of string.
- */
- (void) dm_strncpy(vgname, info->vginfo->vgname, sizeof(vgname));
- memcpy(vgid, info->vginfo->vgid, sizeof(vgid));
-
- if (get_pv_from_vg_by_id(info->fmt, vgname, vgid,
- info->dev->pvid, pv))
- return 1;
- }
-
- return 0;
-}
-
int lvmcache_populate_pv_fields(struct lvmcache_info *info,
- struct physical_volume *pv,
- int scan_label_only)
+ struct volume_group *vg,
+ struct physical_volume *pv)
{
struct data_area_list *da;
-
- /* Have we already cached vgname? */
- if (!scan_label_only && _get_pv_if_in_vg(info, pv))
- return 1;
-
- /* Perform full scan (just the first time) and try again */
- if (!scan_label_only && !critical_section() && !full_scan_done()) {
- lvmcache_force_next_label_scan();
- lvmcache_label_scan(info->fmt->cmd);
-
- if (_get_pv_if_in_vg(info, pv))
- return 1;
+
+ if (!info->label) {
+ log_error("No cached label for orphan PV %s", pv_dev_name(pv));
+ return 0;
}
- /* Orphan */
+ pv->label_sector = info->label->sector;
pv->dev = info->dev;
pv->fmt = info->fmt;
pv->size = info->device_size >> SECTOR_SHIFT;
pv->vg_name = FMT_TEXT_ORPHAN_VG_NAME;
memcpy(&pv->id, &info->dev->pvid, sizeof(pv->id));
+ if (!pv->size) {
+ log_error("PV %s size is zero.", dev_name(info->dev));
+ return 0;
+ }
+
/* Currently only support exactly one data area */
if (dm_list_size(&info->das) != 1) {
log_error("Must be exactly one data area (found %d) on PV %s",
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 826e91e..1b5379c 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -145,8 +145,8 @@ int lvmcache_fid_add_mdas(struct lvmcache_info *info, struct format_instance *fi
int lvmcache_fid_add_mdas_pv(struct lvmcache_info *info, struct format_instance *fid);
int lvmcache_fid_add_mdas_vg(struct lvmcache_vginfo *vginfo, struct format_instance *fid);
int lvmcache_populate_pv_fields(struct lvmcache_info *info,
- struct physical_volume *pv,
- int scan_label_only);
+ struct volume_group *vg,
+ struct physical_volume *pv);
int lvmcache_check_format(struct lvmcache_info *info, const struct format_type *fmt);
void lvmcache_del_mdas(struct lvmcache_info *info);
void lvmcache_del_das(struct lvmcache_info *info);
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 9538080..ee1f11d 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1593,36 +1593,6 @@ static uint64_t _metadata_locn_offset_raw(void *metadata_locn)
return mdac->area.start;
}
-static int _text_pv_read(const struct format_type *fmt, const char *pv_name,
- struct physical_volume *pv, int scan_label_only)
-{
- struct lvmcache_info *info;
- struct device *dev;
-
- if (!(dev = dev_cache_get(pv_name, fmt->cmd->filter)))
- return_0;
-
- if (lvmetad_used()) {
- info = lvmcache_info_from_pvid(dev->pvid, dev, 0);
- if (!info && !lvmetad_pv_lookup_by_dev(fmt->cmd, dev, NULL))
- return 0;
- info = lvmcache_info_from_pvid(dev->pvid, dev, 0);
- } else {
- struct label *label;
- if (!(label_read(dev, &label, UINT64_C(0))))
- return_0;
- info = label->info;
- }
-
- if (!info)
- return_0;
-
- if (!lvmcache_populate_pv_fields(info, pv, scan_label_only))
- return 0;
-
- return 1;
-}
-
static int _text_pv_initialise(const struct format_type *fmt,
struct pv_create_args *pva,
struct physical_volume *pv)
@@ -2471,7 +2441,6 @@ static struct format_instance *_text_create_text_instance(const struct format_ty
static struct format_handler _text_handler = {
.scan = _text_scan,
- .pv_read = _text_pv_read,
.pv_initialise = _text_pv_initialise,
.pv_setup = _text_pv_setup,
.pv_add_metadata_area = _text_pv_add_metadata_area,
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index b4ee204..570cbe6 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -38,10 +38,9 @@
#include <sys/param.h>
static struct physical_volume *_pv_read(struct cmd_context *cmd,
- struct dm_pool *pvmem,
- const char *pv_name,
- struct format_instance *fid,
- uint32_t warn_flags, int scan_label_only);
+ const struct format_type *fmt,
+ struct volume_group *vg,
+ struct lvmcache_info *info);
static int _alignment_overrides_default(unsigned long data_alignment,
unsigned long default_pe_align)
@@ -330,37 +329,6 @@ bad:
return NULL;
}
-int get_pv_from_vg_by_id(const struct format_type *fmt, const char *vg_name,
- const char *vgid, const char *pvid,
- struct physical_volume *pv)
-{
- struct volume_group *vg;
- struct pv_list *pvl;
- uint32_t warn_flags = WARN_PV_READ | WARN_INCONSISTENT;
- int r = 0, consistent = 0;
-
- if (!(vg = vg_read_internal(fmt->cmd, vg_name, vgid, warn_flags, &consistent))) {
- log_error("get_pv_from_vg_by_id: vg_read_internal failed to read VG %s",
- vg_name);
- return 0;
- }
-
- dm_list_iterate_items(pvl, &vg->pvs) {
- if (id_equal(&pvl->pv->id, (const struct id *) pvid)) {
- if (!_copy_pv(fmt->cmd->mem, pv, pvl->pv)) {
- log_error("internal PV duplication failed");
- r = 0;
- goto out;
- }
- r = 1;
- goto out;
- }
- }
-out:
- release_vg(vg);
- return r;
-}
-
static int _move_pv(struct volume_group *vg_from, struct volume_group *vg_to,
const char *pv_name, int enforce_pv_from_source)
{
@@ -3246,9 +3214,7 @@ static int _check_mda_in_use(struct metadata_area *mda, void *_in_use)
struct _vg_read_orphan_baton {
struct cmd_context *cmd;
struct volume_group *vg;
- uint32_t warn_flags;
- int consistent;
- int repair;
+ const struct format_type *fmt;
};
/*
@@ -3345,8 +3311,7 @@ static int _vg_read_orphan_pv(struct lvmcache_info *info, void *baton)
uint32_t ext_version;
uint32_t ext_flags;
- if (!(pv = _pv_read(b->vg->cmd, b->vg->vgmem, dev_name(lvmcache_device(info)),
- b->vg->fid, b->warn_flags, 0))) {
+ if (!(pv = _pv_read(b->cmd, b->fmt, b->vg, info))) {
stack;
return 1;
}
@@ -3453,10 +3418,22 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
vg->free_count = 0;
baton.cmd = cmd;
- baton.warn_flags = warn_flags;
+ baton.fmt = fmt;
baton.vg = vg;
- baton.consistent = 1;
- baton.repair = *consistent;
+
+ /*
+ * vg_read for a normal VG will rescan labels for all the devices
+ * in the VG, in case something changed on disk between the initial
+ * label scan and acquiring the VG lock. We don't rescan labels
+ * here because this is only called in two ways:
+ *
+ * 1. for reporting, in which case it doesn't matter if something
+ * changed between the label scan and printing the PVs here
+ *
+ * 2. pvcreate_each_device() for pvcreate//vgcreate/vgextend,
+ * which already does the label rescan after taking the
+ * orphan lock.
+ */
while ((pvl = (struct pv_list *) dm_list_first(&head.list))) {
dm_list_del(&pvl->list);
@@ -3468,7 +3445,6 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
if (!lvmcache_foreach_pv(vginfo, _vg_read_orphan_pv, &baton))
return_NULL;
- *consistent = baton.consistent;
return vg;
}
@@ -4686,86 +4662,40 @@ const char *find_vgname_from_pvname(struct cmd_context *cmd,
return find_vgname_from_pvid(cmd, pvid);
}
-/* FIXME Use label functions instead of PV functions */
static struct physical_volume *_pv_read(struct cmd_context *cmd,
- struct dm_pool *pvmem,
- const char *pv_name,
- struct format_instance *fid,
- uint32_t warn_flags, int scan_label_only)
+ const struct format_type *fmt,
+ struct volume_group *vg,
+ struct lvmcache_info *info)
{
struct physical_volume *pv;
- struct label *label;
- struct lvmcache_info *info;
- struct device *dev;
- const struct format_type *fmt;
- int found;
-
- if (!(dev = dev_cache_get(pv_name, cmd->filter)))
- return_NULL;
-
- if (lvmetad_used()) {
- info = lvmcache_info_from_pvid(dev->pvid, dev, 0);
- if (!info) {
- if (!lvmetad_pv_lookup_by_dev(cmd, dev, &found))
- return_NULL;
- if (!found) {
- if (warn_flags & WARN_PV_READ)
- log_error("No physical volume found in lvmetad cache for %s",
- pv_name);
- return NULL;
- }
- if (!(info = lvmcache_info_from_pvid(dev->pvid, dev, 0))) {
- if (warn_flags & WARN_PV_READ)
- log_error("No cache info in lvmetad cache for %s.",
- pv_name);
- return NULL;
- }
- }
- label = lvmcache_get_label(info);
- } else {
- if (!(label_read(dev, &label, UINT64_C(0)))) {
- if (warn_flags & WARN_PV_READ)
- log_error("No physical volume label read from %s",
- pv_name);
- return NULL;
- }
- info = (struct lvmcache_info *) label->info;
- }
+ struct device *dev = lvmcache_device(info);
- fmt = lvmcache_fmt(info);
-
- pv = _alloc_pv(pvmem, dev);
- if (!pv) {
- log_error("pv allocation for '%s' failed", pv_name);
+ if (!(pv = _alloc_pv(vg->vgmem, NULL))) {
+ log_error("pv allocation failed");
return NULL;
}
- pv->label_sector = label->sector;
-
- /* FIXME Move more common code up here */
- if (!(lvmcache_fmt(info)->ops->pv_read(lvmcache_fmt(info), pv_name, pv, scan_label_only))) {
- log_error("Failed to read existing physical volume '%s'",
- pv_name);
- goto bad;
+ if (fmt->ops->pv_read) {
+ /* format1 and pool */
+ if (!(fmt->ops->pv_read(fmt, dev_name(dev), pv, 0))) {
+ log_error("Failed to read existing physical volume '%s'", dev_name(dev));
+ goto bad;
+ }
+ } else {
+ /* format text */
+ if (!lvmcache_populate_pv_fields(info, vg, pv))
+ goto_bad;
}
- if (!pv->size)
- goto bad;
-
- if (!alloc_pv_segment_whole_pv(pvmem, pv))
+ if (!alloc_pv_segment_whole_pv(vg->vgmem, pv))
goto_bad;
- if (fid)
- lvmcache_fid_add_mdas(info, fid, (const char *) &pv->id, ID_LEN);
- else {
- lvmcache_fid_add_mdas(info, fmt->orphan_vg->fid, (const char *) &pv->id, ID_LEN);
- pv_set_fid(pv, fmt->orphan_vg->fid);
- }
-
+ lvmcache_fid_add_mdas(info, vg->fid, (const char *) &pv->id, ID_LEN);
+ pv_set_fid(pv, vg->fid);
return pv;
bad:
free_pv_fid(pv);
- dm_pool_free(pvmem, pv);
+ dm_pool_free(vg->vgmem, pv);
return NULL;
}
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 5b8d690..83983b4 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -371,12 +371,6 @@ uint32_t vg_bad_status_bits(const struct volume_group *vg, uint64_t status);
int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
struct physical_volume *pv, int new_pv);
-
-/* Find a PV within a given VG */
-int get_pv_from_vg_by_id(const struct format_type *fmt, const char *vg_name,
- const char *vgid, const char *pvid,
- struct physical_volume *pv);
-
struct logical_volume *find_lv_in_vg_by_lvid(struct volume_group *vg,
const union lvid *lvid);
6 years
- 1
- 0
master - vgcreate: improve the use of label_scan
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=5f138f36040297d0929...
Commit: 5f138f36040297d092977a3b547cdefffb5ac4e8
Parent: e3e5beec74ac0037917f5e9a2693c6ccb16debac
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Thu Oct 26 14:32:30 2017 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:22:45 2018 -0500
vgcreate: improve the use of label_scan
The old code was doing unnecessary label scans when
checking to see if the new VG name exists. A single
label_scan is sufficient if it is done after the
new VG lock is held.
---
tools/vgcreate.c | 28 ++++++++++++++++++++--------
1 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/tools/vgcreate.c b/tools/vgcreate.c
index af0c363..87a296f 100644
--- a/tools/vgcreate.c
+++ b/tools/vgcreate.c
@@ -26,7 +26,6 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
const char *clustered_message = "";
char *vg_name;
struct arg_value_group_list *current_group;
- uint32_t rc;
if (!argc) {
log_error("Please provide volume group name and "
@@ -66,17 +65,30 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
return_ECMD_FAILED;
cmd->lockd_gl_disable = 1;
- lvmcache_seed_infos_from_lvmetad(cmd);
-
/*
* Check if the VG name already exists. This should be done before
* creating PVs on any of the devices.
+ *
+ * When searching if a VG name exists, acquire the VG lock,
+ * 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)
+ * will return non-NULL.
*/
- if ((rc = vg_lock_newname(cmd, vp_new.vg_name)) != SUCCESS) {
- if (rc == FAILED_EXIST)
- log_error("A volume group called %s already exists.", vp_new.vg_name);
- else
- log_error("Can't get lock for %s.", vp_new.vg_name);
+
+ if (!lock_vol(cmd, vp_new.vg_name, LCK_VG_WRITE, NULL)) {
+ log_error("Can't get lock for %s.", vp_new.vg_name);
+ return ECMD_FAILED;
+ }
+
+ lvmcache_force_next_label_scan();
+ lvmcache_label_scan(cmd); /* Does nothing when using lvmetad. */
+ lvmcache_seed_infos_from_lvmetad(cmd); /* Does nothing unless using lvmetad. */
+
+ if (lvmcache_fmt_from_vgname(cmd, vp_new.vg_name, NULL, 0)) {
+ unlock_vg(cmd, NULL, vp_new.vg_name);
+ log_error("A volume group called %s already exists.", vp_new.vg_name);
return ECMD_FAILED;
}
6 years
- 1
- 0
master - lvmpolld: update to use new scanning correctly
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=7b0a8f47be7df13aab0...
Commit: 7b0a8f47be7df13aab0552599aa2dc2233cc223c
Parent: aa833bdd8aa6a3e3557b19fa3877b0eee806df63
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Apr 13 14:40:00 2018 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:22:48 2018 -0500
lvmpolld: update to use new scanning correctly
---
tools/polldaemon.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/tools/polldaemon.c b/tools/polldaemon.c
index cf7a947..83f0424 100644
--- a/tools/polldaemon.c
+++ b/tools/polldaemon.c
@@ -126,6 +126,14 @@ static void _nanosleep(unsigned secs, unsigned allow_zero_time)
static void _sleep_and_rescan_devices(struct cmd_context *cmd, struct daemon_parms *parms)
{
if (parms->interval && !parms->aborting) {
+ /*
+ * FIXME: do we really need to drop everything and then rescan
+ * everything between each iteration? What change exactly does
+ * each iteration check for, and does seeing that require
+ * rescanning everything?
+ */
+ lvmcache_destroy(cmd, 1, 0);
+ label_scan_destroy(cmd);
dev_close_all();
_nanosleep(parms->interval, 1);
lvmcache_label_scan(cmd);
@@ -141,6 +149,9 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id,
uint32_t lockd_state = 0;
int ret;
+ if (!parms->wait_before_testing)
+ lvmcache_label_scan(cmd);
+
/* Poll for completion */
while (!finished) {
if (parms->wait_before_testing)
6 years
- 1
- 0
master - lvmetad: use new label_scan for update from pvscan
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=e3e5beec74ac0037917...
Commit: e3e5beec74ac0037917f5e9a2693c6ccb16debac
Parent: 9c71fa02144619a67993920cee2146fed820f49c
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Feb 7 13:58:40 2018 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:22:43 2018 -0500
lvmetad: use new label_scan for update from pvscan
Take advantage of the common implementation with aio
and reduced disk reads.
---
lib/cache/lvmetad.c | 38 ++++++++++++++++++++++++-------
lib/commands/toolcontext.h | 2 +-
lib/format_text/import_vsn1.c | 6 ++++-
tools/pvscan.c | 49 +++++++++++++++++++++++++++++++++++------
4 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index 552dbf0..81ba1b7 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -2240,9 +2240,12 @@ int lvmetad_pvscan_single(struct cmd_context *cmd, struct device *dev,
struct label *label;
struct lvmcache_info *info;
struct _lvmetad_pvscan_baton baton;
+ const struct format_type *fmt;
/* Create a dummy instance. */
struct format_instance_ctx fic = { .type = 0 };
+ log_debug_lvmetad("Scan metadata from dev %s", dev_name(dev));
+
if (!lvmetad_used()) {
log_error("Cannot proceed since lvmetad is not active.");
return 0;
@@ -2253,23 +2256,31 @@ int lvmetad_pvscan_single(struct cmd_context *cmd, struct device *dev,
return 1;
}
- if (!label_read(dev, &label, 0)) {
- log_print_unless_silent("No PV label found on %s.", dev_name(dev));
+ if (!(info = lvmcache_info_from_pvid(dev->pvid, dev, 0))) {
+ log_print_unless_silent("No PV info found on %s for PVID %s.", dev_name(dev), dev->pvid);
+ if (!lvmetad_pv_gone_by_dev(dev))
+ goto_bad;
+ return 1;
+ }
+
+ if (!(label = lvmcache_get_label(info))) {
+ log_print_unless_silent("No PV label found for %s.", dev_name(dev));
if (!lvmetad_pv_gone_by_dev(dev))
goto_bad;
return 1;
}
- info = (struct lvmcache_info *) label->info;
+ fmt = lvmcache_fmt(info);
+ baton.cmd = cmd;
baton.vg = NULL;
- baton.fid = lvmcache_fmt(info)->ops->create_instance(lvmcache_fmt(info), &fic);
+ baton.fid = fmt->ops->create_instance(fmt, &fic);
if (!baton.fid)
goto_bad;
- if (baton.fid->fmt->features & FMT_OBSOLETE) {
- lvmcache_fmt(info)->ops->destroy_instance(baton.fid);
+ if (fmt->features & FMT_OBSOLETE) {
+ fmt->ops->destroy_instance(baton.fid);
log_warn("WARNING: Disabling lvmetad cache which does not support obsolete (lvm1) metadata.");
lvmetad_set_disabled(cmd, LVMETAD_DISABLE_REASON_LVM1);
_found_lvm1_metadata = 1;
@@ -2283,9 +2294,9 @@ int lvmetad_pvscan_single(struct cmd_context *cmd, struct device *dev,
lvmcache_foreach_mda(info, _lvmetad_pvscan_single, &baton);
if (!baton.vg)
- lvmcache_fmt(info)->ops->destroy_instance(baton.fid);
+ fmt->ops->destroy_instance(baton.fid);
- if (!lvmetad_pv_found(cmd, (const struct id *) &dev->pvid, dev, lvmcache_fmt(info),
+ if (!lvmetad_pv_found(cmd, (const struct id *) &dev->pvid, dev, fmt,
label->sector, baton.vg, found_vgnames, changed_vgnames)) {
release_vg(baton.vg);
goto_bad;
@@ -2351,6 +2362,13 @@ int lvmetad_pvscan_all_devs(struct cmd_context *cmd, int do_wait)
replacing_other_update = 1;
}
+ label_scan(cmd);
+
+ if (lvmcache_found_duplicate_pvs()) {
+ log_warn("WARNING: Scan found duplicate PVs.");
+ return 0;
+ }
+
log_verbose("Scanning all devices to update lvmetad.");
if (!(iter = dev_iter_create(cmd->lvmetad_filter, 1))) {
@@ -2721,6 +2739,8 @@ void lvmetad_validate_global_cache(struct cmd_context *cmd, int force)
*/
_lvmetad_get_pv_cache_list(cmd, &pvc_before);
+ log_debug_lvmetad("Rescan all devices to validate global cache.");
+
/*
* Update the local lvmetad cache so it correctly reflects any
* changes made on remote hosts. (It's possible that this command
@@ -2789,7 +2809,7 @@ void lvmetad_validate_global_cache(struct cmd_context *cmd, int force)
_update_changed_pvs_in_udev(cmd, &pvc_before, &pvc_after);
}
- log_debug_lvmetad("Validating global lvmetad cache finished");
+ log_debug_lvmetad("Rescanned all devices");
}
int lvmetad_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const char *vgid)
diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h
index f04afec..d20cef1 100644
--- a/lib/commands/toolcontext.h
+++ b/lib/commands/toolcontext.h
@@ -167,8 +167,8 @@ struct cmd_context {
unsigned pv_notify:1;
unsigned activate_component:1; /* command activates component LV */
unsigned process_component_lvs:1; /* command processes also component LVs */
-
unsigned mirror_warn_printed:1; /* command already printed warning about non-monitored mirrors */
+ unsigned pvscan_cache_single:1;
/*
* Filtering.
*/
diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index b41d83c..dee5379 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -221,7 +221,11 @@ static int _read_pv(struct format_instance *fid,
if (!id_write_format(&pv->id, buffer, sizeof(buffer)))
buffer[0] = '\0';
- log_error_once("Couldn't find device with uuid %s.", buffer);
+
+ if (fid->fmt->cmd && !fid->fmt->cmd->pvscan_cache_single)
+ log_error_once("Couldn't find device with uuid %s.", buffer);
+ else
+ log_debug_metadata("Couldn't find device with uuid %s.", buffer);
}
if (!(pv->vg_name = dm_pool_strdup(mem, vg->name)))
diff --git a/tools/pvscan.c b/tools/pvscan.c
index 6581990..ab6ea0b 100644
--- a/tools/pvscan.c
+++ b/tools/pvscan.c
@@ -300,8 +300,10 @@ static int _pvscan_autoactivate(struct cmd_context *cmd, struct pvscan_aa_params
static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
{
struct pvscan_aa_params pp = { 0 };
+ struct dm_list single_devs;
struct dm_list found_vgnames;
struct device *dev;
+ struct device_list *devl;
const char *pv_name;
const char *reason = NULL;
int32_t major = -1;
@@ -434,8 +436,12 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
* to drop any devices that have left.)
*/
- if (argc || devno_args)
+ if (argc || devno_args) {
log_verbose("Scanning devices on command line.");
+ cmd->pvscan_cache_single = 1;
+ }
+
+ dm_list_init(&single_devs);
while (argc--) {
pv_name = *argv++;
@@ -453,8 +459,11 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
} else {
/* Add device path to lvmetad. */
log_debug("Scanning dev %s for lvmetad cache.", pv_name);
- if (!lvmetad_pvscan_single(cmd, dev, &found_vgnames, &pp.changed_vgnames))
- add_errors++;
+
+ if (!(devl = dm_pool_zalloc(cmd->mem, sizeof(*devl))))
+ return_0;
+ devl->dev = dev;
+ dm_list_add(&single_devs, &devl->list);
}
} else {
if (sscanf(pv_name, "%d:%d", &major, &minor) != 2) {
@@ -471,8 +480,11 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
} else {
/* Add major:minor to lvmetad. */
log_debug("Scanning dev %d:%d for lvmetad cache.", major, minor);
- if (!lvmetad_pvscan_single(cmd, dev, &found_vgnames, &pp.changed_vgnames))
- add_errors++;
+
+ if (!(devl = dm_pool_zalloc(cmd->mem, sizeof(*devl))))
+ return_0;
+ devl->dev = dev;
+ dm_list_add(&single_devs, &devl->list);
}
}
@@ -482,9 +494,20 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
}
}
+ if (!dm_list_empty(&single_devs)) {
+ label_scan_devs(cmd, &single_devs);
+
+ dm_list_iterate_items(devl, &single_devs) {
+ if (!lvmetad_pvscan_single(cmd, devl->dev, &found_vgnames, &pp.changed_vgnames))
+ add_errors++;
+ }
+ }
+
if (!devno_args)
goto activate;
+ dm_list_init(&single_devs);
+
/* Process any grouped --major --minor args */
dm_list_iterate_items(current_group, &cmd->arg_value_groups) {
major = grouped_arg_int_value(current_group->arg_values, major_ARG, major);
@@ -503,8 +526,11 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
} else {
/* Add major:minor to lvmetad. */
log_debug("Scanning dev %d:%d for lvmetad cache.", major, minor);
- if (!lvmetad_pvscan_single(cmd, dev, &found_vgnames, &pp.changed_vgnames))
- add_errors++;
+
+ if (!(devl = dm_pool_zalloc(cmd->mem, sizeof(*devl))))
+ return_0;
+ devl->dev = dev;
+ dm_list_add(&single_devs, &devl->list);
}
if (sigint_caught()) {
@@ -513,6 +539,15 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv)
}
}
+ if (!dm_list_empty(&single_devs)) {
+ label_scan_devs(cmd, &single_devs);
+
+ dm_list_iterate_items(devl, &single_devs) {
+ if (!lvmetad_pvscan_single(cmd, devl->dev, &found_vgnames, &pp.changed_vgnames))
+ add_errors++;
+ }
+ }
+
/*
* In the process of scanning devices, lvmetad may have become
* disabled. If so, revert to scanning for the autoactivation step.
6 years
- 1
- 0
master - bcache: intercept test mode before write
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=aa833bdd8aa6a3e3557...
Commit: aa833bdd8aa6a3e3557b19fa3877b0eee806df63
Parent: 9b6a62f9445b104f8b4f14b1ebe8258b360950e4
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Apr 9 13:57:44 2018 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:22:48 2018 -0500
bcache: intercept test mode before write
Don't allow writes in test mode. test mode should be
more sophisticated than just faking writes, and this
should be a last defense for cases where test mode is
not being checked correctly.
---
lib/label/label.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/lib/label/label.c b/lib/label/label.c
index c11a040..4b18f56 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -922,6 +922,9 @@ bool dev_write_bytes(struct device *dev, off_t start, size_t len, void *data)
{
int ret;
+ if (test_mode())
+ return true;
+
if (!scan_bcache) {
if (!dev_open(dev))
return false;
@@ -955,6 +958,9 @@ bool dev_write_zeros(struct device *dev, off_t start, size_t len)
{
int ret;
+ if (test_mode())
+ return true;
+
if (!scan_bcache) {
if (!dev_open(dev))
return false;
6 years
- 1
- 0
master - lvmetad: use new label_scan for update from lvmlockd
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=9c71fa02144619a6799...
Commit: 9c71fa02144619a67993920cee2146fed820f49c
Parent: 098c843c50cdcc2e4f4162037e1ff5975624f3e2
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Thu Oct 26 10:58:23 2017 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:21:41 2018 -0500
lvmetad: use new label_scan for update from lvmlockd
When lvmlockd indicates that the lvmetad cache is out of
date because of changes by another node, lvmetad_pvscan_vg()
rescans the devices in the VG to update lvmetad. Use the
new label_scan in this function to use the common code and
take advantage of the new aio and reduced reads.
---
lib/cache/lvmetad.c | 435 +++++++++++++++++++++++++++++++++++----------------
1 files changed, 298 insertions(+), 137 deletions(-)
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index 4b7410c..552dbf0 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -39,7 +39,7 @@ static int64_t _lvmetad_update_timeout;
static int _found_lvm1_metadata = 0;
-static struct volume_group *_lvmetad_pvscan_vg(struct cmd_context *cmd, struct volume_group *vg);
+static struct volume_group *_lvmetad_pvscan_vg(struct cmd_context *cmd, struct volume_group *vg, const char *vgid, struct format_type *fmt);
static uint64_t _monotonic_seconds(void)
{
@@ -1093,14 +1093,17 @@ struct volume_group *lvmetad_vg_lookup(struct cmd_context *cmd, const char *vgna
* invalidated the cached vg.
*/
if (rescan) {
- if (!(vg2 = _lvmetad_pvscan_vg(cmd, vg))) {
+ if (!(vg2 = _lvmetad_pvscan_vg(cmd, vg, vgid, fmt))) {
log_debug_lvmetad("VG %s from lvmetad not found during rescan.", vgname);
fid = NULL;
release_vg(vg);
vg = NULL;
goto out;
}
+ fid->ref_count++;
release_vg(vg);
+ fid->ref_count--;
+ fmt->ops->destroy_instance(fid);
vg = vg2;
fid = vg2->fid;
}
@@ -1108,14 +1111,14 @@ struct volume_group *lvmetad_vg_lookup(struct cmd_context *cmd, const char *vgna
dm_list_iterate_items(pvl, &vg->pvs) {
if (!_pv_update_struct_pv(pvl->pv, fid)) {
vg = NULL;
- goto_out; /* FIXME error path */
+ goto_out; /* FIXME: use an error path that disables lvmetad */
}
}
dm_list_iterate_items(pvl, &vg->pvs_outdated) {
if (!_pv_update_struct_pv(pvl->pv, fid)) {
vg = NULL;
- goto_out; /* FIXME error path */
+ goto_out; /* FIXME: use an error path that disables lvmetad */
}
}
@@ -1761,6 +1764,7 @@ int lvmetad_pv_gone_by_dev(struct device *dev)
*/
struct _lvmetad_pvscan_baton {
+ struct cmd_context *cmd;
struct volume_group *vg;
struct format_instance *fid;
};
@@ -1771,7 +1775,7 @@ static int _lvmetad_pvscan_single(struct metadata_area *mda, void *baton)
struct volume_group *vg;
if (mda_is_ignored(mda) ||
- !(vg = mda->ops->vg_read(b->fid, "", mda, NULL, NULL, 1)))
+ !(vg = mda->ops->vg_read(b->fid, "", mda, NULL, NULL)))
return 1;
/* FIXME Also ensure contents match etc. */
@@ -1784,6 +1788,33 @@ static int _lvmetad_pvscan_single(struct metadata_area *mda, void *baton)
}
/*
+ * FIXME: handle errors and do proper comparison of metadata from each area
+ * like vg_read and fall back to real vg_read from disk if there's any problem.
+ */
+
+static int _lvmetad_pvscan_vg_single(struct metadata_area *mda, void *baton)
+{
+ struct _lvmetad_pvscan_baton *b = baton;
+ struct volume_group *vg = NULL;
+
+ if (mda_is_ignored(mda))
+ return 1;
+
+ if (!(vg = mda->ops->vg_read(b->fid, "", mda, NULL, NULL)))
+ return 1;
+
+ if (!b->vg)
+ b->vg = vg;
+ else if (vg->seqno > b->vg->seqno) {
+ release_vg(b->vg);
+ b->vg = vg;
+ } else
+ release_vg(vg);
+
+ return 1;
+}
+
+/*
* The lock manager may detect that the vg cached in lvmetad is out of date,
* due to something like an lvcreate from another host.
* This is limited to changes that only affect the vg (not global state like
@@ -1792,41 +1823,41 @@ static int _lvmetad_pvscan_single(struct metadata_area *mda, void *baton)
* the VG, and that PV may have been reused for another VG.
*/
-static struct volume_group *_lvmetad_pvscan_vg(struct cmd_context *cmd, struct volume_group *vg)
+static struct volume_group *_lvmetad_pvscan_vg(struct cmd_context *cmd, struct volume_group *vg,
+ const char *vgid, struct format_type *fmt)
{
char pvid_s[ID_LEN + 1] __attribute__((aligned(8)));
char uuid[64] __attribute__((aligned(8)));
- struct label *label;
- struct volume_group *vg_ret = NULL;
- struct dm_config_tree *vgmeta_ret = NULL;
struct dm_config_tree *vgmeta;
struct pv_list *pvl, *pvl_new;
- struct device_list *devl, *devl_new, *devlsafe;
+ struct device_list *devl, *devlsafe;
struct dm_list pvs_scan;
struct dm_list pvs_drop;
- struct dm_list pvs_new;
+ struct lvmcache_vginfo *vginfo = NULL;
struct lvmcache_info *info = NULL;
struct format_instance *fid;
struct format_instance_ctx fic = { .type = 0 };
struct _lvmetad_pvscan_baton baton;
+ struct volume_group *save_vg;
+ struct dm_config_tree *save_meta;
struct device *save_dev = NULL;
uint32_t save_seqno = 0;
- int missing_devs = 0;
- int check_new_pvs = 0;
+ int found_new_pvs = 0;
+ int retried_reads = 0;
int found;
+ save_vg = NULL;
+ save_meta = NULL;
+ save_dev = NULL;
+ save_seqno = 0;
+
dm_list_init(&pvs_scan);
dm_list_init(&pvs_drop);
- dm_list_init(&pvs_new);
- log_debug_lvmetad("Rescanning VG %s (seqno %u).", vg->name, vg->seqno);
+ log_debug_lvmetad("Rescan VG %s to update lvmetad (seqno %u).", vg->name, vg->seqno);
/*
- * Another host may have added a PV to the VG, and some
- * commands do not always populate their lvmcache with
- * all devs from lvmetad, so they would fail to find
- * the new PV when scanning the VG. So make sure this
- * command knows about all PVs from lvmetad.
+ * Make sure this command knows about all PVs from lvmetad.
*/
lvmcache_seed_infos_from_lvmetad(cmd);
@@ -1841,54 +1872,111 @@ static struct volume_group *_lvmetad_pvscan_vg(struct cmd_context *cmd, struct v
dm_list_add(&pvs_scan, &devl->list);
}
-scan_more:
+ /*
+ * Rescan labels/metadata only from devs that we previously
+ * saw in the VG. If we find below that there are new PVs
+ * in the VG, we'll have to rescan all devices to find which
+ * device(s) are now being used.
+ */
+ log_debug_lvmetad("Rescan VG %s scanning data from devs in previous metadata.", vg->name);
+
+ label_scan_devs(cmd, &pvs_scan);
/*
- * Run the equivalent of lvmetad_pvscan_single on each dev in the VG.
+ * Check if any pvs_scan entries are no longer PVs.
+ * In that case, label_read/_find_label_header will have
+ * found no label_header, and would have dropped the
+ * info struct for the device from lvmcache. So, if
+ * we look up the info struct here and don't find it,
+ * we can infer it's no longer a PV.
+ *
+ * FIXME: we should record specific results from the
+ * label_read and then check specifically for whatever
+ * result means "no label was found", rather than going
+ * about this indirectly via the lvmcache side effects.
*/
dm_list_iterate_items_safe(devl, devlsafe, &pvs_scan) {
- if (!devl->dev)
- continue;
+ if (!(info = lvmcache_info_from_pvid(devl->dev->pvid, devl->dev, 0))) {
+ /* Another host removed this PV from the VG. */
+ log_debug_lvmetad("Rescan VG %s from %s dropping dev (no label).",
+ vg->name, dev_name(devl->dev));
+ dm_list_move(&pvs_drop, &devl->list);
+ }
+ }
- log_debug_lvmetad("Rescan VG %s scanning %s.", vg->name, dev_name(devl->dev));
+ fic.type = FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
+ fic.context.vg_ref.vg_name = vg->name;
+ fic.context.vg_ref.vg_id = vgid;
- if (!label_read(devl->dev, &label, 0)) {
- /* Another host removed this PV from the VG. */
- log_debug_lvmetad("Rescan VG %s found %s was removed.", vg->name, dev_name(devl->dev));
+ retry_reads:
- if ((info = lvmcache_info_from_pvid(devl->dev->pvid, NULL, 0)))
- lvmcache_del(info);
+ if (!(fid = fmt->ops->create_instance(fmt, &fic))) {
+ /* FIXME: are there only internal reasons for failures here? */
+ log_error("Reading VG %s failed to create format instance.", vg->name);
+ return NULL;
+ }
- dm_list_move(&pvs_drop, &devl->list);
- continue;
- }
+ /* FIXME: not sure if this is necessary */
+ fid->ref_count++;
- info = (struct lvmcache_info *) label->info;
+ baton.fid = fid;
+ baton.cmd = cmd;
- baton.vg = NULL;
- baton.fid = lvmcache_fmt(info)->ops->create_instance(lvmcache_fmt(info), &fic);
- if (!baton.fid)
- return_NULL;
+ /*
+ * FIXME: this vg_read path does not have the ability to repair
+ * any problems with the VG, e.g. VG on one dev has an older
+ * seqno. When vg_read() is reworked, we need to fall back
+ * to using that from here (and vg_read's from lvmetad) when
+ * there is a problem. Perhaps by disabling lvmetad when a
+ * VG problem is detected, causing commands to fully fall
+ * back to disk, which will repair the VG. Then lvmetad can
+ * be repopulated and re-enabled (possibly automatically.)
+ */
- if (baton.fid->fmt->features & FMT_OBSOLETE) {
- log_debug_lvmetad("Ignoring obsolete format on PV %s in VG %s.", dev_name(devl->dev), vg->name);
- lvmcache_fmt(info)->ops->destroy_instance(baton.fid);
+ /*
+ * Do a low level vg_read on each dev, verify the vg returned
+ * from metadata on each device is for the VG being read
+ * (the PV may have been removed from the VG being read and
+ * added to a different one), and return this vg to the caller
+ * as the current vg to use.
+ *
+ * The label scan above will have saved in lvmcache which
+ * vg each device is used in, so we could figure that part
+ * out without doing the vg_read.
+ */
+ dm_list_iterate_items_safe(devl, devlsafe, &pvs_scan) {
+ if (!devl->dev)
+ continue;
+
+ log_debug_lvmetad("Rescan VG %s getting metadata from %s.",
+ vg->name, dev_name(devl->dev));
+
+ /*
+ * The info struct for this dev knows what and where
+ * the mdas are for this dev (the label scan saved
+ * the mda locations for this dev on the lvmcache info struct).
+ */
+ if (!(info = lvmcache_info_from_pvid(devl->dev->pvid, devl->dev, 0))) {
+ log_debug_lvmetad("Rescan VG %s from %s dropping dev (no info).",
+ vg->name, dev_name(devl->dev));
dm_list_move(&pvs_drop, &devl->list);
continue;
}
+ baton.vg = NULL;
+
/*
* Read VG metadata from this dev's mdas.
*/
- lvmcache_foreach_mda(info, _lvmetad_pvscan_single, &baton);
+ lvmcache_foreach_mda(info, _lvmetad_pvscan_vg_single, &baton);
/*
* The PV may have been removed from the VG by another host
* since we last read the VG.
*/
if (!baton.vg) {
- log_debug_lvmetad("Rescan VG %s did not find %s.", vg->name, dev_name(devl->dev));
- lvmcache_fmt(info)->ops->destroy_instance(baton.fid);
+ log_debug_lvmetad("Rescan VG %s from %s dropping dev (no metadata).",
+ vg->name, dev_name(devl->dev));
dm_list_move(&pvs_drop, &devl->list);
continue;
}
@@ -1898,10 +1986,15 @@ scan_more:
* different VG since we last read the VG.
*/
if (strcmp(baton.vg->name, vg->name)) {
- log_debug_lvmetad("Rescan VG %s found different VG %s on PV %s.",
- vg->name, baton.vg->name, dev_name(devl->dev));
+ log_debug_lvmetad("Rescan VG %s from %s dropping dev (other VG %s).",
+ vg->name, dev_name(devl->dev), baton.vg->name);
+ release_vg(baton.vg);
+ continue;
+ }
+
+ if (!(vgmeta = export_vg_to_config_tree(baton.vg))) {
+ log_error("VG export to config tree failed");
release_vg(baton.vg);
- dm_list_move(&pvs_drop, &devl->list);
continue;
}
@@ -1911,20 +2004,35 @@ scan_more:
* read from each other dev.
*/
- if (!save_seqno)
- save_seqno = baton.vg->seqno;
-
- if (!(vgmeta = export_vg_to_config_tree(baton.vg))) {
- log_error("VG export to config tree failed");
- release_vg(baton.vg);
- return NULL;
+ if (save_vg && (save_seqno != baton.vg->seqno)) {
+ /* FIXME: fall back to vg_read to correct this. */
+ log_warn("WARNING: inconsistent metadata for VG %s on devices %s seqno %u and %s seqno %u.",
+ vg->name, dev_name(save_dev), save_seqno,
+ dev_name(devl->dev), baton.vg->seqno);
+ log_warn("WARNING: temporarily disable lvmetad to repair metadata.");
+
+ /* Use the most recent */
+ if (save_seqno < baton.vg->seqno) {
+ release_vg(save_vg);
+ dm_config_destroy(save_meta);
+ save_vg = baton.vg;
+ save_meta = vgmeta;
+ save_seqno = baton.vg->seqno;
+ save_dev = devl->dev;
+ } else {
+ release_vg(baton.vg);
+ dm_config_destroy(vgmeta);
+ }
+ continue;
}
- if (!vgmeta_ret) {
- vgmeta_ret = vgmeta;
+ if (!save_vg) {
+ save_vg = baton.vg;
+ save_meta = vgmeta;
+ save_seqno = baton.vg->seqno;
save_dev = devl->dev;
} else {
- struct dm_config_node *meta1 = vgmeta_ret->root;
+ struct dm_config_node *meta1 = save_meta->root;
struct dm_config_node *meta2 = vgmeta->root;
struct dm_config_node *sib1 = meta1->sib;
struct dm_config_node *sib2 = meta2->sib;
@@ -1949,73 +2057,128 @@ scan_more:
meta2->sib = NULL;
if (compare_config(meta1, meta2)) {
+ /* FIXME: fall back to vg_read to correct this. */
+ log_warn("WARNING: inconsistent metadata for VG %s on devices %s seqno %u and %s seqno %u.",
+ vg->name, dev_name(save_dev), save_seqno,
+ dev_name(devl->dev), baton.vg->seqno);
+ log_warn("WARNING: temporarily disable lvmetad to repair metadata.");
log_error("VG %s metadata comparison failed for device %s vs %s",
vg->name, dev_name(devl->dev), save_dev ? dev_name(save_dev) : "none");
- _log_debug_inequality(vg->name, vgmeta_ret->root, vgmeta->root);
+ _log_debug_inequality(vg->name, save_meta->root, vgmeta->root);
meta1->sib = sib1;
meta2->sib = sib2;
- dm_config_destroy(vgmeta);
- dm_config_destroy(vgmeta_ret);
+
+ /* no right choice, just use the previous copy */
release_vg(baton.vg);
- return NULL;
+ dm_config_destroy(vgmeta);
}
meta1->sib = sib1;
meta2->sib = sib2;
+ release_vg(baton.vg);
dm_config_destroy(vgmeta);
}
+ }
- /*
- * Look for any new PVs in the VG metadata that were not in our
- * previous version of the VG. Add them to pvs_new to be
- * scanned in this loop just like the old PVs.
- */
- if (!check_new_pvs) {
- check_new_pvs = 1;
- dm_list_iterate_items(pvl_new, &baton.vg->pvs) {
- found = 0;
- dm_list_iterate_items(pvl, &vg->pvs) {
- if (pvl_new->pv->dev != pvl->pv->dev)
- continue;
- found = 1;
- break;
- }
- if (found)
- continue;
- if (!pvl_new->pv->dev) {
- strncpy(pvid_s, (char *) &pvl_new->pv->id, sizeof(pvid_s) - 1);
- if (!id_write_format((const struct id *)&pvid_s, uuid, sizeof(uuid)))
- stack;
- log_error("Device not found for PV %s in VG %s", uuid, vg->name);
- missing_devs++;
+ /* FIXME: see above */
+ fid->ref_count--;
+
+ /*
+ * Look for any new PVs in the VG metadata that were not in our
+ * previous version of the VG.
+ *
+ * (Don't look for new PVs after a rescan and retry.)
+ */
+ found_new_pvs = 0;
+
+ if (save_vg && !retried_reads) {
+ dm_list_iterate_items(pvl_new, &save_vg->pvs) {
+ found = 0;
+ dm_list_iterate_items(pvl, &vg->pvs) {
+ if (pvl_new->pv->dev != pvl->pv->dev)
continue;
- }
- if (!(devl_new = dm_pool_zalloc(cmd->mem, sizeof(*devl_new))))
- return_NULL;
- devl_new->dev = pvl_new->pv->dev;
- dm_list_add(&pvs_new, &devl_new->list);
- log_debug_lvmetad("Rescan VG %s found %s was added.", vg->name, dev_name(devl_new->dev));
+ found = 1;
+ break;
+ }
+
+ /*
+ * PV in new VG metadata not found in old VG metadata.
+ * There's a good chance we don't know about this new
+ * PV or what device it's on; a label scan is needed
+ * of all devices so we know which device the VG is
+ * now using.
+ */
+ if (!found) {
+ found_new_pvs++;
+ strncpy(pvid_s, (char *) &pvl_new->pv->id, sizeof(pvid_s) - 1);
+ if (!id_write_format((const struct id *)&pvid_s, uuid, sizeof(uuid)))
+ stack;
+ log_debug_lvmetad("Rescan VG %s found new PV %s.", vg->name, uuid);
}
}
+ }
- release_vg(baton.vg);
+ if (!save_vg && retried_reads) {
+ log_error("VG %s not found after rescanning devices.", vg->name);
+ goto out;
}
/*
- * Do the same scanning above for any new PVs.
+ * Do a full rescan of devices, then look up which devices the
+ * scan found for this VG name, and select those devices to
+ * read metadata from in the loop above (rather than the list
+ * of devices we created from our last copy of the vg metadata.)
+ *
+ * Case 1: VG we knew is no longer on any of the devices we knew it
+ * to be on (save_vg is NULL, which means the metadata wasn't found
+ * when reading mdas on each of the initial pvs_scan devices).
+ * Rescan all devs and then retry reading metadata from the devs that
+ * the scan finds associated with this VG.
+ *
+ * Case 2: VG has new PVs but we don't know what devices they are
+ * so rescan all devs and then retry reading metadata from the devs
+ * that the scan finds associated with this VG.
+ *
+ * (N.B. after a retry, we don't check for found_new_pvs.)
*/
- if (!dm_list_empty(&pvs_new)) {
+ if (!save_vg || found_new_pvs) {
+ if (!save_vg)
+ log_debug_lvmetad("Rescan VG %s did not find VG on previous devs.", vg->name);
+ if (found_new_pvs)
+ log_debug_lvmetad("Rescan VG %s scanning all devs to find new PVs.", vg->name);
+
+ label_scan(cmd);
+
+ if (!(vginfo = lvmcache_vginfo_from_vgname(vg->name, NULL))) {
+ log_error("VG %s vg info not found after rescanning devices.", vg->name);
+ goto out;
+ }
+
+ /*
+ * Set pvs_scan to devs that the label scan found
+ * in the VG and retry the metadata reading loop.
+ */
dm_list_init(&pvs_scan);
- dm_list_splice(&pvs_scan, &pvs_new);
- dm_list_init(&pvs_new);
- log_debug_lvmetad("Rescan VG %s found new PVs to scan.", vg->name);
- goto scan_more;
- }
- if (missing_devs) {
- if (vgmeta_ret)
- dm_config_destroy(vgmeta_ret);
- return_NULL;
+ if (!lvmcache_get_vg_devs(cmd, vginfo, &pvs_scan)) {
+ log_error("VG %s info devs not found after rescanning devices.", vg->name);
+ goto out;
+ }
+
+ log_debug_lvmetad("Rescan VG %s has %d PVs after label scan.",
+ vg->name, dm_list_size(&pvs_scan));
+
+ if (save_vg)
+ release_vg(save_vg);
+ if (save_meta)
+ dm_config_destroy(save_meta);
+ save_vg = NULL;
+ save_meta = NULL;
+ save_dev = NULL;
+ save_seqno = 0;
+ found_new_pvs = 0;
+ retried_reads = 1;
+ goto retry_reads;
}
/*
@@ -2024,52 +2187,50 @@ scan_more:
dm_list_iterate_items(devl, &pvs_drop) {
if (!devl->dev)
continue;
- log_debug_lvmetad("Rescan VG %s dropping %s.", vg->name, dev_name(devl->dev));
- if (!lvmetad_pv_gone_by_dev(devl->dev))
- return_NULL;
+ log_debug_lvmetad("Rescan VG %s removing %s from lvmetad.", vg->name, dev_name(devl->dev));
+ if (!lvmetad_pv_gone_by_dev(devl->dev)) {
+ /* FIXME: use an error path that disables lvmetad */
+ log_error("Failed to remove %s from lvmetad.", dev_name(devl->dev));
+ }
}
/*
- * Update the VG in lvmetad.
+ * Update lvmetad with the newly read version of the VG.
+ * When the seqno is unchanged the cached VG can be left.
*/
- if (vgmeta_ret) {
- fid = lvmcache_fmt(info)->ops->create_instance(lvmcache_fmt(info), &fic);
- if (!(vg_ret = import_vg_from_config_tree(vgmeta_ret, fid))) {
- log_error("VG import from config tree failed");
- lvmcache_fmt(info)->ops->destroy_instance(fid);
- goto out;
+ if (save_vg && (save_seqno != vg->seqno)) {
+ dm_list_iterate_items(devl, &pvs_scan) {
+ if (!devl->dev)
+ continue;
+ log_debug_lvmetad("Rescan VG %s removing %s from lvmetad to replace.",
+ vg->name, dev_name(devl->dev));
+ if (!lvmetad_pv_gone_by_dev(devl->dev)) {
+ /* FIXME: use an error path that disables lvmetad */
+ log_error("Failed to remove %s from lvmetad.", dev_name(devl->dev));
+ }
}
+ log_debug_lvmetad("Rescan VG %s updating lvmetad from seqno %u to seqno %u.",
+ vg->name, vg->seqno, save_seqno);
+
/*
- * Update lvmetad with the newly read version of the VG.
- * When the seqno is unchanged the cached VG can be left.
+ * If this vg_update fails the cached metadata in
+ * lvmetad will remain invalid.
*/
- if (save_seqno != vg->seqno) {
- dm_list_iterate_items(devl, &pvs_scan) {
- if (!devl->dev)
- continue;
- log_debug_lvmetad("Rescan VG %s dropping to replace %s.", vg->name, dev_name(devl->dev));
- if (!lvmetad_pv_gone_by_dev(devl->dev))
- return_NULL;
- }
-
- log_debug_lvmetad("Rescan VG %s updating lvmetad from seqno %u to seqno %u.",
- vg->name, vg->seqno, save_seqno);
-
- /*
- * If this vg_update fails the cached metadata in
- * lvmetad will remain invalid.
- */
- vg_ret->lvmetad_update_pending = 1;
- if (!lvmetad_vg_update_finish(vg_ret))
- log_error("Failed to update lvmetad with new VG meta");
+ save_vg->lvmetad_update_pending = 1;
+ if (!lvmetad_vg_update_finish(save_vg)) {
+ /* FIXME: use an error path that disables lvmetad */
+ log_error("Failed to update lvmetad with new VG meta");
}
- dm_config_destroy(vgmeta_ret);
}
out:
- if (vg_ret)
- log_debug_lvmetad("Rescan VG %s done (seqno %u).", vg_ret->name, vg_ret->seqno);
- return vg_ret;
+ if (!save_vg && fid)
+ fmt->ops->destroy_instance(fid);
+ if (save_meta)
+ dm_config_destroy(save_meta);
+ if (save_vg)
+ log_debug_lvmetad("Rescan VG %s done (new seqno %u).", save_vg->name, save_vg->seqno);
+ return save_vg;
}
int lvmetad_pvscan_single(struct cmd_context *cmd, struct device *dev,
6 years
- 1
- 0
master - lvmcache: simplify
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=9b6a62f9445b104f8b4...
Commit: 9b6a62f9445b104f8b4f14b1ebe8258b360950e4
Parent: c0973e70a58e7e14e9cca29a0f8ad12719ea554f
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Apr 9 13:40:49 2018 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:22:48 2018 -0500
lvmcache: simplify
Recent changes allow some major simplification of the way
lvmcache works and is used. lvmcache_label_scan is now
called in a controlled fashion at the start of commands,
and not via various unpredictable side effects. Remove
various calls to it from other places. lvmcache_label_scan
should not be called from anywhere during a command, because
it produces an incorrect representation of PVs with no MDAs,
and misclassifies them as orphans. This has been a long
standing problem. The invalid flag and rescanning based on
that is no longer used and removed. The 'force' variation is
no longer needed and removed.
---
daemons/clvmd/lvm-functions.c | 1 -
lib/cache/lvmcache.c | 259 ++++++----------------------------------
lib/cache/lvmcache.h | 8 --
lib/format1/disk-rep.c | 1 -
lib/format1/lvm1-label.c | 1 -
lib/format_pool/disk_rep.c | 3 -
lib/format_text/text_label.c | 1 -
lib/metadata/metadata-liblvm.c | 1 -
lib/metadata/metadata.c | 10 +--
libdm/libdm-config.c | 10 +-
liblvm/lvm_vg.c | 1 -
tools/toollib.c | 6 +
tools/vgcreate.c | 1 -
tools/vgmerge.c | 3 +
tools/vgsplit.c | 3 +
15 files changed, 54 insertions(+), 255 deletions(-)
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index c278692..6254122 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -662,7 +662,6 @@ int do_refresh_cache(void)
}
init_ignore_suspended_devices(1);
- lvmcache_force_next_label_scan();
lvmcache_label_scan(cmd);
label_scan_destroy(cmd); /* destroys bcache (to close devs), keeps lvmcache */
dm_pool_empty(cmd->mem);
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index ef180b9..c8046f7 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -29,7 +29,6 @@
#include "lvmetad.h"
#include "lvmetad-client.h"
-#define CACHE_INVALID 0x00000001
#define CACHE_LOCKED 0x00000002
/* One per device */
@@ -169,15 +168,6 @@ void lvmcache_seed_infos_from_lvmetad(struct cmd_context *cmd)
static void _update_cache_info_lock_state(struct lvmcache_info *info, int locked)
{
- int was_locked = (info->status & CACHE_LOCKED) ? 1 : 0;
-
- /*
- * Cache becomes invalid whenever lock state changes unless
- * exclusive VG_GLOBAL is held (i.e. while scanning).
- */
- if (!lvmcache_vgname_is_locked(VG_GLOBAL) && (was_locked != locked))
- info->status |= CACHE_INVALID;
-
if (locked)
info->status |= CACHE_LOCKED;
else
@@ -235,16 +225,11 @@ static void _suspended_vg_free(struct lvmcache_vginfo *vginfo, int free_old, int
static void _drop_metadata(const char *vgname, int drop_precommitted)
{
struct lvmcache_vginfo *vginfo;
- struct lvmcache_info *info;
if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, NULL)))
return;
if (drop_precommitted)
- dm_list_iterate_items(info, &vginfo->infos)
- info->status |= CACHE_INVALID;
-
- if (drop_precommitted)
_suspended_vg_free(vginfo, 0, 1);
else
_suspended_vg_free(vginfo, 1, 1);
@@ -703,39 +688,6 @@ const char *lvmcache_vgid_from_vgname(struct cmd_context *cmd, const char *vgnam
return NULL;
}
-static int _info_is_valid(struct lvmcache_info *info)
-{
- if (info->status & CACHE_INVALID)
- return 0;
-
- /*
- * The caller must hold the VG lock to manipulate metadata.
- * In a cluster, remote nodes sometimes read metadata in the
- * knowledge that the controlling node is holding the lock.
- * So if the VG appears to be unlocked here, it should be safe
- * to use the cached value.
- */
- if (info->vginfo && !lvmcache_vgname_is_locked(info->vginfo->vgname))
- return 1;
-
- if (!(info->status & CACHE_LOCKED))
- return 0;
-
- return 1;
-}
-
-/* vginfo is invalid if it does not contain at least one valid info */
-static int _vginfo_is_invalid(struct lvmcache_vginfo *vginfo)
-{
- struct lvmcache_info *info;
-
- dm_list_iterate_items(info, &vginfo->infos)
- if (_info_is_valid(info))
- return 0;
-
- return 1;
-}
-
/*
* If valid_only is set, data will only be returned if the cached data is
* known still to be valid.
@@ -765,9 +717,6 @@ struct lvmcache_info *lvmcache_info_from_pvid(const char *pvid, struct device *d
return NULL;
}
- if (valid_only && !_info_is_valid(info))
- return NULL;
-
return info;
}
@@ -805,64 +754,6 @@ char *lvmcache_vgname_from_pvid(struct cmd_context *cmd, const char *pvid)
}
/*
- * FIXME: get rid of the CACHE_INVALID state and rescanning
- * infos with that flag. The code should just know which devices
- * need scanning and when.
- */
-static int _label_scan_invalid(struct cmd_context *cmd)
-{
- struct dm_list devs;
- struct dm_hash_node *n;
- struct device_list *devl;
- struct lvmcache_info *info;
- int dev_count = 0;
- int ret;
-
- dm_list_init(&devs);
-
- dm_hash_iterate(n, _pvid_hash) {
- if (!(info = dm_hash_get_data(_pvid_hash, n)))
- continue;
-
- if (!(info->status & CACHE_INVALID))
- continue;
-
- if (!(devl = dm_pool_zalloc(cmd->mem, sizeof(*devl))))
- return_0;
-
- devl->dev = info->dev;
- dm_list_add(&devs, &devl->list);
- dev_count++;
- }
-
- if (dm_list_empty(&devs))
- return 1;
-
- log_debug_cache("Scanning %d devs with invalid info.", dev_count);
-
- ret = label_scan_devs(cmd, &devs);
-
- return ret;
-}
-
-/*
- * lvmcache_label_scan() remembers that it has already
- * been called, and will not scan labels if it's called
- * again. (It will rescan "INVALID" devices if called again.)
- *
- * To force lvmcache_label_scan() to rescan labels on all devices,
- * call lvmcache_force_next_label_scan() before calling
- * lvmcache_label_scan().
- */
-
-static int _force_label_scan;
-
-void lvmcache_force_next_label_scan(void)
-{
- _force_label_scan = 1;
-}
-
-/*
* Check if any PVs in vg->pvs have the same PVID as any
* entries in _unused_duplicate_devices.
*/
@@ -1233,6 +1124,30 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const
return 1;
}
+/*
+ * Uses label_scan to populate lvmcache with 'vginfo' struct for each VG
+ * and associated 'info' structs for those VGs. Only VG summary information
+ * is used to assemble the vginfo/info during the scan, so the resulting
+ * representation of VG/PV state is incomplete and even incorrect.
+ * Specifically, PVs with no MDAs are considered orphans and placed in the
+ * orphan vginfo by lvmcache_label_scan. This is corrected during the
+ * processing phase as each vg_read() uses VG metadata for each VG to correct
+ * the lvmcache state, i.e. it moves no-MDA PVs from the orphan vginfo onto
+ * the correct vginfo. Once vg_read() is finished for all VGs, all of the
+ * incorrectly placed PVs should have been moved from the orphan vginfo
+ * onto their correct vginfo's, and the orphan vginfo should (in theory)
+ * represent only real orphan PVs. (Note: if lvmcache_label_scan is run
+ * after vg_read udpates to lvmcache state, then the lvmcache will be
+ * incorrect again, so do not run lvmcache_label_scan during the
+ * processing phase.)
+ *
+ * TODO: in this label scan phase, don't stash no-MDA PVs into the
+ * orphan VG. We know that's a fiction, and it can have harmful/damaging
+ * results. Instead, put them into a temporary list where they can be
+ * pulled from later when vg_read uses metadata to resolve which VG
+ * they actually belong to.
+ */
+
int lvmcache_label_scan(struct cmd_context *cmd)
{
struct dm_list del_cache_devs;
@@ -1264,20 +1179,6 @@ int lvmcache_label_scan(struct cmd_context *cmd)
goto out;
}
- /*
- * Scan devices whose info struct has the INVALID flag set.
- * When scanning has read the pv_header, mda_header and
- * mda locations, it will clear the INVALID flag (via
- * lvmcache_make_valid).
- */
- if (_has_scanned && !_force_label_scan) {
- r = _label_scan_invalid(cmd);
- goto out;
- }
-
- if (_force_label_scan && (cmd->full_filter && !cmd->full_filter->use_count) && !refresh_filters(cmd))
- goto_out;
-
if (!cmd->full_filter) {
log_error("label scan is missing full filter");
goto out;
@@ -1339,28 +1240,16 @@ int lvmcache_label_scan(struct cmd_context *cmd)
dm_list_splice(&_unused_duplicate_devs, &del_cache_devs);
}
- _has_scanned = 1;
-
/* Perform any format-specific scanning e.g. text files */
if (cmd->independent_metadata_areas)
dm_list_iterate_items(fmt, &cmd->formats)
if (fmt->ops->scan && !fmt->ops->scan(fmt, NULL))
goto out;
- /*
- * If we are a long-lived process, write out the updated persistent
- * device cache for the benefit of short-lived processes.
- */
- if (_force_label_scan && cmd->is_long_lived &&
- cmd->dump_filter && cmd->full_filter && cmd->full_filter->dump &&
- !cmd->full_filter->dump(cmd->full_filter, 0))
- stack;
-
r = 1;
out:
_scanning_in_progress = 0;
- _force_label_scan = 0;
dm_list_iterate_items(vginfo, &_vginfos) {
if (is_orphan_vg(vginfo->vgname))
@@ -1768,10 +1657,8 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
uint32_t vgstatus, const char *creation_host,
const struct format_type *fmt)
{
- struct lvmcache_vginfo *vginfo, *primary_vginfo, *orphan_vginfo;
- struct lvmcache_info *info2, *info3;
+ struct lvmcache_vginfo *vginfo, *primary_vginfo;
char mdabuf[32];
- // struct lvmcache_vginfo *old_vginfo, *next;
if (!vgname || (info && info->vginfo && !strcmp(info->vginfo->vgname, vgname)))
return 1;
@@ -1780,44 +1667,12 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
if (info)
_drop_vginfo(info, info->vginfo);
- /* Get existing vginfo or create new one */
if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) {
-/*** FIXME - vginfo ends up duplicated instead of renamed.
- // Renaming? This lookup fails.
- if ((vginfo = vginfo_from_vgid(vgid))) {
- next = vginfo->next;
- old_vginfo = vginfo_from_vgname(vginfo->vgname, NULL);
- if (old_vginfo == vginfo) {
- dm_hash_remove(_vgname_hash, old_vginfo->vgname);
- if (old_vginfo->next) {
- if (!dm_hash_insert(_vgname_hash, old_vginfo->vgname, old_vginfo->next)) {
- log_error("vg hash re-insertion failed: %s",
- old_vginfo->vgname);
- return 0;
- }
- }
- } else do {
- if (old_vginfo->next == vginfo) {
- old_vginfo->next = vginfo->next;
- break;
- }
- } while ((old_vginfo = old_vginfo->next));
- vginfo->next = NULL;
-
- dm_free(vginfo->vgname);
- if (!(vginfo->vgname = dm_strdup(vgname))) {
- log_error("cache vgname alloc failed for %s", vgname);
- return 0;
- }
+ /*
+ * Create a vginfo struct for this VG and put the vginfo
+ * into the hash table.
+ */
- // Rename so can assume new name does not already exist
- if (!dm_hash_insert(_vgname_hash, vginfo->vgname, vginfo->next)) {
- log_error("vg hash re-insertion failed: %s",
- vginfo->vgname);
- return 0;
- }
- } else {
-***/
if (!(vginfo = dm_zalloc(sizeof(*vginfo)))) {
log_error("lvmcache_update_vgname: list alloc failed");
return 0;
@@ -1830,52 +1685,24 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
dm_list_init(&vginfo->infos);
/*
- * If we're scanning and there's an invalidated entry, remove it.
- * Otherwise we risk bogus warnings of duplicate VGs.
+ * A different VG (different uuid) can exist with the same name.
+ * In this case, the two VGs will have separate vginfo structs,
+ * but the second will be linked onto the existing vginfo->next,
+ * not in the hash.
*/
- while ((primary_vginfo = lvmcache_vginfo_from_vgname(vgname, NULL)) &&
- _scanning_in_progress && _vginfo_is_invalid(primary_vginfo)) {
- orphan_vginfo = lvmcache_vginfo_from_vgname(primary_vginfo->fmt->orphan_vg_name, NULL);
- if (!orphan_vginfo) {
- log_error(INTERNAL_ERROR "Orphan vginfo %s lost from cache.",
- primary_vginfo->fmt->orphan_vg_name);
- dm_free(vginfo->vgname);
- dm_free(vginfo);
- return 0;
- }
- dm_list_iterate_items_safe(info2, info3, &primary_vginfo->infos) {
- _vginfo_detach_info(info2);
- _vginfo_attach_info(orphan_vginfo, info2);
- if (info2->mdas.n)
- sprintf(mdabuf, " with %u mdas",
- dm_list_size(&info2->mdas));
- else
- mdabuf[0] = '\0';
- log_debug_cache("lvmcache: %s: now in VG %s%s%s%s%s",
- dev_name(info2->dev),
- vgname, orphan_vginfo->vgid[0] ? " (" : "",
- orphan_vginfo->vgid[0] ? orphan_vginfo->vgid : "",
- orphan_vginfo->vgid[0] ? ")" : "", mdabuf);
- }
-
- if (!_drop_vginfo(NULL, primary_vginfo))
- return_0;
- }
+ primary_vginfo = lvmcache_vginfo_from_vgname(vgname, NULL);
- if (!_insert_vginfo(vginfo, vgid, vgstatus, creation_host,
- primary_vginfo)) {
+ if (!_insert_vginfo(vginfo, vgid, vgstatus, creation_host, primary_vginfo)) {
dm_free(vginfo->vgname);
dm_free(vginfo);
return 0;
}
+
/* Ensure orphans appear last on list_iterate */
if (is_orphan_vg(vgname))
dm_list_add(&_vginfos, &vginfo->list);
else
dm_list_add_h(&_vginfos, &vginfo->list);
-/***
- }
-***/
}
if (info)
@@ -2026,10 +1853,6 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg
!is_orphan_vg(info->vginfo->vgname) && critical_section())
return 1;
- /* If moving PV from orphan to real VG, always mark it valid */
- if (!is_orphan_vg(vgname))
- info->status &= ~CACHE_INVALID;
-
if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus,
vgsummary->creation_host, info->fmt) ||
!_lvmcache_update_vgid(info, info->vginfo, vgid) ||
@@ -2230,8 +2053,6 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller,
}
}
- info->status |= CACHE_INVALID;
-
/*
* Add or update the _pvid_hash mapping, pvid to info.
*/
@@ -2617,14 +2438,6 @@ struct label *lvmcache_get_label(struct lvmcache_info *info) {
return info->label;
}
-/*
- * After label_scan reads pv_header, mda_header and mda locations
- * from a PV, it clears the INVALID flag.
- */
-void lvmcache_make_valid(struct lvmcache_info *info) {
- info->status &= ~CACHE_INVALID;
-}
-
uint64_t lvmcache_device_size(struct lvmcache_info *info) {
return info->device_size;
}
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 4343060..107c993 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -67,13 +67,6 @@ void lvmcache_allow_reads_with_lvmetad(void);
void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset);
-/*
- * lvmcache_label_scan() will scan labels the first time it's
- * called, but not on subsequent calls, unless
- * lvmcache_force_next_label_scan() is called first
- * to force the next lvmcache_label_scan() to scan again.
- */
-void lvmcache_force_next_label_scan(void);
int lvmcache_label_scan(struct cmd_context *cmd);
int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid);
@@ -187,7 +180,6 @@ 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);
-void lvmcache_make_valid(struct lvmcache_info *info);
int lvmcache_is_orphan(struct lvmcache_info *info);
int lvmcache_uncertain_ownership(struct lvmcache_info *info);
unsigned lvmcache_mda_count(struct lvmcache_info *info);
diff --git a/lib/format1/disk-rep.c b/lib/format1/disk-rep.c
index 41955af..281adc1 100644
--- a/lib/format1/disk-rep.c
+++ b/lib/format1/disk-rep.c
@@ -337,7 +337,6 @@ static void __update_lvmcache(const struct format_type *fmt,
lvmcache_set_device_size(info, ((uint64_t)xlate32(dl->pvd.pv_size)) << SECTOR_SHIFT);
lvmcache_del_mdas(info);
- lvmcache_make_valid(info);
}
static struct disk_list *__read_disk(const struct format_type *fmt,
diff --git a/lib/format1/lvm1-label.c b/lib/format1/lvm1-label.c
index 3b8a655..691a05a 100644
--- a/lib/format1/lvm1-label.c
+++ b/lib/format1/lvm1-label.c
@@ -84,7 +84,6 @@ static int _lvm1_read(struct labeller *l, struct device *dev, void *buf,
lvmcache_set_ext_flags(info, 0);
lvmcache_del_mdas(info);
lvmcache_del_bas(info);
- lvmcache_make_valid(info);
return 1;
}
diff --git a/lib/format_pool/disk_rep.c b/lib/format_pool/disk_rep.c
index 374ff44..fe9b03e 100644
--- a/lib/format_pool/disk_rep.c
+++ b/lib/format_pool/disk_rep.c
@@ -111,7 +111,6 @@ int read_pool_label(struct pool_list *pl, struct labeller *l,
lvmcache_set_ext_flags(info, 0);
lvmcache_del_mdas(info);
lvmcache_del_bas(info);
- lvmcache_make_valid(info);
pl->dev = dev;
pl->pv = NULL;
@@ -379,8 +378,6 @@ int read_pool_pds(const struct format_type *fmt, const char *vg_name,
vg_name);
return 0;
}
- if (full_scan > 0)
- lvmcache_force_next_label_scan();
lvmcache_label_scan(fmt->cmd);
} while (1);
diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c
index e65079e..c47a35a 100644
--- a/lib/format_text/text_label.c
+++ b/lib/format_text/text_label.c
@@ -443,7 +443,6 @@ out:
return 0;
}
- lvmcache_make_valid(info);
return 1;
}
diff --git a/lib/metadata/metadata-liblvm.c b/lib/metadata/metadata-liblvm.c
index d8b3b2a..b0b678a 100644
--- a/lib/metadata/metadata-liblvm.c
+++ b/lib/metadata/metadata-liblvm.c
@@ -273,7 +273,6 @@ out:
}
if (scan_needed) {
- lvmcache_force_next_label_scan();
if (!lvmcache_label_scan(cmd)) {
stack;
r = 0;
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 8cb06be..8c8ce25 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -3392,8 +3392,6 @@ static struct volume_group *_vg_read_orphans(struct cmd_context *cmd,
struct pv_list head;
dm_list_init(&head.list);
- lvmcache_label_scan(cmd);
- lvmcache_seed_infos_from_lvmetad(cmd);
if (!(vginfo = lvmcache_vginfo_from_vgname(orphan_vgname, NULL)))
return_NULL;
@@ -3839,11 +3837,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
*/
log_debug_metadata("Reading VG rereading labels for %s", vgname);
- 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);
- }
+ lvmcache_label_rescan_vg(cmd, vgname, vgid);
if (!(fmt = lvmcache_fmt_from_vgname(cmd, vgname, vgid, 0))) {
log_debug_metadata("Cache did not find fmt for vgname %s", vgname);
@@ -4531,7 +4525,6 @@ static struct volume_group *_vg_read_by_vgid(struct cmd_context *cmd,
if (!(vgname = lvmcache_vgname_from_vgid(cmd->mem, vgid))) {
log_debug_metadata("Reading VG by vgid %.8s no VG name found, retrying.", vgid);
lvmcache_destroy(cmd, 0, 0);
- lvmcache_force_next_label_scan();
lvmcache_label_scan(cmd);
}
@@ -5572,7 +5565,6 @@ uint32_t vg_lock_newname(struct cmd_context *cmd, const char *vgname)
unlock_vg(cmd, NULL, vgname);
return FAILED_LOCKING;
}
- lvmcache_force_next_label_scan();
lvmcache_label_scan(cmd);
if (!lvmcache_fmt_from_vgname(cmd, vgname, NULL, 0))
return SUCCESS; /* vgname not found after scanning */
diff --git a/libdm/libdm-config.c b/libdm/libdm-config.c
index 746ef07..3f0d251 100644
--- a/libdm/libdm-config.c
+++ b/libdm/libdm-config.c
@@ -963,7 +963,7 @@ static const char *_find_config_str(const void *start, node_lookup_fn find_fn,
if (n && n->v) {
if ((n->v->type == DM_CFG_STRING) &&
(allow_empty || (*n->v->v.str))) {
- log_very_verbose("Setting %s to %s", path, n->v->v.str);
+ /* log_very_verbose("Setting %s to %s", path, n->v->v.str); */
return n->v->v.str;
}
if ((n->v->type != DM_CFG_STRING) || (!allow_empty && fail))
@@ -994,7 +994,7 @@ static int64_t _find_config_int64(const void *start, node_lookup_fn find,
const struct dm_config_node *n = find(start, path);
if (n && n->v && n->v->type == DM_CFG_INT) {
- log_very_verbose("Setting %s to %" PRId64, path, n->v->v.i);
+ /* log_very_verbose("Setting %s to %" PRId64, path, n->v->v.i); */
return n->v->v.i;
}
@@ -1009,7 +1009,7 @@ static float _find_config_float(const void *start, node_lookup_fn find,
const struct dm_config_node *n = find(start, path);
if (n && n->v && n->v->type == DM_CFG_FLOAT) {
- log_very_verbose("Setting %s to %f", path, n->v->v.f);
+ /* log_very_verbose("Setting %s to %f", path, n->v->v.f); */
return n->v->v.f;
}
@@ -1058,12 +1058,12 @@ static int _find_config_bool(const void *start, node_lookup_fn find,
switch (v->type) {
case DM_CFG_INT:
b = v->v.i ? 1 : 0;
- log_very_verbose("Setting %s to %d", path, b);
+ /* log_very_verbose("Setting %s to %d", path, b); */
return b;
case DM_CFG_STRING:
b = _str_to_bool(v->v.str, fail);
- log_very_verbose("Setting %s to %d", path, b);
+ /* log_very_verbose("Setting %s to %d", path, b); */
return b;
default:
;
diff --git a/liblvm/lvm_vg.c b/liblvm/lvm_vg.c
index 5593579..0678bdc 100644
--- a/liblvm/lvm_vg.c
+++ b/liblvm/lvm_vg.c
@@ -512,7 +512,6 @@ int lvm_scan(lvm_t libh)
int rc = 0;
struct saved_env e = store_user_env((struct cmd_context *)libh);
- lvmcache_force_next_label_scan();
if (!lvmcache_label_scan((struct cmd_context *)libh))
rc = -1;
diff --git a/tools/toollib.c b/tools/toollib.c
index fabf2dc..e887f65 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -1485,6 +1485,12 @@ int change_tag(struct cmd_context *cmd, struct volume_group *vg,
return 1;
}
+/*
+ * FIXME: replace process_each_label() with process_each_vg() which is
+ * based on performing vg_read(), which provides a correct representation
+ * of VGs/PVs, that is not provided by lvmcache_label_scan().
+ */
+
int process_each_label(struct cmd_context *cmd, int argc, char **argv,
struct processing_handle *handle,
process_single_label_fn_t process_single_label)
diff --git a/tools/vgcreate.c b/tools/vgcreate.c
index 87a296f..0c4c428 100644
--- a/tools/vgcreate.c
+++ b/tools/vgcreate.c
@@ -82,7 +82,6 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
return ECMD_FAILED;
}
- lvmcache_force_next_label_scan();
lvmcache_label_scan(cmd); /* Does nothing when using lvmetad. */
lvmcache_seed_infos_from_lvmetad(cmd); /* Does nothing unless using lvmetad. */
diff --git a/tools/vgmerge.c b/tools/vgmerge.c
index 013bef4..67c3498 100644
--- a/tools/vgmerge.c
+++ b/tools/vgmerge.c
@@ -72,6 +72,9 @@ static int _vgmerge_single(struct cmd_context *cmd, const char *vg_name_to,
return ECMD_FAILED;
}
+ lvmcache_label_scan(cmd);
+ lvmcache_seed_infos_from_lvmetad(cmd);
+
if (strcmp(vg_name_to, vg_name_from) > 0)
lock_vg_from_first = 1;
diff --git a/tools/vgsplit.c b/tools/vgsplit.c
index 46c8911..2d39111 100644
--- a/tools/vgsplit.c
+++ b/tools/vgsplit.c
@@ -581,6 +581,9 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
return ECMD_FAILED;
}
+ lvmcache_label_scan(cmd);
+ lvmcache_seed_infos_from_lvmetad(cmd);
+
if (strcmp(vg_name_to, vg_name_from) < 0)
lock_vg_from_first = 0;
6 years
- 1
- 0
master - independent metadata areas: fix bogus code
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=098c843c50cdcc2e4f4...
Commit: 098c843c50cdcc2e4f4162037e1ff5975624f3e2
Parent: d9ef9eb330bdc66dd6d9b45713d5c0b25d645ac0
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Oct 25 13:55:22 2017 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Apr 20 11:21:41 2018 -0500
independent metadata areas: fix bogus code
Fix mixing bitwise & and logical && which was
always 1 in any case.
---
lib/format_text/format-text.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c
index 8c12c82..9538080 100644
--- a/lib/format_text/format-text.c
+++ b/lib/format_text/format-text.c
@@ -1362,7 +1362,9 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu
static int _text_scan(const struct format_type *fmt, const char *vgname)
{
- return (_scan_file(fmt, vgname) & _scan_raw(fmt, vgname));
+ _scan_file(fmt, vgname);
+ _scan_raw(fmt, vgname);
+ return 1;
}
struct _write_single_mda_baton {
6 years
- 1
- 0