From d39aaedf5b722de182b2c2b98675759b64fa6237 Mon Sep 17 00:00:00 2001 From: Martin Schwenke 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 Reviewed-by: Amitay Isaacs (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 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 Reviewed-by: Amitay Isaacs (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 < 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 Reviewed-by: Amitay Isaacs (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" < "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" < "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 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 Reviewed-by: Amitay Isaacs (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 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 Reviewed-by: Amitay Isaacs (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 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 Reviewed-by: Amitay Isaacs (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" < "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" < "unknown key" Failed to load config file $conffile EOF -- 2.18.0 From 4816c6e8b34dfd7e469682db2c9d70ac689a7a6e Mon Sep 17 00:00:00 2001 From: Martin Schwenke 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 Reviewed-by: Amitay Isaacs (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 <