From 3933adef6a0832b8f140e344e42855c2e4dfb051 Mon Sep 17 00:00:00 2001
From: John Spray <john.spray@redhat.com>
Date: Tue, 11 Jul 2017 07:00:54 -0400
Subject: [PATCH] common: reinstate config opt validators

Signed-off-by: John Spray <john.spray@redhat.com>
---
 src/common/config.cc  |  23 ++++----
 src/common/options.cc | 103 +++++++++++++++++++++++++++++++-----
 src/common/options.h  | 120 ++++++------------------------------------
 3 files changed, 116 insertions(+), 130 deletions(-)

diff --git a/src/common/config.cc b/src/common/config.cc
index 12a9c317e9f..0ab8e192f48 100644
--- a/src/common/config.cc
+++ b/src/common/config.cc
@@ -971,35 +971,31 @@ 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 std::string &val, const Option &opt,
+int md_config_t::set_val_impl(const std::string &raw_val, const Option &opt,
                               std::string *error_message)
 {
   assert(lock.is_locked());
 
-  // TODO: hook validators back in
-#if 0
+  std::string val = raw_val;
   if (opt.validator) {
-    int r = opt.validator(&value, error_message);
+    int r = opt.validator(&val, error_message);
     if (r < 0) {
       return r;
     }
   }
-#endif
 
   Option::value_t new_value;
   if (opt.type == Option::TYPE_INT) {
-    std::string err;
-    int64_t f = strict_si_cast<int64_t>(val.c_str(), &err);
-    if (!err.empty()) {
+    int64_t f = strict_si_cast<int64_t>(val.c_str(), error_message);
+    if (!error_message->empty()) {
       return -EINVAL;
     }
     new_value = f;
   } else if (opt.type == Option::TYPE_STR) {
     new_value = val;
   } else if (opt.type == Option::TYPE_FLOAT) {
-    std::string err;
-    double f = strict_strtod(val.c_str(), &err);
-    if (!err.empty()) {
+    double f = strict_strtod(val.c_str(), error_message);
+    if (!error_message->empty()) {
       return -EINVAL;
     } else {
       new_value = f;
@@ -1010,9 +1006,8 @@ int md_config_t::set_val_impl(const std::string &val, const Option &opt,
     } else if (strcasecmp(val.c_str(), "true") == 0) {
       new_value = true;
     } else {
-      std::string err;
-      int b = strict_strtol(val.c_str(), 10, &err);
-      if (!err.empty()) {
+      int b = strict_strtol(val.c_str(), 10, error_message);
+      if (!error_message->empty()) {
 	return -EINVAL;
       }
       new_value = !!b;
diff --git a/src/common/options.cc b/src/common/options.cc
index d0c5b2d3666..1a7f64e6d76 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4,6 +4,11 @@
 #include "acconfig.h"
 #include "options.h"
 
+// Helpers for validators
+#include "include/stringify.h"
+#include <boost/algorithm/string.hpp>
+#include <boost/lexical_cast.hpp>
+#include <boost/regex.hpp>
 
 const std::vector<Option> ceph_options = {
 
@@ -214,6 +219,91 @@ const std::vector<Option> ceph_options = {
   .set_description("port number for the remote graylog server")
   .add_see_also("log_graylog_host"),
 
+  Option("rbd_default_pool", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+  .set_default("rbd")
+  .set_description("")
+  .set_validator([](std::string *value, std::string *error_message){
+    boost::regex pattern("^[^@/]+$");
+    if (!boost::regex_match (*value, pattern)) {
+      *value = "rbd";
+      *error_message = "invalid RBD default pool, resetting to 'rbd'";
+    }
+    return 0;
+  }),
+
+  Option("rbd_default_data_pool", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+  .set_default("")
+  .set_description("")
+  .set_validator([](std::string *value, std::string *error_message){
+    boost::regex pattern("^[^@/]*$");
+    if (!boost::regex_match (*value, pattern)) {
+      *value = "";
+      *error_message = "ignoring invalid RBD data pool";
+    }
+    return 0;
+  }),
+
+  Option("rbd_default_features", Option::TYPE_STR, Option::LEVEL_ADVANCED)
+  .set_default("layering,exclusive-lock,object-map,fast-diff,deep-flatten")
+  .set_description("")
+  .set_safe()
+  .set_validator([](std::string *value, std::string *error_message){
+    static const std::map<std::string, uint64_t> FEATURE_MAP = {
+      {RBD_FEATURE_NAME_LAYERING, RBD_FEATURE_LAYERING},
+      {RBD_FEATURE_NAME_STRIPINGV2, RBD_FEATURE_STRIPINGV2},
+      {RBD_FEATURE_NAME_EXCLUSIVE_LOCK, RBD_FEATURE_EXCLUSIVE_LOCK},
+      {RBD_FEATURE_NAME_OBJECT_MAP, RBD_FEATURE_OBJECT_MAP},
+      {RBD_FEATURE_NAME_FAST_DIFF, RBD_FEATURE_FAST_DIFF},
+      {RBD_FEATURE_NAME_DEEP_FLATTEN, RBD_FEATURE_DEEP_FLATTEN},
+      {RBD_FEATURE_NAME_JOURNALING, RBD_FEATURE_JOURNALING},
+      {RBD_FEATURE_NAME_DATA_POOL, RBD_FEATURE_DATA_POOL},
+    };
+    static_assert((RBD_FEATURE_DATA_POOL << 1) > RBD_FEATURES_ALL,
+                  "new RBD feature added");
+
+    // convert user-friendly comma delimited feature name list to a bitmask
+    // that is used by the librbd API
+    uint64_t features = 0;
+    error_message->clear();
+
+    try {
+      features = boost::lexical_cast<decltype(features)>(*value);
+
+      uint64_t unsupported_features = (features & ~RBD_FEATURES_ALL);
+      if (unsupported_features != 0ull) {
+        features &= RBD_FEATURES_ALL;
+
+        std::stringstream ss;
+        ss << "ignoring unknown feature mask 0x"
+           << std::hex << unsupported_features;
+        *error_message = ss.str();
+      }
+    } catch (const boost::bad_lexical_cast& ) {
+      int r = 0;
+      std::vector<std::string> feature_names;
+      boost::split(feature_names, *value, boost::is_any_of(","));
+      for (auto feature_name: feature_names) {
+        boost::trim(feature_name);
+        auto feature_it = FEATURE_MAP.find(feature_name);
+        if (feature_it != FEATURE_MAP.end()) {
+          features += feature_it->second;
+        } else {
+          if (!error_message->empty()) {
+            *error_message += ", ";
+          }
+          *error_message += "ignoring unknown feature " + feature_name;
+          r = -EINVAL;
+        }
+      }
+
+      if (features == 0 && r == -EINVAL) {
+        features = RBD_FEATURES_DEFAULT;
+      }
+    }
+    *value = stringify(features);
+    return 0;
+  }),
+
 
   // unmodified
   Option("clog_to_monitors", Option::TYPE_STR, Option::LEVEL_ADVANCED)
@@ -4232,10 +4322,6 @@ const std::vector<Option> ceph_options = {
   .set_default(0)
   .set_description(""),
 
-  Option("rbd_default_pool", Option::TYPE_STR, Option::LEVEL_ADVANCED)
-  .set_default("rbd")
-  .set_description(""),
-
   Option("rbd_default_format", Option::TYPE_INT, Option::LEVEL_ADVANCED)
   .set_default(2)
   .set_description(""),
@@ -4252,15 +4338,6 @@ const std::vector<Option> ceph_options = {
   .set_default(0)
   .set_description(""),
 
-  Option("rbd_default_data_pool", Option::TYPE_STR, Option::LEVEL_ADVANCED)
-  .set_default("")
-  .set_description(""),
-
-  Option("rbd_default_features", Option::TYPE_STR, Option::LEVEL_ADVANCED)
-  .set_default("layering,exclusive-lock,object-map,fast-diff,deep-flatten")
-  .set_description("")
-  .set_safe(),
-
   Option("rbd_default_map_options", Option::TYPE_STR, Option::LEVEL_ADVANCED)
   .set_default("")
   .set_description(""),
diff --git a/src/common/options.h b/src/common/options.h
index 53ef1d308e9..24e48eb9bcf 100644
--- a/src/common/options.h
+++ b/src/common/options.h
@@ -9,9 +9,6 @@
 #include "msg/msg_types.h"
 #include "include/uuid.h"
 
-//#include "include/uuid.h"
-//#include "msg/msg_types.h"
-
 struct Option {
   enum type_t {
     TYPE_INT,
@@ -32,7 +29,7 @@ struct Option {
     case TYPE_UUID: return "uuid_d";
     default: return "unknown";
     }
-  }      
+  }
 
   enum level_t {
     LEVEL_BASIC,
@@ -67,6 +64,16 @@ struct Option {
 
   bool safe;
 
+  /**
+   * Return nonzero and set second argument to error string if the
+   * value is invalid.
+   *
+   * These callbacks are more than just validators, as they can also
+   * modify the value as it passes through.
+   */
+  typedef std::function<int(std::string *, std::string *)> validator_fn_t;
+  validator_fn_t validator;
+
   Option(std::string const &name, type_t t, level_t l)
     : name(name), type(t), level(l), safe(false)
   {
@@ -165,6 +172,12 @@ struct Option {
     return *this;
   }
 
+  Option &set_validator(const validator_fn_t  &validator_)
+  {
+    validator = validator_;
+    return *this;
+  }
+
   /**
    * A crude indicator of whether the value may be
    * modified safely at runtime -- should be replaced
@@ -176,104 +189,5 @@ struct Option {
   }
 };
 
-// TODO: reinstate corner case logic for these RBD settings
-#if 0
-#include "include/stringify.h"
-
-#include <boost/algorithm/string.hpp>
-#include <boost/lexical_cast.hpp>
-#include <boost/regex.hpp>
-
-
-class RbdDefaultPool : public Option
-{
-  int validate(md_config_t::option_rbd_default_pool_t *,
-               std::string *value, std::string *error_message) {
-    boost::regex pattern("^[^@/]+$");
-    if (!boost::regex_match (*value, pattern)) {
-      *value = "rbd";
-      *error_message = "invalid RBD default pool, resetting to 'rbd'";
-    }
-    return 0;
-  }
-}
-
-class RbdDefaultDataPool : public Option
-{
-  int validate(md_config_t::option_rbd_default_data_pool_t *,
-               std::string *value, std::string *error_message) {
-    boost::regex pattern("^[^@/]*$");
-    if (!boost::regex_match (*value, pattern)) {
-      *value = "";
-      *error_message = "ignoring invalid RBD data pool";
-    }
-    return 0;
-  }
-}
-
-class RbdDefaultFeatures : public Option
-{
-  int validate(md_config_t::option_rbd_default_features_t *,
-               std::string *value, std::string *error_message) {
-    static const std::map<std::string, uint64_t> FEATURE_MAP = {
-      {RBD_FEATURE_NAME_LAYERING, RBD_FEATURE_LAYERING},
-      {RBD_FEATURE_NAME_STRIPINGV2, RBD_FEATURE_STRIPINGV2},
-      {RBD_FEATURE_NAME_EXCLUSIVE_LOCK, RBD_FEATURE_EXCLUSIVE_LOCK},
-      {RBD_FEATURE_NAME_OBJECT_MAP, RBD_FEATURE_OBJECT_MAP},
-      {RBD_FEATURE_NAME_FAST_DIFF, RBD_FEATURE_FAST_DIFF},
-      {RBD_FEATURE_NAME_DEEP_FLATTEN, RBD_FEATURE_DEEP_FLATTEN},
-      {RBD_FEATURE_NAME_JOURNALING, RBD_FEATURE_JOURNALING},
-      {RBD_FEATURE_NAME_DATA_POOL, RBD_FEATURE_DATA_POOL},
-    };
-    static_assert((RBD_FEATURE_DATA_POOL << 1) > RBD_FEATURES_ALL,
-                  "new RBD feature added");
-
-    // convert user-friendly comma delimited feature name list to a bitmask
-    // that is used by the librbd API
-    uint64_t features = 0;
-    error_message->clear();
-
-    try {
-      features = boost::lexical_cast<decltype(features)>(*value);
-
-      uint64_t unsupported_features = (features & ~RBD_FEATURES_ALL);
-      if (unsupported_features != 0ull) {
-        features &= RBD_FEATURES_ALL;
-
-        std::stringstream ss;
-        ss << "ignoring unknown feature mask 0x"
-           << std::hex << unsupported_features;
-        *error_message = ss.str();
-      }
-    } catch (const boost::bad_lexical_cast& ) {
-      int r = 0;
-      std::vector<std::string> feature_names;
-      boost::split(feature_names, *value, boost::is_any_of(","));
-      for (auto feature_name: feature_names) {
-        boost::trim(feature_name);
-        auto feature_it = FEATURE_MAP.find(feature_name);
-        if (feature_it != FEATURE_MAP.end()) {
-          features += feature_it->second;
-        } else {
-          if (!error_message->empty()) {
-            *error_message += ", ";
-          }
-          *error_message += "ignoring unknown feature " + feature_name;
-          r = -EINVAL;
-        }
-      }
-
-      if (features == 0 && r == -EINVAL) {
-        features = RBD_FEATURES_DEFAULT;
-      }
-    }
-    *value = stringify(features);
-    return 0;
-  }
-
-}
-
-#endif
-
 extern const std::vector<Option> ceph_options;
 
-- 
2.39.5