From 9e2fc91d2d3b10a068ece466877809697860f067 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 10 Nov 2017 17:25:25 -0600 Subject: [PATCH] common/config: make internal_safe_to_start_threads internal There is no reason for this to be exposed like a normal config option (even a legacy one) since it is an internal safety flag. We do keep the clear_() helper there for the unit test. Signed-off-by: Sage Weil --- src/common/ceph_context.cc | 2 +- src/common/config.cc | 28 +++++++++++++++------------- src/common/config.h | 9 ++++++++- src/common/legacy_config_opts.h | 4 ---- src/common/options.cc | 4 ---- src/test/daemon_config.cc | 26 +++++++++----------------- 6 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index 2f5256baf54..b2b3957fcb6 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -743,7 +743,7 @@ void CephContext::start_service_thread() // Trigger callbacks on any config observers that were waiting for // it to become safe to start threads. - _conf->set_val("internal_safe_to_start_threads", "true"); + _conf->set_safe_to_start_threads(); _conf->call_all_observers(); // start admin socket diff --git a/src/common/config.cc b/src/common/config.cc index 404928f0e36..44fc7501067 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -221,7 +221,7 @@ int md_config_t::parse_config_files(const char *conf_files, { Mutex::Locker l(lock); - if (internal_safe_to_start_threads) + if (safe_to_start_threads) return -ENOSYS; if (!cluster.size() && !conf_files) { @@ -372,7 +372,7 @@ int md_config_t::parse_config_files_impl(const std::list &conf_file void md_config_t::parse_env() { Mutex::Locker l(lock); - if (internal_safe_to_start_threads) + if (safe_to_start_threads) return; if (getenv("CEPH_KEYRING")) { set_val_or_die("keyring", getenv("CEPH_KEYRING")); @@ -430,7 +430,7 @@ void md_config_t::_show_config(std::ostream *out, Formatter *f) int md_config_t::parse_argv(std::vector& args) { Mutex::Locker l(lock); - if (internal_safe_to_start_threads) { + if (safe_to_start_threads) { return -ENOSYS; } @@ -662,13 +662,6 @@ void md_config_t::apply_changes(std::ostream *oss) _apply_changes(oss); } -bool md_config_t::_internal_field(const string& s) -{ - if (s == "internal_safe_to_start_threads") - return true; - return false; -} - void md_config_t::_apply_changes(std::ostream *oss) { /* Maps observers to the configuration options that they care about which @@ -698,8 +691,7 @@ void md_config_t::_apply_changes(std::ostream *oss) pair < obs_map_t::iterator, obs_map_t::iterator > range(observers.equal_range(key)); if ((oss) && - (!_get_val(key.c_str(), &bufptr, sizeof(buf))) && - !_internal_field(key)) { + !_get_val(key.c_str(), &bufptr, sizeof(buf))) { (*oss) << key << " = '" << buf << "' "; if (range.first == range.second) { (*oss) << "(not observed, change may require restart) "; @@ -750,6 +742,16 @@ void md_config_t::call_all_observers() } } +void md_config_t::set_safe_to_start_threads() +{ + safe_to_start_threads = true; +} + +void md_config_t::_clear_safe_to_start_threads() +{ + safe_to_start_threads = false; +} + int md_config_t::injectargs(const std::string& s, std::ostream *oss) { int ret; @@ -839,7 +841,7 @@ int md_config_t::set_val(const std::string &key, const char *val, const auto &opt_iter = schema.find(k); if (opt_iter != schema.end()) { const Option &opt = opt_iter->second; - if ((!opt.is_safe()) && internal_safe_to_start_threads) { + if ((!opt.is_safe()) && 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... diff --git a/src/common/config.h b/src/common/config.h index a4efd34efe7..e0266d882ca 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -55,7 +55,7 @@ extern const char *CEPH_CONF_FILE_DEFAULT; * * 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 + * md_config_t::safe_to_start_threads becomes true. You can still * 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 @@ -139,6 +139,9 @@ public: bool _internal_field(const string& k); void call_all_observers(); + void set_safe_to_start_threads(); + void _clear_safe_to_start_threads(); // this is only used by the unit test + // Called by the Ceph daemons to make configuration changes at runtime int injectargs(const std::string &s, std::ostream *oss); @@ -255,6 +258,10 @@ public: std::deque parse_errors; private: + // This will be set to true when it is safe to start threads. + // Once it is true, it will never change. + bool safe_to_start_threads = false; + obs_map_t observers; changed_set_t changed; diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h index e7102a0d97b..7f65bb940a7 100644 --- a/src/common/legacy_config_opts.h +++ b/src/common/legacy_config_opts.h @@ -1519,10 +1519,6 @@ OPTION(rgw_torrent_sha_unit, OPT_INT) // torrent field piece length 512K OPTION(event_tracing, OPT_BOOL) // true if LTTng-UST tracepoints should be enabled -// This will be set to true when it is safe to start threads. -// Once it is true, it will never change. -OPTION(internal_safe_to_start_threads, OPT_BOOL) - OPTION(debug_deliberately_leak_memory, OPT_BOOL) OPTION(rgw_swift_custom_header, OPT_STR) // option to enable swift custom headers diff --git a/src/common/options.cc b/src/common/options.cc index 2e70f1c592e..6064e0c76bf 100644 --- a/src/common/options.cc +++ b/src/common/options.cc @@ -4291,10 +4291,6 @@ std::vector