dm-devel.redhat.com archive mirror
 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 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).