librbd: simplify filtering of config key overrides

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 12e2f99dcb3771d7c807b751d6de49a86590c3f3..1777456319ce489404b124c2eb12ed55ae5e01b9 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -35,6 +35,7 @@
 
 #include "osdc/Striper.h"
 #include <boost/bind.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -742,79 +743,9 @@ public:
     completed_reqs.clear();
   }
 
-  bool ImageCtx::_filter_metadata_confs(const string &prefix,
-                                        map<string, bool> &configs,
-                                        const map<string, bufferlist> &pairs,
-                                        map<string, bufferlist> *res) {
-    size_t conf_prefix_len = prefix.size();
-
-    for (auto it : pairs) {
-      if (it.first.compare(0, std::min(conf_prefix_len, it.first.size()), prefix) > 0)
-        return false;
-
-      if (it.first.size() <= conf_prefix_len)
-        continue;
-
-      string key = it.first.substr(conf_prefix_len, it.first.size() - conf_prefix_len);
-      auto cit = configs.find(key);
-      if (cit != configs.end()) {
-        cit->second = true;
-        res->insert(make_pair(key, it.second));
-      }
-    }
-    return true;
-  }
-
   void ImageCtx::apply_metadata(const std::map<std::string, bufferlist> &meta,
                                 bool thread_safe) {
     ldout(cct, 20) << __func__ << dendl;
-    std::map<string, bool> configs = boost::assign::map_list_of(
-        "rbd_non_blocking_aio", false)(
-        "rbd_cache", false)(
-        "rbd_cache_writethrough_until_flush", false)(
-        "rbd_cache_size", false)(
-        "rbd_cache_max_dirty", false)(
-        "rbd_cache_target_dirty", false)(
-        "rbd_cache_max_dirty_age", false)(
-        "rbd_cache_max_dirty_object", false)(
-        "rbd_cache_block_writes_upfront", false)(
-        "rbd_concurrent_management_ops", false)(
-        "rbd_balance_snap_reads", false)(
-        "rbd_localize_snap_reads", false)(
-        "rbd_balance_parent_reads", false)(
-        "rbd_localize_parent_reads", false)(
-        "rbd_sparse_read_threshold_bytes", false)(
-        "rbd_readahead_trigger_requests", false)(
-        "rbd_readahead_max_bytes", false)(
-        "rbd_readahead_disable_after_bytes", false)(
-        "rbd_clone_copy_on_read", false)(
-        "rbd_blacklist_on_break_lock", false)(
-        "rbd_blacklist_expire_seconds", false)(
-        "rbd_request_timed_out_seconds", false)(
-        "rbd_journal_order", false)(
-        "rbd_journal_splay_width", false)(
-        "rbd_journal_commit_age", false)(
-        "rbd_journal_object_flush_interval", false)(
-        "rbd_journal_object_flush_bytes", false)(
-        "rbd_journal_object_flush_age", false)(
-        "rbd_journal_object_max_in_flight_appends", false)(
-        "rbd_journal_pool", false)(
-        "rbd_journal_max_payload_bytes", false)(
-        "rbd_journal_max_concurrent_object_sets", false)(
-        "rbd_mirroring_resync_after_disconnect", false)(
-        "rbd_mirroring_delete_delay", false)(
-        "rbd_mirroring_replay_delay", false)(
-        "rbd_mtime_update_interval", false)(
-        "rbd_atime_update_interval", false)(
-        "rbd_skip_partial_discard", false)(
-       "rbd_qos_iops_limit", false)(
-       "rbd_qos_bps_limit", false)(
-       "rbd_qos_read_iops_limit", false)(
-       "rbd_qos_write_iops_limit", false)(
-       "rbd_qos_read_bps_limit", false)(
-       "rbd_qos_write_bps_limit", false);
-
-    std::map<std::string, bufferlist> res;
 
     // reset settings back to global defaults
     for (auto& key : config_overrides) {
@@ -824,25 +755,41 @@ public:
 
       config.set_val(key, value);
     }
-
     config_overrides.clear();
-    _filter_metadata_confs(METADATA_CONF_PREFIX, configs, meta, &res);
-    for (auto it : res) {
-      std::string val(it.second.c_str(), it.second.length());
-      int j = config.set_val(it.first.c_str(), val);
-      if (j < 0) {
-        lderr(cct) << __func__ << " failed to set config " << it.first
-                   << " with value " << it.second.c_str() << ": " << j
-                   << dendl;
+
+    // extract config overrides
+    for (auto meta_pair : meta) {
+      if (!boost::starts_with(meta_pair.first, METADATA_CONF_PREFIX)) {
+        continue;
+      }
+
+      std::string key = meta_pair.first.substr(METADATA_CONF_PREFIX.size());
+      if (!boost::starts_with(key, "rbd_")) {
+        // ignore non-RBD configuration keys
+        // TODO use option schema to determine applicable subsystem
+        ldout(cct, 0) << __func__ << ": ignoring config " << key << dendl;
+        continue;
+      }
+
+      if (config.find_option(key) != nullptr) {
+        std::string val(meta_pair.second.c_str(), meta_pair.second.length());
+        int r = config.set_val(key, val);
+        if (r >= 0) {
+          ldout(cct, 20) << __func__ << ": " << key << "=" << val << dendl;
+          config_overrides.insert(key);
+        } else {
+          lderr(cct) << __func__ << ": failed to set config " << key << " "
+                     << "with value " << val << ": " << cpp_strerror(r)
+                     << dendl;
+        }
       }
-      config_overrides.insert(it.first);
     }
 
-#define ASSIGN_OPTION(config, type)                                            \
+#define ASSIGN_OPTION(param, type)                                             \
     do {                                                                       \
       string key = "rbd_";                                                    \
-      key = key + #config;                                                    \
-      config = config.get_val<type>("rbd_"#config);                            \
+      key = key + #param;                                                     \
+      param = config.get_val<type>("rbd_"#param);                              \
     } while (0);
 
     ASSIGN_OPTION(non_blocking_aio, bool);

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index 5d6f41c0280512bec50daecc76c19b62f3ce8cb0..911044202ecdc7982e6b08f710d152076fdb7f7d 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -226,9 +226,6 @@ namespace librbd {
 
     ZTracer::Endpoint trace_endpoint;
 
-    static bool _filter_metadata_confs(const string &prefix, std::map<string, bool> &configs,
-                                       const map<string, bufferlist> &pairs, map<string, bufferlist> *res);
-
     // unit test mock helpers
     static ImageCtx* create(const std::string &image_name,
                             const std::string &image_id,

diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 9363b7370363edeb8352f55558d649d257a06139..c7e71681aed0d125a0792e788e6e83a41f84f9e4 100644 (file)
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -515,51 +515,6 @@ TEST_F(TestInternal, Metadata) {
   ASSERT_STREQ(val.c_str(), "value3");
 }
 
-TEST_F(TestInternal, MetadataFilter) {
-  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
-
-  map<string, bool> test_confs = boost::assign::map_list_of(
-    "aaaaaaa", false)(
-    "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", false)(
-    "cccccccccccccc", false);
-  map<string, bool>::iterator it = test_confs.begin();
-  const string prefix = "test_config_";
-  bool is_continue;
-  librbd::ImageCtx *ictx;
-  ASSERT_EQ(0, open_image(m_image_name, &ictx));
-
-  librbd::Image image1;
-  map<string, bufferlist> pairs, res;
-  pairs["abc"].append("value");
-  pairs["abcabc"].append("value");
-  pairs[prefix+it->first].append("value1");
-  ++it;
-  pairs[prefix+it->first].append("value2");
-  ++it;
-  pairs[prefix+it->first].append("value3");
-  pairs[prefix+"asdfsdaf"].append("value6");
-  pairs[prefix+"zxvzxcv123"].append("value5");
-
-  is_continue = ictx->_filter_metadata_confs(prefix, test_confs, pairs, &res);
-  ASSERT_TRUE(is_continue);
-  ASSERT_TRUE(res.size() == 3U);
-  it = test_confs.begin();
-  ASSERT_TRUE(res.count(it->first));
-  ASSERT_TRUE(it->second);
-  ++it;
-  ASSERT_TRUE(res.count(it->first));
-  ASSERT_TRUE(it->second);
-  ++it;
-  ASSERT_TRUE(res.count(it->first));
-  ASSERT_TRUE(it->second);
-  res.clear();
-
-  pairs["zzzzzzzz"].append("value7");
-  is_continue = ictx->_filter_metadata_confs(prefix, test_confs, pairs, &res);
-  ASSERT_FALSE(is_continue);
-  ASSERT_TRUE(res.size() == 3U);
-}
-
 TEST_F(TestInternal, MetadataConfApply) {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);