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