librbd: keep rbd_default_features setting as bitmask

Support both human readable, comma delimited list of feature
names and also integer bitmask value. Attempting to read the
setting will always result in the feature bitmask integer
value.

This is required to avoid breaking backwards compatibility with
librbd clients that are dependent on the older behavior (e.g.
OpenStack).

Fixes: http://tracker.ceph.com/issues/18247
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

src/CMakeLists.txt

@@ -476,6 +476,7 @@ set(libcommon_files
   common/ceph_strings.cc
   common/ceph_frag.cc
   common/config.cc
+  common/config_validators.cc
   common/utf8.c
   common/mime.c
   common/strtol.cc

src/common/config.cc

@@ -17,6 +17,7 @@
 #include "common/ceph_argparse.h"
 #include "common/common_init.h"
 #include "common/config.h"
+#include "common/config_validators.h"
 #include "common/static_assert.h"
 #include "common/strtol.h"
 #include "common/version.h"
@@ -36,6 +37,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include <type_traits>
 #include <utility>
 #include <boost/type_traits.hpp>
 #include <boost/utility/enable_if.hpp>
@@ -87,6 +89,42 @@ int ceph_resolve_file_search(const std::string& filename_list,
   return ret;
 }
 
+#define OPTION(name, type, def_val)
+#define OPTION_VALIDATOR(name)              \
+struct md_config_t::option_##name##_t {     \
+  typedef decltype(md_config_t::name) type; \
+};
+#define SAFE_OPTION(name, type, def_val)
+#define SUBSYS(name, log, gather)
+#define DEFAULT_SUBSYS(log, gather)
+#include "common/config_opts.h"
+#undef OPTION
+#undef OPTION_VALIDATOR
+#undef SAFE_OPTION
+#undef SUBSYS
+#undef DEFAULT_SUBSYS
+
+namespace {
+
+template <typename T>
+typename std::enable_if<!std::is_constructible<T>::value,
+                        md_config_t::validator_t>::type create_validator() {
+  return md_config_t::validator_t();
+}
+
+template <typename T>
+typename std::enable_if<std::is_constructible<T>::value,
+                        md_config_t::validator_t>::type create_validator() {
+  // if T is defined (and not just forward declared), it implies
+  // that a validator function exists. use a dummy typed pointer to
+  // pick the correct validator function
+  return [](std::string *value, std::string *error_message) {
+      return ::validate(reinterpret_cast<T*>(0), value, error_message);
+    };
+}
+
+} // anonymous namespace
+
 md_config_t::md_config_t()
   : cluster(""),
 
@@ -101,6 +139,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 OPTION_VALIDATOR(name)
 #define SAFE_OPTION(name, type, def_val) OPTION(name, type, def_val)
 #define SUBSYS(name, log, gather)
 #define DEFAULT_SUBSYS(log, gather)
@@ -116,6 +155,7 @@ md_config_t::md_config_t()
 #undef OPTION_OPT_U64
 #undef OPTION_OPT_UUID
 #undef OPTION
+#undef OPTION_VALIDATOR
 #undef SAFE_OPTION
 #undef SUBSYS
 #undef DEFAULT_SUBSYS
@@ -123,13 +163,17 @@ md_config_t::md_config_t()
 {
   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 },
+        config_option{ STRINGIFY(name), type, &md_config_t::name, safe, \
+                       create_validator<option_##name##_t>() },
 #define OPTION(name, type, def_val) OPTION4(name, type, def_val, false)
+#define OPTION_VALIDATOR(name)
 #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 OPTION4
 #undef OPTION
+#undef OPTION_VALIDATOR
 #undef SAFE_OPTION
 #undef SUBSYS
 #undef DEFAULT_SUBSYS
@@ -138,6 +182,7 @@ md_config_t::md_config_t()
     s_tbl(new std::vector<md_config_t::config_option>(std::move(s_config_options)));
   config_options = s_tbl;
 
+  validate_default_settings();
   init_subsys();
 }
 
@@ -148,9 +193,11 @@ void md_config_t::init_subsys()
 #define DEFAULT_SUBSYS(log, gather) \
   subsys.add(ceph_subsys_, "none", log, gather);
 #define OPTION(a, b, c)
+#define OPTION_VALIDATOR(a)
 #define SAFE_OPTION(a, b, c)
 #include "common/config_opts.h"
 #undef OPTION
+#undef OPTION_VALIDATOR
 #undef SAFE_OPTION
 #undef SUBSYS
 #undef DEFAULT_SUBSYS
@@ -282,7 +329,16 @@ int md_config_t::parse_config_files_impl(const std::list<std::string> &conf_file
     std::string val;
     int ret = _get_val_from_conf_file(my_sections, opt.name, val, false);
     if (ret == 0) {
-      set_val_impl(val.c_str(), &opt);
+      std::string error_message;
+      int r = set_val_impl(val, &opt, &error_message);
+      if (warnings != nullptr && (r != 0 || !error_message.empty())) {
+        *warnings << "parse error setting '" << opt.name << "' to '" << val
+                  << "'";
+        if (!error_message.empty()) {
+          *warnings << " (" << error_message << ")";
+        }
+        *warnings << std::endl;
+      }
     }
   }
 
@@ -515,20 +571,23 @@ int md_config_t::parse_option(std::vector<const char*>& args,
     return ret;
   }
 
+  const char *option_name = nullptr;
+  std::string error_message;
   o = 0;
   for (auto& opt_ref: *config_options) {
     ostringstream err;
     config_option const *opt = &opt_ref;
     std::string as_option("--");
     as_option += opt->name;
+    option_name = opt->name;
     if (opt->type == OPT_BOOL) {
       int res;
       if (ceph_argparse_binary_flag(args, i, &res, oss, as_option.c_str(),
 				    (char*)NULL)) {
 	if (res == 0)
-	  set_val_impl("false", opt);
+	  ret = set_val_impl("false", opt, &error_message);
 	else if (res == 1)
-	  set_val_impl("true", opt);
+	  ret = set_val_impl("true", opt, &error_message);
 	else
 	  ret = res;
 	break;
@@ -536,40 +595,46 @@ int md_config_t::parse_option(std::vector<const char*>& args,
 	std::string no("--no-");
 	no += opt->name;
 	if (ceph_argparse_flag(args, i, no.c_str(), (char*)NULL)) {
-	  set_val_impl("false", opt);
+	  ret = set_val_impl("false", opt, &error_message);
 	  break;
 	}
       }
-    }
+    } else if (ceph_argparse_witharg(args, i, &val, err,
-    else if (ceph_argparse_witharg(args, i, &val, err,
                                      as_option.c_str(), (char*)NULL)) {
       if (!err.str().empty()) {
-	*oss << err.str();
+        error_message = err.str();
 	ret = -EINVAL;
 	break;
       }
       if (oss && ((!opt->is_safe()) &&
 		  (observers.find(opt->name) == observers.end()))) {
 	*oss << "You cannot change " << opt->name << " using injectargs.\n";
-	ret = -ENOSYS;
+        return -ENOSYS;
-	break;
-      }
-      int res = set_val_impl(val.c_str(), opt);
-      if (res) {
-	if (oss) {
-	  *oss << "Parse error setting " << opt->name << " to '"
-	       << val << "' using injectargs.\n";
-	  ret = res;
-	  break;
-	} else {
-	  cerr << "parse error setting '" << opt->name << "' to '"
-	       << val << "'\n" << std::endl;
-	}
       }
+      ret = set_val_impl(val, opt, &error_message);
       break;
     }
     ++o;
   }
+
+  if (ret != 0 || !error_message.empty()) {
+    if (oss) {
+      *oss << "Parse error setting " << option_name << " to '"
+           << val << "' using injectargs";
+      if (!error_message.empty()) {
+        *oss << " (" << error_message << ")";
+      }
+      *oss << ".\n";
+    } else {
+      cerr << "parse error setting '" << option_name << "' to '"
+	   << val << "'";
+      if (!error_message.empty()) {
+        cerr << " (" << error_message << ")";
+      }
+      cerr << "\n" << std::endl;
+    }
+  }
+
   if (o == (int)config_options->size()) {
     // ignore
     ++i;
@@ -796,7 +861,10 @@ int md_config_t::set_val(const char *key, const char *val, bool meta, bool safe)
         return -ENOSYS;
       }
     }
-    return set_val_impl(v.c_str(), opt);
+
+    std::string error_message;
+    int r = set_val_impl(v, opt, &error_message);
+    return r;
   }
 
   // couldn't find a configuration option with key 'key'
@@ -843,6 +911,24 @@ md_config_t::config_value_t md_config_t::_get_val(const char *key) const
   return boost::apply_visitor(gvv, opt->md_member_ptr);
 }
 
+int md_config_t::_get_val(const char *key, std::string *value) const {
+  assert(lock.is_locked());
+
+  std::string normalized_key(ConfFile::normalize_key_name(key));
+  config_value_t config_value = _get_val(normalized_key.c_str());
+  if (!boost::get<invalid_config_value_t>(&config_value)) {
+    ostringstream oss;
+    if (bool *flag = boost::get<bool>(&config_value)) {
+      oss << (*flag ? "true" : "false");
+    } else {
+      oss << config_value;
+    }
+    *value = oss.str();
+    return 0;
+  }
+  return -ENOENT;
+}
+
 int md_config_t::_get_val(const char *key, char **buf, int len) const
 {
   assert(lock.is_locked());
@@ -964,10 +1050,19 @@ 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, config_option const *opt)
+int md_config_t::set_val_impl(const std::string &val, config_option const *opt,
+                              std::string *error_message)
 {
   assert(lock.is_locked());
-  int ret = set_val_raw(val, opt);
+  std::string value(val);
+  if (opt->validator) {
+    int r = opt->validator(&value, error_message);
+    if (r < 0) {
+      return r;
+    }
+  }
+
+  int ret = set_val_raw(value.c_str(), opt);
   if (ret)
     return ret;
   changed.insert(opt->name);
@@ -1271,3 +1366,19 @@ void md_config_t::complain_about_parse_errors(CephContext *cct)
 {
   ::complain_about_parse_errors(cct, &parse_errors);
 }
+
+void md_config_t::validate_default_settings() {
+  Mutex::Locker l(lock);
+  for (auto &opt : *config_options) {
+    // normalize config defaults using their validator
+    if (opt.validator) {
+      std::string value;
+      int r = _get_val(opt.name, &value);
+      assert(r == 0);
+
+      std::string error_message;
+      r = set_val_impl(value.c_str(), &opt, &error_message);
+      assert(r == 0);
+    }
+  }
+}

src/common/config.h

@@ -16,6 +16,7 @@
 #define CEPH_CONFIG_H
 
 #include <iosfwd>
+#include <functional>
 #include <vector>
 #include <map>
 #include <set>
@@ -113,12 +114,15 @@ public:
 	OPT_ADDR, OPT_U32, OPT_U64, OPT_UUID
    } opt_type_t;
 
+  typedef std::function<int(std::string*, std::string*)> validator_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
+    validator_t validator;
   private:
     template<typename T> struct get_typed_pointer_visitor : public boost::static_visitor<T const *> {
       md_config_t const *conf;
@@ -241,6 +245,9 @@ public:
   void complain_about_parse_errors(CephContext *cct);
 
 private:
+  void validate_default_settings();
+
+  int _get_val(const char *key, std::string *value) const;
   config_value_t _get_val(const char *key) const;
   void _show_config(std::ostream *out, Formatter *f);
 
@@ -257,7 +264,8 @@ private:
   int parse_config_files_impl(const std::list<std::string> &conf_files,
 			      std::ostream *warnings);
 
-  int set_val_impl(const char *val, config_option const *opt);
+  int set_val_impl(const std::string &val, config_option const *opt,
+                   std::string *error_message);
   int set_val_raw(const char *val, config_option const *opt);
 
   void init_subsys();
@@ -309,11 +317,14 @@ public:
 #define OPTION_OPT_UUID(name) const uuid_d name;
 #define OPTION(name, ty, init) \
   public:                      \
-    OPTION_##ty(name)
+    OPTION_##ty(name)          \
+    struct option_##name##_t;
+#define OPTION_VALIDATOR(name)
 #define SAFE_OPTION(name, ty, init) \
   protected:                        \
     OPTION_##ty(name)               \
-  public:
+  public:                           \
+    struct option_##name##_t;
 #define SUBSYS(name, log, gather)
 #define DEFAULT_SUBSYS(log, gather)
 #include "common/config_opts.h"
@@ -328,6 +339,7 @@ public:
 #undef OPTION_OPT_U64
 #undef OPTION_OPT_UUID
 #undef OPTION
+#undef OPTION_VALIDATOR
 #undef SAFE_OPTION
 #undef SUBSYS
 #undef DEFAULT_SUBSYS
@@ -385,6 +397,7 @@ typedef md_config_t::config_option config_option;
 enum config_subsys_id {
   ceph_subsys_,   // default
 #define OPTION(a,b,c)
+#define OPTION_VALIDATOR(name)
 #define SAFE_OPTION(a,b,c)
 #define SUBSYS(name, log, gather) \
   ceph_subsys_##name,
@@ -392,6 +405,7 @@ enum config_subsys_id {
 #include "common/config_opts.h"
 #undef SUBSYS
 #undef OPTION
+#undef OPTION_VALIDATOR
 #undef SAFE_OPTION
 #undef DEFAULT_SUBSYS
   ceph_subsys_max

src/common/config_opts.h

@@ -1299,9 +1299,26 @@ OPTION(rbd_default_format, OPT_INT, 2)
 OPTION(rbd_default_order, OPT_INT, 22)
 OPTION(rbd_default_stripe_count, OPT_U64, 0) // changing requires stripingv2 feature
 OPTION(rbd_default_stripe_unit, OPT_U64, 0) // changing to non-object size requires stripingv2 feature
-SAFE_OPTION(rbd_default_features, OPT_STR, "layering,exclusive-lock,object-map,fast-diff,deep-flatten")   // only applies to format 2 images
 OPTION(rbd_default_data_pool, OPT_STR, "") // optional default pool for storing image data blocks
 
+/**
+ * RBD features are only applicable for v2 images. This setting accepts either
+ * an integer bitmask value or comma-delimited string of RBD feature names.
+ * This setting is always internally stored as an integer bitmask value. The
+ * mapping between feature bitmask value and feature name is as follows:
+ *
+ *  +1 -> layering
+ *  +2 -> striping
+ *  +4 -> exclusive-lock
+ *  +8 -> object-map
+ *  +16 -> fast-diff
+ *  +32 -> deep-flatten
+ *  +64 -> journaling
+ *  +128 -> data-pool
+ */
+SAFE_OPTION(rbd_default_features, OPT_STR, "layering,exclusive-lock,object-map,fast-diff,deep-flatten")
+OPTION_VALIDATOR(rbd_default_features)
+
 OPTION(rbd_default_map_options, OPT_STR, "") // default rbd map -o / --options
 
 /**

src/common/config_validators.cc

@@ -0,0 +1,70 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#include "common/config_validators.h"
+#include "include/stringify.h"
+#include "include/rbd/features.h"
+#include <map>
+#include <sstream>
+#include <vector>
+#include <boost/algorithm/string.hpp>
+#include <boost/lexical_cast.hpp>
+
+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;
+}
+

src/common/config_validators.h

@@ -0,0 +1,17 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#ifndef CEPH_CONFIG_VALIDATORS
+#define CEPH_CONFIG_VALIDATORS
+
+#include "config.h"
+#include <string>
+
+/**
+ * Global config value validators for the Ceph project
+ */
+
+int validate(md_config_t::option_rbd_default_features_t *type,
+             std::string *value, std::string *error_message);
+
+#endif // CEPH_CONFIG_VALIDATORS

src/librbd/Utils.cc

@@ -64,38 +64,10 @@ std::string generate_image_id(librados::IoCtx &ioctx) {
   return id;
 }
 
-uint64_t parse_rbd_default_features(CephContext* cct)
+uint64_t get_rbd_default_features(CephContext* cct)
 {
-  int ret = 0;
-  uint64_t value = 0;
   auto str_val = cct->_conf->get_val<std::string>("rbd_default_features");
-  try {
+  return boost::lexical_cast<uint64_t>(str_val);
-      value = boost::lexical_cast<decltype(value)>(str_val);
-  } catch (const boost::bad_lexical_cast& ) {
-    map<std::string, int> conf_vals = {{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},
-    };
-    std::vector<std::string> strs;
-    boost::split(strs, str_val, boost::is_any_of(","));
-    for (auto feature: strs) {
-    	boost::trim(feature);
-      if (conf_vals.find(feature) != conf_vals.end()) {
-        value += conf_vals[feature];
-      } else {
-        ret = -EINVAL;
-        lderr(cct) << "ignoring unknown feature " << feature << dendl;
-      }
-    }
-    if (value == 0 && ret == -EINVAL)
-      value = RBD_FEATURES_DEFAULT;
-  }
-  return value;
 }
 
 } // namespace util

src/librbd/Utils.h

@@ -197,7 +197,7 @@ private:
   Context *m_on_finish = nullptr;
 };
 
-uint64_t parse_rbd_default_features(CephContext* cct);
+uint64_t get_rbd_default_features(CephContext* cct);
 
 } // namespace util
 

src/librbd/image/CreateRequest.cc

@@ -135,7 +135,7 @@ CreateRequest<I>::CreateRequest(IoCtx &ioctx, const std::string &image_name,
   m_objmap_name = ObjectMap<>::object_map_name(m_image_id, CEPH_NOSNAP);
 
   if (image_options.get(RBD_IMAGE_OPTION_FEATURES, &m_features) != 0) {
-    m_features = util::parse_rbd_default_features(m_cct);
+    m_features = util::get_rbd_default_features(m_cct);
     m_negotiate_features = true;
   }
 

src/test/librbd/test_librbd.cc

@@ -35,6 +35,7 @@
 #include <iostream>
 #include <algorithm>
 #include <sstream>
+#include <list>
 #include <set>
 #include <vector>
 
@@ -5046,3 +5047,26 @@ TEST_F(TestLibRBD, DiscardAfterWrite)
   read_comp->release();
 }
 
+TEST_F(TestLibRBD, DefaultFeatures) {
+  std::string orig_default_features;
+  ASSERT_EQ(0, _rados.conf_get("rbd_default_features", orig_default_features));
+  BOOST_SCOPE_EXIT_ALL(orig_default_features) {
+    ASSERT_EQ(0, _rados.conf_set("rbd_default_features",
+                                 orig_default_features.c_str()));
+  };
+
+  std::list<std::pair<std::string, std::string> > feature_names_to_bitmask = {
+    {"", orig_default_features},
+    {"layering", "1"},
+    {"layering, exclusive-lock", "5"},
+    {"exclusive-lock,journaling", "68"},
+    {"125", "125"}
+  };
+
+  for (auto &pair : feature_names_to_bitmask) {
+    ASSERT_EQ(0, _rados.conf_set("rbd_default_features", pair.first.c_str()));
+    std::string features;
+    ASSERT_EQ(0, _rados.conf_get("rbd_default_features", features));
+    ASSERT_EQ(pair.second, features);
+  }
+}

src/test/rbd_mirror/test_ImageReplayer.cc

@@ -101,7 +101,7 @@ public:
 					       m_remote_ioctx));
 
     m_image_name = get_temp_image_name();
-    uint64_t features = librbd::util::parse_rbd_default_features(g_ceph_context);
+    uint64_t features = librbd::util::get_rbd_default_features(g_ceph_context);
     features |= RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
     int order = 0;
     EXPECT_EQ(0, librbd::create(m_remote_ioctx, m_image_name.c_str(), 1 << 22,

src/test/rbd_mirror/test_PoolWatcher.cc

@@ -86,7 +86,7 @@ TestPoolWatcher() : m_lock("TestPoolWatcherLock"),
 
   void create_image(const string &pool_name, bool mirrored=true,
 		    string *image_name=nullptr) {
-    uint64_t features = librbd::util::parse_rbd_default_features(g_ceph_context);
+    uint64_t features = librbd::util::get_rbd_default_features(g_ceph_context);
     string name = "image" + stringify(++m_image_number);
     if (mirrored) {
       features |= RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;
@@ -135,7 +135,7 @@ TestPoolWatcher() : m_lock("TestPoolWatcherLock"),
       ictx->state->close();
     }
 
-    uint64_t features = librbd::util::parse_rbd_default_features(g_ceph_context);
+    uint64_t features = librbd::util::get_rbd_default_features(g_ceph_context);
     string name = "clone" + stringify(++m_image_number);
     if (mirrored) {
       features |= RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_JOURNALING;

src/tools/rbd/ArgumentTypes.cc

@@ -343,7 +343,7 @@ std::string get_short_features_help(bool append_suffix) {
 
     std::string suffix;
     if (append_suffix) {
-      if ((pair.first & rbd::utils::parse_rbd_default_features(g_ceph_context)) != 0) {
+      if ((pair.first & rbd::utils::get_rbd_default_features(g_ceph_context)) != 0) {
         suffix += "+";
       }
       if ((pair.first & RBD_FEATURES_MUTABLE) != 0) {

src/tools/rbd/Utils.cc

@@ -543,7 +543,7 @@ int get_image_options(const boost::program_options::variables_map &vm,
     features = vm[at::IMAGE_FEATURES].as<uint64_t>();
     features_specified = true;
   } else {
-    features = parse_rbd_default_features(g_ceph_context);
+    features = get_rbd_default_features(g_ceph_context);
   }
 
   if (vm.count(at::IMAGE_STRIPE_UNIT)) {
@@ -874,40 +874,9 @@ std::string timestr(time_t t) {
   return buf;
 }
 
-// FIXME (asheplyakov): use function from librbd/Utils.cc
+uint64_t get_rbd_default_features(CephContext* cct) {
-
-uint64_t parse_rbd_default_features(CephContext* cct) 
-{
-  int ret = 0;
-  uint64_t value = 0;
   auto features = cct->_conf->get_val<std::string>("rbd_default_features");
-  try {
+  return boost::lexical_cast<uint64_t>(features);
-    value = boost::lexical_cast<decltype(value)>(features);
-  } catch (const boost::bad_lexical_cast& ) {
-    map<std::string, int> conf_vals = {{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},
-    };
-    std::vector<std::string> strs;
-    boost::split(strs, features, boost::is_any_of(","));
-    for (auto feature: strs) {
-      boost::trim(feature);
-      if (conf_vals.find(feature) != conf_vals.end()) {
-        value += conf_vals[feature];
-      } else {
-        ret = -EINVAL;
-        std::cerr << "Warning: unknown rbd feature " << feature << std::endl;
-      }
-    }
-    if (value == 0 && ret == -EINVAL)
-      value = RBD_FEATURES_DEFAULT;
-  }
-  return value;
 }
 
 } // namespace utils

src/tools/rbd/Utils.h

@@ -135,7 +135,7 @@ std::string mirror_image_status_state(librbd::mirror_image_status_t status);
 std::string timestr(time_t t);
 
 // duplicate here to not include librbd_internal lib
-uint64_t parse_rbd_default_features(CephContext* cct);
+uint64_t get_rbd_default_features(CephContext* cct);
 
 } // namespace utils
 } // namespace rbd