crush: CrushWrapper: don't add item to a bucket with != type than wanted

We take little consideration about the type of the bucket we are adding
an item to. Although this works for the vast majority of cases, it was
also leaving room for silly little mistakes to become problematic and
leading a monitor to crash.

For instance, say that we ran:
  'ceph osd crush set 0 osd.0 1 root=foo row=foo'

If root 'foo' exists, then this will work and 'row=foo' will be ignored.
However, if there is no bucket named 'foo', then we would (in order)
create a bucket for row 'foo', adding osd.0 to it, and would then add
osd.0 to bucket 'foo' again -- remember, little consideration regarding
the bucket type was given.

This would trigger a monitor crash due to the recursion done in
'adjust_item_weight'. A solution to this problem is to make sure that we
do not allow specifying multiple buckets with the same name when adding
an item to crush. Not only solves our crash problem, but will also render
invalid any mistake when specifying the wrong bucket type (say, using
'row=bar' when in fact 'bar' is a rack).

Fixes: #3515

Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com>

1 files changed, 8 insertions, 0 deletions

diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc
index 475c8c105e9..f38a7698376 100644
--- a/src/crush/CrushWrapper.cc
+++ b/src/crush/CrushWrapper.cc
@@ -258,6 +258,14 @@ int CrushWrapper::insert_item(CephContext *cct, int item, float weight, string n
     crush_bucket *b = get_bucket(id);
     assert(b);
 
+    if (p->first != b->type) {
+      ldout(cct, 1) << "insert_item existing bucket has type "
+	<< "'" << type_map[b->type] << "' != "
+	<< "'" << type_map[p->first] << "'" << dendl;
+      return -EINVAL;
+    }
+
+
     // make sure the item doesn't already exist in this bucket
     for (unsigned j=0; j<b->size; j++)
       if (b->items[j] == cur) {