April 2018 - lvm2-commits - Fedora Mailing-Lists

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=89f54a5094981c827ca... Commit: 89f54a5094981c827ca557cb1381e5f1a028912c Parent: c29899b910811d0d1dc5607d200a1e59773eff0c Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 13 15:53:59 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:46 2018 -0500 remove debugging print --- lib/label/label.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/lib/label/label.c b/lib/label/label.c index f0ec92f..1268498 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -802,9 +802,7 @@ int label_read_sector(struct device *dev, struct label **labelp, uint64_t scan_s void label_scan_confirm(struct device *dev) { - if (!_in_bcache(dev)) { - log_warn("add dev %s to bcache", dev_name(dev)); + if (!_in_bcache(dev)) label_read(dev, NULL, 0); - } }

6 years

1
0
0 / 0

master - remove unused variable in _pvremove_check_single

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=c29899b910811d0d1dc... Commit: c29899b910811d0d1dc5607d200a1e59773eff0c Parent: a1e3398ffcf5def36934eabbb9ea26db221bbe74 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 13 14:28:24 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:46 2018 -0500 remove unused variable in _pvremove_check_single --- tools/toollib.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/tools/toollib.c b/tools/toollib.c index 0cc2edd..807e34c 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -5279,7 +5279,6 @@ static int _pvremove_check_single(struct cmd_context *cmd, struct pvcreate_params *pp = (struct pvcreate_params *) handle->custom_handle; struct pvcreate_device *pd; struct pvcreate_prompt *prompt; - struct label *label; int found = 0; if (!pv->dev)

6 years

1
0
0 / 0

master - scan: handle no devices

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=a1e3398ffcf5def3693... Commit: a1e3398ffcf5def36934eabbb9ea26db221bbe74 Parent: 9d2add136192a08be7b3441b6fbe583c27b03dc8 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 13 13:37:10 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:46 2018 -0500 scan: handle no devices Still create bcache. --- lib/label/label.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/lib/label/label.c b/lib/label/label.c index 5da781e..f0ec92f 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -577,6 +577,7 @@ int label_scan(struct cmd_context *cmd) struct device_list *devl; struct device *dev; struct io_engine *ioe; + int cache_blocks; log_debug_devs("Finding devices to scan"); @@ -615,6 +616,9 @@ int label_scan(struct cmd_context *cmd) dev_iter_destroy(iter); if (!scan_bcache) { + /* No devices can happen, just create bcache with any small number. */ + if (!(cache_blocks = dm_list_size(&all_devs))) + cache_blocks = 8; /* * 100 is arbitrary, it's the max number of concurrent aio's @@ -630,7 +634,7 @@ int label_scan(struct cmd_context *cmd) * of the devs will not be lvm devices, and we don't need * an entry for those. We might want to change this. */ - if (!(scan_bcache = bcache_create(BCACHE_BLOCK_SIZE_IN_SECTORS, dm_list_size(&all_devs), ioe))) + if (!(scan_bcache = bcache_create(BCACHE_BLOCK_SIZE_IN_SECTORS, cache_blocks, ioe))) return 0; }

6 years

1
0
0 / 0

master - scan: add a dev to bcache before each read to handle write path

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=9d2add136192a08be7b... Commit: 9d2add136192a08be7b3441b6fbe583c27b03dc8 Parent: 6c67c7557c266063db962807aca18fc088ba921e Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 13 12:50:44 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:46 2018 -0500 scan: add a dev to bcache before each read to handle write path This is a temporary hacky workaround to the problem of reads going through bcache and writes not using bcache. The write path wants to read parts of data that it is incrementally writing to disk, but the reads (using bcache) don't work because the writes are not in the bcache. For now, add a dev to bcache before each attempt to read it in case it's being used on the write path. --- lib/format_text/format-text.c | 6 ++++++ lib/label/label.c | 18 ++++++++++++++++++ lib/label/label.h | 1 + 3 files changed, 25 insertions(+), 0 deletions(-) diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index a5d8397..f33451f 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -320,6 +320,8 @@ static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev log_debug_metadata("Reading mda header sector from %s at %llu", dev_name(dev_area->dev), (unsigned long long)dev_area->start); + label_scan_confirm(dev_area->dev); /* FIXME: remove this, ensures dev is in bcache */ + if (!bcache_read_bytes(scan_bcache, dev_area->dev->bcache_fd, dev_area->start, MDA_HEADER_SIZE, mdah)) { log_error("Failed to read metadata area header on %s at %llu", dev_name(dev_area->dev), (unsigned long long)dev_area->start); @@ -462,6 +464,8 @@ static struct raw_locn *_read_metadata_location_vg(struct device_area *dev_area, */ memset(vgnamebuf, 0, sizeof(vgnamebuf)); + label_scan_confirm(dev_area->dev); /* FIXME: remove this, ensures dev is in bcache */ + bcache_read_bytes(scan_bcache, dev_area->dev->bcache_fd, dev_area->start + rlocn->offset, NAME_LEN, vgnamebuf); if (!strncmp(vgnamebuf, vgname, len = strlen(vgname)) && @@ -1209,6 +1213,8 @@ int read_metadata_location_summary(const struct format_type *fmt, return 0; } + label_scan_confirm(dev_area->dev); /* FIXME: remove this, ensures dev is in bcache */ + bcache_read_bytes(scan_bcache, dev_area->dev->bcache_fd, dev_area->start + rlocn->offset, NAME_LEN, buf); while (buf[len] && !isspace(buf[len]) && buf[len] != '{' && diff --git a/lib/label/label.c b/lib/label/label.c index 2ec187c..5da781e 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -786,3 +786,21 @@ int label_read_sector(struct device *dev, struct label **labelp, uint64_t scan_s return label_read(dev, labelp, 0); } +/* + * FIXME: remove this. It should not be needed once writes are going through + * bcache. As it is now, the write path involves multiple writes to a device, + * and later writes want to read previous writes from disk. They do these + * reads using the standard read paths which require the devs to be in bcache, + * but the bcache reads do not find the dev because the writes have gone around + * bcache. To work around this for now, check if each dev is in bcache before + * reading it, and if not add it first. + */ + +void label_scan_confirm(struct device *dev) +{ + if (!_in_bcache(dev)) { + log_warn("add dev %s to bcache", dev_name(dev)); + label_read(dev, NULL, 0); + } +} + diff --git a/lib/label/label.h b/lib/label/label.h index eb62f64..e265a6b 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -109,5 +109,6 @@ void label_scan_invalidate(struct device *dev); void label_scan_destroy(struct cmd_context *cmd); int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector); int label_read_sector(struct device *dev, struct label **labelp, uint64_t scan_sector); +void label_scan_confirm(struct device *dev); #endif

6 years

1
0
0 / 0

master - scan: use separate fd for bcache

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=6c67c7557c266063db9... Commit: 6c67c7557c266063db962807aca18fc088ba921e Parent: 4343280ebc0e2aae0de5fe959c24574b13d0d7be Author: David Teigland <teigland(a)redhat.com> AuthorDate: Tue Feb 13 08:58:35 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:46 2018 -0500 scan: use separate fd for bcache Create a new dev->bcache_fd that the scanning code owns and is in charge of opening/closing. This prevents other parts of lvm code (which do various open/close) from interfering with the bcache fd. A number of dev_open and dev_close are removed from the reading path since the read path now uses the bcache. With that in place, open(O_EXCL) for pvcreate/pvremove can then be fixed. That wouldn't work previously because of other open fds. --- lib/cache/lvmetad.c | 12 +++- lib/config/config.c | 4 +- lib/device/dev-io.c | 28 ++++---- lib/device/device.h | 2 + lib/format_text/format-text.c | 55 +++----------- lib/format_text/text_label.c | 13 --- lib/label/label.c | 166 +++++++++++++++++++++++++++++++--------- lib/label/label.h | 1 + lib/metadata/metadata-liblvm.c | 1 + tools/toollib.c | 44 ++++------- 10 files changed, 187 insertions(+), 139 deletions(-) diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c index 81ba1b7..ed0bcde 100644 --- a/lib/cache/lvmetad.c +++ b/lib/cache/lvmetad.c @@ -2551,11 +2551,18 @@ static int _lvmetad_get_pv_cache_list(struct cmd_context *cmd, struct dm_list *p */ static void _update_pv_in_udev(struct cmd_context *cmd, dev_t devt) { - struct device *dev; - log_debug_devs("device %d:%d open to update udev", + /* + * FIXME: this is diabled as part of removing dev_opens + * to integrate bcache. If this is really needed, we + * can do a separate open/close here. + */ + log_debug_devs("SKIP device %d:%d open to update udev", (int)MAJOR(devt), (int)MINOR(devt)); +#if 0 + struct device *dev; + if (!(dev = dev_cache_get_by_devt(devt, cmd->lvmetad_filter))) { log_error("_update_pv_in_udev no dev found"); return; @@ -2568,6 +2575,7 @@ static void _update_pv_in_udev(struct cmd_context *cmd, dev_t devt) if (!dev_close(dev)) stack; +#endif } /* diff --git a/lib/config/config.c b/lib/config/config.c index 2d7db69..0711b8c 100644 --- a/lib/config/config.c +++ b/lib/config/config.c @@ -534,11 +534,11 @@ int config_file_read_fd(struct dm_config_tree *cft, struct device *dev, dev_io_r return 0; } - if (!bcache_read_bytes(scan_bcache, dev->fd, offset, size, buf)) + if (!bcache_read_bytes(scan_bcache, dev->bcache_fd, offset, size, buf)) goto out; if (size2) { - if (!bcache_read_bytes(scan_bcache, dev->fd, offset2, size2, buf + size)) + if (!bcache_read_bytes(scan_bcache, dev->bcache_fd, offset2, size2, buf + size)) goto out; } diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c index c321e61..39d5d30 100644 --- a/lib/device/dev-io.c +++ b/lib/device/dev-io.c @@ -330,6 +330,8 @@ static int _dev_get_size_file(struct device *dev, uint64_t *size) static int _dev_get_size_dev(struct device *dev, uint64_t *size) { const char *name = dev_name(dev); + int fd = dev->bcache_fd; + int do_close = 0; if (dev->size_seqno == _dev_size_seqno) { log_very_verbose("%s: using cached size %" PRIu64 " sectors", @@ -338,12 +340,16 @@ static int _dev_get_size_dev(struct device *dev, uint64_t *size) return 1; } - if (!dev_open_readonly(dev)) - return_0; + if (fd <= 0) { + if (!dev_open_readonly(dev)) + return_0; + fd = dev_fd(dev); + do_close = 1; + } - if (ioctl(dev_fd(dev), BLKGETSIZE64, size) < 0) { + if (ioctl(fd, BLKGETSIZE64, size) < 0) { log_sys_error("ioctl BLKGETSIZE64", name); - if (!dev_close(dev)) + if (do_close && !dev_close(dev)) log_sys_error("close", name); return 0; } @@ -352,7 +358,7 @@ static int _dev_get_size_dev(struct device *dev, uint64_t *size) dev->size = *size; dev->size_seqno = _dev_size_seqno; - if (!dev_close(dev)) + if (do_close && !dev_close(dev)) log_sys_error("close", name); log_very_verbose("%s: size is %" PRIu64 " sectors", name, *size); @@ -629,17 +635,12 @@ int dev_open_readonly_quiet(struct device *dev) int dev_test_excl(struct device *dev) { - int flags; - int r; + int flags = 0; - flags = vg_write_lock_held() ? O_RDWR : O_RDONLY; flags |= O_EXCL; + flags |= O_RDWR; - r = dev_open_flags(dev, flags, 1, 1); - if (r) - dev_close_immediate(dev); - - return r; + return dev_open_flags(dev, flags, 1, 1); } static void _close(struct device *dev) @@ -659,7 +660,6 @@ static void _close(struct device *dev) static int _dev_close(struct device *dev, int immediate) { - if (dev->fd < 0) { log_error("Attempt to close device '%s' " "which is not open.", dev_name(dev)); diff --git a/lib/device/device.h b/lib/device/device.h index d5eb00f..36d1e3e 100644 --- a/lib/device/device.h +++ b/lib/device/device.h @@ -32,6 +32,7 @@ #define DEV_ASSUMED_FOR_LV 0x00000200 /* Is device assumed for an LV */ #define DEV_NOT_O_NOATIME 0x00000400 /* Don't use O_NOATIME */ #define DEV_IN_BCACHE 0x00000800 /* dev fd is open and used in bcache */ +#define DEV_BCACHE_EXCL 0x00001000 /* bcache_fd should be open EXCL */ /* * Support for external device info. @@ -66,6 +67,7 @@ struct device { int phys_block_size; int block_size; int read_ahead; + int bcache_fd; uint32_t flags; unsigned size_seqno; uint64_t size; diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index 8740a05..a5d8397 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -320,7 +320,7 @@ static int _raw_read_mda_header(struct mda_header *mdah, struct device_area *dev log_debug_metadata("Reading mda header sector from %s at %llu", dev_name(dev_area->dev), (unsigned long long)dev_area->start); - if (!bcache_read_bytes(scan_bcache, dev_area->dev->fd, dev_area->start, MDA_HEADER_SIZE, mdah)) { + if (!bcache_read_bytes(scan_bcache, dev_area->dev->bcache_fd, dev_area->start, MDA_HEADER_SIZE, mdah)) { log_error("Failed to read metadata area header on %s at %llu", dev_name(dev_area->dev), (unsigned long long)dev_area->start); return 0; @@ -462,7 +462,7 @@ static struct raw_locn *_read_metadata_location_vg(struct device_area *dev_area, */ memset(vgnamebuf, 0, sizeof(vgnamebuf)); - bcache_read_bytes(scan_bcache, dev_area->dev->fd, dev_area->start + rlocn->offset, NAME_LEN, vgnamebuf); + bcache_read_bytes(scan_bcache, dev_area->dev->bcache_fd, dev_area->start + rlocn->offset, NAME_LEN, vgnamebuf); if (!strncmp(vgnamebuf, vgname, len = strlen(vgname)) && (isspace(vgnamebuf[len]) || vgnamebuf[len] == '{')) @@ -510,18 +510,12 @@ static int _raw_holds_vgname(struct format_instance *fid, int noprecommit = 0; struct mda_header *mdah; - if (!dev_open_readonly(dev_area->dev)) - return_0; - if (!(mdah = raw_read_mda_header(fid->fmt, dev_area, 0))) return_0; if (_read_metadata_location_vg(dev_area, mdah, 0, vgname, &noprecommit)) r = 1; - if (!dev_close(dev_area->dev)) - stack; - return r; } @@ -595,14 +589,8 @@ static struct volume_group *_vg_read_raw(struct format_instance *fid, struct mda_context *mdac = (struct mda_context *) mda->metadata_locn; struct volume_group *vg; - if (!dev_open_readonly(mdac->area.dev)) - return_NULL; - vg = _vg_read_raw_area(fid, vgname, &mdac->area, vg_fmtdata, use_previous_vg, 0, mda_is_primary(mda)); - if (!dev_close(mdac->area.dev)) - stack; - return vg; } @@ -615,14 +603,8 @@ static struct volume_group *_vg_read_precommit_raw(struct format_instance *fid, struct mda_context *mdac = (struct mda_context *) mda->metadata_locn; struct volume_group *vg; - if (!dev_open_readonly(mdac->area.dev)) - return_NULL; - vg = _vg_read_raw_area(fid, vgname, &mdac->area, vg_fmtdata, use_previous_vg, 1, mda_is_primary(mda)); - if (!dev_close(mdac->area.dev)) - stack; - return vg; } @@ -653,9 +635,6 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg, if (!found) return 1; - if (!dev_open(mdac->area.dev)) - return_0; - if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda)))) goto_out; @@ -694,6 +673,9 @@ static int _vg_write_raw(struct format_instance *fid, struct volume_group *vg, label_scan_invalidate(mdac->area.dev); + if (!dev_open(mdac->area.dev)) + return_0; + /* Write text out, circularly */ if (!dev_write(mdac->area.dev, mdac->area.start + mdac->rlocn.offset, (size_t) (mdac->rlocn.size - new_wrap), MDA_CONTENT_REASON(mda_is_primary(mda)), @@ -879,9 +861,6 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg, int r = 0; int noprecommit = 0; - if (!dev_open(mdac->area.dev)) - return_0; - if (!(mdah = raw_read_mda_header(fid->fmt, &mdac->area, mda_is_primary(mda)))) goto_out; @@ -895,6 +874,9 @@ static int _vg_remove_raw(struct format_instance *fid, struct volume_group *vg, rlocn->checksum = 0; rlocn_set_ignored(mdah->raw_locns, mda_is_ignored(mda)); + if (!dev_open(mdac->area.dev)) + return_0; + if (!_raw_write_mda_header(fid->fmt, mdac->area.dev, mda_is_primary(mda), mdac->area.start, mdah)) { dm_pool_free(fid->fmt->cmd->mem, mdah); @@ -1227,7 +1209,7 @@ int read_metadata_location_summary(const struct format_type *fmt, return 0; } - bcache_read_bytes(scan_bcache, dev_area->dev->fd, dev_area->start + rlocn->offset, NAME_LEN, buf); + bcache_read_bytes(scan_bcache, dev_area->dev->bcache_fd, dev_area->start + rlocn->offset, NAME_LEN, buf); while (buf[len] && !isspace(buf[len]) && buf[len] != '{' && len < (NAME_LEN - 1)) @@ -1338,15 +1320,9 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu dm_list_iterate_items(rl, raw_list) { log_debug_metadata("Scanning independent dev %s", dev_name(rl->dev_area.dev)); - /* FIXME We're reading mdah twice here... */ - if (!dev_open_readonly(rl->dev_area.dev)) { - stack; - continue; - } - if (!(mdah = raw_read_mda_header(fmt, &rl->dev_area, 0))) { stack; - goto close_dev; + continue; } if (read_metadata_location_summary(fmt, mdah, 0, &rl->dev_area, &vgsummary, NULL)) { @@ -1356,9 +1332,6 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu lvmcache_set_independent_location(vg->name); } } - close_dev: - if (!dev_close(rl->dev_area.dev)) - stack; } return 1; @@ -1488,9 +1461,6 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume if (!lvmcache_update_das(info, pv)) return_0; - if (!dev_open(pv->dev)) - return_0; - baton.pv = pv; baton.fmt = fmt; @@ -1502,8 +1472,6 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume if (!label_write(pv->dev, label)) { stack; - if (!dev_close(pv->dev)) - stack; return 0; } @@ -1513,9 +1481,6 @@ static int _text_pv_write(const struct format_type *fmt, struct physical_volume * update the cache afterwards? */ - if (!dev_close(pv->dev)) - return_0; - return 1; } diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c index 1c322dd..206ae3f 100644 --- a/lib/format_text/text_label.c +++ b/lib/format_text/text_label.c @@ -338,12 +338,6 @@ static int _read_mda_header_and_metadata(struct metadata_area *mda, void *baton) * TODO: make lvmcache smarter and move this cache logic there */ - if (!dev_open_readonly(mdac->area.dev)) { - mda_set_ignored(mda, 1); - stack; - return 1; - } - if (!(mdah = raw_read_mda_header(fmt, &mdac->area, mda_is_primary(mda)))) { stack; goto close_dev; @@ -355,23 +349,16 @@ static int _read_mda_header_and_metadata(struct metadata_area *mda, void *baton) log_debug_metadata("Ignoring mda on device %s at offset " FMTu64, dev_name(mdac->area.dev), mdac->area.start); - if (!dev_close(mdac->area.dev)) - stack; return 1; } if (read_metadata_location_summary(fmt, mdah, mda_is_primary(mda), &mdac->area, &vgsummary, &mdac->free_sectors) && !lvmcache_update_vgname_and_id(p->info, &vgsummary)) { - if (!dev_close(mdac->area.dev)) - stack; return_0; } close_dev: - if (!dev_close(mdac->area.dev)) - stack; - return 1; } diff --git a/lib/label/label.c b/lib/label/label.c index 57d5248..2ec187c 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -116,6 +116,8 @@ int label_remove(struct device *dev) log_very_verbose("Scanning for labels to wipe from %s", dev_name(dev)); + label_scan_invalidate(dev); + if (!dev_open(dev)) return_0; @@ -125,8 +127,6 @@ int label_remove(struct device *dev) */ dev_flush(dev); - label_scan_invalidate(dev); - if (!dev_read(dev, UINT64_C(0), LABEL_SCAN_SIZE, DEV_IO_LABEL, readbuf)) { log_debug_devs("%s: Failed to read label area", dev_name(dev)); goto out; @@ -396,6 +396,71 @@ static int _process_block(struct device *dev, struct block *bb, int *is_lvm_devi return ret; } +static int _scan_dev_open(struct device *dev) +{ + const char *name; + int flags = 0; + int fd; + + if (dev->flags & DEV_IN_BCACHE) { + log_error("scan_dev_open %s DEV_IN_BCACHE already set", dev_name(dev)); + dev->flags &= ~DEV_IN_BCACHE; + } + + if (dev->bcache_fd > 0) { + log_error("scan_dev_open %s already open with fd %d", + dev_name(dev), dev->bcache_fd); + return 0; + } + + if (!(name = dev_name_confirmed(dev, 1))) { + log_error("scan_dev_open %s no name", dev_name(dev)); + return 0; + } + + flags |= O_RDWR; + flags |= O_DIRECT; + flags |= O_NOATIME; + + if (dev->flags & DEV_BCACHE_EXCL) + flags |= O_EXCL; + + fd = open(name, flags, 0777); + + if (fd < 0) { + if ((errno == EBUSY) && (flags & O_EXCL)) { + log_error("Can't open %s exclusively. Mounted filesystem?", + dev_name(dev)); + } else { + log_error("scan_dev_open %s failed errno %d", dev_name(dev), errno); + } + return 0; + } + + dev->flags |= DEV_IN_BCACHE; + dev->bcache_fd = fd; + return 1; +} + +static int _scan_dev_close(struct device *dev) +{ + if (!(dev->flags & DEV_IN_BCACHE)) + log_error("scan_dev_close %s no DEV_IN_BCACHE set", dev_name(dev)); + + dev->flags &= ~DEV_IN_BCACHE; + dev->flags &= ~DEV_BCACHE_EXCL; + + if (dev->bcache_fd < 0) { + log_error("scan_dev_close %s already closed", dev_name(dev)); + return 0; + } + + if (close(dev->bcache_fd)) + log_warn("close %s errno %d", dev_name(dev), errno); + dev->bcache_fd = -1; + return 1; +} + /* * Read or reread label/metadata from selected devs. * @@ -407,7 +472,7 @@ static int _process_block(struct device *dev, struct block *bb, int *is_lvm_devi * its info is removed from lvmcache. */ -static int _scan_list(struct dm_list *devs) +static int _scan_list(struct dm_list *devs, int *failed) { struct dm_list wait_devs; struct dm_list done_devs; @@ -439,19 +504,16 @@ static int _scan_list(struct dm_list *devs) if (!rem_prefetches) break; - /* - * The in-bcache flag corresponds with this dev_open. - * Clearing the in-bcache flag should be paired with - * a dev_close. (This dev may already be in bcache.) - */ if (!_in_bcache(devl->dev)) { - if (!dev_open_readonly(devl->dev)) { + if (!_scan_dev_open(devl->dev)) { log_debug_devs("%s: Failed to open device.", dev_name(devl->dev)); + dm_list_del(&devl->list); + scan_failed_count++; continue; } } - bcache_prefetch(scan_bcache, devl->dev->fd, 0); + bcache_prefetch(scan_bcache, devl->dev->bcache_fd, 0); rem_prefetches--; @@ -462,12 +524,12 @@ static int _scan_list(struct dm_list *devs) dm_list_iterate_items_safe(devl, devl2, &wait_devs) { bb = NULL; - if (!bcache_get(scan_bcache, devl->dev->fd, 0, 0, &bb)) { + if (!bcache_get(scan_bcache, devl->dev->bcache_fd, 0, 0, &bb)) { log_debug_devs("%s: Failed to scan device.", dev_name(devl->dev)); scan_failed_count++; scan_failed = 1; } else { - log_debug_devs("Processing data from device %s fd %d block %p", dev_name(devl->dev), devl->dev->fd, bb); + log_debug_devs("Processing data from device %s fd %d block %p", dev_name(devl->dev), devl->dev->bcache_fd, bb); _process_block(devl->dev, bb, &is_lvm_device); scan_lvm_count++; scan_failed = 0; @@ -483,12 +545,8 @@ static int _scan_list(struct dm_list *devs) * drop it from bcache. */ if (scan_failed || !is_lvm_device) { - devl->dev->flags &= ~DEV_IN_BCACHE; - bcache_invalidate_fd(scan_bcache, devl->dev->fd); - dev_close(devl->dev); - } else { - /* The device must be kept open while it's in bcache. */ - devl->dev->flags |= DEV_IN_BCACHE; + bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _scan_dev_close(devl->dev); } dm_list_del(&devl->list); @@ -498,12 +556,13 @@ static int _scan_list(struct dm_list *devs) if (!dm_list_empty(devs)) goto scan_more; - /* FIXME: let the caller know if some lvm devices failed to be scanned. */ - log_debug_devs("Scanned %d devices: %d for lvm, %d failed.", dm_list_size(&done_devs), scan_lvm_count, scan_failed_count); - return 0; + if (failed) + *failed = scan_failed_count; + + return 1; } /* @@ -548,8 +607,10 @@ int label_scan(struct cmd_context *cmd) * label_scan should not generally be called a second time, * so this will usually not be true. */ - if (_in_bcache(dev)) - bcache_invalidate_fd(scan_bcache, dev->fd); + if (_in_bcache(dev)) { + bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _scan_dev_close(dev); + } }; dev_iter_destroy(iter); @@ -573,7 +634,9 @@ int label_scan(struct cmd_context *cmd) return 0; } - return _scan_list(&all_devs); + _scan_list(&all_devs, NULL); + + return 1; } /* @@ -589,19 +652,48 @@ int label_scan_devs(struct cmd_context *cmd, struct dm_list *devs) struct device_list *devl; dm_list_iterate_items(devl, devs) { - if (_in_bcache(devl->dev)) - bcache_invalidate_fd(scan_bcache, devl->dev->fd); + if (_in_bcache(devl->dev)) { + bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _scan_dev_close(devl->dev); + } + } + + _scan_list(devs, NULL); + + /* FIXME: this function should probably fail if any devs couldn't be scanned */ + + return 1; +} + +int label_scan_devs_excl(struct dm_list *devs) +{ + struct device_list *devl; + int failed = 0; + + dm_list_iterate_items(devl, devs) { + if (_in_bcache(devl->dev)) { + bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _scan_dev_close(devl->dev); + } + /* + * With this flag set, _scan_dev_open() done by + * _scan_list() will do open EXCL + */ + devl->dev->flags |= DEV_BCACHE_EXCL; } - return _scan_list(devs); + _scan_list(devs, &failed); + + if (failed) + return 0; + return 1; } void label_scan_invalidate(struct device *dev) { if (_in_bcache(dev)) { - dev->flags &= ~DEV_IN_BCACHE; - bcache_invalidate_fd(scan_bcache, dev->fd); - dev_close(dev); + bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _scan_dev_close(dev); } } @@ -645,7 +737,7 @@ int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector { struct dm_list one_dev; struct device_list *devl; - int ret; + int failed = 0; /* scanning is done by list, so make a single item list for this dev */ if (!(devl = dm_zalloc(sizeof(*devl)))) @@ -654,10 +746,12 @@ int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector dm_list_init(&one_dev); dm_list_add(&one_dev, &devl->list); - if (_in_bcache(dev)) - bcache_invalidate_fd(scan_bcache, dev->fd); + if (_in_bcache(dev)) { + bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _scan_dev_close(dev); + } - ret = _scan_list(&one_dev); + _scan_list(&one_dev, &failed); /* * FIXME: this ugliness of returning a pointer to the label is @@ -671,7 +765,9 @@ int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector *labelp = lvmcache_get_label(info); } - return ret; + if (failed) + return 0; + return 1; } /* diff --git a/lib/label/label.h b/lib/label/label.h index d9e36bc..eb62f64 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -104,6 +104,7 @@ extern struct bcache *scan_bcache; int label_scan(struct cmd_context *cmd); int label_scan_devs(struct cmd_context *cmd, struct dm_list *devs); +int label_scan_devs_excl(struct dm_list *devs); void label_scan_invalidate(struct device *dev); void label_scan_destroy(struct cmd_context *cmd); int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector); diff --git a/lib/metadata/metadata-liblvm.c b/lib/metadata/metadata-liblvm.c index 388e8d9..d8b3b2a 100644 --- a/lib/metadata/metadata-liblvm.c +++ b/lib/metadata/metadata-liblvm.c @@ -241,6 +241,7 @@ static int _pvcreate_check(struct cmd_context *cmd, const char *name, name); goto out; } + dev_close(dev); if (!wipe_known_signatures(cmd, dev, name, TYPE_LVM1_MEMBER | TYPE_LVM2_MEMBER, diff --git a/tools/toollib.c b/tools/toollib.c index f6169ae..0cc2edd 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -5082,16 +5082,6 @@ static int _pvcreate_check_single(struct cmd_context *cmd, log_debug("Checking pvcreate arg %s which has existing PVID: %.32s.", pv_dev_name(pv), pv->dev->pvid[0] ? pv->dev->pvid : "<none>"); - /* - * This test will fail if the device belongs to an MD array. - */ - if (!dev_test_excl(pv->dev)) { - /* FIXME Detect whether device-mapper itself is still using it */ - log_error("Can't open %s exclusively. Mounted filesystem?", - pv_dev_name(pv)); - dm_list_move(&pp->arg_fail, &pd->list); - return 1; - } /* * Don't allow using a device with duplicates. @@ -5223,14 +5213,6 @@ static int _pv_confirm_single(struct cmd_context *cmd, if (!found) return 1; - /* Repeat the same from check_single. */ - if (!dev_test_excl(pv->dev)) { - /* FIXME Detect whether device-mapper itself is still using it */ - log_error("Can't open %s exclusively. Mounted filesystem?", - pv_dev_name(pv)); - goto fail; - } - /* * What kind of device is this: an orphan PV, an uninitialized/unused * device, a PV used in a VG. @@ -5323,16 +5305,6 @@ static int _pvremove_check_single(struct cmd_context *cmd, log_debug("Checking device %s for pvremove %.32s.", pv_dev_name(pv), pv->dev->pvid[0] ? pv->dev->pvid : ""); - /* - * This test will fail if the device belongs to an MD array. - */ - if (!dev_test_excl(pv->dev)) { - /* FIXME Detect whether device-mapper itself is still using it */ - log_error("Can't open %s exclusively. Mounted filesystem?", - pv_dev_name(pv)); - dm_list_move(&pp->arg_fail, &pd->list); - return 1; - } /* * Is there a pv here already? @@ -5458,8 +5430,10 @@ int pvcreate_each_device(struct cmd_context *cmd, struct volume_group *orphan_vg; struct dm_list remove_duplicates; struct dm_list arg_sort; + struct dm_list rescan_devs; struct pv_list *pvl; struct pv_list *vgpvl; + struct device_list *devl; const char *pv_name; int consistent = 0; int must_use_all = (cmd->cname->flags & MUST_USE_ALL_ARGS); @@ -5470,6 +5444,7 @@ int pvcreate_each_device(struct cmd_context *cmd, dm_list_init(&remove_duplicates); dm_list_init(&arg_sort); + dm_list_init(&rescan_devs); handle->custom_handle = pp; @@ -5715,6 +5690,19 @@ int pvcreate_each_device(struct cmd_context *cmd, do_command: + dm_list_iterate_items(pd, &pp->arg_process) { + if (!(devl = dm_pool_zalloc(cmd->mem, sizeof(*devl)))) + goto bad; + devl->dev = pd->dev; + dm_list_add(&rescan_devs, &devl->list); + } + + log_debug("Rescanning devices with exclusive open"); + if (!label_scan_devs_excl(&rescan_devs)) { + log_debug("Failed to rescan devs excl"); + goto bad; + } + /* * Reorder arg_process entries to match the original order of args. */

6 years

1
0
0 / 0

master - process_each_label: use lvmcache

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=4343280ebc0e2aae0de... Commit: 4343280ebc0e2aae0de5fe959c24574b13d0d7be Parent: f17c2cf7c614e8f6e4ffe9b7415d628ecab61d47 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Feb 9 11:24:40 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:46 2018 -0500 process_each_label: use lvmcache In the same way as the other process_each functions. In the common case all the info that's needed can be used from lvmcache after a label scan. But this means that unchosen devs for duplicate PVs need to be handled explicitly. --- tools/toollib.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 53 insertions(+), 7 deletions(-) diff --git a/tools/toollib.c b/tools/toollib.c index 6593195..f6169ae 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -1493,13 +1493,19 @@ int process_each_label(struct cmd_context *cmd, int argc, char **argv, struct label *label; struct dev_iter *iter; struct device *dev; - + struct lvmcache_info *info; + struct dm_list process_duplicates; + struct device_list *devl; int ret_max = ECMD_PROCESSED; int ret; int opt = 0; + dm_list_init(&process_duplicates); + log_set_report_object_type(LOG_REPORT_OBJECT_TYPE_LABEL); + lvmcache_label_scan(cmd); + if (argc) { for (; opt < argc; opt++) { if (!(dev = dev_cache_get(argv[opt], cmd->full_filter))) { @@ -1509,14 +1515,54 @@ int process_each_label(struct cmd_context *cmd, int argc, char **argv, continue; } + if (!(label = lvmcache_get_dev_label(dev))) { + if (!lvmcache_dev_is_unchosen_duplicate(dev)) { + log_error("No physical volume label read from %s.", argv[opt]); + ret_max = ECMD_FAILED; + } else { + if (!(devl = dm_malloc(sizeof(*devl)))) + return_0; + devl->dev = dev; + dm_list_add(&process_duplicates, &devl->list); + } + continue; + } + log_set_report_object_name_and_id(dev_name(dev), NULL); - if (!label_read(dev, &label, 0)) { - log_error("No physical volume label read from %s.", - argv[opt]); - ret_max = ECMD_FAILED; + ret = process_single_label(cmd, label, handle); + report_log_ret_code(ret); + + if (ret > ret_max) + ret_max = ret; + + log_set_report_object_name_and_id(NULL, NULL); + + if (sigint_caught()) + break; + } + + dm_list_iterate_items(devl, &process_duplicates) { + /* + * remove the existing dev for this pvid from lvmcache + * so that the duplicate dev can replace it. + */ + if ((info = lvmcache_info_from_pvid(devl->dev->pvid, NULL, 0))) + lvmcache_del(info); + + /* + * add info to lvmcache from the duplicate dev. + */ + label_read(devl->dev, NULL, 0); + + /* + * the info/label should now be found because + * the label_read should have added it. + */ + if (!(label = lvmcache_get_dev_label(devl->dev))) continue; - } + + log_set_report_object_name_and_id(dev_name(dev), NULL); ret = process_single_label(cmd, label, handle); report_log_ret_code(ret); @@ -1541,7 +1587,7 @@ int process_each_label(struct cmd_context *cmd, int argc, char **argv, while ((dev = dev_iter_get(iter))) { - if (!label_read(dev, &label, 0)) + if (!(label = lvmcache_get_dev_label(dev))) continue; log_set_report_object_name_and_id(dev_name(label->dev), NULL);

6 years

1
0
0 / 0

master - pvremove: device check doesn't require label_read

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=f17c2cf7c614e8f6e4f... Commit: f17c2cf7c614e8f6e4ffe9b7415d628ecab61d47 Parent: 29c6c171217753daf5603432bcaeaed2e8dc8418 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Feb 9 12:43:12 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:45 2018 -0500 pvremove: device check doesn't require label_read It just needs to check if the device was found during the scan, which means checking if it exists in lvmcache. --- lib/cache/lvmcache.c | 19 +++++++++++++++++++ lib/cache/lvmcache.h | 2 ++ tools/toollib.c | 13 ++++++++----- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 28e46bb..87bcc37 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -2562,6 +2562,25 @@ int lvmcache_foreach_ba(struct lvmcache_info *info, return 1; } +struct label *lvmcache_get_dev_label(struct device *dev) +{ + struct lvmcache_info *info; + + if ((info = lvmcache_info_from_pvid(dev->pvid, NULL, 0))) { + /* dev would be different for a duplicate */ + if (info->dev == dev) + return info->label; + } + return NULL; +} + +int lvmcache_has_dev_info(struct device *dev) +{ + if (lvmcache_info_from_pvid(dev->pvid, NULL, 0)) + return 1; + return 0; +} + /* * The lifetime of the label returned is tied to the lifetime of the * lvmcache_info which is the same as lvmcache itself. diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 1856344..3967b29 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -160,6 +160,8 @@ uint32_t lvmcache_ext_flags(struct lvmcache_info *info); const struct format_type *lvmcache_fmt(struct lvmcache_info *info); struct label *lvmcache_get_label(struct lvmcache_info *info); +struct label *lvmcache_get_dev_label(struct device *dev); +int lvmcache_has_dev_info(struct device *dev); void lvmcache_update_pv(struct lvmcache_info *info, struct physical_volume *pv, const struct format_type *fmt); diff --git a/tools/toollib.c b/tools/toollib.c index 1c216d8..6593195 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -5292,14 +5292,12 @@ static int _pvremove_check_single(struct cmd_context *cmd, * Is there a pv here already? * If not, this is an error unless you used -f. */ - if (!label_read(pd->dev, &label, 0)) { + if (!lvmcache_has_dev_info(pv->dev)) { if (pp->force) { dm_list_move(&pp->arg_process, &pd->list); return 1; } else { - log_error("No PV label found on %s.", pd->name); - dm_list_move(&pp->arg_fail, &pd->list); - return 1; + pd->is_not_pv = 1; } } @@ -5308,7 +5306,11 @@ static int _pvremove_check_single(struct cmd_context *cmd, * device, a PV used in a VG. */ - if (vg && !is_orphan_vg(vg->name)) { + if (pd->is_not_pv) { + /* Device is not a PV. */ + log_debug("Found pvremove arg %s: device is not a PV.", pd->name); + + } else if (vg && !is_orphan_vg(vg->name)) { /* Device is a PV used in a VG. */ log_debug("Found pvremove arg %s: pv is used in %s.", pd->name, vg->name); pd->is_vg_pv = 1; @@ -5330,6 +5332,7 @@ static int _pvremove_check_single(struct cmd_context *cmd, else pp->orphan_vg_name = FMT_TEXT_ORPHAN_VG_NAME; } else { + /* FIXME: is it possible to reach here? */ log_debug("Found pvremove arg %s: device is not a PV.", pd->name); /* Device is not a PV. */ pd->is_not_pv = 1;

6 years

1
0
0 / 0

master - format-text.c log message fixes

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=29c6c171217753daf56... Commit: 29c6c171217753daf5603432bcaeaed2e8dc8418 Parent: d9a77e8bb4d25af33625d3cdddf5288207101d5b Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Feb 7 15:20:39 2018 -0600 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:45 2018 -0500 format-text.c log message fixes --- lib/format_text/format-text.c | 56 ++++++++++++++++++++++------------------ 1 files changed, 31 insertions(+), 25 deletions(-) diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index ee1f11d..8740a05 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -468,12 +468,11 @@ static struct raw_locn *_read_metadata_location_vg(struct device_area *dev_area, (isspace(vgnamebuf[len]) || vgnamebuf[len] == '{')) return rlocn; - log_debug_metadata("Volume group name found in %smetadata on %s at " FMTu64 " does " - "not match expected name %s.", - *precommitted ? "precommitted " : "", - dev_name(dev_area->dev), dev_area->start + rlocn->offset, vgname); + log_error("Metadata on %s at %llu has wrong VG name \"%s\" expected %s.", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset), + vgnamebuf, vgname); - bad: if ((info = lvmcache_info_from_pvid(dev_area->dev->pvid, dev_area->dev, 0)) && !lvmcache_update_vgname_and_id(info, &vgsummary_orphan)) stack; @@ -553,8 +552,10 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid, wrap = (uint32_t) ((rlocn->offset + rlocn->size) - mdah->size); if (wrap > rlocn->offset) { - log_error("VG %s metadata on %s (" FMTu64 " bytes) too large for circular buffer (" FMTu64 " bytes)", - vgname, dev_name(area->dev), rlocn->size, mdah->size - MDA_HEADER_SIZE); + log_error("Metadata for VG %s on %s at %llu size %llu is too large for circular buffer.", + vgname, dev_name(area->dev), + (unsigned long long)(area->start + rlocn->offset), + (unsigned long long)rlocn->size); goto out; } @@ -572,10 +573,10 @@ static struct volume_group *_vg_read_raw_area(struct format_instance *fid, that skips parsing the metadata which also returns NULL. */ } - log_debug_metadata("Found metadata on %s at %"FMTu64" size %"FMTu64" for VG %s", + log_debug_metadata("Found metadata on %s at %llu size %llu for VG %s", dev_name(area->dev), - area->start + rlocn->offset, - rlocn->size, + (unsigned long long)(area->start + rlocn->offset), + (unsigned long long)rlocn->size, vgname); if (vg && precommitted) @@ -1220,8 +1221,9 @@ int read_metadata_location_summary(const struct format_type *fmt, * If no valid offset, do not try to search for vgname */ if (!rlocn->offset) { - log_debug_metadata("Metadata location on %s at %"FMTu64" has offset 0.", - dev_name(dev_area->dev), dev_area->start + rlocn->offset); + log_debug_metadata("Metadata location on %s at %llu has offset 0.", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset)); return 0; } @@ -1235,9 +1237,10 @@ int read_metadata_location_summary(const struct format_type *fmt, /* Ignore this entry if the characters aren't permissible */ if (!validate_name(buf)) { - log_error("Metadata location on %s at %"FMTu64" begins with invalid VG name.", - dev_name(dev_area->dev), dev_area->start + rlocn->offset); - return_0; + log_error("Metadata location on %s at %llu begins with invalid VG name.", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset)); + return 0; } /* We found a VG - now check the metadata */ @@ -1245,8 +1248,9 @@ int read_metadata_location_summary(const struct format_type *fmt, wrap = (uint32_t) ((rlocn->offset + rlocn->size) - mdah->size); if (wrap > rlocn->offset) { - log_error("Metadata location on %s at %"FMTu64" is too large for circular buffer.", - dev_name(dev_area->dev), dev_area->start + rlocn->offset); + log_error("Metadata location on %s at %llu is too large for circular buffer.", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset)); return 0; } @@ -1277,22 +1281,24 @@ int read_metadata_location_summary(const struct format_type *fmt, (off_t) (dev_area->start + MDA_HEADER_SIZE), wrap, calc_crc, vgsummary->vgname ? 1 : 0, vgsummary)) { - log_error("Metadata location on %s at %"FMTu64" has invalid summary for VG.", - dev_name(dev_area->dev), dev_area->start + rlocn->offset); + log_error("Metadata location on %s at %llu has invalid summary for VG.", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset)); return 0; } /* Ignore this entry if the characters aren't permissible */ if (!validate_name(vgsummary->vgname)) { - log_error("Metadata location on %s at %"FMTu64" has invalid VG name.", - dev_name(dev_area->dev), dev_area->start + rlocn->offset); + log_error("Metadata location on %s at %llu has invalid VG name.", + dev_name(dev_area->dev), + (unsigned long long)(dev_area->start + rlocn->offset)); return 0; } - log_debug_metadata("Found metadata summary on %s at %"FMTu64" size %"FMTu64" for VG %s", + log_debug_metadata("Found metadata summary on %s at %llu size %llu for VG %s", dev_name(dev_area->dev), - dev_area->start + rlocn->offset, - rlocn->size, + (unsigned long long)(dev_area->start + rlocn->offset), + (unsigned long long)rlocn->size, vgsummary->vgname); if (mda_free_sectors) { @@ -1344,7 +1350,7 @@ static int _scan_raw(const struct format_type *fmt, const char *vgname __attribu } if (read_metadata_location_summary(fmt, mdah, 0, &rl->dev_area, &vgsummary, NULL)) { - vg = _vg_read_raw_area(&fid, vgsummary.vgname, &rl->dev_area, NULL, NULL, 0, 0, 0); + vg = _vg_read_raw_area(&fid, vgsummary.vgname, &rl->dev_area, NULL, NULL, 0, 0); if (vg) { lvmcache_update_vg(vg, 0); lvmcache_set_independent_location(vg->name);

6 years

1
0
0 / 0

master - clvm: rescan when VG or PV not found

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=1409c4a1c2031984517... Commit: 1409c4a1c20319845171b91edb6310e25ae81792 Parent: c42a18d372f314aa51d9d7877342f1d78fd68973 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Fri Apr 20 16:09:49 2018 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 16:09:49 2018 -0500 clvm: rescan when VG or PV not found Rescan devices to update lvmcache content when clvmd vg_read doesn't find a VG or PV. --- lib/metadata/metadata.c | 28 ++++++++++++++++++++++++---- 1 files changed, 24 insertions(+), 4 deletions(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 685c589..2df9abe 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -4585,6 +4585,7 @@ 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); + label_scan_destroy(cmd); lvmcache_label_scan(cmd); } @@ -4597,14 +4598,33 @@ static struct volume_group *_vg_read_by_vgid(struct cmd_context *cmd, label_scan_setup_bcache(); - if ((vg = _vg_read(cmd, vgname, vgid, warn_flags, &consistent, precommitted))) { - /* Does it matter if consistent is 0 or 1? */ - label_scan_destroy(cmd); - return vg; + if (!(vg = _vg_read(cmd, vgname, vgid, warn_flags, &consistent, precommitted))) { + log_error("Rescan devices to look for missing VG."); + goto scan; + } + + if (vg_missing_pv_count(vg)) { + log_error("Rescan devices to look for missing PVs."); + release_vg(vg); + goto scan; } + label_scan_destroy(cmd); /* drop bcache to close devs, keep lvmcache */ + return vg; + + scan: + lvmcache_destroy(cmd, 0, 0); label_scan_destroy(cmd); + lvmcache_label_scan(cmd); + + if (!(vg = _vg_read(cmd, vgname, vgid, warn_flags, &consistent, precommitted))) + goto fail; + + label_scan_destroy(cmd); /* drop bcache to close devs, keep lvmcache */ + return vg; + fail: + label_scan_destroy(cmd); /* drop bache to close devs, keep lvmcache */ log_debug_metadata("Reading VG by vgid %.8s not found.", vgid); return NULL; }

6 years

1
0
0 / 0

master - lvmcache: simplify metadata cache

by David Teigland

Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=d9a77e8bb4d25af3362... Commit: d9a77e8bb4d25af33625d3cdddf5288207101d5b Parent: 79c4971210a6337563ffa2fca08fb636423d93d4 Author: David Teigland <teigland(a)redhat.com> AuthorDate: Wed Nov 1 09:35:40 2017 -0500 Committer: David Teigland <teigland(a)redhat.com> CommitterDate: Fri Apr 20 11:22:45 2018 -0500 lvmcache: simplify metadata cache The copy of VG metadata stored in lvmcache was not being used in general. It pretended to be a generic VG metadata cache, but was not being used except for clvmd activation. There it was used to avoid reading from disk while devices were suspended, i.e. in resume. This removes the code that attempted to make this look like a generic metadata cache, and replaces with with something narrowly targetted to what it's actually used for. This is a way of passing the VG from suspend to resume in clvmd. Since in the case of clvmd one caller can't simply pass the same VG to both suspend and resume, suspend needs to stash the VG somewhere that resume can grab it from. (resume doesn't want to read it from disk since devices are suspended.) The lvmcache vginfo struct is used as a convenient place to stash the VG to pass it from suspend to resume, even though it isn't related to the lvmcache or vginfo. These suspended_vg* vginfo fields should not be used or touched anywhere else, they are only to be used for passing the VG data from suspend to resume in clvmd. The VG data being passed between suspend and resume is never modified, and will only exist in the brief period between suspend and resume in clvmd. suspend has both old (current) and new (precommitted) copies of the VG metadata. It stashes both of these in the vginfo prior to suspending devices. When vg_commit is successful, it sets a flag in vginfo as before, signaling the transition from old to new metadata. resume grabs the VG stashed by suspend. If the vg_commit happened, it grabs the new VG, and if the vg_commit didn't happen it grabs the old VG. The VG is then used to resume LVs. This isolates clvmd-specific code and usage from the normal lvm vg_read code, making the code simpler and the behavior easier to verify. Sequence of operations: - lv_suspend() has both vg_old and vg_new and stashes a copy of each onto the vginfo: lvmcache_save_suspended_vg(vg_old); lvmcache_save_suspended_vg(vg_new); - vg_commit() happens, which causes all clvmd instances to call lvmcache_commit_metadata(vg). A flag is set in the vginfo indicating the transition from the old to new VG: vginfo->suspended_vg_committed = 1; - lv_resume() needs either vg_old or vg_new to use in resuming LVs. It doesn't want to read the VG from disk since devices are suspended, so it gets the VG stashed by lv_suspend: vg = lvmcache_get_suspended_vg(vgid); If the vg_commit did not happen, suspended_vg_committed will not be set, and in this case, lvmcache_get_suspended_vg() will return the old VG instead of the new VG, and it will resume LVs based on the old metadata. --- lib/activate/activate.c | 82 ++++++++- lib/cache/lvmcache.c | 405 +++++++++++++++++------------------------- lib/cache/lvmcache.h | 7 +- lib/commands/toolcontext.c | 4 - lib/config/config_settings.h | 7 +- lib/config/defaults.h | 1 - lib/metadata/metadata.c | 71 ++------ lib/metadata/vg.c | 5 - lib/misc/lvm-globals.c | 12 -- lib/misc/lvm-globals.h | 2 - tools/commands.h | 8 +- tools/lvmcmdline.c | 1 - tools/tools.h | 1 - 13 files changed, 264 insertions(+), 342 deletions(-) diff --git a/lib/activate/activate.c b/lib/activate/activate.c index 6611e99..565e643 100644 --- a/lib/activate/activate.c +++ b/lib/activate/activate.c @@ -28,6 +28,7 @@ #include "config.h" #include "segtype.h" #include "sharedlib.h" +#include "lvmcache.h" #include <limits.h> #include <fcntl.h> @@ -2172,6 +2173,17 @@ static int _lv_suspend(struct cmd_context *cmd, const char *lvid_s, if (!lv_info(cmd, lv, laopts->origin_only, &info, 0, 0)) goto_out; + /* + * Save old and new (current and precommitted) versions of the + * VG metadata for lv_resume() to use, since lv_resume can't + * read metadata given that devices are suspended. lv_resume() + * will resume LVs using the old/current metadata if the vg_commit + * did happen (or failed), and it will resume LVs using the + * new/precommitted metadata if the vg_commit succeeded. + */ + lvmcache_save_suspended_vg(lv->vg, 0); + lvmcache_save_suspended_vg(lv_pre->vg, 1); + if (!info.exists || info.suspended) { if (!error_if_not_suspended) { r = 1; @@ -2378,16 +2390,55 @@ static int _lv_resume(struct cmd_context *cmd, const char *lvid_s, struct lv_activate_opts *laopts, int error_if_not_active, const struct logical_volume *lv) { - const struct logical_volume *lv_to_free = NULL; struct dm_list *snh; + struct volume_group *vg = NULL; + struct logical_volume *lv_found = NULL; + const union lvid *lvid; + const char *vgid; struct lvinfo info; int r = 0; if (!activation()) return 1; - if (!lv && !(lv_to_free = lv = lv_from_lvid(cmd, lvid_s, 0))) - goto_out; + /* + * When called in clvmd, lvid_s is set and lv is not. We need to + * get the VG metadata without reading disks because devs are + * suspended. lv_suspend() saved old and new VG metadata for us + * to use here. If vg_commit() happened, lvmcache_get_suspended_vg + * will return the new metadata for us to use in resuming LVs. + * If vg_commit() did not happen, lvmcache_get_suspended_vg + * returns the old metadata which we use to resume LVs. + */ + if (!lv && lvid_s) { + lvid = (const union lvid *) lvid_s; + vgid = (const char *)lvid->id[0].uuid; + + if ((vg = lvmcache_get_suspended_vg(vgid))) { + log_debug_activation("Resuming LVID %s found saved vg seqno %d %s", lvid_s, vg->seqno, vg->name); + if ((lv_found = find_lv_in_vg_by_lvid(vg, lvid))) { + log_debug_activation("Resuming LVID %s found saved LV %s", lvid_s, display_lvname(lv_found)); + lv = lv_found; + } else + log_debug_activation("Resuming LVID %s did not find saved LV", lvid_s); + } else + log_debug_activation("Resuming LVID %s did not find saved VG", lvid_s); + + /* + * resume must have been called without a preceding suspend, + * so we need to read the vg. + */ + + if (!lv) { + log_debug_activation("Resuming LVID %s reading VG", lvid_s); + if (!(lv_found = lv_from_lvid(cmd, lvid_s, 0))) { + log_debug_activation("Resuming LVID %s failed to read VG", lvid_s); + goto out; + } + + lv = lv_found; + } + } if (!lv_is_origin(lv) && !lv_is_thin_volume(lv) && !lv_is_thin_pool(lv)) laopts->origin_only = 0; @@ -2448,9 +2499,6 @@ needs_resume: r = 1; out: - if (lv_to_free) - release_vg(lv_to_free->vg); - return r; } @@ -2587,6 +2635,10 @@ int lv_activation_filter(struct cmd_context *cmd, const char *lvid_s, int *activate_lv, const struct logical_volume *lv) { const struct logical_volume *lv_to_free = NULL; + struct volume_group *vg = NULL; + struct logical_volume *lv_found = NULL; + const union lvid *lvid; + const char *vgid; int r = 0; if (!activation()) { @@ -2594,6 +2646,24 @@ int lv_activation_filter(struct cmd_context *cmd, const char *lvid_s, return 1; } + /* + * This function is called while devices are suspended, + * so try to use the copy of the vg that was saved in + * lv_suspend. + */ + if (!lv && lvid_s) { + lvid = (const union lvid *) lvid_s; + vgid = (const char *)lvid->id[0].uuid; + + if ((vg = lvmcache_get_suspended_vg(vgid))) { + log_debug_activation("activation_filter for %s found saved VG seqno %d %s", lvid_s, vg->seqno, vg->name); + if ((lv_found = find_lv_in_vg_by_lvid(vg, lvid))) { + log_debug_activation("activation_filter for %s found saved LV %s", lvid_s, display_lvname(lv_found)); + lv = lv_found; + } + } + } + if (!lv && !(lv_to_free = lv = lv_from_lvid(cmd, lvid_s, 0))) goto_out; diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 8e119b5..28e46bb 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -63,16 +63,42 @@ struct lvmcache_vginfo { char *lock_type; uint32_t mda_checksum; size_t mda_size; - size_t vgmetadata_size; - char *vgmetadata; /* Copy of VG metadata as format_text string */ - struct dm_config_tree *cft; /* Config tree created from vgmetadata */ - /* Lifetime is directly tied to vgmetadata */ - struct volume_group *cached_vg; - unsigned holders; - unsigned vg_use_count; /* Counter of vg reusage */ - unsigned precommitted; /* Is vgmetadata live or precommitted? */ - unsigned cached_vg_invalidated; /* Signal to regenerate cached_vg */ int independent_metadata_location; /* metadata read from independent areas */ + + /* + * The following are not related to lvmcache or vginfo, + * but are borrowing the vginfo to store the data. + * + * suspended_vg_* are used only by clvmd suspend/resume. + * In suspend, both old (current) and new (precommitted) + * metadata is saved. (Each in three forms: buffer, cft, + * and vg). In resume, if the vg was committed + * (suspended_vg_committed is set), then LVs are resumed + * using the new metadata, but if the vg wasn't committed, + * then LVs are resumed using the old metadata. + * + * suspended_vg_committed is set to 1 when clvmd gets + * LCK_VG_COMMIT from vg_commit(). + * + * These fields are only used between suspend and resume + * in clvmd, and should never be used in any other way. + * The contents of this data are never changed. This + * data does not really belong in lvmcache, it's unrelated + * to lvmcache or vginfo, but it's just a convenient place + * for clvmd to stash the VG between suspend and resume + * (since the same caller isn't present to pass the VG to + * both suspend and resume in the case of clvmd.) + * + * This data is not really a "cache" of the VG, it is just + * a location to pass the VG between suspend and resume. + */ + int suspended_vg_committed; + char *suspended_vg_old_buf; + struct dm_config_tree *suspended_vg_old_cft; + struct volume_group *suspended_vg_old; + char *suspended_vg_new_buf; + struct dm_config_tree *suspended_vg_new_cft; + struct volume_group *suspended_vg_new; }; static struct dm_hash_table *_pvid_hash = NULL; @@ -139,73 +165,7 @@ void lvmcache_seed_infos_from_lvmetad(struct cmd_context *cmd) _has_scanned = 1; } -/* Volume Group metadata cache functions */ -static void _free_cached_vgmetadata(struct lvmcache_vginfo *vginfo) -{ - if (!vginfo || !vginfo->vgmetadata) - return; - - dm_free(vginfo->vgmetadata); - - vginfo->vgmetadata = NULL; - - /* Release also cached config tree */ - if (vginfo->cft) { - dm_config_destroy(vginfo->cft); - vginfo->cft = NULL; - } - - log_debug_cache("lvmcache: VG %s wiped.", vginfo->vgname); - - release_vg(vginfo->cached_vg); -} - -/* - * Cache VG metadata against the vginfo with matching vgid. - */ -static void _store_metadata(struct volume_group *vg, unsigned precommitted) -{ - char uuid[64] __attribute__((aligned(8))); - struct lvmcache_vginfo *vginfo; - char *data; - size_t size; - - if (!(vginfo = lvmcache_vginfo_from_vgid((const char *)&vg->id))) { - stack; - return; - } - - if (!(size = export_vg_to_buffer(vg, &data))) { - stack; - _free_cached_vgmetadata(vginfo); - return; - } - - /* Avoid reparsing of the same data string */ - if (vginfo->vgmetadata && vginfo->vgmetadata_size == size && - strcmp(vginfo->vgmetadata, data) == 0) - dm_free(data); - else { - _free_cached_vgmetadata(vginfo); - vginfo->vgmetadata_size = size; - vginfo->vgmetadata = data; - } - - vginfo->precommitted = precommitted; - - if (!id_write_format((const struct id *)vginfo->vgid, uuid, sizeof(uuid))) { - stack; - return; - } - - log_debug_cache("lvmcache: VG %s (%s) stored (%" PRIsize_t " bytes%s).", - vginfo->vgname, uuid, size, - precommitted ? ", precommitted" : ""); -} - -static void _update_cache_info_lock_state(struct lvmcache_info *info, - int locked, - int *cached_vgmetadata_valid) +static void _update_cache_info_lock_state(struct lvmcache_info *info, int locked) { int was_locked = (info->status & CACHE_LOCKED) ? 1 : 0; @@ -213,10 +173,8 @@ static void _update_cache_info_lock_state(struct lvmcache_info *info, * 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)) { + if (!lvmcache_vgname_is_locked(VG_GLOBAL) && (was_locked != locked)) info->status |= CACHE_INVALID; - *cached_vgmetadata_valid = 0; - } if (locked) info->status |= CACHE_LOCKED; @@ -228,14 +186,9 @@ static void _update_cache_vginfo_lock_state(struct lvmcache_vginfo *vginfo, int locked) { struct lvmcache_info *info; - int cached_vgmetadata_valid = 1; dm_list_iterate_items(info, &vginfo->infos) - _update_cache_info_lock_state(info, locked, - &cached_vgmetadata_valid); - - if (!cached_vgmetadata_valid) - _free_cached_vgmetadata(vginfo); + _update_cache_info_lock_state(info, locked); } static void _update_cache_lock_state(const char *vgname, int locked) @@ -248,6 +201,35 @@ static void _update_cache_lock_state(const char *vgname, int locked) _update_cache_vginfo_lock_state(vginfo, locked); } +static void _suspended_vg_free(struct lvmcache_vginfo *vginfo, int free_old, int free_new) +{ + if (free_old) { + if (vginfo->suspended_vg_old_buf) + dm_free(vginfo->suspended_vg_old_buf); + if (vginfo->suspended_vg_old_cft) + dm_config_destroy(vginfo->suspended_vg_old_cft); + if (vginfo->suspended_vg_old) + release_vg(vginfo->suspended_vg_old); + + vginfo->suspended_vg_old_buf = NULL; + vginfo->suspended_vg_old_cft = NULL; + vginfo->suspended_vg_old = NULL; + } + + if (free_new) { + if (vginfo->suspended_vg_new_buf) + dm_free(vginfo->suspended_vg_new_buf); + if (vginfo->suspended_vg_new_cft) + dm_config_destroy(vginfo->suspended_vg_new_cft); + if (vginfo->suspended_vg_new) + release_vg(vginfo->suspended_vg_new); + + vginfo->suspended_vg_new_buf = NULL; + vginfo->suspended_vg_new_cft = NULL; + vginfo->suspended_vg_new = NULL; + } +} + static void _drop_metadata(const char *vgname, int drop_precommitted) { struct lvmcache_vginfo *vginfo; @@ -256,25 +238,98 @@ static void _drop_metadata(const char *vgname, int drop_precommitted) if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, NULL))) return; - /* - * Invalidate cached PV labels. - * If cached precommitted metadata exists that means we - * already invalidated the PV labels (before caching it) - * and we must not do it again. - */ - if (!drop_precommitted && vginfo->precommitted && !vginfo->vgmetadata) - log_error(INTERNAL_ERROR "metadata commit (or revert) missing before " - "dropping metadata from cache."); - - if (drop_precommitted || !vginfo->precommitted) + if (drop_precommitted) dm_list_iterate_items(info, &vginfo->infos) info->status |= CACHE_INVALID; - _free_cached_vgmetadata(vginfo); - - /* VG revert */ if (drop_precommitted) - vginfo->precommitted = 0; + _suspended_vg_free(vginfo, 0, 1); + else + _suspended_vg_free(vginfo, 1, 1); +} + +void lvmcache_save_suspended_vg(struct volume_group *vg, int precommitted) +{ + struct lvmcache_vginfo *vginfo; + struct format_instance *fid; + struct format_instance_ctx fic; + struct volume_group *susp_vg = NULL; + struct dm_config_tree *susp_cft = NULL; + char *susp_buf = NULL; + size_t size; + int new = precommitted; + int old = !precommitted; + + if (!(vginfo = lvmcache_vginfo_from_vgid((const char *)&vg->id))) + goto_bad; + + /* already saved */ + if (old && vginfo->suspended_vg_old && + (vginfo->suspended_vg_old->seqno == vg->seqno)) + return; + + /* already saved */ + if (new && vginfo->suspended_vg_new && + (vginfo->suspended_vg_new->seqno == vg->seqno)) + return; + + _suspended_vg_free(vginfo, old, new); + + if (!(size = export_vg_to_buffer(vg, &susp_buf))) + goto_bad; + + fic.type = FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS; + fic.context.vg_ref.vg_name = vginfo->vgname; + fic.context.vg_ref.vg_id = vginfo->vgid; + if (!(fid = vginfo->fmt->ops->create_instance(vginfo->fmt, &fic))) + goto_bad; + + if (!(susp_cft = config_tree_from_string_without_dup_node_check(susp_buf))) + goto_bad; + + if (!(susp_vg = import_vg_from_config_tree(susp_cft, fid))) + goto_bad; + + if (old) { + vginfo->suspended_vg_old_buf = susp_buf; + vginfo->suspended_vg_old_cft = susp_cft; + vginfo->suspended_vg_old = susp_vg; + log_debug_cache("lvmcache saved suspended vg old seqno %d %s", vg->seqno, vg->name); + } else { + vginfo->suspended_vg_new_buf = susp_buf; + vginfo->suspended_vg_new_cft = susp_cft; + vginfo->suspended_vg_new = susp_vg; + log_debug_cache("lvmcache saved suspended vg new seqno %d %s", vg->seqno, vg->name); + } + return; + +bad: + _suspended_vg_free(vginfo, old, new); + log_debug_cache("lvmcache failed to save suspended pre %d vg %s", precommitted, vg->name); +} + +struct volume_group *lvmcache_get_suspended_vg(const char *vgid) +{ + struct lvmcache_vginfo *vginfo; + + if (!(vginfo = lvmcache_vginfo_from_vgid(vgid))) + return_NULL; + + + if (vginfo->suspended_vg_committed) + return vginfo->suspended_vg_new; + else + return vginfo->suspended_vg_old; +} + +void lvmcache_drop_suspended_vg(struct volume_group *vg) +{ + struct lvmcache_vginfo *vginfo; + + if (!(vginfo = lvmcache_vginfo_from_vgid((const char *)&vg->id))) + return; + + _suspended_vg_free(vginfo, 1, 1); } /* @@ -289,11 +344,7 @@ void lvmcache_commit_metadata(const char *vgname) if (!(vginfo = lvmcache_vginfo_from_vgname(vgname, NULL))) return; - if (vginfo->precommitted) { - log_debug_cache("lvmcache: Upgraded pre-committed VG %s metadata to committed.", - vginfo->vgname); - vginfo->precommitted = 0; - } + vginfo->suspended_vg_committed = 1; } void lvmcache_drop_metadata(const char *vgname, int drop_precommitted) @@ -674,18 +725,6 @@ static int _info_is_valid(struct lvmcache_info *info) return 1; } -static int _vginfo_is_valid(struct lvmcache_vginfo *vginfo) -{ - struct lvmcache_info *info; - - /* Invalid if any info is invalid */ - dm_list_iterate_items(info, &vginfo->infos) - if (!_info_is_valid(info)) - 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) { @@ -1315,121 +1354,6 @@ int lvmcache_label_scan(struct cmd_context *cmd) return r; } -struct volume_group *lvmcache_get_vg(struct cmd_context *cmd, const char *vgname, - const char *vgid, unsigned precommitted) -{ - struct lvmcache_vginfo *vginfo; - struct volume_group *vg = NULL; - struct format_instance *fid; - struct format_instance_ctx fic; - - /* - * We currently do not store precommitted metadata in lvmetad at - * all. This means that any request for precommitted metadata is served - * using the classic scanning mechanics, and read from disk or from - * lvmcache. - */ - if (lvmetad_used() && !precommitted) { - /* Still serve the locally cached VG if available */ - if (vgid && (vginfo = lvmcache_vginfo_from_vgid(vgid)) && - vginfo->vgmetadata && (vg = vginfo->cached_vg)) - goto out; - return lvmetad_vg_lookup(cmd, vgname, vgid); - } - - if (!vgid || !(vginfo = lvmcache_vginfo_from_vgid(vgid)) || !vginfo->vgmetadata) - return NULL; - - if (!_vginfo_is_valid(vginfo)) - return NULL; - - /* - * Don't return cached data if either: - * (i) precommitted metadata is requested but we don't have it cached - * - caller should read it off disk; - * (ii) live metadata is requested but we have precommitted metadata cached - * and no devices are suspended so caller may read it off disk. - * - * If live metadata is requested but we have precommitted metadata cached - * and devices are suspended, we assume this precommitted metadata has - * already been preloaded and committed so it's OK to return it as live. - * Note that we do not clear the PRECOMMITTED flag. - */ - if ((precommitted && !vginfo->precommitted) || - (!precommitted && vginfo->precommitted && !critical_section())) - return NULL; - - /* Use already-cached VG struct when available */ - if ((vg = vginfo->cached_vg) && !vginfo->cached_vg_invalidated) - goto out; - - release_vg(vginfo->cached_vg); - - fic.type = FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS; - fic.context.vg_ref.vg_name = vginfo->vgname; - fic.context.vg_ref.vg_id = vgid; - if (!(fid = vginfo->fmt->ops->create_instance(vginfo->fmt, &fic))) - return_NULL; - - /* Build config tree from vgmetadata, if not yet cached */ - if (!vginfo->cft && - !(vginfo->cft = - config_tree_from_string_without_dup_node_check(vginfo->vgmetadata))) - goto_bad; - - if (!(vg = import_vg_from_config_tree(vginfo->cft, fid))) - goto_bad; - - /* Cache VG struct for reuse */ - vginfo->cached_vg = vg; - vginfo->holders = 1; - vginfo->vg_use_count = 0; - vginfo->cached_vg_invalidated = 0; - vg->vginfo = vginfo; - - if (!dm_pool_lock(vg->vgmem, detect_internal_vg_cache_corruption())) - goto_bad; - -out: - vginfo->holders++; - vginfo->vg_use_count++; - log_debug_cache("Using cached %smetadata for VG %s with %u holder(s).", - vginfo->precommitted ? "pre-committed " : "", - vginfo->vgname, vginfo->holders); - - return vg; - -bad: - _free_cached_vgmetadata(vginfo); - return NULL; -} - -// #if 0 -int lvmcache_vginfo_holders_dec_and_test_for_zero(struct lvmcache_vginfo *vginfo) -{ - log_debug_cache("VG %s decrementing %d holder(s) at %p.", - vginfo->cached_vg->name, vginfo->holders, vginfo->cached_vg); - - if (--vginfo->holders) - return 0; - - if (vginfo->vg_use_count > 1) - log_debug_cache("VG %s reused %d times.", - vginfo->cached_vg->name, vginfo->vg_use_count); - - /* Debug perform crc check only when it's been used more then once */ - if (!dm_pool_unlock(vginfo->cached_vg->vgmem, - detect_internal_vg_cache_corruption() && - (vginfo->vg_use_count > 1))) - stack; - - vginfo->cached_vg->vginfo = NULL; - vginfo->cached_vg = NULL; - - return 1; -} -// #endif - int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal, struct dm_list *vgnameids) { @@ -1615,8 +1539,6 @@ static int _free_vginfo(struct lvmcache_vginfo *vginfo) struct lvmcache_vginfo *primary_vginfo, *vginfo2; int r = 1; - _free_cached_vgmetadata(vginfo); - vginfo2 = primary_vginfo = lvmcache_vginfo_from_vgname(vginfo->vgname, NULL); if (vginfo == primary_vginfo) { @@ -1639,6 +1561,7 @@ static int _free_vginfo(struct lvmcache_vginfo *vginfo) dm_free(vginfo->system_id); dm_free(vginfo->vgname); dm_free(vginfo->creation_host); + _suspended_vg_free(vginfo, 1, 1); if (*vginfo->vgid && _vgid_hash && lvmcache_vginfo_from_vgid(vginfo->vgid) == vginfo) @@ -2076,12 +1999,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 making a PV into an orphan, any cached VG metadata may become - * invalid, incorrectly still referencing device structs. - * (Example: pvcreate -ff) */ - if (is_orphan_vg(vgname) && info->vginfo && !is_orphan_vg(info->vginfo->vgname)) - info->vginfo->cached_vg_invalidated = 1; - /* If moving PV from orphan to real VG, always mark it valid */ if (!is_orphan_vg(vgname)) info->status &= ~CACHE_INVALID; @@ -2117,10 +2034,6 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted) return_0; } - /* store text representation of vg to cache */ - if (vg->cmd->current_settings.cache_vgmetadata) - _store_metadata(vg, precommitted); - return 1; } @@ -2657,6 +2570,10 @@ 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; } diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 1b5379c..1856344 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -133,9 +133,6 @@ int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal, struct dm_list *lvmcache_get_pvids(struct cmd_context *cmd, const char *vgname, const char *vgid); -/* Returns cached volume group metadata. */ -struct volume_group *lvmcache_get_vg(struct cmd_context *cmd, const char *vgname, - const char *vgid, unsigned precommitted); void lvmcache_drop_metadata(const char *vgname, int drop_precommitted); void lvmcache_commit_metadata(const char *vgname); @@ -219,4 +216,8 @@ int lvmcache_get_vg_devs(struct cmd_context *cmd, struct dm_list *devs); void lvmcache_set_independent_location(const char *vgname); +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); + #endif diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index fe6b8a3..3dc3e2d 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -688,9 +688,6 @@ static int _process_config(struct cmd_context *cmd) if (find_config_tree_bool(cmd, report_two_word_unknown_device_CFG, NULL)) init_unknown_device_name("unknown device"); - init_detect_internal_vg_cache_corruption - (find_config_tree_bool(cmd, global_detect_internal_vg_cache_corruption_CFG, NULL)); - if (!_init_system_id(cmd)) return_0; @@ -2010,7 +2007,6 @@ struct cmd_context *create_toolcontext(unsigned is_long_lived, if (set_filters && !init_filters(cmd, 1)) goto_out; - cmd->default_settings.cache_vgmetadata = 1; cmd->current_settings = cmd->default_settings; cmd->initialized.config = 1; diff --git a/lib/config/config_settings.h b/lib/config/config_settings.h index 077fb15..b778f4c 100644 --- a/lib/config/config_settings.h +++ b/lib/config/config_settings.h @@ -868,11 +868,8 @@ cfg(global_abort_on_internal_errors_CFG, "abort_on_internal_errors", global_CFG_ "Treat any internal errors as fatal errors, aborting the process that\n" "encountered the internal error. Please only enable for debugging.\n") -cfg(global_detect_internal_vg_cache_corruption_CFG, "detect_internal_vg_cache_corruption", global_CFG_SECTION, 0, CFG_TYPE_BOOL, DEFAULT_DETECT_INTERNAL_VG_CACHE_CORRUPTION, vsn(2, 2, 96), NULL, 0, NULL, - "Internal verification of VG structures.\n" - "Check if CRC matches when a parsed VG is used multiple times. This\n" - "is useful to catch unexpected changes to cached VG structures.\n" - "Please only enable for debugging.\n") +cfg(global_detect_internal_vg_cache_corruption_CFG, "detect_internal_vg_cache_corruption", global_CFG_SECTION, 0, CFG_TYPE_BOOL, 0, vsn(2, 2, 96), NULL, vsn(2, 2, 174), NULL, + "No longer used.\n") cfg(global_metadata_read_only_CFG, "metadata_read_only", global_CFG_SECTION, 0, CFG_TYPE_BOOL, DEFAULT_METADATA_READ_ONLY, vsn(2, 2, 75), NULL, 0, NULL, "No operations that change on-disk metadata are permitted.\n" diff --git a/lib/config/defaults.h b/lib/config/defaults.h index d9e19d9..7cebd84 100644 --- a/lib/config/defaults.h +++ b/lib/config/defaults.h @@ -179,7 +179,6 @@ #define DEFAULT_LOGLEVEL 0 #define DEFAULT_INDENT 1 #define DEFAULT_ABORT_ON_INTERNAL_ERRORS 0 -#define DEFAULT_DETECT_INTERNAL_VG_CACHE_CORRUPTION 0 #define DEFAULT_UNITS "r" #define DEFAULT_SUFFIX 1 #define DEFAULT_HOSTTAGS 0 diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 570cbe6..5d3f835 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -3767,7 +3767,6 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, struct dm_list all_pvs; char uuid[64] __attribute__((aligned(8))); - unsigned seqno = 0; int reappeared = 0; struct cached_vg_fmtdata *vg_fmtdata = NULL; /* Additional format-specific data about the vg */ unsigned use_previous_vg; @@ -3788,7 +3787,7 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, } if (lvmetad_used() && !use_precommitted) { - if ((correct_vg = lvmcache_get_vg(cmd, vgname, vgid, precommitted))) { + if ((correct_vg = lvmetad_vg_lookup(cmd, vgname, vgid))) { dm_list_iterate_items(pvl, &correct_vg->pvs) reappeared += _check_reappeared_pv(correct_vg, pvl->pv, *consistent); if (reappeared && *consistent) @@ -3819,23 +3818,6 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, } /* - * If cached metadata was inconsistent and *consistent is set - * then repair it now. Otherwise just return it. - * Also return if use_precommitted is set due to the FIXME in - * the missing PV logic below. - */ - if ((correct_vg = lvmcache_get_vg(cmd, vgname, vgid, precommitted)) && - (use_precommitted || !*consistent)) { - *consistent = 1; - return correct_vg; - } else { - if (correct_vg && correct_vg->seqno > seqno) - seqno = correct_vg->seqno; - release_vg(correct_vg); - correct_vg = NULL; - } - - /* * Rescan the devices that are associated with this vg in lvmcache. * This repeats what was done by the command's initial label scan, * but only the devices associated with this VG. @@ -4521,21 +4503,10 @@ static struct volume_group *_vg_read_by_vgid(struct cmd_context *cmd, unsigned precommitted) { const char *vgname; - struct dm_list *vgnames; struct volume_group *vg; - struct dm_str_list *strl; uint32_t warn_flags = WARN_PV_READ | WARN_INCONSISTENT; int consistent = 0; - /* Is corresponding vgname already cached? */ - if (lvmcache_vgid_is_cached(vgid)) { - if ((vg = _vg_read(cmd, NULL, vgid, warn_flags, &consistent, precommitted)) && - id_equal(&vg->id, (const struct id *)vgid)) { - return vg; - } - release_vg(vg); - } - /* * When using lvmlockd we should never reach this point. * The VG is locked, then vg_read() is done, which gets @@ -4548,36 +4519,28 @@ static struct volume_group *_vg_read_by_vgid(struct cmd_context *cmd, /* Mustn't scan if memory locked: ensure cache gets pre-populated! */ if (critical_section()) - return_NULL; + log_debug_metadata("Reading VG by vgid in critical section pre %d vgid %.8s", precommitted, vgid); - /* FIXME Need a genuine read by ID here - don't vg_read_internal by name! */ - /* FIXME Disabled vgrenames while active for now because we aren't - * allowed to do a full scan here any more. */ + 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); + } - // The slow way - full scan required to cope with vgrename - lvmcache_force_next_label_scan(); - lvmcache_label_scan(cmd); - if (!(vgnames = get_vgnames(cmd, 0))) { - log_error("vg_read_by_vgid: get_vgnames failed"); + if (!(vgname = lvmcache_vgname_from_vgid(cmd->mem, vgid))) { + log_debug_metadata("Reading VG by vgid %.8s no VG name found.", vgid); return NULL; } - dm_list_iterate_items(strl, vgnames) { - vgname = strl->str; - if (!vgname) - continue; // FIXME Unnecessary? - consistent = 0; - if ((vg = _vg_read(cmd, vgname, vgid, warn_flags, &consistent, precommitted)) && - id_equal(&vg->id, (const struct id *)vgid)) { - if (!consistent) { - release_vg(vg); - return NULL; - } - return vg; - } - release_vg(vg); + consistent = 0; + + if ((vg = _vg_read(cmd, vgname, vgid, warn_flags, &consistent, precommitted))) { + /* Does it matter if consistent is 0 or 1? */ + return vg; } + log_debug_metadata("Reading VG by vgid %.8s not found.", vgid); return NULL; } @@ -4593,7 +4556,7 @@ struct logical_volume *lv_from_lvid(struct cmd_context *cmd, const char *lvid_s, log_very_verbose("Finding %svolume group for uuid %s", precommitted ? "precommitted " : "", lvid_s); if (!(vg = _vg_read_by_vgid(cmd, (const char *)lvid->id[0].uuid, precommitted))) { - log_error("Volume group for uuid not found: %s", lvid_s); + log_error("Reading VG not found for LVID %s", lvid_s); return NULL; } diff --git a/lib/metadata/vg.c b/lib/metadata/vg.c index 4c808da..0b69e42 100644 --- a/lib/metadata/vg.c +++ b/lib/metadata/vg.c @@ -97,11 +97,6 @@ void release_vg(struct volume_group *vg) if (!vg || (vg->fid && vg == vg->fid->fmt->orphan_vg)) return; - /* Check if there are any vginfo holders */ - if (vg->vginfo && - !lvmcache_vginfo_holders_dec_and_test_for_zero(vg->vginfo)) - return; - release_vg(vg->vg_committed); release_vg(vg->vg_precommitted); _free_vg(vg); diff --git a/lib/misc/lvm-globals.c b/lib/misc/lvm-globals.c index 0575d21..0f384bb 100644 --- a/lib/misc/lvm-globals.c +++ b/lib/misc/lvm-globals.c @@ -53,8 +53,6 @@ static int _activation_checks = 0; static char _sysfs_dir_path[PATH_MAX] = ""; static int _dev_disable_after_error_count = DEFAULT_DISABLE_AFTER_ERROR_COUNT; static uint64_t _pv_min_size = (DEFAULT_PV_MIN_SIZE_KB * 1024L >> SECTOR_SHIFT); -static int _detect_internal_vg_cache_corruption = - DEFAULT_DETECT_INTERNAL_VG_CACHE_CORRUPTION; static const char *_unknown_device_name = DEFAULT_UNKNOWN_DEVICE_NAME; void init_verbose(int level) @@ -197,11 +195,6 @@ void init_pv_min_size(uint64_t sectors) _pv_min_size = sectors; } -void init_detect_internal_vg_cache_corruption(int detect) -{ - _detect_internal_vg_cache_corruption = detect; -} - void set_cmd_name(const char *cmd) { (void) dm_strncpy(_cmd_name, cmd, sizeof(_cmd_name)); @@ -384,11 +377,6 @@ uint64_t pv_min_size(void) return _pv_min_size; } -int detect_internal_vg_cache_corruption(void) -{ - return _detect_internal_vg_cache_corruption; -} - const char *unknown_device_name(void) { return _unknown_device_name; diff --git a/lib/misc/lvm-globals.h b/lib/misc/lvm-globals.h index 14a7d43..e23d598 100644 --- a/lib/misc/lvm-globals.h +++ b/lib/misc/lvm-globals.h @@ -51,7 +51,6 @@ void init_udev_checking(int checking); void init_dev_disable_after_error_count(int value); void init_pv_min_size(uint64_t sectors); void init_activation_checks(int checks); -void init_detect_internal_vg_cache_corruption(int detect); void init_retry_deactivation(int retry); void init_unknown_device_name(const char *name); @@ -85,7 +84,6 @@ int udev_checking(void); const char *sysfs_dir_path(void); uint64_t pv_min_size(void); int activation_checks(void); -int detect_internal_vg_cache_corruption(void); int retry_deactivation(void); const char *unknown_device_name(void); diff --git a/tools/commands.h b/tools/commands.h index d65330a..cbd527b 100644 --- a/tools/commands.h +++ b/tools/commands.h @@ -43,7 +43,7 @@ xx(lastlog, xx(lvchange, "Change the attributes of logical volume(s)", - CACHE_VGMETADATA | PERMITTED_READ_ONLY) + PERMITTED_READ_ONLY) xx(lvconvert, "Change logical volume layout", @@ -127,7 +127,7 @@ xx(pvdata, xx(pvdisplay, "Display various attributes of physical volume(s)", - CACHE_VGMETADATA | PERMITTED_READ_ONLY | ENABLE_ALL_DEVS | ENABLE_DUPLICATE_DEVS | LOCKD_VG_SH) + PERMITTED_READ_ONLY | ENABLE_ALL_DEVS | ENABLE_DUPLICATE_DEVS | LOCKD_VG_SH) /* 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", - CACHE_VGMETADATA | 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) xx(pvscan, "List all physical volumes", @@ -173,7 +173,7 @@ xx(vgcfgrestore, xx(vgchange, "Change volume group attributes", - CACHE_VGMETADATA | PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT) + PERMITTED_READ_ONLY | ALL_VGS_IS_DEFAULT) xx(vgck, "Check the consistency of volume group(s)", diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c index 26510bc..fc96b8d 100644 --- a/tools/lvmcmdline.c +++ b/tools/lvmcmdline.c @@ -2281,7 +2281,6 @@ static int _get_current_settings(struct cmd_context *cmd) cmd->current_settings.archive = arg_int_value(cmd, autobackup_ARG, cmd->current_settings.archive); cmd->current_settings.backup = arg_int_value(cmd, autobackup_ARG, cmd->current_settings.backup); - cmd->current_settings.cache_vgmetadata = cmd->cname->flags & CACHE_VGMETADATA ? 1 : 0; if (arg_is_set(cmd, readonly_ARG)) { cmd->current_settings.activation = 0; diff --git a/tools/tools.h b/tools/tools.h index 33cbf10..0886551 100644 --- a/tools/tools.h +++ b/tools/tools.h @@ -113,7 +113,6 @@ struct arg_value_group_list { uint32_t prio; }; -#define CACHE_VGMETADATA 0x00000001 #define PERMITTED_READ_ONLY 0x00000002 /* Process all VGs if none specified on the command line. */ #define ALL_VGS_IS_DEFAULT 0x00000004

6 years

1
0
0 / 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

lvm2-commits April 2018