From ff626f0ea962f3cf5005be3a74a15333045c003e Mon Sep 17 00:00:00 2001 From: Colin Patrick McCabe Date: Fri, 10 Jun 2011 14:30:17 -0700 Subject: [PATCH] bufferlist: get rid of derr usage bufferlist doesn't need to use dout/derr. Only one function used it, and that was only to return an error string. Signed-off-by: Colin McCabe --- src/Makefile.am | 1 + src/auth/KeyRing.cc | 5 +- src/cauthtool.cc | 10 ++-- src/cmon.cc | 21 +++++-- src/common/buffer.cc | 60 ++++++++++--------- src/cosd.cc | 5 +- src/crush/CrushWrapper.h | 3 +- src/crushtool.cc | 7 ++- src/include/buffer.h | 18 +----- src/mon/MonMap.cc | 3 +- src/osdmaptool.cc | 12 ++-- src/psim.cc | 6 +- src/test/cli/cauthtool/list-nonexistent-bin.t | 4 +- src/test/cli/cauthtool/list-nonexistent.t | 4 +- src/test/cli/monmaptool/add-exists.t | 1 - src/test/cli/monmaptool/add-many.t | 1 - src/test/cli/monmaptool/clobber.t | 1 - src/test/cli/monmaptool/create-print.t | 1 - src/test/cli/monmaptool/create-with-add.t | 1 - src/test/cli/monmaptool/print-nonexistent.t | 1 - src/test/cli/monmaptool/rm-nonexistent.t | 1 - src/test/cli/monmaptool/rm.t | 1 - src/test/cli/osdmaptool/print-nonexistent.t | 3 +- src/tools/common.cc | 7 ++- 24 files changed, 91 insertions(+), 86 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 61fb2e998d339..0455dbfb635ab 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -824,6 +824,7 @@ noinst_HEADERS = \ common/run_cmd.h\ common/safe_io.h\ common/config.h\ + common/config_obs.h\ common/ceph_crypto.h\ common/utf8.h\ common/secret.h\ diff --git a/src/auth/KeyRing.cc b/src/auth/KeyRing.cc index cb6c4ab648d46..5f21a1d3e3a60 100644 --- a/src/auth/KeyRing.cc +++ b/src/auth/KeyRing.cc @@ -194,9 +194,10 @@ int KeyRing::load(const std::string &filename) return -EINVAL; bufferlist bl; - int ret = bl.read_file(filename.c_str(), true); + std::string err; + int ret = bl.read_file(filename.c_str(), &err); if (ret < 0) { - derr << "error reading file: " << filename << dendl; + derr << "error reading file: " << filename << ": " << err << dendl; return ret; } diff --git a/src/cauthtool.cc b/src/cauthtool.cc index bb3dcd5622724..e7086d92d1691 100644 --- a/src/cauthtool.cc +++ b/src/cauthtool.cc @@ -145,7 +145,8 @@ int main(int argc, const char **argv) cout << "creating " << fn << std::endl; modified = true; } else { - r = bl.read_file(fn, true); + std::string err; + r = bl.read_file(fn, &err); if (r >= 0) { try { bufferlist::iterator iter = bl.begin(); @@ -155,7 +156,7 @@ int main(int argc, const char **argv) exit(1); } } else { - cerr << "can't open " << fn << ": " << strerror(-r) << std::endl; + cerr << "can't open " << fn << ": " << err << std::endl; exit(1); } } @@ -164,7 +165,8 @@ int main(int argc, const char **argv) if (import_keyring) { KeyRing other; bufferlist obl; - int r = obl.read_file(import_keyring); + std::string err; + int r = obl.read_file(import_keyring, &err); if (r >= 0) { try { bufferlist::iterator iter = obl.begin(); @@ -179,7 +181,7 @@ int main(int argc, const char **argv) keyring.import(other); modified = true; } else { - cerr << "can't open " << import_keyring << ": " << strerror(-r) << std::endl; + cerr << "can't open " << import_keyring << ": " << err << std::endl; exit(1); } } diff --git a/src/cmon.cc b/src/cmon.cc index 612a65fd0dd63..c497171385e72 100644 --- a/src/cmon.cc +++ b/src/cmon.cc @@ -97,15 +97,22 @@ int main(int argc, const char **argv) // load monmap bufferlist monmapbl, osdmapbl; - int err = monmapbl.read_file(g_conf->monmap.c_str()); - if (err < 0) + std::string error; + int err = monmapbl.read_file(g_conf->monmap.c_str(), &error); + if (err < 0) { + cout << argv[0] << ": error reading " << g_conf->monmap.c_str() << ": " + << error << std::endl; exit(1); + } MonMap monmap; monmap.decode(monmapbl); - err = osdmapbl.read_file(osdmapfn); - if (err < 0) + err = osdmapbl.read_file(osdmapfn, &error); + if (err < 0) { + cout << argv[0] << ": error reading " << osdmapfn << ": " + << error << std::endl; exit(1); + } // go MonitorStore store(g_conf->mon_data); @@ -169,9 +176,11 @@ int main(int argc, const char **argv) // inject new monmap? if (inject_monmap) { bufferlist bl; - int r = bl.read_file(inject_monmap); + std::string error; + int r = bl.read_file(inject_monmap, &error); if (r) { - cerr << "unable to read monmap from " << inject_monmap << std::endl; + cerr << "unable to read monmap from " << inject_monmap << ": " + << error << std::endl; exit(1); } diff --git a/src/common/buffer.cc b/src/common/buffer.cc index a96f3690689fd..85a2e3c339c15 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -14,14 +14,12 @@ #include "armor.h" -#include "common/debug.h" -#include "common/errno.h" #include "common/environment.h" +#include "common/errno.h" #include "common/safe_io.h" -#include "common/config.h" -#include "include/Spinlock.h" -#include "include/types.h" +#include "common/simple_spin.h" #include "include/atomic.h" +#include "include/types.h" #include #include @@ -31,7 +29,15 @@ namespace ceph { -Spinlock buffer_lock("buffer_lock"); +#ifdef BUFFER_DEBUG +static uint32_t simple_spinlock_t buffer_debug_lock = SIMPLE_SPINLOCK_INITIALIZER; +# define bdout { simple_spin_lock(&buffer_debug_lock); std::cout +# define bendl std::endl; simple_spin_unlock(&buffer_debug_lock); } +#else +# define bdout if (0) { std::cout +# define bendl std::endl; } +#endif + atomic_t buffer_total_alloc; bool buffer_track_alloc = get_env_bool("CEPH_BUFFER_TRACK"); @@ -373,10 +379,10 @@ void buffer::list::rebuild_page_aligned() while (p != _buffers.end()) { // keep anything that's already page sized+aligned if (p->is_page_aligned() && p->is_n_page_sized()) { - /*generic_dout(0) << " segment " << (void*)p->c_str() - << " offset " << ((unsigned long)p->c_str() & ~PAGE_MASK) - << " length " << p->length() - << " " << (p->length() & ~PAGE_MASK) << " ok" << dendl; + /*cout << " segment " << (void*)p->c_str() + << " offset " << ((unsigned long)p->c_str() & ~PAGE_MASK) + << " length " << p->length() + << " " << (p->length() & ~PAGE_MASK) << " ok" << std::endl; */ p++; continue; @@ -386,11 +392,11 @@ void buffer::list::rebuild_page_aligned() list unaligned; unsigned offset = 0; do { - /*generic_dout(0) << " segment " << (void*)p->c_str() - << " offset " << ((unsigned long)p->c_str() & ~PAGE_MASK) - << " length " << p->length() << " " << (p->length() & ~PAGE_MASK) - << " overall offset " << offset << " " << (offset & ~PAGE_MASK) - << " not ok" << dendl; + /*cout << " segment " << (void*)p->c_str() + << " offset " << ((unsigned long)p->c_str() & ~PAGE_MASK) + << " length " << p->length() << " " << (p->length() & ~PAGE_MASK) + << " overall offset " << offset << " " << (offset & ~PAGE_MASK) + << " not ok" << std::endl; */ offset += p->length(); unaligned.push_back(*p); @@ -405,14 +411,14 @@ void buffer::list::rebuild_page_aligned() } -int buffer::list::read_file(const char *fn, bool silent) +int buffer::list::read_file(const char *fn, std::string *error) { int fd = TEMP_FAILURE_RETRY(::open(fn, O_RDONLY)); if (fd < 0) { int err = errno; - if (!silent) { - derr << "can't open " << fn << ": " << cpp_strerror(err) << dendl; - } + std::ostringstream oss; + oss << "can't open " << fn << ": " << cpp_strerror(err); + *error = oss.str(); return -err; } @@ -422,20 +428,20 @@ int buffer::list::read_file(const char *fn, bool silent) ssize_t ret = read_fd(fd, st.st_size); if (ret < 0) { - if (!silent) { - derr << "bufferlist::read_file(" << fn << "): read error:" - << cpp_strerror(ret) << dendl; - } + std::ostringstream oss; + oss << "bufferlist::read_file(" << fn << "): read error:" + << cpp_strerror(ret); + *error = oss.str(); TEMP_FAILURE_RETRY(::close(fd)); return ret; } else if (ret != st.st_size) { // Premature EOF. // Perhaps the file changed between stat() and read()? - if (!silent) { - derr << "bufferlist::read_file(" << fn << "): warning: got premature " - << "EOF:" << dendl; - } + std::ostringstream oss; + oss << "bufferlist::read_file(" << fn << "): warning: got premature EOF."; + *error = oss.str(); + // not actually an error, but weird } TEMP_FAILURE_RETRY(::close(fd)); return 0; diff --git a/src/cosd.cc b/src/cosd.cc index 73b79dd0470b9..531ea6dc9adca 100644 --- a/src/cosd.cc +++ b/src/cosd.cc @@ -85,7 +85,8 @@ int main(int argc, const char **argv) if (dump_pg_log) { common_init_finish(&g_ceph_context); bufferlist bl; - int r = bl.read_file(dump_pg_log); + std::string error; + int r = bl.read_file(dump_pg_log, &error); if (r >= 0) { PG::Log::Entry e; bufferlist::iterator p = bl.begin(); @@ -101,7 +102,7 @@ int main(int argc, const char **argv) derr << pos << ":\t" << e << dendl; } } else { - derr << "unable to open " << dump_pg_log << ": " << cpp_strerror(r) << dendl; + derr << "unable to open " << dump_pg_log << ": " << error << dendl; } return 0; } diff --git a/src/crush/CrushWrapper.h b/src/crush/CrushWrapper.h index 53db4d3f379ab..0f9c8d19fe2f9 100644 --- a/src/crush/CrushWrapper.h +++ b/src/crush/CrushWrapper.h @@ -393,7 +393,8 @@ public: int read_from_file(const char *fn) { bufferlist bl; - int r = bl.read_file(fn); + std::string error; + int r = bl.read_file(fn, &error); if (r < 0) return r; bufferlist::iterator blp = bl.begin(); decode(blp); diff --git a/src/crushtool.cc b/src/crushtool.cc index e4b5dda115c31..bfb71fa41748d 100644 --- a/src/crushtool.cc +++ b/src/crushtool.cc @@ -936,10 +936,11 @@ int main(int argc, const char **argv) if (infn) { bufferlist bl; - int r = bl.read_file(infn); + std::string error; + int r = bl.read_file(infn, &error); if (r < 0) { - char buf[80]; - cerr << me << ": error reading '" << infn << "': " << strerror_r(-r, buf, sizeof(buf)) << std::endl; + cerr << me << ": error reading '" << infn << "': " + << error << std::endl; exit(1); } bufferlist::iterator p = bl.begin(); diff --git a/src/include/buffer.h b/src/include/buffer.h index a27f7c9fbbcc3..9e9f62475ced0 100644 --- a/src/include/buffer.h +++ b/src/include/buffer.h @@ -61,24 +61,8 @@ void *valloc(size_t); # include #endif -//#define BUFFER_DEBUG - -#ifdef BUFFER_DEBUG -# include "Spinlock.h" -#endif - namespace ceph { -#ifdef BUFFER_DEBUG -extern Spinlock buffer_lock; -# define bdout { buffer_lock.lock(); std::cout -# define bendl std::endl; buffer_lock.unlock(); } -#else -# define bdout if (0) { std::cout -# define bendl std::endl; } -#endif - - class buffer { /* * exceptions @@ -916,7 +900,7 @@ public: void decode_base64(list& o); void hexdump(std::ostream &out) const; - int read_file(const char *fn, bool silent=false); + int read_file(const char *fn, std::string *error); ssize_t read_fd(int fd, size_t len); int write_file(const char *fn, int mode=0644); int write_fd(int fd) const; diff --git a/src/mon/MonMap.cc b/src/mon/MonMap.cc index cb00dfde289af..2eddf47a5ebcf 100644 --- a/src/mon/MonMap.cc +++ b/src/mon/MonMap.cc @@ -20,7 +20,8 @@ int MonMap::read(const char *fn) { // read bufferlist bl; - int r = bl.read_file(fn); + std::string error; + int r = bl.read_file(fn, &error); if (r < 0) return r; decode(bl); diff --git a/src/osdmaptool.cc b/src/osdmaptool.cc index fb6e7f25b51e4..4787e256f9384 100644 --- a/src/osdmaptool.cc +++ b/src/osdmaptool.cc @@ -120,7 +120,8 @@ int main(int argc, const char **argv) int r = 0; struct stat st; if (!createsimple && !clobber) { - r = bl.read_file(fn); + std::string error; + r = bl.read_file(fn, &error); if (r == 0) { try { osdmap.decode(bl); @@ -131,8 +132,7 @@ int main(int argc, const char **argv) } } else { - cerr << me << ": couldn't open " << fn << ": " << cpp_strerror(-r) - << std::endl; + cerr << me << ": couldn't open " << fn << ": " << error << std::endl; return -1; } } @@ -154,9 +154,11 @@ int main(int argc, const char **argv) if (import_crush) { bufferlist cbl; - r = cbl.read_file(import_crush); + std::string error; + r = cbl.read_file(import_crush, &error); if (r) { - cerr << me << ": error reading crush map from " << import_crush << std::endl; + cerr << me << ": error reading crush map from " << import_crush + << ": " << error << std::endl; exit(1); } diff --git a/src/psim.cc b/src/psim.cc index 39099115e577e..6d1bc655049b8 100644 --- a/src/psim.cc +++ b/src/psim.cc @@ -13,7 +13,11 @@ int main(int argc, char **argv) * $ ./osdmaptool --createsimple .ceph_monmap 40 --clobber .ceph_osdmap */ bufferlist bl; - bl.read_file(".ceph_osdmap"); + std::string error; + if (bl.read_file(".ceph_osdmap", &error)) { + cout << argv[0] << ": error reading .ceph_osdmap: " << error << std::endl; + return 1; + } OSDMap osdmap; osdmap.decode(bl); diff --git a/src/test/cli/cauthtool/list-nonexistent-bin.t b/src/test/cli/cauthtool/list-nonexistent-bin.t index 009b598d91d88..38169065159ff 100644 --- a/src/test/cli/cauthtool/list-nonexistent-bin.t +++ b/src/test/cli/cauthtool/list-nonexistent-bin.t @@ -1,7 +1,7 @@ $ cauthtool --list --bin nonexistent - can't open nonexistent: No such file or directory + can't open nonexistent: can't open nonexistent: error 2: No such file or directory [1] $ cauthtool -l --bin nonexistent - can't open nonexistent: No such file or directory + can't open nonexistent: can't open nonexistent: error 2: No such file or directory [1] diff --git a/src/test/cli/cauthtool/list-nonexistent.t b/src/test/cli/cauthtool/list-nonexistent.t index 68308cce833a2..792bbac8a093b 100644 --- a/src/test/cli/cauthtool/list-nonexistent.t +++ b/src/test/cli/cauthtool/list-nonexistent.t @@ -1,7 +1,7 @@ $ cauthtool --list nonexistent - can't open nonexistent: No such file or directory + can't open nonexistent: can't open nonexistent: error 2: No such file or directory [1] $ cauthtool -l nonexistent - can't open nonexistent: No such file or directory + can't open nonexistent: can't open nonexistent: error 2: No such file or directory [1] diff --git a/src/test/cli/monmaptool/add-exists.t b/src/test/cli/monmaptool/add-exists.t index 9bc74880514e7..c229415b01b31 100644 --- a/src/test/cli/monmaptool/add-exists.t +++ b/src/test/cli/monmaptool/add-exists.t @@ -1,6 +1,5 @@ $ monmaptool --create mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (0 monitors) diff --git a/src/test/cli/monmaptool/add-many.t b/src/test/cli/monmaptool/add-many.t index b272a10d81c9c..0417bc14af6ba 100644 --- a/src/test/cli/monmaptool/add-many.t +++ b/src/test/cli/monmaptool/add-many.t @@ -1,6 +1,5 @@ $ monmaptool --create mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (0 monitors) diff --git a/src/test/cli/monmaptool/clobber.t b/src/test/cli/monmaptool/clobber.t index 9e93d7f2dc397..9de5471d114bc 100644 --- a/src/test/cli/monmaptool/clobber.t +++ b/src/test/cli/monmaptool/clobber.t @@ -1,6 +1,5 @@ $ monmaptool --create --add foo 2.3.4.5:6789 mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (1 monitors) diff --git a/src/test/cli/monmaptool/create-print.t b/src/test/cli/monmaptool/create-print.t index 70d77a79333dd..bede746b208cf 100644 --- a/src/test/cli/monmaptool/create-print.t +++ b/src/test/cli/monmaptool/create-print.t @@ -1,6 +1,5 @@ $ monmaptool --create mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (0 monitors) diff --git a/src/test/cli/monmaptool/create-with-add.t b/src/test/cli/monmaptool/create-with-add.t index 0ef0d59c35901..2065cff92847c 100644 --- a/src/test/cli/monmaptool/create-with-add.t +++ b/src/test/cli/monmaptool/create-with-add.t @@ -1,6 +1,5 @@ $ monmaptool --create --add foo 2.3.4.5:6789 mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (1 monitors) diff --git a/src/test/cli/monmaptool/print-nonexistent.t b/src/test/cli/monmaptool/print-nonexistent.t index 7e35dcd588e38..7c5d746d945ca 100644 --- a/src/test/cli/monmaptool/print-nonexistent.t +++ b/src/test/cli/monmaptool/print-nonexistent.t @@ -1,5 +1,4 @@ $ monmaptool --print nonexistent monmaptool: monmap file nonexistent - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open nonexistent: error 2: No such file or directory (re) monmaptool: couldn't open nonexistent: No such file or directory [255] diff --git a/src/test/cli/monmaptool/rm-nonexistent.t b/src/test/cli/monmaptool/rm-nonexistent.t index df572881a4900..3e97b85d69b77 100644 --- a/src/test/cli/monmaptool/rm-nonexistent.t +++ b/src/test/cli/monmaptool/rm-nonexistent.t @@ -1,6 +1,5 @@ $ monmaptool --create --add foo 2.3.4.5:6789 mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (1 monitors) diff --git a/src/test/cli/monmaptool/rm.t b/src/test/cli/monmaptool/rm.t index 8e8732a8e505b..981f4fb9d21f8 100644 --- a/src/test/cli/monmaptool/rm.t +++ b/src/test/cli/monmaptool/rm.t @@ -1,6 +1,5 @@ $ monmaptool --create --add foo 2.3.4.5:6789 mymonmap monmaptool: monmap file mymonmap - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open mymonmap: error 2: No such file or directory (re) monmaptool: generated fsid [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12} (re) monmaptool: writing epoch 1 to mymonmap (1 monitors) diff --git a/src/test/cli/osdmaptool/print-nonexistent.t b/src/test/cli/osdmaptool/print-nonexistent.t index 1e3672b45d548..72b5c82520f30 100644 --- a/src/test/cli/osdmaptool/print-nonexistent.t +++ b/src/test/cli/osdmaptool/print-nonexistent.t @@ -1,5 +1,4 @@ $ osdmaptool --print nonexistent osdmaptool: osdmap file 'nonexistent' - \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}\.\d+ [0-9a-f]{8,} can't open nonexistent: error 2: No such file or directory (re) - osdmaptool: couldn't open nonexistent: error 2: No such file or directory + osdmaptool: couldn't open nonexistent: can't open nonexistent: error 2: No such file or directory [255] diff --git a/src/tools/common.cc b/src/tools/common.cc index 2de08841f0dc8..ded95bd37822d 100644 --- a/src/tools/common.cc +++ b/src/tools/common.cc @@ -450,12 +450,13 @@ int run_command(CephToolCtx *ctx, const char *line) bufferlist out; if (infile) { - if (out.read_file(infile) == 0) { + std::string error; + if (out.read_file(infile, &error) == 0) { if (!ctx->concise) *ctx->log << "read " << out.length() << " from " << infile << std::endl; } else { - char buf[80]; - *ctx->log << "couldn't read from " << infile << ": " << strerror_r(errno, buf, sizeof(buf)) << std::endl; + *ctx->log << "couldn't read from " << infile << ": " + << error << std::endl; return 0; } } -- 2.39.5