dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [dm-devel] [PATCH 0/5] Mulitpath: miscellaneous patches
@ 2021-07-29 21:46 Benjamin Marzinski
  2021-07-29 21:46 ` [dm-devel] [PATCH 1/5] multipath.conf: fix typo in ghost_delay description Benjamin Marzinski
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Benjamin Marzinski @ 2021-07-29 21:46 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck

Here are a couple of unconnected patches. They (at least
libmultipath.version) are meant to be applied on top of
Martin's recent patchset.

Benjamin Marzinski (5):
  multipath.conf: fix typo in ghost_delay description
  mpathpersist: fail commands when no usable paths exist
  multipath: print warning if multipathd is not running.
  libmultipath: remove unneeded code in coalesce_paths
  libmultipath: deal with dynamic PTHREAD_STACK_MIN

 libmpathpersist/mpath_persist.c   |  8 ++++-
 libmultipath/configure.c          | 57 ++++++-------------------------
 libmultipath/configure.h          |  1 +
 libmultipath/libmultipath.version |  5 +++
 libmultipath/util.c               |  4 +--
 multipath/main.c                  |  5 +++
 multipath/multipath.conf.5        |  2 +-
 7 files changed, 31 insertions(+), 51 deletions(-)

-- 
2.17.2

--
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 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	[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	[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	[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	[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	[flat|nested] 12+ messages in thread

* Re: [dm-devel] [PATCH 1/5] multipath.conf: fix typo in ghost_delay description
  2021-07-29 21:46 ` [dm-devel] [PATCH 1/5] multipath.conf: fix typo in ghost_delay description Benjamin Marzinski
@ 2021-08-12 10:22   ` Martin Wilck
  0 siblings, 0 replies; 12+ messages in thread
From: Martin Wilck @ 2021-08-12 10:22 UTC (permalink / raw)
  To: bmarzins, christophe.varoqui; +Cc: dm-devel

On Do, 2021-07-29 at 16:46 -0500, Benjamin Marzinski wrote:
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>



--
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 2/5] mpathpersist: fail commands when no usable paths exist
  2021-07-29 21:46 ` [dm-devel] [PATCH 2/5] mpathpersist: fail commands when no usable paths exist Benjamin Marzinski
@ 2021-08-12 10:23   ` Martin Wilck
  0 siblings, 0 replies; 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:
> "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>

Reviewed-by: Martin Wilck <mwilck@suse.com>


--
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-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 4/5] libmultipath: remove unneeded code in coalesce_paths
  2021-07-29 21:46 ` [dm-devel] [PATCH 4/5] libmultipath: remove unneeded code in coalesce_paths Benjamin Marzinski
@ 2021-08-12 10:23   ` Martin Wilck
  0 siblings, 0 replies; 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:
> 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>

Reviewed-by: Martin Wilck <mwilck@suse.com>


--
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 5/5] libmultipath: deal with dynamic PTHREAD_STACK_MIN
  2021-07-29 21:46 ` [dm-devel] [PATCH 5/5] libmultipath: deal with dynamic PTHREAD_STACK_MIN Benjamin Marzinski
@ 2021-08-12 10:23   ` Martin Wilck
  0 siblings, 0 replies; 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:
> 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>

Reviewed-by: Martin Wilck <mwilck@suse.com>


--
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

end of thread, other threads:[~2021-08-30 21:28 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
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
2021-08-12 10:23   ` Martin Wilck
2021-08-30 21:28     ` Benjamin Marzinski
2021-07-29 21:46 ` [dm-devel] [PATCH 4/5] libmultipath: remove unneeded code in coalesce_paths 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
2021-08-12 10:23   ` Martin Wilck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).