From: David Zafman Date: Mon, 9 Sep 2013 20:01:12 +0000 (-0700) Subject: crushtool: do not dump core with non-unique bucket IDs X-Git-Tag: v0.71~141 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=8c76f3a0f9cf100ea2c941dc2b61c470aa5033d7;p=ceph.git crushtool: do not dump core with non-unique bucket IDs Return -EEXIST on duplicate ID BUG FIX: crush_add_bucket() mixes error returns and IDs Add optional argument to return generated ID Fixes: #6246 Signed-off-by: David Zafman Reviewed-by: Sage Weil --- diff --git a/src/crush/CrushCompiler.cc b/src/crush/CrushCompiler.cc index aee621d8e32..5f92bf7e4ec 100644 --- a/src/crush/CrushCompiler.cc +++ b/src/crush/CrushCompiler.cc @@ -527,9 +527,17 @@ int CrushCompiler::parse_bucket(iter_t const& i) item_id[name] = id; item_weight[id] = bucketweight; - crush.add_bucket(id, alg, hash, type, size, &items[0], &weights[0]); - crush.set_item_name(id, name.c_str()); - return 0; + assert(id != 0); + int r = crush.add_bucket(id, alg, hash, type, size, &items[0], &weights[0], NULL); + if (r < 0) { + if (r == -EEXIST) + err << "Duplicate bucket id " << id << std::endl; + else + err << "add_bucket failed " << strerror(-r) << std::endl; + return r; + } + r = crush.set_item_name(id, name.c_str()); + return r; } int CrushCompiler::parse_rule(iter_t const& i) diff --git a/src/crush/CrushWrapper.cc b/src/crush/CrushWrapper.cc index e96e6123aab..bab9f9a817e 100644 --- a/src/crush/CrushWrapper.cc +++ b/src/crush/CrushWrapper.cc @@ -363,9 +363,15 @@ int CrushWrapper::insert_item(CephContext *cct, int item, float weight, string n if (!name_exists(q->second)) { ldout(cct, 5) << "insert_item creating bucket " << q->second << dendl; - int empty = 0; - cur = add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, p->first, 1, &cur, &empty); - set_item_name(cur, q->second); + int empty = 0, newid; + int r = add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, p->first, 1, &cur, &empty, &newid); + if (r < 0) { + char buf[128]; + ldout(cct, 1) << "add_bucket failure error: " << strerror_r(-r, buf, sizeof(buf)) << dendl; + return r; + } + set_item_name(newid, q->second); + cur = newid; continue; } diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h index 3d07a281956..80906e4fe18 100644 --- a/src/crush/CrushWrapper.h +++ b/src/crush/CrushWrapper.h @@ -720,10 +720,10 @@ public: /* modifiers */ int add_bucket(int bucketno, int alg, int hash, int type, int size, - int *items, int *weights) { + int *items, int *weights, int *idout) { crush_bucket *b = crush_make_bucket(alg, hash, type, size, items, weights); assert(b); - return crush_add_bucket(crush, bucketno, b); + return crush_add_bucket(crush, bucketno, b, idout); } void finalize() { diff --git a/src/crush/builder.c b/src/crush/builder.c index 2eb6ff5fc1e..9bfde0bd8e2 100644 --- a/src/crush/builder.c +++ b/src/crush/builder.c @@ -123,7 +123,8 @@ int crush_get_next_bucket_id(struct crush_map *map) int crush_add_bucket(struct crush_map *map, int id, - struct crush_bucket *bucket) + struct crush_bucket *bucket, + int *idout) { int pos; @@ -148,13 +149,16 @@ int crush_add_bucket(struct crush_map *map, memset(map->buckets + oldsize, 0, (map->max_buckets-oldsize) * sizeof(map->buckets[0])); } - assert(map->buckets[pos] == 0); + if (map->buckets[pos] != 0) { + return -EEXIST; + } /* add it */ bucket->id = id; map->buckets[pos] = bucket; - return id; + if (idout) *idout = id; + return 0; } int crush_remove_bucket(struct crush_map *map, struct crush_bucket *bucket) diff --git a/src/crush/builder.h b/src/crush/builder.h index 7d30c882343..1003c353e60 100644 --- a/src/crush/builder.h +++ b/src/crush/builder.h @@ -15,7 +15,7 @@ extern int crush_add_rule(struct crush_map *map, struct crush_rule *rule, int ru extern int crush_get_next_bucket_id(struct crush_map *map); extern int crush_add_bucket(struct crush_map *map, int bucketno, - struct crush_bucket *bucket); + struct crush_bucket *bucket, int *idout); struct crush_bucket *crush_make_bucket(int alg, int hash, int type, int size, int *items, int *weights); extern int crush_bucket_add_item(struct crush_bucket *bucket, int item, int weight); extern int crush_bucket_adjust_item_weight(struct crush_bucket *bucket, int item, int weight); diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 03f70a05a35..5450532e46d 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -2699,9 +2699,15 @@ bool OSDMonitor::prepare_command(MMonCommand *m) err = -EINVAL; goto reply; } - int bucketno = newcrush.add_bucket(0, CRUSH_BUCKET_STRAW, + int bucketno; + err = newcrush.add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, type, 0, NULL, - NULL); + NULL, &bucketno); + if (err < 0) { + char buf[128]; + ss << "add_bucket error: '" << strerror_r(-err, buf, sizeof(buf)) << "'"; + goto reply; + } err = newcrush.set_item_name(bucketno, name); if (err < 0) { ss << "error setting bucket name to '" << name << "'"; diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc index 4b35b0c48ea..8007d613b8c 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -1845,7 +1845,9 @@ void OSDMap::build_simple_crush_map(CephContext *cct, CrushWrapper& crush, crush.set_type_name(6, "root"); // root - int rootid = crush.add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, 6 /* pool */, 0, NULL, NULL); + int rootid; + int r = crush.add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, 6 /* pool */, 0, NULL, NULL, &rootid); + assert(r == 0); crush.set_item_name(rootid, "default"); for (int o=0; o hosts, racks; // root - int rootid = crush.add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, 6 /* pool */, 0, NULL, NULL); + int rootid; + int r = crush.add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_DEFAULT, 6 /* pool */, 0, NULL, NULL, &rootid); + assert(r == 0); crush.set_item_name(rootid, "default"); // add osds diff --git a/src/tools/crushtool.cc b/src/tools/crushtool.cc index 75c26c098b6..03c83f24156 100644 --- a/src/tools/crushtool.cc +++ b/src/tools/crushtool.cc @@ -569,7 +569,11 @@ int main(int argc, const char **argv) crush_bucket *b = crush_make_bucket(buckettype, CRUSH_HASH_DEFAULT, type, j, items, weights); assert(b); - int id = crush_add_bucket(crush.crush, 0, b); + int id; + int r = crush_add_bucket(crush.crush, 0, b, &id); + if (r < 0) { + dout(0) << "Couldn't add root bucket: " << strerror(-r) << dendl; + } rootid = id; char format[20];