From: Sage Weil <sage@redhat.com>
Date: Fri, 4 Aug 2017 02:39:05 +0000 (-0400)
Subject: crush: remove cleanup_classes()
X-Git-Tag: v12.1.3~103^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c121501ad3e96c3b11954cf641c37e5b8ef53de9;p=ceph-ci.git

crush: remove cleanup_classes()

I can't for the life of me figure out what this is supposed to do.

- why remove (some) classes right after we populated them?
- why remove them after we decode the crush map?

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/crush/CrushCompiler.cc b/src/crush/CrushCompiler.cc
index 89253fb20db..eb4f7b7cf4a 100644
--- a/src/crush/CrushCompiler.cc
+++ b/src/crush/CrushCompiler.cc
@@ -291,8 +291,6 @@ int CrushCompiler::decompile_choose_args(const std::pair<const long unsigned int
 
 int CrushCompiler::decompile(ostream &out)
 {
-  crush.cleanup_classes();
-
   out << "# begin crush map\n";
 
   // only dump tunables if they differ from the defaults
@@ -1100,7 +1098,6 @@ int CrushCompiler::parse_crush(iter_t const& i)
   }
 
   //err << "max_devices " << crush.get_max_devices() << std::endl;
-  crush.cleanup_classes();
   crush.finalize();
 
   return 0;
diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 511828f6bb8..9c03358aad3 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -1425,11 +1425,6 @@ int CrushWrapper::populate_classes(
   return 0;
 }
 
-int CrushWrapper::cleanup_classes()
-{
-  return trim_roots_with_class(true);
-}
-
 int CrushWrapper::trim_roots_with_class(bool unused)
 {
   set<int> roots;
@@ -1948,10 +1943,7 @@ int CrushWrapper::rebuild_roots_with_classes()
   if (r < 0)
     return r;
   class_bucket.clear();
-  r = populate_classes(old_class_bucket);
-  if (r < 0)
-    return r;
-  return trim_roots_with_class(true);
+  return populate_classes(old_class_bucket);
 }
 
 void CrushWrapper::encode(bufferlist& bl, uint64_t features) const
@@ -2206,7 +2198,6 @@ void CrushWrapper::decode(bufferlist::iterator& blp)
       for (auto &c : class_name)
 	class_rname[c.second] = c.first;
       ::decode(class_bucket, blp);
-      cleanup_classes();
     }
     if (!blp.end()) {
       __u32 choose_args_size;
diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index 5cb0e795cf3..a4b86c23229 100644
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -1220,7 +1220,6 @@ public:
   int rebuild_roots_with_classes();
   /* remove unused roots generated for class devices */
   int trim_roots_with_class(bool unused);
-  int cleanup_classes();
 
   void start_choose_profile() {
     free(crush->choose_tries);
diff --git a/src/test/crush/CrushWrapper.cc b/src/test/crush/CrushWrapper.cc
index 7929bffc429..4502d0d0e69 100644
--- a/src/test/crush/CrushWrapper.cc
+++ b/src/test/crush/CrushWrapper.cc
@@ -1204,7 +1204,7 @@ TEST(CrushWrapper, split_id_class) {
   ASSERT_EQ(-1, retrieved_class_id);
 }
 
-TEST(CrushWrapper, populate_and_cleanup_classes) {
+TEST(CrushWrapper, populate_classes) {
   CrushWrapper c;
   c.create();
   c.set_type_name(1, "root");
@@ -1226,10 +1226,6 @@ TEST(CrushWrapper, populate_and_cleanup_classes) {
   old_class_bucket = c.class_bucket;
   ASSERT_EQ(c.populate_classes(old_class_bucket), 0);
   ASSERT_EQ(old_class_bucket, c.class_bucket);
-  
-  c.class_bucket.clear();
-  ASSERT_EQ(c.cleanup_classes(), 0);
-  ASSERT_FALSE(c.name_exists("default~ssd"));
 }
 
 TEST(CrushWrapper, remove_class_name) {