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