diff --git a/lib/label/label.c b/lib/label/label.c index b999f8729..43b05a257 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -847,8 +847,11 @@ static void _free_hints(struct dm_list *hints) } /* - * Scan and cache lvm data from all devices on the system. - * The cache should be empty/reset before calling this. + * Scan devices on the system to discover which are LVM devices. + * Info about the LVM devices (PVs) is saved in lvmcache in a + * basic/summary form (info/vginfo structs). The vg_read phase + * uses this summary info to know which PVs to look at for + * processing a given VG. */ int label_scan(struct cmd_context *cmd) @@ -860,7 +863,8 @@ int label_scan(struct cmd_context *cmd) struct device_list *devl, *devl2; struct device *dev; uint64_t max_metadata_size_bytes; - int newhints = 0; + int using_hints; + int create_hints = 0; /* NEWHINTS_NONE */ log_debug_devs("Finding devices to scan"); @@ -869,20 +873,37 @@ int label_scan(struct cmd_context *cmd) dm_list_init(&hints_list); /* - * Iterate through all the devices in dev-cache (block devs that appear - * under /dev that could possibly hold a PV and are not excluded by - * filters). Read each to see if it's an lvm device, and if so - * populate lvmcache with some basic info about the device and the VG - * on it. This info will be used by the vg_read() phase of the - * command. + * dev_cache_scan() creates a list of devices on the system + * (saved in in dev-cache) which we can iterate through to + * search for LVM devs. The dev cache list either comes from + * looking at dev nodes under /dev, or from udev. */ dev_cache_scan(); + /* + * Set up the iterator that is needed to step through each device in + * dev cache. + */ if (!(iter = dev_iter_create(cmd->filter, 0))) { log_error("Scanning failed to get devices."); return 0; } + log_debug_devs("Filtering devices to scan"); + + /* + * Iterate through all devices in dev cache and apply filters + * to exclude devs that we do not need to scan. Those devs + * that pass the filters are returned by the iterator and + * saved in a list of devs that we will proceed to scan to + * check if they are LVM devs. IOW this loop is the + * application of filters (those that do not require reading + * the devs) to the list of all devices. It does that because + * the 'cmd->filter' is used above when setting up the iterator. + * Unfortunately, it's not obvious that this is what's happening + * here. filters that require reading the device are not applied + * here, but in process_block(), see DEV_FILTER_AFTER_SCAN. + */ while ((dev = dev_iter_get(cmd, iter))) { if (!(devl = zalloc(sizeof(*devl)))) continue; @@ -901,13 +922,12 @@ int label_scan(struct cmd_context *cmd) /* * When md devices exist that use the old superblock at the * end of the device, then in order to detect and filter out - * the component devices of those md devs, we need to enable - * the full md filter which scans both the start and the end - * of every device. This doubles the amount of scanning i/o, - * which we want to avoid. FIXME: it may not be worth the - * cost of double i/o just to avoid displaying md component - * devs in 'pvs', which is a pretty harmless effect from a - * pretty uncommon situation. + * the component devices of those md devs, we enable the full + * md filter which scans both the start and the end of every + * device. This doubles the amount of scanning i/o, which we + * want to avoid. FIXME: this forces start+end scanning of + * every device, but it would be more efficient to limit the + * end scan only to PVs. */ if (dev_is_md_with_end_superblock(cmd->dev_types, dev)) cmd->use_full_md_check = 1; @@ -920,7 +940,10 @@ int label_scan(struct cmd_context *cmd) } /* - * In some common cases we can avoid scanning all devices. + * In some common cases we can avoid scanning all devices + * by using hints which tell us which devices are PVs, which + * are the only devices we actually need to scan. Without + * hints we need to scan all devs to find which are PVs. * * TODO: if the command is using hints and a single vgname * arg, we can also take the vg lock here, prior to scanning. @@ -930,10 +953,12 @@ int label_scan(struct cmd_context *cmd) * able to avoid rescan in vg_read, but locking early would * apply to more cases.) */ - if (!get_hints(cmd, &hints_list, &newhints, &all_devs, &scan_devs)) { + if (!get_hints(cmd, &hints_list, &create_hints, &all_devs, &scan_devs)) { dm_list_splice(&scan_devs, &all_devs); dm_list_init(&hints_list); - } + using_hints = 0; + } else + using_hints = 1; log_debug("Will scan %d devices skip %d", dm_list_size(&scan_devs), dm_list_size(&all_devs)); @@ -979,17 +1004,18 @@ int label_scan(struct cmd_context *cmd) dm_list_init(&cmd->hints); - if (!dm_list_empty(&hints_list)) { + /* + * If we're using hints to limit which devs we scanned, verify + * that those hints were valid, and if not we need to scan the + * rest of the devs. + */ + if (using_hints) { if (!validate_hints(cmd, &hints_list)) { - /* - * We scanned a subset of all devices based on hints. - * With the results from the scan we may decide that - * the hints are not valid, so scan all others. - */ log_debug("Will scan %d remaining devices", dm_list_size(&all_devs)); _scan_list(cmd, cmd->filter, &all_devs, NULL); _free_hints(&hints_list); - newhints = 0; + using_hints = 0; + create_hints = 0; } else { /* The hints may be used by another device iteration. */ dm_list_splice(&cmd->hints, &hints_list); @@ -1006,8 +1032,15 @@ int label_scan(struct cmd_context *cmd) free(devl); } - if (newhints) - write_hint_file(cmd, newhints); + /* + * If hints were not available/usable, then we scanned all devs, + * and we now know which are PVs. Save this list of PVs we've + * identified as hints for the next command to use. + * (create_hints variable has NEWHINTS_X value which indicates + * the reason for creating the new hints.) + */ + if (create_hints) + write_hint_file(cmd, create_hints); return 1; }