All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes
@ 2020-07-09 10:19 mwilck
  2020-07-09 10:19 ` [PATCH 36/39] libmultipath: add macro DEV_LOSS_TMO_UNSET mwilck
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: mwilck @ 2020-07-09 10:19 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, hi Ben,

This is part II of a larger patch series for multpath-tools I've been preparing.
It contains some minor fixes for dev_loss_tmo handling, mostly logging.
It's based on the previously submitted part I.

The full series will also be available here:
https://github.com/mwilck/multipath-tools/tree/ups/submit-2007

There are tags in that repo for each part of the series.
This part is tagged "submit-200709-2".

Regards,
Martin


Martin Wilck (4):
  libmultipath: add macro DEV_LOSS_TMO_UNSET
  libmultipath: improve logging for dev_loss_tmo override
  libmultipath: print message if setting dev_loss_tmo is unsupported
  libmultipath: increase log level of limiting dev_loss_tmo

 libmultipath/defaults.h  |  1 +
 libmultipath/dict.c      |  4 +--
 libmultipath/discovery.c | 56 ++++++++++++++++++++++++++++++----------
 libmultipath/propsel.c   |  2 +-
 4 files changed, 46 insertions(+), 17 deletions(-)

-- 
2.26.2

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 36/39] libmultipath: add macro DEV_LOSS_TMO_UNSET
  2020-07-09 10:19 [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes mwilck
@ 2020-07-09 10:19 ` mwilck
  2020-07-09 10:19 ` [PATCH 37/39] libmultipath: improve logging for dev_loss_tmo override mwilck
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: mwilck @ 2020-07-09 10:19 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

The special value 0 means "unset" for dev_loss. Make this more
explicit by using a macro.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/defaults.h  |  1 +
 libmultipath/dict.c      |  4 ++--
 libmultipath/discovery.c | 12 +++++++-----
 libmultipath/propsel.c   |  2 +-
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 0574e8f..39a5e41 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -58,6 +58,7 @@
 #define CHECKINT_UNDEF		UINT_MAX
 #define DEFAULT_CHECKINT	5
 
+#define DEV_LOSS_TMO_UNSET	0U
 #define MAX_DEV_LOSS_TMO	UINT_MAX
 #define DEFAULT_PIDFILE		"/" RUN_DIR "/multipathd.pid"
 #define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 0e9ea38..be3029c 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -873,7 +873,7 @@ set_dev_loss(vector strvec, void *ptr)
 	if (!strcmp(buff, "infinity"))
 		*uint_ptr = MAX_DEV_LOSS_TMO;
 	else if (sscanf(buff, "%u", uint_ptr) != 1)
-		*uint_ptr = 0;
+		*uint_ptr = DEV_LOSS_TMO_UNSET;
 
 	FREE(buff);
 	return 0;
@@ -882,7 +882,7 @@ set_dev_loss(vector strvec, void *ptr)
 int
 print_dev_loss(char * buff, int len, unsigned long v)
 {
-	if (!v)
+	if (v == DEV_LOSS_TMO_UNSET)
 		return 0;
 	if (v >= MAX_DEV_LOSS_TMO)
 		return snprintf(buff, len, "\"infinity\"");
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 81a3fad..f7d6672 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -671,7 +671,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 					rport_id, value, -ret);
 		}
 	}
-	if (mpp->dev_loss > 0) {
+	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET) {
 		snprintf(value, 16, "%u", mpp->dev_loss);
 		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
 					   value, strlen(value));
@@ -705,7 +705,7 @@ sysfs_set_session_tmo(struct multipath *mpp, struct path *pp)
 	condlog(4, "target%d:%d:%d -> %s", pp->sg_id.host_no,
 		pp->sg_id.channel, pp->sg_id.scsi_id, session_id);
 
-	if (mpp->dev_loss) {
+	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET) {
 		condlog(3, "%s: ignoring dev_loss_tmo on iSCSI", pp->dev);
 	}
 	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
@@ -747,7 +747,7 @@ sysfs_set_nexus_loss_tmo(struct multipath *mpp, struct path *pp)
 	condlog(4, "target%d:%d:%d -> %s", pp->sg_id.host_no,
 		pp->sg_id.channel, pp->sg_id.scsi_id, end_dev_id);
 
-	if (mpp->dev_loss) {
+	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET) {
 		snprintf(value, 11, "%u", mpp->dev_loss);
 		if (sysfs_attr_set_value(sas_dev, "I_T_nexus_loss_timeout",
 					 value, strlen(value)) <= 0)
@@ -782,13 +782,15 @@ sysfs_set_scsi_tmo (struct multipath *mpp, unsigned int checkint)
 			mpp->alias, dev_loss_tmo);
 	}
 	mpp->dev_loss = dev_loss_tmo;
-	if (mpp->dev_loss && mpp->fast_io_fail > 0 &&
+	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET &&
+	    mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
 	    (unsigned int)mpp->fast_io_fail >= mpp->dev_loss) {
 		condlog(3, "%s: turning off fast_io_fail (%d is not smaller than dev_loss_tmo)",
 			mpp->alias, mpp->fast_io_fail);
 		mpp->fast_io_fail = MP_FAST_IO_FAIL_OFF;
 	}
-	if (!mpp->dev_loss && mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
+	if (mpp->dev_loss == DEV_LOSS_TMO_UNSET &&
+	    mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
 		return 0;
 
 	vector_foreach_slot(mpp->paths, pp, i) {
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 2233527..555bd96 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -766,7 +766,7 @@ int select_dev_loss(struct config *conf, struct multipath *mp)
 	mp_set_ovr(dev_loss);
 	mp_set_hwe(dev_loss);
 	mp_set_conf(dev_loss);
-	mp->dev_loss = 0;
+	mp->dev_loss = DEV_LOSS_TMO_UNSET;
 	return 0;
 out:
 	print_dev_loss(buff, 12, mp->dev_loss);
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 37/39] libmultipath: improve logging for dev_loss_tmo override
  2020-07-09 10:19 [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes mwilck
  2020-07-09 10:19 ` [PATCH 36/39] libmultipath: add macro DEV_LOSS_TMO_UNSET mwilck
@ 2020-07-09 10:19 ` mwilck
  2020-07-09 10:19 ` [PATCH 38/39] libmultipath: print message if setting dev_loss_tmo is unsupported mwilck
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: mwilck @ 2020-07-09 10:19 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

Print a more meaningful warning message, and at higher level, if
the configured dev_loss_tmo can't be applied because it conflicts
with no_path_retry.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index f7d6672..e2aea81 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -774,14 +774,15 @@ sysfs_set_scsi_tmo (struct multipath *mpp, unsigned int checkint)
 			no_path_retry_tmo = MAX_DEV_LOSS_TMO;
 		if (no_path_retry_tmo > dev_loss_tmo)
 			dev_loss_tmo = no_path_retry_tmo;
-		condlog(3, "%s: update dev_loss_tmo to %u",
-			mpp->alias, dev_loss_tmo);
 	} else if (mpp->no_path_retry == NO_PATH_RETRY_QUEUE) {
 		dev_loss_tmo = MAX_DEV_LOSS_TMO;
-		condlog(3, "%s: update dev_loss_tmo to %u",
-			mpp->alias, dev_loss_tmo);
 	}
-	mpp->dev_loss = dev_loss_tmo;
+	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET &&
+	    mpp->dev_loss != dev_loss_tmo) {
+		condlog(2, "%s: Using dev_loss_tmo=%u instead of %u because of no_path_retry setting",
+			mpp->alias, dev_loss_tmo, mpp->dev_loss);
+		mpp->dev_loss = dev_loss_tmo;
+	}
 	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET &&
 	    mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
 	    (unsigned int)mpp->fast_io_fail >= mpp->dev_loss) {
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 38/39] libmultipath: print message if setting dev_loss_tmo is unsupported
  2020-07-09 10:19 [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes mwilck
  2020-07-09 10:19 ` [PATCH 36/39] libmultipath: add macro DEV_LOSS_TMO_UNSET mwilck
  2020-07-09 10:19 ` [PATCH 37/39] libmultipath: improve logging for dev_loss_tmo override mwilck
@ 2020-07-09 10:19 ` mwilck
  2020-07-09 10:19 ` [PATCH 39/39] libmultipath: increase log level of limiting dev_loss_tmo mwilck
  2020-07-17  1:05 ` [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes Benjamin Marzinski
  4 siblings, 0 replies; 6+ messages in thread
From: mwilck @ 2020-07-09 10:19 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

If dev_loss_tmo can't be set because it's not supported for the
protocol at hand, let the user know.

Fixme: we could implement this for other protocols such as NVMe.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index e2aea81..c00af7e 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -33,6 +33,8 @@
 #include "unaligned.h"
 #include "prioritizers/alua_rtpg.h"
 #include "foreign.h"
+#include "configure.h"
+#include "print.h"
 
 struct vpd_vendor_page vpd_vendor_pages[VPD_VP_ARRAY_SIZE] = {
 	[VPD_VP_UNDEF]	= { 0x00, "undef" },
@@ -765,6 +767,7 @@ sysfs_set_scsi_tmo (struct multipath *mpp, unsigned int checkint)
 	struct path *pp;
 	int i;
 	unsigned int dev_loss_tmo = mpp->dev_loss;
+	struct path *err_path = NULL;
 
 	if (mpp->no_path_retry > 0) {
 		uint64_t no_path_retry_tmo =
@@ -795,12 +798,34 @@ sysfs_set_scsi_tmo (struct multipath *mpp, unsigned int checkint)
 		return 0;
 
 	vector_foreach_slot(mpp->paths, pp, i) {
-		if (pp->sg_id.proto_id == SCSI_PROTOCOL_FCP)
+		if (pp->bus != SYSFS_BUS_SCSI) {
+			if (!err_path)
+				err_path = pp;
+			continue;
+		}
+
+		switch (pp->sg_id.proto_id) {
+		case SCSI_PROTOCOL_FCP:
 			sysfs_set_rport_tmo(mpp, pp);
-		if (pp->sg_id.proto_id == SCSI_PROTOCOL_ISCSI)
+			continue;
+		case SCSI_PROTOCOL_ISCSI:
 			sysfs_set_session_tmo(mpp, pp);
-		if (pp->sg_id.proto_id == SCSI_PROTOCOL_SAS)
+			continue;
+		case SCSI_PROTOCOL_SAS:
 			sysfs_set_nexus_loss_tmo(mpp, pp);
+			continue;
+		default:
+			if (!err_path)
+				err_path = pp;
+		}
+	}
+
+	if (err_path) {
+		char proto_buf[32];
+
+		snprint_path_protocol(proto_buf, sizeof(proto_buf), err_path);
+		condlog(2, "%s: setting dev_loss_tmo is unsupported for protocol %s",
+			mpp->alias, proto_buf);
 	}
 	return 0;
 }
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 39/39] libmultipath: increase log level of limiting dev_loss_tmo
  2020-07-09 10:19 [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes mwilck
                   ` (2 preceding siblings ...)
  2020-07-09 10:19 ` [PATCH 38/39] libmultipath: print message if setting dev_loss_tmo is unsupported mwilck
@ 2020-07-09 10:19 ` mwilck
  2020-07-17  1:05 ` [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes Benjamin Marzinski
  4 siblings, 0 replies; 6+ messages in thread
From: mwilck @ 2020-07-09 10:19 UTC (permalink / raw)
  To: Christophe Varoqui, Benjamin Marzinski; +Cc: dm-devel, Martin Wilck

From: Martin Wilck <mwilck@suse.com>

Users who aren't familar with the complex dependencies between
dev_loss_tmo, no_path_retry, and fast_io_fail may be confused
if their settings aren't applied. Print this log message at the
default level.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index c00af7e..e26aae2 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -651,7 +651,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 		}
 	} else if (mpp->dev_loss > DEFAULT_DEV_LOSS_TMO &&
 		mpp->no_path_retry != NO_PATH_RETRY_QUEUE) {
-		condlog(3, "%s: limiting dev_loss_tmo to %d, since "
+		condlog(2, "%s: limiting dev_loss_tmo to %d, since "
 			"fast_io_fail is not set",
 			rport_id, DEFAULT_DEV_LOSS_TMO);
 		mpp->dev_loss = DEFAULT_DEV_LOSS_TMO;
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes
  2020-07-09 10:19 [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes mwilck
                   ` (3 preceding siblings ...)
  2020-07-09 10:19 ` [PATCH 39/39] libmultipath: increase log level of limiting dev_loss_tmo mwilck
@ 2020-07-17  1:05 ` Benjamin Marzinski
  4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Marzinski @ 2020-07-17  1:05 UTC (permalink / raw)
  To: mwilck; +Cc: dm-devel

On Thu, Jul 09, 2020 at 12:19:48PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Hi Christophe, hi Ben,
> 
> This is part II of a larger patch series for multpath-tools I've been preparing.
> It contains some minor fixes for dev_loss_tmo handling, mostly logging.
> It's based on the previously submitted part I.
> 
> The full series will also be available here:
> https://github.com/mwilck/multipath-tools/tree/ups/submit-2007
> 
> There are tags in that repo for each part of the series.
> This part is tagged "submit-200709-2".
> 
> Regards,
> Martin
> 

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
for the part.

> 
> Martin Wilck (4):
>   libmultipath: add macro DEV_LOSS_TMO_UNSET
>   libmultipath: improve logging for dev_loss_tmo override
>   libmultipath: print message if setting dev_loss_tmo is unsupported
>   libmultipath: increase log level of limiting dev_loss_tmo
> 
>  libmultipath/defaults.h  |  1 +
>  libmultipath/dict.c      |  4 +--
>  libmultipath/discovery.c | 56 ++++++++++++++++++++++++++++++----------
>  libmultipath/propsel.c   |  2 +-
>  4 files changed, 46 insertions(+), 17 deletions(-)
> 
> -- 
> 2.26.2

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-17  1:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 10:19 [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes mwilck
2020-07-09 10:19 ` [PATCH 36/39] libmultipath: add macro DEV_LOSS_TMO_UNSET mwilck
2020-07-09 10:19 ` [PATCH 37/39] libmultipath: improve logging for dev_loss_tmo override mwilck
2020-07-09 10:19 ` [PATCH 38/39] libmultipath: print message if setting dev_loss_tmo is unsupported mwilck
2020-07-09 10:19 ` [PATCH 39/39] libmultipath: increase log level of limiting dev_loss_tmo mwilck
2020-07-17  1:05 ` [PATCH 00/39] multipath-tools series part II: dev_loss_tmo fixes Benjamin Marzinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.