crush: safe check for 'ceph osd crush swap-bucket'

The swap method is just swap the two buckets'weight,son and name.So there is a problem when we swap parent and children. When swap two buckets,one should not be ancestor of the other,or there will be a loop,which would destory the structure.

Signed-off-by: Carudy <zhaoyang.han@easystack.cn>

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 8f2d63e77c8cc3d637f8e481c3f3fd14386eb58a..edf2567537b9b429537377efb8b56e3378adfa60 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -1035,6 +1035,18 @@ int CrushWrapper::detach_bucket(CephContext *cct, int item)
   return bucket_weight;
 }
 
+bool CrushWrapper::is_parent_of(int child, int p) const
+{
+  int parent = 0;
+  while (!get_immediate_parent_id(child, &parent)) {
+    if (parent == p) {
+      return true;
+    }
+    child = parent;
+  }
+  return false;
+}
+
 int CrushWrapper::swap_bucket(CephContext *cct, int src, int dst)
 {
   if (src >= 0 || dst >= 0)
@@ -1043,6 +1055,9 @@ int CrushWrapper::swap_bucket(CephContext *cct, int src, int dst)
     return -EINVAL;
   crush_bucket *a = get_bucket(src);
   crush_bucket *b = get_bucket(dst);
+  if (is_parent_of(a->id, b->id) || is_parent_of(b->id, a->id)) {
+    return -EINVAL;
+  }
   unsigned aw = a->weight;
   unsigned bw = b->weight;
 

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 737ba583ececebcd90dd8ae99ddfef6bd8423d43..27d5c20bc916fd61d418294aee235e705c5de13b 100644 (file)
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -627,6 +627,7 @@ private:
    * @return true if present
    */
   bool _search_item_exists(int i) const;
+  bool is_parent_of(int child, int p) const;
 public:
 
   /**

diff --git a/src/test/crush/CrushWrapper.cc b/src/test/crush/CrushWrapper.cc
index 4502d0d0e69b379984f75437651bbeb8fbf0555d..efd70889b6fc407516841a9d95917692da888aac 100644 (file)
--- a/src/test/crush/CrushWrapper.cc
+++ b/src/test/crush/CrushWrapper.cc
@@ -174,6 +174,9 @@ TEST(CrushWrapper, swap_bucket) {
   ASSERT_EQ(1, c->get_bucket_item(a, 1));
   ASSERT_EQ(2, c->get_bucket_item(a, 2));
   ASSERT_EQ(3, c->get_bucket_item(b, 0));
+       
+  // check if it can swap parent with child
+  ASSERT_EQ(-EINVAL, c->swap_bucket(g_ceph_context, root, a));
 
   c->swap_bucket(g_ceph_context, a, b);
   ASSERT_EQ(0x30000, c->get_item_weight(b));