* [dm-devel] [PATCH 1/5] multipath.conf: fix typo in ghost_delay description
2021-07-29 21:46 [dm-devel] [PATCH 0/5] Mulitpath: miscellaneous patches Benjamin Marzinski
@ 2021-07-29 21:46 ` Benjamin Marzinski
2021-08-12 10:22 ` Martin Wilck
2021-07-29 21:46 ` [dm-devel] [PATCH 2/5] mpathpersist: fail commands when no usable paths exist Benjamin Marzinski
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Benjamin Marzinski @ 2021-07-29 21:46 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipath/multipath.conf.5 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 064e4826..d6b8c7f6 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1251,7 +1251,7 @@ The default is: in \fB/sys/block/<dev>/queue/max_sectors_kb\fR
Sets the number of seconds that multipath will wait after creating a device
with only ghost paths before marking it ready for use in systemd. This gives
the active paths time to appear before the multipath runs the hardware handler
-to switch the ghost paths to active ones. Setting this to \fI0\fR or \fIon\fR
+to switch the ghost paths to active ones. Setting this to \fI0\fR or \fIno\fR
makes multipath immediately mark a device with only ghost paths as ready.
.RS
.TP
--
2.17.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [dm-devel] [PATCH 2/5] mpathpersist: fail commands when no usable paths exist
2021-07-29 21:46 [dm-devel] [PATCH 0/5] Mulitpath: miscellaneous patches Benjamin Marzinski
2021-07-29 21:46 ` [dm-devel] [PATCH 1/5] multipath.conf: fix typo in ghost_delay description Benjamin Marzinski
@ 2021-07-29 21:46 ` Benjamin Marzinski
2021-08-12 10:23 ` Martin Wilck
2021-07-29 21:46 ` [dm-devel] [PATCH 3/5] multipath: print warning if multipathd is not running Benjamin Marzinski
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Benjamin Marzinski @ 2021-07-29 21:46 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
"mpathpersist -oCK <reservation_key> <device>" will return success if it
is run on devices with no usable paths, but nothing is actually done.
The -L command will fail, but it should give up sooner, and with a more
helpful error message.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathpersist/mpath_persist.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 190e9707..26710e79 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -604,7 +604,8 @@ int mpath_prout_common(struct multipath *mpp,int rq_servact, int rq_scope,
return ret ;
}
}
- return MPATH_PR_SUCCESS;
+ condlog (0, "%s: no path available", mpp->wwid);
+ return MPATH_PR_DMMP_ERROR;
}
int send_prout_activepath(char * dev, int rq_servact, int rq_scope,
@@ -663,6 +664,11 @@ int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope,
active_pathcount = count_active_paths(mpp);
+ if (active_pathcount == 0) {
+ condlog (0, "%s: no path available", mpp->wwid);
+ return MPATH_PR_DMMP_ERROR;
+ }
+
struct threadinfo thread[active_pathcount];
memset(thread, 0, sizeof(thread));
for (i = 0; i < active_pathcount; i++){
--
2.17.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [dm-devel] [PATCH 3/5] multipath: print warning if multipathd is not running.
2021-07-29 21:46 [dm-devel] [PATCH 0/5] Mulitpath: miscellaneous patches Benjamin Marzinski
2021-07-29 21:46 ` [dm-devel] [PATCH 1/5] multipath.conf: fix typo in ghost_delay description Benjamin Marzinski
2021-07-29 21:46 ` [dm-devel] [PATCH 2/5] mpathpersist: fail commands when no usable paths exist Benjamin Marzinski
@ 2021-07-29 21:46 ` Benjamin Marzinski
2021-08-12 10:23 ` Martin Wilck
2021-07-29 21:46 ` [dm-devel] [PATCH 4/5] libmultipath: remove unneeded code in coalesce_paths Benjamin Marzinski
2021-07-29 21:46 ` [dm-devel] [PATCH 5/5] libmultipath: deal with dynamic PTHREAD_STACK_MIN Benjamin Marzinski
4 siblings, 1 reply; 12+ messages in thread
From: Benjamin Marzinski @ 2021-07-29 21:46 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
If multipath notices that multipath devices exist or were created, and
multipathd is not running, it now prints a warning message, so users are
notified of the issue.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/configure.c | 13 +++++++++++--
libmultipath/configure.h | 1 +
libmultipath/libmultipath.version | 5 +++++
multipath/main.c | 5 +++++
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 12278640..0c00bf50 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1082,7 +1082,8 @@ deadmap (struct multipath * mpp)
return 1; /* dead */
}
-int check_daemon(void)
+extern int
+check_daemon(void)
{
int fd;
char *reply;
@@ -1137,6 +1138,8 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
struct config *conf = NULL;
int allow_queueing;
struct bitfield *size_mismatch_seen;
+ bool map_processed = false;
+ bool no_daemon = false;
/* ignore refwwid if it's empty */
if (refwwid && !strlen(refwwid))
@@ -1288,7 +1291,9 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
conf = get_multipath_config();
allow_queueing = conf->allow_queueing;
put_multipath_config(conf);
- if (!is_daemon && !allow_queueing && !check_daemon()) {
+ if (!is_daemon && !allow_queueing &&
+ (no_daemon || !check_daemon())) {
+ no_daemon = true;
if (mpp->no_path_retry != NO_PATH_RETRY_UNDEF &&
mpp->no_path_retry != NO_PATH_RETRY_FAIL)
condlog(3, "%s: multipathd not running, unset "
@@ -1311,6 +1316,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
else
remove_map(mpp, vecs->pathvec, vecs->mpvec,
KEEP_VEC);
+ map_processed = true;
}
/*
* Flush maps with only dead paths (ie not in sysfs)
@@ -1336,6 +1342,9 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
condlog(2, "%s: remove (dead)", alias);
}
}
+ if (map_processed && !is_daemon && (no_daemon || !check_daemon()))
+ condlog(2, "multipath devices exist, but multipathd service is not running");
+
ret = CP_OK;
out:
free(size_mismatch_seen);
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 92a5aba6..efe18b7d 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -59,3 +59,4 @@ struct udev_device *get_udev_device(const char *dev, enum devtypes dev_type);
void trigger_paths_udev_change(struct multipath *mpp, bool is_mpath);
void trigger_partitions_udev_change(struct udev_device *dev, const char *action,
int len);
+int check_daemon(void);
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 1d84d97c..7b48265f 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -285,3 +285,8 @@ global:
print_strbuf;
truncate_strbuf;
} LIBMULTIPATH_8.0.0;
+
+LIBMULTIPATH_8.2.0 {
+global:
+ check_daemon;
+} LIBMULTIPATH_8.1.0;
diff --git a/multipath/main.c b/multipath/main.c
index 8fc0e15f..33377147 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -180,6 +180,7 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector pathvec, char * refwwid)
int i;
struct multipath * mpp;
int flags = (cmd == CMD_LIST_SHORT ? DI_NOIO : DI_ALL);
+ bool maps_present = false;
if (dm_get_maps(curmp))
return 1;
@@ -212,11 +213,15 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector pathvec, char * refwwid)
if (cmd == CMD_CREATE)
reinstate_paths(mpp);
+
+ maps_present = true;
}
if (cmd == CMD_LIST_SHORT || cmd == CMD_LIST_LONG)
print_foreign_topology(libmp_verbosity);
+ if (maps_present && !check_daemon())
+ condlog(2, "multipath devices exist, but multipathd service is not running");
return 0;
}
--
2.17.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [dm-devel] [PATCH 3/5] multipath: print warning if multipathd is not running.
2021-07-29 21:46 ` [dm-devel] [PATCH 3/5] multipath: print warning if multipathd is not running Benjamin Marzinski
@ 2021-08-12 10:23 ` Martin Wilck
2021-08-30 21:28 ` Benjamin Marzinski
0 siblings, 1 reply; 12+ messages in thread
From: Martin Wilck @ 2021-08-12 10:23 UTC (permalink / raw)
To: bmarzins, christophe.varoqui; +Cc: dm-devel
On Do, 2021-07-29 at 16:46 -0500, Benjamin Marzinski wrote:
> If multipath notices that multipath devices exist or were created,
> and
> multipathd is not running, it now prints a warning message, so users
> are
> notified of the issue.
>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
I'm not sure about this. Are there zero valid use cases for using
multipath without multipathd? On production systems, I agree,
multipathd should always be running. Personally wouldn't want to see
this warning every time I run "multipath" while multipathd is
(temporarily) disabled. Have you got user requests for this feature?
In particular, I dislike the idea of putting this code into
libmultipath. I would love to get rid of the "is_daemon" logic some
day. If at all, the detection of the situation and the warning should
be implemented in multipath, IMO.
The message should be prefixed with the word "Warning: " to make sure
the admin understands that he's supposed to take action.
Regards,
Martin
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [dm-devel] [PATCH 3/5] multipath: print warning if multipathd is not running.
2021-08-12 10:23 ` Martin Wilck
@ 2021-08-30 21:28 ` Benjamin Marzinski
0 siblings, 0 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2021-08-30 21:28 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel
On Thu, Aug 12, 2021 at 10:23:09AM +0000, Martin Wilck wrote:
> On Do, 2021-07-29 at 16:46 -0500, Benjamin Marzinski wrote:
> > If multipath notices that multipath devices exist or were created,
> > and
> > multipathd is not running, it now prints a warning message, so users
> > are
> > notified of the issue.
> >
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
>
> I'm not sure about this. Are there zero valid use cases for using
> multipath without multipathd?
There certainly are. That's why I tried to limit the warning to only
cases where the user was creating, reloading, or listing devices and
found/created some. In testing, sure, you might want multipathd
disabled, but in general, if you have multipath devices, you want
multipathd running. If you have it temporarily disabled for some
reason, I don't see much harm in having multipath remind you of that.
> On production systems, I agree,
> multipathd should always be running. Personally wouldn't want to see
> this warning every time I run "multipath" while multipathd is
> (temporarily) disabled. Have you got user requests for this feature?
Well, Support has requested this. Apparently they have repeatedly seen
cases where people don't have any multipath devices present, because
multipathd is not running. They run multipath, the devices appear, and
they move on, unaware that multipathd isn't running for whatever
reason.
> In particular, I dislike the idea of putting this code into
> libmultipath. I would love to get rid of the "is_daemon" logic some
> day. If at all, the detection of the situation and the warning should
> be implemented in multipath, IMO.
Sure. I can rework this to keep the logic inside multipath,
> The message should be prefixed with the word "Warning: " to make sure
> the admin understands that he's supposed to take action.
Makes sense.
-Ben
> Regards,
> Martin
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [dm-devel] [PATCH 4/5] libmultipath: remove unneeded code in coalesce_paths
2021-07-29 21:46 [dm-devel] [PATCH 0/5] Mulitpath: miscellaneous patches Benjamin Marzinski
` (2 preceding siblings ...)
2021-07-29 21:46 ` [dm-devel] [PATCH 3/5] multipath: print warning if multipathd is not running Benjamin Marzinski
@ 2021-07-29 21:46 ` Benjamin Marzinski
2021-08-12 10:23 ` Martin Wilck
2021-07-29 21:46 ` [dm-devel] [PATCH 5/5] libmultipath: deal with dynamic PTHREAD_STACK_MIN Benjamin Marzinski
4 siblings, 1 reply; 12+ messages in thread
From: Benjamin Marzinski @ 2021-07-29 21:46 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
The code at the end of coalesce_paths() removes a multipath device that
was just created/reloaded, if none of its path devices have pp->dev set.
This code is very old, and no longer has any actual effect. Multipath
devices no longer get placed in pathvec without having pp->dev set. Even
if they could, there's no point in creating/reloading the device and
then immediately removing it.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/configure.c | 46 ----------------------------------------
1 file changed, 46 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 0c00bf50..e64eb7bc 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1060,28 +1060,6 @@ int domap(struct multipath *mpp, char *params, int is_daemon)
return DOMAP_FAIL;
}
-static int
-deadmap (struct multipath * mpp)
-{
- int i, j;
- struct pathgroup * pgp;
- struct path * pp;
-
- if (!mpp->pg)
- return 1;
-
- vector_foreach_slot (mpp->pg, pgp, i) {
- if (!pgp->paths)
- continue;
-
- vector_foreach_slot (pgp->paths, pp, j)
- if (strlen(pp->dev))
- return 0; /* alive */
- }
-
- return 1; /* dead */
-}
-
extern int
check_daemon(void)
{
@@ -1318,30 +1296,6 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
KEEP_VEC);
map_processed = true;
}
- /*
- * Flush maps with only dead paths (ie not in sysfs)
- * Keep maps with only failed paths
- */
- if (mpvec) {
- vector_foreach_slot (newmp, mpp, i) {
- char alias[WWID_SIZE];
-
- if (!deadmap(mpp))
- continue;
-
- strlcpy(alias, mpp->alias, WWID_SIZE);
-
- vector_del_slot(newmp, i);
- i--;
- remove_map(mpp, vecs->pathvec, vecs->mpvec, KEEP_VEC);
-
- if (dm_flush_map(alias))
- condlog(2, "%s: remove failed (dead)",
- alias);
- else
- condlog(2, "%s: remove (dead)", alias);
- }
- }
if (map_processed && !is_daemon && (no_daemon || !check_daemon()))
condlog(2, "multipath devices exist, but multipathd service is not running");
--
2.17.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread* [dm-devel] [PATCH 5/5] libmultipath: deal with dynamic PTHREAD_STACK_MIN
2021-07-29 21:46 [dm-devel] [PATCH 0/5] Mulitpath: miscellaneous patches Benjamin Marzinski
` (3 preceding siblings ...)
2021-07-29 21:46 ` [dm-devel] [PATCH 4/5] libmultipath: remove unneeded code in coalesce_paths Benjamin Marzinski
@ 2021-07-29 21:46 ` Benjamin Marzinski
2021-08-12 10:23 ` Martin Wilck
4 siblings, 1 reply; 12+ messages in thread
From: Benjamin Marzinski @ 2021-07-29 21:46 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
Starting in glibc-2.34 (commit 5d98a7da), PTHREAD_STACK_MIN is defined
as sysconf(_SC_THREAD_STACK_MIN) if _GNU_SOURCE is defined. sysconf()
returns a long and can, at least in theory, return -1. This change
causes compilation to fail in setup_thread_attr() due to a comparision
with different signedness, since stacksize is a size_t.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/util.c b/libmultipath/util.c
index e2fafe85..ea858409 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -223,8 +223,8 @@ setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached)
ret = pthread_attr_init(attr);
assert(ret == 0);
- if (stacksize < PTHREAD_STACK_MIN)
- stacksize = PTHREAD_STACK_MIN;
+ if (PTHREAD_STACK_MIN > 0 && stacksize < (size_t)PTHREAD_STACK_MIN)
+ stacksize = (size_t)PTHREAD_STACK_MIN;
ret = pthread_attr_setstacksize(attr, stacksize);
assert(ret == 0);
if (detached) {
--
2.17.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread