From: Carudy <zhaoyang.han@easystack.cn>
Date: Wed, 30 Aug 2017 02:06:54 +0000 (+0800)
Subject: crush: safe check for 'ceph osd crush swap-bucket'
X-Git-Tag: v13.0.1~777^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F17335%2Fhead;p=ceph.git

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 8f2d63e77c8c..edf2567537b9 100644
--- 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 737ba583ecec..27d5c20bc916 100644
--- 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 4502d0d0e69b..efd70889b6fc 100644
--- 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));