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 <colin.mccabe@dreamhost.com>

diff --git a/src/auth/KeyRing.cc b/src/auth/KeyRing.cc
index 6a673afd12198353ae94b933fc7da1c3f9850340..25579b938eb2838a9a9c13ea22a53b583d107ffb 100644 (file)
--- 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<std::string> 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 e4862ffcf9f6ef2652e10365335f15dba7478b1f..f1d0701537ad3b7bb0ee920f3c3ab641de6f4682 100644 (file)
--- 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<std::string> 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<string, bufferlist> 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 a95c572853c6a5cb7dcd608ffad26bb5ab0739b0..0b221c8a04300c6f5df8d7a95462cc9222035931 100644 (file)
--- 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<std::string> *parse_errors)
+{
+  common_init();
+
+  if (fname)
+    filename = strdup(fname);
+  else
+    filename = NULL;
+
+  return parse();
+}
+
+int ConfFile::parse_bufferlist(ceph::bufferlist *pbl_,
+                              std::deque<std::string> *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 a5e3eec0c916b77d27f4f172aa438fb645c6709b..790051308b93c332128e84e8ba453bae3e92c2df 100644 (file)
--- a/src/common/ConfUtils.h
+++ b/src/common/ConfUtils.h
@@ -2,6 +2,7 @@
 #define CEPH_CONFUTILS_H
 
 
+#include <deque>
 #include <string.h>
 #include <map>
 #include <string>
@@ -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<std::string> *parse_errors);
+       int parse_bufferlist(ceph::bufferlist *bl,
+                            std::deque<std::string> *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 32f75428f98b495382acd1e7c70eb2cd13d24b6f..f68be5350a193844402a5037dbdbe81d1cd69e9e 100644 (file)
--- a/src/common/common_init.cc
+++ b/src/common/common_init.cc
@@ -26,6 +26,7 @@
 #include "include/color.h"
 
 #include <errno.h>
+#include <deque>
 #include <syslog.h>
 
 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<std::string> *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<std::string>::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<std::string> 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 4e649fa9412a82f0e3306fedc66b43dfce61bbca..bd549f456c7a08141c86995c13a296a8b8e00681 100644 (file)
--- 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 <deque>
 #include <stdint.h>
 #include <string>
 #include <vector>
@@ -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<std::string> *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 5814a5bb5109b1892b454c3ef8b60491a03cc8d8..4a5de4b8655f12b791a125cfdda8c18f108f8681 100644 (file)
--- 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<std::string> &conf_files)
+parse_config_files(const std::list<std::string> &conf_files,
+                  std::deque<std::string> *parse_errors)
 {
   // open new conf
   list<string>::const_iterator c = conf_files.begin();
@@ -519,8 +520,8 @@ parse_config_files(const std::list<std::string> &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 d200b5a8b37f8183d460f75114107e3902f9a361..bfa34e871db63296710965dcb083c41d02d161d3 100644 (file)
--- 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<std::string> &conf_files);
+  int parse_config_files(const std::list<std::string> &conf_files,
+                        std::deque<std::string> *parse_errors);
 
   // Absorb config settings from the environment
   void parse_env();

diff --git a/src/librados.cc b/src/librados.cc
index 2a0d7560e6be7b9b90ab34d64d2a8588e4ad9d08..042ddb8333cca310aac52d7b8b2ad0ef5091cc9a 100644 (file)
--- 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<std::string> conf_files;
   get_str_list(path, conf_files);
-  int ret = g_conf.parse_config_files(conf_files);
+  std::deque<std::string> 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 d7e356881a8f182cf5cc46e916b8bba2c816eae4..84a0092e1665c002a54ccca814129df9be405936 100644 (file)
--- a/src/test/confutils.cc
+++ b/src/test/confutils.cc
@@ -205,29 +205,36 @@ const char unicode_config_1[] = "\
 ";
 
 TEST(ParseFiles1, ConfUtils) {
+  std::deque<std::string> 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<std::string> 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<std::string> 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<std::string> 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
 }