From 932b9720bb074e49ac920642b3fe9c3d84019787 Mon Sep 17 00:00:00 2001
From: David Teigland <teigland@redhat.com>
Date: Mon, 28 Feb 2022 17:37:12 -0600
Subject: [PATCH 39/54] devices: use dev-cache aliases handling from label scan
functions
The label scan functions where doing some device alias validation
which is now better handled by the dev-cache layer, so just use
that.
---
lib/device/dev-cache.c | 4 +-
lib/device/dev-cache.h | 1 +
lib/label/label.c | 143 ++++++++++-------------------------------
3 files changed, 36 insertions(+), 112 deletions(-)
diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c
index 58e67e130..b0759b06c 100644
--- a/lib/device/dev-cache.c
+++ b/lib/device/dev-cache.c
@@ -1410,7 +1410,7 @@ static void _remove_alias(struct device *dev, const char *name)
* deactivated LV. Those old paths are all invalid and are dropped here.
*/
-static void _verify_aliases(struct device *dev)
+void dev_cache_verify_aliases(struct device *dev)
{
struct dm_str_list *strl, *strl2;
struct stat st;
@@ -1459,7 +1459,7 @@ static struct device *_dev_cache_get(struct cmd_context *cmd, const char *name,
_remove_alias(dev, name);
/* Remove any other names in dev->aliases that are incorrect. */
- _verify_aliases(dev);
+ dev_cache_verify_aliases(dev);
}
return NULL;
}
diff --git a/lib/device/dev-cache.h b/lib/device/dev-cache.h
index 51c3fc6c3..321a56d7b 100644
--- a/lib/device/dev-cache.h
+++ b/lib/device/dev-cache.h
@@ -55,6 +55,7 @@ int dev_cache_add_dir(const char *path);
struct device *dev_cache_get(struct cmd_context *cmd, const char *name, struct dev_filter *f);
struct device *dev_cache_get_existing(struct cmd_context *cmd, const char *name, struct dev_filter *f);
struct device *dev_cache_get_by_devt(struct cmd_context *cmd, dev_t devt);
+void dev_cache_verify_aliases(struct device *dev);
struct device *dev_hash_get(const char *name);
diff --git a/lib/label/label.c b/lib/label/label.c
index cf707f7a3..06958b502 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -458,7 +458,6 @@ static int _scan_dev_open(struct device *dev)
const char *name;
const char *modestr;
struct stat sbuf;
- int retried = 0;
int flags = 0;
int fd, di;
@@ -478,14 +477,23 @@ static int _scan_dev_open(struct device *dev)
return 0;
}
+ next_name:
/*
* All the names for this device (major:minor) are kept on
* dev->aliases, the first one is the primary/preferred name.
+ *
+ * The default name preferences in dev-cache mean that the first
+ * name in dev->aliases is not a symlink for scsi devices, but is
+ * the /dev/mapper/ symlink for mpath devices.
+ *
+ * If preferred names are set to symlinks, should this
+ * first attempt to open using a non-symlink?
+ *
+ * dm_list_first() returns NULL if the list is empty.
*/
if (!(name_list = dm_list_first(&dev->aliases))) {
- /* Shouldn't happen */
- log_error("Device open %s %d:%d has no path names.",
- dev_name(dev), (int)MAJOR(dev->dev), (int)MINOR(dev->dev));
+ log_error("Device open %d:%d has no path names.",
+ (int)MAJOR(dev->dev), (int)MINOR(dev->dev));
return 0;
}
name_sl = dm_list_item(name_list, struct dm_str_list);
@@ -513,50 +521,34 @@ static int _scan_dev_open(struct device *dev)
modestr = "ro";
}
-retry_open:
-
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));
+ return 0;
} else {
- int major, minor;
-
/*
- * Shouldn't happen, if it does, print stat info to help figure
- * out what's wrong.
+ * drop name from dev->aliases and use verify_aliases to
+ * drop any other invalid aliases before retrying open with
+ * any remaining valid paths.
*/
-
- major = (int)MAJOR(dev->dev);
- minor = (int)MINOR(dev->dev);
-
- log_error("Device open %s %d:%d failed errno %d", name, major, minor, errno);
-
- if (stat(name, &sbuf)) {
- log_debug_devs("Device open %s %d:%d stat failed errno %d",
- name, major, minor, errno);
- } else if (sbuf.st_rdev != dev->dev) {
- log_debug_devs("Device open %s %d:%d stat %d:%d does not match.",
- name, major, minor,
- (int)MAJOR(sbuf.st_rdev), (int)MINOR(sbuf.st_rdev));
- }
-
- if (!retried) {
- /*
- * FIXME: remove this, the theory for this retry is that
- * there may be a udev race that we can sometimes mask by
- * retrying. This is here until we can figure out if it's
- * needed and if so fix the real problem.
- */
- usleep(5000);
- log_debug_devs("Device open %s retry", dev_name(dev));
- retried = 1;
- goto retry_open;
- }
+ log_debug("Drop alias for %d:%d failed open %s (%d)",
+ (int)MAJOR(dev->dev), (int)MINOR(dev->dev), name, errno);
+ dev_cache_failed_path(dev, name);
+ dev_cache_verify_aliases(dev);
+ goto next_name;
}
- return 0;
+ }
+
+ /* Verify that major:minor from the path still match dev. */
+ if ((fstat(fd, &sbuf) < 0) || (sbuf.st_rdev != dev->dev)) {
+ log_warn("Invalid path %s for device %d:%d, trying different path.",
+ name, (int)MAJOR(dev->dev), (int)MINOR(dev->dev));
+ (void)close(fd);
+ dev_cache_failed_path(dev, name);
+ dev_cache_verify_aliases(dev);
+ goto next_name;
}
dev->flags |= DEV_IN_BCACHE;
@@ -604,37 +596,6 @@ static int _scan_dev_close(struct device *dev)
return 1;
}
-static void _drop_bad_aliases(struct device *dev)
-{
- struct dm_str_list *strl, *strl2;
- const char *name;
- struct stat sbuf;
- int major = (int)MAJOR(dev->dev);
- int minor = (int)MINOR(dev->dev);
- int bad;
-
- dm_list_iterate_items_safe(strl, strl2, &dev->aliases) {
- name = strl->str;
- bad = 0;
-
- if (stat(name, &sbuf)) {
- bad = 1;
- log_debug_devs("Device path check %d:%d %s stat failed errno %d",
- major, minor, name, errno);
- } else if (sbuf.st_rdev != dev->dev) {
- bad = 1;
- log_debug_devs("Device path check %d:%d %s stat %d:%d does not match.",
- major, minor, name,
- (int)MAJOR(sbuf.st_rdev), (int)MINOR(sbuf.st_rdev));
- }
-
- if (bad) {
- log_debug_devs("Device path check %d:%d dropping path %s.", major, minor, name);
- dev_cache_failed_path(dev, name);
- }
- }
-}
-
// Like bcache_invalidate, only it throws any dirty data away if the
// write fails.
static void _invalidate_di(struct bcache *cache, int di)
@@ -662,10 +623,8 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f,
char headers_buf[HEADERS_BUF_SIZE];
struct dm_list wait_devs;
struct dm_list done_devs;
- struct dm_list reopen_devs;
struct device_list *devl, *devl2;
struct block *bb;
- int retried_open = 0;
int scan_read_errors = 0;
int scan_process_errors = 0;
int scan_failed_count = 0;
@@ -676,7 +635,6 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f,
dm_list_init(&wait_devs);
dm_list_init(&done_devs);
- dm_list_init(&reopen_devs);
log_debug_devs("Scanning %d devices for VG info", dm_list_size(devs));
@@ -700,9 +658,9 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f,
if (!_in_bcache(devl->dev)) {
if (!_scan_dev_open(devl->dev)) {
- log_debug_devs("Scan failed to open %s.", dev_name(devl->dev));
+ log_debug_devs("Scan failed to open %d:%d %s.",
+ (int)MAJOR(devl->dev->dev), (int)MINOR(devl->dev->dev), dev_name(devl->dev));
dm_list_del(&devl->list);
- dm_list_add(&reopen_devs, &devl->list);
devl->dev->flags |= DEV_SCAN_NOT_READ;
continue;
}
@@ -786,41 +744,6 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f,
if (!dm_list_empty(devs))
goto scan_more;
- /*
- * We're done scanning all the devs. If we failed to open any of them
- * the first time through, refresh device paths and retry. We failed
- * to open the devs on the reopen_devs list.
- *
- * FIXME: it's not clear if or why this helps.
- */
- if (!dm_list_empty(&reopen_devs)) {
- if (retried_open) {
- /* Don't try again. */
- scan_failed_count += dm_list_size(&reopen_devs);
- dm_list_splice(&done_devs, &reopen_devs);
- goto out;
- }
- retried_open = 1;
-
- dm_list_iterate_items_safe(devl, devl2, &reopen_devs) {
- _drop_bad_aliases(devl->dev);
-
- if (dm_list_empty(&devl->dev->aliases)) {
- log_warn("WARNING: Scan ignoring device %d:%d with no paths.",
- (int)MAJOR(devl->dev->dev),
- (int)MINOR(devl->dev->dev));
-
- dm_list_del(&devl->list);
- lvmcache_del_dev(devl->dev);
- scan_failed_count++;
- }
- }
-
- /* Put devs that failed to open back on the original list to retry. */
- dm_list_splice(devs, &reopen_devs);
- goto scan_more;
- }
-out:
log_debug_devs("Scanned devices: read errors %d process errors %d failed %d",
scan_read_errors, scan_process_errors, scan_failed_count);
--
2.34.3