mon: Fix default replicated pool ruleset choice

Specifically, in the case where the configured
default ruleset is CEPH_DEFAULT_CRUSH_REPLICATED_RULESET,
instead of assuming ruleset 0 exists, choose the lowest
numbered ruleset.

In the case where an explicit ruleset is passed to
OSDMonitor::prepare_pool_crush_ruleset, verify
that it really exists.

The idea is to eliminate cases where a pool could
exist with its crush ruleset set to something
other than a value ruleset ID.

Fixes: #8373
Signed-off-by: John Spray <john.spray@inktank.com>

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index c0eabe37f51aa149bce0b11f34af2ac4655ed294..52730a1d3903fd1ca5dbda650614ac65d1b1d1bd 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -1,4 +1,5 @@
 
+#include "osd/osd_types.h"
 #include "common/debug.h"
 #include "common/Formatter.h"
 #include "common/errno.h"
@@ -1374,6 +1375,12 @@ void CrushWrapper::generate_test_instances(list<CrushWrapper*>& o)
   // fixme
 }
 
+/**
+ * Determine the default CRUSH ruleset ID to be used with
+ * newly created replicated pools.
+ *
+ * @returns a ruleset ID (>=0) or an error (<0)
+ */
 int CrushWrapper::get_osd_pool_default_crush_replicated_ruleset(CephContext *cct)
 {
   int crush_ruleset = cct->_conf->osd_pool_default_crush_replicated_ruleset;
@@ -1388,6 +1395,11 @@ int CrushWrapper::get_osd_pool_default_crush_replicated_ruleset(CephContext *cct
                   << dendl;
     crush_ruleset = cct->_conf->osd_pool_default_crush_rule;
   }
+
+  if (crush_ruleset == CEPH_DEFAULT_CRUSH_REPLICATED_RULESET) {
+    crush_ruleset = find_first_ruleset(pg_pool_t::TYPE_REPLICATED);
+  }
+
   return crush_ruleset;
 }
 

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 3b2e6e6979ac3cc00201fd5b71984c5d2cfbf1d7..0a633a51f5cba4fc51bf1c21460873c7a95fdca7 100644 (file)
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -863,6 +863,36 @@ public:
     if (!crush) return -1;
     return crush_find_rule(crush, ruleset, type, size);
   }
+
+  bool ruleset_exists(int const ruleset) const {
+    for (size_t i = 0; i < crush->max_rules; ++i) {
+     if (crush->rules[i]->mask.ruleset == ruleset) {
+       return true;
+     }
+    }
+
+    return false;
+  }
+
+  /**
+   * Return the lowest numbered ruleset of type `type`
+   *
+   * @returns a ruleset ID, or -1 if no matching rulesets found.
+   */
+  int find_first_ruleset(int type) const {
+    int result = -1;
+
+    for (size_t i = 0; i < crush->max_rules; ++i) {
+      if (crush->rules[i]
+          && crush->rules[i]->mask.type == type
+          && (crush->rules[i]->mask.ruleset < result || result == -1)) {
+        result = crush->rules[i]->mask.ruleset;
+      }
+    }
+
+    return result;
+  }
+
   void do_rule(int rule, int x, vector<int>& out, int maxout,
               const vector<__u32>& weight) const {
     Mutex::Locker l(mapper_lock);
@@ -902,7 +932,7 @@ public:
   void dump_tree(const vector<__u32>& w, ostream *out, Formatter *f) const;
   static void generate_test_instances(list<CrushWrapper*>& o);
 
-  static int get_osd_pool_default_crush_replicated_ruleset(CephContext *cct);
+  int get_osd_pool_default_crush_replicated_ruleset(CephContext *cct);
 
   static bool is_valid_crush_name(const string& s);
   static bool is_valid_crush_loc(CephContext *cct,

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 63b5de88f0e7b09107c66ea2d235028747b2306b..778ec405e1564dad3ec75e8bf95b377d4e3c73a1 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -3129,8 +3129,12 @@ int OSDMonitor::prepare_pool_crush_ruleset(const unsigned pool_type,
   if (*crush_ruleset < 0) {
     switch (pool_type) {
     case pg_pool_t::TYPE_REPLICATED:
-      *crush_ruleset =
-       CrushWrapper::get_osd_pool_default_crush_replicated_ruleset(g_ceph_context);
+      *crush_ruleset = osdmap.crush->get_osd_pool_default_crush_replicated_ruleset(g_ceph_context);
+      if (*crush_ruleset < 0) {
+        // Errors may happen e.g. if no valid ruleset is available
+        ss << "No suitable CRUSH ruleset exists";
+        return *crush_ruleset;
+      }
       break;
     case pg_pool_t::TYPE_ERASURE:
       {
@@ -3158,6 +3162,11 @@ int OSDMonitor::prepare_pool_crush_ruleset(const unsigned pool_type,
       return -EINVAL;
       break;
     }
+  } else {
+    if (!osdmap.crush->ruleset_exists(*crush_ruleset)) {
+      ss << "CRUSH ruleset " << *crush_ruleset << " not found";
+      return -ENOENT;
+    }
   }
 
   return 0;

diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc
index 24404626b09d25bbec29503ed5a1a66886092757..88d8d14735ad5c8f7061026808682f07f06ce991 100644 (file)
--- a/src/osd/OSDMap.cc
+++ b/src/osd/OSDMap.cc
@@ -2509,8 +2509,18 @@ int OSDMap::build_simple(CephContext *cct, epoch_t e, uuid_d &fsid,
   pool_names.push_back("metadata");
   pool_names.push_back("rbd");
 
+  stringstream ss;
+  int r;
+  if (nosd >= 0)
+    r = build_simple_crush_map(cct, *crush, nosd, &ss);
+  else
+    r = build_simple_crush_map_from_conf(cct, *crush, &ss);
+
   int poolbase = get_max_osd() ? get_max_osd() : 1;
 
+  int const default_replicated_ruleset = crush->get_osd_pool_default_crush_replicated_ruleset(cct);
+  assert(default_replicated_ruleset >= 0);
+
   for (vector<string>::iterator p = pool_names.begin();
        p != pool_names.end(); ++p) {
     int64_t pool = ++pool_max;
@@ -2520,8 +2530,7 @@ int OSDMap::build_simple(CephContext *cct, epoch_t e, uuid_d &fsid,
       pools[pool].flags |= pg_pool_t::FLAG_HASHPSPOOL;
     pools[pool].size = cct->_conf->osd_pool_default_size;
     pools[pool].min_size = cct->_conf->get_osd_pool_default_min_size();
-    pools[pool].crush_ruleset =
-      CrushWrapper::get_osd_pool_default_crush_replicated_ruleset(cct);
+    pools[pool].crush_ruleset = default_replicated_ruleset;
     pools[pool].object_hash = CEPH_STR_HASH_RJENKINS;
     pools[pool].set_pg_num(poolbase << pg_bits);
     pools[pool].set_pgp_num(poolbase << pgp_bits);
@@ -2532,13 +2541,6 @@ int OSDMap::build_simple(CephContext *cct, epoch_t e, uuid_d &fsid,
     name_pool[*p] = pool;
   }
 
-  stringstream ss;
-  int r;
-  if (nosd >= 0)
-    r = build_simple_crush_map(cct, *crush, nosd, &ss);
-  else
-    r = build_simple_crush_map_from_conf(cct, *crush, &ss);
-
   if (r < 0)
     lderr(cct) << ss.str() << dendl;
   

diff --git a/src/test/cli/osdmaptool/create-print.t b/src/test/cli/osdmaptool/create-print.t
index c1c3a6d88c07c70d27a1e8a822bb69ef082aeed0..9ebd27470b8a0690141966df2e22825c04a46e2a 100644 (file)
--- a/src/test/cli/osdmaptool/create-print.t
+++ b/src/test/cli/osdmaptool/create-print.t
@@ -91,10 +91,6 @@
   osdmaptool: osdmap file 'myosdmap'
   default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
   default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 0
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 0
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 0
   $ osdmaptool --print myosdmap | grep 'pool 0'
   osdmaptool: osdmap file 'myosdmap'
   pool 0 'data' replicated size 3 min_size 2 crush_ruleset 55 object_hash rjenkins pg_num 192 pgp_num 192 last_change 0 flags hashpspool crash_replay_interval 45 stripe_width 0
@@ -102,10 +98,6 @@
   osdmaptool: osdmap file 'myosdmap'
   default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
   default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 66
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 66
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 66
   $ osdmaptool --print myosdmap | grep 'pool 0'
   osdmaptool: osdmap file 'myosdmap'
   pool 0 'data' replicated size 3 min_size 2 crush_ruleset 55 object_hash rjenkins pg_num 192 pgp_num 192 last_change 0 flags hashpspool crash_replay_interval 45 stripe_width 0

diff --git a/src/test/cli/osdmaptool/create-racks.t b/src/test/cli/osdmaptool/create-racks.t
index a6cbac244b747fff0170e282e84da6857d5215f1..33fa9eefa992c114b6d117d596dbea82293d9efb 100644 (file)
--- a/src/test/cli/osdmaptool/create-racks.t
+++ b/src/test/cli/osdmaptool/create-racks.t
@@ -805,10 +805,6 @@
   osdmaptool: osdmap file 'om'
   default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
   default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 0
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 0
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 0
   $ osdmaptool --print om | grep 'pool 0'
   osdmaptool: osdmap file 'om'
   pool 0 'data' replicated size 3 min_size 2 crush_ruleset 55 object_hash rjenkins pg_num 15296 pgp_num 15296 last_change 0 flags hashpspool crash_replay_interval 45 stripe_width 0
@@ -816,10 +812,6 @@
   osdmaptool: osdmap file 'om'
   default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
   default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 66
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 66
-  default_crush_rule is deprecated use osd_pool_default_crush_replicated_ruleset instead
-  default_crush_rule = 55 overrides osd_pool_default_crush_replicated_ruleset = 66
   $ osdmaptool --print om | grep 'pool 0'
   osdmaptool: osdmap file 'om'
   pool 0 'data' replicated size 3 min_size 2 crush_ruleset 55 object_hash rjenkins pg_num 15296 pgp_num 15296 last_change 0 flags hashpspool crash_replay_interval 45 stripe_width 0