mon/ConfigMap: push kv parsing logic into add_option()

Push shared code into ConfigMap from mon + mgr.  Pass in a function to
fetch the Option*, since in the mon case it may come from somewhere other
than g_conf.

Signed-off-by: Sage Weil <sage@newdream.net>
(cherry picked from commit d8ccd221549cdd28702932deb148f075cb6d88b3)

Fixed conflict: `mopt.localized_name = name;` is added to add_option()
(due to 3821722e5660437298a7c0f41e1061d363090103)

diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc
index 701b41bef8e1ce099ae9fe73f1da73c86d5b38df..4f75670591afd8597f003c87ded7e61b9f484bb9 100644 (file)
--- a/src/mgr/ActivePyModules.cc
+++ b/src/mgr/ActivePyModules.cc
@@ -866,42 +866,11 @@ void ActivePyModules::_refresh_config_map()
     string who;
     config_map.parse_key(key, &name, &who);
 
-    const Option *opt = g_conf().find_option(name);
-    if (!opt) {
-      config_map.stray_options.push_back(
-       std::unique_ptr<Option>(
-         new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
-      opt = config_map.stray_options.back().get();
-    }
-
-    string err;
-    int r = opt->pre_validate(&value, &err);
-    if (r < 0) {
-      dout(10) << __func__ << " pre-validate failed on '" << name << "' = '"
-              << value << "' for " << name << dendl;
-    }
-
-    MaskedOption mopt(opt);
-    mopt.raw_value = value;
-    string section_name;
-    if (who.size() &&
-       !ConfigMap::parse_mask(who, &section_name, &mopt.mask)) {
-      derr << __func__ << " invalid mask for key " << key << dendl;
-    } else if (opt->has_flag(Option::FLAG_NO_MON_UPDATE)) {
-      dout(10) << __func__ << " NO_MON_UPDATE option '"
-              << name << "' = '" << value << "' for " << name
-              << dendl;
-    } else {
-      Section *section = &config_map.global;;
-      if (section_name.size() && section_name != "global") {
-       if (section_name.find('.') != std::string::npos) {
-         section = &config_map.by_id[section_name];
-       } else {
-         section = &config_map.by_type[section_name];
-       }
-      }
-      section->options.insert(make_pair(name, std::move(mopt)));
-    }
+    config_map.add_option(
+      g_ceph_context, name, who, value,
+      [&](const std::string& name) {
+       return  g_conf().find_option(name);
+      });
   }
 }
 

diff --git a/src/mon/ConfigMap.cc b/src/mon/ConfigMap.cc
index dff1a94c1e6fc3e300eb1cc62695c16c6d9d1582..86528c1dedfeef11998583a1bdb9ca0e2c113601 100644 (file)
--- a/src/mon/ConfigMap.cc
+++ b/src/mon/ConfigMap.cc
@@ -7,6 +7,10 @@
 #include "crush/CrushWrapper.h"
 #include "common/entity_name.h"
 
+#define dout_subsys ceph_subsys_mon
+#undef dout_prefix
+#include "common/dout.h"
+
 using namespace std::literals;
 
 using std::cerr;
@@ -250,6 +254,59 @@ void ConfigMap::parse_key(
   }
 }
 
+int ConfigMap::add_option(
+  CephContext *cct,
+  const std::string& name,
+  const std::string& who,
+  const std::string& orig_value,
+  std::function<const Option *(const std::string&)> get_opt)
+{
+  const Option *opt = get_opt(name);
+  if (!opt) {
+    ldout(cct, 10) << __func__ << " unrecognized option '" << name << "'" << dendl;
+    stray_options.push_back(
+      std::unique_ptr<Option>(
+       new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
+    opt = stray_options.back().get();
+  }
+
+  string err;
+  string value = orig_value;
+  int r = opt->pre_validate(&value, &err);
+  if (r < 0) {
+    ldout(cct, 10) << __func__ << " pre-validate failed on '" << name << "' = '"
+                  << value << "' for " << name << dendl;
+  }
+
+  int ret = 0;
+  MaskedOption mopt(opt);
+  mopt.raw_value = value;
+  mopt.localized_name = name;
+  string section_name;
+  if (who.size() &&
+      !ConfigMap::parse_mask(who, &section_name, &mopt.mask)) {
+    lderr(cct) << __func__ << " invalid mask for option " << name << " mask " << who
+              << dendl;
+    ret = -EINVAL;
+  } else if (opt->has_flag(Option::FLAG_NO_MON_UPDATE)) {
+    ldout(cct, 10) << __func__ << " NO_MON_UPDATE option '"
+                  << name << "' = '" << value << "' for " << name
+                  << dendl;
+    ret = -EINVAL;
+  } else {
+    Section *section = &global;;
+    if (section_name.size() && section_name != "global") {
+      if (section_name.find('.') != std::string::npos) {
+       section = &by_id[section_name];
+      } else {
+       section = &by_type[section_name];
+      }
+    }
+    section->options.insert(make_pair(name, std::move(mopt)));
+  }
+  return ret;
+}
+
 
 // --------------
 

diff --git a/src/mon/ConfigMap.h b/src/mon/ConfigMap.h
index bf2bd97c0ba38f9533841c26b5788a448f9cc7e4..5a14d089a72005b0d9056ec983e113b5fa58e4b7 100644 (file)
--- a/src/mon/ConfigMap.h
+++ b/src/mon/ConfigMap.h
@@ -149,6 +149,13 @@ struct ConfigMap {
     const std::string& in,
     std::string *section,
     OptionMask *mask);
+
+  int add_option(
+    CephContext *cct,
+    const std::string& name,
+    const std::string& who,
+    const std::string& value,
+    std::function<const Option *(const std::string&)> get_opt);
 };
 
 

diff --git a/src/mon/ConfigMonitor.cc b/src/mon/ConfigMonitor.cc
index a5f61cbea529219f7f82f4e9ddc5831f4c88e2fa..6480674926dad399c249e500d11c032e493f4947 100644 (file)
--- a/src/mon/ConfigMonitor.cc
+++ b/src/mon/ConfigMonitor.cc
@@ -785,9 +785,10 @@ void ConfigMonitor::load_config()
 
   unsigned num = 0;
   KeyValueDB::Iterator it = mon.store->get_iterator(KV_PREFIX);
-  it->lower_bound(KEY_PREFIX);
-  while (it->valid() &&
-        it->key().compare(0, KEY_PREFIX.size(), KEY_PREFIX) == 0) {
+  for (it->lower_bound(KEY_PREFIX);
+       it->valid() &&
+        it->key().compare(0, KEY_PREFIX.size(), KEY_PREFIX) == 0;
+       it->next(), ++num) {
     string key = it->key().substr(KEY_PREFIX.size());
     string value = it->value().to_str();
 
@@ -811,58 +812,19 @@ void ConfigMonitor::load_config()
       }
     }
 
-    const Option *opt = g_conf().find_option(name);
-    if (!opt) {
-      opt = mon.mgrmon()->find_module_option(name);
-    }
-    if (!opt) {
-      dout(10) << __func__ << " unrecognized option '" << name << "'" << dendl;
-      config_map.stray_options.push_back(
-       std::unique_ptr<Option>(
-         new Option(name, Option::TYPE_STR, Option::LEVEL_UNKNOWN)));
-      opt = config_map.stray_options.back().get();
-    }
-
-    string err;
-    int r = opt->pre_validate(&value, &err);
-    if (r < 0) {
-      dout(10) << __func__ << " pre-validate failed on '" << name << "' = '"
-              << value << "' for " << name << dendl;
-    }
-    
-    MaskedOption mopt(opt);
-    mopt.raw_value = value;
-    mopt.localized_name = name;
-    string section_name;
-    if (who.size() &&
-       !ConfigMap::parse_mask(who, &section_name, &mopt.mask)) {
-      derr << __func__ << " invalid mask for key " << key << dendl;
-      pending_cleanup[key].reset();
-    } else if (opt->has_flag(Option::FLAG_NO_MON_UPDATE)) {
-      dout(10) << __func__ << " NO_MON_UPDATE option '"
-              << name << "' = '" << value << "' for " << name
-              << dendl;
-      pending_cleanup[key].reset();
-    } else {
-      if (section_name.empty()) {
-       // we prefer global/$option instead of just $option
-       derr << __func__ << " adding global/ prefix to key '" << key << "'"
-            << dendl;
-       pending_cleanup[key].reset();
-       pending_cleanup["global/"s + key] = it->value();
-      }
-      Section *section = &config_map.global;;
-      if (section_name.size() && section_name != "global") {
-       if (section_name.find('.') != std::string::npos) {
-         section = &config_map.by_id[section_name];
-       } else {
-         section = &config_map.by_type[section_name];
+    int r = config_map.add_option(
+      g_ceph_context, name, who, value,
+      [&](const std::string& name) {
+       const Option *opt = g_conf().find_option(name);
+       if (!opt) {
+         opt = mon.mgrmon()->find_module_option(name);
        }
-      }
-      section->options.insert(make_pair(name, std::move(mopt)));
-      ++num;
+       return opt;
+      });
+    if (r == -EINVAL) {
+      dout(10) << __func__ << " will clean up key " << key << dendl;
+      pending_cleanup[key].reset();
     }
-    it->next();
   }
   dout(10) << __func__ << " got " << num << " keys" << dendl;