[dm-devel] [PATCH v3 0/9] improving config parsing warnings

[dm-devel] [PATCH v3 0/9] improving config parsing warnings

[Benjamin Marzinski]

This is a series of patches to make multipath provide better warnings
when parsing multipath.conf. The first three patches fix small issues.
The last six patches make multipath print warning messages with
the file and line number, when it was previously either accepting
invalid values or silently changing them.

Changes from v2->v3, as suggested by Martin Wilck:
0007: still print the value of the deprecated options.
0008: revert to v1 patch

Changes from v1->v2 (by v1 patch number), as suggested by Martin Wilck:

0005: use MAX_VEROSITY enum in the verbosity handler declaration.
0006: added a new patch, 0007, to warn that these options are now
      deprecated, and no longer print them when the have the default
      value.  They are still used for now.
0007: functions that accept "0" as a special input value will now
      check if the integer gotten by do_set_int() is 0, and
      covert that to the special case.

Benjamin Marzinski (9):
  libmultipath: add section name to invalid keyword output
  libmultipath: use typedef for keyword handler function
  libmultipath: print the correct file when parsing fails
  libmultipath: pass file and line number to keyword handlers
  libmultipath: make set_int take a range for valid values
  libmultipath: improve checks for set_str
  libmultipath: deprecate file and directory config options
  libmultipath: split set_int to enable reuse
  libmultipath: cleanup invalid config handling

 libmultipath/dict.c        | 492 +++++++++++++++++++++++++------------
 libmultipath/parser.c      |  31 ++-
 libmultipath/parser.h      |  15 +-
 multipath/multipath.conf.5 |   5 +
 4 files changed, 365 insertions(+), 178 deletions(-)

-- 
2.17.2

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

[dm-devel] [PATCH v3 1/9] libmultipath: add section name to invalid keyword output

[Benjamin Marzinski]

If users forget the closing brace for a section in multipath.conf,
multipath has no way to detect that. When it sees the keyword at the
start of the next section, it will complain that there is an invalid
keyword, because that keyword doesn't belong in previous section (which
was never ended with a closing brace). This can confuse users. To make
this easier to understand, when multipath prints an invalid keyword
message, it now also prints the current section name, which can give
users a hint that they didn't end the previous section.

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

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 8ca91bf2..611054f7 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -504,7 +504,7 @@ validate_config_strvec(vector strvec, const char *file)
 
 static int
 process_stream(struct config *conf, FILE *stream, vector keywords,
-	       const char *file)
+	       const char *section, const char *file)
 {
 	int i;
 	int r = 0, t;
@@ -568,16 +568,22 @@ process_stream(struct config *conf, FILE *stream, vector keywords,
 				if (keyword->sub) {
 					kw_level++;
 					r += process_stream(conf, stream,
-							    keyword->sub, file);
+							    keyword->sub,
+							    keyword->string,
+							    file);
 					kw_level--;
 				}
 				break;
 			}
 		}
-		if (i >= VECTOR_SIZE(keywords))
-			condlog(1, "%s line %d, invalid keyword: %s",
-				file, line_nr, str);
-
+		if (i >= VECTOR_SIZE(keywords)) {
+			if (section)
+				condlog(1, "%s line %d, invalid keyword in the %s section: %s",
+					file, line_nr, section, str);
+			else
+				condlog(1, "%s line %d, invalid keyword: %s",
+					file, line_nr, str);
+		}
 		free_strvec(strvec);
 	}
 	if (kw_level == 1)
@@ -608,7 +614,7 @@ process_file(struct config *conf, const char *file)
 
 	/* Stream handling */
 	line_nr = 0;
-	r = process_stream(conf, stream, conf->keywords, file);
+	r = process_stream(conf, stream, conf->keywords, NULL, file);
 	fclose(stream);
 	//free_keywords(keywords);
 
-- 
2.17.2

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

[dm-devel] [PATCH v3 2/9] libmultipath: use typedef for keyword handler function

[Benjamin Marzinski]

Don't keep writing out the function type.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/parser.c |  6 +++---
 libmultipath/parser.h | 15 ++++++---------
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 611054f7..ebe1cbd9 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -33,7 +33,7 @@ static int line_nr;
 
 int
 keyword_alloc(vector keywords, char *string,
-	      int (*handler) (struct config *, vector),
+	      handler_fn *handler,
 	      print_fn *print,
 	      int unique)
 {
@@ -72,7 +72,7 @@ install_sublevel_end(void)
 
 int
 _install_keyword(vector keywords, char *string,
-		 int (*handler) (struct config *, vector),
+		 handler_fn *handler,
 		 print_fn *print,
 		 int unique)
 {
@@ -558,7 +558,7 @@ process_stream(struct config *conf, FILE *stream, vector keywords,
 						goto out;
 				}
 				if (keyword->handler) {
-				    t = (*keyword->handler) (conf, strvec);
+				    t = keyword->handler(conf, strvec);
 					r += t;
 					if (t)
 						condlog(1, "multipath.conf +%d, parsing failed: %s",
diff --git a/libmultipath/parser.h b/libmultipath/parser.h
index b43d46f8..3452bde1 100644
--- a/libmultipath/parser.h
+++ b/libmultipath/parser.h
@@ -43,10 +43,11 @@ struct strbuf;
 
 /* keyword definition */
 typedef int print_fn(struct config *, struct strbuf *, const void *);
+typedef int handler_fn(struct config *, vector);
 
 struct keyword {
 	char *string;
-	int (*handler) (struct config *, vector);
+	handler_fn *handler;
 	print_fn *print;
 	vector sub;
 	int unique;
@@ -62,18 +63,14 @@ struct keyword {
 	for (i = 0; i < (k)->sub->allocated && ((p) = (k)->sub->slot[i]); i++)
 
 /* Prototypes */
-extern int keyword_alloc(vector keywords, char *string,
-			 int (*handler) (struct config *, vector),
-			 print_fn *print,
-			 int unique);
+extern int keyword_alloc(vector keywords, char *string, handler_fn *handler,
+			 print_fn *print, int unique);
 #define install_keyword_root(str, h) keyword_alloc(keywords, str, h, NULL, 1)
 extern void install_sublevel(void);
 extern void install_sublevel_end(void);
 
-extern int _install_keyword(vector keywords, char *string,
-			    int (*handler) (struct config *, vector),
-			    print_fn *print,
-			    int unique);
+extern int _install_keyword(vector keywords, char *string, handler_fn *handler,
+			    print_fn *print, int unique);
 #define install_keyword(str, vec, pri) _install_keyword(keywords, str, vec, pri, 1)
 #define install_keyword_multi(str, vec, pri) _install_keyword(keywords, str, vec, pri, 0)
 extern void dump_keywords(vector keydump, int level);
-- 
2.17.2

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

[dm-devel] [PATCH v3 3/9] libmultipath: print the correct file when parsing fails

[Benjamin Marzinski]

Don't assume that parsing failed on multipath.conf

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

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index ebe1cbd9..d5595fb0 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -561,8 +561,8 @@ process_stream(struct config *conf, FILE *stream, vector keywords,
 				    t = keyword->handler(conf, strvec);
 					r += t;
 					if (t)
-						condlog(1, "multipath.conf +%d, parsing failed: %s",
-							line_nr, buf);
+						condlog(1, "%s line %d, parsing failed: %s",
+							file, line_nr, buf);
 				}
 
 				if (keyword->sub) {
-- 
2.17.2

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

[dm-devel] [PATCH v3 4/9] libmultipath: pass file and line number to keyword handlers

[Benjamin Marzinski]

This will make it possible for the keyword handlers to print more useful
warning messages. It will be used by future patches.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/dict.c   | 143 +++++++++++++++++++++++++-----------------
 libmultipath/parser.c |   3 +-
 libmultipath/parser.h |   2 +-
 3 files changed, 90 insertions(+), 58 deletions(-)

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 7a727389..eb2c44c0 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -29,7 +29,7 @@
 #include "strbuf.h"
 
 static int
-set_int(vector strvec, void *ptr)
+set_int(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	int *int_ptr = (int *)ptr;
 	char *buff, *eptr;
@@ -58,7 +58,7 @@ set_int(vector strvec, void *ptr)
 }
 
 static int
-set_uint(vector strvec, void *ptr)
+set_uint(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	unsigned int *uint_ptr = (unsigned int *)ptr;
 	char *buff, *eptr, *p;
@@ -90,7 +90,7 @@ set_uint(vector strvec, void *ptr)
 }
 
 static int
-set_str(vector strvec, void *ptr)
+set_str(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char **str_ptr = (char **)ptr;
 
@@ -105,7 +105,7 @@ set_str(vector strvec, void *ptr)
 }
 
 static int
-set_yes_no(vector strvec, void *ptr)
+set_yes_no(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char * buff;
 	int *int_ptr = (int *)ptr;
@@ -124,7 +124,7 @@ set_yes_no(vector strvec, void *ptr)
 }
 
 static int
-set_yes_no_undef(vector strvec, void *ptr)
+set_yes_no_undef(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char * buff;
 	int *int_ptr = (int *)ptr;
@@ -187,9 +187,10 @@ static int print_yes_no_undef(struct strbuf *buff, long v)
 
 #define declare_def_handler(option, function)				\
 static int								\
-def_ ## option ## _handler (struct config *conf, vector strvec)		\
+def_ ## option ## _handler (struct config *conf, vector strvec,		\
+			    const char *file, int line_nr)		\
 {									\
-	return function (strvec, &conf->option);			\
+	return function (strvec, &conf->option, file, line_nr);		\
 }
 
 #define declare_def_snprint(option, function)				\
@@ -224,12 +225,13 @@ snprint_def_ ## option (struct config *conf, struct strbuf *buff,	\
 
 #define declare_hw_handler(option, function)				\
 static int								\
-hw_ ## option ## _handler (struct config *conf, vector strvec)		\
+hw_ ## option ## _handler (struct config *conf, vector strvec,		\
+			   const char *file, int line_nr)		\
 {									\
 	struct hwentry * hwe = VECTOR_LAST_SLOT(conf->hwtable);		\
 	if (!hwe)							\
 		return 1;						\
-	return function (strvec, &hwe->option);				\
+	return function (strvec, &hwe->option, file, line_nr);		\
 }
 
 #define declare_hw_snprint(option, function)				\
@@ -243,11 +245,12 @@ snprint_hw_ ## option (struct config *conf, struct strbuf *buff,	\
 
 #define declare_ovr_handler(option, function)				\
 static int								\
-ovr_ ## option ## _handler (struct config *conf, vector strvec)		\
+ovr_ ## option ## _handler (struct config *conf, vector strvec,		\
+			    const char *file, int line_nr)		\
 {									\
 	if (!conf->overrides)						\
 		return 1;						\
-	return function (strvec, &conf->overrides->option);		\
+	return function (strvec, &conf->overrides->option, file, line_nr); \
 }
 
 #define declare_ovr_snprint(option, function)				\
@@ -260,12 +263,13 @@ snprint_ovr_ ## option (struct config *conf, struct strbuf *buff,	\
 
 #define declare_mp_handler(option, function)				\
 static int								\
-mp_ ## option ## _handler (struct config *conf, vector strvec)		\
+mp_ ## option ## _handler (struct config *conf, vector strvec,		\
+			   const char *file, int line_nr)		\
 {									\
 	struct mpentry * mpe = VECTOR_LAST_SLOT(conf->mptable);		\
 	if (!mpe)							\
 		return 1;						\
-	return function (strvec, &mpe->option);				\
+	return function (strvec, &mpe->option, file, line_nr);		\
 }
 
 #define declare_mp_snprint(option, function)				\
@@ -277,9 +281,10 @@ snprint_mp_ ## option (struct config *conf, struct strbuf *buff,	\
 	return function(buff, mpe->option);				\
 }
 
-static int checkint_handler(struct config *conf, vector strvec)
+static int checkint_handler(struct config *conf, vector strvec,
+			    const char *file, int line_nr)
 {
-	int rc = set_uint(strvec, &conf->checkint);
+	int rc = set_uint(strvec, &conf->checkint, file, line_nr);
 
 	if (rc)
 		return rc;
@@ -302,9 +307,10 @@ declare_def_snprint(reassign_maps, print_yes_no)
 declare_def_handler(multipath_dir, set_str)
 declare_def_snprint(multipath_dir, print_str)
 
-static int def_partition_delim_handler(struct config *conf, vector strvec)
+static int def_partition_delim_handler(struct config *conf, vector strvec,
+				       const char *file, int line_nr)
 {
-	int rc = set_str(strvec, &conf->partition_delim);
+	int rc = set_str(strvec, &conf->partition_delim, file, line_nr);
 
 	if (rc != 0)
 		return rc;
@@ -334,13 +340,13 @@ static const char * const find_multipaths_optvals[] = {
 };
 
 static int
-def_find_multipaths_handler(struct config *conf, vector strvec)
+def_find_multipaths_handler(struct config *conf, vector strvec,
+			    const char *file, int line_nr)
 {
 	char *buff;
 	int i;
 
-	if (set_yes_no_undef(strvec, &conf->find_multipaths) == 0 &&
-	    conf->find_multipaths != FIND_MULTIPATHS_UNDEF)
+	if (set_yes_no_undef(strvec, &conf->find_multipaths, file, line_nr) == 0 && conf->find_multipaths != FIND_MULTIPATHS_UNDEF)
 		return 0;
 
 	buff = set_value(strvec);
@@ -396,7 +402,8 @@ static int snprint_uid_attrs(struct config *conf, struct strbuf *buff,
 	return total;
 }
 
-static int uid_attrs_handler(struct config *conf, vector strvec)
+static int uid_attrs_handler(struct config *conf, vector strvec,
+			     const char *file, int line_nr)
 {
 	char *val;
 
@@ -597,7 +604,8 @@ declare_hw_handler(skip_kpartx, set_yes_no_undef)
 declare_hw_snprint(skip_kpartx, print_yes_no_undef)
 declare_mp_handler(skip_kpartx, set_yes_no_undef)
 declare_mp_snprint(skip_kpartx, print_yes_no_undef)
-static int def_disable_changed_wwids_handler(struct config *conf, vector strvec)
+static int def_disable_changed_wwids_handler(struct config *conf, vector strvec,
+					     const char *file, int line_nr)
 {
 	return 0;
 }
@@ -629,20 +637,23 @@ declare_def_snprint_defstr(enable_foreign, print_str,
 			   DEFAULT_ENABLE_FOREIGN)
 
 static int
-def_config_dir_handler(struct config *conf, vector strvec)
+def_config_dir_handler(struct config *conf, vector strvec, const char *file,
+		       int line_nr)
 {
 	/* this is only valid in the main config file */
 	if (conf->processed_main_config)
 		return 0;
-	return set_str(strvec, &conf->config_dir);
+	return set_str(strvec, &conf->config_dir, file, line_nr);
 }
 declare_def_snprint(config_dir, print_str)
 
 #define declare_def_attr_handler(option, function)			\
 static int								\
-def_ ## option ## _handler (struct config *conf, vector strvec)		\
+def_ ## option ## _handler (struct config *conf, vector strvec,		\
+			    const char *file, int line_nr)		\
 {									\
-	return function (strvec, &conf->option, &conf->attribute_flags);\
+	return function (strvec, &conf->option, &conf->attribute_flags, \
+			 file, line_nr);				\
 }
 
 #define declare_def_attr_snprint(option, function)			\
@@ -655,12 +666,14 @@ snprint_def_ ## option (struct config *conf, struct strbuf *buff,	\
 
 #define declare_mp_attr_handler(option, function)			\
 static int								\
-mp_ ## option ## _handler (struct config *conf, vector strvec)		\
+mp_ ## option ## _handler (struct config *conf, vector strvec,		\
+			   const char *file, int line_nr)		\
 {									\
 	struct mpentry * mpe = VECTOR_LAST_SLOT(conf->mptable);		\
 	if (!mpe)							\
 		return 1;						\
-	return function (strvec, &mpe->option, &mpe->attribute_flags);	\
+	return function (strvec, &mpe->option, &mpe->attribute_flags,	\
+			 file, line_nr);				\
 }
 
 #define declare_mp_attr_snprint(option, function)			\
@@ -673,7 +686,7 @@ snprint_mp_ ## option (struct config *conf, struct strbuf *buff,	\
 }
 
 static int
-set_mode(vector strvec, void *ptr, int *flags)
+set_mode(vector strvec, void *ptr, int *flags, const char *file, int line_nr)
 {
 	mode_t mode;
 	mode_t *mode_ptr = (mode_t *)ptr;
@@ -694,7 +707,7 @@ set_mode(vector strvec, void *ptr, int *flags)
 }
 
 static int
-set_uid(vector strvec, void *ptr, int *flags)
+set_uid(vector strvec, void *ptr, int *flags, const char *file, int line_nr)
 {
 	uid_t uid;
 	uid_t *uid_ptr = (uid_t *)ptr;
@@ -719,7 +732,7 @@ set_uid(vector strvec, void *ptr, int *flags)
 }
 
 static int
-set_gid(vector strvec, void *ptr, int *flags)
+set_gid(vector strvec, void *ptr, int *flags, const char *file, int line_nr)
 {
 	gid_t gid;
 	gid_t *gid_ptr = (gid_t *)ptr;
@@ -786,7 +799,7 @@ declare_mp_attr_handler(gid, set_gid)
 declare_mp_attr_snprint(gid, print_gid)
 
 static int
-set_undef_off_zero(vector strvec, void *ptr)
+set_undef_off_zero(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char * buff;
 	int *int_ptr = (int *)ptr;
@@ -827,7 +840,7 @@ declare_hw_handler(fast_io_fail, set_undef_off_zero)
 declare_hw_snprint(fast_io_fail, print_undef_off_zero)
 
 static int
-set_dev_loss(vector strvec, void *ptr)
+set_dev_loss(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char * buff;
 	unsigned int *uint_ptr = (unsigned int *)ptr;
@@ -870,7 +883,7 @@ declare_hw_handler(eh_deadline, set_undef_off_zero)
 declare_hw_snprint(eh_deadline, print_undef_off_zero)
 
 static int
-set_pgpolicy(vector strvec, void *ptr)
+set_pgpolicy(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char * buff;
 	int *int_ptr = (int *)ptr;
@@ -936,7 +949,8 @@ get_sys_max_fds(int *max_fds)
 
 
 static int
-max_fds_handler(struct config *conf, vector strvec)
+max_fds_handler(struct config *conf, vector strvec, const char *file,
+		int line_nr)
 {
 	char * buff;
 	int r = 0, max_fds;
@@ -981,7 +995,7 @@ snprint_max_fds (struct config *conf, struct strbuf *buff, const void *data)
 }
 
 static int
-set_rr_weight(vector strvec, void *ptr)
+set_rr_weight(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	int *int_ptr = (int *)ptr;
 	char * buff;
@@ -1025,7 +1039,7 @@ declare_mp_handler(rr_weight, set_rr_weight)
 declare_mp_snprint(rr_weight, print_rr_weight)
 
 static int
-set_pgfailback(vector strvec, void *ptr)
+set_pgfailback(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	int *int_ptr = (int *)ptr;
 	char * buff;
@@ -1075,7 +1089,7 @@ declare_mp_handler(pgfailback, set_pgfailback)
 declare_mp_snprint(pgfailback, print_pgfailback)
 
 static int
-no_path_retry_helper(vector strvec, void *ptr)
+no_path_retry_helper(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	int *int_ptr = (int *)ptr;
 	char * buff;
@@ -1120,7 +1134,8 @@ declare_mp_handler(no_path_retry, no_path_retry_helper)
 declare_mp_snprint(no_path_retry, print_no_path_retry)
 
 static int
-def_log_checker_err_handler(struct config *conf, vector strvec)
+def_log_checker_err_handler(struct config *conf, vector strvec,
+			    const char *file, int line_nr)
 {
 	char * buff;
 
@@ -1193,7 +1208,8 @@ print_reservation_key(struct strbuf *buff,
 }
 
 static int
-def_reservation_key_handler(struct config *conf, vector strvec)
+def_reservation_key_handler(struct config *conf, vector strvec,
+			    const char *file, int line_nr)
 {
 	return set_reservation_key(strvec, &conf->reservation_key,
 				   &conf->sa_flags,
@@ -1209,7 +1225,8 @@ snprint_def_reservation_key (struct config *conf, struct strbuf *buff,
 }
 
 static int
-mp_reservation_key_handler(struct config *conf, vector strvec)
+mp_reservation_key_handler(struct config *conf, vector strvec, const char *file,
+			   int line_nr)
 {
 	struct mpentry * mpe = VECTOR_LAST_SLOT(conf->mptable);
 	if (!mpe)
@@ -1229,7 +1246,7 @@ snprint_mp_reservation_key (struct config *conf, struct strbuf *buff,
 }
 
 static int
-set_off_int_undef(vector strvec, void *ptr)
+set_off_int_undef(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	int *int_ptr = (int *)ptr;
 	char * buff;
@@ -1370,7 +1387,8 @@ declare_hw_snprint(recheck_wwid, print_yes_no_undef)
 
 
 static int
-def_uxsock_timeout_handler(struct config *conf, vector strvec)
+def_uxsock_timeout_handler(struct config *conf, vector strvec, const char *file,
+			   int line_nr)
 {
 	unsigned int uxsock_timeout;
 	char *buff;
@@ -1390,7 +1408,8 @@ def_uxsock_timeout_handler(struct config *conf, vector strvec)
 }
 
 static int
-hw_vpd_vendor_handler(struct config *conf, vector strvec)
+hw_vpd_vendor_handler(struct config *conf, vector strvec, const char *file,
+		      int line_nr)
 {
 	int i;
 	char *buff;
@@ -1430,7 +1449,8 @@ snprint_hw_vpd_vendor(struct config *conf, struct strbuf *buff,
  * blacklist block handlers
  */
 static int
-blacklist_handler(struct config *conf, vector strvec)
+blacklist_handler(struct config *conf, vector strvec, const char*file,
+		  int line_nr)
 {
 	if (!conf->blist_devnode)
 		conf->blist_devnode = vector_alloc();
@@ -1452,7 +1472,8 @@ blacklist_handler(struct config *conf, vector strvec)
 }
 
 static int
-blacklist_exceptions_handler(struct config *conf, vector strvec)
+blacklist_exceptions_handler(struct config *conf, vector strvec,
+			     const char *file, int line_nr)
 {
 	if (!conf->elist_devnode)
 		conf->elist_devnode = vector_alloc();
@@ -1475,7 +1496,8 @@ blacklist_exceptions_handler(struct config *conf, vector strvec)
 
 #define declare_ble_handler(option)					\
 static int								\
-ble_ ## option ## _handler (struct config *conf, vector strvec)		\
+ble_ ## option ## _handler (struct config *conf, vector strvec,		\
+			    const char *file, int line_nr)		\
 {									\
 	char *buff;							\
 	int rc;								\
@@ -1494,7 +1516,8 @@ ble_ ## option ## _handler (struct config *conf, vector strvec)		\
 
 #define declare_ble_device_handler(name, option, vend, prod)		\
 static int								\
-ble_ ## option ## _ ## name ## _handler (struct config *conf, vector strvec) \
+ble_ ## option ## _ ## name ## _handler (struct config *conf, vector strvec, \
+					 const char *file, int line_nr)	\
 {									\
 	char * buff;							\
 	int rc;								\
@@ -1536,13 +1559,15 @@ snprint_ble_simple (struct config *conf, struct strbuf *buff, const void *data)
 }
 
 static int
-ble_device_handler(struct config *conf, vector strvec)
+ble_device_handler(struct config *conf, vector strvec, const char *file,
+		   int line_nr)
 {
 	return alloc_ble_device(conf->blist_device);
 }
 
 static int
-ble_except_device_handler(struct config *conf, vector strvec)
+ble_except_device_handler(struct config *conf, vector strvec, const char *file,
+			  int line_nr)
 {
 	return alloc_ble_device(conf->elist_device);
 }
@@ -1574,7 +1599,8 @@ static int snprint_bled_product(struct config *conf, struct strbuf *buff,
  * devices block handlers
  */
 static int
-devices_handler(struct config *conf, vector strvec)
+devices_handler(struct config *conf, vector strvec, const char *file,
+		int line_nr)
 {
 	if (!conf->hwtable)
 		conf->hwtable = vector_alloc();
@@ -1586,7 +1612,8 @@ devices_handler(struct config *conf, vector strvec)
 }
 
 static int
-device_handler(struct config *conf, vector strvec)
+device_handler(struct config *conf, vector strvec, const char *file,
+	       int line_nr)
 {
 	struct hwentry * hwe;
 
@@ -1623,7 +1650,8 @@ declare_hw_snprint(hwhandler, print_str)
  * overrides handlers
  */
 static int
-overrides_handler(struct config *conf, vector strvec)
+overrides_handler(struct config *conf, vector strvec, const char *file,
+		  int line_nr)
 {
 	if (!conf->overrides)
 		conf->overrides = alloc_hwe();
@@ -1640,7 +1668,8 @@ overrides_handler(struct config *conf, vector strvec)
  * multipaths block handlers
  */
 static int
-multipaths_handler(struct config *conf, vector strvec)
+multipaths_handler(struct config *conf, vector strvec, const char *file,
+		   int line_nr)
 {
 	if (!conf->mptable)
 		conf->mptable = vector_alloc();
@@ -1652,7 +1681,8 @@ multipaths_handler(struct config *conf, vector strvec)
 }
 
 static int
-multipath_handler(struct config *conf, vector strvec)
+multipath_handler(struct config *conf, vector strvec, const char *file,
+		  int line_nr)
 {
 	struct mpentry * mpe;
 
@@ -1681,7 +1711,8 @@ declare_mp_snprint(alias, print_str)
  */
 
 static int
-deprecated_handler(struct config *conf, vector strvec)
+deprecated_handler(struct config *conf, vector strvec, const char *file,
+		   int line_nr)
 {
 	char * buff;
 
diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index d5595fb0..68262d0e 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -558,7 +558,8 @@ process_stream(struct config *conf, FILE *stream, vector keywords,
 						goto out;
 				}
 				if (keyword->handler) {
-				    t = keyword->handler(conf, strvec);
+				    t = keyword->handler(conf, strvec, file,
+							 line_nr);
 					r += t;
 					if (t)
 						condlog(1, "%s line %d, parsing failed: %s",
diff --git a/libmultipath/parser.h b/libmultipath/parser.h
index 3452bde1..11ea2278 100644
--- a/libmultipath/parser.h
+++ b/libmultipath/parser.h
@@ -43,7 +43,7 @@ struct strbuf;
 
 /* keyword definition */
 typedef int print_fn(struct config *, struct strbuf *, const void *);
-typedef int handler_fn(struct config *, vector);
+typedef int handler_fn(struct config *, vector, const char *file, int line_nr);
 
 struct keyword {
 	char *string;
-- 
2.17.2

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

[dm-devel] [PATCH v3 5/9] libmultipath: make set_int take a range for valid values

[Benjamin Marzinski]

If a value outside of the valid range is passed to set_int, it caps the
value at appropriate limit, and issues a warning.

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

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index eb2c44c0..57b6a7b6 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -29,7 +29,8 @@
 #include "strbuf.h"
 
 static int
-set_int(vector strvec, void *ptr, const char *file, int line_nr)
+set_int(vector strvec, void *ptr, int min, int max, const char *file,
+	int line_nr)
 {
 	int *int_ptr = (int *)ptr;
 	char *buff, *eptr;
@@ -44,11 +45,17 @@ set_int(vector strvec, void *ptr, const char *file, int line_nr)
 	if (eptr > buff)
 		while (isspace(*eptr))
 			eptr++;
-	if (*buff == '\0' || *eptr != '\0' || res > INT_MAX || res < INT_MIN) {
-		condlog(1, "%s: invalid value for %s: \"%s\"",
-			__func__, (char*)VECTOR_SLOT(strvec, 0), buff);
+	if (*buff == '\0' || *eptr != '\0') {
+		condlog(1, "%s line %d, invalid value for %s: \"%s\"",
+			file, line_nr, (char*)VECTOR_SLOT(strvec, 0), buff);
 		rc = 1;
 	} else {
+		if (res > max || res < min) {
+			res = (res > max) ? max : min;
+			condlog(1, "%s line %d, value for %s too %s, capping at %ld",
+			file, line_nr, (char*)VECTOR_SLOT(strvec, 0),
+			(res == max)? "large" : "small", res);
+		}
 		rc = 0;
 		*int_ptr = res;
 	}
@@ -77,8 +84,8 @@ set_uint(vector strvec, void *ptr, const char *file, int line_nr)
 		while (isspace(*eptr))
 			eptr++;
 	if (*buff == '\0' || *eptr != '\0' || !isdigit(*p) || res > UINT_MAX) {
-		condlog(1, "%s: invalid value for %s: \"%s\"",
-			__func__, (char*)VECTOR_SLOT(strvec, 0), buff);
+		condlog(1, "%s line %d, invalid value for %s: \"%s\"",
+			file, line_nr, (char*)VECTOR_SLOT(strvec, 0), buff);
 		rc = 1;
 	} else {
 		rc = 0;
@@ -193,6 +200,14 @@ def_ ## option ## _handler (struct config *conf, vector strvec,		\
 	return function (strvec, &conf->option, file, line_nr);		\
 }
 
+#define declare_def_range_handler(option, minval, maxval)			\
+static int								\
+def_ ## option ## _handler (struct config *conf, vector strvec,         \
+			    const char *file, int line_nr)		\
+{									\
+	return set_int(strvec, &conf->option, minval, maxval, file, line_nr); \
+}
+
 #define declare_def_snprint(option, function)				\
 static int								\
 snprint_def_ ## option (struct config *conf, struct strbuf *buff,	\
@@ -234,6 +249,18 @@ hw_ ## option ## _handler (struct config *conf, vector strvec,		\
 	return function (strvec, &hwe->option, file, line_nr);		\
 }
 
+#define declare_hw_range_handler(option, minval, maxval)		\
+static int								\
+hw_ ## option ## _handler (struct config *conf, vector strvec,		\
+			   const char *file, int line_nr)		\
+{									\
+	struct hwentry * hwe = VECTOR_LAST_SLOT(conf->hwtable);		\
+	if (!hwe)							\
+		return 1;						\
+	return set_int(strvec, &hwe->option, minval, maxval, file, line_nr); \
+}
+
+
 #define declare_hw_snprint(option, function)				\
 static int								\
 snprint_hw_ ## option (struct config *conf, struct strbuf *buff,	\
@@ -253,6 +280,17 @@ ovr_ ## option ## _handler (struct config *conf, vector strvec,		\
 	return function (strvec, &conf->overrides->option, file, line_nr); \
 }
 
+#define declare_ovr_range_handler(option, minval, maxval)		\
+static int								\
+ovr_ ## option ## _handler (struct config *conf, vector strvec,		\
+			    const char *file, int line_nr)		\
+{									\
+	if (!conf->overrides)						\
+		return 1;						\
+	return set_int(strvec, &conf->overrides->option, minval, maxval, \
+		       file, line_nr); \
+}
+
 #define declare_ovr_snprint(option, function)				\
 static int								\
 snprint_ovr_ ## option (struct config *conf, struct strbuf *buff,	\
@@ -272,6 +310,17 @@ mp_ ## option ## _handler (struct config *conf, vector strvec,		\
 	return function (strvec, &mpe->option, file, line_nr);		\
 }
 
+#define declare_mp_range_handler(option, minval, maxval)		\
+static int								\
+mp_ ## option ## _handler (struct config *conf, vector strvec,		\
+			   const char *file, int line_nr)		\
+{									\
+	struct mpentry * mpe = VECTOR_LAST_SLOT(conf->mptable);		\
+	if (!mpe)							\
+		return 1;						\
+	return set_int(strvec, &mpe->option, minval, maxval, file, line_nr); \
+}
+
 #define declare_mp_snprint(option, function)				\
 static int								\
 snprint_mp_ ## option (struct config *conf, struct strbuf *buff,	\
@@ -298,7 +347,7 @@ declare_def_snprint(checkint, print_int)
 declare_def_handler(max_checkint, set_uint)
 declare_def_snprint(max_checkint, print_int)
 
-declare_def_handler(verbosity, set_int)
+declare_def_range_handler(verbosity, 0, MAX_VERBOSITY)
 declare_def_snprint(verbosity, print_int)
 
 declare_def_handler(reassign_maps, set_yes_no)
@@ -473,22 +522,22 @@ declare_ovr_snprint(checker_name, print_str)
 declare_hw_handler(checker_name, set_str)
 declare_hw_snprint(checker_name, print_str)
 
-declare_def_handler(minio, set_int)
+declare_def_range_handler(minio, 0, INT_MAX)
 declare_def_snprint_defint(minio, print_int, DEFAULT_MINIO)
-declare_ovr_handler(minio, set_int)
+declare_ovr_range_handler(minio, 0, INT_MAX)
 declare_ovr_snprint(minio, print_nonzero)
-declare_hw_handler(minio, set_int)
+declare_hw_range_handler(minio, 0, INT_MAX)
 declare_hw_snprint(minio, print_nonzero)
-declare_mp_handler(minio, set_int)
+declare_mp_range_handler(minio, 0, INT_MAX)
 declare_mp_snprint(minio, print_nonzero)
 
-declare_def_handler(minio_rq, set_int)
+declare_def_range_handler(minio_rq, 0, INT_MAX)
 declare_def_snprint_defint(minio_rq, print_int, DEFAULT_MINIO_RQ)
-declare_ovr_handler(minio_rq, set_int)
+declare_ovr_range_handler(minio_rq, 0, INT_MAX)
 declare_ovr_snprint(minio_rq, print_nonzero)
-declare_hw_handler(minio_rq, set_int)
+declare_hw_range_handler(minio_rq, 0, INT_MAX)
 declare_hw_snprint(minio_rq, print_nonzero)
-declare_mp_handler(minio_rq, set_int)
+declare_mp_range_handler(minio_rq, 0, INT_MAX)
 declare_mp_snprint(minio_rq, print_nonzero)
 
 declare_def_handler(queue_without_daemon, set_yes_no)
@@ -512,7 +561,7 @@ snprint_def_queue_without_daemon(struct config *conf, struct strbuf *buff,
 	return append_strbuf_quoted(buff, qwd);
 }
 
-declare_def_handler(checker_timeout, set_int)
+declare_def_range_handler(checker_timeout, 0, INT_MAX)
 declare_def_snprint(checker_timeout, print_nonzero)
 
 declare_def_handler(allow_usb_devices, set_yes_no)
@@ -583,13 +632,13 @@ declare_hw_snprint(deferred_remove, print_yes_no_undef)
 declare_mp_handler(deferred_remove, set_yes_no_undef)
 declare_mp_snprint(deferred_remove, print_yes_no_undef)
 
-declare_def_handler(retrigger_tries, set_int)
+declare_def_range_handler(retrigger_tries, 0, INT_MAX)
 declare_def_snprint(retrigger_tries, print_int)
 
-declare_def_handler(retrigger_delay, set_int)
+declare_def_range_handler(retrigger_delay, 0, INT_MAX)
 declare_def_snprint(retrigger_delay, print_int)
 
-declare_def_handler(uev_wait_timeout, set_int)
+declare_def_range_handler(uev_wait_timeout, 0, INT_MAX)
 declare_def_snprint(uev_wait_timeout, print_int)
 
 declare_def_handler(strict_timing, set_yes_no)
@@ -616,19 +665,19 @@ static int snprint_def_disable_changed_wwids(struct config *conf,
 	return print_ignored(buff);
 }
 
-declare_def_handler(remove_retries, set_int)
+declare_def_range_handler(remove_retries, 0, INT_MAX)
 declare_def_snprint(remove_retries, print_int)
 
-declare_def_handler(max_sectors_kb, set_int)
+declare_def_range_handler(max_sectors_kb, 0, INT_MAX)
 declare_def_snprint(max_sectors_kb, print_nonzero)
-declare_ovr_handler(max_sectors_kb, set_int)
+declare_ovr_range_handler(max_sectors_kb, 0, INT_MAX)
 declare_ovr_snprint(max_sectors_kb, print_nonzero)
-declare_hw_handler(max_sectors_kb, set_int)
+declare_hw_range_handler(max_sectors_kb, 0, INT_MAX)
 declare_hw_snprint(max_sectors_kb, print_nonzero)
-declare_mp_handler(max_sectors_kb, set_int)
+declare_mp_range_handler(max_sectors_kb, 0, INT_MAX)
 declare_mp_snprint(max_sectors_kb, print_nonzero)
 
-declare_def_handler(find_multipaths_timeout, set_int)
+declare_def_range_handler(find_multipaths_timeout, INT_MIN, INT_MAX)
 declare_def_snprint_defint(find_multipaths_timeout, print_int,
 			   DEFAULT_FIND_MULTIPATHS_TIMEOUT)
 
@@ -1385,27 +1434,7 @@ declare_ovr_snprint(recheck_wwid, print_yes_no_undef)
 declare_hw_handler(recheck_wwid, set_yes_no_undef)
 declare_hw_snprint(recheck_wwid, print_yes_no_undef)
 
-
-static int
-def_uxsock_timeout_handler(struct config *conf, vector strvec, const char *file,
-			   int line_nr)
-{
-	unsigned int uxsock_timeout;
-	char *buff;
-
-	buff = set_value(strvec);
-	if (!buff)
-		return 1;
-
-	if (sscanf(buff, "%u", &uxsock_timeout) == 1 &&
-	    uxsock_timeout > DEFAULT_REPLY_TIMEOUT)
-		conf->uxsock_timeout = uxsock_timeout;
-	else
-		conf->uxsock_timeout = DEFAULT_REPLY_TIMEOUT;
-
-	free(buff);
-	return 0;
-}
+declare_def_range_handler(uxsock_timeout, DEFAULT_REPLY_TIMEOUT, INT_MAX)
 
 static int
 hw_vpd_vendor_handler(struct config *conf, vector strvec, const char *file,
-- 
2.17.2

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

[dm-devel] [PATCH v3 6/9] libmultipath: improve checks for set_str

[Benjamin Marzinski]

multipath always requires absolute pathnames, so make sure all file and
directory names start with a slash.  Also check that the directories
exist.  Finally, some strings, like the alias, will be used in paths.
These must not contain the slash character '/', since it is a forbidden
character in file/directory names. This patch adds seperate handlers for
these three cases. If a config line is invalid, these handlers retain
the existing config string, if any.

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

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 57b6a7b6..149d3348 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -5,6 +5,8 @@
  * Copyright (c) 2005 Kiyoshi Ueda, NEC
  */
 #include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
 #include <pwd.h>
 #include <string.h>
 #include "checkers.h"
@@ -111,6 +113,72 @@ set_str(vector strvec, void *ptr, const char *file, int line_nr)
 	return 0;
 }
 
+static int
+set_dir(vector strvec, void *ptr, const char *file, int line_nr)
+{
+	char **str_ptr = (char **)ptr;
+	char *old_str = *str_ptr;
+	struct stat sb;
+
+	*str_ptr = set_value(strvec);
+	if (!*str_ptr) {
+		free(old_str);
+		return 1;
+	}
+	if ((*str_ptr)[0] != '/'){
+		condlog(1, "%s line %d, %s is not an absolute directory path. Ignoring", file, line_nr, *str_ptr);
+		*str_ptr = old_str;
+	} else {
+		if (stat(*str_ptr, &sb) == 0 && S_ISDIR(sb.st_mode))
+			free(old_str);
+		else {
+			condlog(1, "%s line %d, %s is not an existing directory. Ignoring", file, line_nr, *str_ptr);
+			*str_ptr = old_str;
+		}
+	}
+	return 0;
+}
+
+static int
+set_path(vector strvec, void *ptr, const char *file, int line_nr)
+{
+	char **str_ptr = (char **)ptr;
+	char *old_str = *str_ptr;
+
+	*str_ptr = set_value(strvec);
+	if (!*str_ptr) {
+		free(old_str);
+		return 1;
+	}
+	if ((*str_ptr)[0] != '/'){
+		condlog(1, "%s line %d, %s is not an absolute path. Ignoring",
+			file, line_nr, *str_ptr);
+		*str_ptr = old_str;
+	} else
+		free(old_str);
+	return 0;
+}
+
+static int
+set_str_noslash(vector strvec, void *ptr, const char *file, int line_nr)
+{
+	char **str_ptr = (char **)ptr;
+	char *old_str = *str_ptr;
+
+	*str_ptr = set_value(strvec);
+	if (!*str_ptr) {
+		free(old_str);
+		return 1;
+	}
+	if (strchr(*str_ptr, '/')) {
+		condlog(1, "%s line %d, %s cannot contain a slash. Ignoring",
+			file, line_nr, *str_ptr);
+		*str_ptr = old_str;
+	} else
+		free(old_str);
+	return 0;
+}
+
 static int
 set_yes_no(vector strvec, void *ptr, const char *file, int line_nr)
 {
@@ -353,13 +421,13 @@ declare_def_snprint(verbosity, print_int)
 declare_def_handler(reassign_maps, set_yes_no)
 declare_def_snprint(reassign_maps, print_yes_no)
 
-declare_def_handler(multipath_dir, set_str)
+declare_def_handler(multipath_dir, set_dir)
 declare_def_snprint(multipath_dir, print_str)
 
 static int def_partition_delim_handler(struct config *conf, vector strvec,
 				       const char *file, int line_nr)
 {
-	int rc = set_str(strvec, &conf->partition_delim, file, line_nr);
+	int rc = set_str_noslash(strvec, &conf->partition_delim, file, line_nr);
 
 	if (rc != 0)
 		return rc;
@@ -490,11 +558,11 @@ declare_hw_snprint(prio_name, print_str)
 declare_mp_handler(prio_name, set_str)
 declare_mp_snprint(prio_name, print_str)
 
-declare_def_handler(alias_prefix, set_str)
+declare_def_handler(alias_prefix, set_str_noslash)
 declare_def_snprint_defstr(alias_prefix, print_str, DEFAULT_ALIAS_PREFIX)
-declare_ovr_handler(alias_prefix, set_str)
+declare_ovr_handler(alias_prefix, set_str_noslash)
 declare_ovr_snprint(alias_prefix, print_str)
-declare_hw_handler(alias_prefix, set_str)
+declare_hw_handler(alias_prefix, set_str_noslash)
 declare_hw_snprint(alias_prefix, print_str)
 
 declare_def_handler(prio_args, set_str)
@@ -586,13 +654,13 @@ declare_hw_snprint(user_friendly_names, print_yes_no_undef)
 declare_mp_handler(user_friendly_names, set_yes_no_undef)
 declare_mp_snprint(user_friendly_names, print_yes_no_undef)
 
-declare_def_handler(bindings_file, set_str)
+declare_def_handler(bindings_file, set_path)
 declare_def_snprint(bindings_file, print_str)
 
-declare_def_handler(wwids_file, set_str)
+declare_def_handler(wwids_file, set_path)
 declare_def_snprint(wwids_file, print_str)
 
-declare_def_handler(prkeys_file, set_str)
+declare_def_handler(prkeys_file, set_path)
 declare_def_snprint(prkeys_file, print_str)
 
 declare_def_handler(retain_hwhandler, set_yes_no_undef)
@@ -692,7 +760,7 @@ def_config_dir_handler(struct config *conf, vector strvec, const char *file,
 	/* this is only valid in the main config file */
 	if (conf->processed_main_config)
 		return 0;
-	return set_str(strvec, &conf->config_dir, file, line_nr);
+	return set_path(strvec, &conf->config_dir, file, line_nr);
 }
 declare_def_snprint(config_dir, print_str)
 
@@ -1732,7 +1800,7 @@ multipath_handler(struct config *conf, vector strvec, const char *file,
 declare_mp_handler(wwid, set_str)
 declare_mp_snprint(wwid, print_str)
 
-declare_mp_handler(alias, set_str)
+declare_mp_handler(alias, set_str_noslash)
 declare_mp_snprint(alias, print_str)
 
 /*
-- 
2.17.2

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

[dm-devel] [PATCH v3 7/9] libmultipath: deprecate file and directory config options

[Benjamin Marzinski]

Having multipath able to select pathnames for the files and directories
it needs causes unnecessary maintainer headaches. Mark these as
deprecated, but still support them for now.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/dict.c        | 19 +++++++++++++++----
 multipath/multipath.conf.5 |  5 +++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 149d3348..d14be340 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -268,6 +268,15 @@ def_ ## option ## _handler (struct config *conf, vector strvec,		\
 	return function (strvec, &conf->option, file, line_nr);		\
 }
 
+#define declare_def_warn_handler(option, function)			\
+static int								\
+def_ ## option ## _handler (struct config *conf, vector strvec,		\
+			    const char *file, int line_nr)		\
+{									\
+	condlog(2, "%s line %d, \"" #option "\" is deprecated and will be disabled in a future release", file, line_nr);				\
+	return function (strvec, &conf->option, file, line_nr);		\
+}
+
 #define declare_def_range_handler(option, minval, maxval)			\
 static int								\
 def_ ## option ## _handler (struct config *conf, vector strvec,         \
@@ -421,7 +430,7 @@ declare_def_snprint(verbosity, print_int)
 declare_def_handler(reassign_maps, set_yes_no)
 declare_def_snprint(reassign_maps, print_yes_no)
 
-declare_def_handler(multipath_dir, set_dir)
+declare_def_warn_handler(multipath_dir, set_dir)
 declare_def_snprint(multipath_dir, print_str)
 
 static int def_partition_delim_handler(struct config *conf, vector strvec,
@@ -654,13 +663,13 @@ declare_hw_snprint(user_friendly_names, print_yes_no_undef)
 declare_mp_handler(user_friendly_names, set_yes_no_undef)
 declare_mp_snprint(user_friendly_names, print_yes_no_undef)
 
-declare_def_handler(bindings_file, set_path)
+declare_def_warn_handler(bindings_file, set_path)
 declare_def_snprint(bindings_file, print_str)
 
-declare_def_handler(wwids_file, set_path)
+declare_def_warn_handler(wwids_file, set_path)
 declare_def_snprint(wwids_file, print_str)
 
-declare_def_handler(prkeys_file, set_path)
+declare_def_warn_handler(prkeys_file, set_path)
 declare_def_snprint(prkeys_file, print_str)
 
 declare_def_handler(retain_hwhandler, set_yes_no_undef)
@@ -760,6 +769,8 @@ def_config_dir_handler(struct config *conf, vector strvec, const char *file,
 	/* this is only valid in the main config file */
 	if (conf->processed_main_config)
 		return 0;
+	condlog(2, "%s line %d, \"config_dir\" is deprecated and will be disabled in a future release",
+		file, line_nr);
 	return set_path(strvec, &conf->config_dir, file, line_nr);
 }
 declare_def_snprint(config_dir, print_str)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 42a15ffd..17771e27 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -178,6 +178,7 @@ The default is: \fBno\fR
 .
 .TP
 .B multipath_dir
+This option is deprecated, and will be removed in a future release.
 Directory where the dynamic shared objects are stored. Defined at compile time,
 commonly \fI/lib64/multipath/\fR or \fI/lib/multipath/\fR.
 .RS
@@ -742,6 +743,7 @@ The default is: \fB<unset>\fR
 .
 .TP
 .B bindings_file
+This option is deprecated, and will be removed in a future release.
 The full pathname of the binding file to be used when the user_friendly_names
 option is set.
 .RS
@@ -752,6 +754,7 @@ The default is: \fB/etc/multipath/bindings\fR
 .
 .TP
 .B wwids_file
+This option is deprecated, and will be removed in a future release.
 The full pathname of the WWIDs file, which is used by multipath to keep track
 of the WWIDs for LUNs it has created multipath devices on in the past.
 .RS
@@ -762,6 +765,7 @@ The default is: \fB/etc/multipath/wwids\fR
 .
 .TP
 .B prkeys_file
+This option is deprecated, and will be removed in a future release.
 The full pathname of the prkeys file, which is used by multipathd to keep
 track of the persistent reservation key used for a specific WWID, when
 \fIreservation_key\fR is set to \fBfile\fR.
@@ -933,6 +937,7 @@ The default is: \fB<unset>\fR
 .
 .TP
 .B config_dir
+This option is deprecated, and will be removed in a future release.
 If set to anything other than "", multipath will search this directory
 alphabetically for file ending in ".conf" and it will read configuration
 information from them, just as if it was in \fI/etc/multipath.conf\fR.
-- 
2.17.2

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

[dm-devel] [PATCH v3 8/9] libmultipath: split set_int to enable reuse

[Benjamin Marzinski]

Split the code that does the actual value parsing out of set_int(), into
a helper function, do_set_int(), so that it can be used by other
handlers. These functions no longer set the config value at all, when
they have invalid input.

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

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index d14be340..68647061 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -31,17 +31,12 @@
 #include "strbuf.h"
 
 static int
-set_int(vector strvec, void *ptr, int min, int max, const char *file,
-	int line_nr)
+do_set_int(vector strvec, void *ptr, int min, int max, const char *file,
+	int line_nr, char *buff)
 {
 	int *int_ptr = (int *)ptr;
-	char *buff, *eptr;
+	char *eptr;
 	long res;
-	int rc;
-
-	buff = set_value(strvec);
-	if (!buff)
-		return 1;
 
 	res = strtol(buff, &eptr, 10);
 	if (eptr > buff)
@@ -50,17 +45,30 @@ set_int(vector strvec, void *ptr, int min, int max, const char *file,
 	if (*buff == '\0' || *eptr != '\0') {
 		condlog(1, "%s line %d, invalid value for %s: \"%s\"",
 			file, line_nr, (char*)VECTOR_SLOT(strvec, 0), buff);
-		rc = 1;
-	} else {
-		if (res > max || res < min) {
-			res = (res > max) ? max : min;
-			condlog(1, "%s line %d, value for %s too %s, capping at %ld",
+		return 1;
+	}
+	if (res > max || res < min) {
+		res = (res > max) ? max : min;
+		condlog(1, "%s line %d, value for %s too %s, capping at %ld",
 			file, line_nr, (char*)VECTOR_SLOT(strvec, 0),
-			(res == max)? "large" : "small", res);
-		}
-		rc = 0;
-		*int_ptr = res;
+		(res == max)? "large" : "small", res);
 	}
+	*int_ptr = res;
+	return 0;
+}
+
+static int
+set_int(vector strvec, void *ptr, int min, int max, const char *file,
+	int line_nr)
+{
+	char *buff;
+	int rc;
+
+	buff = set_value(strvec);
+	if (!buff)
+		return 1;
+
+	rc = do_set_int(strvec, ptr, min, max, file, line_nr, buff);
 
 	FREE(buff);
 	return rc;
@@ -929,6 +937,7 @@ declare_mp_attr_snprint(gid, print_gid)
 static int
 set_undef_off_zero(vector strvec, void *ptr, const char *file, int line_nr)
 {
+	int rc = 0;
 	char * buff;
 	int *int_ptr = (int *)ptr;
 
@@ -938,10 +947,10 @@ set_undef_off_zero(vector strvec, void *ptr, const char *file, int line_nr)
 
 	if (strcmp(buff, "off") == 0)
 		*int_ptr = UOZ_OFF;
-	else if (sscanf(buff, "%d", int_ptr) != 1 ||
-		 *int_ptr < UOZ_ZERO)
-		*int_ptr = UOZ_UNDEF;
-	else if (*int_ptr == 0)
+	else
+		rc = do_set_int(strvec, int_ptr, 0, INT_MAX, file, line_nr,
+				buff);
+	if (rc == 0 && *int_ptr == 0)
 		*int_ptr = UOZ_ZERO;
 
 	FREE(buff);
@@ -1093,14 +1102,12 @@ max_fds_handler(struct config *conf, vector strvec, const char *file,
 		/* Assume safe limit */
 		max_fds = 4096;
 	}
-	if (strlen(buff) == 3 &&
-	    !strcmp(buff, "max"))
-		conf->max_fds = max_fds;
-	else
-		conf->max_fds = atoi(buff);
-
-	if (conf->max_fds > max_fds)
+	if (!strcmp(buff, "max")) {
 		conf->max_fds = max_fds;
+		r = 0;
+	} else
+		r = do_set_int(strvec, &conf->max_fds, 0, max_fds, file,
+			       line_nr, buff);
 
 	FREE(buff);
 
@@ -1169,6 +1176,7 @@ declare_mp_snprint(rr_weight, print_rr_weight)
 static int
 set_pgfailback(vector strvec, void *ptr, const char *file, int line_nr)
 {
+	int rc = 0;
 	int *int_ptr = (int *)ptr;
 	char * buff;
 
@@ -1183,11 +1191,11 @@ set_pgfailback(vector strvec, void *ptr, const char *file, int line_nr)
 	else if (strlen(buff) == 10 && !strcmp(buff, "followover"))
 		*int_ptr = -FAILBACK_FOLLOWOVER;
 	else
-		*int_ptr = atoi(buff);
+		rc = do_set_int(strvec, ptr, 0, INT_MAX, file, line_nr, buff);
 
 	FREE(buff);
 
-	return 0;
+	return rc;
 }
 
 int
@@ -1219,6 +1227,7 @@ declare_mp_snprint(pgfailback, print_pgfailback)
 static int
 no_path_retry_helper(vector strvec, void *ptr, const char *file, int line_nr)
 {
+	int rc = 0;
 	int *int_ptr = (int *)ptr;
 	char * buff;
 
@@ -1230,11 +1239,11 @@ no_path_retry_helper(vector strvec, void *ptr, const char *file, int line_nr)
 		*int_ptr = NO_PATH_RETRY_FAIL;
 	else if (!strcmp(buff, "queue"))
 		*int_ptr = NO_PATH_RETRY_QUEUE;
-	else if ((*int_ptr = atoi(buff)) < 1)
-		*int_ptr = NO_PATH_RETRY_UNDEF;
+	else
+		rc = do_set_int(strvec, ptr, 1, INT_MAX, file, line_nr, buff);
 
 	FREE(buff);
-	return 0;
+	return rc;
 }
 
 int
@@ -1376,6 +1385,7 @@ snprint_mp_reservation_key (struct config *conf, struct strbuf *buff,
 static int
 set_off_int_undef(vector strvec, void *ptr, const char *file, int line_nr)
 {
+	int rc =0;
 	int *int_ptr = (int *)ptr;
 	char * buff;
 
@@ -1385,11 +1395,11 @@ set_off_int_undef(vector strvec, void *ptr, const char *file, int line_nr)
 
 	if (!strcmp(buff, "no") || !strcmp(buff, "0"))
 		*int_ptr = NU_NO;
-	else if ((*int_ptr = atoi(buff)) < 1)
-		*int_ptr = NU_UNDEF;
+	else
+		rc = do_set_int(strvec, ptr, 1, INT_MAX, file, line_nr, buff);
 
 	FREE(buff);
-	return 0;
+	return rc;
 }
 
 int
-- 
2.17.2

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

[dm-devel] [PATCH v3 9/9] libmultipath: cleanup invalid config handling

[Benjamin Marzinski]

Add error reporting to the remaining config handlers. If the value is
invalid, do not change the existing config option's value. Also print
an error whenever 0 is returned for an invalid value. When the handler
returns 1, config processing already fails with an error message.

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

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 68647061..c534d703 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -199,8 +199,11 @@ set_yes_no(vector strvec, void *ptr, const char *file, int line_nr)
 
 	if (strcmp(buff, "yes") == 0 || strcmp(buff, "1") == 0)
 		*int_ptr = YN_YES;
-	else
+	else if (strcmp(buff, "no") == 0 || strcmp(buff, "0") == 0)
 		*int_ptr = YN_NO;
+	else
+		condlog(1, "%s line %d, invalid value for %s: \"%s\"",
+			file, line_nr, (char*)VECTOR_SLOT(strvec, 0), buff);
 
 	FREE(buff);
 	return 0;
@@ -221,7 +224,8 @@ set_yes_no_undef(vector strvec, void *ptr, const char *file, int line_nr)
 	else if (strcmp(buff, "yes") == 0 || strcmp(buff, "1") == 0)
 		*int_ptr = YNU_YES;
 	else
-		*int_ptr = YNU_UNDEF;
+		condlog(1, "%s line %d, invalid value for %s: \"%s\"",
+			file, line_nr, (char*)VECTOR_SLOT(strvec, 0), buff);
 
 	FREE(buff);
 	return 0;
@@ -480,9 +484,6 @@ def_find_multipaths_handler(struct config *conf, vector strvec,
 	char *buff;
 	int i;
 
-	if (set_yes_no_undef(strvec, &conf->find_multipaths, file, line_nr) == 0 && conf->find_multipaths != FIND_MULTIPATHS_UNDEF)
-		return 0;
-
 	buff = set_value(strvec);
 	if (!buff)
 		return 1;
@@ -495,9 +496,14 @@ def_find_multipaths_handler(struct config *conf, vector strvec,
 		}
 	}
 
-	if (conf->find_multipaths == YNU_UNDEF) {
-		condlog(0, "illegal value for find_multipaths: %s", buff);
-		conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
+	if (i >= __FIND_MULTIPATHS_LAST) {
+		if (strcmp(buff, "no") == 0 || strcmp(buff, "0") == 0)
+			conf->find_multipaths = FIND_MULTIPATHS_OFF;
+		else if (strcmp(buff, "yes") == 0 || strcmp(buff, "1") == 0)
+			conf->find_multipaths = FIND_MULTIPATHS_ON;
+		else
+			condlog(1, "%s line %d, invalid value for find_multipaths: \"%s\"",
+				file, line_nr, buff);
 	}
 
 	FREE(buff);
@@ -546,8 +552,10 @@ static int uid_attrs_handler(struct config *conf, vector strvec,
 	if (!val)
 		return 1;
 	if (parse_uid_attrs(val, conf))
-		condlog(1, "error parsing uid_attrs: \"%s\"", val);
-	condlog(3, "parsed %d uid_attrs", VECTOR_SIZE(&conf->uid_attrs));
+		condlog(1, "%s line %d,error parsing uid_attrs: \"%s\"", file,
+			line_nr, val);
+	else
+		condlog(4, "parsed %d uid_attrs", VECTOR_SIZE(&conf->uid_attrs));
 	FREE(val);
 	return 0;
 }
@@ -775,8 +783,11 @@ def_config_dir_handler(struct config *conf, vector strvec, const char *file,
 		       int line_nr)
 {
 	/* this is only valid in the main config file */
-	if (conf->processed_main_config)
+	if (conf->processed_main_config) {
+		condlog(1, "%s line %d, config_dir option only valid in /etc/multipath.conf",
+			file, line_nr);
 		return 0;
+	}
 	condlog(2, "%s line %d, \"config_dir\" is deprecated and will be disabled in a future release",
 		file, line_nr);
 	return set_path(strvec, &conf->config_dir, file, line_nr);
@@ -836,7 +847,9 @@ set_mode(vector strvec, void *ptr, int *flags, const char *file, int line_nr)
 	if (sscanf(buff, "%o", &mode) == 1 && mode <= 0777) {
 		*flags |= (1 << ATTR_MODE);
 		*mode_ptr = mode;
-	}
+	} else
+		condlog(1, "%s line %d, invalid value for mode: \"%s\"",
+			file, line_nr, buff);
 
 	FREE(buff);
 	return 0;
@@ -861,7 +874,9 @@ set_uid(vector strvec, void *ptr, int *flags, const char *file, int line_nr)
 	else if (sscanf(buff, "%u", &uid) == 1){
 		*flags |= (1 << ATTR_UID);
 		*uid_ptr = uid;
-	}
+	} else
+		condlog(1, "%s line %d, invalid value for uid: \"%s\"",
+			file, line_nr, buff);
 
 	FREE(buff);
 	return 0;
@@ -887,7 +902,9 @@ set_gid(vector strvec, void *ptr, int *flags, const char *file, int line_nr)
 	else if (sscanf(buff, "%u", &gid) == 1){
 		*flags |= (1 << ATTR_GID);
 		*gid_ptr = gid;
-	}
+	} else
+		condlog(1, "%s line %d, invalid value for gid: \"%s\"",
+			file, line_nr, buff);
 	FREE(buff);
 	return 0;
 }
@@ -989,7 +1006,8 @@ set_dev_loss(vector strvec, void *ptr, const char *file, int line_nr)
 	if (!strcmp(buff, "infinity"))
 		*uint_ptr = MAX_DEV_LOSS_TMO;
 	else if (sscanf(buff, "%u", uint_ptr) != 1)
-		*uint_ptr = DEV_LOSS_TMO_UNSET;
+		condlog(1, "%s line %d, invalid value for dev_loss_tmo: \"%s\"",
+			file, line_nr, buff);
 
 	FREE(buff);
 	return 0;
@@ -1023,13 +1041,19 @@ static int
 set_pgpolicy(vector strvec, void *ptr, const char *file, int line_nr)
 {
 	char * buff;
+	int policy;
 	int *int_ptr = (int *)ptr;
 
 	buff = set_value(strvec);
 	if (!buff)
 		return 1;
 
-	*int_ptr = get_pgpolicy_id(buff);
+	policy = get_pgpolicy_id(buff);
+	if (policy != IOPOLICY_UNDEF)
+		*int_ptr = policy;
+	else
+		condlog(1, "%s line %d, invalid value for path_grouping_policy: \"%s\"",
+			file, line_nr, buff);
 	FREE(buff);
 
 	return 0;
@@ -1142,10 +1166,11 @@ set_rr_weight(vector strvec, void *ptr, const char *file, int line_nr)
 
 	if (!strcmp(buff, "priorities"))
 		*int_ptr = RR_WEIGHT_PRIO;
-
-	if (!strcmp(buff, "uniform"))
+	else if (!strcmp(buff, "uniform"))
 		*int_ptr = RR_WEIGHT_NONE;
-
+	else
+		condlog(1, "%s line %d, invalid value for rr_weight: \"%s\"",
+			file, line_nr, buff);
 	FREE(buff);
 
 	return 0;
@@ -1281,10 +1306,13 @@ def_log_checker_err_handler(struct config *conf, vector strvec,
 	if (!buff)
 		return 1;
 
-	if (strlen(buff) == 4 && !strcmp(buff, "once"))
+	if (!strcmp(buff, "once"))
 		conf->log_checker_err = LOG_CHKR_ERR_ONCE;
-	else if (strlen(buff) == 6 && !strcmp(buff, "always"))
+	else if (!strcmp(buff, "always"))
 		conf->log_checker_err = LOG_CHKR_ERR_ALWAYS;
+	else
+		condlog(1, "%s line %d, invalid value for log_checker_err: \"%s\"",
+			file, line_nr, buff);
 
 	free(buff);
 	return 0;
@@ -1545,7 +1573,8 @@ hw_vpd_vendor_handler(struct config *conf, vector strvec, const char *file,
 			goto out;
 		}
 	}
-	hwe->vpd_vendor_id = 0;
+	condlog(1, "%s line %d, invalid value for vpd_vendor: \"%s\"",
+		file, line_nr, buff);
 out:
 	FREE(buff);
 	return 0;
-- 
2.17.2

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

Re: [dm-devel] [PATCH v3 7/9] libmultipath: deprecate file and directory config options

[Martin Wilck]

On Thu, 2021-11-11 at 12:53 -0600, Benjamin Marzinski wrote:
> Having multipath able to select pathnames for the files and directories
> it needs causes unnecessary maintainer headaches. Mark these as
> deprecated, but still support them for now.
> 
> 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