lvm2-commits June 2019

lvm2-commits@lists.fedorahosted.org

3 participants
101 discussions

master - move pv header repairs to vg_write
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=de3d3b11f493a89ebee2e… Commit: de3d3b11f493a89ebee2eab7efae70d33face954 Parent: 89914a541f081e1c234a455c17d82269cd90364b Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Feb 6 13:18:45 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 move pv header repairs to vg_write Correct PV header in-use or version fields from vg_write instead of vg_read. --- lib/format_text/format-text.c | 14 +++- lib/metadata/metadata.c | 168 +++++----------------------------------- 2 files changed, 34 insertions(+), 148 deletions(-) diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index 6e224f0..4f0a004 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -1600,12 +1600,16 @@ static int _text_pv_needs_rewrite(const struct format_type *fmt, struct physical { struct lvmcache_info *info; uint32_t ext_vsn; + uint32_t ext_flags; *needs_rewrite = 0; if (!pv->is_labelled) return 1; + if (!pv->dev) + return 1; + if (!(info = lvmcache_info_from_pvid((const char *)&pv->id, pv->dev, 0))) { log_error("Failed to find cached info for PV %s.", pv_dev_name(pv)); return 0; @@ -1613,8 +1617,16 @@ static int _text_pv_needs_rewrite(const struct format_type *fmt, struct physical ext_vsn = lvmcache_ext_version(info); - if (ext_vsn < PV_HEADER_EXTENSION_VSN) + if (ext_vsn < PV_HEADER_EXTENSION_VSN) { + log_debug("PV %s header needs rewrite for new ext version", dev_name(pv->dev)); + *needs_rewrite = 1; + } + + ext_flags = lvmcache_ext_flags(info); + if (!(ext_flags & PV_EXT_USED)) { + log_debug("PV %s header needs rewrite to set ext used", dev_name(pv->dev)); *needs_rewrite = 1; + } return 1; } diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index a13be57..804d0cf 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -2810,57 +2810,6 @@ static int _pv_in_pv_list(struct physical_volume *pv, struct dm_list *head) return 0; } -/* - * Check if any of the PVs in VG still contain old PV headers - * and if yes, schedule them for PV header update. - */ -static int _vg_update_old_pv_ext_if_needed(struct volume_group *vg) -{ - struct pv_list *pvl, *new_pvl; - int pv_needs_rewrite; - - if (!(vg->fid->fmt->features & FMT_PV_FLAGS)) - return 1; - - dm_list_iterate_items(pvl, &vg->pvs) { - if (is_missing_pv(pvl->pv) || - !pvl->pv->fmt->ops->pv_needs_rewrite) - continue; - - if (_pv_in_pv_list(pvl->pv, &vg->pv_write_list)) - continue; - - if (!pvl->pv->fmt->ops->pv_needs_rewrite(pvl->pv->fmt, pvl->pv, - &pv_needs_rewrite)) - return_0; - - if (pv_needs_rewrite) { - /* - * Schedule PV for writing only once! - */ - if (_pv_in_pv_list(pvl->pv, &vg->pv_write_list)) - continue; - - if (!(new_pvl = dm_pool_zalloc(vg->vgmem, sizeof(*new_pvl)))) { - log_error("pv_to_write allocation for '%s' failed", pv_dev_name(pvl->pv)); - return 0; - } - new_pvl->pv = pvl->pv; - dm_list_add(&vg->pv_write_list, &new_pvl->list); - log_debug("PV %s has old extension header, updating to newest version.", - pv_dev_name(pvl->pv)); - } - } - - if (!dm_list_empty(&vg->pv_write_list) && - (!vg_write(vg) || !vg_commit(vg))) { - log_error("Failed to update old PV extension headers in VG %s.", vg->name); - return 0; - } - - return 1; -} - static int _check_historical_lv_is_valid(struct historical_logical_volume *hlv) { struct glv_list *glvl; @@ -2995,7 +2944,7 @@ static void _wipe_outdated_pvs(struct cmd_context *cmd, struct volume_group *vg) int vg_write(struct volume_group *vg) { struct dm_list *mdah; - struct pv_list *pvl, *pvl_safe; + struct pv_list *pvl, *pvl_safe, *new_pvl; struct metadata_area *mda; struct lv_list *lvl; int revert = 0, wrote = 0; @@ -3063,6 +3012,26 @@ int vg_write(struct volume_group *vg) memlock_unlock(vg->cmd); vg->seqno++; + dm_list_iterate_items(pvl, &vg->pvs) { + int update_pv_header = 0; + + if (_pv_in_pv_list(pvl->pv, &vg->pv_write_list)) + continue; + + if (!pvl->pv->fmt->ops->pv_needs_rewrite(pvl->pv->fmt, pvl->pv, &update_pv_header)) + continue; + + if (!update_pv_header) + continue; + + if (!(new_pvl = dm_pool_zalloc(vg->vgmem, sizeof(*new_pvl)))) + continue; + + new_pvl->pv = pvl->pv; + dm_list_add(&vg->pv_write_list, &new_pvl->list); + log_warn("WARNING: updating PV header on %s for VG %s.", pv_dev_name(pvl->pv), vg->name); + } + dm_list_iterate_items_safe(pvl, pvl_safe, &vg->pv_write_list) { if (!pv_write(vg->cmd, pvl->pv, 1)) return_0; @@ -3607,88 +3576,6 @@ static int _repair_inconsistent_vg(struct volume_group *vg, uint32_t lockd_state return 1; } -static int _check_or_repair_pv_ext(struct cmd_context *cmd, - struct volume_group *vg, - uint32_t lockd_state, - int repair, int *inconsistent_pvs) -{ - char uuid[64] __attribute__((aligned(8))); - struct lvmcache_info *info; - uint32_t ext_version, ext_flags; - struct pv_list *pvl; - unsigned pvs_fixed = 0; - int r = 0; - - *inconsistent_pvs = 0; - - dm_list_iterate_items(pvl, &vg->pvs) { - /* Missing PV - nothing to do. */ - if (is_missing_pv(pvl->pv)) - continue; - - if (!pvl->pv->dev) { - /* is_missing_pv doesn't catch NULL dev */ - memset(&uuid, 0, sizeof(uuid)); - if (!id_write_format(&pvl->pv->id, uuid, sizeof(uuid))) - goto_out; - log_warn("WARNING: Not repairing PV %s with missing device.", uuid); - continue; - } - - if (!(info = lvmcache_info_from_pvid(pvl->pv->dev->pvid, pvl->pv->dev, 0))) { - log_error("Failed to find cached info for PV %s.", pv_dev_name(pvl->pv)); - goto out; - } - - ext_version = lvmcache_ext_version(info); - if (ext_version < 2) - continue; - - ext_flags = lvmcache_ext_flags(info); - if (!(ext_flags & PV_EXT_USED)) { - if (!repair) { - *inconsistent_pvs = 1; - /* we're not repairing now, so no need to - * check further PVs - inconsistent_pvs is already - * set and that will trigger the repair next time */ - return 1; - } - - if (_is_foreign_vg(vg)) { - log_verbose("Skip repair of PV %s that is in foreign " - "VG %s but not marked as used.", - pv_dev_name(pvl->pv), vg->name); - *inconsistent_pvs = 1; - } else if (vg_is_shared(vg) && !(lockd_state & LDST_EX)) { - log_warn("Skip repair of PV %s that is in shared " - "VG %s but not marked as used.", - pv_dev_name(pvl->pv), vg->name); - *inconsistent_pvs = 1; - } else { - log_warn("WARNING: Repairing Physical Volume %s that is " - "in Volume Group %s but not marked as used.", - pv_dev_name(pvl->pv), vg->name); - - /* pv write will set correct ext_flags */ - if (!pv_write(cmd, pvl->pv, 1)) { - *inconsistent_pvs = 1; - log_error("Failed to repair physical volume \"%s\".", - pv_dev_name(pvl->pv)); - goto out; - } - pvs_fixed++; - } - } - } - - r = 1; -out: - if ((pvs_fixed > 0) && !_repair_inconsistent_vg(vg, lockd_state)) - return_0; - - return r; -} - /* Caller sets consistent to 1 if it's safe for vg_read_internal to correct * inconsistent metadata on disk (i.e. the VG write lock is held). * This guarantees only consistent metadata is returned. @@ -3724,7 +3611,6 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, int inconsistent_mdas = 0; int inconsistent_mda_count = 0; int strip_historical_lvs = enable_repair; - int update_old_pv_ext = enable_repair; unsigned use_precommitted = precommitted; struct dm_list *pvids; struct pv_list *pvl; @@ -4258,19 +4144,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, return NULL; } - /* 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, lockd_state, skipped_rescan ? 0 : enable_repair, - &inconsistent_pvs)) { - release_vg(correct_vg); - return_NULL; - } - if (correct_vg && enable_repair && !skipped_rescan) { - if (update_old_pv_ext && !_vg_update_old_pv_ext_if_needed(correct_vg)) { - release_vg(correct_vg); - return_NULL; - } - if (strip_historical_lvs && !vg_strip_outdated_historical_lvs(correct_vg)) { release_vg(correct_vg); return_NULL;

1 0

master - process_each_pv handle outdated pvs
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=89914a541f081e1c234a4… Commit: 89914a541f081e1c234a455c17d82269cd90364b Parent: ab61a6d85da4e4f1d540e40ebac77999bf3c9a57 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Feb 6 13:00:33 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 process_each_pv handle outdated pvs process_each_pv should account for outdated pvs in the list of all devices it is processing. --- tools/toollib.c | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-) diff --git a/tools/toollib.c b/tools/toollib.c index 8e882e9..78b1745 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -4156,12 +4156,16 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, struct physical_volume *pv; struct pv_list *pvl; struct device_id_list *dil; + struct device_list *devl; + struct dm_list outdated_devs; const char *pv_name; int process_pv; int do_report_ret_code = 1; int ret_max = ECMD_PROCESSED; int ret = 0; + dm_list_init(&outdated_devs); + log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_PV); vg_uuid[0] = '\0'; @@ -4247,6 +4251,12 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, break; log_set_report_object_name_and_id(NULL, NULL); } + + if (!is_orphan_vg(vg->name)) + lvmcache_get_outdated_devs(cmd, vg->name, (const char *)&vg->id, &outdated_devs); + dm_list_iterate_items(devl, &outdated_devs) + _device_list_remove(all_devices, devl->dev); + do_report_ret_code = 0; out: if (do_report_ret_code)

1 0

master - move wipe_outdated_pvs to vg_write
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=ab61a6d85da4e4f1d540e… Commit: ab61a6d85da4e4f1d540e40ebac77999bf3c9a57 Parent: 45b164f62cae25da3ac4b9f0d4f87ecaee1575fa Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Feb 6 12:32:26 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 move wipe_outdated_pvs to vg_write and implement it based on a device, not based on a pv struct (which is not available when the device is not a part of the vg.) currently only the vgremove command wipes outdated pvs until more advanced recovery is added in a subsequent commit --- lib/commands/toolcontext.h | 1 + lib/format_text/format-text.c | 33 +++++++++++ lib/format_text/format-text.h | 3 + lib/metadata/metadata.c | 123 +++++++++++++++++++++++------------------ tools/vgremove.c | 2 + 5 files changed, 109 insertions(+), 53 deletions(-) diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h index 7d373ab..6e4530c 100644 --- a/lib/commands/toolcontext.h +++ b/lib/commands/toolcontext.h @@ -179,6 +179,7 @@ struct cmd_context { unsigned use_hints:1; /* if hints are enabled this cmd can use them */ unsigned pvscan_recreate_hints:1; /* enable special case hint handling for pvscan --cache */ unsigned scan_lvs:1; + unsigned wipe_outdated_pvs:1; /* * Devices and filtering. diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index 9d58c53..6e224f0 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -2463,3 +2463,36 @@ bad: return NULL; } + +int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev, + struct metadata_area *mda) +{ + struct mda_context *mdac = mda->metadata_locn; + uint64_t start_byte = mdac->area.start; + struct mda_header *mdab; + struct raw_locn *rlocn_slot0; + struct raw_locn *rlocn_slot1; + uint32_t bad_fields = 0; + + if (!(mdab = raw_read_mda_header(cmd->fmt, &mdac->area, mda_is_primary(mda), 0, &bad_fields))) { + log_error("Failed to read outdated pv mda header on %s", dev_name(dev)); + return 0; + } + + rlocn_slot0 = &mdab->raw_locns[0]; + rlocn_slot1 = &mdab->raw_locns[1]; + + rlocn_slot0->offset = 0; + rlocn_slot0->size = 0; + rlocn_slot0->checksum = 0; + rlocn_slot1->offset = 0; + rlocn_slot1->size = 0; + rlocn_slot1->checksum = 0; + + if (!_raw_write_mda_header(cmd->fmt, dev, mda_is_primary(mda), start_byte, mdab)) { + log_error("Failed to write outdated pv mda header on %s", dev_name(dev)); + return 0; + } + + return 1; +} diff --git a/lib/format_text/format-text.h b/lib/format_text/format-text.h index c42d5c0..2345d52 100644 --- a/lib/format_text/format-text.h +++ b/lib/format_text/format-text.h @@ -77,4 +77,7 @@ struct data_area_list { struct disk_locn disk_locn; }; +int text_wipe_outdated_pv_mda(struct cmd_context *cmd, struct device *dev, + struct metadata_area *mda); + #endif diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index bd6ec4d..a13be57 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -28,11 +28,14 @@ #include "lib/display/display.h" #include "lib/locking/locking.h" #include "lib/format_text/archiver.h" +#include "lib/format_text/format-text.h" +#include "lib/format_text/layout.h" +#include "lib/format_text/import-export.h" #include "lib/config/defaults.h" #include "lib/locking/lvmlockd.h" -#include "time.h" #include "lib/notify/lvmnotify.h" +#include <time.h> #include <math.h> static struct physical_volume *_pv_read(struct cmd_context *cmd, @@ -2922,6 +2925,69 @@ static int _handle_historical_lvs(struct volume_group *vg) return 1; } +static void _wipe_outdated_pvs(struct cmd_context *cmd, struct volume_group *vg) +{ + struct dm_list devs; + struct dm_list *mdas = NULL; + struct device_list *devl; + struct device *dev; + struct metadata_area *mda; + struct label *label; + struct lvmcache_info *info; + uint32_t ext_flags; + + dm_list_init(&devs); + + /* + * When vg_read selected a good copy of the metadata, it used it to + * update the lvmcache representation of the VG (lvmcache_update_vg). + * At that point outdated PVs were recognized and moved into the + * vginfo->outdated_infos list. Here we clear the PVs on that list. + */ + + lvmcache_get_outdated_devs(cmd, vg->name, (const char *)&vg->id, &devs); + + dm_list_iterate_items(devl, &devs) { + dev = devl->dev; + + lvmcache_get_outdated_mdas(cmd, vg->name, (const char *)&vg->id, dev, &mdas); + + if (mdas) { + dm_list_iterate_items(mda, mdas) { + log_warn("WARNING: wiping mda on outdated PV %s", dev_name(dev)); + + if (!text_wipe_outdated_pv_mda(cmd, dev, mda)) + log_warn("WARNING: failed to wipe mda on outdated PV %s", dev_name(dev)); + } + } + + if (!(label = lvmcache_get_dev_label(dev))) { + log_error("_wipe_outdated_pvs no label for %s", dev_name(dev)); + continue; + } + + info = label->info; + ext_flags = lvmcache_ext_flags(info); + ext_flags &= ~PV_EXT_USED; + lvmcache_set_ext_version(info, PV_HEADER_EXTENSION_VSN); + lvmcache_set_ext_flags(info, ext_flags); + + log_warn("WARNING: wiping header on outdated PV %s", dev_name(dev)); + + if (!label_write(dev, label)) + log_warn("WARNING: failed to wipe header on outdated PV %s", dev_name(dev)); + + lvmcache_del(info); + } + + /* + * A vgremove will involve many vg_write() calls (one for each lv + * removed) but we only need to wipe pvs once, so clear the outdated + * list so it won't be wiped again. + */ + lvmcache_del_outdated_devs(cmd, vg->name, (const char *)&vg->id); +} + /* * After vg_write() returns success, * caller MUST call either vg_commit() or vg_revert() @@ -2986,6 +3052,9 @@ int vg_write(struct volume_group *vg) return 0; } + if (vg->cmd->wipe_outdated_pvs) + _wipe_outdated_pvs(vg->cmd, vg); + if (critical_section()) log_error(INTERNAL_ERROR "Writing metadata in critical section."); @@ -3538,52 +3607,6 @@ static int _repair_inconsistent_vg(struct volume_group *vg, uint32_t lockd_state return 1; } -static int _wipe_outdated_pvs(struct cmd_context *cmd, struct volume_group *vg, struct dm_list *to_check, uint32_t lockd_state) -{ - struct pv_list *pvl, *pvl2; - char uuid[64] __attribute__((aligned(8))); - - if (lvmcache_found_duplicate_pvs()) { - log_debug_metadata("Skip wiping outdated PVs with duplicates."); - return 0; - } - - /* - * Cannot write foreign VGs, the owner will repair it. - * Also, if another host is updating its VG, we may read - * the PVs while some are written but not others, making - * some PVs look outdated to us just because we're reading - * the VG while it's only partially written out. - */ - if (_is_foreign_vg(vg)) { - log_debug_metadata("Skip wiping outdated PVs for foreign VG."); - return 0; - } - - if (vg_is_shared(vg) && !(lockd_state & LDST_EX)) { - log_verbose("Skip wiping outdated PVs for shared VG without exclusive lock."); - return 0; - } - - dm_list_iterate_items(pvl, to_check) { - dm_list_iterate_items(pvl2, &vg->pvs) { - if (pvl->pv->dev == pvl2->pv->dev) - goto next_pv; - } - - - if (!id_write_format(&pvl->pv->id, uuid, sizeof(uuid))) - return_0; - log_warn("WARNING: Removing PV %s (%s) that no longer belongs to VG %s", - pv_dev_name(pvl->pv), uuid, vg->name); - if (!pv_write_orphan(cmd, pvl->pv)) - return_0; -next_pv: - ; - } - return 1; -} - static int _check_or_repair_pv_ext(struct cmd_context *cmd, struct volume_group *vg, uint32_t lockd_state, @@ -4217,12 +4240,6 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, release_vg(correct_vg); return NULL; } - - if (!_wipe_outdated_pvs(cmd, correct_vg, &all_pvs, lockd_state)) { - _free_pv_list(&all_pvs); - release_vg(correct_vg); - return_NULL; - } } _free_pv_list(&all_pvs); diff --git a/tools/vgremove.c b/tools/vgremove.c index 1aae87d..23640f6 100644 --- a/tools/vgremove.c +++ b/tools/vgremove.c @@ -99,6 +99,8 @@ int vgremove(struct cmd_context *cmd, int argc, char **argv) clear_hint_file(cmd); + cmd->wipe_outdated_pvs = 1; + cmd->handles_missing_pvs = 1; ret = process_each_vg(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, 0,

1 0

master - create separate lvmcache update functions for read and write
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=45b164f62cae25da3ac4b… Commit: 45b164f62cae25da3ac4b9f0d4f87ecaee1575fa Parent: 027e0e92e6edcde98fd951286c21a29f22f3ec20 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Feb 6 12:10:13 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 create separate lvmcache update functions for read and write The vg read and vg write cases need to update lvmcache differently, so create separate functions for them. The read case now handles checking for outdated mdas and moves them aside into a new list to be repaired in a subsequent commit. --- lib/cache/lvmcache.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++- lib/cache/lvmcache.h | 3 +- lib/metadata/metadata.c | 6 +- 3 files changed, 130 insertions(+), 5 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 3987173..08c0459 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -1685,7 +1685,27 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg return 1; } -int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted) +/* + * FIXME: quit trying to mirror changes that a command is making into lvmcache. + * + * First, it's complicated and hard to ensure it's done correctly in every case + * (it would be much easier and safer to just toss out what's in lvmcache and + * reread the info to recreate it from scratch instead of trying to make sure + * every possible discrete state change is correct.) + * + * Second, it's unnecessary if commands just use the vg they are modifying + * rather than also trying to get info from lvmcache. The lvmcache state + * should be populated by label_scan, used to perform vg_read's, and then + * ignored (or dropped so it can't be used). + * + * lvmcache info is already used very little after a command begins its + * operation. The code that's supposed to keep the lvmcache in sync with + * changes being made to disk could be half wrong and we wouldn't know it. + * That creates a landmine for someone who might try to use a bit of it that + * isn't being updated correctly. + */ + +int lvmcache_update_vg_from_write(struct volume_group *vg) { struct pv_list *pvl; struct lvmcache_info *info; @@ -1710,6 +1730,110 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted) } /* + * The lvmcache representation of a VG after label_scan can be incorrect + * because the label_scan does not use the full VG metadata to construct + * vginfo/info. PVs that don't hold VG metadata weren't attached to the vginfo + * during label scan, and PVs with outdated metadata (claiming to be in the VG, + * but not listed in the latest metadata) were attached to the vginfo, but + * shouldn't be. After vg_read() gets the full metdata in the form of a 'vg', + * this function is called to fix up the lvmcache representation of the VG + * using the 'vg'. + */ + +int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted) +{ + struct pv_list *pvl; + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info, *info2; + struct metadata_area *mda; + char pvid_s[ID_LEN + 1] __attribute__((aligned(8))); + struct lvmcache_vgsummary vgsummary = { + .vgname = vg->name, + .vgstatus = vg->status, + .vgid = vg->id, + .system_id = vg->system_id, + .lock_type = vg->lock_type + }; + + if (!(vginfo = lvmcache_vginfo_from_vgname(vg->name, (const char *)&vg->id))) { + log_error(INTERNAL_ERROR "lvmcache_update_vg %s no vginfo", vg->name); + return 0; + } + + /* + * The label scan doesn't know when a PV with old metadata has been + * removed from the VG. Now with the vg we can tell, so remove the + * info for a PV that has been removed from the VG with + * vgreduce --removemissing. + */ + dm_list_iterate_items_safe(info, info2, &vginfo->infos) { + int found = 0; + dm_list_iterate_items(pvl, &vg->pvs) { + if (pvl->pv->dev != info->dev) + continue; + found = 1; + break; + } + + if (found) + continue; + + log_warn("WARNING: outdated PV %s seqno %u has been removed in current VG %s seqno %u.", + dev_name(info->dev), info->summary_seqno, vg->name, vginfo->seqno); + + _drop_vginfo(info, vginfo); /* remove from vginfo->infos */ + dm_list_add(&vginfo->outdated_infos, &info->list); + } + + dm_list_iterate_items(pvl, &vg->pvs) { + (void) dm_strncpy(pvid_s, (char *) &pvl->pv->id, sizeof(pvid_s)); + + if (!(info = lvmcache_info_from_pvid(pvid_s, pvl->pv->dev, 0))) { + log_debug_cache("lvmcache_update_vg %s no info for %s %s", + vg->name, + (char *) &pvl->pv->id, + pvl->pv->dev ? dev_name(pvl->pv->dev) : "missing"); + continue; + } + + log_debug_cache("lvmcache_update_vg %s for info %s", + vg->name, dev_name(info->dev)); + + /* + * FIXME: use a different function that just attaches info's that + * had no metadata onto the correct vginfo. + * + * info's for PVs without metadata were not connected to the + * vginfo by label_scan, so do it here. + */ + if (!lvmcache_update_vgname_and_id(info, &vgsummary)) { + log_debug_cache("lvmcache_update_vg %s failed to update info for %s", + vg->name, dev_name(info->dev)); + } + + /* + * Ignored mdas were not copied from info->mdas to + * fid->metadata_areas... when create_text_instance (at the + * start of vg_read) called lvmcache_fid_add_mdas_vg because at + * that point the info's were not connected to the vginfo + * (since label_scan didn't know this without metadata.) + */ + dm_list_iterate_items(mda, &info->mdas) { + if (!mda_is_ignored(mda)) + continue; + log_debug("lvmcache_update_vg %s copy ignored mdas for %s", vg->name, dev_name(info->dev)); + if (!lvmcache_fid_add_mdas_pv(info, vg->fid)) { + log_debug_cache("lvmcache_update_vg %s failed to update mdas for %s", + vg->name, dev_name(info->dev)); + } + break; + } + } + + return 1; +} + +/* * We can see multiple different devices with the * same pvid, i.e. duplicates. * diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index d4c19f9..1921709 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -84,7 +84,8 @@ void lvmcache_del_dev(struct device *dev); /* Update things */ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vgsummary *vgsummary); -int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted); +int lvmcache_update_vg_from_read(struct volume_group *vg, unsigned precommitted); +int lvmcache_update_vg_from_write(struct volume_group *vg); void lvmcache_lock_vgname(const char *vgname, int read_only); void lvmcache_unlock_vgname(const char *vgname); diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 49c1e74..bd6ec4d 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -3098,7 +3098,7 @@ static int _vg_commit_mdas(struct volume_group *vg) /* Update cache first time we succeed */ if (!failed && !cache_updated) { - lvmcache_update_vg(vg, 0); + lvmcache_update_vg_from_write(vg); cache_updated = 1; } } @@ -3993,7 +3993,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, * If there is no precommitted metadata, committed metadata * is read and stored in the cache even if use_precommitted is set */ - lvmcache_update_vg(correct_vg, correct_vg->status & PRECOMMITTED); + lvmcache_update_vg_from_read(correct_vg, correct_vg->status & PRECOMMITTED); if (!(pvids = lvmcache_get_pvids(cmd, vgname, vgid))) { release_vg(correct_vg); @@ -4149,7 +4149,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, * If there is no precommitted metadata, committed metadata * is read and stored in the cache even if use_precommitted is set */ - lvmcache_update_vg(correct_vg, (correct_vg->status & PRECOMMITTED)); + lvmcache_update_vg_from_read(correct_vg, (correct_vg->status & PRECOMMITTED)); if (inconsistent) { /* FIXME Test should be if we're *using* precommitted metadata not if we were searching for it */

1 0

master - fix vg_commit return value
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=027e0e92e6edcde98fd95… Commit: 027e0e92e6edcde98fd951286c21a29f22f3ec20 Parent: 86d831b9165d71769cd0fc05b52bc7469760e2f0 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 5 14:02:24 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 fix vg_commit return value The existing comment was desribing the correct behavior, but the code didn't match. The commit is successful if one mda was committed. Making it depend on the result of the internal lvmcache update was wrong. --- lib/metadata/metadata.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 65da7e1..49c1e74 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -3072,6 +3072,7 @@ static int _vg_commit_mdas(struct volume_group *vg) struct metadata_area *mda, *tmda; struct dm_list ignored; int failed = 0; + int good = 0; int cache_updated = 0; /* Rearrange the metadata_areas_in_use so ignored mdas come first. */ @@ -3092,27 +3093,31 @@ static int _vg_commit_mdas(struct volume_group *vg) !mda->ops->vg_commit(vg->fid, vg, mda)) { stack; failed = 1; - } + } else + good++; + /* Update cache first time we succeed */ if (!failed && !cache_updated) { lvmcache_update_vg(vg, 0); cache_updated = 1; } } - return cache_updated; + if (good) + return 1; + return 0; } /* Commit pending changes */ int vg_commit(struct volume_group *vg) { - int cache_updated = 0; struct pv_list *pvl; + int ret; - cache_updated = _vg_commit_mdas(vg); + ret = _vg_commit_mdas(vg); set_vg_notify(vg->cmd); - if (cache_updated) { + if (ret) { /* * We need to clear old_name after a successful commit. * The volume_group structure could be reused later. @@ -3126,7 +3131,7 @@ int vg_commit(struct volume_group *vg) } /* If at least one mda commit succeeded, it was committed */ - return cache_updated; + return ret; } /* Don't commit any pending changes */

1 0

master - change args for text label read function
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=86d831b9165d71769cd0f… Commit: 86d831b9165d71769cd0fc05b52bc7469760e2f0 Parent: 889b5d3183314985d4d0930b30f0cd4b0f482f69 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 5 13:40:34 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 change args for text label read function Have the caller pass the label_sector to the read function so the read function can set the sector field in the label struct, instead of having the read function return a pointer to the label for the caller to set the sector field. Also have the read function return a flag indicating to the caller that the scanned device was identified as a duplicate pv. --- lib/cache/lvmcache.c | 14 ++++++++++---- lib/cache/lvmcache.h | 6 +++--- lib/format_text/format-text.c | 7 +++---- lib/format_text/text_label.c | 27 +++++++++++++++++++++------ lib/label/label.c | 40 ++++++++++++++++++++++++++++++++-------- lib/label/label.h | 2 +- 6 files changed, 70 insertions(+), 26 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 3e89b02..3987173 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -1760,7 +1760,7 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted) * transient duplicate? */ -static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev) +static struct lvmcache_info * _create_info(struct labeller *labeller, struct device *dev, uint64_t label_sector) { struct lvmcache_info *info; struct label *label; @@ -1773,6 +1773,9 @@ static struct lvmcache_info * _create_info(struct labeller *labeller, struct dev return NULL; } + label->dev = dev; + label->sector = label_sector; + info->dev = dev; info->fmt = labeller->fmt; @@ -1788,8 +1791,9 @@ static struct lvmcache_info * _create_info(struct labeller *labeller, struct dev } struct lvmcache_info *lvmcache_add(struct labeller *labeller, - const char *pvid, struct device *dev, - const char *vgname, const char *vgid, uint32_t vgstatus) + const char *pvid, struct device *dev, uint64_t label_sector, + const char *vgname, const char *vgid, uint32_t vgstatus, + int *is_duplicate) { char pvid_s[ID_LEN + 1] __attribute__((aligned(8))); char uuid[64] __attribute__((aligned(8))); @@ -1817,7 +1821,7 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller, info = lvmcache_info_from_pvid(dev->pvid, NULL, 0); if (!info) { - info = _create_info(labeller, dev); + info = _create_info(labeller, dev, label_sector); created = 1; } @@ -1849,6 +1853,8 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller, dm_list_add(&_found_duplicate_devs, &devl->list); _found_duplicate_pvs = 1; + if (is_duplicate) + *is_duplicate = 1; return NULL; } diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 21f29ef..d4c19f9 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -74,9 +74,9 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const /* Add/delete a device */ struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid, - struct device *dev, - const char *vgname, const char *vgid, - uint32_t vgstatus); + struct device *dev, uint64_t label_sector, + const char *vgname, const char *vgid, + uint32_t vgstatus, int *is_duplicate); int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt); void lvmcache_del(struct lvmcache_info *info); void lvmcache_del_dev(struct device *dev); diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index df1e56b..9d58c53 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -1513,13 +1513,12 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume /* Add a new cache entry with PV info or update existing one. */ if (!(info = lvmcache_add(fmt->labeller, (const char *) &pv->id, - pv->dev, pv->vg_name, - is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0))) + pv->dev, pv->label_sector, pv->vg_name, + is_orphan_vg(pv->vg_name) ? pv->vg_name : pv->vg ? (const char *) &pv->vg->id : NULL, 0, NULL))) return_0; + /* lvmcache_add() creates info and info->label structs for the dev, get info->label. */ label = lvmcache_get_label(info); - label->sector = pv->label_sector; - label->dev = pv->dev; lvmcache_update_pv(info, pv, fmt); diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c index 5f4bcfd..42184de 100644 --- a/lib/format_text/text_label.c +++ b/lib/format_text/text_label.c @@ -371,8 +371,8 @@ fail: return 0; } -static int _text_read(struct labeller *l, struct device *dev, void *label_buf, - struct label **label) +static int _text_read(struct labeller *labeller, struct device *dev, void *label_buf, + uint64_t label_sector, int *is_duplicate) { struct label_header *lh = (struct label_header *) label_buf; struct pv_header *pvhdr; @@ -382,18 +382,33 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf, uint64_t offset; uint32_t ext_version; struct _update_mda_baton baton; + struct label *label; /* * PV header base */ pvhdr = (struct pv_header *) ((char *) label_buf + xlate32(lh->offset_xl)); - if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev, + /* + * FIXME: stop adding the device to lvmcache initially as an orphan + * (and then moving it later) and instead just add it when we know the + * VG. + * + * If another device with this same PVID has already been seen, + * lvmcache_add will put this device in the duplicates list in lvmcache + * and return NULL. At the end of label_scan, the duplicate devs are + * compared, and if another dev is preferred for this PV, then the + * existing dev is removed from lvmcache and _text_read is called again + * for this dev, and lvmcache_add will add it. + * + * Other reasons for lvmcache_add to return NULL are internal errors. + */ + if (!(info = lvmcache_add(labeller, (char *)pvhdr->pv_uuid, dev, label_sector, FMT_TEXT_ORPHAN_VG_NAME, - FMT_TEXT_ORPHAN_VG_NAME, 0))) + FMT_TEXT_ORPHAN_VG_NAME, 0, is_duplicate))) return_0; - *label = lvmcache_get_label(info); + label = lvmcache_get_label(info); lvmcache_set_device_size(info, xlate64(pvhdr->device_size_xl)); @@ -441,7 +456,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf, } out: baton.info = info; - baton.label = *label; + baton.label = label; /* * In the vg_read phase, we compare all mdas and decide which to use diff --git a/lib/label/label.c b/lib/label/label.c index f6ba2d8..4d008ed 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -356,9 +356,9 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, int *is_lvm_device) { char label_buf[LABEL_SIZE] __attribute__((aligned(8))); - struct label *label = NULL; struct labeller *labeller; uint64_t sector = 0; + int is_duplicate = 0; int ret = 0; int pass; @@ -423,17 +423,41 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, /* * This is the point where the scanning code dives into the rest of - * lvm. ops->read() is usually _text_read() which reads the pv_header, - * mda locations, mda contents. As these bits of data are read, they - * are saved into lvmcache as info/vginfo structs. + * lvm. ops->read() is _text_read() which reads the pv_header, mda + * locations, and metadata text. All of the info it finds about the PV + * and VG is stashed in lvmcache which saves it in the form of + * info/vginfo structs. That lvmcache info is used later when the + * command wants to read the VG to do something to it. */ + ret = labeller->ops->read(labeller, dev, label_buf, sector, &is_duplicate); - if ((ret = (labeller->ops->read)(labeller, dev, label_buf, &label)) && label) { - label->dev = dev; - label->sector = sector; - } else { + if (!ret) { /* FIXME: handle errors */ lvmcache_del_dev(dev); + + if (is_duplicate) { + /* + * _text_read() called lvmcache_add() which found an + * existing info struct for this PVID but for a + * different dev. lvmcache_add() did not add an info + * struct for this dev, but added this dev to the list + * of duplicate devs. + */ + log_warn("WARNING: scan found duplicate PVID %s on %s", dev->pvid, dev_name(dev)); + } else { + /* + * Leave the info in lvmcache because the device is + * present and can still be used even if it has + * metadata that we can't process (we can get metadata + * from another PV/mda.) _text_read only saves mdas + * with good metadata in lvmcache (this includes old + * metadata), and if a PV has no mdas with good + * metadata, then the info for the PV will be in + * lvmcache with empty info->mdas, and it will behave + * like a PV with no mdas (a common configuration.) + */ + log_warn("WARNING: scan failed to get metadata summary from %s PVID %s", dev_name(dev), dev->pvid); + } } out: return ret; diff --git a/lib/label/label.h b/lib/label/label.h index 8f1f0e2..07bb77d 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -65,7 +65,7 @@ struct label_ops { * Read a label from a volume. */ int (*read) (struct labeller * l, struct device * dev, - void *label_buf, struct label ** label); + void *label_buf, uint64_t label_sector, int *is_duplicate); /* * Populate label_type etc.

1 0

master - add mda arg to add_mda
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=889b5d3183314985d4d09… Commit: 889b5d3183314985d4d0930b30f0cd4b0f482f69 Parent: b2447e3538db370d1e12d328c25ca1f078ddabf6 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 5 13:24:23 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 add mda arg to add_mda Allow the caller of lvmcache_add_mda() to have the new mda returned. --- lib/cache/lvmcache.c | 5 +++-- lib/cache/lvmcache.h | 3 ++- lib/format_text/format-text.c | 2 +- lib/format_text/format-text.h | 3 ++- lib/format_text/text_label.c | 7 +++++-- 5 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index f54eff0..3e89b02 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -2086,9 +2086,10 @@ void lvmcache_del_bas(struct lvmcache_info *info) } int lvmcache_add_mda(struct lvmcache_info *info, struct device *dev, - uint64_t start, uint64_t size, unsigned ignored) + uint64_t start, uint64_t size, unsigned ignored, + struct metadata_area **mda_new) { - return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored); + return add_mda(info->fmt, NULL, &info->mdas, dev, start, size, ignored, mda_new); } int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size) diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 16cbb48..21f29ef 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -129,7 +129,8 @@ void lvmcache_del_mdas(struct lvmcache_info *info); void lvmcache_del_das(struct lvmcache_info *info); void lvmcache_del_bas(struct lvmcache_info *info); int lvmcache_add_mda(struct lvmcache_info *info, struct device *dev, - uint64_t start, uint64_t size, unsigned ignored); + uint64_t start, uint64_t size, unsigned ignored, + struct metadata_area **mda_new); int lvmcache_add_da(struct lvmcache_info *info, uint64_t start, uint64_t size); int lvmcache_add_ba(struct lvmcache_info *info, uint64_t start, uint64_t size); diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index f5e08fa..df1e56b 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -1547,7 +1547,7 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume // if fmt is not the same as info->fmt we are in trouble if (!lvmcache_add_mda(info, mdac->area.dev, mdac->area.start, mdac->area.size, - mda_is_ignored(mda))) + mda_is_ignored(mda), NULL)) return_0; } diff --git a/lib/format_text/format-text.h b/lib/format_text/format-text.h index 1300e58..c42d5c0 100644 --- a/lib/format_text/format-text.h +++ b/lib/format_text/format-text.h @@ -61,7 +61,8 @@ int add_ba(struct dm_pool *mem, struct dm_list *eas, uint64_t start, uint64_t size); void del_bas(struct dm_list *bas); int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *mdas, - struct device *dev, uint64_t start, uint64_t size, unsigned ignored); + struct device *dev, uint64_t start, uint64_t size, unsigned ignored, + struct metadata_area **mda_new); void del_mdas(struct dm_list *mdas); /* On disk */ diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c index 6eca3c1..5f4bcfd 100644 --- a/lib/format_text/text_label.c +++ b/lib/format_text/text_label.c @@ -243,7 +243,8 @@ void del_bas(struct dm_list *bas) /* FIXME: refactor this function with other mda constructor code */ int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list *mdas, - struct device *dev, uint64_t start, uint64_t size, unsigned ignored) + struct device *dev, uint64_t start, uint64_t size, unsigned ignored, + struct metadata_area **mda_new) { /* FIXME List size restricted by pv_header SECTOR_SIZE */ struct metadata_area *mdal, *mda; @@ -295,6 +296,8 @@ int add_mda(const struct format_type *fmt, struct dm_pool *mem, struct dm_list * mda_set_ignored(mdal, ignored); dm_list_add(mdas, &mdal->list); + if (mda_new) + *mda_new = mdal; return 1; } @@ -408,7 +411,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *label_buf, /* Metadata area headers */ dlocn_xl++; while ((offset = xlate64(dlocn_xl->offset))) { - lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0); + lvmcache_add_mda(info, dev, offset, xlate64(dlocn_xl->size), 0, NULL); dlocn_xl++; }

1 0

master - keep track of which mdas have old metadata in lvmcache
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=b2447e3538db370d1e12d… Commit: b2447e3538db370d1e12d328c25ca1f078ddabf6 Parent: 0b18c25d934564015402de33e15a267045ed1b8c Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 5 13:09:56 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 keep track of which mdas have old metadata in lvmcache This will be used for more advanced repair in a subsequent commit. --- lib/cache/lvmcache.c | 173 ++++++++++++++++++++++++++++++++++++++++---------- lib/cache/lvmcache.h | 10 ++- 2 files changed, 145 insertions(+), 38 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index a860660..f54eff0 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -42,6 +42,10 @@ struct lvmcache_info { uint32_t status; bool mda1_bad; /* label scan found bad metadata in mda1 */ bool mda2_bad; /* label scan found bad metadata in mda2 */ + bool summary_seqno_mismatch; /* two mdas on this dev has mismatching metadata */ + int summary_seqno; /* vg seqno found on this dev during scan */ + int mda1_seqno; + int mda2_seqno; }; /* One per VG */ @@ -61,7 +65,7 @@ struct lvmcache_vginfo { uint32_t mda_checksum; size_t mda_size; int seqno; - int scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */ + bool scan_summary_mismatch; /* vgsummary from devs had mismatching seqno or checksum */ }; static struct dm_hash_table *_pvid_hash = NULL; @@ -1515,12 +1519,9 @@ int lvmcache_add_orphan_vginfo(const char *vgname, struct format_type *fmt) } /* - * FIXME: get rid of other callers of this function which call it - * in odd cases to "fix up" some bit of lvmcache state. Make those - * callers fix up what they need to directly, and leave this function - * with one purpose and caller. + * Returning 0 causes the caller to remove the info struct for this + * device from lvmcache, which will make it look like a missing device. */ - int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vgsummary *vgsummary) { const char *vgname = vgsummary->vgname; @@ -1546,6 +1547,7 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg * Puts the vginfo into the vgname hash table. */ if (!_lvmcache_update_vgname(info, vgname, vgid, vgsummary->vgstatus, vgsummary->creation_host, info->fmt)) { + /* shouldn't happen, internal error */ log_error("Failed to update VG %s info in lvmcache.", vgname); return 0; } @@ -1554,6 +1556,7 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg * Puts the vginfo into the vgid hash table. */ if (!_lvmcache_update_vgid(info, info->vginfo, vgid)) { + /* shouldn't happen, internal error */ log_error("Failed to update VG %s info in lvmcache.", vgname); return 0; } @@ -1569,50 +1572,114 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg if (!vgsummary->seqno && !vgsummary->mda_size && !vgsummary->mda_checksum) return 1; + /* + * Keep track of which devs/mdas have old versions of the metadata. + * The values we keep in vginfo are from the metadata with the largest + * seqno. One dev may have more recent metadata than another dev, and + * one mda may have more recent metadata than the other mda on the same + * device. + * + * When a device holds old metadata, the info struct for the device + * remains in lvmcache, so the device is not treated as missing. + * Also the mda struct containing the old metadata is kept on + * info->mdas. This means that vg_read will read metadata from + * the mda again (and probably see the same old metadata). It + * also means that vg_write will use the mda to write new metadata + * into the mda that currently has the old metadata. + */ + if (vgsummary->mda_num == 1) + info->mda1_seqno = vgsummary->seqno; + else if (vgsummary->mda_num == 2) + info->mda2_seqno = vgsummary->seqno; + + if (!info->summary_seqno) + info->summary_seqno = vgsummary->seqno; + else { + if (info->summary_seqno == vgsummary->seqno) { + /* This mda has the same metadata as the prev mda on this dev. */ + return 1; + + } else if (info->summary_seqno > vgsummary->seqno) { + /* This mda has older metadata than the prev mda on this dev. */ + info->summary_seqno_mismatch = true; + + } else if (info->summary_seqno < vgsummary->seqno) { + /* This mda has newer metadata than the prev mda on this dev. */ + info->summary_seqno_mismatch = true; + info->summary_seqno = vgsummary->seqno; + } + } + + /* this shouldn't happen */ if (!(vginfo = info->vginfo)) return 1; if (!vginfo->seqno) { vginfo->seqno = vgsummary->seqno; + vginfo->mda_checksum = vgsummary->mda_checksum; + vginfo->mda_size = vgsummary->mda_size; - log_debug_cache("lvmcache %s: VG %s: set seqno to %d", - dev_name(info->dev), vginfo->vgname, vginfo->seqno); + log_debug_cache("lvmcache %s mda%d VG %s set seqno %u checksum %x mda_size %zu", + dev_name(info->dev), vgsummary->mda_num, vgname, + vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size); + goto update_vginfo; + + } else if (vgsummary->seqno < vginfo->seqno) { + vginfo->scan_summary_mismatch = true; - } 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. */ + log_debug_cache("lvmcache %s mda%d VG %s older seqno %u checksum %x mda_size %zu", + dev_name(info->dev), vgsummary->mda_num, vgname, + vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size); return 1; - } - if (!vginfo->mda_size) { + } else if (vgsummary->seqno > vginfo->seqno) { + vginfo->scan_summary_mismatch = true; + + /* Replace vginfo values with values from newer metadata. */ + vginfo->seqno = vgsummary->seqno; vginfo->mda_checksum = vgsummary->mda_checksum; vginfo->mda_size = vgsummary->mda_size; - log_debug_cache("lvmcache %s: 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. */ + log_debug_cache("lvmcache %s mda%d VG %s newer seqno %u checksum %x mda_size %zu", + dev_name(info->dev), vgsummary->mda_num, vgname, + vgsummary->seqno, vgsummary->mda_checksum, vgsummary->mda_size); + + goto update_vginfo; + } else { + /* + * Same seqno as previous metadata we saw for this VG. + * If the metadata somehow has a different checksum or size, + * even though it has the same seqno, something has gone wrong. + * FIXME: test this case: VG has two PVs, first goes missing, + * second updated to seqno 4, first comes back and second goes + * missing, first updated to seqno 4, second comes back, now + * both are present with same seqno but different checksums. + */ + + if ((vginfo->mda_size != vgsummary->mda_size) || (vginfo->mda_checksum != vgsummary->mda_checksum)) { + log_warn("WARNING: scan of VG %s from %s mda%d found mda_checksum %x mda_size %zu vs %x %zu", + vgname, dev_name(info->dev), vgsummary->mda_num, + vgsummary->mda_checksum, vgsummary->mda_size, + vginfo->mda_checksum, vginfo->mda_size); + vginfo->scan_summary_mismatch = true; + return 0; + } + + /* + * The seqno and checksum matches what was previously seen; + * the summary values have already been saved in vginfo. + */ return 1; } - /* - * If a dev has an unmatching checksum, ignore the other - * info from it, keeping the info we already saved. - */ + update_vginfo: if (!_lvmcache_update_vgstatus(info, vgsummary->vgstatus, vgsummary->creation_host, vgsummary->lock_type, vgsummary->system_id)) { + /* + * This shouldn't happen, it's an internal errror, and we can leave + * the info in place without saving the summary values in vginfo. + */ log_error("Failed to update VG %s info in lvmcache.", vgname); - return 0; } return 1; @@ -2325,17 +2392,17 @@ int lvmcache_vg_is_foreign(struct cmd_context *cmd, const char *vgname, const ch * lvmcache: info for dev5 is deleted, FIXME: use a defective state */ -int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid) +bool lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid) { struct lvmcache_vginfo *vginfo; if (!vgname || !vgid) - return 1; + return true; if ((vginfo = lvmcache_vginfo_from_vgid(vgid))) return vginfo->scan_summary_mismatch; - return 1; + return true; } static uint64_t _max_metadata_size; @@ -2394,6 +2461,42 @@ struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd, return NULL; } +/* + * This is used by the metadata repair command to check if + * the metadata on a dev needs repair because it's old. + */ +bool lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev) +{ + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info; + + /* shouldn't happen */ + if (!vgname || !vgid) + return false; + + /* shouldn't happen */ + if (!(vginfo = lvmcache_vginfo_from_vgid(vgid))) + return false; + + /* shouldn't happen */ + if (!(info = lvmcache_info_from_pvid(dev->pvid, NULL, 0))) + return false; + + /* writing to a new PV */ + if (!info->summary_seqno) + return false; + + /* on same dev, one mda has newer metadata than the other */ + if (info->summary_seqno_mismatch) + return true; + + /* one or both mdas on this dev has older metadata than another dev */ + if (vginfo->seqno > info->summary_seqno) + return true; + + return false; +} + void lvmcache_get_outdated_devs(struct cmd_context *cmd, const char *vgname, const char *vgid, struct dm_list *devs) diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 0f83c80..16cbb48 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -57,10 +57,12 @@ struct lvmcache_vgsummary { char *creation_host; const char *system_id; const char *lock_type; + uint32_t seqno; uint32_t mda_checksum; size_t mda_size; - int zero_offset; - int seqno; + int mda_num; /* 1 = summary from mda1, 2 = summary from mda2 */ + unsigned mda_ignored:1; + unsigned zero_offset:1; }; int lvmcache_init(struct cmd_context *cmd); @@ -202,7 +204,7 @@ int lvmcache_get_vg_devs(struct cmd_context *cmd, struct dm_list *devs); void lvmcache_set_independent_location(const char *vgname); -int lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid); +bool lvmcache_scan_mismatch(struct cmd_context *cmd, const char *vgname, const char *vgid); int lvmcache_vginfo_has_pvid(struct lvmcache_vginfo *vginfo, char *pvid); @@ -222,6 +224,8 @@ int dev_in_device_list(struct device *dev, struct dm_list *head); bool lvmcache_has_bad_metadata(struct device *dev); +bool lvmcache_has_old_metadata(struct cmd_context *cmd, const char *vgname, const char *vgid, struct device *dev); + void lvmcache_get_outdated_devs(struct cmd_context *cmd, const char *vgname, const char *vgid, struct dm_list *devs);

1 0

master - ability to keep track of outdated pvs in lvmcache
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=0b18c25d934564015402d… Commit: 0b18c25d934564015402de33e15a267045ed1b8c Parent: 650524b9559bb56339b281af52db9f4df91038ae Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 5 12:55:51 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 ability to keep track of outdated pvs in lvmcache Outdated PVs hold metadata for VG from which they have been removed. Add the ability to keep track of these in lvmcache. This will be used for more advanced repair in a subsequent commit. --- lib/cache/lvmcache.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ lib/cache/lvmcache.h | 15 +++++++++ 2 files changed, 95 insertions(+), 0 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index a36e145..a860660 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -48,6 +48,7 @@ struct lvmcache_info { struct lvmcache_vginfo { struct dm_list list; /* Join these vginfos together */ struct dm_list infos; /* List head for lvmcache_infos */ + struct dm_list outdated_infos; /* vg_read moves info from infos to outdated_infos */ const struct format_type *fmt; char *vgname; /* "" == orphan */ uint32_t status; @@ -1389,6 +1390,7 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info, return 0; } dm_list_init(&vginfo->infos); + dm_list_init(&vginfo->outdated_infos); /* * A different VG (different uuid) can exist with the same name. @@ -2392,3 +2394,81 @@ struct metadata_area *lvmcache_get_mda(struct cmd_context *cmd, return NULL; } +void lvmcache_get_outdated_devs(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct dm_list *devs) +{ + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info; + struct device_list *devl; + + if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) { + log_error(INTERNAL_ERROR "lvmcache_get_outdated_devs no vginfo %s", vgname); + return; + } + + dm_list_iterate_items(info, &vginfo->outdated_infos) { + if (!(devl = zalloc(sizeof(*devl)))) + return; + devl->dev = info->dev; + dm_list_add(devs, &devl->list); + } +} + +void lvmcache_del_outdated_devs(struct cmd_context *cmd, + const char *vgname, const char *vgid) +{ + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info, *info2; + + if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) { + log_error(INTERNAL_ERROR "lvmcache_get_outdated_devs no vginfo"); + return; + } + + dm_list_iterate_items_safe(info, info2, &vginfo->outdated_infos) + lvmcache_del(info); +} + +void lvmcache_get_outdated_mdas(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct device *dev, + struct dm_list **mdas) +{ + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info; + + *mdas = NULL; + + if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) { + log_error(INTERNAL_ERROR "lvmcache_get_outdated_mdas no vginfo"); + return; + } + + dm_list_iterate_items(info, &vginfo->outdated_infos) { + if (info->dev != dev) + continue; + *mdas = &info->mdas; + return; + } +} + +bool lvmcache_is_outdated_dev(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct device *dev) +{ + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info; + + if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) { + log_error(INTERNAL_ERROR "lvmcache_get_outdated_mdas no vginfo"); + return false; + } + + dm_list_iterate_items(info, &vginfo->outdated_infos) { + if (info->dev == dev) + return true; + } + + return false; +} diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index dd6730e..0f83c80 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -222,6 +222,21 @@ int dev_in_device_list(struct device *dev, struct dm_list *head); bool lvmcache_has_bad_metadata(struct device *dev); +void lvmcache_get_outdated_devs(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct dm_list *devs); +void lvmcache_get_outdated_mdas(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct device *dev, + struct dm_list **mdas); + +bool lvmcache_is_outdated_dev(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct device *dev); + +void lvmcache_del_outdated_devs(struct cmd_context *cmd, + const char *vgname, const char *vgid); + void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda); void lvmcache_get_bad_mdas(struct cmd_context *cmd,

1 0

master - ability to keep track of bad mdas in lvmcache
by David Teigland 07 Jun '19

07 Jun '19

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=650524b9559bb56339b28… Commit: 650524b9559bb56339b281af52db9f4df91038ae Parent: aeafdc1f45a5963290a5bc3ea1661018b072d817 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 5 12:39:08 2019 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Jun 7 15:54:04 2019 -0500 ability to keep track of bad mdas in lvmcache mda's that cannot be processed by lvm because of some corruption can be kept on a separate list. These will be used for more advanced repair in a subsequent commit. --- lib/cache/lvmcache.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++ lib/cache/lvmcache.h | 8 ++++++ lib/metadata/metadata.h | 3 +- 3 files changed, 65 insertions(+), 1 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index b62e5e2..a36e145 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -31,6 +31,7 @@ struct lvmcache_info { struct dm_list mdas; /* list head for metadata areas */ struct dm_list das; /* list head for data areas */ struct dm_list bas; /* list head for bootloader areas */ + struct dm_list bad_mdas;/* list head for bad metadata areas */ struct lvmcache_vginfo *vginfo; /* NULL == unknown */ struct label *label; const struct format_type *fmt; @@ -39,6 +40,8 @@ struct lvmcache_info { uint32_t ext_version; /* Extension version */ uint32_t ext_flags; /* Extension flags */ uint32_t status; + bool mda1_bad; /* label scan found bad metadata in mda1 */ + bool mda2_bad; /* label scan found bad metadata in mda2 */ }; /* One per VG */ @@ -175,6 +178,54 @@ static void _destroy_duplicate_device_list(struct dm_list *head) dm_list_init(head); } +bool lvmcache_has_bad_metadata(struct device *dev) +{ + struct lvmcache_info *info; + + if (!(info = lvmcache_info_from_pvid(dev->pvid, dev, 0))) { + /* shouldn't happen */ + log_error("No lvmcache info for checking bad metadata on %s", dev_name(dev)); + return false; + } + + if (info->mda1_bad || info->mda2_bad) + return true; + return false; +} + +void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda) +{ + if (mda->mda_num == 1) + info->mda1_bad = true; + else if (mda->mda_num == 2) + info->mda2_bad = true; + dm_list_add(&info->bad_mdas, &mda->list); +} + +void lvmcache_get_bad_mdas(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct dm_list *bad_mda_list) +{ + struct lvmcache_vginfo *vginfo; + struct lvmcache_info *info; + struct mda_list *mdal; + struct metadata_area *mda, *mda2; + + if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, vgid))) { + log_error(INTERNAL_ERROR "lvmcache_get_bad_mdas no vginfo %s", vgname); + return; + } + + dm_list_iterate_items(info, &vginfo->infos) { + dm_list_iterate_items_safe(mda, mda2, &info->bad_mdas) { + if (!(mdal = zalloc(sizeof(*mdal)))) + continue; + mdal->mda = mda; + dm_list_add(bad_mda_list, &mdal->list); + } + } +} + static void _vginfo_attach_info(struct lvmcache_vginfo *vginfo, struct lvmcache_info *info) { @@ -1945,6 +1996,10 @@ void lvmcache_del_mdas(struct lvmcache_info *info) if (info->mdas.n) del_mdas(&info->mdas); dm_list_init(&info->mdas); + + if (info->bad_mdas.n) + del_mdas(&info->bad_mdas); + dm_list_init(&info->bad_mdas); } void lvmcache_del_das(struct lvmcache_info *info) diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 5e59b94..dd6730e 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -220,4 +220,12 @@ void lvmcache_save_metadata_size(uint64_t val); int dev_in_device_list(struct device *dev, struct dm_list *head); +bool lvmcache_has_bad_metadata(struct device *dev); + +void lvmcache_save_bad_mda(struct lvmcache_info *info, struct metadata_area *mda); + +void lvmcache_get_bad_mdas(struct cmd_context *cmd, + const char *vgname, const char *vgid, + struct dm_list *bad_mda_list); + #endif diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h index d904aa6..6d158fe 100644 --- a/lib/metadata/metadata.h +++ b/lib/metadata/metadata.h @@ -185,6 +185,7 @@ struct metadata_area { struct metadata_area_ops *ops; void *metadata_locn; uint32_t status; + int mda_num; uint32_t bad_fields; /* BAD_MDA_ flags are set to indicate errors found when reading */ uint32_t ignore_bad_fields; /* BAD_MDA_ flags are set to indicate errors to ignore */ }; @@ -248,7 +249,7 @@ struct name_list { struct mda_list { struct dm_list list; - struct device_area mda; + struct metadata_area *mda; }; struct peg_list {

1 0

← Newer
1
...
5
6
7
8
9
10
11
Older →

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits June 2019