crush: check for invalid names in loc[]

Add the is_valid_crush_loc helper to test for invalid crush names in
insert_item and update_item, before performing any side
effect. Implement the associated unit tests.

Signed-off-by: Loic Dachary <loic@dachary.org>
This commit is contained in:
Loic Dachary 2013-12-05 19:41:50 +01:00
parent 39ddd213da
commit aedbc99ffc
3 changed files with 78 additions and 20 deletions

View File

@ -336,16 +336,8 @@ int CrushWrapper::insert_item(CephContext *cct, int item, float weight, string n
if (!is_valid_crush_name(name))
return -EINVAL;
for (map<string,string>::const_iterator l = loc.begin(); l != loc.end(); l++) {
if (!is_valid_crush_name(l->second)) {
ldout(cct, 1) << "insert_item with loc["
<< l->first << "] = '"
<< l->second << "' is not a valid crush name ([A-Za-z0-9_-.]+)"
<< dendl;
return -ENFILE;
}
}
if (!is_valid_crush_loc(cct, loc))
return -EINVAL;
if (name_exists(name)) {
if (get_item_id(name) != item) {
@ -517,6 +509,9 @@ int CrushWrapper::update_item(CephContext *cct, int item, float weight, string n
if (!is_valid_crush_name(name))
return -EINVAL;
if (!is_valid_crush_loc(cct, loc))
return -EINVAL;
// compare quantized (fixed-point integer) weights!
int iweight = (int)(weight * (float)0x10000);
int old_iweight;
@ -1145,3 +1140,19 @@ bool CrushWrapper::is_valid_crush_name(const string& s)
}
return true;
}
bool CrushWrapper::is_valid_crush_loc(CephContext *cct,
const map<string,string> loc)
{
for (map<string,string>::const_iterator l = loc.begin(); l != loc.end(); l++) {
if (!is_valid_crush_name(l->first) ||
!is_valid_crush_name(l->second)) {
ldout(cct, 1) << "loc["
<< l->first << "] = '"
<< l->second << "' not a valid crush name ([A-Za-z0-9_-.]+)"
<< dendl;
return false;
}
}
return true;
}

View File

@ -797,6 +797,8 @@ public:
static bool is_valid_crush_name(const string& s);
static bool is_valid_crush_loc(CephContext *cct,
const map<string,string> loc);
};
WRITE_CLASS_ENCODER(CrushWrapper)

View File

@ -29,6 +29,35 @@
#include "crush/CrushWrapper.h"
TEST(CrushWrapper, update_item) {
CrushWrapper *c = new CrushWrapper;
c->create();
#define ROOT_TYPE 3
c->set_type_name(ROOT_TYPE, "root");
#define RACK_TYPE 2
c->set_type_name(RACK_TYPE, "rack");
#define HOST_TYPE 1
c->set_type_name(HOST_TYPE, "host");
#define OSD_TYPE 0
c->set_type_name(OSD_TYPE, "osd");
int rootno;
c->add_bucket(0, CRUSH_BUCKET_STRAW, CRUSH_HASH_RJENKINS1,
ROOT_TYPE, 0, NULL, NULL, &rootno);
c->set_item_name(rootno, "default");
int item = 0;
// invalid names anywhere in loc trigger an error
{
map<string,string> loc;
loc["rack"] = "\001";
EXPECT_EQ(-EINVAL, c->update_item(g_ceph_context, item, 1.0,
"osd." + stringify(item), loc));
}
}
TEST(CrushWrapper, insert_item) {
CrushWrapper *c = new CrushWrapper;
c->create();
@ -48,6 +77,15 @@ TEST(CrushWrapper, insert_item) {
c->set_item_name(rootno, "default");
int item = 0;
// invalid names anywhere in loc trigger an error
{
map<string,string> loc;
loc["rack"] = "\001";
EXPECT_EQ(-EINVAL, c->insert_item(g_ceph_context, item, 1.0,
"osd." + stringify(item), loc));
}
// insert an item in an existing bucket
{
map<string,string> loc;
@ -71,16 +109,6 @@ TEST(CrushWrapper, insert_item) {
EXPECT_EQ(0, c->insert_item(g_ceph_context, item, 1.0,
"osd." + stringify(item), loc));
}
// implicit creation of a bucket with an invalid name fails
{
map<string,string> loc;
loc["root"] = "default";
loc["rack"] = "\001";
item++;
EXPECT_EQ(-ENFILE, c->insert_item(g_ceph_context, item, 1.0,
"osd." + stringify(item), loc));
}
// adding to an existing item name that is not associated with a bucket
{
std::string name = "ITEM_WITHOUT_BUCKET";
@ -200,6 +228,23 @@ TEST(CrushWrapper, is_valid_crush_name) {
EXPECT_FALSE(CrushWrapper::is_valid_crush_name("\001"));
}
TEST(CrushWrapper, is_valid_crush_loc) {
map<string,string> loc;
EXPECT_TRUE(CrushWrapper::is_valid_crush_loc(g_ceph_context, loc));
loc["good"] = "better";
EXPECT_TRUE(CrushWrapper::is_valid_crush_loc(g_ceph_context, loc));
{
map<string,string> loc;
loc["\005"] = "default";
EXPECT_FALSE(CrushWrapper::is_valid_crush_loc(g_ceph_context, loc));
}
{
map<string,string> loc;
loc["rack"] = "\003";
EXPECT_FALSE(CrushWrapper::is_valid_crush_loc(g_ceph_context, loc));
}
}
int main(int argc, char **argv) {
vector<const char*> args;
argv_to_vec(argc, (const char **)argv, args);