crush: fix class_is_in_use()

A class can be considered as in-use only if it is referenced by
any of the existing crush rules.

The patch also makes the output more human readable. For example:

./bin/ceph osd crush rule create-replicated myrule default host ssd
./bin/ceph osd crush class rm ssd
Error EBUSY: class 'ssd' still referenced by crush_rule 'myrule'

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

diff --git a/qa/standalone/crush/crush-classes.sh b/qa/standalone/crush/crush-classes.sh
index b6b38a9a9f03eab8682ed52b6d4476f376504c28..c81a4550dc8e605ff5f29f908eb959e7dc34f963 100755 (executable)
--- a/qa/standalone/crush/crush-classes.sh
+++ b/qa/standalone/crush/crush-classes.sh
@@ -164,6 +164,28 @@ function TEST_mon_classes() {
     fi
 
     ceph osd crush rule create-replicated foo-rule foo host abc || return 1
+
+    # test class_is_in_use
+    ceph osd crush set-device-class hdd osd.0 || return 1
+    ceph osd crush set-device-class ssd osd.1 || return 1
+    ceph osd crush rule create-replicated foo-hdd1 default host hdd || return 1
+    ceph osd crush rule create-replicated foo-hdd2 default host hdd || return 1
+    ceph osd crush rule create-replicated foo-ssd default host ssd || return 1
+    expect_failure $dir EBUSY ceph osd crush class rm hdd || return 1
+    expect_failure $dir EBUSY ceph osd crush class rm ssd || return 1
+    expect_failure $dir EBUSY ceph osd crush class rename hdd asdf || return 1
+    expect_failure $dir EBUSY ceph osd crush class rename ssd qwer || return 1
+    ceph osd crush rule rm foo-hdd1 || return 1
+    expect_failure $dir EBUSY ceph osd crush class rm hdd || return 1 # still referenced by foo-hdd2
+    ceph osd crush rule rm foo-hdd2 || return 1
+    ceph osd crush rule rm foo-ssd || return 1
+    ceph osd crush class rename hdd asdf || return 1
+    ceph osd crush class rename ssd qwer || return 1
+    ceph osd crush class rm asdf || return 1
+    ceph osd crush class rm qwer || return 1
+    expect_failure $dir EBUSY ceph osd crush class rm abc || return 1 # still referenced by foo-rule
+    ceph osd crush rule rm foo-rule || return 1
+    ceph osd crush class rm abc || return 1
 }
 
 main crush-classes "$@"

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index c9845e792cbe36619fd842a4ee58958b1b9f56c6..2fc492e7f37201a6ccb235fcb7d95c2e2df263e2 100644 (file)
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -1372,18 +1372,39 @@ int CrushWrapper::get_parent_of_type(int item, int type) const
   return item;
 }
 
-bool CrushWrapper::class_is_in_use(int class_id)
-{
-  for (auto &i : class_bucket)
-    for (auto &j : i.second)
-      if (j.first == class_id)
-       return true;
-
-  for (auto &i : class_map)
-    if (i.second == class_id)
-      return true;
 
-  return false;
+bool CrushWrapper::class_is_in_use(int class_id, ostream *ss)
+{
+  list<unsigned> rules;
+  for (unsigned i = 0; i < crush->max_rules; ++i) {
+    crush_rule *r = crush->rules[i];
+    if (!r)
+      continue;
+    for (unsigned j = 0; j < r->len; ++j) {
+      if (r->steps[j].op == CRUSH_RULE_TAKE) {
+        int root = r->steps[j].arg1;
+        for (auto &p : class_bucket) {
+          auto& q = p.second;
+          if (q.count(class_id) && q[class_id] == root) {
+            rules.push_back(i);
+          }
+        }
+      }
+    }
+  }
+  if (rules.empty()) {
+    return false;
+  }
+  if (ss) {
+    ostringstream os;
+    for (auto &p: rules) {
+      os << "'" << get_rule_name(p) <<"',";
+    }
+    string out(os.str());
+    out.resize(out.size() - 1); // drop last ','
+    *ss << "still referenced by crush_rule(s): " << out;
+  }
+  return true;
 }
 
 int CrushWrapper::populate_classes()

diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 02ece73316a66ae9e8ee968c3af16fdd91f91ce6..459f0b59dc28f0fcf70f9dfab2083902f9aedae1 100644 (file)
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -1203,7 +1203,7 @@ public:
 
   int update_device_class(int id, const string& class_name, const string& name, ostream *ss);
   int device_class_clone(int original, int device_class, int *clone);
-  bool class_is_in_use(int class_id);
+  bool class_is_in_use(int class_id, ostream *ss = nullptr);
   int populate_classes();
   int rebuild_roots_with_classes();
   /* remove unused roots generated for class devices */

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index f802c29a5ed8d04a0450fb00f536cdb588eea962..8fc05f8619514d6aff322e00f8962eee414d2afd 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -7610,9 +7610,10 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
 
     int class_id = newcrush.get_class_id(device_class);
 
-    if (newcrush.class_is_in_use(class_id)) {
+    stringstream ts;
+    if (newcrush.class_is_in_use(class_id, &ts)) {
       err = -EBUSY;
-      ss << "class '" << device_class << "' is in use";
+      ss << "class '" << device_class << "' " << ts.str();
       goto reply;
     }
 
@@ -7664,9 +7665,10 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
 
     int class_id = newcrush.get_class_id(srcname);
 
-    if (newcrush.class_is_in_use(class_id)) {
+    stringstream ts;
+    if (newcrush.class_is_in_use(class_id, &ts)) {
       err = -EBUSY;
-      ss << "class '" << srcname << "' is in use";
+      ss << "class '" << srcname << "' " << ts.str();
       goto reply;
     }