From 7c25de3799da8cdb12f0eaf28b187c5ea9ba5803 Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Tue, 13 Feb 2018 15:56:47 +0800
Subject: [PATCH] common/config: specialize the settings for size

Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/common/config.cc           | 29 ++++++++++++++
 src/common/options.cc          | 17 +++++++--
 src/common/options.h           | 70 ++++++++++++++++++++++------------
 src/mgr/DaemonServer.cc        |  8 ++--
 src/test/common/test_config.cc |  8 ++++
 5 files changed, 99 insertions(+), 33 deletions(-)

diff --git a/src/common/config.cc b/src/common/config.cc
index a1f0fbdbf2e..b0912f90ae9 100644
--- a/src/common/config.cc
+++ b/src/common/config.cc
@@ -1413,6 +1413,22 @@ int md_config_t::_rm_val(const std::string& key, int level)
   return 0;
 }
 
+namespace {
+template<typename Size>
+struct get_size_visitor : public boost::static_visitor<Size>
+{
+  template<typename T>
+  Size operator()(const T&) const {
+    return -1;
+  }
+  Size operator()(const Option::size_t& sz) const {
+    return static_cast<Size>(sz.value);
+  }
+  Size operator()(const Size& v) const {
+    return v;
+  }
+};
+
 /**
  * Handles assigning from a variant-of-types to a variant-of-pointers-to-types
  */
@@ -1433,7 +1449,20 @@ class assign_visitor : public boost::static_visitor<>
 
     *member = boost::get<T>(val);
   }
+  void operator()(uint64_t md_config_t::* ptr) const
+  {
+    using T = uint64_t;
+    auto member = const_cast<T*>(&(conf->*(boost::get<const T md_config_t::*>(ptr))));
+    *member = boost::apply_visitor(get_size_visitor<T>{}, val);
+  }
+  void operator()(int64_t md_config_t::* ptr) const
+  {
+    using T = int64_t;
+    auto member = const_cast<T*>(&(conf->*(boost::get<const T md_config_t::*>(ptr))));
+    *member = boost::apply_visitor(get_size_visitor<T>{}, val);
+  }
 };
+} // anonymous namespace
 
 void md_config_t::update_legacy_vals()
 {
diff --git a/src/common/options.cc b/src/common/options.cc
index 88fb0689521..a2a61f13c1d 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -34,6 +34,9 @@ void Option::dump_value(const char *field_name,
     f->dump_float(field_name, boost::get<double>(v));
   } else if (type == TYPE_BOOL) {
     f->dump_bool(field_name, boost::get<bool>(v));
+  } else if (type == TYPE_SIZE) {
+    auto bytes = boost::get<size_t>(v);
+    f->dump_stream(field_name) << prettybyte_t(bytes.value);
   } else {
     f->dump_stream(field_name) << v;
   }
@@ -146,6 +149,12 @@ int Option::parse_value(
       return -EINVAL;
     }
     *out = uuid;
+  } else if (type == Option::TYPE_SIZE) {
+    Option::size_t sz{strict_sistrtoll(val.c_str(), error_message)};
+    if (!error_message->empty()) {
+      return -EINVAL;
+    }
+    *out = sz;
   } else {
     ceph_abort();
   }
@@ -4611,7 +4620,7 @@ std::vector<Option> get_global_options() {
                           "if you simply do not require the most up to date "
                           "performance counter data."),
 
-    Option("mgr_client_bytes", Option::TYPE_UINT, Option::LEVEL_DEV)
+    Option("mgr_client_bytes", Option::TYPE_SIZE, Option::LEVEL_DEV)
     .set_default(128_M)
     .add_service("mgr"),
 
@@ -4619,7 +4628,7 @@ std::vector<Option> get_global_options() {
     .set_default(512)
     .add_service("mgr"),
 
-    Option("mgr_osd_bytes", Option::TYPE_UINT, Option::LEVEL_DEV)
+    Option("mgr_osd_bytes", Option::TYPE_SIZE, Option::LEVEL_DEV)
     .set_default(512_M)
     .add_service("mgr"),
 
@@ -4627,7 +4636,7 @@ std::vector<Option> get_global_options() {
     .set_default(8192)
     .add_service("mgr"),
 
-    Option("mgr_mds_bytes", Option::TYPE_UINT, Option::LEVEL_DEV)
+    Option("mgr_mds_bytes", Option::TYPE_SIZE, Option::LEVEL_DEV)
     .set_default(128_M)
     .add_service("mgr"),
 
@@ -4635,7 +4644,7 @@ std::vector<Option> get_global_options() {
     .set_default(128)
     .add_service("mgr"),
 
-    Option("mgr_mon_bytes", Option::TYPE_UINT, Option::LEVEL_DEV)
+    Option("mgr_mon_bytes", Option::TYPE_SIZE, Option::LEVEL_DEV)
     .set_default(128_M)
     .add_service("mgr"),
 
diff --git a/src/common/options.h b/src/common/options.h
index ff7a8b8b629..8504ba891d1 100644
--- a/src/common/options.h
+++ b/src/common/options.h
@@ -19,6 +19,7 @@ struct Option {
     TYPE_BOOL,
     TYPE_ADDR,
     TYPE_UUID,
+    TYPE_SIZE,
   };
 
   const char *type_to_str(type_t t) const {
@@ -30,6 +31,7 @@ struct Option {
     case TYPE_BOOL: return "bool";
     case TYPE_ADDR: return "entity_addr_t";
     case TYPE_UUID: return "uuid_d";
+    case TYPE_SIZE: return "size_t";
     default: return "unknown";
     }
   }
@@ -63,6 +65,16 @@ struct Option {
     FLAG_CREATE = 0x10,         ///< option only has effect at daemon creation
   };
 
+  struct size_t {
+    std::size_t value;
+    operator uint64_t() const {
+      return static_cast<uint64_t>(value);
+    }
+    bool operator==(const size_t& rhs) const {
+      return value == rhs.value;
+    }
+  };
+
   using value_t = boost::variant<
     boost::blank,
     std::string,
@@ -71,6 +83,7 @@ struct Option {
     double,
     bool,
     entity_addr_t,
+    size_t,
     uuid_d>;
   const std::string name;
   const type_t type;
@@ -124,21 +137,24 @@ struct Option {
     // While value_t is nullable (via boost::blank), we don't ever
     // want it set that way in an Option instance: within an instance,
     // the type of ::value should always match the declared type.
-    if (type == TYPE_INT) {
-      value = int64_t(0);
-    } else if (type == TYPE_UINT) {
-      value = uint64_t(0);
-    } else if (type == TYPE_STR) {
-      value = std::string("");
-    } else if (type == TYPE_FLOAT) {
-      value = 0.0;
-    } else if (type == TYPE_BOOL) {
-      value = false;
-    } else if (type == TYPE_ADDR) {
-      value = entity_addr_t();
-    } else if (type == TYPE_UUID) {
-      value = uuid_d();
-    } else {
+    switch (type) {
+    case TYPE_INT:
+      value = int64_t(0); break;
+    case TYPE_UINT:
+      value = uint64_t(0); break;
+    case TYPE_STR:
+      value = std::string(""); break;
+    case TYPE_FLOAT:
+      value = 0.0; break;
+    case TYPE_BOOL:
+      value = false; break;
+    case TYPE_ADDR:
+      value = entity_addr_t(); break;
+    case TYPE_UUID:
+      value = uuid_d(); break;
+    case TYPE_SIZE:
+      value = size_t{0}; break;
+    default:
       ceph_abort();
     }
   }
@@ -175,15 +191,18 @@ struct Option {
   // a float option to "0" actually sets the double part of variant.
   template<typename T, typename is_integer<T>::type = 0>
   Option& set_value(value_t& v, T new_value) {
-    if (type == TYPE_INT) {
-      v = int64_t(new_value);
-    } else if (type == TYPE_UINT) {
-      v = uint64_t(new_value);
-    } else if (type == TYPE_FLOAT) {
-      v = double(new_value);
-    } else if (type == TYPE_BOOL) {
-      v = bool(new_value);
-    } else {
+    switch (type) {
+    case TYPE_INT:
+      v = int64_t(new_value); break;
+    case TYPE_UINT:
+      v = uint64_t(new_value); break;
+    case TYPE_FLOAT:
+      v = double(new_value); break;
+    case TYPE_BOOL:
+      v = bool(new_value); break;
+    case TYPE_SIZE:
+      v = size_t{static_cast<std::size_t>(new_value)}; break;
+    default:
       std::cerr << "Bad type in set_value: " << name << ": "
                 << typeid(T).name() << std::endl;
       ceph_abort();
@@ -296,7 +315,8 @@ struct Option {
     return
       (has_flag(FLAG_RUNTIME)
        || type == TYPE_BOOL || type == TYPE_INT
-       || type == TYPE_UINT || type == TYPE_FLOAT)
+       || type == TYPE_UINT || type == TYPE_FLOAT
+       || type == TYPE_SIZE)
       && !has_flag(FLAG_STARTUP)
       && !has_flag(FLAG_CLUSTER_CREATE)
       && !has_flag(FLAG_CREATE);
diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc
index ea3f700fe5c..0d5dac7a095 100644
--- a/src/mgr/DaemonServer.cc
+++ b/src/mgr/DaemonServer.cc
@@ -49,19 +49,19 @@ DaemonServer::DaemonServer(MonClient *monc_,
 			   LogChannelRef audit_clog_)
     : Dispatcher(g_ceph_context),
       client_byte_throttler(new Throttle(g_ceph_context, "mgr_client_bytes",
-					 g_conf->get_val<uint64_t>("mgr_client_bytes"))),
+					 g_conf->get_val<Option::size_t>("mgr_client_bytes"))),
       client_msg_throttler(new Throttle(g_ceph_context, "mgr_client_messages",
 					g_conf->get_val<uint64_t>("mgr_client_messages"))),
       osd_byte_throttler(new Throttle(g_ceph_context, "mgr_osd_bytes",
-				      g_conf->get_val<uint64_t>("mgr_osd_bytes"))),
+				      g_conf->get_val<Option::size_t>("mgr_osd_bytes"))),
       osd_msg_throttler(new Throttle(g_ceph_context, "mgr_osd_messsages",
 				     g_conf->get_val<uint64_t>("mgr_osd_messages"))),
       mds_byte_throttler(new Throttle(g_ceph_context, "mgr_mds_bytes",
-				      g_conf->get_val<uint64_t>("mgr_mds_bytes"))),
+				      g_conf->get_val<Option::size_t>("mgr_mds_bytes"))),
       mds_msg_throttler(new Throttle(g_ceph_context, "mgr_mds_messsages",
 				     g_conf->get_val<uint64_t>("mgr_mds_messages"))),
       mon_byte_throttler(new Throttle(g_ceph_context, "mgr_mon_bytes",
-				      g_conf->get_val<uint64_t>("mgr_mon_bytes"))),
+				      g_conf->get_val<Option::size_t>("mgr_mon_bytes"))),
       mon_msg_throttler(new Throttle(g_ceph_context, "mgr_mon_messsages",
 				     g_conf->get_val<uint64_t>("mgr_mon_messages"))),
       msgr(nullptr),
diff --git a/src/test/common/test_config.cc b/src/test/common/test_config.cc
index 043ef8bbbf3..5b2b62d5068 100644
--- a/src/test/common/test_config.cc
+++ b/src/test/common/test_config.cc
@@ -145,6 +145,14 @@ TEST(md_config_t, set_val)
     free(run_dir);
     free(admin_socket);
   }
+  // set_val should support SI conversion
+  {
+    auto expected = Option::size_t{512 << 20};
+    EXPECT_EQ(0, conf.set_val("mgr_osd_bytes", "512M", nullptr));
+    EXPECT_EQ(expected, conf.get_val<Option::size_t>("mgr_osd_bytes"));
+    EXPECT_EQ(-EINVAL, conf.set_val("mgr_osd_bytes", "512 bits", nullptr));
+    EXPECT_EQ(expected, conf.get_val<Option::size_t>("mgr_osd_bytes"));
+  }
 }
 
 TEST(Option, validation)
-- 
2.39.5