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 <xiaoxi.chen@intel.com>

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 56e3d16a0539f6cb7321a15b623b9d48ae1c41f4..2d1fa9559a3f436671407057c4291bb1f3b9b4b8 100644 (file)
--- 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<int32_t, string> type_map; /* bucket/device type names */
   std::map<int32_t, string> name_map; /* bucket/device names */
   std::map<int32_t, string> rule_name_map;
 
+private:
+  struct crush_map *crush;
   /* reverse maps */
   bool have_rmaps;
   std::map<string, int> 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 896e58fb12905a4663e7f4602a23c334264d8753..dd0b54272dbd5f5e91677bc8b2e5cc8347c7f7ee 100644 (file)
--- 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<int> 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<int> 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 08818c477cf4b5a484dc8b7b6242701c28d3eb88..ccb0efda74a7c7d57b7158520e95347daa716073 100644 (file)
--- 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;
        }