]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
config: thread safe options
authorAlexey Sheplyakov <asheplyakov@mirantis.com>
Fri, 23 Sep 2016 09:33:39 +0000 (12:33 +0300)
committerAlexey Sheplyakov <asheplyakov@mirantis.com>
Mon, 17 Oct 2016 06:06:28 +0000 (09:06 +0300)
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 <asheplyakov@mirantis.com>
src/common/config.cc
src/common/config.h
src/test/common/test_config.cc

index ac2a407bbc28caf306a77abb20de8d0be421b2a6..9f03206fba4ee26d841c60d26235513050848498 100644 (file)
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <utility>
+#include <boost/type_traits.hpp>
+#include <boost/utility/enable_if.hpp>
+
 /* 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<md_config_t::config_option> 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<decltype(s_config_options)>
+    s_tbl(new std::vector<md_config_t::config_option>(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<config_option*> stack;
+       list<config_option const *> stack;
        expand_meta(s, NULL, stack, warnings);
        p++;
       } else {
@@ -279,12 +278,11 @@ int md_config_t::parse_config_files_impl(const std::list<std::string> &conf_file
 
   std::vector <std::string> 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<const char*>& 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<const char*>& 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<const char*>& 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<bool> {
+  template<typename T,
+           typename boost::enable_if<boost::is_integral<T>, int>::type = 0>
+  bool operator()(const T md_config_t::* /* member_ptr */) const {
+    return true;
+  }
+  template<typename T,
+           typename boost::enable_if_c<!boost::is_integral<T>::value, int>::type = 0>
+  bool operator()(const T md_config_t::* /* member_ptr */) const {
+    return false;
+  }
+};
+
+struct is_float_member : public boost::static_visitor<bool> {
+  template<typename T,
+           typename boost::enable_if<boost::is_float<T>, int>::type = 0>
+  bool operator()(const T md_config_t::* /* member_ptr */) const {
+    return true;
+  }
+  template<typename T,
+           typename boost::enable_if_c<!boost::is_float<T>::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::config_value_t> {
+  md_config_t const *conf;
+public:
+  explicit get_value_generic_visitor(md_config_t const *conf_) : conf(conf_) { }
+  template<typename T> 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<invalid_config_value_t>(&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<bool>(&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<std::string> *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 <std::string> &sectio
   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<int>(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<long long>(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<typename T> struct strtox_helper;
+
+template<> struct strtox_helper<float> {
+  static inline void apply(const char *val, float &x, std::string &err) {
+    x = strict_strtof(val, &err);
+  }
+};
+
+template<> struct strtox_helper<double> {
+  static inline void apply(const char *val, double &x, std::string &err) {
+    x = strict_strtod(val, &err);
+  }
+};
+
+template<typename T> static inline int strict_strtox(const char *val, T &x) {
+  std::string err;
+  strtox_helper<T>::apply(val, x, err);
+  return err.empty() ? 0 : -EINVAL;
+}
+
+class set_value_visitor : public boost::static_visitor<int> {
+  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<std::string *>(&(conf->*member_ptr));
+    *ptr = val ? val : "";
+    return 0;
+  }
+
+  int operator()(const bool md_config_t::* member_ptr) {
+    bool *ptr = const_cast<bool *>(&(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<typename T,
+    typename boost::enable_if<boost::is_member_function_pointer<decltype(&T::parse)>, int>::type = 0>
+      int operator()(const T md_config_t::* member_ptr) {
+       T *obj = const_cast<T *>(&(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<uint32_t>(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<uint64_t>(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<typename T,
+    typename boost::enable_if<boost::is_floating_point<T>, int>::type = 0>
+      int operator()(const T md_config_t::* member_ptr) {
+       T* ptr = const_cast<T *>(&(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<typename T,
+    typename boost::enable_if_c<boost::is_integral<T>::value &&
+      !boost::is_same<T, bool>::value, int>::type = 0>
+      int operator()(const T md_config_t::* member_ptr) {
+       std::string err;
+       T f = strict_si_cast<T>(val, &err);
+       if (!err.empty()) {
+         return -EINVAL;
+       }
+       T *ptr = const_cast<T *>(&(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<config_option *> stack;
-      expand_meta(*str, opt, stack, &oss);
+  for (auto& opt: *config_options) {
+    std::string *str;
+    opt.conf_ptr(str, this);
+    if (str) {
+      list<config_option const *> 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<config_option *> stack;
+  list<config_option const *> stack;
   return expand_meta(origval, NULL, stack, oss);
 }
 
 bool md_config_t::expand_meta(std::string &origval,
-                             config_option *opt,
-                             std::list<config_option *> stack,
+                             config_option const *opt,
+                             std::list<config_option const *> 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<config_option *>::iterator i = stack.begin();
+    for (list<config_option const *>::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<config_option *>::iterator j = stack.begin();
+       for (list<config_option const *>::iterator j = stack.begin();
             j != stack.end();
             ++j) {
-         *oss << (*j)->name << "=" << *(string *)(*j)->conf_ptr(this) << std::endl;
+         *oss << (*j)->name << "=" << *((*j)->conf_ptr<std::string>(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<md_config_t *>(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)));
   }
 }
 
index d844395cba650f12dba555a271e052f6bdcfaac3..eb22a6f57aab9600431230c6e6248688e94b94a0 100644 (file)
@@ -19,6 +19,7 @@
 #include <vector>
 #include <map>
 #include <set>
+#include <boost/variant.hpp>
 
 #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<invalid_config_value_t,
+                         int,
+                         long long,
+                         std::string,
+                         double,
+                         float,
+                         bool,
+                         entity_addr_t,
+                         uint32_t,
+                         uint64_t,
+                         uuid_d> config_value_t;
+  typedef boost::variant<const int md_config_t::*,
+                         const long long md_config_t::*,
+                         const std::string md_config_t::*,
+                         const double md_config_t::*,
+                         const float md_config_t::*,
+                         const bool md_config_t::*,
+                         const entity_addr_t md_config_t::*,
+                         const uint32_t md_config_t::*,
+                         const uint64_t md_config_t::*,
+                         const uuid_d md_config_t::*> 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<typename T> struct get_typed_pointer_visitor : public boost::static_visitor<T const *> {
+      md_config_t const *conf;
+      explicit get_typed_pointer_visitor(md_config_t const *conf_) : conf(conf_) { }
+      template<typename U,
+       typename boost::enable_if<boost::is_same<T, U>, int>::type = 0>
+         T const *operator()(const U md_config_t::* member_ptr) {
+           return &(conf->*member_ptr);
+         }
+      template<typename U,
+       typename boost::enable_if_c<!boost::is_same<T, U>::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<typename T> void conf_ptr(T const *&ptr, md_config_t const *conf) const {
+      get_typed_pointer_visitor<T> gtpv(conf);
+      ptr = boost::apply_visitor(gtpv, md_member_ptr);
+    }
+    template<typename T> void conf_ptr(T *&ptr, md_config_t *conf) const {
+      get_typed_pointer_visitor<T> gtpv(conf);
+      ptr = const_cast<T *>(boost::apply_visitor(gtpv, md_member_ptr));
+    }
+    template<typename T> T const *conf_ptr(md_config_t const *conf) const {
+      get_typed_pointer_visitor<T> gtpv(conf);
+      return boost::apply_visitor(gtpv, md_member_ptr);
+    }
+    template<typename T> T *conf_ptr(md_config_t *conf) const {
+      get_typed_pointer_visitor<T> gtpv(conf);
+      return const_cast<T *>(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<typename T> T get_val(const char *key) const;
 
   void get_all_keys(std::vector<std::string> *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 <std::string> &sections) const;
@@ -178,8 +257,8 @@ private:
   int parse_config_files_impl(const std::list<std::string> &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<config_option *> stack,
+                  config_option const *opt,
+                  std::list<config_option const *> 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<const std::vector<config_option>> 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<typename T>
+struct get_typed_value_visitor : public boost::static_visitor<T> {
+  template<typename U,
+    typename boost::enable_if<boost::is_same<T, U>, int>::type = 0>
+      T operator()(U & val) {
+       return std::move(val);
+      }
+  template<typename U,
+    typename boost::enable_if_c<!boost::is_same<T, U>::value, int>::type = 0>
+      T operator()(U &val) {
+       assert("wrong type or option does not exist" == nullptr);
+      }
+};
+
+template<typename T> T md_config_t::get_val(const char *key) const {
+  config_value_t generic_val = this->get_val_generic(key);
+  get_typed_value_visitor<T> 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
 };
index 328626744ea43e9ff74808957a5d9455973ae351..42128f02fd5baaf535a1d47dc9b809b26adceed3 100644 (file)
 #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") {