lvm2-commits April 2018

lvm2-commits@lists.fedorahosted.org

5 participants
238 discussions

master - liblvm2app: missed the addition of lvmcache_label_scan
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=c42a18d372f314aa51d9d… 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 */

1 0

master - scan: skip device rescan in vg_read
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=aee27dc7bad5734012885… 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 */

1 0

master - lvmpolld: update to use new scanning correctly
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=7b0a8f47be7df13aab055… 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)

1 0

master - bcache: intercept test mode before write
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=aa833bdd8aa6a3e3557b1… 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;

1 0

master - lvmcache: simplify
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=9b6a62f9445b104f8b4f1… 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;

1 0

master - dev_cache: clean up scan
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=c0973e70a58e7e14e9cca… Commit: c0973e70a58e7e14e9cca29a0f8ad12719ea554f Parent: 89c65d4f71e51c2db4fcba176546d2474e3451bd Author: David Teigland <teigland(a)redhat.com> AuthorDate: Mon Apr 9 11:13:43 2018 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:48 2018 -0500 dev_cache: clean up scan Pull out all of the twisted logic and simply call dev_cache_scan at the start of the command prior to label scan. --- daemons/clvmd/lvm-functions.c | 1 - lib/cache/lvmcache.c | 5 +-- lib/commands/toolcontext.c | 1 - lib/device/dev-cache.c | 40 ++++---------------------------------- lib/device/dev-cache.h | 6 +--- lib/filters/filter-persistent.c | 13 ------------ lib/label/label.c | 2 +- lib/misc/lvm-globals.c | 11 ---------- lib/misc/lvm-globals.h | 2 - tools/lvmcmdline.c | 1 - tools/polldaemon.c | 9 +++---- tools/pvscan.c | 4 +- tools/toollib.c | 2 - tools/vgimportclone.c | 1 - 14 files changed, 16 insertions(+), 82 deletions(-) diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c index 64bdab3..c278692 100644 --- a/daemons/clvmd/lvm-functions.c +++ b/daemons/clvmd/lvm-functions.c @@ -661,7 +661,6 @@ int do_refresh_cache(void) return -1; } - init_full_scan_done(0); init_ignore_suspended_devices(1); lvmcache_force_next_label_scan(); lvmcache_label_scan(cmd); diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 78665bf..ef180b9 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -157,6 +157,8 @@ void lvmcache_seed_infos_from_lvmetad(struct cmd_context *cmd) if (!lvmetad_used() || _has_scanned) return; + dev_cache_scan(); + if (!lvmetad_pv_list_to_lvmcache(cmd)) { stack; return; @@ -357,9 +359,6 @@ void lvmcache_drop_metadata(const char *vgname, int drop_precommitted) _drop_metadata(FMT_TEXT_ORPHAN_VG_NAME, 0); _drop_metadata(FMT_LVM1_ORPHAN_VG_NAME, 0); _drop_metadata(FMT_POOL_ORPHAN_VG_NAME, 0); - - /* Indicate that PVs could now be missing from the cache */ - init_full_scan_done(0); } else _drop_metadata(vgname, drop_precommitted); } diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index 3dc3e2d..a54f4d7 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -1648,7 +1648,6 @@ static void _init_rand(struct cmd_context *cmd) static void _init_globals(struct cmd_context *cmd) { - init_full_scan_done(0); init_mirror_in_sync(0); } diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c index e72ffd6..52edad8 100644 --- a/lib/device/dev-cache.c +++ b/lib/device/dev-cache.c @@ -1077,12 +1077,11 @@ static int _insert(const char *path, const struct stat *info, return 1; } -static void _full_scan(int dev_scan) +void dev_cache_scan(void) { struct dir_list *dl; - if (_cache.has_scanned && !dev_scan) - return; + _cache.has_scanned = 1; _insert_dirs(&_cache.dirs); @@ -1090,9 +1089,6 @@ static void _full_scan(int dev_scan) dm_list_iterate_items(dl, &_cache.files) _insert_file(dl->dir); - - _cache.has_scanned = 1; - init_full_scan_done(1); } int dev_cache_has_scanned(void) @@ -1100,14 +1096,6 @@ int dev_cache_has_scanned(void) return _cache.has_scanned; } -void dev_cache_scan(int do_scan) -{ - if (!do_scan) - _cache.has_scanned = 1; - else - _full_scan(1); -} - static int _init_preferred_names(struct cmd_context *cmd) { const struct dm_config_node *cn; @@ -1171,7 +1159,6 @@ out: int dev_cache_init(struct cmd_context *cmd) { _cache.names = NULL; - _cache.has_scanned = 0; if (!(_cache.mem = dm_pool_create("dev_cache", 10 * 1024))) return_0; @@ -1413,7 +1400,7 @@ struct device *dev_cache_get(const char *name, struct dev_filter *f) _insert(name, info_available ? &buf : NULL, 0, obtain_device_list_from_udev()); d = (struct device *) dm_hash_lookup(_cache.names, name); if (!d) { - _full_scan(0); + dev_cache_scan(); d = (struct device *) dm_hash_lookup(_cache.names, name); } } @@ -1469,7 +1456,7 @@ struct device *dev_cache_get_by_devt(dev_t dev, struct dev_filter *f) } } - _full_scan(0); + dev_cache_scan(); d = _dev_cache_seek_devt(dev); } @@ -1477,17 +1464,7 @@ struct device *dev_cache_get_by_devt(dev_t dev, struct dev_filter *f) f->passes_filter(f, d))) ? d : NULL; } -void dev_cache_full_scan(struct dev_filter *f) -{ - if (f && f->wipe) { - f->wipe(f); /* might call _full_scan(1) */ - if (!full_scan_done()) - _full_scan(1); - } else - _full_scan(1); -} - -struct dev_iter *dev_iter_create(struct dev_filter *f, int dev_scan) +struct dev_iter *dev_iter_create(struct dev_filter *f, int unused) { struct dev_iter *di = dm_malloc(sizeof(*di)); @@ -1496,13 +1473,6 @@ struct dev_iter *dev_iter_create(struct dev_filter *f, int dev_scan) return NULL; } - if (dev_scan && !trust_cache()) { - /* Flag gets reset between each command */ - if (!full_scan_done()) - dev_cache_full_scan(f); - } else - _full_scan(0); - di->current = btree_first(_cache.devices); di->filter = f; if (di->filter) diff --git a/lib/device/dev-cache.h b/lib/device/dev-cache.h index 546b1fe..4797274 100644 --- a/lib/device/dev-cache.h +++ b/lib/device/dev-cache.h @@ -46,10 +46,8 @@ int dev_cache_exit(void); */ int dev_cache_check_for_open_devices(void); -/* Trigger(1) or avoid(0) a scan */ -void dev_cache_scan(int do_scan); +void dev_cache_scan(void); int dev_cache_has_scanned(void); -void dev_cache_full_scan(struct dev_filter *f); int dev_cache_add_dir(const char *path); int dev_cache_add_loopfile(const char *path); @@ -66,7 +64,7 @@ void dev_set_preferred_name(struct dm_str_list *sl, struct device *dev); * Object for iterating through the cache. */ struct dev_iter; -struct dev_iter *dev_iter_create(struct dev_filter *f, int dev_scan); +struct dev_iter *dev_iter_create(struct dev_filter *f, int unused); void dev_iter_destroy(struct dev_iter *iter); struct device *dev_iter_get(struct dev_iter *iter); diff --git a/lib/filters/filter-persistent.c b/lib/filters/filter-persistent.c index 5bc0861..a4151c2 100644 --- a/lib/filters/filter-persistent.c +++ b/lib/filters/filter-persistent.c @@ -48,11 +48,7 @@ static void _persistent_filter_wipe(struct dev_filter *f) { struct pfilter *pf = (struct pfilter *) f->private; - log_verbose("Wiping cache of LVM-capable devices"); dm_hash_wipe(pf->devices); - - /* Trigger complete device scan */ - dev_cache_scan(1); } static int _read_array(struct pfilter *pf, struct dm_config_tree *cft, @@ -126,15 +122,6 @@ int persistent_filter_load(struct dev_filter *f, struct dm_config_tree **cft_out /* _read_array(pf, cft, "persistent_filter_cache/invalid_devices", PF_BAD_DEVICE); */ - /* Did we find anything? */ - if (dm_hash_get_num_entries(pf->devices)) { - /* We populated dev_cache ourselves */ - dev_cache_scan(0); - if (!dev_cache_index_devs()) - stack; - r = 1; - } - log_very_verbose("Loaded persistent filter cache from %s", pf->file); out: diff --git a/lib/label/label.c b/lib/label/label.c index 6c3be05..c11a040 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -631,7 +631,7 @@ int label_scan(struct cmd_context *cmd) * on it. This info will be used by the vg_read() phase of the * command. */ - dev_cache_full_scan(cmd->full_filter); + dev_cache_scan(); if (!(iter = dev_iter_create(cmd->full_filter, 0))) { log_error("Scanning failed to get devices."); diff --git a/lib/misc/lvm-globals.c b/lib/misc/lvm-globals.c index 0f384bb..9941489 100644 --- a/lib/misc/lvm-globals.c +++ b/lib/misc/lvm-globals.c @@ -28,7 +28,6 @@ static int _md_filtering = 0; static int _internal_filtering = 0; static int _fwraid_filtering = 0; static int _pvmove = 0; -static int _full_scan_done = 0; /* Restrict to one full scan during each cmd */ static int _obtain_device_list_from_udev = DEFAULT_OBTAIN_DEVICE_LIST_FROM_UDEV; static enum dev_ext_e _external_device_info_source = DEV_EXT_NONE; static int _trust_cache = 0; /* Don't scan when incomplete VGs encountered */ @@ -92,11 +91,6 @@ void init_pvmove(int level) _pvmove = level; } -void init_full_scan_done(int level) -{ - _full_scan_done = level; -} - void init_obtain_device_list_from_udev(int device_list_from_udev) { _obtain_device_list_from_udev = device_list_from_udev; @@ -253,11 +247,6 @@ int pvmove_mode(void) return _pvmove; } -int full_scan_done(void) -{ - return _full_scan_done; -} - int obtain_device_list_from_udev(void) { return _obtain_device_list_from_udev; diff --git a/lib/misc/lvm-globals.h b/lib/misc/lvm-globals.h index e23d598..b383891 100644 --- a/lib/misc/lvm-globals.h +++ b/lib/misc/lvm-globals.h @@ -29,7 +29,6 @@ void init_md_filtering(int level); void init_internal_filtering(int level); void init_fwraid_filtering(int level); void init_pvmove(int level); -void init_full_scan_done(int level); void init_external_device_info_source(enum dev_ext_e src); void init_obtain_device_list_from_udev(int device_list_from_udev); void init_trust_cache(int trustcache); @@ -63,7 +62,6 @@ int md_filtering(void); int internal_filtering(void); int fwraid_filtering(void); int pvmove_mode(void); -int full_scan_done(void); int obtain_device_list_from_udev(void); enum dev_ext_e external_device_info_source(void); int trust_cache(void); diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c index bcb2c53..c7ac4b6 100644 --- a/tools/lvmcmdline.c +++ b/tools/lvmcmdline.c @@ -2446,7 +2446,6 @@ static void _apply_current_settings(struct cmd_context *cmd) _apply_current_output_settings(cmd); init_test(cmd->current_settings.test); - init_full_scan_done(0); init_mirror_in_sync(0); init_dmeventd_monitor(DEFAULT_DMEVENTD_MONITOR); diff --git a/tools/polldaemon.c b/tools/polldaemon.c index d69284d..cf7a947 100644 --- a/tools/polldaemon.c +++ b/tools/polldaemon.c @@ -123,13 +123,12 @@ static void _nanosleep(unsigned secs, unsigned allow_zero_time) while (!nanosleep(&wtime, &wtime) && errno == EINTR) {} } -static void _sleep_and_rescan_devices(struct daemon_parms *parms) +static void _sleep_and_rescan_devices(struct cmd_context *cmd, struct daemon_parms *parms) { if (parms->interval && !parms->aborting) { dev_close_all(); _nanosleep(parms->interval, 1); - /* Devices might have changed while we slept */ - init_full_scan_done(0); + lvmcache_label_scan(cmd); } } @@ -145,7 +144,7 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id, /* Poll for completion */ while (!finished) { if (parms->wait_before_testing) - _sleep_and_rescan_devices(parms); + _sleep_and_rescan_devices(cmd, parms); /* * An ex VG lock is needed because the check can call finish_copy @@ -218,7 +217,7 @@ int wait_for_single_lv(struct cmd_context *cmd, struct poll_operation_id *id, * continue polling an LV that doesn't have a "status". */ if (!parms->wait_before_testing && !finished) - _sleep_and_rescan_devices(parms); + _sleep_and_rescan_devices(cmd, parms); } return 1; diff --git a/tools/pvscan.c b/tools/pvscan.c index ab6ea0b..1fcf606 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -288,8 +288,6 @@ static int _pvscan_autoactivate(struct cmd_context *cmd, struct pvscan_aa_params pp->refresh_all = 1; } - dev_cache_full_scan(cmd->full_filter); - ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, 0, 0, handle, _pvscan_autoactivate_single); destroy_processing_handle(cmd, handle); @@ -495,6 +493,7 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv) } if (!dm_list_empty(&single_devs)) { + dev_cache_scan(); label_scan_devs(cmd, &single_devs); dm_list_iterate_items(devl, &single_devs) { @@ -540,6 +539,7 @@ static int _pvscan_cache(struct cmd_context *cmd, int argc, char **argv) } if (!dm_list_empty(&single_devs)) { + dev_cache_scan(); label_scan_devs(cmd, &single_devs); dm_list_iterate_items(devl, &single_devs) { diff --git a/tools/toollib.c b/tools/toollib.c index 7e3b021..fabf2dc 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -5503,8 +5503,6 @@ int pvcreate_each_device(struct cmd_context *cmd, return 0; } - dev_cache_full_scan(cmd->full_filter); - lvmcache_label_scan(cmd); /* diff --git a/tools/vgimportclone.c b/tools/vgimportclone.c index 146d3b4..224d0b5 100644 --- a/tools/vgimportclone.c +++ b/tools/vgimportclone.c @@ -332,7 +332,6 @@ retry_name: dm_list_iterate_items(vd, &vp.arg_import) internal_filter_allow(cmd->mem, vd->dev); lvmcache_destroy(cmd, 1, 0); - dev_cache_full_scan(cmd->full_filter); log_debug("Changing VG %s to %s.", vp.old_vgname, vp.new_vgname);

1 0

master - remove unnecessary REQUIRES_FULL_LABEL_SCAN
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=89c65d4f71e51c2db4fcb… Commit: 89c65d4f71e51c2db4fcba176546d2474e3451bd Parent: 45e5e702c1d488df4898a41db0c00ead63c5f6ee Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Apr 6 13:18:03 2018 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:48 2018 -0500 remove unnecessary REQUIRES_FULL_LABEL_SCAN we always scan all devices --- tools/command.c | 3 +-- tools/commands.h | 2 +- tools/lvmcmdline.c | 7 +------ tools/tools.h | 7 ++----- tools/vgrename.c | 5 ----- 5 files changed, 5 insertions(+), 19 deletions(-) diff --git a/tools/command.c b/tools/command.c index 8944399..f3b5d82 100644 --- a/tools/command.c +++ b/tools/command.c @@ -134,13 +134,12 @@ static inline int configtype_arg(struct cmd_context *cmd __attribute__((unused)) #define ALLOW_UUID_AS_NAME 0x00000010 #define LOCKD_VG_SH 0x00000020 #define NO_METADATA_PROCESSING 0x00000040 -#define REQUIRES_FULL_LABEL_SCAN 0x00000080 +#define IGNORE_PERSISTENT_FILTER 0x00000080 #define MUST_USE_ALL_ARGS 0x00000100 #define NO_LVMETAD_AUTOSCAN 0x00000200 #define ENABLE_DUPLICATE_DEVS 0x00000400 #define DISALLOW_TAG_ARGS 0x00000800 #define GET_VGNAME_FROM_OPTIONS 0x00001000 -#define IGNORE_PERSISTENT_FILTER 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 cbd527b..3d142c3 100644 --- a/tools/commands.h +++ b/tools/commands.h @@ -225,7 +225,7 @@ xx(vgremove, xx(vgrename, "Rename a volume group", - ALLOW_UUID_AS_NAME | REQUIRES_FULL_LABEL_SCAN) + ALLOW_UUID_AS_NAME) xx(vgs, "Display information about volume groups", diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c index 3774014..bcb2c53 100644 --- a/tools/lvmcmdline.c +++ b/tools/lvmcmdline.c @@ -2727,11 +2727,6 @@ static int _cmd_no_lvmetad_autoscan(struct cmd_context *cmd) return cmd->cname->flags & NO_LVMETAD_AUTOSCAN; } -static int _cmd_requires_full_label_scan(struct cmd_context *cmd) -{ - return cmd->cname->flags & REQUIRES_FULL_LABEL_SCAN; -} - static int _cmd_ignores_persistent_filter(struct cmd_context *cmd) { return cmd->cname->flags & IGNORE_PERSISTENT_FILTER; @@ -2865,7 +2860,7 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv) * Similarly ignore the persistent cache if the command is going to discard it regardless. */ if (!cmd->initialized.filters && !_cmd_no_meta_proc(cmd) && - !init_filters(cmd, !(refresh_done || _cmd_requires_full_label_scan(cmd) || _cmd_ignores_persistent_filter(cmd)))) + !init_filters(cmd, !(refresh_done || _cmd_ignores_persistent_filter(cmd)))) return_ECMD_FAILED; if (arg_is_set(cmd, readonly_ARG)) diff --git a/tools/tools.h b/tools/tools.h index 0886551..d4d2fb2 100644 --- a/tools/tools.h +++ b/tools/tools.h @@ -124,8 +124,8 @@ struct arg_value_group_list { #define LOCKD_VG_SH 0x00000020 /* Command does not process any metadata. */ #define NO_METADATA_PROCESSING 0x00000040 -/* Command wants to scan for new devices and force labels to be read from them all. */ -#define REQUIRES_FULL_LABEL_SCAN 0x00000080 +/* Command must not load the contents saved by the persistent filter */ +#define IGNORE_PERSISTENT_FILTER 0x00000080 /* Command must use all specified arg names and fail if all cannot be used. */ #define MUST_USE_ALL_ARGS 0x00000100 /* Command wants to control the device scan for lvmetad itself. */ @@ -136,9 +136,6 @@ 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 -/* Command must not load the contents saved by the persistent filter */ -#define IGNORE_PERSISTENT_FILTER 0x00002000 - void usage(const char *name); /* the argument verify/normalise functions */ diff --git a/tools/vgrename.c b/tools/vgrename.c index 5c69faf..4f2a08b 100644 --- a/tools/vgrename.c +++ b/tools/vgrename.c @@ -59,11 +59,6 @@ static int _vgrename_single(struct cmd_context *cmd, const char *vg_name, /* * Check if a VG already exists with the new VG name. * - * When not using lvmetad, it's essential that a full scan has - * been done to ensure we see all existing VG names, so we - * do not use an existing name. This has been done by - * process_each_vg REQUIRES_FULL_LABEL_SCAN. - * * (FIXME: We could look for the new name in the list of all * VGs that process_each_vg created, but we don't have access * to that list here, so we have to look in lvmcache.

1 0

master - scan: improve io error checking and reporting
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=45e5e702c1d488df4898a… Commit: 45e5e702c1d488df4898a41db0c00ead63c5f6ee Parent: 6d05859862cebe79981557fe1a1005a530302f70 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Apr 6 13:12:26 2018 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:48 2018 -0500 scan: improve io error checking and reporting --- lib/label/label.c | 30 +++++++++++++++++++----------- 1 files changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/label/label.c b/lib/label/label.c index 14a7e90..6c3be05 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -466,17 +466,20 @@ static int _scan_list(struct dm_list *devs, int *failed) struct dm_list done_devs; struct device_list *devl, *devl2; struct block *bb; + int scan_open_errors = 0; + int scan_read_errors = 0; + int scan_process_errors = 0; int scan_failed_count = 0; - int scan_lvm_count = 0; int rem_prefetches; int scan_failed; int is_lvm_device; + int error; int ret; dm_list_init(&wait_devs); dm_list_init(&done_devs); - log_debug_devs("Scanning %d devices.", dm_list_size(devs)); + log_debug_devs("Scanning %d devices for VG info", dm_list_size(devs)); scan_more: rem_prefetches = bcache_max_prefetches(scan_bcache); @@ -498,6 +501,7 @@ static int _scan_list(struct dm_list *devs, int *failed) log_debug_devs("Scan failed to open %s.", dev_name(devl->dev)); dm_list_del(&devl->list); dm_list_add(&done_devs, &devl->list); + scan_open_errors++; scan_failed_count++; continue; } @@ -513,11 +517,15 @@ static int _scan_list(struct dm_list *devs, int *failed) dm_list_iterate_items_safe(devl, devl2, &wait_devs) { bb = NULL; + error = 0; + scan_failed = 0; + is_lvm_device = 0; - if (!bcache_get(scan_bcache, devl->dev->bcache_fd, 0, 0, &bb)) { - log_debug_devs("Scan failed to read %s.", dev_name(devl->dev)); - scan_failed_count++; + if (!bcache_get(scan_bcache, devl->dev->bcache_fd, 0, 0, &bb, &error)) { + log_debug_devs("Scan failed to read %s error %d.", dev_name(devl->dev), error); scan_failed = 1; + scan_read_errors++; + scan_failed_count++; lvmcache_del_dev(devl->dev); } else { log_debug_devs("Processing data from device %s fd %d block %p", dev_name(devl->dev), devl->dev->bcache_fd, bb); @@ -526,12 +534,10 @@ static int _scan_list(struct dm_list *devs, int *failed) if (!ret && is_lvm_device) { log_debug_devs("Scan failed to process %s", dev_name(devl->dev)); - scan_failed_count++; scan_failed = 1; + scan_process_errors++; + scan_failed_count++; lvmcache_del_dev(devl->dev); - } else { - scan_lvm_count++; - scan_failed = 0; } } @@ -556,8 +562,8 @@ static int _scan_list(struct dm_list *devs, int *failed) if (!dm_list_empty(devs)) goto scan_more; - log_debug_devs("Scanned devices: %d lvm, %d failed.", - scan_lvm_count, scan_failed_count); + log_debug_devs("Scanned devices: open errors %d read errors %d process errors %d", + scan_open_errors, scan_read_errors, scan_process_errors); if (failed) *failed = scan_failed_count; @@ -649,6 +655,8 @@ int label_scan(struct cmd_context *cmd) }; dev_iter_destroy(iter); + log_debug_devs("Found %d devices to scan", dm_list_size(&all_devs)); + if (!scan_bcache) { /* * FIXME: there should probably be some max number of

1 0

master - bcache: let caller see an error
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=6d05859862cebe7998155… Commit: 6d05859862cebe79981557fe1a1005a530302f70 Parent: ae21305ee7fa60edbd85824d83ce554989161189 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Apr 6 13:11:39 2018 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:48 2018 -0500 bcache: let caller see an error --- lib/device/bcache.c | 21 ++++++++++++++------- lib/device/bcache.h | 9 ++++++++- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/lib/device/bcache.c b/lib/device/bcache.c index 9034776..ea8f702 100644 --- a/lib/device/bcache.c +++ b/lib/device/bcache.c @@ -234,7 +234,6 @@ static bool _async_wait(struct io_engine *ioe, io_complete_fn fn) fn((void *) cb->context, 0); } else { - log_warn("short io"); fn(cb->context, -ENODATA); } @@ -538,10 +537,8 @@ static void _complete_io(void *context, int err) */ dm_list_del(&b->list); - if (b->error) { - log_warn("bcache io error %d fd %d", b->error, b->fd); + if (b->error) memset(b->data, 0, cache->block_sectors << SECTOR_SHIFT); - } /* Things don't work with this block of code, but work without it. */ #if 0 @@ -583,6 +580,7 @@ static void _issue_low_level(struct block *b, enum dir d) dm_list_add(&cache->io_pending, &b->list); if (!cache->engine->issue(cache->engine, d, b->fd, sb, se, b->data, b)) { + /* FIXME: if io_submit() set an errno, return that instead of EIO? */ _complete_io(b, -EIO); return; } @@ -904,7 +902,7 @@ void bcache_prefetch(struct bcache *cache, int fd, block_address index) } bool bcache_get(struct bcache *cache, int fd, block_address index, - unsigned flags, struct block **result) + unsigned flags, struct block **result, int *error) { struct block *b; @@ -916,12 +914,19 @@ bool bcache_get(struct bcache *cache, int fd, block_address index, *result = b; + if (error) + *error = b->error; + if (b->error) return false; return true; } *result = NULL; + + if (error) + *error = -BCACHE_NO_BLOCK; + log_error("bcache failed to get block %u fd %d", (uint32_t)index, fd); return false; } @@ -1068,7 +1073,9 @@ bool bcache_read_bytes(struct bcache *cache, int fd, off_t start, size_t len, vo bcache_prefetch(cache, fd, i); for (i = bb; i < be; i++) { - if (!bcache_get(cache, fd, i, 0, &b)) { + if (!bcache_get(cache, fd, i, 0, &b, NULL)) { + log_error("bcache_read failed to get block %u fd %d bb %u be %u", + (uint32_t)i, fd, (uint32_t)bb, (uint32_t)be); errors++; continue; } @@ -1105,7 +1112,7 @@ bool bcache_write_bytes(struct bcache *cache, int fd, off_t start, size_t len, v bcache_prefetch(cache, fd, i); for (i = bb; i < be; i++) { - if (!bcache_get(cache, fd, i, 0, &b)) { + if (!bcache_get(cache, fd, i, 0, &b, NULL)) { log_error("bcache_write failed to get block %u fd %d bb %u be %u", (uint32_t)i, fd, (uint32_t)bb, (uint32_t)be); errors++; diff --git a/lib/device/bcache.h b/lib/device/bcache.h index d5f6d0a..999223a 100644 --- a/lib/device/bcache.h +++ b/lib/device/bcache.h @@ -29,6 +29,13 @@ /*----------------------------------------------------------------*/ +/* + * bcache-specific error numbers + * These supplement standard -EXXX error numbers and + * should not overlap. + */ +#define BCACHE_NO_BLOCK 201 + enum dir { DIR_READ, DIR_WRITE @@ -120,7 +127,7 @@ void bcache_prefetch(struct bcache *cache, int fd, block_address index); * Returns true on success. */ bool bcache_get(struct bcache *cache, int fd, block_address index, - unsigned flags, struct block **result); + unsigned flags, struct block **result, int *error); void bcache_put(struct block *b); /*

1 0

master - scan: drop bcache between lvm shell commands
by David Teigland 23 Apr '18

23 Apr '18

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=ae21305ee7fa60edbd858… Commit: ae21305ee7fa60edbd85824d83ce554989161189 Parent: a01a8d71723740577e5fe9635412940d941b5da9 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Apr 6 13:05:17 2018 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:48 2018 -0500 scan: drop bcache between lvm shell commands A running lvm shell keeps all lvm devices open unless the bcache is dropped. --- lib/cache/lvmcache.c | 22 ++++++++++++++++++++-- tools/lvmcmdline.c | 7 +++---- tools/toollib.c | 1 - 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 6479080..78665bf 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -406,7 +406,7 @@ int lvmcache_verify_lock_order(const char *vgname) return 1; if (!_lock_hash) - return_0; + return 1; dm_hash_iterate(n, _lock_hash) { if (!dm_hash_get_data(_lock_hash, n)) @@ -836,6 +836,9 @@ static int _label_scan_invalid(struct cmd_context *cmd) 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); @@ -1236,8 +1239,10 @@ int lvmcache_label_scan(struct cmd_context *cmd) struct dm_list del_cache_devs; struct dm_list add_cache_devs; struct lvmcache_info *info; + struct lvmcache_vginfo *vginfo; struct device_list *devl; struct format_type *fmt; + int vginfo_count = 0; int r = 0; @@ -1247,6 +1252,8 @@ int lvmcache_label_scan(struct cmd_context *cmd) return 1; } + log_debug_cache("Finding VG info"); + /* Avoid recursion when a PVID can't be found! */ if (_scanning_in_progress) return 0; @@ -1315,6 +1322,8 @@ int lvmcache_label_scan(struct cmd_context *cmd) dm_list_init(&del_cache_devs); dm_list_init(&add_cache_devs); + log_debug_cache("Resolving duplicate devices"); + _choose_preferred_devs(cmd, &del_cache_devs, &add_cache_devs); dm_list_iterate_items(devl, &del_cache_devs) { @@ -1354,6 +1363,14 @@ int lvmcache_label_scan(struct cmd_context *cmd) _scanning_in_progress = 0; _force_label_scan = 0; + dm_list_iterate_items(vginfo, &_vginfos) { + if (is_orphan_vg(vginfo->vgname)) + continue; + vginfo_count++; + } + + log_debug_cache("Found VG info for %d VGs", vginfo_count); + return r; } @@ -2291,7 +2308,8 @@ static void _lvmcache_destroy_lockname(struct dm_hash_node *n) void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset) { struct dm_hash_node *n; - log_verbose("Wiping internal VG cache"); + + log_debug_cache("Dropping VG info"); _has_scanned = 0; diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c index fc96b8d..3774014 100644 --- a/tools/lvmcmdline.c +++ b/tools/lvmcmdline.c @@ -3013,10 +3013,9 @@ int lvm_run_command(struct cmd_context *cmd, int argc, char **argv) lvmnotify_send(cmd); out: - if (test_mode()) { - log_verbose("Test mode: Wiping internal cache"); - lvmcache_destroy(cmd, 1, 0); - } + + lvmcache_destroy(cmd, 1, 1); + label_scan_destroy(cmd); if ((config_string_cft = remove_config_tree_by_source(cmd, CONFIG_STRING))) dm_config_destroy(config_string_cft); diff --git a/tools/toollib.c b/tools/toollib.c index c8dd9d3..7e3b021 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -4508,7 +4508,6 @@ int process_each_pv(struct cmd_context *cmd, * before process_each_pv is called. */ if (!trust_cache() && !orphans_locked) { - log_debug("Scanning for available devices"); lvmcache_destroy(cmd, 1, 0); /*

1 0

← Newer
1
2
3
4
5
6
7
8
9
...
24
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits April 2018