Attachment 14463 Details for Bug 13589 – Additional patches for 4.9

[patch] Additional patches for 4.9

BZ13589-4.9.patch (text/plain), 13.07 KB, created by Martin Schwenke on 2018-09-04 00:49:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2018-09-04 00:49:24 UTC

Size: 13.07 KB

patch

obsolete

>From d39aaedf5b722de182b2c2b98675759b64fa6237 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 09:06:51 +1000
>Subject: [PATCH 1/7] ctdb-daemon: Drop incorrect log message
>
>The message is incorrect because the actual failure was loading the
>config file.  Instead of fixing the message, drop it because
>ctdb_config_load() already logs the failure.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit b5453bc27add11a7288772a59adcc605328b9098)
>---
> ctdb/server/ctdbd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/ctdb/server/ctdbd.c b/ctdb/server/ctdbd.c
>index ef829e5b233..721347c4733 100644
>--- a/ctdb/server/ctdbd.c
>+++ b/ctdb/server/ctdbd.c
>@@ -223,7 +223,7 @@ int main(int argc, const char *argv[])
> 
> 	ret = ctdbd_config_load(ctdb, &conf);
> 	if (ret != 0) {
>-		fprintf(stderr, "Failed to setup config file handling\n");
>+		/* ctdbd_config_load() logs the failure */
> 		goto fail;
> 	}
> 
>-- 
>2.18.0
>
>
>From ece59d13454af8ccd1197a1e917c8103f478395a Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 19:57:56 +1000
>Subject: [PATCH 2/7] ctdb-common: Fix log message for conf option with unknown
> section
>
>This covers both options that appear before a section and options in
>unknown sections.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit 421d828f6cb7c13d5f33c6cc1c6be254554588a4)
>---
> ctdb/common/conf.c                  | 5 +++--
> ctdb/tests/cunit/config_test_001.sh | 2 +-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c
>index 669ac23a74b..a6e0f830328 100644
>--- a/ctdb/common/conf.c
>+++ b/ctdb/common/conf.c
>@@ -1099,11 +1099,12 @@ static bool conf_load_option(const char *name,
> 
> 	if (state->s == NULL) {
> 		if (state->conf->ignore_unknown) {
>-			D_DEBUG("conf: ignoring unknown option \"%s\"\n",
>+			D_DEBUG("conf: unknown section for option \"%s\"\n",
> 				name);
> 			return true;
> 		} else {
>-			D_ERR("conf: unknown option \"%s\"\n", name);
>+			D_ERR("conf: unknown section for option \"%s\"\n",
>+			      name);
> 			state->err = EINVAL;
> 			return false;
> 		}
>diff --git a/ctdb/tests/cunit/config_test_001.sh b/ctdb/tests/cunit/config_test_001.sh
>index bf1e589c318..d5536bc7f2a 100755
>--- a/ctdb/tests/cunit/config_test_001.sh
>+++ b/ctdb/tests/cunit/config_test_001.sh
>@@ -81,7 +81,7 @@ foobar = cat
> EOF
> 
> required_result 22 <<EOF
>-conf: unknown option "foobar"
>+conf: unknown section for option "foobar"
> Failed to load config file $conffile
> EOF
> unit_test ctdb-config validate
>-- 
>2.18.0
>
>
>From dc4ce0004763e9bcd53f77857656f7cf840ada7b Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 08:32:12 +1000
>Subject: [PATCH 3/7] ctdb-common: Log a message for unknown conf option
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit ebb28c57a17777ea15afab63cd0742dd79b30ffe)
>---
> ctdb/common/conf.c                  | 6 ++++++
> ctdb/tests/cunit/conf_test_001.sh   | 1 +
> ctdb/tests/cunit/config_test_001.sh | 1 +
> 3 files changed, 8 insertions(+)
>
>diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c
>index a6e0f830328..cd1fa4acd2b 100644
>--- a/ctdb/common/conf.c
>+++ b/ctdb/common/conf.c
>@@ -1113,8 +1113,14 @@ static bool conf_load_option(const char *name,
> 	opt = conf_option_find(state->s, name);
> 	if (opt == NULL) {
> 		if (state->conf->ignore_unknown) {
>+			D_DEBUG("conf: unknown option [%s] -> \"%s\"\n",
>+				state->s->name,
>+				name);
> 			return true;
> 		} else {
>+			D_ERR("conf: unknown option [%s] -> \"%s\"\n",
>+			      state->s->name,
>+			      name);
> 			state->err = ENOENT;
> 			return false;
> 		}
>diff --git a/ctdb/tests/cunit/conf_test_001.sh b/ctdb/tests/cunit/conf_test_001.sh
>index 08a51b00d9b..a1a84585b1e 100755
>--- a/ctdb/tests/cunit/conf_test_001.sh
>+++ b/ctdb/tests/cunit/conf_test_001.sh
>@@ -121,6 +121,7 @@ cat > "$conffile" <<EOF
> EOF
> 
> required_result 2 <<EOF
>+conf: unknown option [section1] -> "foo"
> [section1]
> 	# key1 = value1
> 	# key2 = 10
>diff --git a/ctdb/tests/cunit/config_test_001.sh b/ctdb/tests/cunit/config_test_001.sh
>index d5536bc7f2a..e58f710ac4d 100755
>--- a/ctdb/tests/cunit/config_test_001.sh
>+++ b/ctdb/tests/cunit/config_test_001.sh
>@@ -102,6 +102,7 @@ cat > "$conffile" <<EOF
> EOF
> 
> required_result 2 <<EOF
>+conf: unknown option [database] -> "unknown key"
> Failed to load config file $conffile
> EOF
> unit_test ctdb-config validate
>-- 
>2.18.0
>
>
>From eceeabf71c5bf71ddd00e9fa7f05d8f501a298d3 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 09:34:12 +1000
>Subject: [PATCH 4/7] ctdb-common: Log a message when an invalid conf value is
> encountered
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit a017d3181ac1062b66ae506a8a523f7455630fce)
>---
> ctdb/common/conf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c
>index cd1fa4acd2b..f673c61fa8d 100644
>--- a/ctdb/common/conf.c
>+++ b/ctdb/common/conf.c
>@@ -1135,6 +1135,10 @@ static bool conf_load_option(const char *name,
> 	value.type = opt->type;
> 	ret = conf_value_from_string(tmp_ctx, value_str, &value);
> 	if (ret != 0) {
>+		D_ERR("conf: invalid value [%s] -> \"%s\" = \"%s\"\n",
>+		      state->s->name,
>+		      name,
>+		      value_str);
> 		talloc_free(tmp_ctx);
> 		state->err = ret;
> 		return false;
>-- 
>2.18.0
>
>
>From 0b82cef475a8b65172e9eb74ff0cac925d7a2dde Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 08:45:25 +1000
>Subject: [PATCH 5/7] ctdb-common: Avoid ENOENT for unknown conf type tags
>
>Only use ENOENT for missing configuration file.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit f1084400387c0b1257b6d92ee6e8a89504d788fc)
>---
> ctdb/common/conf.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c
>index f673c61fa8d..2758d2628f2 100644
>--- a/ctdb/common/conf.c
>+++ b/ctdb/common/conf.c
>@@ -155,7 +155,7 @@ static int conf_value_from_string(TALLOC_CTX *mem_ctx,
> 		break;
> 
> 	default:
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	return ret;
>@@ -232,7 +232,7 @@ static int conf_value_copy(TALLOC_CTX *mem_ctx,
> 		break;
> 
> 	default:
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	return 0;
>-- 
>2.18.0
>
>
>From 4337b4ef3507a627e88371bcc38bcdc6f7b69b53 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 08:42:04 +1000
>Subject: [PATCH 6/7] ctdb-common: Avoid ENOENT for unknown conf options
>
>Only use ENOENT for missing configuration file.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit 920ed66ba7e874ca23d72fff9342fbd64a1e329f)
>---
> ctdb/common/conf.c                  | 12 ++++++------
> ctdb/tests/cunit/conf_test_001.sh   |  2 +-
> ctdb/tests/cunit/config_test_001.sh |  2 +-
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
>diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c
>index 2758d2628f2..8bc375b166d 100644
>--- a/ctdb/common/conf.c
>+++ b/ctdb/common/conf.c
>@@ -1121,7 +1121,7 @@ static bool conf_load_option(const char *name,
> 			D_ERR("conf: unknown option [%s] -> \"%s\"\n",
> 			      state->s->name,
> 			      name);
>-			state->err = ENOENT;
>+			state->err = EINVAL;
> 			return false;
> 		}
> 	}
>@@ -1207,16 +1207,16 @@ static int conf_set(struct conf_context *conf,
> 
> 	s = conf_section_find(conf, section);
> 	if (s == NULL) {
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	opt = conf_option_find(s, key);
> 	if (opt == NULL) {
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	if (opt->type != value->type) {
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	ok = conf_option_same_value(opt, value);
>@@ -1291,12 +1291,12 @@ static int conf_get(struct conf_context *conf,
> 
> 	s = conf_section_find(conf, section);
> 	if (s == NULL) {
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	opt = conf_option_find(s, key);
> 	if (opt == NULL) {
>-		return ENOENT;
>+		return EINVAL;
> 	}
> 
> 	if (opt->type != type) {
>diff --git a/ctdb/tests/cunit/conf_test_001.sh b/ctdb/tests/cunit/conf_test_001.sh
>index a1a84585b1e..caca9314953 100755
>--- a/ctdb/tests/cunit/conf_test_001.sh
>+++ b/ctdb/tests/cunit/conf_test_001.sh
>@@ -120,7 +120,7 @@ cat > "$conffile" <<EOF
>     key2 = 20
> EOF
> 
>-required_result 2 <<EOF
>+required_error EINVAL <<EOF
> conf: unknown option [section1] -> "foo"
> [section1]
> 	# key1 = value1
>diff --git a/ctdb/tests/cunit/config_test_001.sh b/ctdb/tests/cunit/config_test_001.sh
>index e58f710ac4d..d213aa88bfd 100755
>--- a/ctdb/tests/cunit/config_test_001.sh
>+++ b/ctdb/tests/cunit/config_test_001.sh
>@@ -101,7 +101,7 @@ cat > "$conffile" <<EOF
> 	log level = debug
> EOF
> 
>-required_result 2 <<EOF
>+required_error EINVAL <<EOF
> conf: unknown option [database] -> "unknown key"
> Failed to load config file $conffile
> EOF
>-- 
>2.18.0
>
>
>From 4816c6e8b34dfd7e469682db2c9d70ac689a7a6e Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Fri, 31 Aug 2018 09:35:14 +1000
>Subject: [PATCH 7/7] ctdb-common: Process the whole config file even if an
> error occurs
>
>At the moment multiple errors will be encountered one at a time, on
>each load or validate.  Instead, allow all configuration errors to
>printed in a single pass.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13589
>
>Signed-off-by: Martin Schwenke <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(cherry picked from commit 4f1727fe0bf2b0962a5d131d60a416b8f459ad94)
>---
> ctdb/common/conf.c                  | 16 ++++++++++------
> ctdb/tests/cunit/conf_test_001.sh   |  1 +
> ctdb/tests/cunit/config_test_005.sh |  6 ++++++
> 3 files changed, 17 insertions(+), 6 deletions(-)
>
>diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c
>index 8bc375b166d..3d668de3122 100644
>--- a/ctdb/common/conf.c
>+++ b/ctdb/common/conf.c
>@@ -1048,6 +1048,10 @@ static int conf_load_internal(struct conf_context *conf)
> 		}
> 	}
> 
>+	if (state.err != 0) {
>+		goto fail;
>+	}
>+
> 	conf_all_update(conf);
> 	return 0;
> 
>@@ -1066,7 +1070,7 @@ static bool conf_load_section(const char *section, void *private_data)
> 		ok = conf_section_validate(state->conf, state->s, state->mode);
> 		if (!ok) {
> 			state->err = EINVAL;
>-			return false;
>+			return true;
> 		}
> 	}
> 
>@@ -1078,7 +1082,7 @@ static bool conf_load_section(const char *section, void *private_data)
> 		} else {
> 			D_ERR("conf: unknown section [%s]\n", section);
> 			state->err = EINVAL;
>-			return false;
>+			return true;
> 		}
> 	}
> 
>@@ -1106,7 +1110,7 @@ static bool conf_load_option(const char *name,
> 			D_ERR("conf: unknown section for option \"%s\"\n",
> 			      name);
> 			state->err = EINVAL;
>-			return false;
>+			return true;
> 		}
> 	}
> 
>@@ -1122,7 +1126,7 @@ static bool conf_load_option(const char *name,
> 			      state->s->name,
> 			      name);
> 			state->err = EINVAL;
>-			return false;
>+			return true;
> 		}
> 	}
> 
>@@ -1141,7 +1145,7 @@ static bool conf_load_option(const char *name,
> 		      value_str);
> 		talloc_free(tmp_ctx);
> 		state->err = ret;
>-		return false;
>+		return true;
> 	}
> 
> 	ok = conf_option_same_value(opt, &value);
>@@ -1153,7 +1157,7 @@ static bool conf_load_option(const char *name,
> 	if (ret != 0) {
> 		talloc_free(tmp_ctx);
> 		state->err = ret;
>-		return false;
>+		return true;
> 	}
> 
> done:
>diff --git a/ctdb/tests/cunit/conf_test_001.sh b/ctdb/tests/cunit/conf_test_001.sh
>index caca9314953..d2ffa988745 100755
>--- a/ctdb/tests/cunit/conf_test_001.sh
>+++ b/ctdb/tests/cunit/conf_test_001.sh
>@@ -106,6 +106,7 @@ EOF
> 
> required_result 22 <<EOF
> conf: unknown section [section2]
>+conf: unknown section for option "foo"
> [section1]
> 	# key1 = value1
> 	# key2 = 10
>diff --git a/ctdb/tests/cunit/config_test_005.sh b/ctdb/tests/cunit/config_test_005.sh
>index df636766b40..cd48a84a78a 100755
>--- a/ctdb/tests/cunit/config_test_005.sh
>+++ b/ctdb/tests/cunit/config_test_005.sh
>@@ -30,6 +30,10 @@ EOF
> required_result 22 <<EOF
> volatile database directory "${dbdir_volatile}" does not exist
> conf: validation for option "volatile database directory" failed
>+persistent database directory "${dbdir_persistent}" does not exist
>+conf: validation for option "persistent database directory" failed
>+state database directory "${dbdir_state}" does not exist
>+conf: validation for option "state database directory" failed
> Failed to load config file $conffile
> EOF
> unit_test ctdb-config validate
>@@ -39,6 +43,8 @@ mkdir -p "$dbdir_volatile"
> required_result 22 <<EOF
> persistent database directory "${dbdir_persistent}" does not exist
> conf: validation for option "persistent database directory" failed
>+state database directory "${dbdir_state}" does not exist
>+conf: validation for option "state database directory" failed
> Failed to load config file $conffile
> EOF
> unit_test ctdb-config validate
>-- 
>2.18.0
>

Flags:

amitay: review+

Actions: View

Attachments on bug 13589: 14447 | 14448 | 14463