[dm-devel] [PATCH v2 0/7] Add protocol specific config subsection

[dm-devel] [PATCH v2 0/7] Add protocol specific config subsection

[Benjamin Marzinski]

Some storage arrays can be accessed using multiple protocols at the same
time.  I've have customers request the ability to set different values
for the path specific timeouts, like fast_io_fail_tmo, based on the
protocol used to access the path. In order to make this possible, this
patchset adds a new protocol subsection to the overrides section. This
allows users to set a device config's path specific timeouts, such as
dev_loss_tmo, fast_io_fail_tmo, and eh_deadline on a per-protocol basis.

Changes from v1 (all based on suggestions from Martin Wilck):
0003: Adds the protocol subsection to the overrides section instead of
      the device subsection, pulling in code from original patch 0007
0005: Checks the pctable of the overrides section instead of the
      hwes, pulling in code from original patch 0007
Original patches 0006 and 0007 have been removed.
0007: (original patch 0009) modified the man page to no longer
      reference the protocol subsection under the device
      subsection

Benjamin Marzinski (7):
  libmultipath: steal the src string pointer in merge_str()
  libmultipath: make protocol_name global
  libmultipath: add a protocol subsection to multipath.conf
  libmultipath: Set the scsi timeout parameters by path
  libmultipath: check the overrides pctable for path variables
  libmultipath: fix eh_deadline documentation
  libmultipath: Add documentation for the protocol subsection

 libmultipath/config.c      |  99 ++++++++++++++++++---
 libmultipath/config.h      |  10 +++
 libmultipath/configure.c   |   5 +-
 libmultipath/dict.c        |  99 +++++++++++++++++++++
 libmultipath/discovery.c   | 174 +++++++++++++++++++++----------------
 libmultipath/discovery.h   |   2 +-
 libmultipath/print.c       |  67 ++++++++++----
 libmultipath/propsel.c     |  65 +++++++++-----
 libmultipath/propsel.h     |   6 +-
 libmultipath/structs.c     |  19 +++-
 libmultipath/structs.h     |   7 +-
 multipath/multipath.conf.5 |  36 ++++++++
 12 files changed, 452 insertions(+), 137 deletions(-)

-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 1/7] libmultipath: steal the src string pointer in merge_str()

[Benjamin Marzinski]

Instead of allocating a copy when the original string is going to be
freed right after the merge, just steal the pointer. Also, merge_mpe()
can't get called with NULL arguments.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/config.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index c595e768..612941b8 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -387,9 +387,9 @@ set_param_str(const char * str)
 }
 
 #define merge_str(s) \
-	if (!dst->s && src->s) { \
-		if (!(dst->s = set_param_str(src->s))) \
-			return 1; \
+	if (!dst->s && src->s && strlen(src->s)) { \
+		dst->s = src->s; \
+		src->s = NULL; \
 	}
 
 #define merge_num(s) \
@@ -397,7 +397,7 @@ set_param_str(const char * str)
 		dst->s = src->s
 
 
-static int
+static void
 merge_hwe (struct hwentry * dst, struct hwentry * src)
 {
 	char id[SCSI_VENDOR_SIZE+PATH_PRODUCT_SIZE];
@@ -449,15 +449,11 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
 	reconcile_features_with_options(id, &dst->features,
 					&dst->no_path_retry,
 					&dst->retain_hwhandler);
-	return 0;
 }
 
-static int
+static void
 merge_mpe(struct mpentry *dst, struct mpentry *src)
 {
-	if (!dst || !src)
-		return 1;
-
 	merge_str(alias);
 	merge_str(uid_attribute);
 	merge_str(getuid);
@@ -499,8 +495,6 @@ merge_mpe(struct mpentry *dst, struct mpentry *src)
 	merge_num(uid);
 	merge_num(gid);
 	merge_num(mode);
-
-	return 0;
 }
 
 void merge_mptable(vector mptable)
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 2/7] libmultipath: make protocol_name global

[Benjamin Marzinski]

Future patches will make use of this, so move it out of
snprint_path_protocol()

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/print.c   | 17 -----------------
 libmultipath/structs.c | 18 ++++++++++++++++++
 libmultipath/structs.h |  1 +
 3 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index bf88f301..27c2cf1a 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -754,23 +754,6 @@ snprint_path_failures(struct strbuf *buff, const struct path * pp)
 int
 snprint_path_protocol(struct strbuf *buff, const struct path * pp)
 {
-	static const char * const protocol_name[LAST_BUS_PROTOCOL_ID + 1] = {
-		[SYSFS_BUS_UNDEF] = "undef",
-		[SYSFS_BUS_CCW] = "ccw",
-		[SYSFS_BUS_CCISS] = "cciss",
-		[SYSFS_BUS_NVME] = "nvme",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_FCP] = "scsi:fcp",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SPI] = "scsi:spi",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SSA] = "scsi:ssa",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SBP] = "scsi:sbp",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SRP] = "scsi:srp",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_ISCSI] = "scsi:iscsi",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SAS] = "scsi:sas",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_ADT] = "scsi:adt",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_ATA] = "scsi:ata",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_USB] = "scsi:usb",
-		[SYSFS_BUS_SCSI + SCSI_PROTOCOL_UNSPEC] = "scsi:unspec",
-	};
 	const char *pn = protocol_name[bus_protocol_id(pp)];
 
 	assert(pn != NULL);
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 4b62da54..04cfdcdc 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -20,6 +20,24 @@
 #include "dm-generic.h"
 #include "devmapper.h"
 
+const char * const protocol_name[LAST_BUS_PROTOCOL_ID + 1] = {
+	[SYSFS_BUS_UNDEF] = "undef",
+	[SYSFS_BUS_CCW] = "ccw",
+	[SYSFS_BUS_CCISS] = "cciss",
+	[SYSFS_BUS_NVME] = "nvme",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_FCP] = "scsi:fcp",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SPI] = "scsi:spi",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SSA] = "scsi:ssa",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SBP] = "scsi:sbp",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SRP] = "scsi:srp",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_ISCSI] = "scsi:iscsi",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_SAS] = "scsi:sas",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_ADT] = "scsi:adt",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_ATA] = "scsi:ata",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_USB] = "scsi:usb",
+	[SYSFS_BUS_SCSI + SCSI_PROTOCOL_UNSPEC] = "scsi:unspec",
+};
+
 struct adapter_group *
 alloc_adaptergroup(void)
 {
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index d94f93a0..3722e31b 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -192,6 +192,7 @@ enum scsi_protocol {
  */
 #define LAST_BUS_PROTOCOL_ID (SYSFS_BUS_SCSI + SCSI_PROTOCOL_UNSPEC)
 unsigned int bus_protocol_id(const struct path *pp);
+extern const char * const protocol_name[];
 
 #define SCSI_INVALID_LUN ~0ULL
 
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 3/7] libmultipath: add a protocol subsection to multipath.conf

[Benjamin Marzinski]

Some storage arrays can be accessed using multiple protocols at the same
time.  In these cases, users may want to set path attributes
differently, depending on the protocol that the path is using. To allow
this, add a protocol subsection to the overrides section in
multipath.conf, which allows select path-specific options to be set.
This commit simply adds the subsection, and handles merging matching
entries. Future patches will make use of the section.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c | 83 ++++++++++++++++++++++++++++++++++++
 libmultipath/config.h | 10 +++++
 libmultipath/dict.c   | 99 +++++++++++++++++++++++++++++++++++++++++++
 libmultipath/print.c  | 50 ++++++++++++++++++++++
 4 files changed, 242 insertions(+)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 612941b8..5fe71562 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -237,6 +237,18 @@ const char *get_mpe_wwid(const struct _vector *mptable, const char *alias)
 	return NULL;
 }
 
+static void
+free_pctable (vector pctable)
+{
+	int i;
+	struct pcentry *pce;
+
+	vector_foreach_slot(pctable, pce, i)
+		free(pce);
+
+	vector_free(pctable);
+}
+
 void
 free_hwe (struct hwentry * hwe)
 {
@@ -282,6 +294,9 @@ free_hwe (struct hwentry * hwe)
 	if (hwe->bl_product)
 		free(hwe->bl_product);
 
+	if (hwe->pctable)
+		free_pctable(hwe->pctable);
+
 	free(hwe);
 }
 
@@ -363,6 +378,15 @@ alloc_hwe (void)
 	return hwe;
 }
 
+struct pcentry *
+alloc_pce (void)
+{
+	struct pcentry *pce = (struct pcentry *)
+				calloc(1, sizeof(struct pcentry));
+	pce->type = -1;
+	return pce;
+}
+
 static char *
 set_param_str(const char * str)
 {
@@ -396,6 +420,13 @@ set_param_str(const char * str)
 	if (!dst->s && src->s) \
 		dst->s = src->s
 
+static void
+merge_pce(struct pcentry *dst, struct pcentry *src)
+{
+	merge_num(fast_io_fail);
+	merge_num(dev_loss);
+	merge_num(eh_deadline);
+}
 
 static void
 merge_hwe (struct hwentry * dst, struct hwentry * src)
@@ -602,6 +633,51 @@ out:
 	return 1;
 }
 
+static void
+validate_pctable(struct hwentry *ovr, int idx, const char *table_desc)
+{
+	struct pcentry *pce;
+
+	if (!ovr || !ovr->pctable)
+		return;
+
+	vector_foreach_slot_after(ovr->pctable, pce, idx) {
+		if (pce->type < 0) {
+			condlog(0, "protocol section in %s missing type",
+				table_desc);
+			vector_del_slot(ovr->pctable, idx--);
+			free(pce);
+		}
+	}
+
+	if (VECTOR_SIZE(ovr->pctable) == 0) {
+		vector_free(ovr->pctable);
+		ovr->pctable = NULL;
+	}
+}
+
+static void
+merge_pctable(struct hwentry *ovr)
+{
+	struct pcentry *pce1, *pce2;
+	int i, j;
+
+	if (!ovr || !ovr->pctable)
+		return;
+
+	vector_foreach_slot(ovr->pctable, pce1, i) {
+		j = i + 1;
+		vector_foreach_slot_after(ovr->pctable, pce2, j) {
+			if (pce1->type != pce2->type)
+				continue;
+			merge_pce(pce2,pce1);
+			vector_del_slot(ovr->pctable, i--);
+			free(pce1);
+			break;
+		}
+	}
+}
+
 static void
 factorize_hwtable (vector hw, int n, const char *table_desc)
 {
@@ -750,6 +826,7 @@ process_config_dir(struct config *conf, char *dir)
 	int i, n;
 	char path[LINE_MAX];
 	int old_hwtable_size;
+	int old_pctable_size = 0;
 
 	if (dir[0] != '/') {
 		condlog(1, "config_dir '%s' must be a fully qualified path",
@@ -776,11 +853,15 @@ process_config_dir(struct config *conf, char *dir)
 			continue;
 
 		old_hwtable_size = VECTOR_SIZE(conf->hwtable);
+		old_pctable_size = conf->overrides ?
+				   VECTOR_SIZE(conf->overrides->pctable) : 0;
 		snprintf(path, LINE_MAX, "%s/%s", dir, namelist[i]->d_name);
 		path[LINE_MAX-1] = '\0';
 		process_file(conf, path);
 		factorize_hwtable(conf->hwtable, old_hwtable_size,
 				  namelist[i]->d_name);
+		validate_pctable(conf->overrides, old_pctable_size,
+				 namelist[i]->d_name);
 	}
 	pthread_cleanup_pop(1);
 }
@@ -888,6 +969,7 @@ int _init_config (const char *file, struct config *conf)
 			goto out;
 		}
 		factorize_hwtable(conf->hwtable, builtin_hwtable_size, file);
+		validate_pctable(conf->overrides, 0, file);
 	}
 
 	conf->processed_main_config = 1;
@@ -988,6 +1070,7 @@ int _init_config (const char *file, struct config *conf)
 			goto out;
 	}
 
+	merge_pctable(conf->overrides);
 	merge_mptable(conf->mptable);
 	merge_blacklist(conf->blist_devnode);
 	merge_blacklist(conf->blist_property);
diff --git a/libmultipath/config.h b/libmultipath/config.h
index c73389b5..b7bca9a8 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -40,6 +40,13 @@ enum force_reload_types {
 	FORCE_RELOAD_WEAK,
 };
 
+struct pcentry {
+	int type;
+	int fast_io_fail;
+	unsigned int dev_loss;
+	int eh_deadline;
+};
+
 struct hwentry {
 	char * vendor;
 	char * product;
@@ -85,6 +92,8 @@ struct hwentry {
 	int vpd_vendor_id;
 	int recheck_wwid;
 	char * bl_product;
+
+	vector pctable;
 };
 
 struct mpentry {
@@ -284,6 +293,7 @@ const char *get_mpe_wwid (const struct _vector *mptable, const char *alias);
 
 struct hwentry * alloc_hwe (void);
 struct mpentry * alloc_mpe (void);
+struct pcentry * alloc_pce (void);
 
 void free_hwe (struct hwentry * hwe);
 void free_hwtable (vector hwtable);
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 26cbe785..8e11fd70 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -417,6 +417,29 @@ snprint_mp_ ## option (struct config *conf, struct strbuf *buff,	\
 	return function(buff, mpe->option);				\
 }
 
+#define declare_pc_handler(option, function)				\
+static int								\
+pc_ ## option ## _handler (struct config *conf, vector strvec,		\
+			   const char *file, int line_nr)		\
+{									\
+	struct pcentry *pce;						\
+	if (!conf->overrides || !conf->overrides->pctable)		\
+		return 1;						\
+	pce = VECTOR_LAST_SLOT(conf->overrides->pctable);		\
+	if (!pce)							\
+		return 1;						\
+	return function (strvec, &pce->option, file, line_nr);		\
+}
+
+#define declare_pc_snprint(option, function)				\
+static int								\
+snprint_pc_ ## option (struct config *conf, struct strbuf *buff,	\
+		       const void *data)				\
+{									\
+	const struct pcentry *pce  = (const struct pcentry *)data;	\
+	return function(buff, pce->option);				\
+}
+
 static int checkint_handler(struct config *conf, vector strvec,
 			    const char *file, int line_nr)
 {
@@ -1046,6 +1069,8 @@ declare_ovr_handler(fast_io_fail, set_undef_off_zero)
 declare_ovr_snprint(fast_io_fail, print_undef_off_zero)
 declare_hw_handler(fast_io_fail, set_undef_off_zero)
 declare_hw_snprint(fast_io_fail, print_undef_off_zero)
+declare_pc_handler(fast_io_fail, set_undef_off_zero)
+declare_pc_snprint(fast_io_fail, print_undef_off_zero)
 
 static int
 set_dev_loss(vector strvec, void *ptr, const char *file, int line_nr)
@@ -1083,6 +1108,8 @@ declare_ovr_handler(dev_loss, set_dev_loss)
 declare_ovr_snprint(dev_loss, print_dev_loss)
 declare_hw_handler(dev_loss, set_dev_loss)
 declare_hw_snprint(dev_loss, print_dev_loss)
+declare_pc_handler(dev_loss, set_dev_loss)
+declare_pc_snprint(dev_loss, print_dev_loss)
 
 declare_def_handler(eh_deadline, set_undef_off_zero)
 declare_def_snprint(eh_deadline, print_undef_off_zero)
@@ -1090,6 +1117,8 @@ declare_ovr_handler(eh_deadline, set_undef_off_zero)
 declare_ovr_snprint(eh_deadline, print_undef_off_zero)
 declare_hw_handler(eh_deadline, set_undef_off_zero)
 declare_hw_snprint(eh_deadline, print_undef_off_zero)
+declare_pc_handler(eh_deadline, set_undef_off_zero)
+declare_pc_snprint(eh_deadline, print_undef_off_zero)
 
 static int
 set_pgpolicy(vector strvec, void *ptr, const char *file, int line_nr)
@@ -1897,6 +1926,69 @@ declare_mp_snprint(wwid, print_str)
 declare_mp_handler(alias, set_str_noslash)
 declare_mp_snprint(alias, print_str)
 
+
+static int
+protocol_handler(struct config *conf, vector strvec, const char *file,
+               int line_nr)
+{
+	struct pcentry *pce;
+
+	if (!conf->overrides)
+		return 1;
+
+	if (!conf->overrides->pctable &&
+	    !(conf->overrides->pctable = vector_alloc()))
+		return 1;
+
+	if (!(pce = alloc_pce()))
+		return 1;
+
+	if (!vector_alloc_slot(conf->overrides->pctable)) {
+		free(pce);
+		return 1;
+	}
+	vector_set_slot(conf->overrides->pctable, pce);
+
+	return 0;
+}
+
+static int
+set_protocol_type(vector strvec, void *ptr, const char *file, int line_nr)
+{
+	int *int_ptr = (int *)ptr;
+	char *buff;
+	int i;
+
+	buff = set_value(strvec);
+
+	if (!buff)
+		return 1;
+
+	for (i = 0; i <= LAST_BUS_PROTOCOL_ID; i++) {
+		if (protocol_name[i] && !strcmp(buff, protocol_name[i])) {
+			*int_ptr = i;
+			break;
+		}
+	}
+	if (i > LAST_BUS_PROTOCOL_ID)
+		condlog(1, "%s line %d, invalid value for type: \"%s\"",
+			file, line_nr, buff);
+
+	free(buff);
+	return 0;
+}
+
+static int
+print_protocol_type(struct strbuf *buff, int type)
+{
+	if (type < 0)
+		return 0;
+	return append_strbuf_quoted(buff, protocol_name[type]);
+}
+
+declare_pc_handler(type, set_protocol_type)
+declare_pc_snprint(type, print_protocol_type)
+
 /*
  * deprecated handlers
  */
@@ -2138,6 +2230,13 @@ init_keywords(vector keywords)
 	install_keyword("ghost_delay", &ovr_ghost_delay_handler, &snprint_ovr_ghost_delay);
 	install_keyword("all_tg_pt", &ovr_all_tg_pt_handler, &snprint_ovr_all_tg_pt);
 	install_keyword("recheck_wwid", &ovr_recheck_wwid_handler, &snprint_ovr_recheck_wwid);
+	install_keyword_multi("protocol", &protocol_handler, NULL);
+	install_sublevel();
+	install_keyword("type", &pc_type_handler, &snprint_pc_type);
+	install_keyword("fast_io_fail_tmo", &pc_fast_io_fail_handler, &snprint_pc_fast_io_fail);
+	install_keyword("dev_loss_tmo", &pc_dev_loss_handler, &snprint_pc_dev_loss);
+	install_keyword("eh_deadline", &pc_eh_deadline_handler, &snprint_pc_eh_deadline);
+	install_sublevel_end();
 
 	install_keyword_root("multipaths", &multipaths_handler);
 	install_keyword_multi("multipath", &multipath_handler, NULL);
diff --git a/libmultipath/print.c b/libmultipath/print.c
index 27c2cf1a..68a793e7 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1406,6 +1406,52 @@ int snprint_multipath_topology_json (struct strbuf *buff,
 	return get_strbuf_len(buff) - initial_len;
 }
 
+static int
+snprint_pcentry (const struct config *conf, struct strbuf *buff,
+		 const struct pcentry *pce)
+{
+	int i, rc;
+	struct keyword *kw;
+	struct keyword * rootkw;
+	size_t initial_len = get_strbuf_len(buff);
+
+	rootkw = find_keyword(conf->keywords, NULL, "overrides");
+	assert(rootkw && rootkw->sub);
+	rootkw = find_keyword(conf->keywords, rootkw->sub, "protocol");
+	assert(rootkw);
+
+	if ((rc = append_strbuf_str(buff, "\tprotocol {\n")) < 0)
+		return rc;
+
+	iterate_sub_keywords(rootkw, kw, i) {
+		if ((rc = snprint_keyword(buff, "\t\t%k %v\n", kw, pce)) < 0)
+			return rc;
+	}
+
+	if ((rc = append_strbuf_str(buff, "\t}\n")) < 0)
+		return rc;
+	return get_strbuf_len(buff) - initial_len;
+}
+
+static int
+snprint_pctable (const struct config *conf, struct strbuf *buff,
+		 const struct _vector *pctable)
+{
+	int i, rc;
+	struct pcentry *pce;
+	struct keyword * rootkw;
+	size_t initial_len = get_strbuf_len(buff);
+
+	rootkw = find_keyword(conf->keywords, NULL, "overrides");
+	assert(rootkw);
+
+	vector_foreach_slot(pctable, pce, i) {
+		if ((rc = snprint_pcentry(conf, buff, pce)) < 0)
+			return rc;
+	}
+	return get_strbuf_len(buff) - initial_len;
+}
+
 static int
 snprint_hwentry (const struct config *conf,
 		 struct strbuf *buff, const struct hwentry * hwe)
@@ -1560,6 +1606,10 @@ static int snprint_overrides(const struct config *conf, struct strbuf *buff,
 		if ((rc = snprint_keyword(buff, "\t%k %v\n", kw, NULL)) < 0)
 			return rc;
 	}
+
+	if (overrides->pctable &&
+	    (rc = snprint_pctable(conf, buff, overrides->pctable)) < 0)
+		return rc;
 out:
 	if ((rc = append_strbuf_str(buff, "}\n")) < 0)
 		return rc;
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 4/7] libmultipath: Set the scsi timeout parameters by path

[Benjamin Marzinski]

Instead of dev_loss, fast_io_fail, and eh_deadline belonging to the
multipath structure, have them belong to the path structure. This means
that they are selected per path, and that sysfs_set_scsi_tmo() doesn't
assume that all paths of a multipath device will have the same value.
Currently they will all be the same, but a future patch will make it
possible for paths to have different values based on their protocol.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/configure.c |   5 +-
 libmultipath/discovery.c | 174 ++++++++++++++++++++++-----------------
 libmultipath/discovery.h |   2 +-
 libmultipath/propsel.c   |  42 +++++-----
 libmultipath/propsel.h   |   6 +-
 libmultipath/structs.c   |   1 -
 libmultipath/structs.h   |   6 +-
 7 files changed, 127 insertions(+), 109 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index eca11ba0..09ae708d 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -329,9 +329,6 @@ int setup_map(struct multipath *mpp, char **params, struct vectors *vecs)
 	select_mode(conf, mpp);
 	select_uid(conf, mpp);
 	select_gid(conf, mpp);
-	select_fast_io_fail(conf, mpp);
-	select_dev_loss(conf, mpp);
-	select_eh_deadline(conf, mpp);
 	select_reservation_key(conf, mpp);
 	select_deferred_remove(conf, mpp);
 	select_marginal_path_err_sample_time(conf, mpp);
@@ -347,7 +344,7 @@ int setup_map(struct multipath *mpp, char **params, struct vectors *vecs)
 	select_ghost_delay(conf, mpp);
 	select_flush_on_last_del(conf, mpp);
 
-	sysfs_set_scsi_tmo(mpp, conf->checkint);
+	sysfs_set_scsi_tmo(conf, mpp);
 	marginal_pathgroups = conf->marginal_pathgroups;
 	pthread_cleanup_pop(1);
 
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index b969fba1..c6ba1967 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -598,13 +598,13 @@ sysfs_get_asymmetric_access_state(struct path *pp, char *buff, int buflen)
 }
 
 static int
-sysfs_set_eh_deadline(struct multipath *mpp, struct path *pp)
+sysfs_set_eh_deadline(struct path *pp)
 {
 	struct udev_device *hostdev;
 	char host_name[HOST_NAME_LEN], value[16];
 	int ret, len;
 
-	if (mpp->eh_deadline == EH_DEADLINE_UNSET)
+	if (pp->eh_deadline == EH_DEADLINE_UNSET)
 		return 0;
 
 	sprintf(host_name, "host%d", pp->sg_id.host_no);
@@ -613,12 +613,12 @@ sysfs_set_eh_deadline(struct multipath *mpp, struct path *pp)
 	if (!hostdev)
 		return 1;
 
-	if (mpp->eh_deadline == EH_DEADLINE_OFF)
+	if (pp->eh_deadline == EH_DEADLINE_OFF)
 		len = sprintf(value, "off");
-	else if (mpp->eh_deadline == EH_DEADLINE_ZERO)
+	else if (pp->eh_deadline == EH_DEADLINE_ZERO)
 		len = sprintf(value, "0");
 	else
-		len = sprintf(value, "%d", mpp->eh_deadline);
+		len = sprintf(value, "%d", pp->eh_deadline);
 
 	ret = sysfs_attr_set_value(hostdev, "eh_deadline",
 				   value, len + 1);
@@ -642,8 +642,8 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	unsigned int tmo;
 	int ret;
 
-	if (mpp->dev_loss == DEV_LOSS_TMO_UNSET &&
-	    mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
+	if (pp->dev_loss == DEV_LOSS_TMO_UNSET &&
+	    pp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
 		return;
 
 	sprintf(rport_id, "rport-%d:%d-%d",
@@ -685,14 +685,14 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	 * then set fast_io_fail, and _then_ set dev_loss_tmo
 	 * to the correct value.
 	 */
-	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
-	    mpp->fast_io_fail != MP_FAST_IO_FAIL_ZERO &&
-	    mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
+	if (pp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
+	    pp->fast_io_fail != MP_FAST_IO_FAIL_ZERO &&
+	    pp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
 		/* Check if we need to temporarily increase dev_loss_tmo */
-		if ((unsigned int)mpp->fast_io_fail >= tmo) {
+		if ((unsigned int)pp->fast_io_fail >= tmo) {
 			/* Increase dev_loss_tmo temporarily */
 			snprintf(value, sizeof(value), "%u",
-				 (unsigned int)mpp->fast_io_fail + 1);
+				 (unsigned int)pp->fast_io_fail + 1);
 			ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
 						   value, strlen(value));
 			if (ret <= 0) {
@@ -706,20 +706,20 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 				goto out;
 			}
 		}
-	} else if (mpp->dev_loss > DEFAULT_DEV_LOSS_TMO &&
-		mpp->no_path_retry != NO_PATH_RETRY_QUEUE) {
+	} else if (pp->dev_loss > DEFAULT_DEV_LOSS_TMO &&
+		   mpp->no_path_retry != NO_PATH_RETRY_QUEUE) {
 		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;
+		pp->dev_loss = DEFAULT_DEV_LOSS_TMO;
 	}
-	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
-		if (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
+	if (pp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
+		if (pp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
 			sprintf(value, "off");
-		else if (mpp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
+		else if (pp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
 			sprintf(value, "0");
 		else
-			snprintf(value, 16, "%u", mpp->fast_io_fail);
+			snprintf(value, 16, "%u", pp->fast_io_fail);
 		ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
 					   value, strlen(value));
 		if (ret <= 0) {
@@ -730,8 +730,8 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 					rport_id, value, -ret);
 		}
 	}
-	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET) {
-		snprintf(value, 16, "%u", mpp->dev_loss);
+	if (pp->dev_loss != DEV_LOSS_TMO_UNSET) {
+		snprintf(value, 16, "%u", pp->dev_loss);
 		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
 					   value, strlen(value));
 		if (ret <= 0) {
@@ -747,15 +747,15 @@ out:
 }
 
 static void
-sysfs_set_session_tmo(struct multipath *mpp, struct path *pp)
+sysfs_set_session_tmo(struct path *pp)
 {
 	struct udev_device *session_dev = NULL;
 	char session_id[64];
 	char value[11];
 
-	if (mpp->dev_loss != DEV_LOSS_TMO_UNSET)
+	if (pp->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)
+	if (pp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
 		return;
 
 	sprintf(session_id, "session%d", pp->sg_id.transport_id);
@@ -769,15 +769,15 @@ 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->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
-		if (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF) {
+	if (pp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
+		if (pp->fast_io_fail == MP_FAST_IO_FAIL_OFF) {
 			condlog(3, "%s: can't switch off fast_io_fail_tmo "
 				"on iSCSI", pp->dev);
-		} else if (mpp->fast_io_fail == MP_FAST_IO_FAIL_ZERO) {
+		} else if (pp->fast_io_fail == MP_FAST_IO_FAIL_ZERO) {
 			condlog(3, "%s: can't set fast_io_fail_tmo to '0'"
 				"on iSCSI", pp->dev);
 		} else {
-			snprintf(value, 11, "%u", mpp->fast_io_fail);
+			snprintf(value, 11, "%u", pp->fast_io_fail);
 			if (sysfs_attr_set_value(session_dev, "recovery_tmo",
 						 value, strlen(value)) <= 0) {
 				condlog(3, "%s: Failed to set recovery_tmo, "
@@ -790,14 +790,14 @@ sysfs_set_session_tmo(struct multipath *mpp, struct path *pp)
 }
 
 static void
-sysfs_set_nexus_loss_tmo(struct multipath *mpp, struct path *pp)
+sysfs_set_nexus_loss_tmo(struct path *pp)
 {
 	struct udev_device *parent, *sas_dev = NULL;
 	const char *end_dev_id = NULL;
 	char value[11];
 	static const char ed_str[] = "end_device-";
 
-	if (!pp->udev || mpp->dev_loss == DEV_LOSS_TMO_UNSET)
+	if (!pp->udev || pp->dev_loss == DEV_LOSS_TMO_UNSET)
 		return;
 
 	for (parent = udev_device_get_parent(pp->udev);
@@ -824,8 +824,8 @@ 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 != DEV_LOSS_TMO_UNSET) {
-		snprintf(value, 11, "%u", mpp->dev_loss);
+	if (pp->dev_loss != DEV_LOSS_TMO_UNSET) {
+		snprintf(value, 11, "%u", pp->dev_loss);
 		if (sysfs_attr_set_value(sas_dev, "I_T_nexus_loss_timeout",
 					 value, strlen(value)) <= 0)
 			condlog(3, "%s: failed to update "
@@ -836,76 +836,98 @@ sysfs_set_nexus_loss_tmo(struct multipath *mpp, struct path *pp)
 	return;
 }
 
+static void
+scsi_tmo_error_msg(struct path *pp)
+{
+	STATIC_BITFIELD(bf, LAST_BUS_PROTOCOL_ID + 1);
+	STRBUF_ON_STACK(proto_buf);
+	unsigned int proto_id = bus_protocol_id(pp);
+
+	snprint_path_protocol(&proto_buf, pp);
+	condlog(2, "%s: setting scsi timeouts is unsupported for protocol %s",
+		pp->dev, get_strbuf_str(&proto_buf));
+	set_bit_in_bitfield(proto_id, bf);
+}
+
 int
-sysfs_set_scsi_tmo (struct multipath *mpp, unsigned int checkint)
+sysfs_set_scsi_tmo (struct config *conf, struct multipath *mpp)
 {
 	struct path *pp;
 	int i;
-	unsigned int dev_loss_tmo = mpp->dev_loss;
-	struct path *err_path = NULL;
-	STATIC_BITFIELD(bf, LAST_BUS_PROTOCOL_ID + 1);
+	unsigned int min_dev_loss = 0;
+	bool warn_dev_loss = false;
+	bool warn_fast_io_fail = false;
 
 	if (mpp->no_path_retry > 0) {
 		uint64_t no_path_retry_tmo =
-			(uint64_t)mpp->no_path_retry * checkint;
+			(uint64_t)mpp->no_path_retry * conf->checkint;
 
 		if (no_path_retry_tmo > MAX_DEV_LOSS_TMO)
-			no_path_retry_tmo = MAX_DEV_LOSS_TMO;
-		if (no_path_retry_tmo > dev_loss_tmo)
-			dev_loss_tmo = no_path_retry_tmo;
-	} else if (mpp->no_path_retry == NO_PATH_RETRY_QUEUE) {
-		dev_loss_tmo = MAX_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) {
-		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 == DEV_LOSS_TMO_UNSET &&
-	    mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET &&
-	    mpp->eh_deadline == EH_DEADLINE_UNSET)
-		return 0;
+			min_dev_loss = MAX_DEV_LOSS_TMO;
+		else
+			min_dev_loss = no_path_retry_tmo;
+	} else if (mpp->no_path_retry == NO_PATH_RETRY_QUEUE)
+		min_dev_loss = MAX_DEV_LOSS_TMO;
 
 	vector_foreach_slot(mpp->paths, pp, i) {
+		select_fast_io_fail(conf, pp);
+		select_dev_loss(conf, pp);
+		select_eh_deadline(conf, pp);
+
+		if (pp->dev_loss == DEV_LOSS_TMO_UNSET &&
+		    pp->fast_io_fail == MP_FAST_IO_FAIL_UNSET &&
+		    pp->eh_deadline == EH_DEADLINE_UNSET)
+			continue;
+
 		if (pp->bus != SYSFS_BUS_SCSI) {
-			if (!err_path)
-				err_path = pp;
+			scsi_tmo_error_msg(pp);
 			continue;
 		}
+		sysfs_set_eh_deadline(pp);
+
+		if (pp->dev_loss == DEV_LOSS_TMO_UNSET &&
+		    pp->fast_io_fail == MP_FAST_IO_FAIL_UNSET)
+			continue;
+
+		if (pp->sg_id.proto_id != SCSI_PROTOCOL_FCP &&
+		    pp->sg_id.proto_id != SCSI_PROTOCOL_ISCSI &&
+		    pp->sg_id.proto_id != SCSI_PROTOCOL_SAS) {
+			scsi_tmo_error_msg(pp);
+			continue;
+		}
+
+		if (pp->dev_loss != DEV_LOSS_TMO_UNSET &&
+		    pp->dev_loss < min_dev_loss) {
+			warn_dev_loss = true;
+			pp->dev_loss = min_dev_loss;
+		}
+		if (pp->dev_loss != DEV_LOSS_TMO_UNSET &&
+		    pp->fast_io_fail > 0 &&
+		    (unsigned int)pp->fast_io_fail >= pp->dev_loss) {
+			warn_fast_io_fail = true;
+			pp->fast_io_fail = MP_FAST_IO_FAIL_OFF;
+		}
 
 		switch (pp->sg_id.proto_id) {
 		case SCSI_PROTOCOL_FCP:
 			sysfs_set_rport_tmo(mpp, pp);
 			break;
 		case SCSI_PROTOCOL_ISCSI:
-			sysfs_set_session_tmo(mpp, pp);
+			sysfs_set_session_tmo(pp);
 			break;
 		case SCSI_PROTOCOL_SAS:
-			sysfs_set_nexus_loss_tmo(mpp, pp);
+			sysfs_set_nexus_loss_tmo(pp);
 			break;
 		default:
-			if (!err_path)
-				err_path = pp;
+			break;
 		}
-		sysfs_set_eh_deadline(mpp, pp);
-	}
-
-	if (err_path && !is_bit_set_in_bitfield(bus_protocol_id(pp), bf)) {
-		STRBUF_ON_STACK(proto_buf);
-
-		snprint_path_protocol(&proto_buf, err_path);
-		condlog(2, "%s: setting dev_loss_tmo is unsupported for protocol %s",
-			mpp->alias, get_strbuf_str(&proto_buf));
-		set_bit_in_bitfield(bus_protocol_id(pp), bf);
 	}
+	if (warn_dev_loss)
+		condlog(2, "%s: Raising dev_loss_tmo to %u because of no_path_retry setting",
+			mpp->alias, min_dev_loss);
+	if (warn_fast_io_fail)
+		condlog(3, "%s: turning off fast_io_fail (not smaller than dev_loss_tmo)",
+			mpp->alias);
 	return 0;
 }
 
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index 466af345..acd51792 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -42,7 +42,7 @@ int alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
 int store_pathinfo (vector pathvec, struct config *conf,
 		    struct udev_device *udevice, int flag,
 		    struct path **pp_ptr);
-int sysfs_set_scsi_tmo (struct multipath *mpp, unsigned int checkint);
+int sysfs_set_scsi_tmo (struct config *conf, struct multipath *mpp);
 int sysfs_get_timeout(const struct path *pp, unsigned int *timeout);
 int sysfs_get_iscsi_ip_address(const struct path *pp, char *ip_address);
 int sysfs_get_host_adapter_name(const struct path *pp,
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 1419ec6f..d2d70090 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -769,53 +769,53 @@ int select_minio(struct config *conf, struct multipath *mp)
 		return select_minio_bio(conf, mp);
 }
 
-int select_fast_io_fail(struct config *conf, struct multipath *mp)
+int select_fast_io_fail(struct config *conf, struct path *pp)
 {
 	const char *origin;
 	STRBUF_ON_STACK(buff);
 
-	mp_set_ovr(fast_io_fail);
-	mp_set_hwe(fast_io_fail);
-	mp_set_conf(fast_io_fail);
-	mp_set_default(fast_io_fail, DEFAULT_FAST_IO_FAIL);
+	pp_set_ovr(fast_io_fail);
+	pp_set_hwe(fast_io_fail);
+	pp_set_conf(fast_io_fail);
+	pp_set_default(fast_io_fail, DEFAULT_FAST_IO_FAIL);
 out:
-	print_undef_off_zero(&buff, mp->fast_io_fail);
-	condlog(3, "%s: fast_io_fail_tmo = %s %s", mp->alias,
+	print_undef_off_zero(&buff, pp->fast_io_fail);
+	condlog(3, "%s: fast_io_fail_tmo = %s %s", pp->dev,
 		get_strbuf_str(&buff), origin);
 	return 0;
 }
 
-int select_dev_loss(struct config *conf, struct multipath *mp)
+int select_dev_loss(struct config *conf, struct path *pp)
 {
 	const char *origin;
 	STRBUF_ON_STACK(buff);
 
-	mp_set_ovr(dev_loss);
-	mp_set_hwe(dev_loss);
-	mp_set_conf(dev_loss);
-	mp->dev_loss = DEV_LOSS_TMO_UNSET;
+	pp_set_ovr(dev_loss);
+	pp_set_hwe(dev_loss);
+	pp_set_conf(dev_loss);
+	pp->dev_loss = DEV_LOSS_TMO_UNSET;
 	return 0;
 out:
-	print_dev_loss(&buff, mp->dev_loss);
-	condlog(3, "%s: dev_loss_tmo = %s %s", mp->alias,
+	print_dev_loss(&buff, pp->dev_loss);
+	condlog(3, "%s: dev_loss_tmo = %s %s", pp->dev,
 		get_strbuf_str(&buff), origin);
 	return 0;
 }
 
-int select_eh_deadline(struct config *conf, struct multipath *mp)
+int select_eh_deadline(struct config *conf, struct path *pp)
 {
 	const char *origin;
 	STRBUF_ON_STACK(buff);
 
-	mp_set_ovr(eh_deadline);
-	mp_set_hwe(eh_deadline);
-	mp_set_conf(eh_deadline);
-	mp->eh_deadline = EH_DEADLINE_UNSET;
+	pp_set_ovr(eh_deadline);
+	pp_set_hwe(eh_deadline);
+	pp_set_conf(eh_deadline);
+	pp->eh_deadline = EH_DEADLINE_UNSET;
 	/* not changing sysfs in default cause, so don't print anything */
 	return 0;
 out:
-	print_undef_off_zero(&buff, mp->eh_deadline);
-	condlog(3, "%s: eh_deadline = %s %s", mp->alias,
+	print_undef_off_zero(&buff, pp->eh_deadline);
+	condlog(3, "%s: eh_deadline = %s %s", pp->dev,
 		get_strbuf_str(&buff), origin);
 	return 0;
 }
diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
index 72a7e33c..152ca44c 100644
--- a/libmultipath/propsel.h
+++ b/libmultipath/propsel.h
@@ -16,9 +16,9 @@ int select_minio(struct config *conf, struct multipath *mp);
 int select_mode(struct config *conf, struct multipath *mp);
 int select_uid(struct config *conf, struct multipath *mp);
 int select_gid(struct config *conf, struct multipath *mp);
-int select_fast_io_fail(struct config *conf, struct multipath *mp);
-int select_dev_loss(struct config *conf, struct multipath *mp);
-int select_eh_deadline(struct config *conf, struct multipath *mp);
+int select_fast_io_fail(struct config *conf, struct path *pp);
+int select_dev_loss(struct config *conf, struct path *pp);
+int select_eh_deadline(struct config *conf, struct path *pp);
 int select_reservation_key(struct config *conf, struct multipath *mp);
 int select_retain_hwhandler (struct config *conf, struct multipath * mp);
 int select_detect_prio(struct config *conf, struct path * pp);
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index 04cfdcdc..2c9be041 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -246,7 +246,6 @@ alloc_multipath (void)
 		mpp->bestpg = 1;
 		mpp->mpcontext = NULL;
 		mpp->no_path_retry = NO_PATH_RETRY_UNDEF;
-		mpp->fast_io_fail = MP_FAST_IO_FAIL_UNSET;
 		dm_multipath_to_gen(mpp)->ops = &dm_gen_multipath_ops;
 	}
 	return mpp;
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 3722e31b..a6749367 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -349,6 +349,9 @@ struct path {
 	int marginal;
 	int vpd_vendor_id;
 	int recheck_wwid;
+	int fast_io_fail;
+	unsigned int dev_loss;
+	int eh_deadline;
 	/* configlet pointers */
 	vector hwe;
 	struct gen_path generic_path;
@@ -376,7 +379,6 @@ struct multipath {
 	int minio;
 	int flush_on_last_del;
 	int attribute_flags;
-	int fast_io_fail;
 	int retain_hwhandler;
 	int deferred_remove;
 	bool in_recovery;
@@ -395,8 +397,6 @@ struct multipath {
 	int needs_paths_uevent;
 	int ghost_delay;
 	int ghost_delay_tick;
-	unsigned int dev_loss;
-	int eh_deadline;
 	uid_t uid;
 	gid_t gid;
 	mode_t mode;
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 5/7] libmultipath: check the overrides pctable for path variables

[Benjamin Marzinski]

Paths will now also check the pctable when checking for attribtes that
exists in both the overrides section and the protocol subsection. Values
in a matching pcentry will be used in preference to values in the
overrides hwentry.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/propsel.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index d2d70090..72b42991 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -79,6 +79,8 @@ static const char conf_origin[] =
 	"(setting: multipath.conf defaults/devices section)";
 static const char overrides_origin[] =
 	"(setting: multipath.conf overrides section)";
+static const char overrides_pce_origin[] =
+	"(setting: multipath.conf overrides protocol section)";
 static const char cmdline_origin[] =
 	"(setting: multipath command line [-p] flag)";
 static const char autodetect_origin[] =
@@ -146,6 +148,27 @@ do {									\
 	}								\
 } while (0)
 
+#define pp_set_ovr_pce(var)						\
+do {									\
+	struct pcentry *_pce;						\
+	int _i;								\
+									\
+	if (conf->overrides) {						\
+		vector_foreach_slot(conf->overrides->pctable, _pce, _i) {	\
+			if (_pce->type == (int)bus_protocol_id(pp) && _pce->var) {	\
+				pp->var = _pce->var;			\
+				origin = overrides_pce_origin;		\
+				goto out;				\
+			}						\
+		}							\
+		if (conf->overrides->var) {				\
+			pp->var = conf->overrides->var;			\
+			origin = overrides_origin;			\
+			goto out;					\
+		}							\
+	}								\
+} while (0)
+
 int select_mode(struct config *conf, struct multipath *mp)
 {
 	const char *origin;
@@ -774,7 +797,7 @@ int select_fast_io_fail(struct config *conf, struct path *pp)
 	const char *origin;
 	STRBUF_ON_STACK(buff);
 
-	pp_set_ovr(fast_io_fail);
+	pp_set_ovr_pce(fast_io_fail);
 	pp_set_hwe(fast_io_fail);
 	pp_set_conf(fast_io_fail);
 	pp_set_default(fast_io_fail, DEFAULT_FAST_IO_FAIL);
@@ -790,7 +813,7 @@ int select_dev_loss(struct config *conf, struct path *pp)
 	const char *origin;
 	STRBUF_ON_STACK(buff);
 
-	pp_set_ovr(dev_loss);
+	pp_set_ovr_pce(dev_loss);
 	pp_set_hwe(dev_loss);
 	pp_set_conf(dev_loss);
 	pp->dev_loss = DEV_LOSS_TMO_UNSET;
@@ -807,7 +830,7 @@ int select_eh_deadline(struct config *conf, struct path *pp)
 	const char *origin;
 	STRBUF_ON_STACK(buff);
 
-	pp_set_ovr(eh_deadline);
+	pp_set_ovr_pce(eh_deadline);
 	pp_set_hwe(eh_deadline);
 	pp_set_conf(eh_deadline);
 	pp->eh_deadline = EH_DEADLINE_UNSET;
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 6/7] libmultipath: fix eh_deadline documentation

[Benjamin Marzinski]

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipath/multipath.conf.5 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 605b46e0..97695da4 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1636,6 +1636,8 @@ section:
 .TP
 .B dev_loss_tmo
 .TP
+.B eh_deadline
+.TP
 .B flush_on_last_del
 .TP
 .B user_friendly_names
@@ -1722,6 +1724,8 @@ the values are taken from the \fIdevices\fR or \fIdefaults\fR sections:
 .TP
 .B dev_loss_tmo
 .TP
+.B eh_deadline
+.TP
 .B user_friendly_names
 .TP
 .B retain_attached_hw_handler
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH v2 7/7] libmultipath: Add documentation for the protocol subsection

[Benjamin Marzinski]

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipath/multipath.conf.5 | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 97695da4..198a79dd 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1764,6 +1764,38 @@ the values are taken from the \fIdevices\fR or \fIdefaults\fR sections:
 .RE
 .PD
 .LP
+The overrides section also recognizes the optional \fIprotocol\fR subsection,
+and can contain multiple protocol subsections. Path devices are matched against
+the protocol subsection using the mandatory \fItype\fR attribute.  Attributes
+in a matching protocol subsection take precedence over attributes in the rest
+of the overrides section. If there are multiple matching protocol subsections,
+later entries take precedence.
+.TP
+.B protocol subsection
+The protocol subsection recognizes the following mandatory attribute:
+.RS
+.TP
+.B type
+The protocol string of the path device. The possible values are \fIscsi:fcp\fR,
+\fIscsi:spi\fR, \fIscsi:ssa\fR, \fIscsi:sbp\fR, \fIscsi:srp\fR,
+\fIscsi:iscsi\fR, \fIscsi:sas\fR, \fIscsi:adt\fR, \fIscsi:ata\fR,
+\fIscsi:unspec\fR, \fIccw\fR, \fIcciss\fR, \fInvme\fR, and \fIundef\fR. This is
+\fBnot\fR a regular expression. the path device protcol string must match
+exactly. The protocol that a path is using can be viewed by running
+\fBmultipathd show paths format "%d %P"\fR
+.LP
+The following attributes are optional; if not set, the default values are taken
+from the \fIoverrides\fR, \fIdevices\fR, or \fIdefaults\fR section:
+.sp 1
+.PD .1v
+.RS
+.TP
+.B fast_io_fail_tmo
+.TP
+.B dev_loss_tmo
+.TP
+.B eh_deadline
+.PD
 .
 .
 .\" ----------------------------------------------------------------------------
-- 
2.17.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2 3/7] libmultipath: add a protocol subsection to multipath.conf

[Martin Wilck]

On Wed, 2022-04-13 at 23:27 -0500, Benjamin Marzinski wrote:
> Some storage arrays can be accessed using multiple protocols at the
> same
> time.  In these cases, users may want to set path attributes
> differently, depending on the protocol that the path is using. To
> allow
> this, add a protocol subsection to the overrides section in
> multipath.conf, which allows select path-specific options to be set.
> This commit simply adds the subsection, and handles merging matching
> entries. Future patches will make use of the section.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Looks good, just a minor nit below.

> ---
>  libmultipath/config.c | 83 ++++++++++++++++++++++++++++++++++++
>  libmultipath/config.h | 10 +++++
>  libmultipath/dict.c   | 99
> +++++++++++++++++++++++++++++++++++++++++++
>  libmultipath/print.c  | 50 ++++++++++++++++++++++
>  4 files changed, 242 insertions(+)
> 
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 612941b8..5fe71562 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -237,6 +237,18 @@ const char *get_mpe_wwid(const struct _vector
> *mptable, const char *alias)
>         return NULL;
>  }
>  
> +static void
> +free_pctable (vector pctable)
> +{
> +       int i;
> +       struct pcentry *pce;
> +
> +       vector_foreach_slot(pctable, pce, i)
> +               free(pce);
> +
> +       vector_free(pctable);
> +}
> +
>  void
>  free_hwe (struct hwentry * hwe)
>  {
> @@ -282,6 +294,9 @@ free_hwe (struct hwentry * hwe)
>         if (hwe->bl_product)
>                 free(hwe->bl_product);
>  
> +       if (hwe->pctable)
> +               free_pctable(hwe->pctable);
> +
>         free(hwe);
>  }
>  
> @@ -363,6 +378,15 @@ alloc_hwe (void)
>         return hwe;
>  }
>  
> +struct pcentry *
> +alloc_pce (void)
> +{
> +       struct pcentry *pce = (struct pcentry *)
> +                               calloc(1, sizeof(struct pcentry));
> +       pce->type = -1;


Use a symbolic value, like PCE_INVALID?

> +       return pce;
> +}
> +
>  static char *
>  set_param_str(const char * str)
>  {
> @@ -396,6 +420,13 @@ set_param_str(const char * str)
>         if (!dst->s && src->s) \
>                 dst->s = src->s
>  
> +static void
> +merge_pce(struct pcentry *dst, struct pcentry *src)
> +{
> +       merge_num(fast_io_fail);
> +       merge_num(dev_loss);
> +       merge_num(eh_deadline);
> +}
>  
>  static void
>  merge_hwe (struct hwentry * dst, struct hwentry * src)
> @@ -602,6 +633,51 @@ out:
>         return 1;
>  }
>  
> +static void
> +validate_pctable(struct hwentry *ovr, int idx, const char
> *table_desc)
> +{
> +       struct pcentry *pce;
> +
> +       if (!ovr || !ovr->pctable)
> +               return;
> +
> +       vector_foreach_slot_after(ovr->pctable, pce, idx) {
> +               if (pce->type < 0) {
> +                       condlog(0, "protocol section in %s missing
> type",
> +                               table_desc);
> +                       vector_del_slot(ovr->pctable, idx--);
> +                       free(pce);
> +               }
> +       }
> +
> +       if (VECTOR_SIZE(ovr->pctable) == 0) {
> +               vector_free(ovr->pctable);
> +               ovr->pctable = NULL;
> +       }
> +}
> +
> +static void
> +merge_pctable(struct hwentry *ovr)
> +{
> +       struct pcentry *pce1, *pce2;
> +       int i, j;
> +
> +       if (!ovr || !ovr->pctable)
> +               return;
> +
> +       vector_foreach_slot(ovr->pctable, pce1, i) {
> +               j = i + 1;
> +               vector_foreach_slot_after(ovr->pctable, pce2, j) {
> +                       if (pce1->type != pce2->type)
> +                               continue;
> +                       merge_pce(pce2,pce1);
> +                       vector_del_slot(ovr->pctable, i--);
> +                       free(pce1);
> +                       break;
> +               }
> +       }
> +}
> +
>  static void
>  factorize_hwtable (vector hw, int n, const char *table_desc)
>  {
> @@ -750,6 +826,7 @@ process_config_dir(struct config *conf, char
> *dir)
>         int i, n;
>         char path[LINE_MAX];
>         int old_hwtable_size;
> +       int old_pctable_size = 0;
>  
>         if (dir[0] != '/') {
>                 condlog(1, "config_dir '%s' must be a fully qualified
> path",
> @@ -776,11 +853,15 @@ process_config_dir(struct config *conf, char
> *dir)
>                         continue;
>  
>                 old_hwtable_size = VECTOR_SIZE(conf->hwtable);
> +               old_pctable_size = conf->overrides ?
> +                                  VECTOR_SIZE(conf->overrides-
> >pctable) : 0;
>                 snprintf(path, LINE_MAX, "%s/%s", dir, namelist[i]-
> >d_name);
>                 path[LINE_MAX-1] = '\0';
>                 process_file(conf, path);
>                 factorize_hwtable(conf->hwtable, old_hwtable_size,
>                                   namelist[i]->d_name);
> +               validate_pctable(conf->overrides, old_pctable_size,
> +                                namelist[i]->d_name);
>         }
>         pthread_cleanup_pop(1);
>  }
> @@ -888,6 +969,7 @@ int _init_config (const char *file, struct config
> *conf)
>                         goto out;
>                 }
>                 factorize_hwtable(conf->hwtable,
> builtin_hwtable_size, file);
> +               validate_pctable(conf->overrides, 0, file);
>         }
>  
>         conf->processed_main_config = 1;
> @@ -988,6 +1070,7 @@ int _init_config (const char *file, struct
> config *conf)
>                         goto out;
>         }
>  
> +       merge_pctable(conf->overrides);
>         merge_mptable(conf->mptable);
>         merge_blacklist(conf->blist_devnode);
>         merge_blacklist(conf->blist_property);
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index c73389b5..b7bca9a8 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -40,6 +40,13 @@ enum force_reload_types {
>         FORCE_RELOAD_WEAK,
>  };
>  
> +struct pcentry {
> +       int type;
> +       int fast_io_fail;
> +       unsigned int dev_loss;
> +       int eh_deadline;
> +};
> +
>  struct hwentry {
>         char * vendor;
>         char * product;
> @@ -85,6 +92,8 @@ struct hwentry {
>         int vpd_vendor_id;
>         int recheck_wwid;
>         char * bl_product;
> +
> +       vector pctable;
>  };
>  
>  struct mpentry {
> @@ -284,6 +293,7 @@ const char *get_mpe_wwid (const struct _vector
> *mptable, const char *alias);
>  
>  struct hwentry * alloc_hwe (void);
>  struct mpentry * alloc_mpe (void);
> +struct pcentry * alloc_pce (void);
>  
>  void free_hwe (struct hwentry * hwe);
>  void free_hwtable (vector hwtable);
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 26cbe785..8e11fd70 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -417,6 +417,29 @@ snprint_mp_ ## option (struct config *conf,
> struct strbuf *buff,   \
>         return function(buff, mpe-
> >option);                             \
>  }
>  
> +#define declare_pc_handler(option,
> function)                           \
> +static
> int                                                             \
> +pc_ ## option ## _handler (struct config *conf, vector
> strvec,         \
> +                          const char *file, int
> line_nr)               \
> +{                                                                   
>    \
> +       struct pcentry
> *pce;                                            \
> +       if (!conf->overrides || !conf->overrides-
> >pctable)              \
> +               return
> 1;                                               \
> +       pce = VECTOR_LAST_SLOT(conf->overrides-
> >pctable);               \
> +       if
> (!pce)                                                       \
> +               return
> 1;                                               \
> +       return function (strvec, &pce->option, file,
> line_nr);          \
> +}
> +
> +#define declare_pc_snprint(option,
> function)                           \
> +static
> int                                                             \
> +snprint_pc_ ## option (struct config *conf, struct strbuf
> *buff,       \
> +                      const void
> *data)                                \
> +{                                                                   
>    \
> +       const struct pcentry *pce  = (const struct pcentry
> *)data;      \
> +       return function(buff, pce-
> >option);                             \
> +}
> +
>  static int checkint_handler(struct config *conf, vector strvec,
>                             const char *file, int line_nr)
>  {
> @@ -1046,6 +1069,8 @@ declare_ovr_handler(fast_io_fail,
> set_undef_off_zero)
>  declare_ovr_snprint(fast_io_fail, print_undef_off_zero)
>  declare_hw_handler(fast_io_fail, set_undef_off_zero)
>  declare_hw_snprint(fast_io_fail, print_undef_off_zero)
> +declare_pc_handler(fast_io_fail, set_undef_off_zero)
> +declare_pc_snprint(fast_io_fail, print_undef_off_zero)
>  
>  static int
>  set_dev_loss(vector strvec, void *ptr, const char *file, int
> line_nr)
> @@ -1083,6 +1108,8 @@ declare_ovr_handler(dev_loss, set_dev_loss)
>  declare_ovr_snprint(dev_loss, print_dev_loss)
>  declare_hw_handler(dev_loss, set_dev_loss)
>  declare_hw_snprint(dev_loss, print_dev_loss)
> +declare_pc_handler(dev_loss, set_dev_loss)
> +declare_pc_snprint(dev_loss, print_dev_loss)
>  
>  declare_def_handler(eh_deadline, set_undef_off_zero)
>  declare_def_snprint(eh_deadline, print_undef_off_zero)
> @@ -1090,6 +1117,8 @@ declare_ovr_handler(eh_deadline,
> set_undef_off_zero)
>  declare_ovr_snprint(eh_deadline, print_undef_off_zero)
>  declare_hw_handler(eh_deadline, set_undef_off_zero)
>  declare_hw_snprint(eh_deadline, print_undef_off_zero)
> +declare_pc_handler(eh_deadline, set_undef_off_zero)
> +declare_pc_snprint(eh_deadline, print_undef_off_zero)
>  
>  static int
>  set_pgpolicy(vector strvec, void *ptr, const char *file, int
> line_nr)
> @@ -1897,6 +1926,69 @@ declare_mp_snprint(wwid, print_str)
>  declare_mp_handler(alias, set_str_noslash)
>  declare_mp_snprint(alias, print_str)
>  
> +
> +static int
> +protocol_handler(struct config *conf, vector strvec, const char
> *file,
> +               int line_nr)
> +{
> +       struct pcentry *pce;
> +
> +       if (!conf->overrides)
> +               return 1;
> +
> +       if (!conf->overrides->pctable &&
> +           !(conf->overrides->pctable = vector_alloc()))
> +               return 1;
> +
> +       if (!(pce = alloc_pce()))
> +               return 1;
> +
> +       if (!vector_alloc_slot(conf->overrides->pctable)) {
> +               free(pce);
> +               return 1;
> +       }
> +       vector_set_slot(conf->overrides->pctable, pce);
> +
> +       return 0;
> +}
> +
> +static int
> +set_protocol_type(vector strvec, void *ptr, const char *file, int
> line_nr)
> +{
> +       int *int_ptr = (int *)ptr;
> +       char *buff;
> +       int i;
> +
> +       buff = set_value(strvec);
> +
> +       if (!buff)
> +               return 1;
> +
> +       for (i = 0; i <= LAST_BUS_PROTOCOL_ID; i++) {
> +               if (protocol_name[i] && !strcmp(buff,
> protocol_name[i])) {
> +                       *int_ptr = i;
> +                       break;
> +               }
> +       }
> +       if (i > LAST_BUS_PROTOCOL_ID)
> +               condlog(1, "%s line %d, invalid value for type:
> \"%s\"",
> +                       file, line_nr, buff);
> +
> +       free(buff);
> +       return 0;
> +}
> +
> +static int
> +print_protocol_type(struct strbuf *buff, int type)
> +{
> +       if (type < 0)
> +               return 0;
> +       return append_strbuf_quoted(buff, protocol_name[type]);
> +}
> +
> +declare_pc_handler(type, set_protocol_type)
> +declare_pc_snprint(type, print_protocol_type)
> +
>  /*
>   * deprecated handlers
>   */
> @@ -2138,6 +2230,13 @@ init_keywords(vector keywords)
>         install_keyword("ghost_delay", &ovr_ghost_delay_handler,
> &snprint_ovr_ghost_delay);
>         install_keyword("all_tg_pt", &ovr_all_tg_pt_handler,
> &snprint_ovr_all_tg_pt);
>         install_keyword("recheck_wwid", &ovr_recheck_wwid_handler,
> &snprint_ovr_recheck_wwid);
> +       install_keyword_multi("protocol", &protocol_handler, NULL);
> +       install_sublevel();
> +       install_keyword("type", &pc_type_handler, &snprint_pc_type);
> +       install_keyword("fast_io_fail_tmo", &pc_fast_io_fail_handler,
> &snprint_pc_fast_io_fail);
> +       install_keyword("dev_loss_tmo", &pc_dev_loss_handler,
> &snprint_pc_dev_loss);
> +       install_keyword("eh_deadline", &pc_eh_deadline_handler,
> &snprint_pc_eh_deadline);
> +       install_sublevel_end();
>  
>         install_keyword_root("multipaths", &multipaths_handler);
>         install_keyword_multi("multipath", &multipath_handler, NULL);
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index 27c2cf1a..68a793e7 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -1406,6 +1406,52 @@ int snprint_multipath_topology_json (struct
> strbuf *buff,
>         return get_strbuf_len(buff) - initial_len;
>  }
>  
> +static int
> +snprint_pcentry (const struct config *conf, struct strbuf *buff,
> +                const struct pcentry *pce)
> +{
> +       int i, rc;
> +       struct keyword *kw;
> +       struct keyword * rootkw;
> +       size_t initial_len = get_strbuf_len(buff);
> +
> +       rootkw = find_keyword(conf->keywords, NULL, "overrides");
> +       assert(rootkw && rootkw->sub);
> +       rootkw = find_keyword(conf->keywords, rootkw->sub,
> "protocol");
> +       assert(rootkw);
> +
> +       if ((rc = append_strbuf_str(buff, "\tprotocol {\n")) < 0)
> +               return rc;
> +
> +       iterate_sub_keywords(rootkw, kw, i) {
> +               if ((rc = snprint_keyword(buff, "\t\t%k %v\n", kw,
> pce)) < 0)
> +                       return rc;
> +       }
> +
> +       if ((rc = append_strbuf_str(buff, "\t}\n")) < 0)
> +               return rc;
> +       return get_strbuf_len(buff) - initial_len;
> +}
> +
> +static int
> +snprint_pctable (const struct config *conf, struct strbuf *buff,
> +                const struct _vector *pctable)
> +{
> +       int i, rc;
> +       struct pcentry *pce;
> +       struct keyword * rootkw;
> +       size_t initial_len = get_strbuf_len(buff);
> +
> +       rootkw = find_keyword(conf->keywords, NULL, "overrides");
> +       assert(rootkw);
> +
> +       vector_foreach_slot(pctable, pce, i) {
> +               if ((rc = snprint_pcentry(conf, buff, pce)) < 0)
> +                       return rc;
> +       }
> +       return get_strbuf_len(buff) - initial_len;
> +}
> +
>  static int
>  snprint_hwentry (const struct config *conf,
>                  struct strbuf *buff, const struct hwentry * hwe)
> @@ -1560,6 +1606,10 @@ static int snprint_overrides(const struct
> config *conf, struct strbuf *buff,
>                 if ((rc = snprint_keyword(buff, "\t%k %v\n", kw,
> NULL)) < 0)
>                         return rc;
>         }
> +
> +       if (overrides->pctable &&
> +           (rc = snprint_pctable(conf, buff, overrides->pctable)) <
> 0)
> +               return rc;
>  out:
>         if ((rc = append_strbuf_str(buff, "}\n")) < 0)
>                 return rc;

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH v2 4/7] libmultipath: Set the scsi timeout parameters by path

[Martin Wilck]

On Wed, 2022-04-13 at 23:27 -0500, Benjamin Marzinski wrote:
> Instead of dev_loss, fast_io_fail, and eh_deadline belonging to the
> multipath structure, have them belong to the path structure. This
> means
> that they are selected per path, and that sysfs_set_scsi_tmo()
> doesn't
> assume that all paths of a multipath device will have the same value.
> Currently they will all be the same, but a future patch will make it
> possible for paths to have different values based on their protocol.
> 
> 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

Re: [dm-devel] [PATCH v2 5/7] libmultipath: check the overrides pctable for path variables

[Martin Wilck]

On Wed, 2022-04-13 at 23:27 -0500, Benjamin Marzinski wrote:
> Paths will now also check the pctable when checking for attribtes
> that
> exists in both the overrides section and the protocol subsection.
> Values
> in a matching pcentry will be used in preference to values in the
> overrides hwentry.
> 
> 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

Re: [dm-devel] [PATCH v2 6/7] libmultipath: fix eh_deadline documentation

[Martin Wilck]

On Wed, 2022-04-13 at 23:27 -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

Re: [dm-devel] [PATCH v2 7/7] libmultipath: Add documentation for the protocol subsection

[Martin Wilck]

On Wed, 2022-04-13 at 23:27 -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

Re: [dm-devel] [PATCH v2 3/7] libmultipath: add a protocol subsection to multipath.conf

[Martin Wilck]

On Thu, 2022-04-14 at 09:27 +0200, Martin Wilck wrote:
> On Wed, 2022-04-13 at 23:27 -0500, Benjamin Marzinski wrote:
> > Some storage arrays can be accessed using multiple protocols at the
> > same
> > time.  In these cases, users may want to set path attributes
> > differently, depending on the protocol that the path is using. To
> > allow
> > this, add a protocol subsection to the overrides section in
> > multipath.conf, which allows select path-specific options to be
> > set.
> > This commit simply adds the subsection, and handles merging
> > matching
> > entries. Future patches will make use of the section.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Looks good, just a minor nit below.

As this is really minor and I'm going to be out of office next week:

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

... and I'm going to push this series to my "queue" branch.

Perhaps you want to send an add-on patch with a symbolic value later.

Regards
Martin

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel