From 7e818cd1ad94a5c7c01cb4093ef46d7b73d41a0a Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Fri, 18 May 2018 13:11:57 -0500
Subject: [PATCH] crush: update choose_args on bucket removal, resize, or
 position mismatch

The specific bug I see is that a bucket no longer exists but its
choose_args still does.  However, I'm also taking the opportunity to
verify that the choose_args agrees with the bucket sizes and position
counts everywhere else, too.  Check for

- ids or weight_sets for buckets that don't exist or aren't straw2
- weight_set_positions that don't match the choose_args
- weight_set sizes that don't match the bucket size

Fixes: http://tracker.ceph.com/issues/24167
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 564ef28a4014cb5d9959b1925154bbf7863fd0d4)
---
 src/crush/CrushWrapper.cc | 83 +++++++++++++++++++++++++++++++++++++++
 src/crush/CrushWrapper.h  |  3 ++
 2 files changed, 86 insertions(+)

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index b3cc3c0fc1362..29b81cdbeb859 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -357,6 +357,7 @@ bool CrushWrapper::_maybe_remove_last_instance(CephContext *cct, int item, bool
     if (class_bucket.count(item) != 0)
       class_bucket.erase(item);
     class_remove_item(item);
+    update_choose_args(cct);
   }
   if ((item >= 0 || !unlink_only) && name_map.count(item)) {
     ldout(cct, 5) << "_maybe_remove_last_instance removing name for item " << item << dendl;
@@ -399,9 +400,90 @@ int CrushWrapper::remove_root(int item)
   if (class_bucket.count(item) != 0)
     class_bucket.erase(item);
   class_remove_item(item);
+  update_choose_args(nullptr);
   return 0;
 }
 
+void CrushWrapper::update_choose_args(CephContext *cct)
+{
+  for (auto& i : choose_args) {
+    crush_choose_arg_map &arg_map = i.second;
+    unsigned positions = get_choose_args_positions(arg_map);
+    for (int j = 0; j < crush->max_buckets; ++j) {
+      crush_bucket *b = crush->buckets[j];
+      auto& carg = arg_map.args[j];
+      // strip out choose_args for any buckets that no longer exist
+      if (!b || b->alg != CRUSH_BUCKET_STRAW2) {
+	if (carg.ids) {
+	  if (cct)
+	    ldout(cct,0) << __func__ << " removing " << i.first << " bucket "
+			 << (-1-j) << " ids" << dendl;
+	  free(carg.ids);
+	  carg.ids = 0;
+	  carg.ids_size = 0;
+	}
+	if (carg.weight_set) {
+	  if (cct)
+	    ldout(cct,0) << __func__ << " removing " << i.first << " bucket "
+			 << (-1-j) << " weight_sets" << dendl;
+	  for (unsigned p = 0; p < carg.weight_set_positions; ++p) {
+	    free(carg.weight_set[p].weights);
+	  }
+	  free(carg.weight_set);
+	  carg.weight_set = 0;
+	  carg.weight_set_positions = 0;
+	}
+	continue;
+      } else if (positions != carg.weight_set_positions) {
+	// missing/mismatched positions?
+	if (cct)
+	  ldout(cct,0) << __func__ << " fixing " << i.first << " bucket "
+		       << (-1-j) << " positions " << carg.weight_set_positions
+		       << " -> " << positions << dendl;
+	auto old_pos = carg.weight_set_positions;
+	auto old_ws = carg.weight_set;
+	carg.weight_set_positions = positions;
+	carg.weight_set = static_cast<crush_weight_set*>(
+	  calloc(sizeof(crush_weight_set), positions));
+	for (unsigned p = 0; p < positions; ++p) {
+	  carg.weight_set[p].size = b->size;
+	  carg.weight_set[p].weights = (__u32*)calloc(b->size, sizeof(__u32));
+	  if (old_ws && old_pos < p) {
+	    auto max = std::max<unsigned>(old_ws[p].size, b->size);
+	    for (unsigned k = 0; k < max; ++k) {
+	      carg.weight_set[p].weights[k] = old_ws[p].weights[k];
+	    }
+	  }
+	}
+	if (old_ws) {
+	  for (unsigned p = 0; p < old_pos; ++p) {
+	    free(old_ws[p].weights);
+	  }
+	  free(old_ws);
+	}
+      }
+      // mis-sized weight_sets?
+      for (unsigned p = 0; p < positions; ++p) {
+	if (carg.weight_set[p].size != b->size) {
+	  if (cct)
+	    ldout(cct,0) << __func__ << " fixing " << i.first << " bucket "
+			 << (-1-j) << " position " << p
+			 << " size " << carg.weight_set[p].size << " -> "
+			 << b->size << dendl;
+	  auto old_ws = carg.weight_set[p];
+	  carg.weight_set[p].size = b->size;
+	  carg.weight_set[p].weights = (__u32*)calloc(b->size, sizeof(__u32));
+	  auto max = std::max<unsigned>(old_ws.size, b->size);
+	  for (unsigned k = 0; k < max; ++k) {
+	    carg.weight_set[p].weights[k] = old_ws.weights[k];
+	  }
+	  free(old_ws.weights);
+	}
+      }
+    }
+  }
+}
+
 int CrushWrapper::remove_item(CephContext *cct, int item, bool unlink_only)
 {
   ldout(cct, 5) << "remove_item " << item
@@ -2545,6 +2627,7 @@ void CrushWrapper::decode(bufferlist::iterator& blp)
 	choose_args[choose_args_index] = arg_map;
       }
     }
+    update_choose_args(nullptr); // in case we decode a legacy "corrupted" map
     finalize();
   }
   catch (...) {
diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h
index bef4e075f19de..5ebf2c8e91e2d 100644
--- a/src/crush/CrushWrapper.h
+++ b/src/crush/CrushWrapper.h
@@ -1464,6 +1464,9 @@ public:
     choose_args.clear();
   }
 
+  // remove choose_args for buckets that no longer exist, create them for new buckets
+  void update_choose_args(CephContext *cct);
+
   // adjust choose_args_map weight, preserving the hierarchical summation
   // property.  used by callers optimizing layouts by tweaking weights.
   int _choose_args_adjust_item_weight_in_bucket(
-- 
2.39.5