Skip to contentSkip to conversation

multipath: Check blacklists as soon as possible

PATCH Superseded Benjamin Marzinski 1 participant 1 message Jan 8, 2013 Device mapper

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Benjamin Marzinski

author maintainer
#0 lore raw

PATCH 1/1 multipath: Check blacklists as soon as possible

Multipath does a lot of unnecessary work on devices blacklisted by device type or wwid before ignoring them. When dealing with a large number of devices blacklisted this way, multipath can take long time to complete. The patch makes sure that multipath is checking the blacklists as soon as it has the necessary information to do so. To do this, pathinfo() now takes another flag DI_BLACKLIST, which is only used by store_pathinfo(), that tells it to check if the device should be blacklisted. Doing this cleanly also required changing how store_pathinfo() and rlookup_binding() are called.

---
 libmultipath/alias.c      |   47 +++++++++++++++++-------------
 libmultipath/alias.h      |    2 -
 libmultipath/configure.c  |   70 ++++++++++++++++++++++++++++++----------------
 libmultipath/configure.h  |    2 -
 libmultipath/discovery.c  |   47 +++++++++++++++++++++++-------
 libmultipath/discovery.h  |   11 ++++---
 libmultipath/util.c       |    2 -
 multipath/main.c          |   14 ++++-----
 multipathd/cli_handlers.c |   11 +++----
 multipathd/main.c         |   22 ++++++--------
 10 files changed, 140 insertions(+), 88 deletions(-)
libmultipath/discovery.c +35 -12
Index: multipath-tools-120821/libmultipath/discovery.c
===================================================================
@@ -28,37 +28,45 @@
 #include "prio.h"
 #include "defaults.h"
 
-struct path *
+int
 store_pathinfo (vector pathvec, vector hwtable, struct udev_device *udevice,
-		int flag)
+		int flag, struct path **pp_ptr)
 {
+	int err = 1;
 	struct path * pp;
 	const char * devname;
 
+	if (pp_ptr)
+		*pp_ptr = NULL;
+
 	devname = udev_device_get_sysname(udevice);
 	if (!devname)
-		return NULL;
+		return 1;
 
 	pp = alloc_path();
 
 	if (!pp)
-		return NULL;
+		return 1;
 
 	if(safe_sprintf(pp->dev, "%s", devname)) {
 		condlog(0, "pp->dev too small");
 		goto out;
 	}
 	pp->udev = udev_device_ref(udevice);
-	if (pathinfo(pp, hwtable, flag))
+	err = pathinfo(pp, hwtable, flag | DI_BLACKLIST);
+	if (err)
 		goto out;
 
-	if (store_path(pathvec, pp))
+	err = store_path(pathvec, pp);
+	if (err)
 		goto out;
 
-	return pp;
 out:
-	free_path(pp);
-	return NULL;
+	if (err)
+		free_path(pp);
+	else if (pp_ptr)
+		*pp_ptr = pp;
+	return err;
 }
 
 static int
@@ -78,9 +86,11 @@ path_discover (vector pathvec, struct co
 
 	pp = find_path_by_dev(pathvec, (char *)devname);
 	if (!pp) {
-		pp = store_pathinfo(pathvec, conf->hwtable,
-				    udevice, flag);
-		return (pp ? 0 : 1);
+		if (store_pathinfo(pathvec, conf->hwtable,
+				   udevice, flag, NULL) != 1)
+			return 0;
+		else
+			return 1;
 	}
 	return pathinfo(pp, conf->hwtable, flag);
 }
@@ -853,6 +863,13 @@ pathinfo (struct path *pp, vector hwtabl
 	if (mask & DI_SYSFS && sysfs_pathinfo(pp))
 		return 1;
 
+	if (mask & DI_BLACKLIST && mask & DI_SYSFS) {
+		if (filter_device(conf->blist_device, conf->elist_device,
+				  pp->vendor_id, pp->product_id) > 0) {
+			return 2;
+		}
+	}
+
 	path_state = path_offline(pp);
 
 	/*
@@ -906,6 +923,12 @@ pathinfo (struct path *pp, vector hwtabl
 
 	if (path_state == PATH_UP && (mask & DI_WWID) && !strlen(pp->wwid))
 		get_uid(pp);
+	if (mask & DI_BLACKLIST && mask & DI_WWID) {
+		if (filter_wwid(conf->blist_wwid, conf->elist_wwid,
+				pp->wwid) > 0) {
+			return 2;
+		}
+	}
 
 	return 0;
 
libmultipath/discovery.h +7 -4
Index: multipath-tools-120821/libmultipath/discovery.h
===================================================================
@@ -33,8 +33,9 @@ int do_tur (char *);
 int path_offline (struct path *);
 int get_state (struct path * pp, int daemon);
 int pathinfo (struct path *, vector hwtable, int mask);
-struct path * store_pathinfo (vector pathvec, vector hwtable,
-			      struct udev_device *udevice, int flag);
+int store_pathinfo (vector pathvec, vector hwtable,
+		    struct udev_device *udevice, int flag,
+		    struct path **pp_ptr);
 int sysfs_set_scsi_tmo (struct multipath *mpp);
 int sysfs_get_timeout(struct path *pp, unsigned int *timeout);
 
@@ -46,14 +47,16 @@ enum discovery_mode {
 	__DI_SERIAL,
 	__DI_CHECKER,
 	__DI_PRIO,
-	__DI_WWID
+	__DI_WWID,
+	__DI_BLACKLIST,
 };
 
 #define DI_SYSFS	(1 << __DI_SYSFS)
 #define DI_SERIAL	(1 << __DI_SERIAL)
 #define DI_CHECKER	(1 << __DI_CHECKER)
 #define DI_PRIO		(1 << __DI_PRIO)
-#define DI_WWID 	(1 << __DI_WWID)
+#define DI_WWID		(1 << __DI_WWID)
+#define DI_BLACKLIST	(1 << __DI_BLACKLIST)
 
 #define DI_ALL		(DI_SYSFS  | DI_SERIAL | DI_CHECKER | DI_PRIO | \
 			 DI_WWID)
libmultipath/alias.c +26 -21

104 lines not on this page · raw message

libmultipath/alias.h +1 -1
Index: multipath-tools-120821/libmultipath/alias.h
===================================================================
@@ -9,4 +9,4 @@
 
 char *get_user_friendly_alias(char *wwid, char *file, char *prefix,
 			      int bindings_readonly);
-char *get_user_friendly_wwid(char *alias, char *file);
+int get_user_friendly_wwid(char *alias, char *buff, char *file);
libmultipath/configure.c +47 -23

125 lines not on this page · raw message

libmultipath/configure.h +1 -1
Index: multipath-tools-120821/libmultipath/configure.h
===================================================================
@@ -27,6 +27,6 @@ int setup_map (struct multipath * mpp, c
 int domap (struct multipath * mpp, char * params);
 int reinstate_paths (struct multipath *mpp);
 int coalesce_paths (struct vectors *vecs, vector curmp, char * refwwid, int force_reload);
-char * get_refwwid (char * dev, enum devtypes dev_type, vector pathvec);
+int get_refwwid (char * dev, enum devtypes dev_type, vector pathvec, char **wwid);
 int reload_map(struct vectors *vecs, struct multipath *mpp);
 
libmultipath/util.c +1 -1
Index: multipath-tools-120821/libmultipath/util.c
===================================================================
@@ -27,7 +27,7 @@ basenamecpy (const char * str1, char * s
 	if (!str1 || !strlen(str1))
 		return 0;
 
-	if (strlen(str1) > str2len)
+	if (strlen(str1) >= str2len)
 		return 0;
 
 	if (!str2)
multipath/main.c +7 -7
Index: multipath-tools-120821/multipath/main.c
===================================================================
@@ -262,7 +262,7 @@ configure (void)
 	/*
 	 * if we have a blacklisted device parameter, exit early
 	 */
-	if (dev &&
+	if (dev && conf->dev_type == DEV_DEVNODE &&
 	    (filter_devnode(conf->blist_devnode,
 			    conf->elist_devnode, dev) > 0)) {
 		if (conf->dry_run == 2)
@@ -275,16 +275,16 @@ configure (void)
 	 * failing the translation is fatal (by policy)
 	 */
 	if (conf->dev) {
-		refwwid = get_refwwid(conf->dev, conf->dev_type, pathvec);
-
+		int failed = get_refwwid(conf->dev, conf->dev_type, pathvec,
+					 &refwwid);
 		if (!refwwid) {
-			condlog(3, "scope is nul");
+			if (failed == 2 && conf->dry_run == 2)
+				printf("%s is not a valid multipath device path\n", conf->dev);
+			else
+				condlog(3, "scope is nul");
 			goto out;
 		}
 		condlog(3, "scope limited to %s", refwwid);
-		if (filter_wwid(conf->blist_wwid, conf->elist_wwid,
-				refwwid) > 0)
-			goto out;
 		if (conf->dry_run == 2) {
 			if (check_wwids_file(refwwid, 0) == 0){
 				printf("%s is a valid multipath device path\n", conf->dev);
multipathd/cli_handlers.c +5 -6
Index: multipath-tools-120821/multipathd/cli_handlers.c
===================================================================
@@ -433,19 +433,18 @@ cli_add_path (void * v, char ** reply, i
 		udevice = udev_device_new_from_subsystem_sysname(conf->udev,
 								 "block",
 								 param);
-		pp = store_pathinfo(vecs->pathvec, conf->hwtable,
-				    udevice, DI_ALL);
+		r = store_pathinfo(vecs->pathvec, conf->hwtable,
+				   udevice, DI_ALL, &pp);
 		udev_device_unref(udevice);
 		if (!pp) {
+			if (r == 2)
+				goto blacklisted;
 			condlog(0, "%s: failed to store path info", param);
 			return 1;
 		}
 		pp->checkint = conf->checkint;
 	}
-	r = ev_add_path(pp, vecs);
-	if (r == 2)
-		goto blacklisted;
-	return r;
+	return ev_add_path(pp, vecs);
 blacklisted:
 	*reply = strdup("blacklisted\n");
 	*len = strlen(*reply) + 1;
multipathd/main.c +10 -12
Index: multipath-tools-120821/multipathd/main.c
===================================================================
@@ -299,7 +299,7 @@ ev_add_map (char * dev, char * alias, st
 		condlog(2, "%s: devmap %s registered", alias, dev);
 		return 0;
 	}
-	refwwid = get_refwwid(dev, DEV_DEVMAP, vecs->pathvec);
+	r = get_refwwid(dev, DEV_DEVMAP, vecs->pathvec, &refwwid);
 
 	if (refwwid) {
 		r = coalesce_paths(vecs, NULL, refwwid, 0);
@@ -308,6 +308,8 @@ ev_add_map (char * dev, char * alias, st
 
 	if (!r)
 		condlog(2, "%s: devmap %s added", alias, dev);
+	else if (r == 2)
+		condlog(2, "%s: uev_add_map %s blacklisted", alias, dev);
 	else
 		condlog(0, "%s: uev_add_map %s failed", alias, dev);
 
@@ -373,6 +375,7 @@ static int
 uev_add_path (struct uevent *uev, struct vectors * vecs)
 {
 	struct path *pp;
+	int ret;
 
 	condlog(2, "%s: add path (uevent)", uev->kernel);
 	if (strstr(uev->kernel, "..") != NULL) {
@@ -393,8 +396,11 @@ uev_add_path (struct uevent *uev, struct
 		/*
 		 * get path vital state
 		 */
-		if (!(pp = store_pathinfo(vecs->pathvec, conf->hwtable,
-					  uev->udev, DI_ALL))) {
+		ret = store_pathinfo(vecs->pathvec, conf->hwtable,
+				     uev->udev, DI_ALL, &pp);
+		if (!pp) {
+			if (ret == 2)
+				return 0;
 			condlog(0, "%s: failed to store path info",
 				uev->kernel);
 			return 1;
@@ -402,14 +408,13 @@ uev_add_path (struct uevent *uev, struct
 		pp->checkint = conf->checkint;
 	}
 
-	return (ev_add_path(pp, vecs) != 1)? 0 : 1;
+	return ev_add_path(pp, vecs);
 }
 
 /*
  * returns:
  * 0: added
  * 1: error
- * 2: blacklisted
  */
 int
 ev_add_path (struct path * pp, struct vectors * vecs)
@@ -427,13 +432,6 @@ ev_add_path (struct path * pp, struct ve
 		condlog(0, "%s: failed to get path uid", pp->dev);
 		goto fail; /* leave path added to pathvec */
 	}
-	if (filter_path(conf, pp) > 0){
-		int i = find_slot(vecs->pathvec, (void *)pp);
-		if (i != -1)
-			vector_del_slot(vecs->pathvec, i);
-		free_path(pp);
-		return 2;
-	}
 	mpp = pp->mpp = find_mp_by_wwid(vecs->mpvec, pp->wwid);
 rescan:
 	if (mpp) {