CrushWrapper: pick a ruleset same as rule_id

Originally in the add_simple_ruleset funtion, the ruleset_id
is not reused but rule_id is reused. So after some add/remove
against rules, the newly created rule likely to have
ruleset!=rule_id.

We dont want this happen because we are trying to hold the constraint
that ruleset == rule_id.

Signed-off-by: Xiaoxi Chen <xiaoxi.chen@intel.com>
(cherry picked from commit 78e84f34da83abf5a62ae97bb84ab70774b164a6)

Conflicts:
	src/crush/CrushWrapper.cc
	src/crush/crush.h
	src/test/erasure-code/TestErasureCodeIsa.cc
	src/test/erasure-code/TestErasureCodeJerasure.cc
	src/test/mon/osd-crush.sh

Fixes: #9675

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index b03fa1723d71e7400eda4f8cadd5662d5a156b40..9d8de2dc321f2d7f3b1e3fc32b6f5cb59ad01e35 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -1,6 +1,7 @@
 
 #include "common/debug.h"
 #include "common/Formatter.h"
+#include "common/errno.h"
 
 #include "CrushWrapper.h"
 
@@ -657,15 +658,14 @@ int CrushWrapper::add_simple_rule(string name, string root_name, string failure_
     }
   }
 
-  int ruleset = 0;
-  for (int i = 0; i < get_max_rules(); i++) {
-    if (rule_exists(i) &&
-       get_rule_mask_ruleset(i) >= ruleset) {
-      ruleset = get_rule_mask_ruleset(i) + 1;
-    }
+  int rno = -1;
+  for (rno = 0; rno < get_max_rules(); rno++) {
+    if (!rule_exists(rno) && !ruleset_exists(rno))
+       break;
   }
 
-  crush_rule *rule = crush_make_rule(3, ruleset, 1 /* pg_pool_t::TYPE_REP */, 1, 10);
+  //set the ruleset the same as rule_id(rno)
+  crush_rule *rule = crush_make_rule(3, rno, 1 /* pg_pool_t::TYPE_REP */, 1, 10);
   assert(rule);
   crush_rule_set_step(rule, 0, CRUSH_RULE_TAKE, root, 0);
   if (type)
@@ -679,7 +679,13 @@ int CrushWrapper::add_simple_rule(string name, string root_name, string failure_
                        CRUSH_CHOOSE_N,
                        0);
   crush_rule_set_step(rule, 2, CRUSH_RULE_EMIT, 0, 0);
-  int rno = crush_add_rule(crush, rule, -1);
+  int ret = crush_add_rule(crush, rule, rno);
+  if (ret < 0) {
+    if (err) {
+      *err << "failed to add rule " << rno << " because " << cpp_strerror(ret);
+    }
+    return ret;
+  }
   set_rule_name(rno, name);
   have_rmaps = false;
   return rno;

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 17f61e2cf562215783b12092780d70417188010c..59076dc25e36a0d242a39ad38d00755e05787316 100644 (file)
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -763,9 +763,9 @@ public:
 
   bool ruleset_exists(int ruleset) const {
     for (size_t i = 0; i < crush->max_rules; ++i) {
-     if (crush->rules[i]->mask.ruleset == ruleset) {
-       return true;
-     }
+      if (rule_exists(i) && crush->rules[i]->mask.ruleset == ruleset) {
+       return true;
+      }
     }
 
     return false;