From d61269402d4030fae16d4e1640b2d38428d5f7ab Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sat, 28 Jul 2012 07:39:27 -0700 Subject: [PATCH] config: send warnings to a ostream* argument We shouldn't always send these to stderr. (Among other things, the warning: prefix breaks the gitbuilder error detection.) Signed-off-by: Sage Weil --- src/auth/KeyRing.cc | 3 ++- src/ceph_authtool.cc | 2 +- src/common/ConfUtils.cc | 17 +++++++----- src/common/ConfUtils.h | 6 ++--- src/common/config.cc | 11 +++++--- src/common/config.h | 6 +++-- src/global/global_init.cc | 2 +- src/libcephfs.cc | 2 +- src/librados/librados.cc | 2 +- src/test/confutils.cc | 56 +++++++++++++++++++++++---------------- 10 files changed, 63 insertions(+), 44 deletions(-) diff --git a/src/auth/KeyRing.cc b/src/auth/KeyRing.cc index e7c7f4d235456..cd8dd974ad1c3 100644 --- a/src/auth/KeyRing.cc +++ b/src/auth/KeyRing.cc @@ -165,7 +165,8 @@ void KeyRing::decode_plaintext(bufferlist::iterator& bli) bli.copy_all(bl); ConfFile cf; std::deque parse_errors; - if (cf.parse_bufferlist(&bl, &parse_errors) != 0) { + + if (cf.parse_bufferlist(&bl, &parse_errors, NULL) != 0) { throw buffer::malformed_input("cannot parse buffer"); } diff --git a/src/ceph_authtool.cc b/src/ceph_authtool.cc index 41defd354cac6..035994c17ea9b 100644 --- a/src/ceph_authtool.cc +++ b/src/ceph_authtool.cc @@ -217,7 +217,7 @@ int main(int argc, const char **argv) if (!caps_fn.empty()) { ConfFile cf; std::deque parse_errors; - if (cf.parse_file(caps_fn, &parse_errors) != 0) { + if (cf.parse_file(caps_fn, &parse_errors, &cerr) != 0) { cerr << "could not parse caps file " << caps_fn << std::endl; exit(1); } diff --git a/src/common/ConfUtils.cc b/src/common/ConfUtils.cc index b3f7f39258239..a9e5d7a42f54b 100644 --- a/src/common/ConfUtils.cc +++ b/src/common/ConfUtils.cc @@ -90,7 +90,8 @@ clear() * loading the whole configuration into memory shouldn't be a problem. */ int ConfFile:: -parse_file(const std::string &fname, std::deque *errors) +parse_file(const std::string &fname, std::deque *errors, + std::ostream *warnings) { clear(); @@ -150,7 +151,7 @@ parse_file(const std::string &fname, std::deque *errors) } } - load_from_buffer(buf, sz, errors); + load_from_buffer(buf, sz, errors, warnings); ret = 0; done: @@ -160,11 +161,12 @@ parse_file(const std::string &fname, std::deque *errors) } int ConfFile:: -parse_bufferlist(ceph::bufferlist *bl, std::deque *errors) +parse_bufferlist(ceph::bufferlist *bl, std::deque *errors, + std::ostream *warnings) { clear(); - load_from_buffer(bl->c_str(), bl->length(), errors); + load_from_buffer(bl->c_str(), bl->length(), errors, warnings); return 0; } @@ -280,7 +282,8 @@ std::ostream &operator<<(std::ostream &oss, const ConfFile &cf) } void ConfFile:: -load_from_buffer(const char *buf, size_t sz, std::deque *errors) +load_from_buffer(const char *buf, size_t sz, std::deque *errors, + std::ostream *warnings) { errors->clear(); @@ -368,8 +371,8 @@ load_from_buffer(const char *buf, size_t sz, std::deque *errors) // foo = 2 // will result in foo = 2. cur_section->second.lines.erase(*cline); - if (cline->key.length()) - std::cerr << "warning: line " << line_no << ": '" << cline->key << "' in section '" + if (cline->key.length() && warnings) + *warnings << "warning: line " << line_no << ": '" << cline->key << "' in section '" << cur_section->first << "' redefined " << std::endl; } // add line to current section diff --git a/src/common/ConfUtils.h b/src/common/ConfUtils.h index b3d11696a2c08..e816f68c2a8b2 100644 --- a/src/common/ConfUtils.h +++ b/src/common/ConfUtils.h @@ -62,8 +62,8 @@ class ConfFile { ConfFile(); ~ConfFile(); void clear(); - int parse_file(const std::string &fname, std::deque *errors); - int parse_bufferlist(ceph::bufferlist *bl, std::deque *errors); + int parse_file(const std::string &fname, std::deque *errors, std::ostream *warnings); + int parse_bufferlist(ceph::bufferlist *bl, std::deque *errors, std::ostream *warnings); int read(const std::string §ion, const std::string &key, std::string &val) const; @@ -76,7 +76,7 @@ class ConfFile { private: void load_from_buffer(const char *buf, size_t sz, - std::deque *errors); + std::deque *errors, std::ostream *warnings); static ConfLine* process_line(int line_no, const char *line, std::deque *errors); diff --git a/src/common/config.cc b/src/common/config.cc index c66bc5102e7e6..c744002844247 100644 --- a/src/common/config.cc +++ b/src/common/config.cc @@ -175,7 +175,9 @@ void md_config_t::remove_observer(md_config_obs_t* observer_) } int md_config_t::parse_config_files(const char *conf_files, - std::deque *parse_errors, int flags) + std::deque *parse_errors, + std::ostream *warnings, + int flags) { Mutex::Locker l(lock); if (internal_safe_to_start_threads) @@ -193,11 +195,12 @@ int md_config_t::parse_config_files(const char *conf_files, } std::list cfl; get_str_list(conf_files, cfl); - return parse_config_files_impl(cfl, parse_errors); + return parse_config_files_impl(cfl, parse_errors, warnings); } int md_config_t::parse_config_files_impl(const std::list &conf_files, - std::deque *parse_errors) + std::deque *parse_errors, + std::ostream *warnings) { assert(lock.is_locked()); @@ -207,7 +210,7 @@ int md_config_t::parse_config_files_impl(const std::list &conf_file cf.clear(); string fn = *c; expand_meta(fn); - int ret = cf.parse_file(fn.c_str(), parse_errors); + int ret = cf.parse_file(fn.c_str(), parse_errors, warnings); if (ret == 0) break; else if (ret != -ENOENT) diff --git a/src/common/config.h b/src/common/config.h index 67efc57d73b34..60b13819db532 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -99,7 +99,8 @@ class md_config_t { // Parse a config file int parse_config_files(const char *conf_files, - std::deque *parse_errors, int flags); + std::deque *parse_errors, + std::ostream *warnings, int flags); // Absorb config settings from the environment void parse_env(); @@ -158,7 +159,8 @@ class md_config_t { int parse_injectargs(std::vector& args, std::ostream *oss); int parse_config_files_impl(const std::list &conf_files, - std::deque *parse_errors); + std::deque *parse_errors, + std::ostream *warnings); int set_val_impl(const char *val, const config_option *opt); int set_val_raw(const char *val, const config_option *opt); diff --git a/src/global/global_init.cc b/src/global/global_init.cc index 17f886cd6e98c..8d119d45670d7 100644 --- a/src/global/global_init.cc +++ b/src/global/global_init.cc @@ -74,7 +74,7 @@ void global_init(std::vector < const char * > *alt_def_args, std::vector < const conf->parse_argv(*alt_def_args); // alternative default args std::deque parse_errors; - int ret = conf->parse_config_files(c_str_or_null(conf_file_list), &parse_errors, flags); + int ret = conf->parse_config_files(c_str_or_null(conf_file_list), &parse_errors, &cerr, flags); if (ret == -EDOM) { dout_emergency("global_init: error parsing config file.\n"); _exit(1); diff --git a/src/libcephfs.cc b/src/libcephfs.cc index a240cc3a2fbf5..2e3c7d45c504d 100644 --- a/src/libcephfs.cc +++ b/src/libcephfs.cc @@ -131,7 +131,7 @@ class ceph_mount_info int conf_read_file(const char *path_list) { std::deque parse_errors; - int ret = cct->_conf->parse_config_files(path_list, &parse_errors, 0); + int ret = cct->_conf->parse_config_files(path_list, &parse_errors, NULL, 0); if (ret) return ret; cct->_conf->apply_changes(NULL); diff --git a/src/librados/librados.cc b/src/librados/librados.cc index 69ba440d5de8f..54939353fbcf8 100644 --- a/src/librados/librados.cc +++ b/src/librados/librados.cc @@ -1360,7 +1360,7 @@ extern "C" int rados_conf_read_file(rados_t cluster, const char *path_list) librados::RadosClient *client = (librados::RadosClient *)cluster; md_config_t *conf = client->cct->_conf; std::deque parse_errors; - int ret = conf->parse_config_files(path_list, &parse_errors, 0); + int ret = conf->parse_config_files(path_list, &parse_errors, NULL, 0); if (ret) return ret; conf->parse_env(); // environment variables override diff --git a/src/test/confutils.cc b/src/test/confutils.cc index a504032630d81..196cd58b13733 100644 --- a/src/test/confutils.cc +++ b/src/test/confutils.cc @@ -304,28 +304,29 @@ TEST(Whitespace, ConfUtils) { TEST(ParseFiles0, ConfUtils) { std::deque err; std::string val; + std::ostringstream warn; std::string trivial_conf_1_f(next_tempfile(trivial_conf_1)); ConfFile cf1; - ASSERT_EQ(cf1.parse_file(trivial_conf_1_f.c_str(), &err), 0); + ASSERT_EQ(cf1.parse_file(trivial_conf_1_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); std::string trivial_conf_2_f(next_tempfile(trivial_conf_2)); ConfFile cf2; - ASSERT_EQ(cf2.parse_file(trivial_conf_2_f.c_str(), &err), 0); + ASSERT_EQ(cf2.parse_file(trivial_conf_2_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 1U); bufferlist bl3; bl3.append(trivial_conf_3, strlen(trivial_conf_3)); ConfFile cf3; - ASSERT_EQ(cf3.parse_bufferlist(&bl3, &err), 0); + ASSERT_EQ(cf3.parse_bufferlist(&bl3, &err, &warn), 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf3.read("global", "log dir", val), 0); ASSERT_EQ(val, "barfoo"); std::string trivial_conf_4_f(next_tempfile(trivial_conf_4)); ConfFile cf4; - ASSERT_EQ(cf4.parse_file(trivial_conf_4_f.c_str(), &err), 0); + ASSERT_EQ(cf4.parse_file(trivial_conf_4_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf4.read("global", "log dir", val), 0); ASSERT_EQ(val, "barbaz"); @@ -333,34 +334,36 @@ TEST(ParseFiles0, ConfUtils) { TEST(ParseFiles1, ConfUtils) { std::deque err; + std::ostringstream warn; std::string simple_conf_1_f(next_tempfile(simple_conf_1)); ConfFile cf1; - ASSERT_EQ(cf1.parse_file(simple_conf_1_f.c_str(), &err), 0); + ASSERT_EQ(cf1.parse_file(simple_conf_1_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); std::string simple_conf_2_f(next_tempfile(simple_conf_1)); ConfFile cf2; - ASSERT_EQ(cf2.parse_file(simple_conf_2_f.c_str(), &err), 0); + ASSERT_EQ(cf2.parse_file(simple_conf_2_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); bufferlist bl3; bl3.append(simple_conf_1, strlen(simple_conf_1)); ConfFile cf3; - ASSERT_EQ(cf3.parse_bufferlist(&bl3, &err), 0); + ASSERT_EQ(cf3.parse_bufferlist(&bl3, &err, &warn), 0); ASSERT_EQ(err.size(), 0U); bufferlist bl4; bl4.append(simple_conf_2, strlen(simple_conf_2)); ConfFile cf4; - ASSERT_EQ(cf4.parse_bufferlist(&bl4, &err), 0); + ASSERT_EQ(cf4.parse_bufferlist(&bl4, &err, &warn), 0); ASSERT_EQ(err.size(), 0U); } TEST(ReadFiles1, ConfUtils) { std::deque err; + std::ostringstream warn; std::string simple_conf_1_f(next_tempfile(simple_conf_1)); ConfFile cf1; - ASSERT_EQ(cf1.parse_file(simple_conf_1_f.c_str(), &err), 0); + ASSERT_EQ(cf1.parse_file(simple_conf_1_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); std::string val; @@ -378,7 +381,7 @@ TEST(ReadFiles1, ConfUtils) { bufferlist bl2; bl2.append(simple_conf_2, strlen(simple_conf_2)); ConfFile cf2; - ASSERT_EQ(cf2.parse_bufferlist(&bl2, &err), 0); + ASSERT_EQ(cf2.parse_bufferlist(&bl2, &err, &warn), 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf2.read("osd0", "keyring", val), 0); ASSERT_EQ(val, "osd_keyring"); @@ -390,10 +393,11 @@ TEST(ReadFiles1, ConfUtils) { TEST(ReadFiles2, ConfUtils) { std::deque err; + std::ostringstream warn; std::string conf3_f(next_tempfile(conf3)); ConfFile cf1; std::string val; - ASSERT_EQ(cf1.parse_file(conf3_f.c_str(), &err), 0); + ASSERT_EQ(cf1.parse_file(conf3_f.c_str(), &err, &warn), 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"); @@ -402,7 +406,7 @@ TEST(ReadFiles2, ConfUtils) { std::string unicode_config_1f(next_tempfile(unicode_config_1)); ConfFile cf2; - ASSERT_EQ(cf2.parse_file(unicode_config_1f.c_str(), &err), 0); + ASSERT_EQ(cf2.parse_file(unicode_config_1f.c_str(), &err, &warn), 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"); @@ -410,40 +414,42 @@ TEST(ReadFiles2, ConfUtils) { TEST(IllegalFiles, ConfUtils) { std::deque err; + std::ostringstream warn; std::string illegal_conf1_f(next_tempfile(illegal_conf1)); ConfFile cf1; std::string val; - ASSERT_EQ(cf1.parse_file(illegal_conf1_f.c_str(), &err), 0); + ASSERT_EQ(cf1.parse_file(illegal_conf1_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 1U); bufferlist bl2; bl2.append(illegal_conf2, strlen(illegal_conf2)); ConfFile cf2; - ASSERT_EQ(cf2.parse_bufferlist(&bl2, &err), 0); + ASSERT_EQ(cf2.parse_bufferlist(&bl2, &err, &warn), 0); ASSERT_EQ(err.size(), 1U); std::string illegal_conf3_f(next_tempfile(illegal_conf3)); ConfFile cf3; - ASSERT_EQ(cf3.parse_file(illegal_conf3_f.c_str(), &err), 0); + ASSERT_EQ(cf3.parse_file(illegal_conf3_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 1U); std::string illegal_conf4_f(next_tempfile(illegal_conf4)); ConfFile cf4; - ASSERT_EQ(cf4.parse_file(illegal_conf4_f.c_str(), &err), 0); + ASSERT_EQ(cf4.parse_file(illegal_conf4_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 1U); std::string illegal_conf5_f(next_tempfile(illegal_conf5)); ConfFile cf5; - ASSERT_EQ(cf5.parse_file(illegal_conf5_f.c_str(), &err), 0); + ASSERT_EQ(cf5.parse_file(illegal_conf5_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 1U); } TEST(EscapingFiles, ConfUtils) { std::deque err; + std::ostringstream warn; std::string escaping_conf_1_f(next_tempfile(escaping_conf_1)); ConfFile cf1; std::string val; - ASSERT_EQ(cf1.parse_file(escaping_conf_1_f.c_str(), &err), 0); + ASSERT_EQ(cf1.parse_file(escaping_conf_1_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf1.read("global", "log file", val), 0); @@ -455,7 +461,7 @@ TEST(EscapingFiles, ConfUtils) { std::string escaping_conf_2_f(next_tempfile(escaping_conf_2)); ConfFile cf2; - ASSERT_EQ(cf2.parse_file(escaping_conf_2_f.c_str(), &err), 0); + ASSERT_EQ(cf2.parse_file(escaping_conf_2_f.c_str(), &err, &warn), 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(cf2.read("apple ][", "log file", val), 0); @@ -467,20 +473,21 @@ TEST(EscapingFiles, ConfUtils) { TEST(Overrides, ConfUtils) { md_config_t conf; std::deque err; + std::ostringstream warn; std::string override_conf_1_f(next_tempfile(override_config_1)); conf.name.set(CEPH_ENTITY_TYPE_MON, "0"); - conf.parse_config_files(override_conf_1_f.c_str(), &err, 0); + conf.parse_config_files(override_conf_1_f.c_str(), &err, &warn, 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(conf.log_file, "global_log"); conf.name.set(CEPH_ENTITY_TYPE_MDS, "a"); - conf.parse_config_files(override_conf_1_f.c_str(), &err, 0); + conf.parse_config_files(override_conf_1_f.c_str(), &err, &warn, 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(conf.log_file, "mds_log"); conf.name.set(CEPH_ENTITY_TYPE_OSD, "0"); - conf.parse_config_files(override_conf_1_f.c_str(), &err, 0); + conf.parse_config_files(override_conf_1_f.c_str(), &err, &warn, 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(conf.log_file, "osd0_log"); } @@ -488,10 +495,13 @@ TEST(Overrides, ConfUtils) { TEST(DupKey, ConfUtils) { md_config_t conf; std::deque err; + std::ostringstream warn; std::string dup_key_config_f(next_tempfile(dup_key_config_1)); conf.name.set(CEPH_ENTITY_TYPE_MDS, "a"); - conf.parse_config_files(dup_key_config_f.c_str(), &err, 0); + conf.parse_config_files(dup_key_config_f.c_str(), &err, &warn, 0); ASSERT_EQ(err.size(), 0U); ASSERT_EQ(conf.log_file, string("3")); } + +