On Wed, Apr 04, 2018 at 06:16:25PM +0200, Martin Wilck wrote:
> With "find_multipaths smart", we accept paths as valid if they are
> already part of a multipath map. This patch avoids doing a full path
> and device-mapper map scan for this case, speeding up "multipath -u"
> considerably.

I feel like this supports my idea from 17/20. I really think that in
smart mode, the only time we should be claiming a device as multipath is
on an add event, if it has always been claimed (or pretend claimed) as a
multipath path, or if it is currently a multipath path. Once we have let
a uevent go by for a path without setting SYSTEMD_READY=0, anything else
is free to use it, and we simply can't safely set SYSTEMD_READY=0,
unless we know that multipathd has already grabbed the device. I feel
like checking the wwids file should give you confidence that a device
either currently is a multipath path, or has always been claimed as one.
However, this patch can fix any loop holes where a device isn't in the
wwids file, but it multipathed (I don't know of any offhand).

-Ben


> 
> Signed-off-by: Martin Wilck <mwi...@suse.com>
> ---
>  libmultipath/sysfs.c | 66 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libmultipath/sysfs.h |  2 ++
>  multipath/main.c     | 14 ++++++++++-
>  3 files changed, 81 insertions(+), 1 deletion(-)
> 
> diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
> index 97e0997..589eeaf 100644
> --- a/libmultipath/sysfs.c
> +++ b/libmultipath/sysfs.c
> @@ -27,6 +27,7 @@
>  #include <string.h>
>  #include <dirent.h>
>  #include <libudev.h>
> +#include <fnmatch.h>
>  
>  #include "checkers.h"
>  #include "vector.h"
> @@ -287,3 +288,68 @@ int sysfs_check_holders(char * check_devt, char * 
> new_devt)
>  
>       return 0;
>  }
> +
> +static int select_dm_devs(const struct dirent *di)
> +{
> +     return fnmatch("dm-*", di->d_name, FNM_FILE_NAME) == 0;
> +}
> +
> +static void close_fd(void *arg)
> +{
> +     close((long)arg);
> +}
> +
> +bool sysfs_is_multipathed(const struct path *pp)
> +{
> +     char pathbuf[PATH_MAX];
> +     struct dirent **di;
> +     int n, r, i;
> +     bool found = false;
> +
> +     n = snprintf(pathbuf, sizeof(pathbuf), "/sys/block/%s/holders",
> +                  pp->dev);
> +
> +     if (n >= sizeof(pathbuf)) {
> +             condlog(1, "%s: pathname overflow", __func__);
> +             return false;
> +     }
> +
> +     r = scandir(pathbuf, &di, select_dm_devs, alphasort);
> +     if (r == 0)
> +             return false;
> +     else if (r < 0) {
> +             condlog(1, "%s: error scanning %s", __func__, pathbuf);
> +             return false;
> +     }
> +
> +     pthread_cleanup_push(free, di);
> +     for (i = 0; i < r && !found; i++) {
> +             long fd;
> +             int nr;
> +             char uuid[6];
> +
> +             if (snprintf(pathbuf + n, sizeof(pathbuf) - n,
> +                          "/%s/dm/uuid", di[i]->d_name)
> +                 >= sizeof(pathbuf) - n)
> +                     continue;
> +
> +             fd = open(pathbuf, O_RDONLY);
> +             if (fd == -1) {
> +                     condlog(1, "%s: error opening %s", __func__, pathbuf);
> +                     continue;
> +             }
> +
> +             pthread_cleanup_push(close_fd, (void*)fd);
> +             nr = read(fd, uuid, sizeof(uuid));
> +             if (nr == sizeof(uuid) && !memcmp(uuid, "mpath-", sizeof(uuid)))
> +                     found = true;
> +             else if (nr < 0) {
> +                     condlog(1, "%s: error reading from %s: %s",
> +                             __func__, pathbuf, strerror(errno));
> +             }
> +             pthread_cleanup_pop(1);
> +     }
> +     pthread_cleanup_pop(1);
> +
> +     return found;
> +}
> diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
> index 75c0f9c..9ae30b3 100644
> --- a/libmultipath/sysfs.h
> +++ b/libmultipath/sysfs.h
> @@ -4,6 +4,7 @@
>  
>  #ifndef _LIBMULTIPATH_SYSFS_H
>  #define _LIBMULTIPATH_SYSFS_H
> +#include <stdbool.h>
>  
>  ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
>                            const char * value, size_t value_len);
> @@ -13,4 +14,5 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, 
> const char *attr_name,
>                                unsigned char * value, size_t value_len);
>  int sysfs_get_size (struct path *pp, unsigned long long * size);
>  int sysfs_check_holders(char * check_devt, char * new_devt);
> +bool sysfs_is_multipathed(const struct path *pp);
>  #endif
> diff --git a/multipath/main.c b/multipath/main.c
> index a60a5f3..ae46e5a 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -595,6 +595,15 @@ configure (struct config *conf, enum mpath_cmds cmd,
>                           !ignore_wwids_on(conf)) {
>                               goto print_valid;
>                       }
> +                     /*
> +                      * Shortcut for find_multipaths smart:
> +                      * Quick check if path is already multipathed.
> +                      */
> +                     if (ignore_wwids_on(conf) &&
> +                         sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0))) {
> +                             r = 0;
> +                             goto print_valid;
> +                     }
>               }
>       }
>  
> @@ -667,7 +676,10 @@ configure (struct config *conf, enum mpath_cmds cmd,
>                       condlog(3, "%s: path %s is in use: %s",
>                               __func__, pp->dev,
>                               strerror(errno));
> -                     r = 1;
> +                     /*
> +                      * Check if we raced with multipathd
> +                      */
> +                     r = !sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0));
>               }
>               goto print_valid;
>       }
> -- 
> 2.16.1

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to