Skip to content

Commit

Permalink
config: send warnings to a ostream* argument
Browse files Browse the repository at this point in the history 
We shouldn't always send these to stderr.  (Among other things, the
warning: prefix breaks the gitbuilder error detection.)

Signed-off-by: Sage Weil <[email protected]>
  • Loading branch information
Sage Weil committed Jul 28, 2012
1 parent de4474a commit d612694
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 44 deletions.
3 changes: 2 additions & 1 deletion src/auth/KeyRing.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ void KeyRing::decode_plaintext(bufferlist::iterator& bli)
bli.copy_all(bl);
ConfFile cf;
std::deque<std::string> 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");
}

Expand Down
2 changes: 1 addition & 1 deletion src/ceph_authtool.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ int main(int argc, const char **argv)
if (!caps_fn.empty()) {
ConfFile cf;
std::deque<std::string> 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);
}
Expand Down
17 changes: 10 additions & 7 deletions src/common/ConfUtils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> *errors)
parse_file(const std::string &fname, std::deque<std::string> *errors,
std::ostream *warnings)
{
clear();

Expand Down Expand Up @@ -150,7 +151,7 @@ parse_file(const std::string &fname, std::deque<std::string> *errors)
}
}

load_from_buffer(buf, sz, errors);
load_from_buffer(buf, sz, errors, warnings);
ret = 0;

done:
Expand All @@ -160,11 +161,12 @@ parse_file(const std::string &fname, std::deque<std::string> *errors)
}

int ConfFile::
parse_bufferlist(ceph::bufferlist *bl, std::deque<std::string> *errors)
parse_bufferlist(ceph::bufferlist *bl, std::deque<std::string> *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;
}

Expand Down Expand Up @@ -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<std::string> *errors)
load_from_buffer(const char *buf, size_t sz, std::deque<std::string> *errors,
std::ostream *warnings)
{
errors->clear();

Expand Down Expand Up @@ -368,8 +371,8 @@ load_from_buffer(const char *buf, size_t sz, std::deque<std::string> *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
Expand Down
6 changes: 3 additions & 3 deletions src/common/ConfUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ class ConfFile {
ConfFile();
~ConfFile();
void clear();
int parse_file(const std::string &fname, std::deque<std::string> *errors);
int parse_bufferlist(ceph::bufferlist *bl, std::deque<std::string> *errors);
int parse_file(const std::string &fname, std::deque<std::string> *errors, std::ostream *warnings);
int parse_bufferlist(ceph::bufferlist *bl, std::deque<std::string> *errors, std::ostream *warnings);
int read(const std::string &section, const std::string &key,
std::string &val) const;

Expand All @@ -76,7 +76,7 @@ class ConfFile {

private:
void load_from_buffer(const char *buf, size_t sz,
std::deque<std::string> *errors);
std::deque<std::string> *errors, std::ostream *warnings);
static ConfLine* process_line(int line_no, const char *line,
std::deque<std::string> *errors);

Expand Down
11 changes: 7 additions & 4 deletions src/common/config.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> *parse_errors, int flags)
std::deque<std::string> *parse_errors,
std::ostream *warnings,
int flags)
{
Mutex::Locker l(lock);
if (internal_safe_to_start_threads)
Expand All @@ -193,11 +195,12 @@ int md_config_t::parse_config_files(const char *conf_files,
}
std::list<std::string> 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<std::string> &conf_files,
std::deque<std::string> *parse_errors)
std::deque<std::string> *parse_errors,
std::ostream *warnings)
{
assert(lock.is_locked());

Expand All @@ -207,7 +210,7 @@ int md_config_t::parse_config_files_impl(const std::list<std::string> &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)
Expand Down
6 changes: 4 additions & 2 deletions src/common/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ class md_config_t {

// Parse a config file
int parse_config_files(const char *conf_files,
std::deque<std::string> *parse_errors, int flags);
std::deque<std::string> *parse_errors,
std::ostream *warnings, int flags);

// Absorb config settings from the environment
void parse_env();
Expand Down Expand Up @@ -158,7 +159,8 @@ class md_config_t {
int parse_injectargs(std::vector<const char*>& args,
std::ostream *oss);
int parse_config_files_impl(const std::list<std::string> &conf_files,
std::deque<std::string> *parse_errors);
std::deque<std::string> *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);
Expand Down
2 changes: 1 addition & 1 deletion src/global/global_init.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> 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);
Expand Down
2 changes: 1 addition & 1 deletion src/libcephfs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class ceph_mount_info
int conf_read_file(const char *path_list)
{
std::deque<std::string> 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);
Expand Down
2 changes: 1 addition & 1 deletion src/librados/librados.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> 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
Expand Down
56 changes: 33 additions & 23 deletions src/test/confutils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,63 +304,66 @@ TEST(Whitespace, ConfUtils) {
TEST(ParseFiles0, ConfUtils) {
std::deque<std::string> 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");
}

TEST(ParseFiles1, ConfUtils) {
std::deque<std::string> 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<std::string> 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;
Expand All @@ -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");
Expand All @@ -390,10 +393,11 @@ TEST(ReadFiles1, ConfUtils) {

TEST(ReadFiles2, ConfUtils) {
std::deque<std::string> 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");
Expand All @@ -402,48 +406,50 @@ 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");
}

TEST(IllegalFiles, ConfUtils) {
std::deque<std::string> 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<std::string> 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);
Expand All @@ -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);
Expand All @@ -467,31 +473,35 @@ TEST(EscapingFiles, ConfUtils) {
TEST(Overrides, ConfUtils) {
md_config_t conf;
std::deque<std::string> 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");
}

TEST(DupKey, ConfUtils) {
md_config_t conf;
std::deque<std::string> 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"));
}


0 comments on commit d612694

Please sign in to comment.