Rework sysfs device handling in multipathd

Relying on sysfs devices has the disadvantage that the device
might already been gone by the time we look at it. And we don't
actually need it for eg device-mapper events as we can get all
required information via the device-mapper ioctl.
So only access sysfs if we absolutely have to and try to get
the information from other places if possible.

Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 9ee8b9e82fa3bf1e1f460591b9e680e28765dd0b..5d126eb9000975b4aa038f6b4c80084ec584cf15 100644 (file)
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -627,6 +627,31 @@ out:
        return r;
 }
 
+int
+dm_get_major (char * mapname)
+{
+       int r = -1;
+       struct dm_task *dmt;
+       struct dm_info info;
+
+       if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
+               return 0;
+
+       if (!dm_task_set_name(dmt, mapname))
+               goto out;
+
+       if (!dm_task_run(dmt))
+               goto out;
+
+       if (!dm_task_get_info(dmt, &info))
+               goto out;
+
+       r = info.major;
+out:
+       dm_task_destroy(dmt);
+       return r;
+}
+
 int
 dm_get_minor (char * mapname)
 {

diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 8306993879f453124e49638a11ae15e5510d010d..47e47900a63b59d3d43b60359caab0498f854cd7 100644 (file)
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -32,6 +32,7 @@ int dm_enablegroup(char * mapname, int index);
 int dm_disablegroup(char * mapname, int index);
 int dm_get_maps (vector mp);
 int dm_geteventnr (char *name);
+int dm_get_major (char *name);
 int dm_get_minor (char *name);
 char * dm_mapname(int major, int minor);
 int dm_remove_partmaps (const char * mapname, int need_sync);

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index b5911f6cf4c696a7a30b25ce89406024e0c609aa..a1be5de85379805f3c97f1cdd612432e24108f4d 100644 (file)
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -382,8 +382,7 @@ out:
 }
 
 extern struct multipath *
-add_map_without_path (struct vectors * vecs,
-                     int minor, char * alias)
+add_map_without_path (struct vectors * vecs, char * alias)
 {
        struct multipath * mpp = alloc_multipath();
 

diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index d059da5ea047af8ddd01e119676cd8740ee29a7e..8c745934de4e9252203529f0a09ccae8946ae29e 100644 (file)
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -29,8 +29,7 @@ void remove_map_and_stop_waiter (struct multipath * mpp, struct vectors * vecs,
 void remove_maps (struct vectors * vecs);
 void remove_maps_and_stop_waiters (struct vectors * vecs);
 
-struct multipath * add_map_without_path (struct vectors * vecs,
-                               int minor, char * alias);
+struct multipath * add_map_without_path (struct vectors * vecs, char * alias);
 struct multipath * add_map_with_path (struct vectors * vecs,
                                struct path * pp, int add_vec);
 int update_multipath (struct vectors *vecs, char *mapname);

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index bd9b55ea18ed85e0a3e6c49c953155f144faed12..3ae0bf237483717701041d35e3024772c449a741 100644 (file)
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -363,10 +363,13 @@ int uevent_listen(void)
                uev->envp[i] = NULL;
 
                condlog(3, "uevent '%s' from '%s'", uev->action, uev->devpath);
+               uev->kernel = strrchr(uev->devpath, '/');
+               if (uev->kernel)
+                       uev->kernel++;
 
                /* print payload environment */
                for (i = 0; uev->envp[i] != NULL; i++)
-                       condlog(3, "%s", uev->envp[i]);
+                       condlog(5, "%s", uev->envp[i]);
 
                /*
                 * Queue uevent and poke service pthread.
@@ -387,3 +390,60 @@ exit:
 
        return 1;
 }
+
+extern int
+uevent_get_major(struct uevent *uev)
+{
+       char *p, *q;
+       int i, major = -1;
+
+       for (i = 0; uev->envp[i] != NULL; i++) {
+               if (!strncmp(uev->envp[i], "MAJOR", 5) && strlen(uev->envp[i]) > 6) {
+                       p = uev->envp[i] + 6;
+                       major = strtoul(p, &q, 10);
+                       if (p == q) {
+                               condlog(2, "invalid major '%s'", p);
+                               major = -1;
+                       }
+                       break;
+               }
+       }
+       return major;
+}
+
+extern int
+uevent_get_minor(struct uevent *uev)
+{
+       char *p, *q;
+       int i, minor = -1;
+
+       for (i = 0; uev->envp[i] != NULL; i++) {
+               if (!strncmp(uev->envp[i], "MINOR", 5) && strlen(uev->envp[i]) > 6) {
+                       p = uev->envp[i] + 6;
+                       minor = strtoul(p, &q, 10);
+                       if (p == q) {
+                               condlog(2, "invalid minor '%s'", p);
+                               minor = -1;
+                       }
+                       break;
+               }
+       }
+       return minor;
+}
+
+extern char *
+uevent_get_dm_name(struct uevent *uev)
+{
+       char *p = NULL;
+       int i;
+
+       for (i = 0; uev->envp[i] != NULL; i++) {
+               if (!strncmp(uev->envp[i], "DM_NAME", 6) &&
+                   strlen(uev->envp[i]) > 7) {
+                       p = MALLOC(strlen(uev->envp[i] + 8) + 1);
+                       strcpy(p, uev->envp[i] + 8);
+                       break;
+               }
+       }
+       return p;
+}

diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 7bb6c393c7e62798e6d4210f47f0daa68981ef17..192f94a7300b39113c724294399ab429eee24134 100644 (file)
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -18,6 +18,7 @@ struct uevent {
        char buffer[HOTPLUG_BUFFER_SIZE + OBJECT_SIZE];
        char *devpath;
        char *action;
+       char *kernel;
        char *envp[HOTPLUG_NUM_ENVP];
 };
 
@@ -27,5 +28,8 @@ void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
 int uevent_listen(void);
 int uevent_dispatch(int (*store_uev)(struct uevent *, void * trigger_data),
                    void * trigger_data);
+int uevent_get_major(struct uevent *uev);
+int uevent_get_minor(struct uevent *uev);
+char *uevent_get_dm_name(struct uevent *uev);
 
 #endif /* _UEVENT_H */

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index bbe2d7c54597de0e16c13286f256a08f5f131dd4..8367a7037879496f1c0b56628e2be13ac7925c1b 100644 (file)
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -383,9 +383,10 @@ cli_add_map (void * v, char ** reply, int * len, void * data)
 {
        struct vectors * vecs = (struct vectors *)data;
        char * param = get_keyparam(v, MAP);
-       int minor;
+       int major, minor;
        char dev_path[PATH_SIZE];
-       struct sysfs_device *sysdev;
+       char *alias;
+       int rc;
 
        condlog(2, "%s: add map (operator)", param);
 
@@ -400,13 +401,21 @@ cli_add_map (void * v, char ** reply, int * len, void * data)
                condlog(2, "%s: not a device mapper table", param);
                return 0;
        }
-       sprintf(dev_path,"/block/dm-%d", minor);
-       sysdev = sysfs_device_get(dev_path);
-       if (!sysdev) {
-               condlog(2, "%s: not found in sysfs", param);
+       major = dm_get_major(param);
+       if (major < 0) {
+               condlog(2, "%s: not a device mapper table", param);
+               return 0;
+       }
+       sprintf(dev_path,"dm-%d", minor);
+       alias = dm_mapname(major, minor);
+       if (!alias) {
+               condlog(2, "%s: mapname not found for %d:%d",
+                       param, major, minor);
                return 0;
        }
-       return ev_add_map(sysdev, vecs);
+       rc = ev_add_map(dev_path, alias, vecs);
+       FREE(alias);
+       return rc;
 }
 
 int
@@ -414,10 +423,32 @@ cli_del_map (void * v, char ** reply, int * len, void * data)
 {
        struct vectors * vecs = (struct vectors *)data;
        char * param = get_keyparam(v, MAP);
+       int major, minor;
+       char dev_path[PATH_SIZE];
+       char *alias;
+       int rc;
 
        condlog(2, "%s: remove map (operator)", param);
-
-       return ev_remove_map(param, vecs);
+       minor = dm_get_minor(param);
+       if (minor < 0) {
+               condlog(2, "%s: not a device mapper table", param);
+               return 0;
+       }
+       major = dm_get_major(param);
+       if (major < 0) {
+               condlog(2, "%s: not a device mapper table", param);
+               return 0;
+       }
+       sprintf(dev_path,"dm-%d", minor);
+       alias = dm_mapname(major, minor);
+       if (!alias) {
+               condlog(2, "%s: mapname not found for %d:%d",
+                       param, major, minor);
+               return 0;
+       }
+       rc = ev_remove_map(param, alias, minor, vecs);
+       FREE(alias);
+       return rc;
 }
 
 int resize_map(struct multipath *mpp, unsigned long long size,

diff --git a/multipathd/main.c b/multipathd/main.c
index d8c6f19d666279cf651f5a8a95960705e7d5aab0..1fcfb320927e771d08baf879659c46b01420ca4b 100644 (file)
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -48,6 +48,7 @@
 #include <configure.h>
 #include <prio.h>
 #include <pgpolicies.h>
+#include <uevent.h>
 
 #include "main.h"
 #include "pidfile.h"
@@ -214,32 +215,41 @@ flush_map(struct multipath * mpp, struct vectors * vecs)
 }
 
 static int
-uev_add_map (struct sysfs_device * dev, struct vectors * vecs)
+uev_add_map (struct uevent * uev, struct vectors * vecs)
 {
-       condlog(2, "%s: add map (uevent)", dev->kernel);
-       return ev_add_map(dev, vecs);
+       char *alias;
+       int major = -1, minor = -1, rc;
+
+       condlog(2, "%s: add map (uevent)", uev->kernel);
+       alias = uevent_get_dm_name(uev);
+       if (!alias) {
+               condlog(3, "%s: No DM_NAME in uevent", uev->kernel);
+               major = uevent_get_major(uev);
+               minor = uevent_get_minor(uev);
+               alias = dm_mapname(major, minor);
+               if (!alias) {
+                       condlog(2, "%s: mapname not found for %d:%d",
+                               uev->kernel, major, minor);
+                       return 1;
+               }
+       }
+       rc = ev_add_map(uev->kernel, alias, vecs);
+       FREE(alias);
+       return rc;
 }
 
 int
-ev_add_map (struct sysfs_device * dev, struct vectors * vecs)
+ev_add_map (char * dev, char * alias, struct vectors * vecs)
 {
-       char * alias;
-       int major, minor;
        char * refwwid;
        struct multipath * mpp;
        int map_present;
        int r = 1;
 
-       alias = dm_mapname(major, minor);
-
-       if (!alias)
-               return 1;
-
        map_present = dm_map_present(alias);
 
        if (map_present && dm_type(alias, TGT_MPATH) <= 0) {
                condlog(4, "%s: not a multipath map", alias);
-               FREE(alias);
                return 0;
        }
 
@@ -251,21 +261,19 @@ ev_add_map (struct sysfs_device * dev, struct vectors * vecs)
                 * if we create a multipath mapped device as a result
                 * of uev_add_path
                 */
-               condlog(0, "%s: devmap already registered",
-                       dev->kernel);
-               FREE(alias);
+               condlog(0, "%s: devmap already registered", dev);
                return 0;
        }
 
        /*
         * now we can register the map
         */
-       if (map_present && (mpp = add_map_without_path(vecs, minor, alias))) {
+       if (map_present && (mpp = add_map_without_path(vecs, alias))) {
                sync_map_state(mpp);
-               condlog(2, "%s: devmap %s added", alias, dev->kernel);
+               condlog(2, "%s: devmap %s registered", alias, dev);
                return 0;
        }
-       refwwid = get_refwwid(dev->kernel, DEV_DEVMAP, vecs->pathvec);
+       refwwid = get_refwwid(dev, DEV_DEVMAP, vecs->pathvec);
 
        if (refwwid) {
                r = coalesce_paths(vecs, NULL, refwwid, 0);
@@ -273,47 +281,60 @@ ev_add_map (struct sysfs_device * dev, struct vectors * vecs)
        }
 
        if (!r)
-               condlog(2, "%s: devmap %s added", alias, dev->kernel);
+               condlog(2, "%s: devmap %s added", alias, dev);
        else
-               condlog(0, "%s: uev_add_map %s failed", alias, dev->kernel);
+               condlog(0, "%s: uev_add_map %s failed", alias, dev);
 
        FREE(refwwid);
-       FREE(alias);
        return r;
 }
 
 static int
-uev_remove_map (struct sysfs_device * dev, struct vectors * vecs)
+uev_remove_map (struct uevent * uev, struct vectors * vecs)
 {
-       condlog(2, "%s: remove map (uevent)", dev->kernel);
-       return ev_remove_map(dev->kernel, vecs);
+       char *alias;
+       int minor, rc;
+
+       condlog(2, "%s: remove map (uevent)", uev->kernel);
+       alias = uevent_get_dm_name(uev);
+       if (!alias) {
+               condlog(3, "%s: No DM_NAME in uevent, ignoring", uev->kernel);
+               return 0;
+       }
+       minor = uevent_get_minor(uev);
+       rc = ev_remove_map(uev->kernel, alias, minor, vecs);
+       FREE(alias);
+       return rc;
 }
 
 int
-ev_remove_map (char * devname, struct vectors * vecs)
+ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs)
 {
        struct multipath * mpp;
 
-       mpp = find_mp_by_str(vecs->mpvec, devname);
+       mpp = find_mp_by_minor(vecs->mpvec, minor);
 
        if (!mpp) {
                condlog(2, "%s: devmap not registered, can't remove",
                        devname);
                return 0;
        }
-       flush_map(mpp, vecs);
-
-       return 0;
+       if (strcmp(mpp->alias, alias)) {
+               condlog(2, "%s: minor number mismatch (map %d, event %d)",
+                       mpp->alias, mpp->dmi->minor, minor);
+               return 0;
+       }
+       return flush_map(mpp, vecs);
 }
 
 static int
-uev_umount_map (struct sysfs_device * dev, struct vectors * vecs)
+uev_umount_map (struct uevent * uev, struct vectors * vecs)
 {
        struct multipath * mpp;
 
-       condlog(2, "%s: umount map (uevent)", dev->kernel);
+       condlog(2, "%s: umount map (uevent)", uev->kernel);
 
-       mpp = find_mp_by_str(vecs->mpvec, dev->kernel);
+       mpp = find_mp_by_str(vecs->mpvec, uev->kernel);
 
        if (!mpp)
                return 0;
@@ -327,14 +348,21 @@ uev_umount_map (struct sysfs_device * dev, struct vectors * vecs)
        return 0;
 }
 
+
 static int
-uev_add_path (struct sysfs_device * dev, struct vectors * vecs)
+uev_add_path (struct uevent *uev, struct vectors * vecs)
 {
+       struct sysfs_device * dev;
+
+       dev = sysfs_device_get(uev->devpath);
+       if (!dev) {
+               condlog(2, "%s: not found in sysfs", uev->devpath);
+               return 1;
+       }
        condlog(2, "%s: add path (uevent)", dev->kernel);
        return (ev_add_path(dev->kernel, vecs) != 1)? 0 : 1;
 }
 
-
 /*
  * returns:
  * 0: added
@@ -488,12 +516,19 @@ fail:
 }
 
 static int
-uev_remove_path (struct sysfs_device * dev, struct vectors * vecs)
+uev_remove_path (struct uevent *uev, struct vectors * vecs)
 {
+       struct sysfs_device * dev;
        int retval;
 
-       condlog(2, "%s: remove path (uevent)", dev->kernel);
-       retval = ev_remove_path(dev->kernel, vecs);
+       dev = sysfs_device_get(uev->devpath);
+       if (!dev) {
+               condlog(2, "%s: not found in sysfs", uev->devpath);
+               return 1;
+       }
+       condlog(2, "%s: remove path (uevent)", uev->kernel);
+       retval = ev_remove_path(uev->kernel, vecs);
+
        if (!retval)
                sysfs_device_put(dev);
 
@@ -676,7 +711,6 @@ int
 uev_trigger (struct uevent * uev, void * trigger_data)
 {
        int r = 0;
-       struct sysfs_device *sysdev;
        struct vectors * vecs;
 
        vecs = (struct vectors *)trigger_data;
@@ -684,10 +718,6 @@ uev_trigger (struct uevent * uev, void * trigger_data)
        if (uev_discard(uev->devpath))
                return 0;
 
-       sysdev = sysfs_device_get(uev->devpath);
-       if(!sysdev)
-               return 0;
-
        lock(vecs->lock);
 
        /*
@@ -695,17 +725,17 @@ uev_trigger (struct uevent * uev, void * trigger_data)
         * Add events are ignored here as the tables
         * are not fully initialised then.
         */
-       if (!strncmp(sysdev->kernel, "dm-", 3)) {
+       if (!strncmp(uev->kernel, "dm-", 3)) {
                if (!strncmp(uev->action, "change", 6)) {
-                       r = uev_add_map(sysdev, vecs);
+                       r = uev_add_map(uev, vecs);
                        goto out;
                }
                if (!strncmp(uev->action, "remove", 6)) {
-                       r = uev_remove_map(sysdev, vecs);
+                       r = uev_remove_map(uev, vecs);
                        goto out;
                }
                if (!strncmp(uev->action, "umount", 6)) {
-                       r = uev_umount_map(sysdev, vecs);
+                       r = uev_umount_map(uev, vecs);
                        goto out;
                }
                goto out;
@@ -715,15 +745,15 @@ uev_trigger (struct uevent * uev, void * trigger_data)
         * path add/remove event
         */
        if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
-                          sysdev->kernel) > 0)
+                          uev->kernel) > 0)
                goto out;
 
        if (!strncmp(uev->action, "add", 3)) {
-               r = uev_add_path(sysdev, vecs);
+               r = uev_add_path(uev, vecs);
                goto out;
        }
        if (!strncmp(uev->action, "remove", 6)) {
-               r = uev_remove_path(sysdev, vecs);
+               r = uev_remove_path(uev, vecs);
                goto out;
        }
 
@@ -1526,6 +1556,9 @@ child (void * param)
        FREE(vecs);
        vecs = NULL;
 
+       cleanup_checkers();
+       cleanup_prio();
+
        condlog(2, "--------shut down-------");
 
        if (logsink)

diff --git a/multipathd/main.h b/multipathd/main.h
index 136b7e549f06fc1508e3abe727feee47f4456034..39909675f6d30d4ec39ec732491f57f400c8257e 100644 (file)
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -6,8 +6,8 @@
 int reconfigure (struct vectors *);
 int ev_add_path (char *, struct vectors *);
 int ev_remove_path (char *, struct vectors *);
-int ev_add_map (struct sysfs_device *, struct vectors *);
-int ev_remove_map (char *, struct vectors *);
+int ev_add_map (char *, char *, struct vectors *);
+int ev_remove_map (char *, char *, int, struct vectors *);
 void sync_map_state (struct multipath *);
 
 #endif /* MAIN_H */