[libmultipath] fix user_friendly_names race

Without this patch I could fairly easily get two multipath mpath0 entries
in /var/lib/multipath/bindings by running 8 concurrent instances of
multipath(8) while with the patch I cannot get this problem to occur.

The posix file byte range locks used to provide atomicity for accessing the
entries in the multipath bindings file get released from whenever __any__
descriptor or FILE structure for that file is closed.  This patch delays
the fclose() for the FILE structures used within lookup_binding() and
rlookup_binding() until there is no more need for the atomicity.

Edward Goggin, EMC

diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index 6d103d7..86cae9b 100644 (file)
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
@@ -166,28 +166,14 @@ fail:
 
 
 static int
-lookup_binding(int fd, char *map_wwid, char **map_alias)
+lookup_binding(FILE *f, char *map_wwid, char **map_alias)
 {
        char buf[LINE_MAX];
-       FILE *f;
        unsigned int line_nr = 0;
-       int scan_fd;
        int id = 0;
 
        *map_alias = NULL;
-       scan_fd = dup(fd);
-       if (scan_fd < 0) {
-               condlog(0, "Cannot dup bindings file descriptor : %s",
-                       strerror(errno));
-               return -1;
-       }
-       f = fdopen(scan_fd, "r");
-       if (!f) {
-               condlog(0, "cannot fdopen on bindings file descriptor : %s",
-                       strerror(errno));
-               close(scan_fd);
-               return -1;
-       }
+
        while (fgets(buf, LINE_MAX, f)) {
                char *c, *alias, *wwid;
                int curr_id;
@@ -215,38 +201,22 @@ lookup_binding(int fd, char *map_wwid, char **map_alias)
                        if (*map_alias == NULL)
                                condlog(0, "Cannot copy alias from bindings "
                                        "file : %s", strerror(errno));
-                       fclose(f);
                        return id;
                }
        }
        condlog(3, "No matching wwid [%s] in bindings file.", map_wwid);
-       fclose(f);
        return id;
 }      
 
 static int
-rlookup_binding(int fd, char **map_wwid, char *map_alias)
+rlookup_binding(FILE *f, char **map_wwid, char *map_alias)
 {
        char buf[LINE_MAX];
-       FILE *f;
        unsigned int line_nr = 0;
-       int scan_fd;
        int id = 0;
 
        *map_wwid = NULL;
-       scan_fd = dup(fd);
-       if (scan_fd < 0) {
-               condlog(0, "Cannot dup bindings file descriptor : %s",
-                       strerror(errno));
-               return -1;
-       }
-       f = fdopen(scan_fd, "r");
-       if (!f) {
-               condlog(0, "cannot fdopen on bindings file descriptor : %s",
-                       strerror(errno));
-               close(scan_fd);
-               return -1;
-       }
+
        while (fgets(buf, LINE_MAX, f)) {
                char *c, *alias, *wwid;
                int curr_id;
@@ -274,12 +244,10 @@ rlookup_binding(int fd, char **map_wwid, char *map_alias)
                        if (*map_wwid == NULL)
                                condlog(0, "Cannot copy alias from bindings "
                                        "file : %s", strerror(errno));
-                       fclose(f);
                        return id;
                }
        }
        condlog(3, "No matching alias [%s] in bindings file.", map_alias);
-       fclose(f);
        return id;
 }      
 
@@ -327,7 +295,8 @@ char *
 get_user_friendly_alias(char *wwid, char *file)
 {
        char *alias;
-       int fd, id;
+       int fd, scan_fd, id;
+       FILE *f;
 
        if (!wwid || *wwid == '\0') {
                condlog(3, "Cannot find binding for empty WWID");
@@ -337,14 +306,37 @@ get_user_friendly_alias(char *wwid, char *file)
        fd = open_bindings_file(file);
        if (fd < 0)
                return NULL;
-       id = lookup_binding(fd, wwid, &alias);
+
+       scan_fd = dup(fd);
+       if (scan_fd < 0) {
+               condlog(0, "Cannot dup bindings file descriptor : %s",
+                       strerror(errno));
+               close(fd);
+               return NULL;
+       }
+
+       f = fdopen(scan_fd, "r");
+       if (!f) {
+               condlog(0, "cannot fdopen on bindings file descriptor : %s",
+                       strerror(errno));
+               close(scan_fd);
+               close(fd);
+               return NULL;
+       }
+
+       id = lookup_binding(f, wwid, &alias);
        if (id < 0) {
+               fclose(f);
+               close(scan_fd);
                close(fd);
                return NULL;
        }
+
        if (!alias)
                alias = allocate_binding(fd, wwid, id);
 
+       fclose(f);
+       close(scan_fd);
        close(fd);
        return alias;
 }
@@ -353,7 +345,8 @@ char *
 get_user_friendly_wwid(char *alias, char *file)
 {
        char *wwid;
-       int fd, id;
+       int fd, scan_fd, id;
+       FILE *f;
 
        if (!alias || *alias == '\0') {
                condlog(3, "Cannot find binding for empty alias");
@@ -363,12 +356,34 @@ get_user_friendly_wwid(char *alias, char *file)
        fd = open_bindings_file(file);
        if (fd < 0)
                return NULL;
-       id = rlookup_binding(fd, &wwid, alias);
+
+       scan_fd = dup(fd);
+       if (scan_fd < 0) {
+               condlog(0, "Cannot dup bindings file descriptor : %s",
+                       strerror(errno));
+               close(fd);
+               return NULL;
+       }
+
+       f = fdopen(scan_fd, "r");
+       if (!f) {
+               condlog(0, "cannot fdopen on bindings file descriptor : %s",
+                       strerror(errno));
+               close(scan_fd);
+               close(fd);
+               return NULL;
+       }
+
+       id = rlookup_binding(f, &wwid, alias);
        if (id < 0) {
+               fclose(f);
+               close(scan_fd);
                close(fd);
                return NULL;
        }
 
+       fclose(f);
+       close(scan_fd);
        close(fd);
        return wwid;
 }