diff options
author | David Zafman <david.zafman@inktank.com> | 2013-09-09 13:01:12 -0700 |
---|---|---|
committer | Sage Weil <sage@inktank.com> | 2013-09-24 09:03:58 -0700 |
commit | fb15040b6cec6221baa550ddfffade823f784c4a (patch) | |
tree | e894b851ab671fced48a585845c4d6ddacbe0ef6 | |
parent | 410db3f30c6eb54b807908c1f251ad4026e7d446 (diff) | |
download | ceph-fb15040b6cec6221baa550ddfffade823f784c4a.tar.gz |
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 <david.zafman@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>
(cherry picked from commit 8c76f3a0f9cf100ea2c941dc2b61c470aa5033d7)
-rw-r--r-- | src/crush/CrushCompiler.cc | 14 | ||||
-rw-r--r-- | src/crush/CrushWrapper.cc | 12 | ||||
-rw-r--r-- | src/crush/CrushWrapper.h | 4 | ||||
-rw-r--r-- | src/crush/builder.c | 10 | ||||
-rw-r--r-- | src/crush/builder.h | 2 | ||||
-rw-r--r-- | src/crushtool.cc | 6 | ||||
-rw-r--r-- | src/mon/OSDMonitor.cc | 10 | ||||
-rw-r--r-- | src/osd/OSDMap.cc | 8 |
8 files changed, 49 insertions, 17 deletions
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/crushtool.cc b/src/crushtool.cc index 75c26c098b6..03c83f24156 100644 --- a/src/crushtool.cc +++ b/src/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]; diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index af4f62d9c58..0185ca4d67e 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -2680,9 +2680,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 36a43ff4b57..bc513218dfb 100644 --- a/src/osd/OSDMap.cc +++ b/src/osd/OSDMap.cc @@ -1831,7 +1831,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<nosd; o++) { @@ -1961,7 +1963,9 @@ void OSDMap::build_simple_crush_map_from_conf(CephContext *cct, CrushWrapper& cr set<string> 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 |