From e84b5020a410b9c2cf736efee60ee704f8ea8f9c Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Mon, 25 Jun 2018 12:56:45 +1000 Subject: [PATCH] ctdb-common: Correctly handle conf->reload() Configuration reload should reset the values of configuration options missing from the config file to default. Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke Autobuild-User(master): Martin Schwenke Autobuild-Date(master): Fri Jun 29 15:12:37 CEST 2018 on sn-devel-144 --- ctdb/common/conf.c | 147 +++++++++++++++++++++++++++++++++----- ctdb/tests/cunit/conf_test_001.sh | 48 ++++++++++++- ctdb/tests/src/conf_test.c | 53 ++++++++++++++ 3 files changed, 226 insertions(+), 22 deletions(-) diff --git a/ctdb/common/conf.c b/ctdb/common/conf.c index dccf6613f48..3c1369e138d 100644 --- a/ctdb/common/conf.c +++ b/ctdb/common/conf.c @@ -161,6 +161,44 @@ static int conf_value_from_string(TALLOC_CTX *mem_ctx, return ret; } +static bool conf_value_compare(struct conf_value *old, struct conf_value *new) +{ + if (old == NULL || new == NULL) { + return false; + } + + if (old->type != new->type) { + return false; + } + + switch (old->type) { + case CONF_STRING: + if (old->data.string == NULL && new->data.string == NULL) { + return true; + } + if (old->data.string != NULL && new->data.string != NULL) { + if (strcmp(old->data.string, new->data.string) == 0) { + return true; + } + } + break; + + case CONF_INTEGER: + if (old->data.integer == new->data.integer) { + return true; + } + break; + + case CONF_BOOLEAN: + if (old->data.boolean == new->data.boolean) { + return true; + } + break; + } + + return false; +} + static int conf_value_copy(TALLOC_CTX *mem_ctx, struct conf_value *src, struct conf_value *dst) @@ -409,6 +447,12 @@ static bool conf_option_validate(struct conf_option *opt, return ret; } +static bool conf_option_same_value(struct conf_option *opt, + struct conf_value *new_value) +{ + return conf_value_compare(opt->value, new_value); +} + static int conf_option_new_value(struct conf_option *opt, struct conf_value *new_value, enum conf_update_mode mode) @@ -416,36 +460,56 @@ static int conf_option_new_value(struct conf_option *opt, int ret; bool ok; - ok = conf_option_validate(opt, new_value, mode); - if (!ok) { - D_ERR("conf: validation for option \"%s\" failed\n", - opt->name); - return EINVAL; + if (opt->new_value != &opt->default_value) { + TALLOC_FREE(opt->new_value); } - TALLOC_FREE(opt->new_value); - opt->new_value = talloc_zero(opt, struct conf_value); - if (opt->new_value == NULL) { - return ENOMEM; - } + if (new_value == &opt->default_value) { + /* + * This happens only during load/reload. Set the value to + * default value, so if the config option is dropped from + * config file, then it get's reset to default. + */ + opt->new_value = &opt->default_value; + } else { + ok = conf_option_validate(opt, new_value, mode); + if (!ok) { + D_ERR("conf: validation for option \"%s\" failed\n", + opt->name); + return EINVAL; + } - opt->new_value->type = opt->value->type; - ret = conf_value_copy(opt, new_value, opt->new_value); - if (ret != 0) { - return ret; + opt->new_value = talloc_zero(opt, struct conf_value); + if (opt->new_value == NULL) { + return ENOMEM; + } + + opt->new_value->type = opt->value->type; + ret = conf_value_copy(opt, new_value, opt->new_value); + if (ret != 0) { + return ret; + } } conf_option_set_ptr_value(opt); - if (mode == CONF_MODE_API) { - opt->temporary_modified = true; - } else { - opt->temporary_modified = false; + if (new_value != &opt->default_value) { + if (mode == CONF_MODE_API) { + opt->temporary_modified = true; + } else { + opt->temporary_modified = false; + } } return 0; } +static int conf_option_new_default_value(struct conf_option *opt, + enum conf_update_mode mode) +{ + return conf_option_new_value(opt, &opt->default_value, mode); +} + static void conf_option_default(struct conf_option *opt) { if (! opt->default_set) { @@ -462,7 +526,9 @@ static void conf_option_default(struct conf_option *opt) static void conf_option_reset(struct conf_option *opt) { - TALLOC_FREE(opt->new_value); + if (opt->new_value != &opt->default_value) { + TALLOC_FREE(opt->new_value); + } conf_option_set_ptr_value(opt); } @@ -483,6 +549,11 @@ static void conf_option_update(struct conf_option *opt) conf_option_set_ptr_value(opt); } +static void conf_option_reset_temporary(struct conf_option *opt) +{ + opt->temporary_modified = false; +} + static bool conf_option_is_default(struct conf_option *opt) { return (opt->value == &opt->default_value); @@ -586,6 +657,25 @@ static void conf_all_default(struct conf_context *conf) } } +static int conf_all_temporary_default(struct conf_context *conf, + enum conf_update_mode mode) +{ + struct conf_section *s; + struct conf_option *opt; + int ret; + + for (s = conf->section; s != NULL; s = s->next) { + for (opt = s->option; opt != NULL; opt = opt->next) { + ret = conf_option_new_default_value(opt, mode); + if (ret != 0) { + return ret; + } + } + } + + return 0; +} + static void conf_all_reset(struct conf_context *conf) { struct conf_section *s; @@ -606,6 +696,7 @@ static void conf_all_update(struct conf_context *conf) for (s = conf->section; s != NULL; s = s->next) { for (opt = s->option; opt != NULL; opt = opt->next) { conf_option_update(opt); + conf_option_reset_temporary(opt); } } } @@ -920,6 +1011,7 @@ static int conf_load_internal(struct conf_context *conf) { struct conf_load_state state; FILE *fp; + int ret; bool ok; fp = fopen(conf->filename, "r"); @@ -932,6 +1024,11 @@ static int conf_load_internal(struct conf_context *conf) .mode = (conf->reload ? CONF_MODE_RELOAD : CONF_MODE_LOAD), }; + ret = conf_all_temporary_default(conf, state.mode); + if (ret != 0) { + return ret; + } + ok = tini_parse(fp, false, conf_load_section, @@ -998,6 +1095,7 @@ static bool conf_load_option(const char *name, TALLOC_CTX *tmp_ctx; struct conf_value value; int ret; + bool ok; if (state->s == NULL) { if (state->conf->ignore_unknown) { @@ -1035,6 +1133,11 @@ static bool conf_load_option(const char *name, return false; } + ok = conf_option_same_value(opt, &value); + if (ok) { + goto done; + } + ret = conf_option_new_value(opt, &value, state->mode); if (ret != 0) { talloc_free(tmp_ctx); @@ -1042,6 +1145,7 @@ static bool conf_load_option(const char *name, return false; } +done: talloc_free(tmp_ctx); return true; @@ -1104,6 +1208,11 @@ static int conf_set(struct conf_context *conf, return ENOENT; } + ok = conf_option_same_value(opt, value); + if (ok) { + return 0; + } + ret = conf_option_new_value(opt, value, CONF_MODE_API); if (ret != 0) { conf_option_reset(opt); diff --git a/ctdb/tests/cunit/conf_test_001.sh b/ctdb/tests/cunit/conf_test_001.sh index e83e04c78b7..08a51b00d9b 100755 --- a/ctdb/tests/cunit/conf_test_001.sh +++ b/ctdb/tests/cunit/conf_test_001.sh @@ -32,7 +32,12 @@ unit_test conf_test 4 ok_null unit_test conf_test 5 -ok_null +ok < "$conffile" < "$conffile" < "${conffile}.reload" <