crush: reset bucket->h.items[i] when removing tree item

* crush: so we don't see the reference after the removing, this keeps
  check_item_loc() happy, and move_bucket() use check_item_loc() to see if
  the removed bucket disappears after the removal.
* test: also add unittest_crush_wrapper::CrushWrapper.insert_item

Fixes: http://tracker.ceph.com/issues/16525
Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit a7069c7aab6b3f605f3d8f909af96f87977e8698)

Backport change: modify some codes that only works on c++ 11

diff --git a/src/crush/builder.c b/src/crush/builder.c
index 28d957db21f03d93c4c804f5fbd03cd59cad6718..4d8ccf5eafaf3cd058d99a73358d8b7d131341f0 100644 (file)
--- a/src/crush/builder.c
+++ b/src/crush/builder.c
@@ -1015,7 +1015,8 @@ int crush_remove_tree_bucket_item(struct crush_bucket_tree *bucket, int item)
 
                if (bucket->h.items[i] != item)
                        continue;
-               
+
+               bucket->h.items[i] = 0;
                node = crush_calc_tree_node(i);
                weight = bucket->node_weights[node];
                bucket->node_weights[node] = 0;

diff --git a/src/test/crush/CrushWrapper.cc b/src/test/crush/CrushWrapper.cc
index ddfa0f03e4209849373d8004ab0617ccbe71cfa7..7c92a1f5a8acaa6471162b87eebb606e2ea2ca70 100644 (file)
--- a/src/test/crush/CrushWrapper.cc
+++ b/src/test/crush/CrushWrapper.cc
@@ -708,6 +708,49 @@ TEST(CrushWrapper, insert_item) {
   delete c;
 }
 
+TEST(CrushWrapper, remove_item) {
+  CrushWrapper *c = new CrushWrapper;
+
+  const int ROOT_TYPE = 2;
+  c->set_type_name(ROOT_TYPE, "root");
+  const int HOST_TYPE = 1;
+  c->set_type_name(HOST_TYPE, "host");
+  const int OSD_TYPE = 0;
+  c->set_type_name(OSD_TYPE, "osd");
+
+  {
+    int root;
+    ASSERT_EQ(0, c->add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_RJENKINS1,
+                              ROOT_TYPE, 0, NULL, NULL, &root));
+    c->set_item_name(root, "root0");
+  }
+
+  {
+    int host;
+    c->add_bucket(0, CRUSH_BUCKET_TREE, CRUSH_HASH_RJENKINS1,
+                 HOST_TYPE, 0, NULL, NULL, &host);
+    c->set_item_name(host, "host0");
+  }
+
+  const int num_osd = 12;
+  {
+    map<string, string> loc;
+    loc["root"] = "root0";
+    loc["host"] = "host0";
+
+    for (int item = 0; item < num_osd; item++) {
+      ASSERT_EQ(0, c->insert_item(g_ceph_context, item, 1.0,
+                                 "osd." + stringify(item), loc));
+    }
+  }
+  const int item_to_remove = num_osd / 2;
+  map<string, string> loc;
+  loc.insert(c->get_immediate_parent(item_to_remove));
+  ASSERT_EQ(0, c->remove_item(g_ceph_context, item_to_remove, true));
+  float weight;
+  EXPECT_FALSE(c->check_item_loc(g_ceph_context, item_to_remove, loc, &weight));
+}
+
 TEST(CrushWrapper, item_bucket_names) {
   CrushWrapper *c = new CrushWrapper;
   int index = 123;