From: xie xingguo <xie.xingguo@zte.com.cn>
Date: Thu, 21 Apr 2016 03:50:05 +0000 (+0800)
Subject: crush: fix mishandle result of get_bucket() method
X-Git-Tag: v11.0.0~844^2~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b0b5392d1a403cb482e9632e0f773924e337dcf6;p=ceph.git

crush: fix mishandle result of get_bucket() method

Get_bucket() is supposed to return a pointer to a specific bucket on success.
However, it never returns a null pointer if error occurs. It returns
-ENOENT instead. That's why most caller is misjudge the result code of get_bucket()
method.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
---

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 3450b48fb8ff..dc6280ee8507 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -202,7 +202,7 @@ bool CrushWrapper::subtree_contains(int root, int item) const
     return false;  // root is a leaf
 
   const crush_bucket *b = get_bucket(root);
-  if (!b)
+  if (IS_ERR(b))
     return false;
 
   for (unsigned j=0; j<b->size; j++) {
@@ -243,7 +243,12 @@ int CrushWrapper::remove_item(CephContext *cct, int item, bool unlink_only)
 
   if (item < 0 && !unlink_only) {
     crush_bucket *t = get_bucket(item);
-    if (t && t->size) {
+    if (IS_ERR(t)) {
+      ldout(cct, 1) << "remove_item bucket " << item << " does not exist" << dendl;
+      return -ENOENT;
+    }
+
+    if (t->size) {
       ldout(cct, 1) << "remove_item bucket " << item << " has " << t->size
 		    << " items, not empty" << dendl;
       return -ENOTEMPTY;
@@ -352,7 +357,13 @@ int CrushWrapper::remove_item_under(CephContext *cct, int item, int ancestor, bo
 
   if (item < 0 && !unlink_only) {
     crush_bucket *t = get_bucket(item);
-    if (t && t->size) {
+    if (IS_ERR(t)) {
+      ldout(cct, 1) << "remove_item_under bucket " << item
+                    << " does not exist" << dendl;
+      return -ENOENT;
+    }
+
+    if (t->size) {
       ldout(cct, 1) << "remove_item_under bucket " << item << " has " << t->size
 		    << " items, not empty" << dendl;
       return -ENOTEMPTY;
@@ -555,7 +566,7 @@ int CrushWrapper::get_children(int id, list<int> *children)
   }
 
   crush_bucket *b = get_bucket(id);
-  if (!b) {
+  if (IS_ERR(b)) {
     return -ENOENT;
   }