From d734600f9251b52f525faa35441e2b5dd660161b Mon Sep 17 00:00:00 2001 From: Xiaoxi Chen Date: Fri, 5 Sep 2014 10:56:36 +0800 Subject: [PATCH] Change CrushWrapper::crush to private Currently in CrushWrapper, the member "struct crush_map *crush" is a public member, so people can break the encapsulation and manipulate directly to the crush structure. This is not a good practice for encapsulation and will lead to inconsistent if code mix use the CrushWrapper API and crush C API.A simple example could be: 1.some code use crush_add_rule(C-API) to add a rule, which will not set the have_rmap flag to false in CrushWrapper 2.another code using CrushWrapper trying to look up the newly added rule by name will get a -ENOENT. This patch move CrushWrapper::crush to private, together with three reverse map(type_rmap, name_rmap, rule_name_rmap) and also change codes accessing the CrushWrapper::crush to make it compile. Signed-off-by: Xiaoxi Chen --- src/crush/CrushWrapper.h | 5 ++--- src/test/crush/indep.cc | 33 ++++++++++++++++++--------------- src/tools/crushtool.cc | 4 +--- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h index 56e3d16a0539..2d1fa9559a3f 100644 --- a/src/crush/CrushWrapper.h +++ b/src/crush/CrushWrapper.h @@ -52,16 +52,15 @@ using namespace std; class CrushWrapper { mutable Mutex mapper_lock; public: - struct crush_map *crush; std::map type_map; /* bucket/device type names */ std::map name_map; /* bucket/device names */ std::map rule_name_map; +private: + struct crush_map *crush; /* reverse maps */ bool have_rmaps; std::map type_rmap, name_rmap, rule_name_rmap; - -private: void build_rmaps() { if (have_rmaps) return; build_rmap(type_map, type_rmap); diff --git a/src/test/crush/indep.cc b/src/test/crush/indep.cc index 896e58fb1290..dd0b54272dbd 100644 --- a/src/test/crush/indep.cc +++ b/src/test/crush/indep.cc @@ -51,18 +51,21 @@ CrushWrapper *build_indep_map(CephContext *cct, int num_rack, int num_host, } } } - - crush_rule *rule = crush_make_rule(4, 0, 123, 1, 20); - assert(rule); - crush_rule_set_step(rule, 0, CRUSH_RULE_SET_CHOOSELEAF_TRIES, 10, 0); - crush_rule_set_step(rule, 1, CRUSH_RULE_TAKE, rootno, 0); - crush_rule_set_step(rule, 2, - CRUSH_RULE_CHOOSELEAF_INDEP, - CRUSH_CHOOSE_N, - 1); - crush_rule_set_step(rule, 3, CRUSH_RULE_EMIT, 0, 0); - int rno = crush_add_rule(c->crush, rule, -1); - c->set_rule_name(rno, "data"); + int ret; + int ruleno = 0; + int ruleset = 0; + ruleno = ruleset; + ret = c->add_rule(4, ruleset, 123, 1, 20, ruleno); + assert(ret == ruleno); + ret = c->set_rule_step(ruleno, 0, CRUSH_RULE_SET_CHOOSELEAF_TRIES, 10, 0); + assert(ret == 0); + ret = c->set_rule_step(ruleno, 1, CRUSH_RULE_TAKE, rootno, 0); + assert(ret == 0); + ret = c->set_rule_step(ruleno, 2, CRUSH_RULE_CHOOSELEAF_INDEP, CRUSH_CHOOSE_N, 1); + assert(ret == 0); + ret = c->set_rule_step(ruleno, 3, CRUSH_RULE_EMIT, 0, 0); + assert(ret == 0); + c->set_rule_name(ruleno, "data"); if (false) { Formatter *f = new_formatter("json-pretty"); @@ -140,7 +143,7 @@ TEST(CRUSH, indep_out_alt) { c->dump_tree(weight, &cout, NULL); // need more retries to get 9/9 hosts for x in 0..99 - c->crush->choose_total_tries = 100; + c->set_choose_total_tries(100); for (int x = 0; x < 100; ++x) { vector out; c->do_rule(0, x, out, 9, weight); @@ -166,7 +169,7 @@ TEST(CRUSH, indep_out_contig) { weight[i] = 0; c->dump_tree(weight, &cout, NULL); - c->crush->choose_total_tries = 100; + c->set_choose_total_tries(100); for (int x = 0; x < 100; ++x) { vector out; c->do_rule(0, x, out, 7, weight); @@ -185,7 +188,7 @@ TEST(CRUSH, indep_out_contig) { TEST(CRUSH, indep_out_progressive) { CrushWrapper *c = build_indep_map(g_ceph_context, 3, 3, 3); - c->crush->choose_total_tries = 100; + c->set_choose_total_tries(100); vector<__u32> tweight(c->get_max_devices(), 0x10000); c->dump_tree(tweight, &cout, NULL); diff --git a/src/tools/crushtool.cc b/src/tools/crushtool.cc index 08818c477cf4..ccb0efda74a7 100644 --- a/src/tools/crushtool.cc +++ b/src/tools/crushtool.cc @@ -581,10 +581,8 @@ int main(int argc, const char **argv) dout(2) << " item " << items[j] << " weight " << weights[j] << dendl; } - crush_bucket *b = crush_make_bucket(buckettype, CRUSH_HASH_DEFAULT, type, j, items, weights); - assert(b); int id; - int r = crush_add_bucket(crush.crush, 0, b, &id); + int r = crush.add_bucket(0, buckettype, CRUSH_HASH_DEFAULT, type, j, items, weights, &id); if (r < 0) { dout(2) << "Couldn't add bucket: " << cpp_strerror(r) << dendl; } -- 2.47.3