From e4e83a0dd74d25eefd5e12c70d9ab247d9f70703 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Fri, 14 Jul 2017 19:43:26 +0800 Subject: [PATCH] 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 --- qa/standalone/crush/crush-classes.sh | 22 ++++++++++++++ src/crush/CrushWrapper.cc | 43 +++++++++++++++++++++------- src/crush/CrushWrapper.h | 2 +- src/mon/OSDMonitor.cc | 10 ++++--- 4 files changed, 61 insertions(+), 16 deletions(-) diff --git a/qa/standalone/crush/crush-classes.sh b/qa/standalone/crush/crush-classes.sh index b6b38a9a9f0..c81a4550dc8 100755 --- 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 c9845e792cb..2fc492e7f37 100644 --- 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 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 02ece73316a..459f0b59dc2 100644 --- 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 f802c29a5ed..8fc05f86195 100644 --- 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; } -- 2.39.5