From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Benjamin Marzinski" Subject: Re: [PATCH v4 18/20] multipath -u: quick check if path is multipathed Date: Thu, 12 Apr 2018 13:46:18 -0500 Message-ID: <20180412184618.GT3103@octiron.msp.redhat.com> References: <20180404161627.6244-1-mwilck@suse.com> <20180404161627.6244-19-mwilck@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20180404161627.6244-19-mwilck@suse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com To: Martin Wilck Cc: dm-devel@redhat.com, Julian Andres Klode List-Id: dm-devel.ids 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 > --- > 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 > #include > #include > +#include > > #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 > > 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