From: Alexey Sheplyakov Date: Fri, 23 Sep 2016 09:33:39 +0000 (+0300) Subject: config: thread safe options X-Git-Tag: v11.1.0~573^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f572d0b8c6696cc5d91cc05bf7cbebbee4f774ec;p=ceph.git config: thread safe options For now there are two ways options: 1) direct reference (cct->_conf->foo), 2) registering the observer. The former way is obviously not thread safe, and the latter is a bit cumbersome. Hence the new get_val() method which takes a lock and returns a *copy* of the value. Thus it's safe to modify the option in question at the run time (when threads are already running). Such options should be declared with SAFE_OPTION macro and can be accessed only via get_val()/set_val() methods and observers, attempt to reference them directly (cct->_conf->safe_opt) won't compile. Signed-off-by: Alexey Sheplyakov --- diff --git a/src/common/config.cc b/src/common/config.cc index ac2a407bbc28..9f03206fba4e 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -36,6 +36,10 @@ #include #include +#include +#include +#include + /* Don't use standard Ceph logging in this file. * We can't use logging until it's initialized, and a lot of the necessary * initialization happens here. @@ -61,32 +65,6 @@ const char *CEPH_CONF_FILE_DEFAULT = "$data_dir/config, /etc/ceph/$cluster.conf, #define _STR(x) #x #define STRINGIFY(x) _STR(x) - -void *config_option::conf_ptr(md_config_t *conf) const -{ - void *v = (void*)(((char*)conf) + md_conf_off); - return v; -} - -const void *config_option::conf_ptr(const md_config_t *conf) const -{ - const void *v = (const void*)(((const char*)conf) + md_conf_off); - return v; -} - -struct config_option config_optionsp[] = { -#define OPTION(name, type, def_val) \ - { STRINGIFY(name), type, offsetof(struct md_config_t, name) }, -#define SUBSYS(name, log, gather) -#define DEFAULT_SUBSYS(log, gather) -#include "common/config_opts.h" -#undef OPTION -#undef SUBSYS -#undef DEFAULT_SUBSYS -}; - -const int NUM_CONFIG_OPTIONS = sizeof(config_optionsp) / sizeof(config_option); - int ceph_resolve_file_search(const std::string& filename_list, std::string& result) { @@ -123,6 +101,7 @@ md_config_t::md_config_t() #define OPTION_OPT_U64(name, def_val) name(((uint64_t)1) * def_val), #define OPTION_OPT_UUID(name, def_val) name(def_val), #define OPTION(name, type, def_val) OPTION_##type(name, def_val) +#define SAFE_OPTION(name, type, def_val) OPTION(name, type, def_val) #define SUBSYS(name, log, gather) #define DEFAULT_SUBSYS(log, gather) #include "common/config_opts.h" @@ -137,10 +116,28 @@ md_config_t::md_config_t() #undef OPTION_OPT_U64 #undef OPTION_OPT_UUID #undef OPTION +#undef SAFE_OPTION #undef SUBSYS #undef DEFAULT_SUBSYS lock("md_config_t", true, false) { + static const std::vector s_config_options = { +#define OPTION4(name, type, def_val, safe) \ + config_option{ STRINGIFY(name), type, &md_config_t::name, safe }, +#define OPTION(name, type, def_val) OPTION4(name, type, def_val, false) +#define SAFE_OPTION(name, type, def_val) OPTION4(name, type, def_val, true) +#define SUBSYS(name, log, gather) +#define DEFAULT_SUBSYS(log, gather) +#include "common/config_opts.h" +#undef OPTION +#undef SAFE_OPTION +#undef SUBSYS +#undef DEFAULT_SUBSYS + }; + static std::shared_ptr + s_tbl(new std::vector(std::move(s_config_options))); + config_options = s_tbl; + init_subsys(); } @@ -151,8 +148,10 @@ void md_config_t::init_subsys() #define DEFAULT_SUBSYS(log, gather) \ subsys.add(ceph_subsys_, "none", log, gather); #define OPTION(a, b, c) +#define SAFE_OPTION(a, b, c) #include "common/config_opts.h" #undef OPTION +#undef SAFE_OPTION #undef SUBSYS #undef DEFAULT_SUBSYS } @@ -225,7 +224,7 @@ int md_config_t::parse_config_files(const char *conf_files, string &s = *p; if (s.find("$data_dir") != string::npos) { if (data_dir_option.length()) { - list stack; + list stack; expand_meta(s, NULL, stack, warnings); p++; } else { @@ -279,12 +278,11 @@ int md_config_t::parse_config_files_impl(const std::list &conf_file std::vector my_sections; _get_my_sections(my_sections); - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = &config_optionsp[i]; + for (auto& opt: *config_options) { std::string val; - int ret = _get_val_from_conf_file(my_sections, opt->name, val, false); + int ret = _get_val_from_conf_file(my_sections, opt.name, val, false); if (ret == 0) { - set_val_impl(val.c_str(), opt); + set_val_impl(val.c_str(), &opt); } } @@ -377,14 +375,13 @@ void md_config_t::_show_config(std::ostream *out, Formatter *f) f->dump_string(debug_name.c_str(), ss.str()); } } - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = config_optionsp + i; + for (auto& opt: *config_options) { char *buf; - _get_val(opt->name, &buf, -1); + _get_val(opt.name, &buf, -1); if (out) - *out << opt->name << " = " << buf << std::endl; + *out << opt.name << " = " << buf << std::endl; if (f) - f->dump_string(opt->name, buf); + f->dump_string(opt.name, buf); free(buf); } } @@ -518,9 +515,10 @@ int md_config_t::parse_option(std::vector& args, return ret; } - for (o = 0; o < NUM_CONFIG_OPTIONS; ++o) { + o = 0; + for (auto& opt_ref: *config_options) { ostringstream err; - const config_option *opt = config_optionsp + o; + config_option const *opt = &opt_ref; std::string as_option("--"); as_option += opt->name; if (opt->type == OPT_BOOL) { @@ -550,9 +548,7 @@ int md_config_t::parse_option(std::vector& args, ret = -EINVAL; break; } - if (oss && ( - ((opt->type == OPT_STR) || (opt->type == OPT_ADDR) || - (opt->type == OPT_UUID)) && + if (oss && ((!opt->is_safe()) && (observers.find(opt->name) == observers.end()))) { *oss << "You cannot change " << opt->name << " using injectargs.\n"; ret = -ENOSYS; @@ -572,8 +568,9 @@ int md_config_t::parse_option(std::vector& args, } break; } + ++o; } - if (o == NUM_CONFIG_OPTIONS) { + if (o == (int)config_options->size()) { // ignore ++i; } @@ -712,6 +709,49 @@ void md_config_t::set_val_or_die(const char *key, const char *val) assert(ret == 0); } +struct is_integer_member : public boost::static_visitor { + template, int>::type = 0> + bool operator()(const T md_config_t::* /* member_ptr */) const { + return true; + } + template::value, int>::type = 0> + bool operator()(const T md_config_t::* /* member_ptr */) const { + return false; + } +}; + +struct is_float_member : public boost::static_visitor { + template, int>::type = 0> + bool operator()(const T md_config_t::* /* member_ptr */) const { + return true; + } + template::value, int>::type = 0> + bool operator()(const T md_config_t::* /* member_ptr */) const { + return false; + } +}; + +bool md_config_t::config_option::is_safe() const { + // for now integer and floating point options considered thread safe + return safe || + boost::apply_visitor(is_integer_member(), md_member_ptr) || + boost::apply_visitor(is_float_member(), md_member_ptr); +} + +md_config_t::config_option const *md_config_t::find_config_option(const std::string &normalized_key) const +{ + auto opt_it = std::find_if(config_options->begin(), + config_options->end(), + [normalized_key](const config_option &opt) -> bool { + return strcmp(normalized_key.c_str(), opt.name) == 0; + }); + return config_options->end() == opt_it ? nullptr : &(*opt_it); +} + int md_config_t::set_val(const char *key, const char *val, bool meta, bool safe) { Mutex::Locker l(lock); @@ -746,23 +786,17 @@ int md_config_t::set_val(const char *key, const char *val, bool meta, bool safe) } } - for (int i = 0; i < NUM_CONFIG_OPTIONS; ++i) { - config_option *opt = &config_optionsp[i]; - if (strcmp(opt->name, k.c_str()) == 0) { - if (safe && internal_safe_to_start_threads) { - // If threads have been started... - if ((opt->type == OPT_STR) || (opt->type == OPT_ADDR) || - (opt->type == OPT_UUID)) { - // And this is NOT an integer valued variable.... - if (observers.find(opt->name) == observers.end()) { - // And there is no observer to safely change it... - // You lose. - return -ENOSYS; - } - } + config_option const *opt = find_config_option(k); + if (opt) { + if ((!opt->is_safe()) && safe && internal_safe_to_start_threads) { + // If threads have been started and the option is not thread safe + if (observers.find(opt->name) == observers.end()) { + // And there is no observer to safely change it... + // You lose. + return -ENOSYS; } - return set_val_impl(v.c_str(), opt); } + return set_val_impl(v.c_str(), opt); } // couldn't find a configuration option with key 'key' @@ -776,55 +810,55 @@ int md_config_t::get_val(const char *key, char **buf, int len) const return _get_val(key, buf,len); } -int md_config_t::_get_val(const char *key, char **buf, int len) const +md_config_t::config_value_t md_config_t::get_val_generic(const char *key) const +{ + Mutex::Locker l(lock); + return _get_val(key); +} + +class get_value_generic_visitor : public boost::static_visitor { + md_config_t const *conf; +public: + explicit get_value_generic_visitor(md_config_t const *conf_) : conf(conf_) { } + template md_config_t::config_value_t operator()(const T md_config_t::* member_ptr) { + return md_config_t::config_value_t(conf->*member_ptr); + } +}; + +md_config_t::config_value_t md_config_t::_get_val(const char *key) const { assert(lock.is_locked()); if (!key) - return -EINVAL; + return config_value_t(invalid_config_value_t()); // In key names, leading and trailing whitespace are not significant. string k(ConfFile::normalize_key_name(key)); - for (int i = 0; i < NUM_CONFIG_OPTIONS; ++i) { - const config_option *opt = &config_optionsp[i]; - if (strcmp(opt->name, k.c_str())) - continue; + config_option const *opt = find_config_option(k); + if (!opt) { + return config_value_t(invalid_config_value_t()); + } + get_value_generic_visitor gvv(this); + return boost::apply_visitor(gvv, opt->md_member_ptr); +} + +int md_config_t::_get_val(const char *key, char **buf, int len) const +{ + assert(lock.is_locked()); + + if (!key) + return -EINVAL; + + string k(ConfFile::normalize_key_name(key)); + config_value_t cval = _get_val(k.c_str()); + if (!boost::get(&cval)) { ostringstream oss; - switch (opt->type) { - case OPT_INT: - oss << *(int*)opt->conf_ptr(this); - break; - case OPT_LONGLONG: - oss << *(long long*)opt->conf_ptr(this); - break; - case OPT_STR: - oss << *((std::string*)opt->conf_ptr(this)); - break; - case OPT_FLOAT: - oss << *(float*)opt->conf_ptr(this); - break; - case OPT_DOUBLE: - oss << *(double*)opt->conf_ptr(this); - break; - case OPT_BOOL: { - bool b = *(bool*)opt->conf_ptr(this); - oss << (b ? "true" : "false"); - } - break; - case OPT_U32: - oss << *(uint32_t*)opt->conf_ptr(this); - break; - case OPT_U64: - oss << *(uint64_t*)opt->conf_ptr(this); - break; - case OPT_ADDR: - oss << *(entity_addr_t*)opt->conf_ptr(this); - break; - case OPT_UUID: - oss << *(uuid_d*)opt->conf_ptr(this); - break; + if (bool *flagp = boost::get(&cval)) { + oss << (*flagp ? "true" : "false"); + } else { + oss << cval; } string str(oss.str()); int l = strlen(str.c_str()) + 1; @@ -860,11 +894,11 @@ void md_config_t::get_all_keys(std::vector *keys) const { const std::string negative_flag_prefix("no_"); keys->clear(); - keys->reserve(NUM_CONFIG_OPTIONS); - for (size_t i = 0; i < NUM_CONFIG_OPTIONS; ++i) { - keys->push_back(config_optionsp[i].name); - if (config_optionsp[i].type == OPT_BOOL) { - keys->push_back(negative_flag_prefix + config_optionsp[i].name); + keys->reserve(config_options->size()); + for (auto& opt: *config_options) { + keys->push_back(opt.name); + if (opt.type == OPT_BOOL) { + keys->push_back(negative_flag_prefix + opt.name); } } for (int i = 0; i < subsys.get_num(); ++i) { @@ -930,7 +964,7 @@ int md_config_t::_get_val_from_conf_file(const std::vector §io return -ENOENT; } -int md_config_t::set_val_impl(const char *val, const config_option *opt) +int md_config_t::set_val_impl(const char *val, config_option const *opt) { assert(lock.is_locked()); int ret = set_val_raw(val, opt); @@ -940,89 +974,96 @@ int md_config_t::set_val_impl(const char *val, const config_option *opt) return 0; } -int md_config_t::set_val_raw(const char *val, const config_option *opt) -{ - assert(lock.is_locked()); - switch (opt->type) { - case OPT_INT: { - std::string err; - int f = strict_si_cast(val, &err); - if (!err.empty()) - return -EINVAL; - *(int*)opt->conf_ptr(this) = f; - return 0; - } - case OPT_LONGLONG: { - std::string err; - long long f = strict_si_cast(val, &err); - if (!err.empty()) - return -EINVAL; - *(long long*)opt->conf_ptr(this) = f; - return 0; - } - case OPT_STR: - *(std::string*)opt->conf_ptr(this) = val ? val : ""; - return 0; - case OPT_FLOAT: { - std::string err; - float f = strict_strtof(val, &err); - if (!err.empty()) - return -EINVAL; - *(float*)opt->conf_ptr(this) = f; - return 0; - } - case OPT_DOUBLE: { +template struct strtox_helper; + +template<> struct strtox_helper { + static inline void apply(const char *val, float &x, std::string &err) { + x = strict_strtof(val, &err); + } +}; + +template<> struct strtox_helper { + static inline void apply(const char *val, double &x, std::string &err) { + x = strict_strtod(val, &err); + } +}; + +template static inline int strict_strtox(const char *val, T &x) { + std::string err; + strtox_helper::apply(val, x, err); + return err.empty() ? 0 : -EINVAL; +} + +class set_value_visitor : public boost::static_visitor { + md_config_t const *conf; + const char *val; +public: + explicit set_value_visitor(md_config_t const *conf_, const char *val_) : + conf(conf_), val(val_) { } + + int operator()(const std::string md_config_t::* member_ptr) { + auto *ptr = const_cast(&(conf->*member_ptr)); + *ptr = val ? val : ""; + return 0; + } + + int operator()(const bool md_config_t::* member_ptr) { + bool *ptr = const_cast(&(conf->*member_ptr)); + if (strcasecmp(val, "false") == 0) { + *ptr = false; + } else if (strcasecmp(val, "true") == 0) { + *ptr = true; + } else { std::string err; - double f = strict_strtod(val, &err); - if (!err.empty()) + int b = strict_strtol(val, 10, &err); + if (!err.empty()) { return -EINVAL; - *(double*)opt->conf_ptr(this) = f; - return 0; + } + *ptr = !!b; } - case OPT_BOOL: - if (strcasecmp(val, "false") == 0) - *(bool*)opt->conf_ptr(this) = false; - else if (strcasecmp(val, "true") == 0) - *(bool*)opt->conf_ptr(this) = true; - else { - std::string err; - int b = strict_strtol(val, 10, &err); - if (!err.empty()) + return 0; + } + + // type has parse() member function + template, int>::type = 0> + int operator()(const T md_config_t::* member_ptr) { + T *obj = const_cast(&(conf->*member_ptr)); + if (!obj->parse(val)) { return -EINVAL; - *(bool*)opt->conf_ptr(this) = !!b; + } + return 0; } - return 0; - case OPT_U32: { - std::string err; - int f = strict_si_cast(val, &err); - if (!err.empty()) - return -EINVAL; - *(uint32_t*)opt->conf_ptr(this) = f; - return 0; - } - case OPT_U64: { - std::string err; - uint64_t f = strict_si_cast(val, &err); - if (!err.empty()) - return -EINVAL; - *(uint64_t*)opt->conf_ptr(this) = f; - return 0; - } - case OPT_ADDR: { - entity_addr_t *addr = (entity_addr_t*)opt->conf_ptr(this); - if (!addr->parse(val)) { - return -EINVAL; + + // float, double + template, int>::type = 0> + int operator()(const T md_config_t::* member_ptr) { + T* ptr = const_cast(&(conf->*member_ptr)); + return strict_strtox(val, *ptr); } - return 0; - } - case OPT_UUID: { - uuid_d *u = (uuid_d*)opt->conf_ptr(this); - if (!u->parse(val)) - return -EINVAL; - return 0; - } + + // integers + template::value && + !boost::is_same::value, int>::type = 0> + int operator()(const T md_config_t::* member_ptr) { + std::string err; + T f = strict_si_cast(val, &err); + if (!err.empty()) { + return -EINVAL; + } + T *ptr = const_cast(&(conf->*member_ptr)); + *ptr = f; + return 0; } - return -ENOSYS; +}; + +int md_config_t::set_val_raw(const char *val, config_option const *opt) +{ + assert(lock.is_locked()); + set_value_visitor svv(this, val); + return boost::apply_visitor(svv, opt->md_member_ptr); } static const char *CONF_METAVARIABLES[] = { @@ -1036,12 +1077,12 @@ void md_config_t::expand_all_meta() { // Expand all metavariables ostringstream oss; - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = config_optionsp + i; - if (opt->type == OPT_STR) { - std::string *str = (std::string *)opt->conf_ptr(this); - list stack; - expand_meta(*str, opt, stack, &oss); + for (auto& opt: *config_options) { + std::string *str; + opt.conf_ptr(str, this); + if (str) { + list stack; + expand_meta(*str, &opt, stack, &oss); } } cerr << oss.str(); @@ -1050,13 +1091,13 @@ void md_config_t::expand_all_meta() bool md_config_t::expand_meta(std::string &origval, std::ostream *oss) const { - list stack; + list stack; return expand_meta(origval, NULL, stack, oss); } bool md_config_t::expand_meta(std::string &origval, - config_option *opt, - std::list stack, + config_option const *opt, + std::list stack, std::ostream *oss) const { assert(lock.is_locked()); @@ -1068,17 +1109,17 @@ bool md_config_t::expand_meta(std::string &origval, // ignore an expansion loop and create a human readable // message about it if (opt) { - for (list::iterator i = stack.begin(); + for (list::iterator i = stack.begin(); i != stack.end(); ++i) { if (strcmp(opt->name, (*i)->name) == 0) { *oss << "variable expansion loop at " << opt->name << "=" << origval << std::endl; *oss << "expansion stack: " << std::endl; - for (list::iterator j = stack.begin(); + for (list::iterator j = stack.begin(); j != stack.end(); ++j) { - *oss << (*j)->name << "=" << *(string *)(*j)->conf_ptr(this) << std::endl; + *oss << (*j)->name << "=" << *((*j)->conf_ptr(this)) << std::endl; } return false; } @@ -1162,16 +1203,16 @@ bool md_config_t::expand_meta(std::string &origval, if (!expanded) { // config option? - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = &config_optionsp[i]; - if (var == opt->name) { - if (opt->type == OPT_STR) { - string *origval = (string *)opt->conf_ptr(this); - expand_meta(*origval, opt, stack, oss); + for (auto& opt: *config_options) { + if (var == opt.name) { + string *origval; + opt.conf_ptr(origval, const_cast(this)); + if (origval) { + expand_meta(*origval, &opt, stack, oss); out += *origval; } else { char *vv = NULL; - _get_val(opt->name, &vv, -1); + _get_val(opt.name, &vv, -1); out += vv; free(vv); } @@ -1203,27 +1244,26 @@ void md_config_t::diff( char local_buf[4096]; char other_buf[4096]; - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = &config_optionsp[i]; + for (auto& opt: *config_options) { memset(local_buf, 0, sizeof(local_buf)); memset(other_buf, 0, sizeof(other_buf)); char *other_val = other_buf; - int err = other->get_val(opt->name, &other_val, sizeof(other_buf)); + int err = other->get_val(opt.name, &other_val, sizeof(other_buf)); if (err < 0) { if (err == -ENOENT) { - unknown->insert(opt->name); + unknown->insert(opt.name); } continue; } char *local_val = local_buf; - err = _get_val(opt->name, &local_val, sizeof(local_buf)); + err = _get_val(opt.name, &local_val, sizeof(local_buf)); if (err != 0) continue; if (strcmp(local_val, other_val)) - diff->insert(make_pair(opt->name, make_pair(local_val, other_val))); + diff->insert(make_pair(opt.name, make_pair(local_val, other_val))); } } diff --git a/src/common/config.h b/src/common/config.h index d844395cba65..eb22a6f57aab 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -19,6 +19,7 @@ #include #include #include +#include #include "common/ConfUtils.h" #include "common/entity_name.h" @@ -38,7 +39,6 @@ enum { #define OSD_POOL_ERASURE_CODE_STRIPE_WIDTH 4096 -struct config_option; class CephContext; extern const char *CEPH_CONF_FILE_DEFAULT; @@ -58,16 +58,20 @@ extern const char *CEPH_CONF_FILE_DEFAULT; * * ACCESS * - * There are two ways to read the ceph context-- the old way and the new way. + * There are 3 ways to read the ceph context-- the old way and two new ways. * In the old way, code would simply read the public variables of the - * configuration, without taking a lock. In the new way, code registers a + * configuration, without taking a lock. In the new way #1, code registers a * configuration obserever which receives callbacks when a value changes. These - * callbacks take place under the md_config_t lock. + * callbacks take place under the md_config_t lock. Alternatively one can use + * get_val(const char *name) method to safely get a copy of the value. * * To prevent serious problems resulting from thread-safety issues, we disallow * changing std::string configuration values after * md_config_t::internal_safe_to_start_threads becomes true. You can still - * change integer or floating point values, however. + * change integer or floating point values, and the option declared with + * SAFE_OPTION macro. Notice the latter options can not be read directly + * (conf->foo), one should use either observers or get_val() method + * (conf->get_val("foo")). * * FIXME: really we shouldn't allow changing integer or floating point values * while another thread is reading them, either. @@ -81,6 +85,78 @@ public: * apply_changes */ typedef std::set < std::string > changed_set_t; + struct invalid_config_value_t { }; + typedef boost::variant config_value_t; + typedef boost::variant member_ptr_t; + + typedef enum { + OPT_INT, OPT_LONGLONG, OPT_STR, OPT_DOUBLE, OPT_FLOAT, OPT_BOOL, + OPT_ADDR, OPT_U32, OPT_U64, OPT_UUID + } opt_type_t; + + class config_option { + public: + const char *name; + opt_type_t type; + md_config_t::member_ptr_t md_member_ptr; + bool safe; // promise to access it only via md_config_t::get_val + private: + template struct get_typed_pointer_visitor : public boost::static_visitor { + md_config_t const *conf; + explicit get_typed_pointer_visitor(md_config_t const *conf_) : conf(conf_) { } + template, int>::type = 0> + T const *operator()(const U md_config_t::* member_ptr) { + return &(conf->*member_ptr); + } + template::value, int>::type = 0> + T const *operator()(const U md_config_t::* member_ptr) { + return nullptr; + } + }; + public: + // is it OK to alter the value when threads are running? + bool is_safe() const; + // Given a configuration, return a pointer to this option inside + // that configuration. + template void conf_ptr(T const *&ptr, md_config_t const *conf) const { + get_typed_pointer_visitor gtpv(conf); + ptr = boost::apply_visitor(gtpv, md_member_ptr); + } + template void conf_ptr(T *&ptr, md_config_t *conf) const { + get_typed_pointer_visitor gtpv(conf); + ptr = const_cast(boost::apply_visitor(gtpv, md_member_ptr)); + } + template T const *conf_ptr(md_config_t const *conf) const { + get_typed_pointer_visitor gtpv(conf); + return boost::apply_visitor(gtpv, md_member_ptr); + } + template T *conf_ptr(md_config_t *conf) const { + get_typed_pointer_visitor gtpv(conf); + return const_cast(boost::apply_visitor(gtpv, md_member_ptr)); + } + }; + // Create a new md_config_t structure. md_config_t(); ~md_config_t(); @@ -136,6 +212,8 @@ public: // No metavariables will be returned (they will have already been expanded) int get_val(const char *key, char **buf, int len) const; int _get_val(const char *key, char **buf, int len) const; + config_value_t get_val_generic(const char *key) const; + template T get_val(const char *key) const; void get_all_keys(std::vector *keys) const; @@ -163,6 +241,7 @@ public: void complain_about_parse_errors(CephContext *cct); private: + config_value_t _get_val(const char *key) const; void _show_config(std::ostream *out, Formatter *f); void _get_my_sections(std::vector §ions) const; @@ -178,8 +257,8 @@ private: int parse_config_files_impl(const std::list &conf_files, std::ostream *warnings); - int set_val_impl(const char *val, const config_option *opt); - int set_val_raw(const char *val, const config_option *opt); + int set_val_impl(const char *val, config_option const *opt); + int set_val_raw(const char *val, config_option const *opt); void init_subsys(); @@ -193,8 +272,8 @@ public: // for global_init } private: bool expand_meta(std::string &val, - config_option *opt, - std::list stack, + config_option const *opt, + std::list stack, std::ostream *oss) const; /// expand all metavariables in config structure. @@ -228,7 +307,13 @@ public: #define OPTION_OPT_U32(name) const uint32_t name; #define OPTION_OPT_U64(name) const uint64_t name; #define OPTION_OPT_UUID(name) const uuid_d name; -#define OPTION(name, ty, init) OPTION_##ty(name) +#define OPTION(name, ty, init) \ + public: \ + OPTION_##ty(name) +#define SAFE_OPTION(name, ty, init) \ + protected: \ + OPTION_##ty(name) \ + public: #define SUBSYS(name, log, gather) #define DEFAULT_SUBSYS(log, gather) #include "common/config_opts.h" @@ -243,6 +328,7 @@ public: #undef OPTION_OPT_U64 #undef OPTION_OPT_UUID #undef OPTION +#undef SAFE_OPTION #undef SUBSYS #undef DEFAULT_SUBSYS @@ -259,37 +345,54 @@ public: mutable Mutex lock; friend class test_md_config_t; +protected: + // Tests and possibly users expect options to appear in the output + // of ceph-conf in the same order as declared in config_opts.h + std::shared_ptr> config_options; + config_option const *find_config_option(const std::string& normalized_key) const; }; -typedef enum { - OPT_INT, OPT_LONGLONG, OPT_STR, OPT_DOUBLE, OPT_FLOAT, OPT_BOOL, - OPT_ADDR, OPT_U32, OPT_U64, OPT_UUID -} opt_type_t; +template +struct get_typed_value_visitor : public boost::static_visitor { + template, int>::type = 0> + T operator()(U & val) { + return std::move(val); + } + template::value, int>::type = 0> + T operator()(U &val) { + assert("wrong type or option does not exist" == nullptr); + } +}; + +template T md_config_t::get_val(const char *key) const { + config_value_t generic_val = this->get_val_generic(key); + get_typed_value_visitor gtv; + return boost::apply_visitor(gtv, generic_val); +} + +inline std::ostream& operator<<(std::ostream& o, const md_config_t::invalid_config_value_t& ) { + return o << "INVALID_CONFIG_VALUE"; +} int ceph_resolve_file_search(const std::string& filename_list, std::string& result); -struct config_option { - const char *name; - opt_type_t type; - size_t md_conf_off; +typedef md_config_t::config_option config_option; - // Given a configuration, return a pointer to this option inside - // that configuration. - void *conf_ptr(md_config_t *conf) const; - - const void *conf_ptr(const md_config_t *conf) const; -}; enum config_subsys_id { ceph_subsys_, // default #define OPTION(a,b,c) +#define SAFE_OPTION(a,b,c) #define SUBSYS(name, log, gather) \ ceph_subsys_##name, #define DEFAULT_SUBSYS(log, gather) #include "common/config_opts.h" #undef SUBSYS #undef OPTION +#undef SAFE_OPTION #undef DEFAULT_SUBSYS ceph_subsys_max }; diff --git a/src/test/common/test_config.cc b/src/test/common/test_config.cc index 328626744ea4..42128f02fd5b 100644 --- a/src/test/common/test_config.cc +++ b/src/test/common/test_config.cc @@ -23,22 +23,6 @@ #include "common/errno.h" #include "gtest/gtest.h" -#define _STR(x) #x -#define STRINGIFY(x) _STR(x) - -static struct config_option config_optionsp[] = { -#define OPTION(name, type, def_val) \ - { STRINGIFY(name), type, offsetof(struct md_config_t, name) }, -#define SUBSYS(name, log, gather) -#define DEFAULT_SUBSYS(log, gather) -#include "common/config_opts.h" -#undef OPTION -#undef SUBSYS -#undef DEFAULT_SUBSYS -}; - -static const int NUM_CONFIG_OPTIONS = sizeof(config_optionsp) / sizeof(config_option); - class test_md_config_t : public md_config_t, public ::testing::Test { public: void test_expand_meta() { @@ -113,10 +97,10 @@ public: void test_expand_all_meta() { Mutex::Locker l(lock); int before_count = 0, data_dir = 0; - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = config_optionsp + i; - if (opt->type == OPT_STR) { - std::string *str = (std::string *)opt->conf_ptr(this); + for (auto& opt: *config_options) { + std::string *str; + opt.conf_ptr(str, this); + if (str) { if (str->find("$") != string::npos) before_count++; if (str->find("$data_dir") != string::npos) @@ -129,11 +113,10 @@ public: ASSERT_LT(0, before_count); expand_all_meta(); int after_count = 0; - for (int i = 0; i < NUM_CONFIG_OPTIONS; i++) { - config_option *opt = config_optionsp + i; - if (opt->type == OPT_STR) { - std::string *str = (std::string *)opt->conf_ptr(this); - + for (auto& opt: *config_options) { + std::string *str; + opt.conf_ptr(str, this); + if (str) { size_t pos = 0; while ((pos = str->find("$", pos)) != string::npos) { if (str->substr(pos, 8) != "$channel") {