diff options
author | Joe Thornber <ejt@redhat.com> | 2018-05-16 13:43:02 +0100 |
---|---|---|
committer | Joe Thornber <ejt@redhat.com> | 2018-05-16 13:43:02 +0100 |
commit | 89fdc0b5889d24fe785e7fa4c2be13659d1ed0b2 (patch) | |
tree | 55484f573f545e2bda01371ccf1b3ca4fe128620 /lib/label | |
parent | ccc35e2647b3b78f2fb0d62c25f21fcccbf58950 (diff) | |
parent | 7c852c75c3b3e719d57d3410bf0c2a5e61d67f4e (diff) | |
download | lvm2-89fdc0b5889d24fe785e7fa4c2be13659d1ed0b2.tar.gz |
Merge branch 'master' into 2018-05-11-fork-libdm
Diffstat (limited to 'lib/label')
-rw-r--r-- | lib/label/label.c | 205 | ||||
-rw-r--r-- | lib/label/label.h | 4 |
2 files changed, 125 insertions, 84 deletions
diff --git a/lib/label/label.c b/lib/label/label.c index ba34c7d31..baa81370f 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -251,9 +251,11 @@ static bool _in_bcache(struct device *dev) static struct labeller *_find_lvm_header(struct device *dev, char *scan_buf, + uint32_t scan_buf_sectors, char *label_buf, uint64_t *label_sector, - uint64_t scan_sector) + uint64_t block_sector, + uint64_t start_sector) { struct labeller_i *li; struct labeller *labeller_ret = NULL; @@ -266,25 +268,34 @@ static struct labeller *_find_lvm_header(struct device *dev, * and copy it into label_buf. */ - for (sector = 0; sector < LABEL_SCAN_SECTORS; + for (sector = start_sector; sector < start_sector + LABEL_SCAN_SECTORS; sector += LABEL_SIZE >> SECTOR_SHIFT) { + + /* + * The scan_buf passed in is a bcache block, which is + * BCACHE_BLOCK_SIZE_IN_SECTORS large. So if start_sector is + * one of the last couple sectors in that buffer, we need to + * break early. + */ + if (sector >= scan_buf_sectors) + break; + lh = (struct label_header *) (scan_buf + (sector << SECTOR_SHIFT)); if (!strncmp((char *)lh->id, LABEL_ID, sizeof(lh->id))) { if (found) { log_error("Ignoring additional label on %s at sector %llu", - dev_name(dev), (unsigned long long)(sector + scan_sector)); + dev_name(dev), (unsigned long long)(block_sector + sector)); } - if (xlate64(lh->sector_xl) != sector + scan_sector) { - log_very_verbose("%s: Label for sector %llu found at sector %llu - ignoring.", - dev_name(dev), - (unsigned long long)xlate64(lh->sector_xl), - (unsigned long long)(sector + scan_sector)); + if (xlate64(lh->sector_xl) != sector) { + log_warn("%s: Label for sector %llu found at sector %llu - ignoring.", + dev_name(dev), + (unsigned long long)xlate64(lh->sector_xl), + (unsigned long long)(block_sector + sector)); continue; } - if (calc_crc(INITIAL_CRC, (uint8_t *)&lh->offset_xl, LABEL_SIZE - - ((uint8_t *) &lh->offset_xl - (uint8_t *) lh)) != - xlate32(lh->crc_xl)) { + if (calc_crc(INITIAL_CRC, (uint8_t *)&lh->offset_xl, + LABEL_SIZE - ((uint8_t *) &lh->offset_xl - (uint8_t *) lh)) != xlate32(lh->crc_xl)) { log_very_verbose("Label checksum incorrect on %s - ignoring", dev_name(dev)); continue; } @@ -293,14 +304,14 @@ static struct labeller *_find_lvm_header(struct device *dev, } dm_list_iterate_items(li, &_labellers) { - if (li->l->ops->can_handle(li->l, (char *) lh, sector + scan_sector)) { + if (li->l->ops->can_handle(li->l, (char *) lh, block_sector + sector)) { log_very_verbose("%s: %s label detected at sector %llu", dev_name(dev), li->name, - (unsigned long long)(sector + scan_sector)); + (unsigned long long)(block_sector + sector)); if (found) { log_error("Ignoring additional label on %s at sector %llu", dev_name(dev), - (unsigned long long)(sector + scan_sector)); + (unsigned long long)(block_sector + sector)); continue; } @@ -309,7 +320,7 @@ static struct labeller *_find_lvm_header(struct device *dev, memcpy(label_buf, lh, LABEL_SIZE); if (label_sector) - *label_sector = sector + scan_sector; + *label_sector = block_sector + sector; break; } } @@ -329,7 +340,9 @@ static struct labeller *_find_lvm_header(struct device *dev, * are performed in the processing functions to get that data. */ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, - struct device *dev, struct block *bb, int *is_lvm_device) + struct device *dev, struct block *bb, + uint64_t block_sector, uint64_t start_sector, + int *is_lvm_device) { char label_buf[LABEL_SIZE] __attribute__((aligned(8))); struct label *label = NULL; @@ -345,7 +358,7 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, * data had been read (here). They set this flag to indicate that the * filters should be retested now that data from the device is ready. */ - if (cmd && (dev->flags & DEV_FILTER_AFTER_SCAN)) { + if (f && (dev->flags & DEV_FILTER_AFTER_SCAN)) { dev->flags &= ~DEV_FILTER_AFTER_SCAN; log_debug_devs("Scan filtering %s", dev_name(dev)); @@ -374,7 +387,7 @@ static int _process_block(struct cmd_context *cmd, struct dev_filter *f, * FIXME: we don't need to copy one sector from bb->data into label_buf, * we can just point label_buf at one sector in ld->buf. */ - if (!(labeller = _find_lvm_header(dev, bb->data, label_buf, §or, 0))) { + if (!(labeller = _find_lvm_header(dev, bb->data, BCACHE_BLOCK_SIZE_IN_SECTORS, label_buf, §or, block_sector, start_sector))) { /* * Non-PVs exit here @@ -567,7 +580,7 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, } else { log_debug_devs("Processing data from device %s fd %d block %p", dev_name(devl->dev), devl->dev->bcache_fd, bb); - ret = _process_block(cmd, f, devl->dev, bb, &is_lvm_device); + ret = _process_block(cmd, f, devl->dev, bb, 0, 0, &is_lvm_device); if (!ret && is_lvm_device) { log_debug_devs("Scan failed to process %s", dev_name(devl->dev)); @@ -610,32 +623,46 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, return 1; } +/* + * How many blocks to set up in bcache? Is 1024 a good max? + * + * Currently, we tell bcache to set up N blocks where N + * is the number of devices that are going to be scanned. + * Reasons why this number may not be be a good choice: + * + * - there may be a lot of non-lvm devices, which + * would make this number larger than necessary + * + * - each lvm device may use more than one cache + * block if the metadata is large enough or it + * uses more than one metadata area, which + * would make this number smaller than it + * should be for the best performance. + * + * This is even more tricky to estimate when lvmetad + * is used, because it's hard to predict how many + * devs might need to be scanned when using lvmetad. + * This currently just sets up bcache with MIN blocks. + */ + #define MIN_BCACHE_BLOCKS 32 +#define MAX_BCACHE_BLOCKS 1024 static int _setup_bcache(int cache_blocks) { struct io_engine *ioe; - /* No devices can happen, just create bcache with any small number. */ if (cache_blocks < MIN_BCACHE_BLOCKS) cache_blocks = MIN_BCACHE_BLOCKS; - /* - * 100 is arbitrary, it's the max number of concurrent aio's - * possible, i.e, the number of devices that can be read at - * once. Should this be configurable? - */ + if (cache_blocks > MAX_BCACHE_BLOCKS) + cache_blocks = MAX_BCACHE_BLOCKS; + if (!(ioe = create_async_io_engine())) { log_error("Failed to create bcache io engine."); return 0; } - /* - * Configure one cache block for each device on the system. - * We won't generally need to cache that many because some - * 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, cache_blocks, ioe))) { log_error("Failed to create bcache with %d cache blocks.", cache_blocks); return 0; @@ -653,7 +680,7 @@ int label_scan(struct cmd_context *cmd) { struct dm_list all_devs; struct dev_iter *iter; - struct device_list *devl; + struct device_list *devl, *devl2; struct device *dev; log_debug_devs("Finding devices to scan"); @@ -695,16 +722,17 @@ int label_scan(struct cmd_context *cmd) log_debug_devs("Found %d devices to scan", dm_list_size(&all_devs)); if (!scan_bcache) { - /* - * FIXME: there should probably be some max number of - * cache blocks we use when setting up bcache. - */ if (!_setup_bcache(dm_list_size(&all_devs))) return 0; } _scan_list(cmd, cmd->full_filter, &all_devs, NULL); + dm_list_iterate_items_safe(devl, devl2, &all_devs) { + dm_list_del(&devl->list); + dm_free(devl); + } + return 1; } @@ -834,9 +862,7 @@ void label_scan_destroy(struct cmd_context *cmd) * device, this is not a commonly used function. */ -/* FIXME: remove unused_sector arg */ - -int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector) +int label_read(struct device *dev) { struct dm_list one_dev; struct device_list *devl; @@ -856,17 +882,7 @@ int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector _scan_list(NULL, NULL, &one_dev, &failed); - /* - * FIXME: this ugliness of returning a pointer to the label is - * temporary until the callers can be updated to not use this. - */ - if (labelp) { - struct lvmcache_info *info; - - info = lvmcache_info_from_pvid(dev->pvid, dev, 1); - if (info) - *labelp = lvmcache_get_label(info); - } + dm_free(devl); if (failed) return 0; @@ -875,25 +891,66 @@ int label_read(struct device *dev, struct label **labelp, uint64_t unused_sector /* * Read a label from a specfic, non-zero sector. This is used in only - * one place: pvck -> pv_analyze. + * one place: pvck/pv_analyze. */ -int label_read_sector(struct device *dev, struct label **labelp, uint64_t scan_sector) +int label_read_sector(struct device *dev, uint64_t read_sector) { - if (scan_sector) { - /* TODO: not yet implemented */ - /* When is this done? When does it make sense? Is it actually possible? */ - return 0; + struct block *bb = NULL; + uint64_t block_num; + uint64_t block_sector; + uint64_t start_sector; + int is_lvm_device = 0; + int result; + int ret; + + block_num = read_sector / BCACHE_BLOCK_SIZE_IN_SECTORS; + block_sector = block_num * BCACHE_BLOCK_SIZE_IN_SECTORS; + start_sector = read_sector % BCACHE_BLOCK_SIZE_IN_SECTORS; + + label_scan_open(dev); + + bcache_prefetch(scan_bcache, dev->bcache_fd, block_num); + + if (!bcache_get(scan_bcache, dev->bcache_fd, block_num, 0, &bb)) { + log_error("Scan failed to read %s at %llu", + dev_name(dev), (unsigned long long)block_num); + ret = 0; + goto out; } - return label_read(dev, labelp, 0); + /* + * TODO: check if scan_sector is larger than the bcache block size. + * If it is, we need to fetch a later block from bcache. + */ + + result = _process_block(NULL, NULL, dev, bb, block_sector, start_sector, &is_lvm_device); + + if (!result && is_lvm_device) { + log_error("Scan failed to process %s", dev_name(dev)); + ret = 0; + goto out; + } + + if (!result || !is_lvm_device) { + log_error("Could not find LVM label on %s", dev_name(dev)); + ret = 0; + goto out; + } + + ret = 1; +out: + if (bb) + bcache_put(bb); + return ret; } /* * This is only needed when commands are using lvmetad, in which case they * don't do an initial label_scan, but may later need to rescan certain devs * from disk and call this function. FIXME: is there some better number to - * choose here? + * choose here? How should we predict the number of devices that might need + * scanning when using lvmetad? */ int label_scan_setup_bcache(void) @@ -922,18 +979,10 @@ int label_scan_open(struct device *dev) bool dev_read_bytes(struct device *dev, uint64_t start, size_t len, void *data) { - int ret; - if (!scan_bcache) { - if (!dev_open_readonly(dev)) - return false; - - ret = dev_read(dev, start, len, 0, data); - - if (!dev_close(dev)) - stack; - - return ret ? true : false; + /* Should not happen */ + log_error("dev_read bcache not set up %s", dev_name(dev)); + return false; } if (dev->bcache_fd <= 0) { @@ -956,21 +1005,13 @@ bool dev_read_bytes(struct device *dev, uint64_t start, size_t len, void *data) bool dev_write_bytes(struct device *dev, uint64_t start, size_t len, void *data) { - int ret; - if (test_mode()) return true; if (!scan_bcache) { - if (!dev_open(dev)) - return false; - - ret = dev_write(dev, start, len, 0, data); - - if (!dev_close(dev)) - stack; - - return ret ? true : false; + /* Should not happen */ + log_error("dev_write bcache not set up %s", dev_name(dev)); + return false; } if (dev->bcache_fd <= 0) { @@ -1003,7 +1044,7 @@ bool dev_write_zeros(struct device *dev, uint64_t start, size_t len) return true; if (!scan_bcache) { - log_error("dev_write_zeros %s bcache not set up", dev_name(dev)); + log_error("dev_write_zeros bcache not set up %s", dev_name(dev)); return false; } @@ -1037,7 +1078,7 @@ bool dev_set_bytes(struct device *dev, uint64_t start, size_t len, uint8_t val) return true; if (!scan_bcache) { - log_error("dev_set_bytes %s bcache not set up", dev_name(dev)); + log_error("dev_set_bytes bcache not set up %s", dev_name(dev)); return false; } diff --git a/lib/label/label.h b/lib/label/label.h index 287d6d66c..43ceff768 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -109,8 +109,8 @@ void label_scan_invalidate(struct device *dev); void label_scan_invalidate_lv(struct cmd_context *cmd, struct logical_volume *lv); void label_scan_drop(struct cmd_context *cmd); 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); +int label_read(struct device *dev); +int label_read_sector(struct device *dev, uint64_t scan_sector); void label_scan_confirm(struct device *dev); int label_scan_setup_bcache(void); int label_scan_open(struct device *dev); |