[PATCH 0/4] fixes for path wwid detection and path change uevents

[PATCH 0/4] fixes for path wwid detection and path change uevents

[Martin Wilck]

Hi Christophe,

this small series fixes some minor glitches I found in the current path
discovery code, and attempts to implement the safe part of the functionality
discussed in the thread "multipathd: update path's udev in uev_update_path"
in January (based on an idea from Wu Chongyun).

Martin Wilck (4):
  libmultipath: get_uid: check VPD pages for SCSI only
  libmultipath: get_uid: don't quit prematurely without udev
  libmultipath: uev_update_path: always warn if WWID changed
  libmultipath: uev_update_path: update path properties

 libmultipath/discovery.c | 58 ++++++++++++++++++++++++++++--------------------
 multipathd/main.c        | 29 +++++++++++++++++-------
 2 files changed, 55 insertions(+), 32 deletions(-)

-- 
2.16.1

[PATCH 1/4] libmultipath: get_uid: check VPD pages for SCSI only

[Martin Wilck]

The VPD code won't work for non-SCSI devices, anyway. For indentation
reasons, I moved the "retrigger_tries" case to a separate function,
which is also called only for SCSI devices.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 50 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 3d38a2550980..53182a85fa10 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1807,9 +1807,38 @@ get_vpd_uid(struct path * pp)
 		parent = udev_device_get_parent(parent);
 	}
 
+	if (!parent)
+		return -EINVAL;
+
 	return get_vpd_sysfs(parent, 0x83, pp->wwid, WWID_SIZE);
 }
 
+static ssize_t scsi_uid_fallback(struct path *pp, int path_state,
+			     const char **origin)
+{
+	ssize_t len = 0;
+	int retrigger;
+	struct config *conf;
+
+	conf = get_multipath_config();
+	retrigger = conf->retrigger_tries;
+	put_multipath_config(conf);
+	if (pp->retriggers >= retrigger &&
+	    !strcmp(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE)) {
+		len = get_vpd_uid(pp);
+		*origin = "sysfs";
+		pp->uid_attribute = NULL;
+		if (len < 0 && path_state == PATH_UP) {
+			condlog(1, "%s: failed to get sysfs uid: %s",
+				pp->dev, strerror(-len));
+			len = get_vpd_sgio(pp->fd, 0x83, pp->wwid,
+					   WWID_SIZE);
+			*origin = "sgio";
+		}
+	}
+	return len;
+}
+
 int
 get_uid (struct path * pp, int path_state, struct udev_device *udev)
 {
@@ -1851,7 +1880,6 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev)
 		len = get_rbd_uid(pp);
 		origin = "sysfs";
 	} else {
-		int retrigger;
 
 		if (pp->uid_attribute) {
 			len = get_udev_uid(pp, pp->uid_attribute, udev);
@@ -1861,26 +1889,12 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev)
 					"%s: failed to get udev uid: %s",
 					pp->dev, strerror(-len));
 
-		} else {
+		} else if (pp->bus == SYSFS_BUS_SCSI) {
 			len = get_vpd_uid(pp);
 			origin = "sysfs";
 		}
-		conf = get_multipath_config();
-		retrigger = conf->retrigger_tries;
-		put_multipath_config(conf);
-		if (len <= 0 && pp->retriggers >= retrigger &&
-		    !strcmp(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE)) {
-			len = get_vpd_uid(pp);
-			origin = "sysfs";
-			pp->uid_attribute = NULL;
-			if (len < 0 && path_state == PATH_UP) {
-				condlog(1, "%s: failed to get sysfs uid: %s",
-					pp->dev, strerror(-len));
-				len = get_vpd_sgio(pp->fd, 0x83, pp->wwid,
-						   WWID_SIZE);
-				origin = "sgio";
-			}
-		}
+		if (len <= 0 && pp->bus == SYSFS_BUS_SCSI)
+			len = scsi_uid_fallback(pp, path_state, &origin);
 	}
 	if ( len < 0 ) {
 		condlog(1, "%s: failed to get %s uid: %s",
-- 
2.16.1

[PATCH 2/4] libmultipath: get_uid: don't quit prematurely without udev

[Martin Wilck]

Not all the implemented methods to derive the UID rely on udev
information being present. For example getuid callout, rbd,
and the SCSI vpd code work fine without it. It's unlikely that
we don't get udev data, but we want to be as good as possible
at deriving the uid.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 53182a85fa10..9f2a9c907914 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1853,11 +1853,6 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev)
 		put_multipath_config(conf);
 	}
 
-	if (!udev) {
-		condlog(1, "%s: no udev information", pp->dev);
-		return 1;
-	}
-
 	memset(pp->wwid, 0, WWID_SIZE);
 	if (pp->getuid) {
 		char buff[CALLOUT_MAX_SIZE];
@@ -1881,7 +1876,7 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev)
 		origin = "sysfs";
 	} else {
 
-		if (pp->uid_attribute) {
+		if (udev && pp->uid_attribute) {
 			len = get_udev_uid(pp, pp->uid_attribute, udev);
 			origin = "udev";
 			if (len <= 0)
@@ -1900,6 +1895,7 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev)
 		condlog(1, "%s: failed to get %s uid: %s",
 			pp->dev, origin, strerror(-len));
 		memset(pp->wwid, 0x0, WWID_SIZE);
+		return 1;
 	} else {
 		/* Strip any trailing blanks */
 		c = strchr(pp->wwid, '\0');
-- 
2.16.1

[PATCH 3/4] libmultipath: uev_update_path: always warn if WWID changed

[Martin Wilck]

Print the warning about changed WWID not only if disable_changed_wwids
is set, but always. It's actually more dangerous if that option is
not set.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 6d502aceb252..f7b9676ddb28 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -940,15 +940,18 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 	pp = find_path_by_dev(vecs->pathvec, uev->kernel);
 	if (pp) {
 		struct multipath *mpp = pp->mpp;
+		char wwid[WWID_SIZE];
 
-		if (disable_changed_wwids &&
-		    (strlen(pp->wwid) || pp->wwid_changed)) {
-			char wwid[WWID_SIZE];
+		strcpy(wwid, pp->wwid);
+		get_uid(pp, pp->state, uev->udev);
 
-			strcpy(wwid, pp->wwid);
-			get_uid(pp, pp->state, uev->udev);
-			if (strcmp(wwid, pp->wwid) != 0) {
-				condlog(0, "%s: path wwid changed from '%s' to '%s'. disallowing", uev->kernel, wwid, pp->wwid);
+		if (strncmp(wwid, pp->wwid, WWID_SIZE) != 0) {
+			condlog(0, "%s: path wwid changed from '%s' to '%s'. %s",
+				uev->kernel, wwid, pp->wwid,
+				(disable_changed_wwids ? "disallowing" :
+				 "continuing"));
+			if (disable_changed_wwids &&
+			    (strlen(wwid) || pp->wwid_changed)) {
 				strcpy(pp->wwid, wwid);
 				if (!pp->wwid_changed) {
 					pp->wwid_changed = 1;
@@ -957,7 +960,9 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 						dm_fail_path(pp->mpp->alias, pp->dev_t);
 				}
 				goto out;
-			} else
+			} else if (!disable_changed_wwids)
+				strcpy(pp->wwid, wwid);
+			else
 				pp->wwid_changed = 0;
 		}
 
-- 
2.16.1

[PATCH 4/4] libmultipath: uev_update_path: update path properties

[Martin Wilck]

Update pp->udev and those path attributes that can be cheaply
updated from sysfs, i.e. without IO to the disk.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/multipathd/main.c b/multipathd/main.c
index f7b9676ddb28..90f0b29ade70 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -964,6 +964,14 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 				strcpy(pp->wwid, wwid);
 			else
 				pp->wwid_changed = 0;
+		} else {
+			udev_device_unref(pp->udev);
+			pp->udev = udev_device_ref(uev->udev);
+			conf = get_multipath_config();
+			if (pathinfo(pp, conf, DI_SYSFS|DI_NOIO) != PATHINFO_OK)
+				condlog(1, "%s: pathinfo failed after change uevent",
+					uev->kernel);
+			put_multipath_config(conf);
 		}
 
 		if (pp->initialized == INIT_REQUESTED_UDEV)
-- 
2.16.1