common/options,librbd/Utils: refactor RBD feature validation

Move conversion of RBD features to/from string/uint64_t to helpers, and
use those from the option validator.

Keep the resulting features in integer form to avoid breaking Cinder.
Meh.

Change behavior from prior version:

- fail to parse if any of the specified features is invalid (previously
  we would parse the ones we understood and ignore the others).

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b454f7d866043861547ca332c3871156713aff25..5e1e949b7bd33504e0bb78e46b9e56dda819bbf7 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -536,6 +536,7 @@ set(libcommon_files
   common/dns_resolve.cc
   common/hostname.cc
   common/util.cc
+  librbd/Features.cc
   arch/probe.cc
   ${auth_files}
   ${mds_files})

diff --git a/src/common/options.cc b/src/common/options.cc
index 505cf277e7fae3df9ade7019dd315b0af7b76657..71d97704c2e7cf6d7900fdb95cd0088c7592c5a4 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -14,6 +14,9 @@
 // Definitions for enums
 #include "common/perf_counters.h"
 
+// rbd feature validation
+#include "librbd/Features.h"
+
 
 void Option::dump_value(const char *field_name,
     const Option::value_t &v, Formatter *f) const
@@ -5817,71 +5820,17 @@ static std::vector<Option> get_rbd_options() {
         "+4 -> exclusive-lock, +8 -> object-map, +16 -> fast-diff, "
         "+32 -> deep-flatten, +64 -> journaling, +128 -> data-pool")
     .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_OPERATIONS << 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();
-        }
-        uint64_t internal_features = (features & RBD_FEATURES_INTERNAL);
-        if (internal_features != 0ULL) {
-          features &= ~RBD_FEATURES_INTERNAL;
-
-          std::stringstream ss;
-          ss << "ignoring internal feature mask 0x"
-             << std::hex << internal_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;
-    }),
+    .set_validator([](std::string *value, std::string *error_message) {
+       ostringstream ss;
+       uint64_t features = librbd::rbd_features_from_string(*value, &ss);
+       // Leave this in integer form to avoid breaking Cinder.  Someday
+       // we would like to present this in string form instead...
+       *value = stringify(features);
+       if (ss.str().size()) {
+         return -EINVAL;
+       }
+       return 0;
+      }),
 
     Option("rbd_op_threads", Option::TYPE_INT, Option::LEVEL_ADVANCED)
     .set_default(1)

diff --git a/src/librbd/Features.cc b/src/librbd/Features.cc
new file mode 100644 (file)
index 0000000..5c8c085
--- /dev/null
+++ b/src/librbd/Features.cc
@@ -0,0 +1,105 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#include <boost/lexical_cast.hpp>
+#include <boost/algorithm/string.hpp>
+
+#include "librbd/Features.h"
+#include "include/rbd/features.h"
+
+#include <map>
+#include <vector>
+
+static const std::map<std::string, uint64_t> RBD_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_OPERATIONS << 1) > RBD_FEATURES_ALL,
+             "new RBD feature added");
+
+
+namespace librbd {
+
+std::string rbd_features_to_string(uint64_t features,
+                                  std::ostream *err)
+{
+  std::string r;
+  for (auto& i : RBD_FEATURE_MAP) {
+    if (features & i.second) {
+      if (r.empty()) {
+       r += ",";
+      }
+      r += i.first;
+      features &= ~i.second;
+    }
+  }
+  if (err && features) {
+    *err << "ignoring unknown feature mask 0x"
+        << std::hex << features << std::dec;
+  }
+  return r;
+}
+
+uint64_t rbd_features_from_string(const std::string& orig_value,
+                                 std::ostream *err)
+{
+  uint64_t features = 0;
+  std::string value = orig_value;
+  boost::trim(value);
+
+  // empty string means default features
+  if (!value.size()) {
+    return RBD_FEATURES_DEFAULT;
+  }
+
+  try {
+    // numeric?
+    features = boost::lexical_cast<uint64_t>(value);
+
+    // drop unrecognized bits
+    uint64_t unsupported_features = (features & ~RBD_FEATURES_ALL);
+    if (unsupported_features != 0ull) {
+      features &= RBD_FEATURES_ALL;
+      if (err) {
+       *err << "ignoring unknown feature mask 0x"
+             << std::hex << unsupported_features << std::dec;
+      }
+    }
+    uint64_t internal_features = (features & RBD_FEATURES_INTERNAL);
+    if (internal_features != 0ULL) {
+      features &= ~RBD_FEATURES_INTERNAL;
+      if (err) {
+       *err << "ignoring internal feature mask 0x"
+            << std::hex << internal_features;
+      }
+    }
+  } catch (boost::bad_lexical_cast&) {
+    // feature name list?
+    bool errors = false;
+    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 = RBD_FEATURE_MAP.find(feature_name);
+      if (feature_it != RBD_FEATURE_MAP.end()) {
+       features += feature_it->second;
+      } else if (err) {
+       if (errors) {
+         *err << ", ";
+       } else {
+         errors = true;
+       }
+       *err << "ignoring unknown feature " << feature_name;
+      }
+    }
+  }
+  return features;
+}
+
+} // namespace librbd

diff --git a/src/librbd/Features.h b/src/librbd/Features.h
new file mode 100644 (file)
index 0000000..6a88827
--- /dev/null
+++ b/src/librbd/Features.h
@@ -0,0 +1,16 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#pragma once
+
+#include <string>
+#include <ostream>
+
+namespace librbd {
+
+  std::string rbd_features_to_string(uint64_t features,
+                                    std::ostream *err);
+  uint64_t rbd_features_from_string(const std::string& value,
+                                   std::ostream *err);
+
+} // librbd

diff --git a/src/librbd/Utils.cc b/src/librbd/Utils.cc
index 6f71841d7ee771ef6e1342ae996a7421a22cc1b7..e92c79907e9daf7fcdf21381429329d89a28972a 100644 (file)
--- a/src/librbd/Utils.cc
+++ b/src/librbd/Utils.cc
@@ -10,6 +10,7 @@
 #include "include/rbd/features.h"
 #include "common/dout.h"
 #include "librbd/ImageCtx.h"
+#include "librbd/Features.h"
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -66,10 +67,11 @@ std::string generate_image_id(librados::IoCtx &ioctx) {
 
 uint64_t get_rbd_default_features(CephContext* cct)
 {
-  auto str_val = cct->_conf->get_val<std::string>("rbd_default_features");
-  return boost::lexical_cast<uint64_t>(str_val);
+  auto value = cct->_conf->get_val<std::string>("rbd_default_features");
+  return librbd::rbd_features_from_string(value, nullptr);
 }
 
+
 bool calc_sparse_extent(const bufferptr &bp,
                         size_t sparse_size,
                         uint64_t length,