From: Colin Patrick McCabe Date: Fri, 1 Apr 2011 23:52:02 +0000 (-0700) Subject: config: add ability to complain about parse errs X-Git-Tag: v0.27~161 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=94fade29af9b086555a2c51710d1c826678d1f3a;p=ceph.git config: add ability to complain about parse errs Change the ConfUtils interface so that we have a way to return information about parse errors. Signed-off-by: Colin McCabe --- diff --git a/src/auth/KeyRing.cc b/src/auth/KeyRing.cc index 6a673afd121..25579b938eb 100644 --- a/src/auth/KeyRing.cc +++ b/src/auth/KeyRing.cc @@ -104,8 +104,9 @@ void KeyRing::decode_plaintext(bufferlist::iterator& bli) bufferlist bl; bl.append(orig_src, len); - ConfFile cf(&bl); - if (cf.parse() != 0) { + ConfFile cf; + std::deque parse_errors; + if (cf.parse_bufferlist(&bl, &parse_errors) != 0) { derr << "cannot parse buffer" << dendl; goto done_err; } diff --git a/src/cauthtool.cc b/src/cauthtool.cc index e4862ffcf9f..f1d0701537a 100644 --- a/src/cauthtool.cc +++ b/src/cauthtool.cc @@ -202,16 +202,18 @@ int main(int argc, const char **argv) cout << "added entity " << ename << " auth " << eauth << std::endl; } if (caps_fn) { - ConfFile *cf = new ConfFile(caps_fn); - if (cf->parse() != 0) { + ConfFile cf; + std::deque parse_errors; + if (cf.parse_file(caps_fn, &parse_errors) != 0) { cerr << "could not parse caps file " << caps_fn << std::endl; exit(1); } + complain_about_parse_errors(&parse_errors); map caps; const char *key_names[] = { "mon", "osd", "mds", NULL }; for (int i=0; key_names[i]; i++) { std::string val; - if (cf->read("global", key_names[i], val) == 0) { + if (cf.read("global", key_names[i], val) == 0) { bufferlist bl; ::encode(val, bl); string s(key_names[i]); diff --git a/src/common/ConfUtils.cc b/src/common/ConfUtils.cc index a95c572853c..0b221c8a043 100644 --- a/src/common/ConfUtils.cc +++ b/src/common/ConfUtils.cc @@ -458,22 +458,9 @@ void ConfFile::common_init() default_global = true; } -ConfFile::ConfFile(const char *fname) +ConfFile::ConfFile() : parse_lock("ConfFile::parse_lock", false, false, false) { - common_init(); - - if (fname) - filename = strdup(fname); - else - filename = NULL; -} - -ConfFile::ConfFile(ceph::bufferlist *_pbl) - : parse_lock("ConfFile::parse_lock", false, false, false) -{ - common_init(); - pbl = _pbl; } ConfFile::~ConfFile() @@ -727,6 +714,27 @@ done: return ret; } +int ConfFile::parse_file(const char *fname, std::deque *parse_errors) +{ + common_init(); + + if (fname) + filename = strdup(fname); + else + filename = NULL; + + return parse(); +} + +int ConfFile::parse_bufferlist(ceph::bufferlist *pbl_, + std::deque *parse_errors) +{ + common_init(); + pbl = pbl_; + + return parse(); +} + int ConfFile::parse() { ConfSection *section = NULL; diff --git a/src/common/ConfUtils.h b/src/common/ConfUtils.h index a5e3eec0c91..790051308b9 100644 --- a/src/common/ConfUtils.h +++ b/src/common/ConfUtils.h @@ -2,6 +2,7 @@ #define CEPH_CONFUTILS_H +#include #include #include #include @@ -89,9 +90,9 @@ class ConfFile { int _read(int fd, char *buf, size_t size); int _close(int fd); + int parse(); public: - ConfFile(const char *fname); - ConfFile(ceph::bufferlist *bl); + ConfFile(); ~ConfFile(); const SectionList& get_section_list() { return sections_list; } @@ -99,7 +100,9 @@ public: ConfLine *_find_var(const char *section, const char* var); - int parse(); + int parse_file(const char *fname, std::deque *parse_errors); + int parse_bufferlist(ceph::bufferlist *bl, + std::deque *parse_errors); int read(const char *section, const char *var, std::string &val); diff --git a/src/common/common_init.cc b/src/common/common_init.cc index 32f75428f98..f68be5350a1 100644 --- a/src/common/common_init.cc +++ b/src/common/common_init.cc @@ -26,6 +26,7 @@ #include "include/color.h" #include +#include #include int keyring_init(md_config_t *conf) @@ -110,6 +111,26 @@ md_config_t *common_preinit(const CephInitParameters &iparams, return conf; } +void complain_about_parse_errors(std::deque *parse_errors) +{ + if (parse_errors->empty()) + return; + derr << "Errors while parsing config file!" << dendl; + int cur_err = 0; + static const int MAX_PARSE_ERRORS = 20; + for (std::deque::const_iterator p = parse_errors->begin(); + p != parse_errors->end(); ++p) + { + derr << *p << dendl; + if (cur_err == MAX_PARSE_ERRORS) { + derr << "Suppressed " << (parse_errors->size() - MAX_PARSE_ERRORS) + << " more errors." << dendl; + break; + } + ++cur_err; + } +} + void common_init(std::vector < const char* >& args, uint32_t module_type, code_environment_t code_env, int flags) { @@ -117,7 +138,8 @@ void common_init(std::vector < const char* >& args, ceph_argparse_early_args(args, module_type, flags); md_config_t *conf = common_preinit(iparams, code_env, flags); - int ret = conf->parse_config_files(iparams.get_conf_files()); + std::deque parse_errors; + int ret = conf->parse_config_files(iparams.get_conf_files(), &parse_errors); if (ret == -EDOM) { derr << "common_init: error parsing config file." << dendl; _exit(1); @@ -155,6 +177,9 @@ void common_init(std::vector < const char* >& args, _dout_open_log(); } + // Now we're ready to complain about config file parse errors + complain_about_parse_errors(&parse_errors); + // signal stuff int siglist[] = { SIGPIPE, 0 }; block_signals(NULL, siglist); diff --git a/src/common/common_init.h b/src/common/common_init.h index 4e649fa9412..bd549f456c7 100644 --- a/src/common/common_init.h +++ b/src/common/common_init.h @@ -1,6 +1,7 @@ #ifndef CEPH_COMMON_INIT_H #define CEPH_COMMON_INIT_H +#include #include #include #include @@ -21,6 +22,7 @@ enum common_init_flags_t { int keyring_init(md_config_t *conf); md_config_t *common_preinit(const CephInitParameters &iparams, enum code_environment_t code_env, int flags); +void complain_about_parse_errors(std::deque *parse_errors); void common_init(std::vector < const char* >& args, uint32_t module_type, code_environment_t code_env, int flags); diff --git a/src/common/config.cc b/src/common/config.cc index 5814a5bb510..4a5de4b8655 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -510,7 +510,8 @@ md_config_t:: } int md_config_t:: -parse_config_files(const std::list &conf_files) +parse_config_files(const std::list &conf_files, + std::deque *parse_errors) { // open new conf list::const_iterator c = conf_files.begin(); @@ -519,8 +520,8 @@ parse_config_files(const std::list &conf_files) while (true) { if (c == conf_files.end()) return -EINVAL; - ConfFile *cf_ = new ConfFile(c->c_str()); - int res = cf_->parse(); + ConfFile *cf_ = new ConfFile(); + int res = cf_->parse_file(c->c_str(), parse_errors); if (res == 0) { cf = cf_; break; diff --git a/src/common/config.h b/src/common/config.h index d200b5a8b37..bfa34e871db 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -51,7 +51,8 @@ public: ~md_config_t(); // Parse a config file - int parse_config_files(const std::list &conf_files); + int parse_config_files(const std::list &conf_files, + std::deque *parse_errors); // Absorb config settings from the environment void parse_env(); diff --git a/src/librados.cc b/src/librados.cc index 2a0d7560e6b..042ddb8333c 100644 --- a/src/librados.cc +++ b/src/librados.cc @@ -2828,6 +2828,7 @@ extern "C" int rados_connect(rados_t cluster) if (ret) return ret; librados::RadosClient *radosp = (librados::RadosClient *)cluster; + return radosp->connect(); } @@ -2857,11 +2858,15 @@ extern "C" int rados_conf_read_file(rados_t cluster, const char *path) std::list conf_files; get_str_list(path, conf_files); - int ret = g_conf.parse_config_files(conf_files); + std::deque parse_errors; + int ret = g_conf.parse_config_files(conf_files, &parse_errors); if (ret) return ret; g_conf.parse_env(); // environment variables override g_conf.expand_all_meta(); // handle metavariables in the config + + complain_about_parse_errors(&parse_errors); + return 0; } diff --git a/src/test/confutils.cc b/src/test/confutils.cc index d7e356881a8..84a0092e166 100644 --- a/src/test/confutils.cc +++ b/src/test/confutils.cc @@ -205,29 +205,36 @@ const char unicode_config_1[] = "\ "; TEST(ParseFiles1, ConfUtils) { + std::deque err; std::string simple_conf_1_f(next_tempfile(simple_conf_1)); - ConfFile cf1(simple_conf_1_f.c_str()); - ASSERT_EQ(cf1.parse(), 0); + ConfFile cf1; + ASSERT_EQ(cf1.parse_file(simple_conf_1_f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); std::string simple_conf_2_f(next_tempfile(simple_conf_1)); - ConfFile cf2(simple_conf_2_f.c_str()); - ASSERT_EQ(cf2.parse(), 0); + ConfFile cf2; + ASSERT_EQ(cf2.parse_file(simple_conf_2_f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); bufferlist bl3; bl3.append(simple_conf_1, strlen(simple_conf_1)); - ConfFile cf3(&bl3); - ASSERT_EQ(cf3.parse(), 0); + ConfFile cf3; + ASSERT_EQ(cf3.parse_bufferlist(&bl3, &err), 0); + ASSERT_EQ(err.size(), 0U); bufferlist bl4; bl4.append(simple_conf_2, strlen(simple_conf_2)); - ConfFile cf4(&bl4); - ASSERT_EQ(cf4.parse(), 0); + ConfFile cf4; + ASSERT_EQ(cf4.parse_bufferlist(&bl4, &err), 0); + ASSERT_EQ(err.size(), 0U); } TEST(ReadFiles1, ConfUtils) { + std::deque err; std::string simple_conf_1_f(next_tempfile(simple_conf_1)); - ConfFile cf1(simple_conf_1_f.c_str()); - ASSERT_EQ(cf1.parse(), 0); + ConfFile cf1; + ASSERT_EQ(cf1.parse_file(simple_conf_1_f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); std::string val; ASSERT_EQ(cf1.read("global", "keyring", val), 0); @@ -243,8 +250,9 @@ TEST(ReadFiles1, ConfUtils) { bufferlist bl2; bl2.append(simple_conf_2, strlen(simple_conf_2)); - ConfFile cf2(&bl2); - ASSERT_EQ(cf2.parse(), 0); + ConfFile cf2; + ASSERT_EQ(cf2.parse_bufferlist(&bl2, &err), 0); + ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf2.read("osd0", "keyring", val), 0); ASSERT_EQ(val, "osd_keyring"); @@ -254,35 +262,42 @@ TEST(ReadFiles1, ConfUtils) { } TEST(ReadFiles2, ConfUtils) { + std::deque err; std::string conf3_f(next_tempfile(conf3)); - ConfFile cf1(conf3_f.c_str()); + ConfFile cf1; std::string val; - ASSERT_EQ(cf1.parse(), 0); + ASSERT_EQ(cf1.parse_file(conf3_f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf1.read("global", "log file", val), 0); ASSERT_EQ(val, "/quite/a/long/path/for/a/log/file"); ASSERT_EQ(cf1.read("global", "pid file", val), 0); ASSERT_EQ(val, "spork"); std::string unicode_config_1f(next_tempfile(unicode_config_1)); - ConfFile cf2(unicode_config_1f.c_str()); - ASSERT_EQ(cf2.parse(), 0); + ConfFile cf2; + ASSERT_EQ(cf2.parse_file(unicode_config_1f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf2.read("global", "log file", val), 0); ASSERT_EQ(val, "\x66\xd1\x86\xd1\x9d\xd3\xad\xd3\xae"); } // FIXME: illegal configuration files don't return a parse error currently. TEST(IllegalFiles, ConfUtils) { + std::deque err; std::string illegal_conf1_f(next_tempfile(illegal_conf1)); - ConfFile cf1(illegal_conf1_f.c_str()); + ConfFile cf1; std::string val; - ASSERT_EQ(cf1.parse(), 0); + ASSERT_EQ(cf1.parse_file(illegal_conf1_f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); // FIXME bufferlist bl2; bl2.append(illegal_conf2, strlen(illegal_conf2)); - ConfFile cf2(&bl2); - ASSERT_EQ(cf2.parse(), 0); + ConfFile cf2; + ASSERT_EQ(cf2.parse_bufferlist(&bl2, &err), 0); + ASSERT_EQ(err.size(), 0U); // FIXME std::string illegal_conf3_f(next_tempfile(illegal_conf3)); - ConfFile cf3(illegal_conf3_f.c_str()); - ASSERT_EQ(cf3.parse(), 0); + ConfFile cf3; + ASSERT_EQ(cf3.parse_file(illegal_conf3_f.c_str(), &err), 0); + ASSERT_EQ(err.size(), 0U); // FIXME }