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>

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index b3cc3c0fc1362d23816414802942f2f3c6c785db..29b81cdbeb859c46e0f672529e3ff2ff6cb9e3bb 100644 (file)
--- 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 bef4e075f19ded7e4c4107b3c96e005e31edb378..5ebf2c8e91e2d0dd598e2b431cd8d8ae57c70c2f 100644 (file)
--- 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(