* [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
@ 2017-02-28 16:22 ` Martin Wilck
2017-03-13 23:06 ` Benjamin Marzinski
2017-02-28 16:22 ` [PATCH 02/33] multipathd: start daemon after udev trigger Martin Wilck
` (33 subsequent siblings)
34 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:22 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
With the latest LVM2 update we now have the 'lvm2-lvmetad.service'.
Also we need to specify 'blk-availability.service' in the 'Before='
statement, as just adding it to 'Wants=' assumes the multipathd
service should be running after the blk-availability service.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
multipathd/multipathd.service | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index d26577f3..be13138c 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -1,10 +1,9 @@
[Unit]
Description=Device-Mapper Multipath Device Controller
-Before=iscsi.service iscsid.service lvm2-activation-early.service
-Before=local-fs-pre.target systemd-udev-trigger.service
+Before=iscsi.service iscsid.service lvm2-lvmetad.service lvm2-activation-early.service
+Before=local-fs-pre.target systemd-udev-trigger.service blk-availability.service
After=multipathd.socket systemd-udevd.service
DefaultDependencies=no
-Wants=local-fs-pre.target multipathd.socket blk-availability.service
Conflicts=shutdown.target
[Service]
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements
2017-02-28 16:22 ` [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements Martin Wilck
@ 2017-03-13 23:06 ` Benjamin Marzinski
2017-03-14 7:36 ` Martin Wilck
0 siblings, 1 reply; 57+ messages in thread
From: Benjamin Marzinski @ 2017-03-13 23:06 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
On Tue, Feb 28, 2017 at 05:22:57PM +0100, Martin Wilck wrote:
> From: Hannes Reinecke <hare@suse.de>
>
> With the latest LVM2 update we now have the 'lvm2-lvmetad.service'.
> Also we need to specify 'blk-availability.service' in the 'Before='
> statement, as just adding it to 'Wants=' assumes the multipathd
> service should be running after the blk-availability service.
As far as I can tell, The Before= command only says that if the other
unit is started, this one will be started first. It doesn't take the
place of Wants= or Requires=. So I think we should keep that Wants=
line.
-Ben
>
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
> multipathd/multipathd.service | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
> index d26577f3..be13138c 100644
> --- a/multipathd/multipathd.service
> +++ b/multipathd/multipathd.service
> @@ -1,10 +1,9 @@
> [Unit]
> Description=Device-Mapper Multipath Device Controller
> -Before=iscsi.service iscsid.service lvm2-activation-early.service
> -Before=local-fs-pre.target systemd-udev-trigger.service
> +Before=iscsi.service iscsid.service lvm2-lvmetad.service lvm2-activation-early.service
> +Before=local-fs-pre.target systemd-udev-trigger.service blk-availability.service
> After=multipathd.socket systemd-udevd.service
> DefaultDependencies=no
> -Wants=local-fs-pre.target multipathd.socket blk-availability.service
> Conflicts=shutdown.target
>
> [Service]
> --
> 2.11.0
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements
2017-03-13 23:06 ` Benjamin Marzinski
@ 2017-03-14 7:36 ` Martin Wilck
0 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-03-14 7:36 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel
On Mon, 2017-03-13 at 18:06 -0500, Benjamin Marzinski wrote:
> On Tue, Feb 28, 2017 at 05:22:57PM +0100, Martin Wilck wrote:
> > From: Hannes Reinecke <hare@suse.de>
> >
> > With the latest LVM2 update we now have the 'lvm2-lvmetad.service'.
> > Also we need to specify 'blk-availability.service' in the 'Before='
> > statement, as just adding it to 'Wants=' assumes the multipathd
> > service should be running after the blk-availability service.
>
> As far as I can tell, The Before= command only says that if the other
> unit is started, this one will be started first. It doesn't take the
> place of Wants= or Requires=. So I think we should keep that Wants=
> line.
Do we really "Want" it? blk-availability.service itself includes the
line "WantedBy=sysinit.target". Otherwise it only specifies "After"
dependencies for various block device-related services (except for
multipathd.service, which is the reason why we need "Before" there).
The only actual purpose of "blk-availability.service" is to run
"blkdeactivate" on shutdown. Why does that deserve a "Wants" dependency
on the part of multipathd? The primary purpose of the service is
related to LVM (https://www.redhat.com/archives/lvm-devel/2016-November
/msg00090.html) but none of the LVM services "Want" it AFAICS, so why
should multipath?
Regards
Martin
>
> -Ben
>
> >
> > Signed-off-by: Hannes Reinecke <hare@suse.com>
> > ---
> > multipathd/multipathd.service | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/multipathd/multipathd.service
> > b/multipathd/multipathd.service
> > index d26577f3..be13138c 100644
> > --- a/multipathd/multipathd.service
> > +++ b/multipathd/multipathd.service
> > @@ -1,10 +1,9 @@
> > [Unit]
> > Description=Device-Mapper Multipath Device Controller
> > -Before=iscsi.service iscsid.service lvm2-activation-early.service
> > -Before=local-fs-pre.target systemd-udev-trigger.service
> > +Before=iscsi.service iscsid.service lvm2-lvmetad.service lvm2-
> > activation-early.service
> > +Before=local-fs-pre.target systemd-udev-trigger.service blk-
> > availability.service
> > After=multipathd.socket systemd-udevd.service
> > DefaultDependencies=no
> > -Wants=local-fs-pre.target multipathd.socket blk-
> > availability.service
> > Conflicts=shutdown.target
> >
> > [Service]
> > --
> > 2.11.0
> >
> > --
> > dm-devel mailing list
> > dm-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/dm-devel
>
>
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 02/33] multipathd: start daemon after udev trigger
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
2017-02-28 16:22 ` [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements Martin Wilck
@ 2017-02-28 16:22 ` Martin Wilck
2017-02-28 16:22 ` [PATCH 03/33] Add support for "multipath=off" and "nompath" on kernel cmdline Martin Wilck
` (32 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:22 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
As multipath now relies on udev for device enumeration it needs
to be started after udev trigger has finished sending all events.
Otherwise the daemon will not find any devices during startup
(as udev trigger hasn't been called yet and the udev database is empty).
But after switchover from the initrd there will already be some
multipath device-mapper tables, for which the daemon cannot find
any device. Consequently the daemon will be removing these tables,
only to recreate them later on once udev trigger has run.
This induces a short window during which the device mapper devices
won't be present, causing systemd to umount devices or drop into
emergency mode.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
multipathd/multipathd.service | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index be13138c..126012b8 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -1,8 +1,9 @@
[Unit]
Description=Device-Mapper Multipath Device Controller
+Wants=systemd-udev-trigger.service systemd-udev-settle.service
Before=iscsi.service iscsid.service lvm2-lvmetad.service lvm2-activation-early.service
-Before=local-fs-pre.target systemd-udev-trigger.service blk-availability.service
-After=multipathd.socket systemd-udevd.service
+Before=local-fs-pre.target blk-availability.service
+After=multipathd.socket systemd-udev-trigger.service systemd-udev-settle.service
DefaultDependencies=no
Conflicts=shutdown.target
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 03/33] Add support for "multipath=off" and "nompath" on kernel cmdline
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
2017-02-28 16:22 ` [PATCH 01/33] multipathd.service: fixup Wants= and Before= statements Martin Wilck
2017-02-28 16:22 ` [PATCH 02/33] multipathd: start daemon after udev trigger Martin Wilck
@ 2017-02-28 16:22 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 04/33] multipath: do not check daemon from udev rules Martin Wilck
` (31 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:22 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
Add support for disabling multipathd startup from the kernel command line.
This is useful for debugging purposes.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/multipath.rules | 5 +++++
multipathd/multipathd.service | 2 ++
2 files changed, 7 insertions(+)
diff --git a/multipath/multipath.rules b/multipath/multipath.rules
index c8fb7e68..86defc0c 100644
--- a/multipath/multipath.rules
+++ b/multipath/multipath.rules
@@ -3,6 +3,11 @@ SUBSYSTEM!="block", GOTO="end_mpath"
ACTION!="add|change", GOTO="end_mpath"
KERNEL!="sd*|dasd*", GOTO="end_mpath"
+IMPORT{cmdline}="nompath"
+ENV{nompath}=="?*", GOTO="end_mpath"
+IMPORT{cmdline}="multipath"
+ENV{multipath}=="off", GOTO="end_mpath"
+
ENV{DEVTYPE}!="partition", GOTO="test_dev"
IMPORT{parent}="DM_MULTIPATH_DEVICE_PATH"
ENV{DM_MULTIPATH_DEVICE_PATH}=="1", ENV{ID_FS_TYPE}="none", \
diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index 126012b8..fd66cf62 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -6,6 +6,8 @@ Before=local-fs-pre.target blk-availability.service
After=multipathd.socket systemd-udev-trigger.service systemd-udev-settle.service
DefaultDependencies=no
Conflicts=shutdown.target
+ConditionKernelCommandLine=!nompath
+ConditionKernelCommandLine=!multipath=off
[Service]
Type=notify
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 04/33] multipath: do not check daemon from udev rules
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (2 preceding siblings ...)
2017-02-28 16:22 ` [PATCH 03/33] Add support for "multipath=off" and "nompath" on kernel cmdline Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-04-05 21:54 ` Benjamin Marzinski
2017-02-28 16:23 ` [PATCH 05/33] Invalid error code when using multipathd CLI Martin Wilck
` (30 subsequent siblings)
34 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
As stated previously, multipathd needs to start after udev trigger
has run as otherwise it won't be able to find any devices.
However, this also means that during udevadm trigger the daemon
wouldn't run, and consequently the check in the udev rules will
always be false, causing the device not to be marked as multipath
capable.
As it turns out, calling 'multipath' from udev rules has quite some
challenges. It _should_ check if a device is eligible for multipathing.
But it needs to work under all circumstances, even if the daemon isn't
running yet, as the program will be called from uevents which might
(and will) come in before the daemon is running.
To check if the daemon _should_ be run I'm checking the various
'.wants' directories from systemd, which carries links to the services
systemd will enable eventually. So if the multipathd.service is
listed in there it will be started, even if it isn't started yet.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/util.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
libmultipath/util.h | 1 +
multipath/main.c | 13 +++++++-----
3 files changed, 68 insertions(+), 5 deletions(-)
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 1841f359..be454cb1 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -6,13 +6,16 @@
#include <sys/stat.h>
#include <sys/sysmacros.h>
#include <sys/types.h>
+#include <dirent.h>
#include <unistd.h>
+#include <errno.h>
#include "debug.h"
#include "memory.h"
#include "checkers.h"
#include "vector.h"
#include "structs.h"
+#include "log.h"
size_t
strchop(char *str)
@@ -279,3 +282,59 @@ setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached)
assert(ret == 0);
}
}
+
+int systemd_service_enabled_in(const char *dev, const char *prefix)
+{
+ char path[PATH_SIZE], file[PATH_SIZE], service[PATH_SIZE];
+ DIR *dirfd;
+ struct dirent *d;
+ int found = 0;
+
+ snprintf(service, PATH_SIZE, "multipathd.service");
+ snprintf(path, PATH_SIZE, "%s/systemd/system", prefix);
+ condlog(3, "%s: checking for %s in %s", dev, service, path);
+
+ dirfd = opendir(path);
+ if (dirfd == NULL)
+ return 0;
+
+ while ((d = readdir(dirfd)) != NULL) {
+ char *p;
+ struct stat stbuf;
+
+ if ((strcmp(d->d_name,".") == 0) ||
+ (strcmp(d->d_name,"..") == 0))
+ continue;
+
+ if (strlen(d->d_name) < 6)
+ continue;
+
+ p = d->d_name + strlen(d->d_name) - 6;
+ if (strcmp(p, ".wants"))
+ continue;
+ snprintf(file, PATH_SIZE, "%s/%s/%s",
+ path, d->d_name, service);
+ if (stat(file, &stbuf) == 0) {
+ condlog(3, "%s: found %s", dev, file);
+ found++;
+ break;
+ }
+ }
+ closedir(dirfd);
+
+ return found;
+}
+
+int systemd_service_enabled(const char *dev)
+{
+ int found = 0;
+
+ found = systemd_service_enabled_in(dev, "/etc");
+ if (!found)
+ found = systemd_service_enabled_in(dev, "/usr/lib");
+ if (!found)
+ found = systemd_service_enabled_in(dev, "/lib");
+ if (!found)
+ found = systemd_service_enabled_in(dev, "/run");
+ return found;
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index f3b37ee9..4c1f85c3 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -13,6 +13,7 @@ int devt2devname (char *, int, char *);
dev_t parse_devt(const char *dev_t);
char *convert_dev(char *dev, int is_path_device);
void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
+int systemd_service_enabled(const char *dev);
#define safe_sprintf(var, format, args...) \
snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
diff --git a/multipath/main.c b/multipath/main.c
index 171c08b4..befe4c53 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -682,11 +682,14 @@ main (int argc, char *argv[])
fd = mpath_connect();
if (fd == -1) {
- printf("%s is not a valid multipath device path\n",
- dev);
- goto out;
- }
- mpath_disconnect(fd);
+ condlog(3, "%s: daemon is not running", dev);
+ if (!systemd_service_enabled(dev)) {
+ printf("%s is not a valid "
+ "multipath device path\n", dev);
+ goto out;
+ }
+ } else
+ mpath_disconnect(fd);
}
if (cmd == CMD_REMOVE_WWID && !dev) {
condlog(0, "the -w option requires a device");
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH 04/33] multipath: do not check daemon from udev rules
2017-02-28 16:23 ` [PATCH 04/33] multipath: do not check daemon from udev rules Martin Wilck
@ 2017-04-05 21:54 ` Benjamin Marzinski
2017-04-06 12:10 ` Martin Wilck
0 siblings, 1 reply; 57+ messages in thread
From: Benjamin Marzinski @ 2017-04-05 21:54 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
On Tue, Feb 28, 2017 at 05:23:00PM +0100, Martin Wilck wrote:
> From: Hannes Reinecke <hare@suse.de>
>
> As stated previously, multipathd needs to start after udev trigger
> has run as otherwise it won't be able to find any devices.
> However, this also means that during udevadm trigger the daemon
> wouldn't run, and consequently the check in the udev rules will
> always be false, causing the device not to be marked as multipath
> capable.
>
> As it turns out, calling 'multipath' from udev rules has quite some
> challenges. It _should_ check if a device is eligible for multipathing.
> But it needs to work under all circumstances, even if the daemon isn't
> running yet, as the program will be called from uevents which might
> (and will) come in before the daemon is running.
> To check if the daemon _should_ be run I'm checking the various
> '.wants' directories from systemd, which carries links to the services
> systemd will enable eventually. So if the multipathd.service is
> listed in there it will be started, even if it isn't started yet.
If we are doing this, so that udev will correctly claim multipath
devices before multipathd has started up, it seems like it should be
possible to relax some of the ordering restrictions for the startup. For
instance, I assume you added the requirement that multipathd needed to
start up before lvmetad to make sure it clamined the devices first. If
we don't need multipathd to be running to claim the devices, we can
start them up in any order.
-Ben
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
> libmultipath/util.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> libmultipath/util.h | 1 +
> multipath/main.c | 13 +++++++-----
> 3 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/libmultipath/util.c b/libmultipath/util.c
> index 1841f359..be454cb1 100644
> --- a/libmultipath/util.c
> +++ b/libmultipath/util.c
> @@ -6,13 +6,16 @@
> #include <sys/stat.h>
> #include <sys/sysmacros.h>
> #include <sys/types.h>
> +#include <dirent.h>
> #include <unistd.h>
> +#include <errno.h>
>
> #include "debug.h"
> #include "memory.h"
> #include "checkers.h"
> #include "vector.h"
> #include "structs.h"
> +#include "log.h"
>
> size_t
> strchop(char *str)
> @@ -279,3 +282,59 @@ setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached)
> assert(ret == 0);
> }
> }
> +
> +int systemd_service_enabled_in(const char *dev, const char *prefix)
> +{
> + char path[PATH_SIZE], file[PATH_SIZE], service[PATH_SIZE];
> + DIR *dirfd;
> + struct dirent *d;
> + int found = 0;
> +
> + snprintf(service, PATH_SIZE, "multipathd.service");
> + snprintf(path, PATH_SIZE, "%s/systemd/system", prefix);
> + condlog(3, "%s: checking for %s in %s", dev, service, path);
> +
> + dirfd = opendir(path);
> + if (dirfd == NULL)
> + return 0;
> +
> + while ((d = readdir(dirfd)) != NULL) {
> + char *p;
> + struct stat stbuf;
> +
> + if ((strcmp(d->d_name,".") == 0) ||
> + (strcmp(d->d_name,"..") == 0))
> + continue;
> +
> + if (strlen(d->d_name) < 6)
> + continue;
> +
> + p = d->d_name + strlen(d->d_name) - 6;
> + if (strcmp(p, ".wants"))
> + continue;
> + snprintf(file, PATH_SIZE, "%s/%s/%s",
> + path, d->d_name, service);
> + if (stat(file, &stbuf) == 0) {
> + condlog(3, "%s: found %s", dev, file);
> + found++;
> + break;
> + }
> + }
> + closedir(dirfd);
> +
> + return found;
> +}
> +
> +int systemd_service_enabled(const char *dev)
> +{
> + int found = 0;
> +
> + found = systemd_service_enabled_in(dev, "/etc");
> + if (!found)
> + found = systemd_service_enabled_in(dev, "/usr/lib");
> + if (!found)
> + found = systemd_service_enabled_in(dev, "/lib");
> + if (!found)
> + found = systemd_service_enabled_in(dev, "/run");
> + return found;
> +}
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index f3b37ee9..4c1f85c3 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> @@ -13,6 +13,7 @@ int devt2devname (char *, int, char *);
> dev_t parse_devt(const char *dev_t);
> char *convert_dev(char *dev, int is_path_device);
> void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
> +int systemd_service_enabled(const char *dev);
>
> #define safe_sprintf(var, format, args...) \
> snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
> diff --git a/multipath/main.c b/multipath/main.c
> index 171c08b4..befe4c53 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -682,11 +682,14 @@ main (int argc, char *argv[])
>
> fd = mpath_connect();
> if (fd == -1) {
> - printf("%s is not a valid multipath device path\n",
> - dev);
> - goto out;
> - }
> - mpath_disconnect(fd);
> + condlog(3, "%s: daemon is not running", dev);
> + if (!systemd_service_enabled(dev)) {
> + printf("%s is not a valid "
> + "multipath device path\n", dev);
> + goto out;
> + }
> + } else
> + mpath_disconnect(fd);
> }
> if (cmd == CMD_REMOVE_WWID && !dev) {
> condlog(0, "the -w option requires a device");
> --
> 2.11.0
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 05/33] Invalid error code when using multipathd CLI
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (3 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 04/33] multipath: do not check daemon from udev rules Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 06/33] multipathd: set timeout for CLI commands correctly Martin Wilck
` (29 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When calling the multipathd CLI we're getting the message
error -1 receiving packet
instead of the actual error number.
Problem is a confusion about the return values between
libmpathcmd and uxsock.c.
uxsock.c is assuming a negative return value to be the errno,
but libmpathcmd is returning -1 on error and setting errno.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmpathcmd/mpath_cmd.c | 4 ++++
libmultipath/uxsock.c | 9 +++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index 856e6b48..1496b682 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -141,7 +141,11 @@ int mpath_recv_reply(int fd, char **reply, unsigned int timeout)
*reply = NULL;
len = mpath_recv_reply_len(fd, timeout);
if (len <= 0)
+ return len;
+ if (len > MAX_REPLY_LEN) {
+ errno = EINVAL;
return -1;
+ }
*reply = malloc(len);
if (!*reply)
return -1;
diff --git a/libmultipath/uxsock.c b/libmultipath/uxsock.c
index 492f4b9c..7e5a1449 100644
--- a/libmultipath/uxsock.c
+++ b/libmultipath/uxsock.c
@@ -88,7 +88,9 @@ int ux_socket_listen(const char *name)
*/
int send_packet(int fd, const char *buf)
{
- return mpath_send_cmd(fd, buf);
+ if (mpath_send_cmd(fd, buf) < 0)
+ return -errno;
+ return 0;
}
static int _recv_packet(int fd, char **buf, unsigned int timeout, ssize_t limit)
@@ -98,8 +100,10 @@ static int _recv_packet(int fd, char **buf, unsigned int timeout, ssize_t limit)
*buf = NULL;
len = mpath_recv_reply_len(fd, timeout);
- if (len <= 0)
+ if (len == 0)
return len;
+ if (len < 0)
+ return -errno;
if ((limit > 0) && (len > limit))
return -EINVAL;
(*buf) = MALLOC(len);
@@ -109,6 +113,7 @@ static int _recv_packet(int fd, char **buf, unsigned int timeout, ssize_t limit)
if (err != 0) {
FREE(*buf);
(*buf) = NULL;
+ return -errno;
}
return err;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 06/33] multipathd: set timeout for CLI commands correctly
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (4 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 05/33] Invalid error code when using multipathd CLI Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-04-05 22:07 ` Benjamin Marzinski
2017-02-28 16:23 ` [PATCH 07/33] libmultipath: fall back to search paths by devt Martin Wilck
` (28 subsequent siblings)
34 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
The CLI command timeout wasn't set correctly for CLI commands,
causing it to timeout prematurely before the CLI response
could be received.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
multipathd/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index e317a4c9..86d73f7b 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1070,7 +1070,7 @@ uxsock_trigger (char * str, char ** reply, int * len, bool is_root,
return 1;
}
- r = parse_cmd(str, reply, len, vecs, uxsock_timeout / 1000);
+ r = parse_cmd(str, reply, len, vecs, uxsock_timeout);
if (r > 0) {
if (r == ETIMEDOUT)
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH 06/33] multipathd: set timeout for CLI commands correctly
2017-02-28 16:23 ` [PATCH 06/33] multipathd: set timeout for CLI commands correctly Martin Wilck
@ 2017-04-05 22:07 ` Benjamin Marzinski
2017-04-12 20:26 ` Martin Wilck
2017-04-13 13:11 ` [PATCH] Revert "multipathd: set timeout for CLI commands correctly" Martin Wilck
0 siblings, 2 replies; 57+ messages in thread
From: Benjamin Marzinski @ 2017-04-05 22:07 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
On Tue, Feb 28, 2017 at 05:23:02PM +0100, Martin Wilck wrote:
> From: Hannes Reinecke <hare@suse.de>
>
> The CLI command timeout wasn't set correctly for CLI commands,
> causing it to timeout prematurely before the CLI response
> could be received.
I'm pretty sure that this is wrong. uxsock_timeout is almost always
used in recv_packet, where it is supposed to specify a time in
milliseconds. In parse_cmd, it is being used to limit how long
multipathd will wait to get a lock when replying to a cli command before
giving up, and it's using the timeout in seconds (which is why the code
devided the timeout by 1000). With this patch it will now wait 15
minutes before giving up. After your "multipathd: set timeout for CLI
commands correctly" patch later in this series is applied, multipathd
will wait over an hour for a reply from multipathd before timing out. I
agree that you will need to wait longer for multipathd to grab the vecs
lock than you do when you are simply waiting for all the data in your
socket to show up, so the regular uxsock_timeout value is too small, but
an hour and seven minutes can't be what you intended.
-Ben
> Signed-off-by: Hannes Reinecke <hare@suse.com>
> ---
> multipathd/main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index e317a4c9..86d73f7b 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1070,7 +1070,7 @@ uxsock_trigger (char * str, char ** reply, int * len, bool is_root,
> return 1;
> }
>
> - r = parse_cmd(str, reply, len, vecs, uxsock_timeout / 1000);
> + r = parse_cmd(str, reply, len, vecs, uxsock_timeout);
>
> if (r > 0) {
> if (r == ETIMEDOUT)
> --
> 2.11.0
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH 06/33] multipathd: set timeout for CLI commands correctly
2017-04-05 22:07 ` Benjamin Marzinski
@ 2017-04-12 20:26 ` Martin Wilck
2017-04-13 13:11 ` [PATCH] Revert "multipathd: set timeout for CLI commands correctly" Martin Wilck
1 sibling, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-04-12 20:26 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel
On Wed, 2017-04-05 at 17:07 -0500, Benjamin Marzinski wrote:
> On Tue, Feb 28, 2017 at 05:23:02PM +0100, Martin Wilck wrote:
> > From: Hannes Reinecke <hare@suse.de>
> >
> > The CLI command timeout wasn't set correctly for CLI commands,
> > causing it to timeout prematurely before the CLI response
> > could be received.
>
> I'm pretty sure that this is wrong.
You are right. I will double-check with Hannes.
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH] Revert "multipathd: set timeout for CLI commands correctly"
2017-04-05 22:07 ` Benjamin Marzinski
2017-04-12 20:26 ` Martin Wilck
@ 2017-04-13 13:11 ` Martin Wilck
1 sibling, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-04-13 13:11 UTC (permalink / raw)
To: Benjamin Marzinski, Christophe Varoqui, Hannes Reinecke
Cc: Martin Wilck, dm-devel
This reverts commit a002d124723cef3f2bb4fc33899d2613bdfe3924.
uxsock_timeout is in ms (argument to poll()), whereas
parse_cmd() uses the value to add to tv_sec in a struct timespec.
So the previous code was correct.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipathd/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 0c61caad..b167cb4c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1087,7 +1087,7 @@ uxsock_trigger (char * str, char ** reply, int * len, bool is_root,
return 1;
}
- r = parse_cmd(str, reply, len, vecs, uxsock_timeout);
+ r = parse_cmd(str, reply, len, vecs, uxsock_timeout / 1000);
if (r > 0) {
if (r == ETIMEDOUT)
--
2.12.2
^ permalink raw reply related [flat|nested] 57+ messages in thread
* [PATCH 07/33] libmultipath: fall back to search paths by devt
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (5 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 06/33] multipathd: set timeout for CLI commands correctly Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 08/33] libmultipath: Do not crash on empty features Martin Wilck
` (27 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When removing path the device might already be gone from sysfs,
so we cannot lookup the device name. However, we still should
have the path vector available, so we should be trying to look
it up by using the device number.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/discovery.c | 11 +++++++++--
libmultipath/dmparser.c | 11 +++++++----
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 4e99845e..e4186de1 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -120,8 +120,15 @@ path_discover (vector pathvec, struct config * conf,
pp = find_path_by_dev(pathvec, (char *)devname);
if (!pp) {
- return store_pathinfo(pathvec, conf,
- udevice, flag, NULL);
+ char devt[BLK_DEV_SIZE];
+ dev_t devnum = udev_device_get_devnum(udevice);
+
+ snprintf(devt, BLK_DEV_SIZE, "%d:%d",
+ major(devnum), minor(devnum));
+ pp = find_path_by_devt(pathvec, devt);
+ if (!pp)
+ return store_pathinfo(pathvec, conf,
+ udevice, flag, NULL);
}
return pathinfo(pp, conf, flag);
}
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index b504961f..274eb947 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -330,12 +330,15 @@ int disassemble_map(vector pathvec, char *params, struct multipath *mpp,
if (devt2devname(devname, FILE_NAME_SIZE, word)) {
condlog(2, "%s: cannot find block device",
word);
- FREE(word);
- continue;
+ devname[0] = '\0';
}
- if (pathvec)
- pp = find_path_by_dev(pathvec, devname);
+ if (pathvec) {
+ if (strlen(devname))
+ pp = find_path_by_dev(pathvec, devname);
+ else
+ pp = find_path_by_devt(pathvec, word);
+ }
if (!pp) {
pp = alloc_path();
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 08/33] libmultipath: Do not crash on empty features
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (6 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 07/33] libmultipath: fall back to search paths by devt Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 09/33] multipathd: Set CLI timeout correctly Martin Wilck
` (26 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When adding to an otherwise empty feature list multipath would
crash as it doesn't check for an empty feature list.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/structs.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index f36a0552..4419510d 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -506,7 +506,7 @@ void setup_feature(struct multipath *mpp, char *feature)
int add_feature(char **f, char *n)
{
- int c = 0, d, l;
+ int c = 0, d, l = 0;
char *e, *p, *t;
if (!f)
@@ -528,18 +528,19 @@ int add_feature(char **f, char *n)
}
/* Check if feature is already present */
- if (strstr(*f, n))
- return 0;
-
- /* Get feature count */
- c = strtoul(*f, &e, 10);
- if (*f == e)
- /* parse error */
- return 1;
-
- /* Check if we need to increase feature count space */
- l = strlen(*f) + strlen(n) + 1;
+ if (*f) {
+ if (strstr(*f, n))
+ return 0;
+
+ /* Get feature count */
+ c = strtoul(*f, &e, 10);
+ if (*f == e)
+ /* parse error */
+ return 1;
+ /* Check if we need to increase feature count space */
+ l = strlen(*f) + strlen(n) + 1;
+ }
/* Count new features */
if ((c % 10) == 9)
l++;
@@ -571,7 +572,10 @@ int add_feature(char **f, char *n)
snprintf(p, l + 2, "%0d ", c);
/* Copy the feature string */
- p = strchr(*f, ' ');
+ p = NULL;
+ if (*f)
+ p = strchr(*f, ' ');
+
if (p) {
while (*p == ' ')
p++;
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 09/33] multipathd: Set CLI timeout correctly
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (7 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 08/33] libmultipath: Do not crash on empty features Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 10/33] multipath: avoid crash when using modified configuration Martin Wilck
` (25 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When calling 'multipathd cmd' the CLI timeout isn't set correctly;
calling "multipathd -k'cmd'" uses the correct timeout.
And the default timeout should be increased to 4 seconds to ensure
multipath runs correctly on large installations.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmpathcmd/mpath_cmd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libmpathcmd/mpath_cmd.h b/libmpathcmd/mpath_cmd.h
index 7293d919..6534474c 100644
--- a/libmpathcmd/mpath_cmd.h
+++ b/libmpathcmd/mpath_cmd.h
@@ -25,7 +25,7 @@ extern "C" {
#endif
#define DEFAULT_SOCKET "/org/kernel/linux/storage/multipathd"
-#define DEFAULT_REPLY_TIMEOUT 1000
+#define DEFAULT_REPLY_TIMEOUT 4000
#define MAX_REPLY_LEN 65536
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 10/33] multipath: avoid crash when using modified configuration
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (8 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 09/33] multipathd: Set CLI timeout correctly Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 11/33] multipathd: issue systemd READY after initial configuration Martin Wilck
` (24 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
Occasionally multipath would crash when using a modified configuration.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/discovery.c | 2 +-
multipath/main.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index e4186de1..bd8ab557 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1801,7 +1801,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
{
int path_state;
- if (!pp)
+ if (!pp || !conf)
return PATHINFO_FAILED;
/*
diff --git a/multipath/main.c b/multipath/main.c
index befe4c53..5811630c 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -266,7 +266,8 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector pathvec, char * refwwid)
* 1: Failure
*/
static int
-configure (enum mpath_cmds cmd, enum devtypes dev_type, char *devpath)
+configure (struct config *conf, enum mpath_cmds cmd,
+ enum devtypes dev_type, char *devpath)
{
vector curmp = NULL;
vector pathvec = NULL;
@@ -275,7 +276,6 @@ configure (enum mpath_cmds cmd, enum devtypes dev_type, char *devpath)
int di_flag = 0;
char * refwwid = NULL;
char * dev = NULL;
- struct config *conf;
/*
* allocate core vectors to store paths and multipaths
@@ -295,7 +295,6 @@ configure (enum mpath_cmds cmd, enum devtypes dev_type, char *devpath)
/*
* if we have a blacklisted device parameter, exit early
*/
- conf = get_multipath_config();
if (dev && (dev_type == DEV_DEVNODE ||
dev_type == DEV_UEVENT) &&
cmd != CMD_REMOVE_WWID &&
@@ -304,10 +303,9 @@ configure (enum mpath_cmds cmd, enum devtypes dev_type, char *devpath)
if (cmd == CMD_VALID_PATH)
printf("%s is not a valid multipath device path\n",
devpath);
- put_multipath_config(conf);
goto out;
}
- put_multipath_config(conf);
+
/*
* scope limiting must be translated into a wwid
* failing the translation is fatal (by policy)
@@ -730,7 +728,7 @@ main (int argc, char *argv[])
r = dm_flush_maps(retries);
goto out;
}
- while ((r = configure(cmd, dev_type, dev)) < 0)
+ while ((r = configure(conf, cmd, dev_type, dev)) < 0)
condlog(3, "restart multipath configuration process");
out:
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 11/33] multipathd: issue systemd READY after initial configuration
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (9 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 10/33] multipath: avoid crash when using modified configuration Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 12/33] libmultipath/discovery: do not cache 'access_state' sysfs attribute Martin Wilck
` (23 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
We should be issueing systemd READY only after the initial
configuration has been completed, otherwise systemd might
continue while the multipath devices are not setup up properly.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
multipathd/main.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 86d73f7b..cf44778b 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2361,6 +2361,7 @@ child (void * param)
int i;
#ifdef USE_SYSTEMD
unsigned long checkint;
+ int startup_done = 0;
#endif
int rc;
int pid_fd = -1;
@@ -2509,10 +2510,6 @@ child (void * param)
}
pthread_attr_destroy(&misc_attr);
-#ifdef USE_SYSTEMD
- sd_notify(0, "READY=1");
-#endif
-
while (running_state != DAEMON_SHUTDOWN) {
pthread_cleanup_push(config_cleanup, NULL);
pthread_mutex_lock(&config_lock);
@@ -2534,6 +2531,12 @@ child (void * param)
}
lock_cleanup_pop(vecs->lock);
post_config_state(DAEMON_IDLE);
+#ifdef USE_SYSTEMD
+ if (!startup_done) {
+ sd_notify(0, "READY=1");
+ startup_done = 1;
+ }
+#endif
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 12/33] libmultipath/discovery: do not cache 'access_state' sysfs attribute
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (10 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 11/33] multipathd: issue systemd READY after initial configuration Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 13/33] libmultipath: use existing alias from bindings file Martin Wilck
` (22 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When reading the 'access_state' sysfs attribute we should not be
using libudev as this will cache the value, causing us to lose
any update to the attribute.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/discovery.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index bd8ab557..4d4fc895 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -210,8 +210,6 @@ declare_sysfs_get_str(devtype);
declare_sysfs_get_str(vendor);
declare_sysfs_get_str(model);
declare_sysfs_get_str(rev);
-declare_sysfs_get_str(access_state);
-declare_sysfs_get_str(preferred_path);
int
sysfs_set_max_sectors_kb(struct multipath *mpp)
@@ -527,10 +525,10 @@ sysfs_get_asymmetric_access_state(struct path *pp, char *buff, int buflen)
if (!parent)
return -1;
- if (sysfs_get_access_state(parent, buff, buflen) <= 0)
+ if (sysfs_attr_get_value(parent, "access_state", buff, buflen) <= 0)
return -1;
- if (sysfs_get_preferred_path(parent, value, 16) <= 0)
+ if (sysfs_attr_get_value(parent, "preferred_path", value, 16) <= 0)
return 0;
preferred = strtoul(value, &eptr, 0);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 13/33] libmultipath: use existing alias from bindings file
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (11 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 12/33] libmultipath/discovery: do not cache 'access_state' sysfs attribute Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 14/33] multipath -ll: set DI_SERIAL Martin Wilck
` (21 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
If the bindings file has been edited by hand not all entries conform
to the user_friendly_prefix setting. So if we don't find a matching
alias we need to check if the wwid is set to a different alias; if
so we need to use that.
Otherwise we'll end up with duplicate entries in the bindings file.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/alias.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index 12afef8f..fd6b7f91 100644
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
@@ -107,6 +107,7 @@ lookup_binding(FILE *f, char *map_wwid, char **map_alias, char *prefix)
*map_alias = NULL;
+ rewind(f);
while (fgets(buf, LINE_MAX, f)) {
char *c, *alias, *wwid;
int curr_id;
@@ -280,6 +281,13 @@ use_existing_alias (char *wwid, char *file, char *alias_old,
goto out;
}
+ id = lookup_binding(f, wwid, &alias, NULL);
+ if (alias) {
+ condlog(3, "Use existing binding [%s] for WWID [%s]",
+ alias, wwid);
+ goto out;
+ }
+
/* allocate the existing alias in the bindings file */
id = scan_devname(alias_old, prefix);
if (id <= 0)
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 14/33] multipath -ll: set DI_SERIAL
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (12 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 13/33] libmultipath: use existing alias from bindings file Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map Martin Wilck
` (20 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
Without DI_SERIAL, multipath -ll will not be able to obtain correct
PG priorities in certain configurations (e.g. prio "weightedpath",
prio_args "serial..."). We do not set DI_SERIAL for the multipath -l
case, where information is supposed to come from sysfs+device mapper
only.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipath/main.c b/multipath/main.c
index 5811630c..8ce87a08 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -367,7 +367,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
if (cmd == CMD_LIST_LONG)
/* extended path info '-ll' */
- di_flag |= DI_SYSFS | DI_CHECKER;
+ di_flag |= DI_SYSFS | DI_CHECKER | DI_SERIAL;
else if (cmd == CMD_LIST_SHORT)
/* minimum path info '-l' */
di_flag |= DI_SYSFS;
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (13 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 14/33] multipath -ll: set DI_SERIAL Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-04-05 22:44 ` Benjamin Marzinski
2017-02-28 16:23 ` [PATCH 16/33] multipath: ignore -i if find_multipaths is set Martin Wilck
` (19 subsequent siblings)
34 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
The function dm_suspend_and_flush() introduced in 9a4ff93
tries to remove child maps (partitions) after suspending
the mpath device. This may lock up if removing the partitions
requires I/O. It's better to use the following sequence
of actions: 1) clear queue_if_no_path; 2) remove partitions;
3) suspend; 4) remove (or resume and restore queue_if_no_path
in case of failure).
This patch modifies the implementation by moving the
queue_if_no_path/suspend logic into _dm_flush_map().
A call to _dm_flush_map() with need_suspend=1 replaces
the previous call to dm_suspend_and_flush().
With this change, the mpath device is only suspended after
removing partmaps, avoiding the deadlock.
Fixes: 9a4ff93 "Switch off 'queue_if_no_path' before removing maps"
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/devmapper.c | 102 ++++++++++++++++++++---------------------------
libmultipath/devmapper.h | 9 +++--
2 files changed, 49 insertions(+), 62 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 1576dd01..044be2be 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -760,12 +760,6 @@ out:
}
static int
-has_partmap(const char *name, void *data)
-{
- return 1;
-}
-
-static int
partmap_in_use(const char *name, void *data)
{
int part_count, *ret_count = (int *)data;
@@ -785,9 +779,13 @@ partmap_in_use(const char *name, void *data)
return 0;
}
-int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
+int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
+ int need_suspend, int retries)
{
int r;
+ int queue_if_no_path = 0;
+ unsigned long long mapsize;
+ char params[PARAMS_SIZE] = {0};
if (!dm_is_mpath(mapname))
return 0; /* nothing to do */
@@ -797,6 +795,16 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
return 1;
+ if (need_suspend &&
+ !dm_get_map(mapname, &mapsize, params) &&
+ strstr(params, "queue_if_no_path")) {
+ if (!dm_queue_if_no_path((char *)mapname, 0))
+ queue_if_no_path = 1;
+ else
+ /* Leave queue_if_no_path alone if unset failed */
+ queue_if_no_path = -1;
+ }
+
if (dm_remove_partmaps(mapname, need_sync, deferred_remove))
return 1;
@@ -805,17 +813,36 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
return 1;
}
- r = dm_device_remove(mapname, need_sync, deferred_remove);
+ do {
+ if (need_suspend && queue_if_no_path != -1)
+ dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
- if (r) {
- if (do_deferred(deferred_remove) && dm_map_present(mapname)) {
- condlog(4, "multipath map %s remove deferred",
+ r = dm_device_remove(mapname, need_sync, deferred_remove);
+
+ if (r) {
+ if (do_deferred(deferred_remove)
+ && dm_map_present(mapname)) {
+ condlog(4, "multipath map %s remove deferred",
+ mapname);
+ return 2;
+ }
+ condlog(4, "multipath map %s removed", mapname);
+ return 0;
+ } else {
+ condlog(2, "failed to remove multipath map %s",
mapname);
- return 2;
+ if (need_suspend && queue_if_no_path != -1) {
+ dm_simplecmd_noflush(DM_DEVICE_RESUME,
+ mapname, 0);
+ }
}
- condlog(4, "multipath map %s removed", mapname);
- return 0;
- }
+ if (retries)
+ sleep(1);
+ } while (retries-- > 0);
+
+ if (queue_if_no_path == 1)
+ dm_queue_if_no_path((char *)mapname, 1);
+
return 1;
}
@@ -824,7 +851,7 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
int
dm_flush_map_nopaths(const char * mapname, int deferred_remove)
{
- return _dm_flush_map(mapname, 1, deferred_remove);
+ return _dm_flush_map(mapname, 1, deferred_remove, 0, 0);
}
#else
@@ -832,52 +859,11 @@ dm_flush_map_nopaths(const char * mapname, int deferred_remove)
int
dm_flush_map_nopaths(const char * mapname, int deferred_remove)
{
- return _dm_flush_map(mapname, 1, 0);
+ return _dm_flush_map(mapname, 1, 0, 0, 0);
}
#endif
-int dm_suspend_and_flush_map (const char * mapname, int retries)
-{
- int need_reset = 0, queue_if_no_path = 0;
- unsigned long long mapsize;
- char params[PARAMS_SIZE] = {0};
- int udev_flags = 0;
-
- if (!dm_is_mpath(mapname))
- return 0; /* nothing to do */
-
- /* if the device currently has no partitions, do not
- run kpartx on it if you fail to delete it */
- if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
- udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
-
- if (!dm_get_map(mapname, &mapsize, params)) {
- if (strstr(params, "queue_if_no_path"))
- queue_if_no_path = 1;
- }
-
- if (queue_if_no_path && dm_queue_if_no_path((char *)mapname, 0) == 0)
- need_reset = 1;
-
- do {
- if (!queue_if_no_path || need_reset)
- dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
-
- if (!dm_flush_map(mapname)) {
- condlog(4, "multipath map %s removed", mapname);
- return 0;
- }
- dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags);
- if (retries)
- sleep(1);
- } while (retries-- > 0);
- condlog(2, "failed to remove multipath map %s", mapname);
- if (need_reset)
- dm_queue_if_no_path((char *)mapname, 1);
- return 1;
-}
-
int dm_flush_maps (int retries)
{
int r = 0;
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 3ea43297..aca4454b 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -36,12 +36,13 @@ int dm_get_map(const char *, unsigned long long *, char *);
int dm_get_status(char *, char *);
int dm_type(const char *, char *);
int dm_is_mpath(const char *);
-int _dm_flush_map (const char *, int, int);
+int _dm_flush_map (const char *, int, int, int, int);
int dm_flush_map_nopaths(const char * mapname, int deferred_remove);
-#define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0)
-#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0)
+#define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0, 0, 0)
+#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0, 0, 0)
+#define dm_suspend_and_flush_map(mapname, retries) \
+ _dm_flush_map(mapname, 1, 0, 1, retries)
int dm_cancel_deferred_remove(struct multipath *mpp);
-int dm_suspend_and_flush_map(const char * mapname, int retries);
int dm_flush_maps (int retries);
int dm_fail_path(char * mapname, char * path);
int dm_reinstate_path(char * mapname, char * path);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map
2017-02-28 16:23 ` [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map Martin Wilck
@ 2017-04-05 22:44 ` Benjamin Marzinski
2017-04-12 20:54 ` Martin Wilck
2017-04-13 13:05 ` [PATCH] libmultipath: fix skip_kpartx support for removing maps Martin Wilck
0 siblings, 2 replies; 57+ messages in thread
From: Benjamin Marzinski @ 2017-04-05 22:44 UTC (permalink / raw)
To: Martin Wilck; +Cc: Martin Wilck, dm-devel
On Tue, Feb 28, 2017 at 05:23:11PM +0100, Martin Wilck wrote:
> From: Martin Wilck <mwilck@suse.de>
>
> The function dm_suspend_and_flush() introduced in 9a4ff93
> tries to remove child maps (partitions) after suspending
> the mpath device. This may lock up if removing the partitions
> requires I/O. It's better to use the following sequence
> of actions: 1) clear queue_if_no_path; 2) remove partitions;
> 3) suspend; 4) remove (or resume and restore queue_if_no_path
> in case of failure).
>
> This patch modifies the implementation by moving the
> queue_if_no_path/suspend logic into _dm_flush_map().
> A call to _dm_flush_map() with need_suspend=1 replaces
> the previous call to dm_suspend_and_flush().
>
> With this change, the mpath device is only suspended after
> removing partmaps, avoiding the deadlock.
This patch drops support for disabling partitions, by removing
- /* if the device currently has no partitions, do not
- run kpartx on it if you fail to delete it */
- if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
- udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
> Fixes: 9a4ff93 "Switch off 'queue_if_no_path' before removing maps"
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> libmultipath/devmapper.c | 102 ++++++++++++++++++++---------------------------
> libmultipath/devmapper.h | 9 +++--
> 2 files changed, 49 insertions(+), 62 deletions(-)
>
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 1576dd01..044be2be 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -760,12 +760,6 @@ out:
> }
>
> static int
> -has_partmap(const char *name, void *data)
> -{
> - return 1;
> -}
> -
> -static int
> partmap_in_use(const char *name, void *data)
> {
> int part_count, *ret_count = (int *)data;
> @@ -785,9 +779,13 @@ partmap_in_use(const char *name, void *data)
> return 0;
> }
>
> -int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
> +int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
> + int need_suspend, int retries)
> {
> int r;
> + int queue_if_no_path = 0;
> + unsigned long long mapsize;
> + char params[PARAMS_SIZE] = {0};
>
> if (!dm_is_mpath(mapname))
> return 0; /* nothing to do */
> @@ -797,6 +795,16 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
> if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
> return 1;
>
> + if (need_suspend &&
> + !dm_get_map(mapname, &mapsize, params) &&
> + strstr(params, "queue_if_no_path")) {
> + if (!dm_queue_if_no_path((char *)mapname, 0))
> + queue_if_no_path = 1;
> + else
> + /* Leave queue_if_no_path alone if unset failed */
> + queue_if_no_path = -1;
> + }
> +
> if (dm_remove_partmaps(mapname, need_sync, deferred_remove))
> return 1;
>
> @@ -805,17 +813,36 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
> return 1;
> }
>
> - r = dm_device_remove(mapname, need_sync, deferred_remove);
> + do {
> + if (need_suspend && queue_if_no_path != -1)
> + dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
>
> - if (r) {
> - if (do_deferred(deferred_remove) && dm_map_present(mapname)) {
> - condlog(4, "multipath map %s remove deferred",
> + r = dm_device_remove(mapname, need_sync, deferred_remove);
> +
> + if (r) {
> + if (do_deferred(deferred_remove)
> + && dm_map_present(mapname)) {
> + condlog(4, "multipath map %s remove deferred",
> + mapname);
> + return 2;
> + }
> + condlog(4, "multipath map %s removed", mapname);
> + return 0;
> + } else {
> + condlog(2, "failed to remove multipath map %s",
> mapname);
> - return 2;
This resume command needs to disable adding back partitions if there
aren't any, otherwise it could create partitions on a device that wasn't
supposed to have any.
> + if (need_suspend && queue_if_no_path != -1) {
> + dm_simplecmd_noflush(DM_DEVICE_RESUME,
> + mapname, 0);
> + }
> }
> - condlog(4, "multipath map %s removed", mapname);
> - return 0;
> - }
> + if (retries)
> + sleep(1);
> + } while (retries-- > 0);
> +
> + if (queue_if_no_path == 1)
> + dm_queue_if_no_path((char *)mapname, 1);
> +
> return 1;
> }
>
> @@ -824,7 +851,7 @@ int _dm_flush_map(const char *mapname, int need_sync, int deferred_remove)
> int
> dm_flush_map_nopaths(const char * mapname, int deferred_remove)
> {
> - return _dm_flush_map(mapname, 1, deferred_remove);
> + return _dm_flush_map(mapname, 1, deferred_remove, 0, 0);
> }
>
> #else
> @@ -832,52 +859,11 @@ dm_flush_map_nopaths(const char * mapname, int deferred_remove)
> int
> dm_flush_map_nopaths(const char * mapname, int deferred_remove)
> {
> - return _dm_flush_map(mapname, 1, 0);
> + return _dm_flush_map(mapname, 1, 0, 0, 0);
> }
>
> #endif
>
> -int dm_suspend_and_flush_map (const char * mapname, int retries)
> -{
> - int need_reset = 0, queue_if_no_path = 0;
> - unsigned long long mapsize;
> - char params[PARAMS_SIZE] = {0};
> - int udev_flags = 0;
> -
> - if (!dm_is_mpath(mapname))
> - return 0; /* nothing to do */
> -
> - /* if the device currently has no partitions, do not
> - run kpartx on it if you fail to delete it */
> - if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
> - udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
> -
> - if (!dm_get_map(mapname, &mapsize, params)) {
> - if (strstr(params, "queue_if_no_path"))
> - queue_if_no_path = 1;
> - }
> -
> - if (queue_if_no_path && dm_queue_if_no_path((char *)mapname, 0) == 0)
> - need_reset = 1;
> -
> - do {
> - if (!queue_if_no_path || need_reset)
> - dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
> -
> - if (!dm_flush_map(mapname)) {
> - condlog(4, "multipath map %s removed", mapname);
> - return 0;
> - }
> - dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname, udev_flags);
> - if (retries)
> - sleep(1);
> - } while (retries-- > 0);
> - condlog(2, "failed to remove multipath map %s", mapname);
> - if (need_reset)
> - dm_queue_if_no_path((char *)mapname, 1);
> - return 1;
> -}
> -
> int dm_flush_maps (int retries)
> {
> int r = 0;
> diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
> index 3ea43297..aca4454b 100644
> --- a/libmultipath/devmapper.h
> +++ b/libmultipath/devmapper.h
> @@ -36,12 +36,13 @@ int dm_get_map(const char *, unsigned long long *, char *);
> int dm_get_status(char *, char *);
> int dm_type(const char *, char *);
> int dm_is_mpath(const char *);
> -int _dm_flush_map (const char *, int, int);
> +int _dm_flush_map (const char *, int, int, int, int);
> int dm_flush_map_nopaths(const char * mapname, int deferred_remove);
> -#define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0)
> -#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0)
> +#define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0, 0, 0)
> +#define dm_flush_map_nosync(mapname) _dm_flush_map(mapname, 0, 0, 0, 0)
> +#define dm_suspend_and_flush_map(mapname, retries) \
> + _dm_flush_map(mapname, 1, 0, 1, retries)
> int dm_cancel_deferred_remove(struct multipath *mpp);
> -int dm_suspend_and_flush_map(const char * mapname, int retries);
> int dm_flush_maps (int retries);
> int dm_fail_path(char * mapname, char * path);
> int dm_reinstate_path(char * mapname, char * path);
> --
> 2.11.0
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map
2017-04-05 22:44 ` Benjamin Marzinski
@ 2017-04-12 20:54 ` Martin Wilck
2017-04-13 13:05 ` [PATCH] libmultipath: fix skip_kpartx support for removing maps Martin Wilck
1 sibling, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-04-12 20:54 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel
On Wed, 2017-04-05 at 17:44 -0500, Benjamin Marzinski wrote:
> On Tue, Feb 28, 2017 at 05:23:11PM +0100, Martin Wilck wrote:
> > From: Martin Wilck <mwilck@suse.de>
> >
> > The function dm_suspend_and_flush() introduced in 9a4ff93
> > tries to remove child maps (partitions) after suspending
> > the mpath device. This may lock up if removing the partitions
> > requires I/O. It's better to use the following sequence
> > of actions: 1) clear queue_if_no_path; 2) remove partitions;
> > 3) suspend; 4) remove (or resume and restore queue_if_no_path
> > in case of failure).
> >
> > This patch modifies the implementation by moving the
> > queue_if_no_path/suspend logic into _dm_flush_map().
> > A call to _dm_flush_map() with need_suspend=1 replaces
> > the previous call to dm_suspend_and_flush().
> >
> > With this change, the mpath device is only suspended after
> > removing partmaps, avoiding the deadlock.
>
> This patch drops support for disabling partitions, by removing
> - /* if the device currently has no partitions, do not
> - run kpartx on it if you fail to delete it */
> - if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
> - udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
Sorry, this was an oversight when I forward-ported my patch from our
code base to upstream. I will submit a fix.
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH] libmultipath: fix skip_kpartx support for removing maps
2017-04-05 22:44 ` Benjamin Marzinski
2017-04-12 20:54 ` Martin Wilck
@ 2017-04-13 13:05 ` Martin Wilck
2017-04-14 8:42 ` Christophe Varoqui
1 sibling, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-04-13 13:05 UTC (permalink / raw)
To: Benjamin Marzinski, Christophe Varoqui; +Cc: Martin Wilck, dm-devel
Commit 79a05a4e inadvertently removed the code necessary
to support multipath maps without partitions, as introduced
in b73a34ea "libmultipath: add skip_kpartx option". Revert
this part of the commit, so that skip_kpartx works again.
Fixes: 79a05a4e libmultipath: move suspend logic to _dm_flush_map
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/devmapper.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 026418f8..5fb9d9ac 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -761,6 +761,12 @@ out:
}
static int
+has_partmap(const char *name, void *data)
+{
+ return 1;
+}
+
+static int
partmap_in_use(const char *name, void *data)
{
int part_count, *ret_count = (int *)data;
@@ -785,12 +791,18 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
{
int r;
int queue_if_no_path = 0;
+ int udev_flags = 0;
unsigned long long mapsize;
char params[PARAMS_SIZE] = {0};
if (!dm_is_mpath(mapname))
return 0; /* nothing to do */
+ /* if the device currently has no partitions, do not
+ run kpartx on it if you fail to delete it */
+ if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
+ udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
+
/* If you aren't doing a deferred remove, make sure that no
* devices are in use */
if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
@@ -834,7 +846,7 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
mapname);
if (need_suspend && queue_if_no_path != -1) {
dm_simplecmd_noflush(DM_DEVICE_RESUME,
- mapname, 0);
+ mapname, udev_flags);
}
}
if (retries)
--
2.12.2
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH] libmultipath: fix skip_kpartx support for removing maps
2017-04-13 13:05 ` [PATCH] libmultipath: fix skip_kpartx support for removing maps Martin Wilck
@ 2017-04-14 8:42 ` Christophe Varoqui
0 siblings, 0 replies; 57+ messages in thread
From: Christophe Varoqui @ 2017-04-14 8:42 UTC (permalink / raw)
To: Martin Wilck; +Cc: Martin Wilck, device-mapper development
[-- Attachment #1.1: Type: text/plain, Size: 2297 bytes --]
Merged,
Thanks.
On Thu, Apr 13, 2017 at 3:05 PM, Martin Wilck <mwilck@suse.com> wrote:
> Commit 79a05a4e inadvertently removed the code necessary
> to support multipath maps without partitions, as introduced
> in b73a34ea "libmultipath: add skip_kpartx option". Revert
> this part of the commit, so that skip_kpartx works again.
>
> Fixes: 79a05a4e libmultipath: move suspend logic to _dm_flush_map
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> libmultipath/devmapper.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index 026418f8..5fb9d9ac 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -761,6 +761,12 @@ out:
> }
>
> static int
> +has_partmap(const char *name, void *data)
> +{
> + return 1;
> +}
> +
> +static int
> partmap_in_use(const char *name, void *data)
> {
> int part_count, *ret_count = (int *)data;
> @@ -785,12 +791,18 @@ int _dm_flush_map (const char * mapname, int
> need_sync, int deferred_remove,
> {
> int r;
> int queue_if_no_path = 0;
> + int udev_flags = 0;
> unsigned long long mapsize;
> char params[PARAMS_SIZE] = {0};
>
> if (!dm_is_mpath(mapname))
> return 0; /* nothing to do */
>
> + /* if the device currently has no partitions, do not
> + run kpartx on it if you fail to delete it */
> + if (do_foreach_partmaps(mapname, has_partmap, NULL) == 0)
> + udev_flags |= MPATH_UDEV_NO_KPARTX_FLAG;
> +
> /* If you aren't doing a deferred remove, make sure that no
> * devices are in use */
> if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
> @@ -834,7 +846,7 @@ int _dm_flush_map (const char * mapname, int
> need_sync, int deferred_remove,
> mapname);
> if (need_suspend && queue_if_no_path != -1) {
> dm_simplecmd_noflush(DM_DEVICE_RESUME,
> - mapname, 0);
> + mapname, udev_flags);
> }
> }
> if (retries)
> --
> 2.12.2
>
>
[-- Attachment #1.2: Type: text/html, Size: 3114 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 16/33] multipath: ignore -i if find_multipaths is set
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (14 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 15/33] libmultipath: move suspend logic to _dm_flush_map Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 17/33] multipathd: imply -n " Martin Wilck
` (18 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
multipath's logic for detecing new multipath devices with find_multipaths
currently doesn't work reliably. If paths for a new device are added
sequentially, the first one will be classified as non-multpath (and passed on
to systemd for further processing) while subsequent ones will be seen as
multipath devices. If the paths are added simultaneously (such as at udev
trigger time after switching from the initrd to the rootfs, if device
detection is already finished), whether or not additional paths will be seen
when the udev rules for a given paths are processed is random.
A proper implementation of this device detection would require some sort of
information passing between multipathd and multipath, timeouts waiting for
additional paths to appear when a single one is detected, retriggering of
uevents if the status of a given paths changes, and more fine-grained
treatment of "orphaned" paths in multipathd. All of that is currently
non-existent.
Currently, our only option is to rely on the wwids file for device setup with
find_multipaths. In practice, that means that multipath maps will only be
set up for such devices that have been set up manually by the user before.
This is the behavior of multipath [-c|-u] without the "-i" option.
But "-i" is useful for the !find_multipaths case, where the expectation is
that all non-blacklisted devices are multipathed by default. Because we
can't change the multipath invocation in the udev rules file
depending on the find_multipaths setting, just ignore "-i" if find_multipaths
is set.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/main.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/multipath/main.c b/multipath/main.c
index 8ce87a08..ed57135a 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -619,6 +619,16 @@ main (int argc, char *argv[])
}
}
+ /*
+ * FIXME: new device detection with find_multipaths currently
+ * doesn't work reliably.
+ */
+ if (cmd == CMD_VALID_PATH &&
+ conf->find_multipaths && conf->ignore_wwids) {
+ condlog(2, "ignoring -i flag because find_multipath is set in multipath.conf");
+ conf->ignore_wwids = 0;
+ }
+
if (getuid() != 0) {
fprintf(stderr, "need to be root\n");
exit(1);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 17/33] multipathd: imply -n if find_multipaths is set
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (15 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 16/33] multipath: ignore -i if find_multipaths is set Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-04-05 23:03 ` Benjamin Marzinski
2017-02-28 16:23 ` [PATCH 18/33] multipathd: use weaker "force_reload" at startup Martin Wilck
` (17 subsequent siblings)
34 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
Automatic detection of new devices with find_multipaths
doesn't work correctly currently. Therefore, for now,
imply ignore_new_devs if find_multipaths is seen.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipathd/main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/multipathd/main.c b/multipathd/main.c
index cf44778b..8f464bfb 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2183,6 +2183,10 @@ reconfigure (struct vectors * vecs)
conf->verbosity = verbosity;
if (bindings_read_only)
conf->bindings_read_only = bindings_read_only;
+ if (conf->find_multipaths) {
+ condlog(2, "find_multipaths is set: -n is implied");
+ ignore_new_devs = 1;
+ }
if (ignore_new_devs)
conf->ignore_new_devs = ignore_new_devs;
uxsock_timeout = conf->uxsock_timeout;
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH 17/33] multipathd: imply -n if find_multipaths is set
2017-02-28 16:23 ` [PATCH 17/33] multipathd: imply -n " Martin Wilck
@ 2017-04-05 23:03 ` Benjamin Marzinski
2017-04-12 21:36 ` Martin Wilck
0 siblings, 1 reply; 57+ messages in thread
From: Benjamin Marzinski @ 2017-04-05 23:03 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
On Tue, Feb 28, 2017 at 05:23:13PM +0100, Martin Wilck wrote:
> Automatic detection of new devices with find_multipaths
> doesn't work correctly currently. Therefore, for now,
> imply ignore_new_devs if find_multipaths is seen.
I would rather not do this (at least outside of the initramfs), since it
keeps multipathd from automatically creating multipath devices as
expected when you enable find_multipaths. I admit that these path
devices won't be correctly claimed in udev when they appear for the
first time, but it's hard to believe that they are critical the very
first time they appear on the system. I have a patch that I can send
upstream that triggers a change uevent on path devices when they are
added to the wwids file. This means that these devices will be
correctly claimed by multipath as soon as it gets set up on top of them.
Martin, what do you thing about reverting this change and triggering a
uevent instead?
-Ben
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipathd/main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index cf44778b..8f464bfb 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2183,6 +2183,10 @@ reconfigure (struct vectors * vecs)
> conf->verbosity = verbosity;
> if (bindings_read_only)
> conf->bindings_read_only = bindings_read_only;
> + if (conf->find_multipaths) {
> + condlog(2, "find_multipaths is set: -n is implied");
> + ignore_new_devs = 1;
> + }
> if (ignore_new_devs)
> conf->ignore_new_devs = ignore_new_devs;
> uxsock_timeout = conf->uxsock_timeout;
> --
> 2.11.0
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH 17/33] multipathd: imply -n if find_multipaths is set
2017-04-05 23:03 ` Benjamin Marzinski
@ 2017-04-12 21:36 ` Martin Wilck
2017-04-13 21:54 ` Benjamin Marzinski
0 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-04-12 21:36 UTC (permalink / raw)
To: Benjamin Marzinski; +Cc: dm-devel
On Wed, 2017-04-05 at 18:03 -0500, Benjamin Marzinski wrote:
> On Tue, Feb 28, 2017 at 05:23:13PM +0100, Martin Wilck wrote:
> > Automatic detection of new devices with find_multipaths
> > doesn't work correctly currently. Therefore, for now,
> > imply ignore_new_devs if find_multipaths is seen.
>
> I would rather not do this (at least outside of the initramfs), since
> it
> keeps multipathd from automatically creating multipath devices as
> expected when you enable find_multipaths. I admit that these path
> devices won't be correctly claimed in udev when they appear for the
> first time, but it's hard to believe that they are critical the very
> first time they appear on the system. I have a patch that I can send
> upstream that triggers a change uevent on path devices when they are
> added to the wwids file. This means that these devices will be
> correctly claimed by multipath as soon as it gets set up on top of
> them.
>
> Martin, what do you thing about reverting this change and triggering
> a
> uevent instead?
I haven't been able to obtain stable behavior in my experiments, that's
why I made this patch. The problem is the correspondence between the
multipath invocation in 56-multipath.rules and multipathd. I explained
the problems I had in more detail in the commit message of patch 16 of
the series.
[Note: SUSE uses "-i" in multipath.rules, whereas Fedora/RH does not.
However with patch 16/33 I changed the code such that "-i" is ignored
when find_multipaths is used, so that the multipath call in 56-
multipath.rules actually does the same in both distros in the
find_multipaths case (only considers paths from the WWIDs file).]
For a path that's not in the WWIDs file yet, the udev rule will always
return FALSE (better than with -i, where the result would be
essentially random). That means that the path will be further processed
by udev and systemd, and file systems will be mounted, LVM PVs scanned,
etc.
When multipathd tries to grab this path later, it will most likely face
an EBUSY error. Triggering another uevent won't change much if the
devices are already in use. In the worst case, the multipath map will
be non-functional but the paths will get SYSTEMD_READY=0 set in the new
uevent, so that the device might become non-accessible by the system
either through dm (map not set up successfully) or directly
(SYSTEMD_READY=0).
Doing this right is a hard problem IMO. Hannes and I thought about some
new means of communication between multipath and multipathd that would
guarantee that udev rules and multipathd were in agreement about every
path, something fast and (almost) lock-less like shared memory. We have
no code yet, though. (Note: implementing this would require reverting
our patch "multipathd: start daemon after udev trigger").
One other idea I had was to have the udev rule treat every path as a
multipath device path in the first place, then wait for a certain
amount of time whether additional paths are detected. If yes, maps will
be set up by multipathd and all is good. If no, we'd need to re-trigger
uevents for all "single-path" devices after the timeout, and now we'd
distinguish between multipath and non-multipath as we are now doing it
with "multipath -i -u", thus the remaining single-path devices would be
classified as non-multipath, and eventually be processed by systemd.
Obvious drawback: single-path devices, more often than not the local
SAS or SATA disks, would be processed very late in the boot process.
This could be worked around by blacklisting, just like now.
I actually have an implementation of this already, but we decided
against it for SUSE. If you're interested, I can post it here as PoC.
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 17/33] multipathd: imply -n if find_multipaths is set
2017-04-12 21:36 ` Martin Wilck
@ 2017-04-13 21:54 ` Benjamin Marzinski
0 siblings, 0 replies; 57+ messages in thread
From: Benjamin Marzinski @ 2017-04-13 21:54 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
On Wed, Apr 12, 2017 at 11:36:26PM +0200, Martin Wilck wrote:
> On Wed, 2017-04-05 at 18:03 -0500, Benjamin Marzinski wrote:
> > On Tue, Feb 28, 2017 at 05:23:13PM +0100, Martin Wilck wrote:
> > > Automatic detection of new devices with find_multipaths
> > > doesn't work correctly currently. Therefore, for now,
> > > imply ignore_new_devs if find_multipaths is seen.
> >
> > I would rather not do this (at least outside of the initramfs), since
> > it
> > keeps multipathd from automatically creating multipath devices as
> > expected when you enable find_multipaths. I admit that these path
> > devices won't be correctly claimed in udev when they appear for the
> > first time, but it's hard to believe that they are critical the very
> > first time they appear on the system. I have a patch that I can send
> > upstream that triggers a change uevent on path devices when they are
> > added to the wwids file. This means that these devices will be
> > correctly claimed by multipath as soon as it gets set up on top of
> > them.
> >
> > Martin, what do you thing about reverting this change and triggering
> > a
> > uevent instead?
>
> I haven't been able to obtain stable behavior in my experiments, that's
> why I made this patch. The problem is the correspondence between the
> multipath invocation in 56-multipath.rules and multipathd. I explained
> the problems I had in more detail in the commit message of patch 16 of
> the series.
>
> [Note: SUSE uses "-i" in multipath.rules, whereas Fedora/RH does not.
> However with patch 16/33 I changed the code such that "-i" is ignored
> when find_multipaths is used, so that the multipath call in 56-
> multipath.rules actually does the same in both distros in the
> find_multipaths case (only considers paths from the WWIDs file).]
>
> For a path that's not in the WWIDs file yet, the udev rule will always
> return FALSE (better than with -i, where the result would be
> essentially random). That means that the path will be further processed
> by udev and systemd, and file systems will be mounted, LVM PVs scanned,
> etc.
>
> When multipathd tries to grab this path later, it will most likely face
> an EBUSY error. Triggering another uevent won't change much if the
> devices are already in use. In the worst case, the multipath map will
> be non-functional but the paths will get SYSTEMD_READY=0 set in the new
> uevent, so that the device might become non-accessible by the system
> either through dm (map not set up successfully) or directly
> (SYSTEMD_READY=0).
Yep, but this will only happen the first time a device is seen. It's not
a problem on installation, since you are just setting everything up and
not running off your disks anyway. After that, you are only going to
run into this issue when you first see new storage, which you can pretty
much guarantee isn't essential for the system to function (since your
system was already functioning without it). The only time it can prove
an issue is if you need to transfer an existing filesystem from one
device to another, since this new device will not be immediately
recognized. In redhat's version, you are allowed to add
mpath.wwid=<wwid> to the kernel command line, and when multipathd starts
up, it will automatically add these to the wwids file. I posted this
patch upstream, but Hannes NAK'ed it (IIRC because he didn't like how I
parsed the kernel commandline from /proc/cmdline). If people are
interested, I can repost it, and we can see if we can work out an
acceptable way to do this.
> Doing this right is a hard problem IMO. Hannes and I thought about some
> new means of communication between multipath and multipathd that would
> guarantee that udev rules and multipathd were in agreement about every
> path, something fast and (almost) lock-less like shared memory. We have
> no code yet, though. (Note: implementing this would require reverting
> our patch "multipathd: start daemon after udev trigger").
Sounds intriguing
> One other idea I had was to have the udev rule treat every path as a
> multipath device path in the first place, then wait for a certain
> amount of time whether additional paths are detected. If yes, maps will
> be set up by multipathd and all is good. If no, we'd need to re-trigger
> uevents for all "single-path" devices after the timeout, and now we'd
> distinguish between multipath and non-multipath as we are now doing it
> with "multipath -i -u", thus the remaining single-path devices would be
> classified as non-multipath, and eventually be processed by systemd.
> Obvious drawback: single-path devices, more often than not the local
> SAS or SATA disks, would be processed very late in the boot process.
> This could be worked around by blacklisting, just like now.
>
> I actually have an implementation of this already, but we decided
> against it for SUSE. If you're interested, I can post it here as PoC.
This was actually my first idea with dealing with this, but I gave up on
it also.
I have repeatedly toyed with the idea of changing how multipath
discovers new devices to a method very similar to your patch, but for
all devices. The idea is to scrap /etc/multipath/wwids and
/etc/multipath/bindings, and use a completely new file, say
/etc/multipath/devices. For every multipath device, it would store the
wwid, possibly the device name (regardless of whether or not it's a
user_friendly_name or a multipath's section alias or just the wwid) and
the vendor, product, and revision data. It would definitively say which
devices were multipathed. If a device is in that file, you multipath it.
If not, you don't. This would make it much easier for udev to determine
if a device should be multipathed. It would simply search for the WWID
in that file.
The flip side to this is that multipath would no longer automatically
discover devices. You would have to run
# multipath
to discover any new devices. Further, changing the blacklists in
/etc/multipath.conf wouldn't immediately blacklist existing devices. You
would have to run
# multipath (probably with some option)
to make it rescan the existing devices and remove the blacklisted ones
from /etc/multipath/devices. That's why you need to keep the
vendor/product information in that file, so you can blacklist devices
even if they aren't currently present on the system. This would make
multipath work much more like other virtual devices, which won't exist
until you specifically create them. The difference is that instead
having metadata on the device that says that it should be a built into a
virtual device, the metadata is in a seperate file.
The addition of the device name would also make is possible to associate
a path with a specific multipath device inside the udev database,
instead of simply knowing that it belongs to some multipath device. It
would also avoid all those issues where a device gets multipathed in the
initramfs, but doesn't have a user_friendly_names binding, but in the
regular filesystem it does have a binding, and so the name has to change
(although I believe that those have finally all been sorted in the
existing code).
Redhat support would never let me remove find_multipaths as an option.
Customers hated manually setting up their blacklisting and multipathd
clearly knows enough to find their multipath devices for them. But it
does make claiming devices in udev a pain. However, even the regular
claiming code takes a long time in a udev task that can timeout, causing
all sorts of havoc. This is basically the same idea as your patch for
dealing with find_multipaths, but for all devices. Otherwise multipath
will automatically discover devices if find_multipaths is unset, but
won't if it's set, which seems really confusing.
Obviously, a quick version of this idea would be to simply set
ignore_new_devs all the time. But if we're doing that, I'd really like
to simplify the code that needs to be run for udev to claim the device
and right now blacklisting a device doesn't remove it from
/etc/multipath wwids.
Any thoughts?
> Martin
>
> --
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 57+ messages in thread
* [PATCH 18/33] multipathd: use weaker "force_reload" at startup
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (16 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 17/33] multipathd: imply -n " Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 19/33] libmultipath: setup_features: log msg if queue_if_no_path is ignored Martin Wilck
` (16 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
"force_reload" has originally been created to support multipath -r
(admin triggered reload). multipathd also uses this parameter
in DAEMON_CONFIGURE state, too. But this is causes unnecessary
device-mapper ioctls when multipathd is first started, if existing
maps have been discovered (perhaps set up by previous instance of
multipathd, e.g. in the initrd).
This patch introduces a weaker form of the force_reload parameter
to coalesce_paths(). The parameter can now take three values:
* FORCE_RELOAD_NONE: existing maps aren't touched at all
* FORCE_RELOAD_YES: all maps are rebuilt from scratch and (re)loaded in DM
* FORCE_RELOAD_WEAK: existing maps are compared to the current conf and only
reloaded in DM if there's a difference. This is useful during startup.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/config.h | 6 ++++++
libmultipath/configure.c | 16 ++++++++++++----
multipath/main.c | 2 +-
multipathd/cli_handlers.c | 3 ++-
multipathd/main.c | 12 ++++++++++--
5 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 9a90745b..16f8b1cc 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -36,6 +36,12 @@ enum mpath_cmds {
CMD_ADD_WWID,
};
+enum force_reload_types {
+ FORCE_RELOAD_NONE,
+ FORCE_RELOAD_YES,
+ FORCE_RELOAD_WEAK,
+};
+
struct hwentry {
char * vendor;
char * product;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 5ad30074..f164801b 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -750,8 +750,15 @@ out:
return ret;
}
-int coalesce_paths(struct vectors *vecs, vector newmp, char *refwwid,
- int force_reload, enum mpath_cmds cmd)
+/*
+ * The force_reload parameter determines how coalesce_paths treats existing maps.
+ * FORCE_RELOAD_NONE: existing maps aren't touched at all
+ * FORCE_RELOAD_YES: all maps are rebuilt from scratch and (re)loaded in DM
+ * FORCE_RELOAD_WEAK: existing maps are compared to the current conf and only
+ * reloaded in DM if there's a difference. This is useful during startup.
+ */
+int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
+ int force_reload, enum mpath_cmds cmd)
{
int r = 1;
int k, i;
@@ -769,7 +776,7 @@ int coalesce_paths(struct vectors *vecs, vector newmp, char *refwwid,
if (refwwid && !strlen(refwwid))
refwwid = NULL;
- if (force_reload) {
+ if (force_reload != FORCE_RELOAD_NONE) {
vector_foreach_slot (pathvec, pp1, k) {
pp1->mpp = NULL;
}
@@ -858,7 +865,8 @@ int coalesce_paths(struct vectors *vecs, vector newmp, char *refwwid,
if (cmd == CMD_DRY_RUN)
mpp->action = ACT_DRY_RUN;
if (mpp->action == ACT_UNDEF)
- select_action(mpp, curmp, force_reload);
+ select_action(mpp, curmp,
+ force_reload == FORCE_RELOAD_YES ? 1 : 0);
r = domap(mpp, params, is_daemon);
diff --git a/multipath/main.c b/multipath/main.c
index ed57135a..4174d43d 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -578,7 +578,7 @@ main (int argc, char *argv[])
}
break;
case 'r':
- conf->force_reload = 1;
+ conf->force_reload = FORCE_RELOAD_YES;
break;
case 'i':
conf->ignore_wwids = 1;
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index b20b0546..c59e7fdc 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -778,7 +778,8 @@ cli_add_map (void * v, char ** reply, int * len, void * data)
rc = get_refwwid(CMD_NONE, param, DEV_DEVMAP,
vecs->pathvec, &refwwid);
if (refwwid) {
- if (coalesce_paths(vecs, NULL, refwwid, 0, 1))
+ if (coalesce_paths(vecs, NULL, refwwid,
+ FORCE_RELOAD_NONE, 1))
condlog(2, "%s: coalesce_paths failed",
param);
dm_lib_release();
diff --git a/multipathd/main.c b/multipathd/main.c
index 8f464bfb..e2e73749 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -535,7 +535,8 @@ ev_add_map (char * dev, char * alias, struct vectors * vecs)
r = get_refwwid(CMD_NONE, dev, DEV_DEVMAP, vecs->pathvec, &refwwid);
if (refwwid) {
- r = coalesce_paths(vecs, NULL, refwwid, 0, CMD_NONE);
+ r = coalesce_paths(vecs, NULL, refwwid, FORCE_RELOAD_NONE,
+ CMD_NONE);
dm_lib_release();
}
@@ -2039,6 +2040,7 @@ configure (struct vectors * vecs, int start_waiters)
vector mpvec;
int i, ret;
struct config *conf;
+ static int force_reload = FORCE_RELOAD_WEAK;
if (!vecs->pathvec && !(vecs->pathvec = vector_alloc())) {
condlog(0, "couldn't allocate path vec in configure");
@@ -2082,8 +2084,14 @@ configure (struct vectors * vecs, int start_waiters)
/*
* create new set of maps & push changed ones into dm
+ * In the first call, use FORCE_RELOAD_WEAK to avoid making
+ * superfluous ACT_RELOAD ioctls. Later calls are done
+ * with FORCE_RELOAD_YES.
*/
- if (coalesce_paths(vecs, mpvec, NULL, 1, CMD_NONE)) {
+ ret = coalesce_paths(vecs, mpvec, NULL, force_reload, CMD_NONE);
+ if (force_reload == FORCE_RELOAD_WEAK)
+ force_reload = FORCE_RELOAD_YES;
+ if (ret) {
condlog(0, "configure failed while coalescing paths");
return 1;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 19/33] libmultipath: setup_features: log msg if queue_if_no_path is ignored
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (17 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 18/33] multipathd: use weaker "force_reload" at startup Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 20/33] libmultipath: setup_feature: print log msg if no_path_retry cant be set Martin Wilck
` (15 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/propsel.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index bba8194c..0c499339 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -288,7 +288,9 @@ out:
condlog(1, "%s: config error, overriding 'no_path_retry' value",
mp->alias);
mp->no_path_retry = NO_PATH_RETRY_QUEUE;
- }
+ } else if (mp->no_path_retry != NO_PATH_RETRY_QUEUE)
+ condlog(1, "%s: config error, ignoring 'queue_if_no_path' because no_path_retry=%d",
+ mp->alias, mp->no_path_retry);
}
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 20/33] libmultipath: setup_feature: print log msg if no_path_retry cant be set
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (18 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 19/33] libmultipath: setup_features: log msg if queue_if_no_path is ignored Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 21/33] libmultipath: setup_feature: handle "retain_attached_hw_handler" Martin Wilck
` (14 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/structs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 4419510d..c2565239 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -501,6 +501,9 @@ void setup_feature(struct multipath *mpp, char *feature)
if (!strncmp(feature, "queue_if_no_path", 16)) {
if (mpp->no_path_retry <= NO_PATH_RETRY_UNDEF)
mpp->no_path_retry = NO_PATH_RETRY_QUEUE;
+ else
+ condlog(1, "%s: ignoring feature queue_if_no_path because no_path_retry = %d",
+ mpp->alias, mpp->no_path_retry);
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 21/33] libmultipath: setup_feature: handle "retain_attached_hw_handler"
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (19 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 20/33] libmultipath: setup_feature: print log msg if no_path_retry cant be set Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 22/33] libmultipath: disassemble_map: skip no_path_retry check Martin Wilck
` (13 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/structs.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index c2565239..e225f8b4 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -504,6 +504,12 @@ void setup_feature(struct multipath *mpp, char *feature)
else
condlog(1, "%s: ignoring feature queue_if_no_path because no_path_retry = %d",
mpp->alias, mpp->no_path_retry);
+ } else if (!strcmp(feature, "retain_attached_hw_handler")) {
+ if (mpp->retain_hwhandler != RETAIN_HWHANDLER_OFF)
+ mpp->retain_hwhandler = RETAIN_HWHANDLER_ON;
+ else
+ condlog(1, "%s: ignoring feature 'retain_attached_hw_handler'",
+ mpp->alias);
}
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 22/33] libmultipath: disassemble_map: skip no_path_retry check
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (20 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 21/33] libmultipath: setup_feature: handle "retain_attached_hw_handler" Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 23/33] libmultipath: disassemble_map: treat minio like assemble_map does Martin Wilck
` (12 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
mpp->no_path_retry is already checked in setup_feature() itself,
no need to do it here as well. This allows using setup_feature()
for other features except queue_if_no_path.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/dmparser.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 274eb947..4940e8cf 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -183,10 +183,7 @@ int disassemble_map(vector pathvec, char *params, struct multipath *mpp,
FREE(word);
return 1;
}
- if ((mpp->no_path_retry == NO_PATH_RETRY_UNDEF) ||
- (mpp->no_path_retry == NO_PATH_RETRY_FAIL) ||
- (mpp->no_path_retry == NO_PATH_RETRY_QUEUE))
- setup_feature(mpp, word);
+ setup_feature(mpp, word);
FREE(word);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 23/33] libmultipath: disassemble_map: treat minio like assemble_map does
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (21 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 22/33] libmultipath: disassemble_map: skip no_path_retry check Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 24/33] libmultipath: select_action: check special features separately Martin Wilck
` (11 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
Rather than using 0 for everything except round-robin, read back
the actual minio value from DM.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/dmparser.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 4940e8cf..469e60d2 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -392,19 +392,17 @@ int disassemble_map(vector pathvec, char *params, struct multipath *mpp,
for (k = 0; k < num_paths_args; k++)
if (k == 0) {
+ p += get_word(p, &word);
+ def_minio = atoi(word);
+ FREE(word);
+
if (!strncmp(mpp->selector,
"round-robin", 11)) {
- p += get_word(p, &word);
- def_minio = atoi(word);
if (mpp->rr_weight == RR_WEIGHT_PRIO
&& pp->priority > 0)
def_minio /= pp->priority;
- FREE(word);
- } else {
- p += get_word(p, NULL);
- def_minio = 0;
}
if (def_minio != mpp->minio)
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 24/33] libmultipath: select_action: check special features separately
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (22 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 23/33] libmultipath: disassemble_map: treat minio like assemble_map does Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 25/33] libmultipath: sysfs_attr_set_value: use const char* Martin Wilck
` (10 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
The features queue_if_no_path and retain_attached_hw_handler are
treated separately in libmultipath. Compare these features by looking
at the respective flags, and ignore them when comparing the "features"
string. assemble_map() does the ssame thing when constructing the
features string for device mapper.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/configure.c | 34 ++++++++++++++++++++++++++++++----
1 file changed, 30 insertions(+), 4 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index f164801b..fb5a5a92 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -395,6 +395,7 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
{
struct multipath * cmpp;
struct multipath * cmpp_by_name;
+ char * mpp_feat, * cmpp_feat;
cmpp = find_mp_by_wwid(curmp, mpp->wwid);
cmpp_by_name = find_mp_by_alias(curmp, mpp->alias);
@@ -455,11 +456,11 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
mpp->alias);
return;
}
- if (!mpp->no_path_retry &&
- (strlen(cmpp->features) != strlen(mpp->features) ||
- strcmp(cmpp->features, mpp->features))) {
+
+ if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF &&
+ mpp->no_path_retry != cmpp->no_path_retry) {
mpp->action = ACT_RELOAD;
- condlog(3, "%s: set ACT_RELOAD (features change)",
+ condlog(3, "%s: set ACT_RELOAD (no_path_retry change)",
mpp->alias);
return;
}
@@ -472,6 +473,31 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
mpp->alias);
return;
}
+
+ if (mpp->retain_hwhandler != RETAIN_HWHANDLER_UNDEF &&
+ mpp->retain_hwhandler != cmpp->retain_hwhandler) {
+ mpp->action = ACT_RELOAD;
+ condlog(3, "%s: set ACT_RELOAD (retain_hwhandler change)",
+ mpp->alias);
+ return;
+ }
+
+ cmpp_feat = STRDUP(cmpp->features);
+ mpp_feat = STRDUP(mpp->features);
+ if (cmpp_feat && mpp_feat) {
+ remove_feature(&mpp_feat, "queue_if_no_path");
+ remove_feature(&mpp_feat, "retain_attached_hw_handler");
+ remove_feature(&cmpp_feat, "queue_if_no_path");
+ remove_feature(&cmpp_feat, "retain_attached_hw_handler");
+ if (strncmp(mpp_feat, cmpp_feat, PARAMS_SIZE)) {
+ mpp->action = ACT_RELOAD;
+ condlog(3, "%s: set ACT_RELOAD (features change)",
+ mpp->alias);
+ }
+ }
+ FREE(cmpp_feat);
+ FREE(mpp_feat);
+
if (!cmpp->selector || strncmp(cmpp->selector, mpp->selector,
strlen(mpp->selector))) {
mpp->action = ACT_RELOAD;
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 25/33] libmultipath: sysfs_attr_set_value: use const char*
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (23 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 24/33] libmultipath: select_action: check special features separately Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 26/33] libmultipath: reload map if not known to udev Martin Wilck
` (9 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
The value argument in syfs_attr_set_value should be const char*.
Signed-off-by: Martin Wilck <mwilck@suse.de>
---
libmultipath/sysfs.c | 2 +-
libmultipath/sysfs.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 9cf03117..97e09977 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -154,7 +154,7 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
}
ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
- char * value, size_t value_len)
+ const char * value, size_t value_len)
{
char devpath[PATH_SIZE];
struct stat statbuf;
diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
index 2588c247..75c0f9c1 100644
--- a/libmultipath/sysfs.h
+++ b/libmultipath/sysfs.h
@@ -6,7 +6,7 @@
#define _LIBMULTIPATH_SYSFS_H
ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
- char * value, size_t value_len);
+ const char * value, size_t value_len);
ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
char * value, size_t value_len);
ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 26/33] libmultipath: reload map if not known to udev
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (24 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 25/33] libmultipath: sysfs_attr_set_value: use const char* Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 27/33] libmultipath: differentiate ACT_NOTHING and ACT_IMPOSSIBLE Martin Wilck
` (8 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
The previous changes skip map reload if the existing kernel
state appears correct. Sometimes, boot time race conditions
may cause a device to be correctly set up in the kernel but
not in udev. Check this condition, and reload map in this case.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/configure.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index fb5a5a92..b400bb96 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -39,6 +39,7 @@
#include "util.h"
#include "uxsock.h"
#include "wwids.h"
+#include "sysfs.h"
/* group paths in pg by host adapter
*/
@@ -390,6 +391,35 @@ pgcmp (struct multipath * mpp, struct multipath * cmpp)
return r;
}
+static struct udev_device *
+get_udev_for_mpp(const struct multipath *mpp)
+{
+ dev_t devnum;
+ struct udev_device *udd;
+
+ if (!mpp || !mpp->dmi) {
+ condlog(1, "%s called with empty mpp", __func__);
+ return NULL;
+ }
+
+ devnum = makedev(mpp->dmi->major, mpp->dmi->minor);
+ udd = udev_device_new_from_devnum(udev, 'b', devnum);
+ if (!udd) {
+ condlog(1, "failed to get udev device for %s", mpp->alias);
+ return NULL;
+ }
+ return udd;
+}
+
+static int
+is_mpp_known_to_udev(const struct multipath *mpp)
+{
+ struct udev_device *udd = get_udev_for_mpp(mpp);
+ int ret = (udd != NULL);
+ udev_device_unref(udd);
+ return ret;
+}
+
static void
select_action (struct multipath * mpp, vector curmp, int force_reload)
{
@@ -529,6 +559,12 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
mpp->alias);
return;
}
+ if (!is_mpp_known_to_udev(cmpp)) {
+ mpp->action = ACT_RELOAD;
+ condlog(3, "%s: set ACT_RELOAD (udev device not initialized)",
+ mpp->alias);
+ return;
+ }
mpp->action = ACT_NOTHING;
condlog(3, "%s: set ACT_NOTHING (map unchanged)",
mpp->alias);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 27/33] libmultipath: differentiate ACT_NOTHING and ACT_IMPOSSIBLE
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (25 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 26/33] libmultipath: reload map if not known to udev Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 28/33] libmultipath: coalesce_paths: trigger uevent if nothing done Martin Wilck
` (7 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
select_action uses ACT_NOTHING for two different cases,
1) if changes can't be applied for some reason, and
2) if nothing needs to be done. Introduce ACT_IMPOSSIBLE
for case 1).
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
libmultipath/configure.c | 7 ++++---
libmultipath/configure.h | 1 +
libmultipath/print.c | 2 +-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index b400bb96..025947a8 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -464,13 +464,13 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
/* reset alias to existing alias */
FREE(mpp->alias);
mpp->alias = STRDUP(cmpp->alias);
- mpp->action = ACT_NOTHING;
+ mpp->action = ACT_IMPOSSIBLE;
return;
}
if (pathcount(mpp, PATH_UP) == 0) {
- mpp->action = ACT_NOTHING;
- condlog(3, "%s: set ACT_NOTHING (no usable path)",
+ mpp->action = ACT_IMPOSSIBLE;
+ condlog(3, "%s: set ACT_IMPOSSIBLE (no usable path)",
mpp->alias);
return;
}
@@ -671,6 +671,7 @@ int domap(struct multipath *mpp, char *params, int is_daemon)
switch (mpp->action) {
case ACT_REJECT:
case ACT_NOTHING:
+ case ACT_IMPOSSIBLE:
return DOMAP_EXIST;
case ACT_SWITCHPG:
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 442c956d..fb078a61 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -20,6 +20,7 @@ enum actions {
ACT_RESIZE,
ACT_FORCERENAME,
ACT_DRY_RUN,
+ ACT_IMPOSSIBLE,
};
#define FLUSH_ONE 1
diff --git a/libmultipath/print.c b/libmultipath/print.c
index 00a36267..7c2a1588 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -996,7 +996,7 @@ int snprint_multipath_topology(char *buff, int len, struct multipath *mpp,
if (verbosity > 1 &&
mpp->action != ACT_NOTHING &&
- mpp->action != ACT_UNDEF)
+ mpp->action != ACT_UNDEF && mpp->action != ACT_IMPOSSIBLE)
c += sprintf(c, "%%A: ");
c += sprintf(c, "%%n");
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 28/33] libmultipath: coalesce_paths: trigger uevent if nothing done
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (26 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 27/33] libmultipath: differentiate ACT_NOTHING and ACT_IMPOSSIBLE Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 29/33] kpartx: sanitize delete partitions Martin Wilck
` (6 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel; +Cc: Martin Wilck
From: Martin Wilck <mwilck@suse.de>
The previous patches skip RELOAD actions if there's nothing
to be done. I found a corner case where this may lead to imporperly
initialized device nodes (in my case a by-label link hadn't been
reset to the partition on the multipath device by udev).
Triggering an extra "change" event on the device fixes
this situation.
Signed-off-by: Martin Wilck <mwilck@suse.de>
---
libmultipath/configure.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 025947a8..d9554553 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -411,6 +411,18 @@ get_udev_for_mpp(const struct multipath *mpp)
return udd;
}
+static void
+trigger_udev_change(const struct multipath *mpp)
+{
+ static const char change[] = "change";
+ struct udev_device *udd = get_udev_for_mpp(mpp);
+ if (!udd)
+ return;
+ condlog(3, "triggering %s uevent for %s", change, mpp->alias);
+ sysfs_attr_set_value(udd, "uevent", change, sizeof(change)-1);
+ udev_device_unref(udd);
+}
+
static int
is_mpp_known_to_udev(const struct multipath *mpp)
{
@@ -949,6 +961,18 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
if (r == DOMAP_DRY)
continue;
+ if (r == DOMAP_EXIST && mpp->action == ACT_NOTHING &&
+ force_reload == FORCE_RELOAD_WEAK)
+ /*
+ * First time we're called, and no changes applied.
+ * domap() was a noop. But we can't be sure that
+ * udev has already finished setting up this device
+ * (udev in initrd may have been shut down while
+ * processing this device or its children).
+ * Trigger a change event, just in case.
+ */
+ trigger_udev_change(find_mp_by_wwid(curmp, mpp->wwid));
+
conf = get_multipath_config();
allow_queueing = conf->allow_queueing;
put_multipath_config(conf);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 29/33] kpartx: sanitize delete partitions
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (27 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 28/33] libmultipath: coalesce_paths: trigger uevent if nothing done Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 30/33] tur: Add pthread_testcancel() Martin Wilck
` (5 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
kpartx has a rather braindead method for deleting partitions;
generating 'possible' partition names and trying to remove all
of them.
With this patch kpartx looks at the device-mapper devices on top
of the referenced device, and removes them if they found to be
kpartx-generated devices.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
kpartx/devmapper.c | 229 ++++++++++++++++++++++++++++++++++++++++++++++++++---
kpartx/devmapper.h | 7 +-
kpartx/kpartx.c | 44 ++--------
3 files changed, 229 insertions(+), 51 deletions(-)
diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 2acae25e..cf6650c6 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -252,7 +252,7 @@ out:
}
char *
-dm_mapuuid(int major, int minor)
+dm_mapuuid(const char *mapname)
{
struct dm_task *dmt;
const char *tmp;
@@ -261,9 +261,9 @@ dm_mapuuid(int major, int minor)
if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
return NULL;
+ if (!dm_task_set_name(dmt, mapname))
+ goto out;
dm_task_no_open_count(dmt);
- dm_task_set_major(dmt, major);
- dm_task_set_minor(dmt, minor);
if (!dm_task_run(dmt))
goto out;
@@ -277,7 +277,7 @@ out:
}
int
-dm_devn (char * mapname, int *major, int *minor)
+dm_devn (const char * mapname, int *major, int *minor)
{
int r = 1;
struct dm_task *dmt;
@@ -304,8 +304,8 @@ out:
return r;
}
-int
-dm_get_map(int major, int minor, char * outparams)
+static int
+dm_get_map(char *mapname, char * outparams)
{
int r = 1;
struct dm_task *dmt;
@@ -316,8 +316,8 @@ dm_get_map(int major, int minor, char * outparams)
if (!(dmt = dm_task_create(DM_DEVICE_TABLE)))
return 1;
- dm_task_set_major(dmt, major);
- dm_task_set_minor(dmt, minor);
+ if (!dm_task_set_name(dmt, mapname))
+ goto out;
dm_task_no_open_count(dmt);
if (!dm_task_run(dmt))
@@ -334,15 +334,224 @@ out:
return r;
}
+static int
+dm_get_opencount (const 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;
+
+ if (!info.exists)
+ goto out;
+
+ r = info.open_count;
+out:
+ dm_task_destroy(dmt);
+ return r;
+}
+
+/*
+ * returns:
+ * 1 : match
+ * 0 : no match
+ * -1 : empty map
+ */
+static int
+dm_type(const char * name, char * type)
+{
+ int r = 0;
+ struct dm_task *dmt;
+ uint64_t start, length;
+ char *target_type = NULL;
+ char *params;
+
+ if (!(dmt = dm_task_create(DM_DEVICE_TABLE)))
+ return 0;
+
+ if (!dm_task_set_name(dmt, name))
+ goto out;
+
+ dm_task_no_open_count(dmt);
+
+ if (!dm_task_run(dmt))
+ goto out;
+
+ /* Fetch 1st target */
+ dm_get_next_target(dmt, NULL, &start, &length,
+ &target_type, ¶ms);
+
+ if (!target_type)
+ r = -1;
+ else if (!strcmp(target_type, type))
+ r = 1;
+
+out:
+ dm_task_destroy(dmt);
+ return r;
+}
+
+/*
+ * returns:
+ * 0 : if both uuids end with same suffix which starts with UUID_PREFIX
+ * 1 : otherwise
+ */
+int
+dm_compare_uuid(const char *mapuuid, const char *partname)
+{
+ char *partuuid;
+ int r = 1;
+
+ partuuid = dm_mapuuid(partname);
+ if (!partuuid)
+ return 1;
+
+ if (!strncmp(partuuid, "part", 4)) {
+ char *p = strstr(partuuid, "mpath-");
+ if (p && !strcmp(mapuuid, p))
+ r = 0;
+ }
+ free(partuuid);
+ return r;
+}
+
+struct remove_data {
+ int verbose;
+};
+
+static int
+do_foreach_partmaps (const char * mapname, const char *uuid,
+ int (*partmap_func)(const char *, void *),
+ void *data)
+{
+ struct dm_task *dmt;
+ struct dm_names *names;
+ struct remove_data *rd = data;
+ unsigned next = 0;
+ char params[PARAMS_SIZE];
+ int major, minor;
+ char dev_t[32];
+ int r = 1;
+
+ if (!(dmt = dm_task_create(DM_DEVICE_LIST)))
+ return 1;
+
+ dm_task_no_open_count(dmt);
+
+ if (!dm_task_run(dmt))
+ goto out;
+
+ if (!(names = dm_task_get_names(dmt)))
+ goto out;
+
+ if (!names->dev) {
+ r = 0; /* this is perfectly valid */
+ goto out;
+ }
+
+ if (dm_devn(mapname, &major, &minor))
+ goto out;
+
+ sprintf(dev_t, "%d:%d", major, minor);
+ do {
+ /*
+ * skip our devmap
+ */
+ if (!strcmp(names->name, mapname))
+ goto next;
+
+ /*
+ * skip if we cannot fetch the map table from the kernel
+ */
+ if (dm_get_map(names->name, ¶ms[0]))
+ goto next;
+
+ /*
+ * skip if the table does not map over the multipath map
+ */
+ if (!strstr(params, dev_t))
+ goto next;
+
+ /*
+ * skip if devmap target is not "linear"
+ */
+ if (!dm_type(names->name, "linear")) {
+ if (rd->verbose)
+ printf("%s: is not a linear target. Not removing\n",
+ names->name);
+ goto next;
+ }
+
+ /*
+ * skip if uuids don't match
+ */
+ if (dm_compare_uuid(uuid, names->name)) {
+ if (rd->verbose)
+ printf("%s: is not a kpartx partition. Not removing\n",
+ names->name);
+ goto next;
+ }
+
+ if (partmap_func(names->name, data) != 0)
+ goto out;
+ next:
+ next = names->next;
+ names = (void *) names + next;
+ } while (next);
+
+ r = 0;
+out:
+ dm_task_destroy (dmt);
+ return r;
+}
+
+static int
+remove_partmap(const char *name, void *data)
+{
+ struct remove_data *rd = (struct remove_data *)data;
+ int r = 0;
+
+ if (dm_get_opencount(name)) {
+ if (rd->verbose)
+ printf("%s is in use. Not removing", name);
+ return 1;
+ }
+ if (!dm_simplecmd(DM_DEVICE_REMOVE, name, 0, 0)) {
+ if (rd->verbose)
+ printf("%s: failed to remove\n", name);
+ r = 1;
+ } else if (rd->verbose)
+ printf("del devmap : %s\n", name);
+ return r;
+}
+
+int
+dm_remove_partmaps (char * mapname, char *uuid, int verbose)
+{
+ struct remove_data rd = { verbose };
+ return do_foreach_partmaps(mapname, uuid, remove_partmap, &rd);
+}
+
#define FEATURE_NO_PART "no_partitions"
int
-dm_no_partitions(int major, int minor)
+dm_no_partitions(char *mapname)
{
char params[PARAMS_SIZE], *ptr;
int i, num_features;
- if (dm_get_map(major, minor, params))
+ if (dm_get_map(mapname, params))
return 0;
ptr = params;
diff --git a/kpartx/devmapper.h b/kpartx/devmapper.h
index 436efe11..9988ec0f 100644
--- a/kpartx/devmapper.h
+++ b/kpartx/devmapper.h
@@ -16,8 +16,9 @@ int dm_addmap (int, const char *, const char *, const char *, uint64_t,
int dm_map_present (char *, char **);
char * dm_mapname(int major, int minor);
dev_t dm_get_first_dep(char *devname);
-char * dm_mapuuid(int major, int minor);
-int dm_devn (char * mapname, int *major, int *minor);
-int dm_no_partitions(int major, int minor);
+char * dm_mapuuid(const char *mapname);
+int dm_devn (const char * mapname, int *major, int *minor);
+int dm_remove_partmaps (char * mapname, char *uuid, int verbose);
+int dm_no_partitions(char * mapname);
#endif /* _KPARTX_DEVMAPPER_H */
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index 34527877..58e60ffe 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -355,17 +355,15 @@ main(int argc, char **argv){
off = find_devname_offset(device);
if (!loopdev) {
- uuid = dm_mapuuid(major(buf.st_rdev), minor(buf.st_rdev));
mapname = dm_mapname(major(buf.st_rdev), minor(buf.st_rdev));
+ if (mapname)
+ uuid = dm_mapuuid(mapname);
}
- if (!uuid)
- uuid = device + off;
-
if (!mapname)
mapname = device + off;
- else if (!force_devmap &&
- dm_no_partitions(major(buf.st_rdev), minor(buf.st_rdev))) {
+ if (!force_devmap &&
+ dm_no_partitions(mapname)) {
/* Feature 'no_partitions' is set, return */
return 0;
}
@@ -453,42 +451,12 @@ main(int argc, char **argv){
break;
case DELETE:
- for (j = MAXSLICES-1; j >= 0; j--) {
- char *part_uuid, *reason;
-
- if (safe_sprintf(partname, "%s%s%d",
- mapname, delim, j+1)) {
- fprintf(stderr, "partname too small\n");
- exit(1);
- }
- strip_slash(partname);
-
- if (!dm_map_present(partname, &part_uuid))
- continue;
-
- if (part_uuid && uuid) {
- if (check_uuid(uuid, part_uuid, &reason) != 0) {
- fprintf(stderr, "%s is %s. Not removing\n", partname, reason);
- free(part_uuid);
- continue;
- }
- free(part_uuid);
- }
-
- if (!dm_simplecmd(DM_DEVICE_REMOVE, partname,
- 0, 0)) {
- r++;
- continue;
- }
- if (verbose)
- printf("del devmap : %s\n", partname);
- }
-
+ r = dm_remove_partmaps(mapname, uuid, verbose);
if (loopdev) {
if (del_loop(loopdev)) {
if (verbose)
printf("can't del loop : %s\n",
- loopdev);
+ loopdev);
exit(1);
}
printf("loop deleted : %s\n", loopdev);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 30/33] tur: Add pthread_testcancel()
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (28 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 29/33] kpartx: sanitize delete partitions Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 31/33] multipathd: fixup check for new path states Martin Wilck
` (4 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When the ioctl returns we need to check if a cancellation has
been requested; otherwise we'd be re-setting the state and
overwrite any pending values.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
libmultipath/checkers/tur.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index d9a9e67d..b4a5cb2f 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -268,6 +268,7 @@ static void *tur_thread(void *ctx)
pthread_mutex_unlock(&ct->lock);
state = tur_check(ct->fd, ct->timeout, copy_msg_to_tcc, ct->message);
+ pthread_testcancel();
/* TUR checker done */
pthread_mutex_lock(&ct->lock);
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 31/33] multipathd: fixup check for new path states
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (29 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 30/33] tur: Add pthread_testcancel() Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 32/33] libmultipath/checkers: make RADOS checker optional Martin Wilck
` (3 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
From: Hannes Reinecke <hare@suse.de>
When testing for new path states we should be making sure to
always using the same path state mask. Otherwise we'll miss
out any states.
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
multipathd/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index e2e73749..aa359b4a 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1735,7 +1735,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
pp->checkint = conf->checkint;
put_multipath_config(conf);
- if (newstate == PATH_DOWN || newstate == PATH_SHAKY || newstate == PATH_TIMEOUT) {
+ if (newstate != PATH_UP && newstate != PATH_GHOST) {
/*
* proactively fail path in the DM
*/
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 32/33] libmultipath/checkers: make RADOS checker optional
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (30 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 31/33] multipathd: fixup check for new path states Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 16:23 ` [PATCH 33/33] Make libdmmp build optional Martin Wilck
` (2 subsequent siblings)
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
Some distros lack the rados header files. Use
"make ENABLE_RADOS=0" on such distributions to build
multipath-tools in such cases. The default (to enable RADOS
support) remains unchanged.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
Makefile.inc | 3 +++
libmultipath/checkers/Makefile | 6 ++++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/Makefile.inc b/Makefile.inc
index 93d8e340..b5e910f9 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -8,6 +8,9 @@
#
# WITH_LOCAL_LIBDM = 1
# WITH_LOCAL_LIBSYSFS = 1
+#
+# Uncomment to disable RADOS support (e.g. if rados headers are missing).
+# ENABLE_RADOS = 0
ifeq ($(TOPDIR),)
TOPDIR = ..
diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile
index 11ab76f6..4970fc07 100644
--- a/libmultipath/checkers/Makefile
+++ b/libmultipath/checkers/Makefile
@@ -13,8 +13,10 @@ LIBS= \
libcheckdirectio.so \
libcheckemc_clariion.so \
libcheckhp_sw.so \
- libcheckrdac.so \
- libcheckrbd.so
+ libcheckrdac.so
+ifneq ($(ENABLE_RADOS),0)
+LIBS += libcheckrbd.so
+endif
all: $(LIBS)
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* [PATCH 33/33] Make libdmmp build optional
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (31 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 32/33] libmultipath/checkers: make RADOS checker optional Martin Wilck
@ 2017-02-28 16:23 ` Martin Wilck
2017-02-28 22:44 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
2017-03-22 19:02 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
34 siblings, 0 replies; 57+ messages in thread
From: Martin Wilck @ 2017-02-28 16:23 UTC (permalink / raw)
To: dm-devel
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
Makefile | 6 +++++-
Makefile.inc | 3 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 9f8bf775..cfee0d07 100644
--- a/Makefile
+++ b/Makefile
@@ -30,12 +30,16 @@ BUILDDIRS = \
libmultipath/prioritizers \
libmultipath/checkers \
libmpathpersist \
- libdmmp \
multipath \
multipathd \
mpathpersist \
kpartx
+ifneq ($(ENABLE_LIBDMMP),0)
+BUILDDIRS += \
+ libdmmp
+endif
+
all: recurse
recurse:
diff --git a/Makefile.inc b/Makefile.inc
index b5e910f9..740081e3 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -11,6 +11,9 @@
#
# Uncomment to disable RADOS support (e.g. if rados headers are missing).
# ENABLE_RADOS = 0
+#
+# Uncomment to disable libdmmp support
+# ENABLE_LIBDMMP = 0
ifeq ($(TOPDIR),)
TOPDIR = ..
--
2.11.0
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (32 preceding siblings ...)
2017-02-28 16:23 ` [PATCH 33/33] Make libdmmp build optional Martin Wilck
@ 2017-02-28 22:44 ` Xose Vazquez Perez
2017-03-01 8:12 ` Martin Wilck
2017-03-22 19:02 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
34 siblings, 1 reply; 57+ messages in thread
From: Xose Vazquez Perez @ 2017-02-28 22:44 UTC (permalink / raw)
To: Martin Wilck, dm-devel
On 02/28/2017 05:22 PM, Martin Wilck wrote:
> As announced previously, here comes a collection of multipath-tools patches
> that SUSE is using for SLES12 SP2.
>
> The whole series can be roughly broken down into the following logical
> parts. Details can be found in the indvidual patches.
There is a recent one missing:
---
Subject: libmultipath/propsel: Do not select sysfs prioritizer for RDAC arrays
Recent RDAC (NetApp E-Series) firmware implemented an internal load
balancer and switched to implicit ALUA.
Unfortunately the load balancer relies on periodic REPORT TARGET PORT
GROUPS from the host, so we cannot use the sysfs prioritizer here.
---
Martin, thank you for your work.
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-02-28 22:44 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
@ 2017-03-01 8:12 ` Martin Wilck
2017-03-23 18:43 ` multipath-tools (patch): Do not select sysfs prioritizer for RDAC arrays (was Re: [PATCH 00/33] multipath-tools fixes from SUSE) Xose Vazquez Perez
0 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-03-01 8:12 UTC (permalink / raw)
To: Xose Vazquez Perez, dm-devel
On Tue, 2017-02-28 at 23:44 +0100, Xose Vazquez Perez wrote:
>
> There is a recent one missing:
> ---
> Subject: libmultipath/propsel: Do not select sysfs prioritizer for
> RDAC arrays
Well observed :-) This one still needs verification. We will submit it
when it's final.
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* multipath-tools (patch): Do not select sysfs prioritizer for RDAC arrays (was Re: [PATCH 00/33] multipath-tools fixes from SUSE)
2017-03-01 8:12 ` Martin Wilck
@ 2017-03-23 18:43 ` Xose Vazquez Perez
2017-03-23 20:40 ` Stewart, Sean
0 siblings, 1 reply; 57+ messages in thread
From: Xose Vazquez Perez @ 2017-03-23 18:43 UTC (permalink / raw)
To: Martin Wilck, Hannes Reinecke, Stewart, Sean, Christophe Varoqui
Cc: device-mapper development
On 03/01/2017 09:12 AM, Martin Wilck wrote:
> On Tue, 2017-02-28 at 23:44 +0100, Xose Vazquez Perez wrote:
>>
>> There is a recent one missing:
>> ---
>> Subject: libmultipath/propsel: Do not select sysfs prioritizer for
>> RDAC arrays
>
> Well observed :-) This one still needs verification. We will submit it
> when it's final.
Is it stable/tested enough to be merged?
commit 0119472d847eaa24cae7a0f3b523af82f50dd4df
Author: Hannes Reinecke <hare@suse.de>
Date: Fri Feb 10 15:47:43 2017 +0100
libmultipath/propsel: Do not select sysfs prioritizer for RDAC arrays
Recent RDAC (NetApp E-Series) firmware implemented an internal load
balancer and switched to implicit ALUA.
Unfortunately the load balancer relies on periodic REPORT TARGET PORT
GROUPS from the host, so we cannot use the sysfs prioritizer here.
References: bsc#1004858
Signed-off-by: Hannes Reinecke <hare@suse.com>
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 4d4fc895..46483172 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1112,7 +1112,7 @@ get_vpd_sysfs (struct udev_device *parent, int pg, char * str, int maxlen)
return len;
}
-static int
+int
get_vpd_sgio (int fd, int pg, char * str, int maxlen)
{
int len, buff_len;
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index 3039268d..2479c242 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -35,6 +35,7 @@ int path_discovery (vector pathvec, int flag);
int do_tur (char *);
int path_offline (struct path *);
int get_state (struct path * pp, struct config * conf, int daemon);
+int get_vpd_sgio (int fd, int pg, char * str, int maxlen);
int pathinfo (struct path * pp, struct config * conf, int mask);
int alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
int flag, struct path **pp_ptr);
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 0c499339..87d2d5f9 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -363,6 +363,24 @@ out:
return 0;
}
+/*
+ * Current RDAC (NetApp E-Series) firmware relies
+ * on periodic REPORT TARGET PORT GROUPS for
+ * internal load balancing.
+ * Using the sysfs priority checker defeats this purpose.
+ */
+static int
+check_rdac(struct path * pp)
+{
+ int len;
+ char buff[44];
+
+ len = get_vpd_sgio(pp->fd, 0xC9, buff, 44);
+ if (len <= 0)
+ return 0;
+ return !(memcmp(buff + 4, "vac1", 4));
+}
+
void
detect_prio(struct config *conf, struct path * pp)
{
@@ -372,8 +390,10 @@ detect_prio(struct config *conf, struct path * pp)
if (pp->tpgs <= 0)
return;
- if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
- default_prio = PRIO_SYSFS;
+ if (pp->tpgs == 2 && !check_rdac(pp)) {
+ if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
+ default_prio = PRIO_SYSFS;
+ }
prio_get(conf->multipath_dir, p, default_prio, DEFAULT_PRIO_ARGS);
}
^ permalink raw reply related [flat|nested] 57+ messages in thread* Re: multipath-tools (patch): Do not select sysfs prioritizer for RDAC arrays (was Re: [PATCH 00/33] multipath-tools fixes from SUSE)
2017-03-23 18:43 ` multipath-tools (patch): Do not select sysfs prioritizer for RDAC arrays (was Re: [PATCH 00/33] multipath-tools fixes from SUSE) Xose Vazquez Perez
@ 2017-03-23 20:40 ` Stewart, Sean
0 siblings, 0 replies; 57+ messages in thread
From: Stewart, Sean @ 2017-03-23 20:40 UTC (permalink / raw)
To: Xose Vazquez Perez, Martin Wilck, Hannes Reinecke,
Christophe Varoqui, Schremmer, Steven
Cc: device-mapper development
On 3/23/17, 1:43 PM, "dm-devel-bounces@redhat.com on behalf of Xose Vazquez Perez" <dm-devel-bounces@redhat.com on behalf of xose.vazquez@gmail.com> wrote:
On 03/01/2017 09:12 AM, Martin Wilck wrote:
> On Tue, 2017-02-28 at 23:44 +0100, Xose Vazquez Perez wrote:
>>
>> There is a recent one missing:
>> ---
>> Subject: libmultipath/propsel: Do not select sysfs prioritizer for
>> RDAC arrays
>
> Well observed :-) This one still needs verification. We will submit it
> when it's final.
Is it stable/tested enough to be merged?
This patch has been tested and verified with SUSE 12 as part of our process with the Bugzilla, so based on that I would say yes.
Thanks,
--
Sean Stewart
Linux Software Engineer
Hyperscale Storage Group
NetApp
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-02-28 16:22 [PATCH 00/33] multipath-tools fixes from SUSE Martin Wilck
` (33 preceding siblings ...)
2017-02-28 22:44 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
@ 2017-03-22 19:02 ` Xose Vazquez Perez
2017-03-22 21:29 ` Christophe Varoqui
34 siblings, 1 reply; 57+ messages in thread
From: Xose Vazquez Perez @ 2017-03-22 19:02 UTC (permalink / raw)
To: Martin Wilck, Christophe Varoqui; +Cc: device-mapper development
On 02/28/2017 05:22 PM, Martin Wilck wrote:
> As announced previously, here comes a collection of multipath-tools patches
> that SUSE is using for SLES12 SP2.
From this bundle, only 19 of them were merged.
14 are missing, or ...pending?
^ permalink raw reply [flat|nested] 57+ messages in thread* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-03-22 19:02 ` [PATCH 00/33] multipath-tools fixes from SUSE Xose Vazquez Perez
@ 2017-03-22 21:29 ` Christophe Varoqui
2017-03-23 8:30 ` Christophe Varoqui
0 siblings, 1 reply; 57+ messages in thread
From: Christophe Varoqui @ 2017-03-22 21:29 UTC (permalink / raw)
To: Xose Vazquez Perez; +Cc: device-mapper development, Martin Wilck
[-- Attachment #1.1: Type: text/plain, Size: 398 bytes --]
Pending.
I plan to merge the rest tomorrow.
Le 22 mars 2017 8:02 PM, "Xose Vazquez Perez" <xose.vazquez@gmail.com> a
écrit :
> On 02/28/2017 05:22 PM, Martin Wilck wrote:
>
> > As announced previously, here comes a collection of multipath-tools
> patches
> > that SUSE is using for SLES12 SP2.
>
> From this bundle, only 19 of them were merged.
> 14 are missing, or ...pending?
>
[-- Attachment #1.2: Type: text/html, Size: 715 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-03-22 21:29 ` Christophe Varoqui
@ 2017-03-23 8:30 ` Christophe Varoqui
2017-03-24 7:44 ` Martin Wilck
0 siblings, 1 reply; 57+ messages in thread
From: Christophe Varoqui @ 2017-03-23 8:30 UTC (permalink / raw)
To: Xose Vazquez Perez; +Cc: device-mapper development, Martin Wilck
[-- Attachment #1.1: Type: text/plain, Size: 558 bytes --]
The full set is now merged.
Thanks.
On Wed, Mar 22, 2017 at 10:29 PM, Christophe Varoqui <
christophe.varoqui@opensvc.com> wrote:
> Pending.
> I plan to merge the rest tomorrow.
>
> Le 22 mars 2017 8:02 PM, "Xose Vazquez Perez" <xose.vazquez@gmail.com> a
> écrit :
>
>> On 02/28/2017 05:22 PM, Martin Wilck wrote:
>>
>> > As announced previously, here comes a collection of multipath-tools
>> patches
>> > that SUSE is using for SLES12 SP2.
>>
>> From this bundle, only 19 of them were merged.
>> 14 are missing, or ...pending?
>>
>
[-- Attachment #1.2: Type: text/html, Size: 1225 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-03-23 8:30 ` Christophe Varoqui
@ 2017-03-24 7:44 ` Martin Wilck
2017-04-12 7:38 ` Christophe Varoqui
0 siblings, 1 reply; 57+ messages in thread
From: Martin Wilck @ 2017-03-24 7:44 UTC (permalink / raw)
To: Christophe Varoqui, Xose Vazquez Perez; +Cc: device-mapper development
On Thu, 2017-03-23 at 09:30 +0100, Christophe Varoqui wrote:
> The full set is now merged.
> Thanks.
Thanks a lot, Christophe.
Btw, are you planning a version bump any time soon?
Regards,
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 57+ messages in thread
* Re: [PATCH 00/33] multipath-tools fixes from SUSE
2017-03-24 7:44 ` Martin Wilck
@ 2017-04-12 7:38 ` Christophe Varoqui
0 siblings, 0 replies; 57+ messages in thread
From: Christophe Varoqui @ 2017-04-12 7:38 UTC (permalink / raw)
To: Martin Wilck; +Cc: device-mapper development, Xose Vazquez Perez
[-- Attachment #1.1: Type: text/plain, Size: 579 bytes --]
Hi Martin,
The multipath tools version is now bumped to 0.7.0.
Best regards,
Christophe
On Fri, Mar 24, 2017 at 8:44 AM, Martin Wilck <mwilck@suse.com> wrote:
> On Thu, 2017-03-23 at 09:30 +0100, Christophe Varoqui wrote:
> > The full set is now merged.
> > Thanks.
>
> Thanks a lot, Christophe.
>
> Btw, are you planning a version bump any time soon?
>
> Regards,
> Martin
>
> --
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
>
>
[-- Attachment #1.2: Type: text/html, Size: 1163 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 57+ messages in thread