From 309c74d847f88f906b42bbd12af6c79cc59e645b Mon Sep 17 00:00:00 2001 From: Xiaoxi Chen Date: Wed, 20 Aug 2014 15:35:44 +0800 Subject: [PATCH] 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 (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 --- src/crush/CrushWrapper.cc | 22 ++++++++++++++-------- src/crush/CrushWrapper.h | 6 +++--- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index b03fa1723d7..9d8de2dc321 100644 --- 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 17f61e2cf56..59076dc25e3 100644 --- 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; -- 2.47.3