zed: Misc multipath autoreplace fixes

We recently had a case where our operators replaced a bad
multipathed disk, only to see it fail to autoreplace.  The
zed logs showed that the multipath replacement disk did not pass
the 'is_dm' test in zfs_process_add() even though it should have.
is_dm is set if there exists a sysfs entry for to the
underlying /dev/sd* paths for the multipath disk.  It's
possible this path didn't exist due to a race condition where
the sysfs paths weren't created at the time the udev event came
in to zed, but this was never verified.

This patch updates the check to look for udev properties that
indicate if the new autoreplace disk is an empty multipath disk,
rather than looking for the underlying sysfs entries. It also
adds in additional logging, and fixes a bug where zed allowed
you to use an already zfs-formatted disk from another pool
as a multipath auto-replacement disk.

Furthermore, while testing this patch, I also ran across a case
where a force-faulted disk did not have a ZPOOL_CONFIG_PHYS_PATH
entry in its config.  This prevented it from being autoreplaced.
I added additional logic to derive the PHYS_PATH from the PATH if
the PATH was a /dev/disk/by-vdev/ path.  For example, if PATH
was /dev/disk/by-vdev/L28, then PHYS_PATH would be L28.  This is
safe since by-vdev paths represent physical locations and do not
change between boots.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Closes #13023 

1 files changed, 19 insertions, 10 deletions

diff --git a/lib/libzutil/os/linux/zutil_device_path_os.c b/lib/libzutil/os/linux/zutil_device_path_os.c
index 17247233b..c4a36549d 100644
--- a/lib/libzutil/os/linux/zutil_device_path_os.c
+++ b/lib/libzutil/os/linux/zutil_device_path_os.c
@@ -527,7 +527,7 @@ zfs_dev_is_dm(const char *dev_name)
 boolean_t
 zfs_dev_is_whole_disk(const char *dev_name)
 {
-	struct dk_gpt *label;
+	struct dk_gpt *label = NULL;
 	int fd;
 
 	if ((fd = open(dev_name, O_RDONLY | O_DIRECT | O_CLOEXEC)) < 0)
@@ -613,22 +613,27 @@ zfs_get_underlying_path(const char *dev_name)
 /*
  * A disk is considered a multipath whole disk when:
  *	DEVNAME key value has "dm-"
- *	DM_NAME key value has "mpath" prefix
+ *	MPATH_DEVICE_READY is present
  *	DM_UUID key exists
  *	ID_PART_TABLE_TYPE key does not exist or is not gpt
+ *	ID_FS_LABEL key does not exist (disk isn't labeled)
  */
 static boolean_t
-udev_mpath_whole_disk(struct udev_device *dev)
+is_mpath_udev_sane(struct udev_device *dev)
 {
-	const char *devname, *type, *uuid;
+	const char *devname, *type, *uuid, *label, *mpath_ready;
 
 	devname = udev_device_get_property_value(dev, "DEVNAME");
 	type = udev_device_get_property_value(dev, "ID_PART_TABLE_TYPE");
 	uuid = udev_device_get_property_value(dev, "DM_UUID");
+	label = udev_device_get_property_value(dev, "ID_FS_LABEL");
+	mpath_ready = udev_device_get_property_value(dev, "MPATH_DEVICE_READY");
 
 	if ((devname != NULL && strncmp(devname, "/dev/dm-", 8) == 0) &&
 	    ((type == NULL) || (strcmp(type, "gpt") != 0)) &&
-	    (uuid != NULL)) {
+	    (uuid != NULL) &&
+	    (label == NULL) &&
+	    (mpath_ready != NULL && strncmp(mpath_ready, "1", 1) == 0)) {
 		return (B_TRUE);
 	}
 
@@ -636,7 +641,11 @@ udev_mpath_whole_disk(struct udev_device *dev)
 }
 
 /*
- * Check if a disk is effectively a multipath whole disk
+ * Check if a disk is a multipath "blank" disk:
+ *
+ * 1. The disk has udev values that suggest it's a multipath disk
+ * 2. The disk is not currently labeled with a filesystem of any type
+ * 3. There are no partitions on the disk
  */
 boolean_t
 is_mpath_whole_disk(const char *path)
@@ -645,7 +654,6 @@ is_mpath_whole_disk(const char *path)
 	struct udev_device *dev = NULL;
 	char nodepath[MAXPATHLEN];
 	char *sysname;
-	boolean_t wholedisk = B_FALSE;
 
 	if (realpath(path, nodepath) == NULL)
 		return (B_FALSE);
@@ -660,10 +668,11 @@ is_mpath_whole_disk(const char *path)
 		return (B_FALSE);
 	}
 
-	wholedisk = udev_mpath_whole_disk(dev);
-
+	/* Sanity check some udev values */
+	boolean_t is_sane = is_mpath_udev_sane(dev);
 	udev_device_unref(dev);
-	return (wholedisk);
+
+	return (is_sane);
 }
 
 #else /* HAVE_LIBUDEV */