[dm-devel] [PATCH] multipath: Check blacklists as soon as possible
Benjamin Marzinski
bmarzins at redhat.com
Tue Jan 8 05:58:55 UTC 2013
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.
Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
---
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(-)
Index: multipath-tools-120821/libmultipath/discovery.c
===================================================================
--- multipath-tools-120821.orig/libmultipath/discovery.c
+++ 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;
Index: multipath-tools-120821/libmultipath/discovery.h
===================================================================
--- multipath-tools-120821.orig/libmultipath/discovery.h
+++ 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)
Index: multipath-tools-120821/libmultipath/alias.c
===================================================================
--- multipath-tools-120821.orig/libmultipath/alias.c
+++ multipath-tools-120821/libmultipath/alias.c
@@ -13,6 +13,9 @@
#include "uxsock.h"
#include "alias.h"
#include "file.h"
+#include "vector.h"
+#include "checkers.h"
+#include "structs.h"
/*
@@ -121,23 +124,23 @@ lookup_binding(FILE *f, char *map_wwid,
}
static int
-rlookup_binding(FILE *f, char **map_wwid, char *map_alias)
+rlookup_binding(FILE *f, char *buff, char *map_alias)
{
- char buf[LINE_MAX];
+ char line[LINE_MAX];
unsigned int line_nr = 0;
int id = 0;
- *map_wwid = NULL;
+ buff[0] = '\0';
- while (fgets(buf, LINE_MAX, f)) {
+ while (fgets(line, LINE_MAX, f)) {
char *c, *alias, *wwid;
int curr_id;
line_nr++;
- c = strpbrk(buf, "#\n\r");
+ c = strpbrk(line, "#\n\r");
if (c)
*c = '\0';
- alias = strtok(buf, " \t");
+ alias = strtok(line, " \t");
if (!alias) /* blank line */
continue;
curr_id = scan_devname(alias, NULL); /* TBD: Why this call? */
@@ -150,13 +153,16 @@ rlookup_binding(FILE *f, char **map_wwid
line_nr);
continue;
}
+ if (strlen(wwid) > WWID_SIZE - 1) {
+ condlog(3,
+ "Ignoring too large wwid at %u in bindings file", line_nr);
+ continue;
+ }
if (strcmp(alias, map_alias) == 0){
condlog(3, "Found matching alias [%s] in bindings file."
"\nSetting wwid to %s", alias, wwid);
- *map_wwid = strdup(wwid);
- if (*map_wwid == NULL)
- condlog(0, "Cannot copy alias from bindings "
- "file : %s", strerror(errno));
+ strncpy(buff, wwid, WWID_SIZE);
+ buff[WWID_SIZE - 1] = '\0';
return id;
}
}
@@ -255,36 +261,35 @@ get_user_friendly_alias(char *wwid, char
return alias;
}
-char *
-get_user_friendly_wwid(char *alias, char *file)
+int
+get_user_friendly_wwid(char *alias, char *buff, char *file)
{
- char *wwid;
- int fd, id, unused;
+ int fd, unused;
FILE *f;
if (!alias || *alias == '\0') {
condlog(3, "Cannot find binding for empty alias");
- return NULL;
+ return -1;
}
fd = open_file(file, &unused, BINDINGS_FILE_HEADER);
if (fd < 0)
- return NULL;
+ return -1;
f = fdopen(fd, "r");
if (!f) {
condlog(0, "cannot fdopen on bindings file descriptor : %s",
strerror(errno));
close(fd);
- return NULL;
+ return -1;
}
- id = rlookup_binding(f, &wwid, alias);
- if (id < 0) {
+ rlookup_binding(f, buff, alias);
+ if (!strlen(buff)) {
fclose(f);
- return NULL;
+ return -1;
}
fclose(f);
- return wwid;
+ return 0;
}
Index: multipath-tools-120821/libmultipath/alias.h
===================================================================
--- multipath-tools-120821.orig/libmultipath/alias.h
+++ 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);
Index: multipath-tools-120821/libmultipath/configure.c
===================================================================
--- multipath-tools-120821.orig/libmultipath/configure.c
+++ multipath-tools-120821/libmultipath/configure.c
@@ -676,21 +676,32 @@ coalesce_paths (struct vectors * vecs, v
return 0;
}
-extern char *
-get_refwwid (char * dev, enum devtypes dev_type, vector pathvec)
+/*
+ * returns:
+ * 0 - success
+ * 1 - failure
+ * 2 - blacklist
+ */
+extern int
+get_refwwid (char * dev, enum devtypes dev_type, vector pathvec, char **wwid)
{
+ int ret = 1;
struct path * pp;
char buff[FILE_NAME_SIZE];
char * refwwid = NULL, tmpwwid[WWID_SIZE];
+ if (!wwid)
+ return 1;
+ *wwid = NULL;
+
if (dev_type == DEV_NONE)
- return NULL;
+ return 1;
if (dev_type == DEV_DEVNODE) {
if (basenamecpy(dev, buff, FILE_NAME_SIZE) == 0) {
condlog(1, "basename failed for '%s' (%s)",
dev, buff);
- return NULL;
+ return 1;
}
pp = find_path_by_dev(pathvec, buff);
@@ -699,14 +710,16 @@ get_refwwid (char * dev, enum devtypes d
if (!udevice) {
condlog(2, "%s: can't get udev device", buff);
- return NULL;
+ return 1;
}
- pp = store_pathinfo(pathvec, conf->hwtable, udevice,
- DI_SYSFS | DI_WWID);
+ ret = store_pathinfo(pathvec, conf->hwtable, udevice,
+ DI_SYSFS | DI_WWID, &pp);
udev_device_unref(udevice);
if (!pp) {
- condlog(0, "%s can't store path info", buff);
- return NULL;
+ if (ret == 1)
+ condlog(0, "%s can't store path info",
+ buff);
+ return ret;
}
}
refwwid = pp->wwid;
@@ -721,14 +734,16 @@ get_refwwid (char * dev, enum devtypes d
if (!udevice) {
condlog(2, "%s: can't get udev device", dev);
- return NULL;
+ return 1;
}
- pp = store_pathinfo(pathvec, conf->hwtable, udevice,
- DI_SYSFS | DI_WWID);
+ ret = store_pathinfo(pathvec, conf->hwtable, udevice,
+ DI_SYSFS | DI_WWID, &pp);
udev_device_unref(udevice);
if (!pp) {
- condlog(0, "%s can't store path info", buff);
- return NULL;
+ if (ret == 1)
+ condlog(0, "%s can't store path info",
+ buff);
+ return ret;
}
}
refwwid = pp->wwid;
@@ -738,17 +753,17 @@ get_refwwid (char * dev, enum devtypes d
if (((dm_get_uuid(dev, tmpwwid)) == 0) && (strlen(tmpwwid))) {
refwwid = tmpwwid;
- goto out;
+ goto check;
}
/*
* may be a binding
*/
- refwwid = get_user_friendly_wwid(dev,
- conf->bindings_file);
-
- if (refwwid)
- return refwwid;
+ if (get_user_friendly_wwid(dev, tmpwwid,
+ conf->bindings_file) == 0) {
+ refwwid = tmpwwid;
+ goto check;
+ }
/*
* or may be an alias
@@ -760,12 +775,21 @@ get_refwwid (char * dev, enum devtypes d
*/
if (!refwwid)
refwwid = dev;
+
+check:
+ if (refwwid && strlen(refwwid)) {
+ if (filter_wwid(conf->blist_wwid, conf->elist_wwid,
+ refwwid) > 0)
+ return 2;
+ }
}
out:
- if (refwwid && strlen(refwwid))
- return STRDUP(refwwid);
+ if (refwwid && strlen(refwwid)) {
+ *wwid = STRDUP(refwwid);
+ return 0;
+ }
- return NULL;
+ return 1;
}
extern int reload_map(struct vectors *vecs, struct multipath *mpp)
Index: multipath-tools-120821/libmultipath/configure.h
===================================================================
--- multipath-tools-120821.orig/libmultipath/configure.h
+++ 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);
Index: multipath-tools-120821/libmultipath/util.c
===================================================================
--- multipath-tools-120821.orig/libmultipath/util.c
+++ 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)
Index: multipath-tools-120821/multipath/main.c
===================================================================
--- multipath-tools-120821.orig/multipath/main.c
+++ 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);
Index: multipath-tools-120821/multipathd/cli_handlers.c
===================================================================
--- multipath-tools-120821.orig/multipathd/cli_handlers.c
+++ 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;
Index: multipath-tools-120821/multipathd/main.c
===================================================================
--- multipath-tools-120821.orig/multipathd/main.c
+++ 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) {
More information about the dm-devel
mailing list