lib/device/dev-cache.c    | 161 ++++++++++++++++++++++++++++++++++++----------

 test/shell/dev-aliases.sh |  53 +++++++++++++++

 2 files changed, 179 insertions(+), 35 deletions(-)

 create mode 100644 test/shell/dev-aliases.sh

diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c

index d5f18ff..8082efa 100644

--- a/lib/device/dev-cache.c

+++ b/lib/device/dev-cache.c

@@ -1428,60 +1428,151 @@ struct device *dev_hash_get(const char *name)

 	return (struct device *) dm_hash_lookup(_cache.names, name);

 }

+static void _remove_alias(struct device *dev, const char *name)

+{

+	struct dm_str_list *strl;

+

+	dm_list_iterate_items(strl, &dev->aliases) {

+		if (!strcmp(strl->str, name)) {

+			dm_list_del(&strl->list);

+			return;

+		}

+	}

+}

+

+/*

+ * Check that paths for this dev still refer to the same dev_t.  This is known

+ * to drop invalid paths in the case where lvm deactivates an LV, which causes

+ * that LV path to go away, but that LV path is not removed from dev-cache (it

+ * probably should be).  Later a new path to a different LV is added to

+ * dev-cache, where the new LV has the same major:minor as the previously

+ * deactivated LV.  The new LV will find the existing struct dev, and that

+ * struct dev will have dev->aliases entries that refer to the name of the old

+ * deactivated LV.  Those old paths are all invalid and are dropped here.

+ */

+

+static void _verify_aliases(struct device *dev, const char *newname)

+{

+	struct dm_str_list *strl, *strl2;

+	struct stat st;

+

+	dm_list_iterate_items_safe(strl, strl2, &dev->aliases) {

+		/* newname was just stat'd and added by caller */

+		if (newname && !strcmp(strl->str, newname))

+			continue;

+

+		if (stat(strl->str, &st) || (st.st_rdev != dev->dev)) {

+			log_debug("Drop invalid path %s for %d:%d (new path %s).",

+				  strl->str, (int)MAJOR(dev->dev), (int)MINOR(dev->dev), newname ?: "");

+			dm_hash_remove(_cache.names, strl->str);

+			dm_list_del(&strl->list);

+		}

+	}

+}

+

 struct device *dev_cache_get(struct cmd_context *cmd, const char *name, struct dev_filter *f)

 {

-	struct stat buf;

-	struct device *d = (struct device *) dm_hash_lookup(_cache.names, name);

-	int info_available = 0;

-	int ret = 1;

+	struct device *dev = (struct device *) dm_hash_lookup(_cache.names, name);

+	struct stat st;

+	int ret;

-	if (d && (d->flags & DEV_REGULAR))

-		return d;

+	/*

+	 * DEV_REGULAR means that is "dev" is actually a file, not a device.

+	 * FIXME: I don't think dev-cache is used for files any more and this

+	 * can be dropped?

+	 */

+	if (dev && (dev->flags & DEV_REGULAR))

+		return dev;

+

+	/*

+	 * The requested path is invalid, remove any dev-cache

+	 * info for it.

+	 */

+	if (stat(name, &st)) {

+		if (dev) {

+			log_print("Device path %s is invalid for %d:%d %s.",

+				  name, (int)MAJOR(dev->dev), (int)MINOR(dev->dev), dev_name(dev));

-	/* If the entry's wrong, remove it */

-	if (stat(name, &buf) < 0) {

-		if (d)

 			dm_hash_remove(_cache.names, name);

-		log_sys_very_verbose("stat", name);

-		d = NULL;

-	} else

-		info_available = 1;

-	if (d && (buf.st_rdev != d->dev)) {

-		dm_hash_remove(_cache.names, name);

-		d = NULL;

-	}

+			_remove_alias(dev, name);

-	if (!d) {

-		_insert(name, info_available ? &buf : NULL, 0, obtain_device_list_from_udev());

-		d = (struct device *) dm_hash_lookup(_cache.names, name);

-		if (!d) {

-			log_debug_devs("Device name not found in dev_cache repeat dev_cache_scan for %s", name);

-			dev_cache_scan();

-			d = (struct device *) dm_hash_lookup(_cache.names, name);

+			/* Remove any other names in dev->aliases that are incorrect. */

+			_verify_aliases(dev, NULL);

 		}

+		return NULL;

 	}

-	if (!d)

+	if (!S_ISBLK(st.st_mode)) {

+		log_debug("Not a block device %s.", name);

 		return NULL;

+	}

-	if (d && (d->flags & DEV_REGULAR))

-		return d;

+	/*

+	 * dev-cache has incorrect info for the requested path.

+	 * Remove incorrect info and then add new dev-cache entry.

+	 */

+	if (dev && (st.st_rdev != dev->dev)) {

+		log_print("Device path %s does not match %d:%d %s.",

+			  name, (int)MAJOR(dev->dev), (int)MINOR(dev->dev), dev_name(dev));

+

+		dm_hash_remove(_cache.names, name);

+

+		_remove_alias(dev, name);

+

+		/* Remove any other names in dev->aliases that are incorrect. */

+		_verify_aliases(dev, NULL);

+

+		/* Add new dev-cache entry next. */

+		dev = NULL;

+	}

+

+	/*

+	 * Either add a new struct dev for st_rdev and name,

+	 * or add name as a new alias for an existing struct dev

+	 * for st_rdev.

+	 */

+	if (!dev) {

+		_insert_dev(name, st.st_rdev);

-	if (f && !(d->flags & DEV_REGULAR)) {

-		ret = f->passes_filter(cmd, f, d, NULL);

+		/* Get the struct dev that was just added. */

+		dev = (struct device *) dm_hash_lookup(_cache.names, name);

-		if (ret == -EAGAIN) {

-			log_debug_devs("get device by name defer filter %s", dev_name(d));

-			d->flags |= DEV_FILTER_AFTER_SCAN;

-			ret = 1;

+		if (!dev) {

+			log_error("Failed to get device %s", name);

+			return NULL;

 		}

+

+		_verify_aliases(dev, name);

 	}

-	if (f && !(d->flags & DEV_REGULAR) && !ret)

+	/*

+	 * The caller passed a filter if they only want the dev if it

+	 * passes filters.

+	 */

+

+	if (!f)

+		return dev;

+

+	ret = f->passes_filter(cmd, f, dev, NULL);

+

+	/*

+	 * This might happen if this function is called before

+	 * filters can do i/o.  I don't think this will happen

+	 * any longer and this EAGAIN case can be removed.

+	 */

+	if (ret == -EAGAIN) {

+		log_debug_devs("dev_cache_get filter deferred %s", dev_name(dev));

+		dev->flags |= DEV_FILTER_AFTER_SCAN;

+		ret = 1;

+	}

+

+	if (!ret) {

+		log_debug_devs("dev_cache_get filter excludes %s", dev_name(dev));

 		return NULL;

+	}

-	return d;

+	return dev;

 }

 static struct device *_dev_cache_seek_devt(dev_t dev)

diff --git a/test/shell/dev-aliases.sh b/test/shell/dev-aliases.sh

new file mode 100644

index 0000000..c97cd5d

--- /dev/null

+++ b/test/shell/dev-aliases.sh

@@ -0,0 +1,53 @@

+#!/usr/bin/env bash

+

+# Copyright (C) 2012 Red Hat, Inc. All rights reserved.

+#

+# This copyrighted material is made available to anyone wishing to use,

+# modify, copy, or redistribute it subject to the terms and conditions

+# of the GNU General Public License v.2.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program; if not, write to the Free Software Foundation,

+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

+

+SKIP_WITH_LVMPOLLD=1

+

+. lib/inittest

+

+aux prepare_devs 3

+

+vgcreate $vg $dev1 $dev2 $dev3

+

+#

+# This lvconvert command will deactivate LV1, then internally create a new

+# lv, lvol0, as a poolmetadataspare, then activate lvol0 to zero it.

+# lvol0 will get the same major:minor that LV1 had.  When the code gets

+# the struct dev for lvol0, the new path to lvol0 is added to the

+# dev-cache with it's major:minor.  That major:minor already exists in

+# dev-cache and has the stale LV1 as an alias.  So the path to lvol0 is

+# added as an alias to the existing struct dev (with the correct

+# major:minor), but that struct dev has the stale LV1 path on its aliases

+# list.  The code will now validate all the aliases before returning the

+# dev for lvol0, and will find that the LV1 path is stale and remove it

+# from the aliases.  That will prevent the stale path from being used for

+# the dev in place of the new path.

+#

+# The preferred_name is set to /dev/mapper so that if the stale path still

+# exists, that stale path would be used as the name for the dev, and the

+# wiping code would fail to open that stale name.

+#

+

+lvcreate -n $lv1 -L32M $vg $dev1

+lvcreate -n $lv2 -L16M $vg $dev2

+lvconvert -y --type cache-pool --poolmetadata $lv2 --cachemode writeback $vg/$lv1 --config='devices { preferred_names=["/dev/mapper/"] }' 

+lvremove -y $vg/$lv1

+

+lvcreate -n $lv1 -L32M $vg $dev1

+lvcreate -n $lv2 -L16M $vg $dev2

+lvconvert -y --type cache-pool --poolmetadata $lv2 $vg/$lv1

+lvremove -y $vg/$lv1

+

+# TODO: add more validation of dev aliases being specified as command

+# args in combination with various preferred_names settings.

+

+vgremove -ff  $vg