Merge branch 'wip-mon-store-errorcheck' into next

Reviewed-by: Joao Luis <joao.luis@inktank.com>

9 files changed, 125 insertions, 97 deletions

diff --git a/src/ceph_mon.cc b/src/ceph_mon.cc
index da4529f2765..eee048027aa 100644
--- a/src/ceph_mon.cc
+++ b/src/ceph_mon.cc
@@ -284,14 +284,14 @@ int main(int argc, const char **argv)
   {
     bufferlist mapbl;
     bufferlist latest;
-    store.get_bl_ss(latest, "monmap", "latest");
+    store.get_bl_ss_safe(latest, "monmap", "latest");
     if (latest.length() > 0) {
       bufferlist::iterator p = latest.begin();
       version_t v;
       ::decode(v, p);
       ::decode(mapbl, p);
     } else {
-      store.get_bl_ss(mapbl, "mkfs", "monmap");
+      store.get_bl_ss_safe(mapbl, "mkfs", "monmap");
       if (mapbl.length() == 0) {
 	cerr << "mon fs missing 'monmap/latest' and 'mkfs/monmap'" << std::endl;
 	exit(1);
diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index c43d7e988db..aec2ba04d86 100644
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -96,7 +96,7 @@ void AuthMonitor::create_initial()
 
   KeyRing keyring;
   bufferlist bl;
-  mon->store->get_bl_ss(bl, "mkfs", "keyring");
+  mon->store->get_bl_ss_safe(bl, "mkfs", "keyring");
   bufferlist::iterator p = bl.begin();
   ::decode(keyring, p);
 
diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index dd256b9b14a..72168ac9638 100644
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -555,7 +555,7 @@ bool MDSMonitor::preprocess_command(MMonCommand *m)
       MDSMap *p = &mdsmap;
       if (epoch) {
 	bufferlist b;
-	mon->store->get_bl_sn(b, "mdsmap", epoch);
+	mon->store->get_bl_sn_safe(b, "mdsmap", epoch);
 	if (!b.length()) {
 	  p = 0;
 	  r = -ENOENT;
@@ -597,7 +597,7 @@ bool MDSMonitor::preprocess_command(MMonCommand *m)
 	}
 	epoch_t e = l;
 	bufferlist b;
-	mon->store->get_bl_sn(b,"mdsmap",e);
+	mon->store->get_bl_sn_safe(b,"mdsmap",e);
 	if (!b.length()) {
 	  r = -ENOENT;
 	} else {
diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc
index 79f257783e7..5edecfc14c8 100644
--- a/src/mon/Monitor.cc
+++ b/src/mon/Monitor.cc
@@ -362,7 +362,7 @@ int Monitor::check_features(MonitorStore *store)
   CompatSet ondisk;
 
   bufferlist features;
-  store->get_bl_ss(features, COMPAT_SET_LOC, 0);
+  store->get_bl_ss_safe(features, COMPAT_SET_LOC, 0);
   if (features.length() == 0) {
     generic_dout(0) << "WARNING: mon fs missing feature list.\n"
 	    << "Assuming it is old-style and introducing one." << dendl;
@@ -391,7 +391,7 @@ int Monitor::check_features(MonitorStore *store)
 void Monitor::read_features()
 {
   bufferlist bl;
-  store->get_bl_ss(bl, COMPAT_SET_LOC, 0);
+  store->get_bl_ss_safe(bl, COMPAT_SET_LOC, 0);
   assert(bl.length());
 
   bufferlist::iterator p = bl.begin();
@@ -497,7 +497,7 @@ int Monitor::preinit()
   if (authmon()->paxos->get_version() == 0) {
     dout(10) << "loading initial keyring to bootstrap authentication for mkfs" << dendl;
     bufferlist bl;
-    store->get_bl_ss(bl, "mkfs", "keyring");
+    store->get_bl_ss_safe(bl, "mkfs", "keyring");
     KeyRing keyring;
     bufferlist::iterator p = bl.begin();
     ::decode(keyring, p);
@@ -1032,12 +1032,14 @@ MMonProbe *Monitor::fill_probe_data(MMonProbe *m, Paxos *pax)
   version_t v = MAX(pax->get_first_committed(), m->newest_version + 1);
   int len = 0;
   for (; v <= pax->get_version(); v++) {
-    len += store->get_bl_sn(r->paxos_values[m->machine_name][v], m->machine_name.c_str(), v);
+    store->get_bl_sn_safe(r->paxos_values[m->machine_name][v], m->machine_name.c_str(), v);
+    len += r->paxos_values[m->machine_name][v].length();
     r->gv[m->machine_name][v] = store->get_global_version(m->machine_name.c_str(), v);
     for (list<string>::iterator p = pax->extra_state_dirs.begin();
          p != pax->extra_state_dirs.end();
          ++p) {
-      len += store->get_bl_sn(r->paxos_values[*p][v], p->c_str(), v);      
+      store->get_bl_sn_safe(r->paxos_values[*p][v], p->c_str(), v);
+      len += r->paxos_values[*p][v].length();
     }
     if (len >= g_conf->mon_slurp_bytes)
       break;
@@ -2358,7 +2360,8 @@ int Monitor::write_fsid()
 
   bufferlist b;
   b.append(us);
-  return store->put_bl_ss(b, "cluster_uuid", 0);
+  store->put_bl_ss(b, "cluster_uuid", 0);
+  return 0;
 }
 
 /*
@@ -2382,9 +2385,8 @@ int Monitor::mkfs(bufferlist& osdmapbl)
   bufferlist magicbl;
   magicbl.append(CEPH_MON_ONDISK_MAGIC);
   magicbl.append("\n");
-  r = store->put_bl_ss(magicbl, "magic", 0);
-  if (r < 0)
-    return r;
+  store->put_bl_ss(magicbl, "magic", 0);
+
 
   features = get_supported_features();
   write_features();
diff --git a/src/mon/MonitorStore.cc b/src/mon/MonitorStore.cc
index b9decedac9e..a74b750787c 100644
--- a/src/mon/MonitorStore.cc
+++ b/src/mon/MonitorStore.cc
@@ -35,7 +35,6 @@ static ostream& _prefix(std::ostream *_dout, const string& dir) {
   return *_dout << "store(" << dir << ") ";
 }
 
-
 #include <stdio.h>
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -89,7 +88,8 @@ int MonitorStore::mount()
 
 int MonitorStore::umount()
 {
-  ::close(lock_fd);
+  int close_err = TEMP_FAILURE_RETRY(::close(lock_fd));
+  assert (0 == close_err);
   return 0;
 }
 
@@ -110,7 +110,8 @@ int MonitorStore::mkfs()
     derr << "MonitorStore::mkfs: unable to open " << dir << ": " << cpp_strerror(err) << dendl;
     return err;
   }
-  ::close(fd);
+  int close_err = TEMP_FAILURE_RETRY(::close(fd));
+  assert (0 == close_err);
 
   dout(0) << "created monfs at " << dir << " for "
 	  << g_conf->name.get_id() << dendl;
@@ -134,6 +135,7 @@ version_t MonitorStore::get_int(const char *a, const char *b)
     }
     derr << "MonitorStore::get_int: failed to open '" << fn << "': "
 	 << cpp_strerror(err) << dendl;
+    assert(0 == "failed to open");
     return 0;
   }
   
@@ -143,10 +145,13 @@ version_t MonitorStore::get_int(const char *a, const char *b)
   if (r < 0) {
     derr << "MonitorStore::get_int: failed to read '" << fn << "': "
 	 << cpp_strerror(r) << dendl;
-    TEMP_FAILURE_RETRY(::close(fd));
+    int close_err = TEMP_FAILURE_RETRY(::close(fd));
+    assert(0 == close_err);
+    assert(0); // the file exists; so this is a different failure
     return 0;
   }
-  TEMP_FAILURE_RETRY(::close(fd));
+  int close_err = TEMP_FAILURE_RETRY(::close(fd));
+  assert (0 == close_err);
   
   version_t val = atoi(buf);
   
@@ -196,7 +201,12 @@ void MonitorStore::put_int(version_t val, const char *a, const char *b)
 	 << cpp_strerror(r) << dendl;
     ceph_abort();
   }
-  ::fsync(fd);
+  r = ::fsync(fd);
+  if (r) {
+    derr << "Monitor::put_int: failed to fsync fd for '" << tfn << "': "
+	 << cpp_strerror(r) << dendl;
+    ceph_abort();
+  }
   if (TEMP_FAILURE_RETRY(::close(fd))) {
     derr << "MonitorStore::put_int: failed to close fd for '" << tfn << "': "
 	 << cpp_strerror(r) << dendl;
@@ -245,10 +255,13 @@ bool MonitorStore::exists_bl_ss(const char *a, const char *b)
   int r = ::stat(fn, &st);
   //char buf[80];
   //dout(15) << "exists_bl stat " << fn << " r=" << r << " errno " << errno << " " << strerror_r(errno, buf, sizeof(buf)) << dendl;
+  if (r) {
+    assert (errno == ENOENT);
+  }
   return r == 0;
 }
 
-int MonitorStore::erase_ss(const char *a, const char *b)
+void MonitorStore::erase_ss(const char *a, const char *b)
 {
   char fn[1024];
   char dr[1024];
@@ -261,6 +274,7 @@ int MonitorStore::erase_ss(const char *a, const char *b)
     strcpy(fn, dr);
   }
   int r = ::unlink(fn);
+  assert(0 == r || ENOENT == errno); // callers don't check for existence first
 
   if (b) {
     // wipe out _gv file too, if any.  this is sloppy, but will work.
@@ -270,7 +284,6 @@ int MonitorStore::erase_ss(const char *a, const char *b)
   }
 
   ::rmdir(dr);  // sloppy attempt to clean up empty dirs
-  return r;
 }
 
 int MonitorStore::get_bl_ss(bufferlist& bl, const char *a, const char *b)
@@ -314,7 +327,8 @@ int MonitorStore::get_bl_ss(bufferlist& bl, const char *a, const char *b)
     off += r;
   }
   bl.append(bp);
-  ::close(fd);
+  int close_err = TEMP_FAILURE_RETRY(::close(fd));
+  assert (0 == close_err);
 
   if (b) {
     dout(15) << "get_bl " << a << "/" << b << " = " << bl.length() << " bytes" << dendl;
@@ -325,7 +339,7 @@ int MonitorStore::get_bl_ss(bufferlist& bl, const char *a, const char *b)
   return len;
 }
 
-int MonitorStore::write_bl_ss_impl(bufferlist& bl, const char *a, const char *b, bool append)
+void MonitorStore::write_bl_ss(bufferlist& bl, const char *a, const char *b, bool append)
 {
   int err = 0;
   char fn[1024];
@@ -336,7 +350,7 @@ int MonitorStore::write_bl_ss_impl(bufferlist& bl, const char *a, const char *b,
       err = -errno;
       derr << __func__ << " failed to create dir " << fn
 	   << ": " << cpp_strerror(err) << dendl;
-      return err;
+      assert(0 == "failed to create dir");
     }
     dout(15) << "put_bl " << a << "/" << b << " = " << bl.length() << " bytes" << dendl;
     snprintf(fn, sizeof(fn), "%s/%s/%s", dir.c_str(), a, b);
@@ -352,7 +366,7 @@ int MonitorStore::write_bl_ss_impl(bufferlist& bl, const char *a, const char *b,
       err = -errno;
       derr << "failed to open " << fn << "for append: "
 	   << cpp_strerror(err) << dendl;
-      return err;
+      assert(0 == "failed to open for append");
     }
   } else {
     snprintf(tfn, sizeof(tfn), "%s.new", fn);
@@ -360,42 +374,34 @@ int MonitorStore::write_bl_ss_impl(bufferlist& bl, const char *a, const char *b,
     if (fd < 0) {
       err = -errno;
       derr << "failed to open " << tfn << ": " << cpp_strerror(err) << dendl;
-      return err;
+      assert(0 == "failed to open");
     }
   }
   
   err = bl.write_fd(fd);
-
-  if (!err)
-    ::fsync(fd);
-  ::close(fd);
-  if (!append && !err) {
-    int r = ::rename(tfn, fn);
-    if (r < 0) {
+  assert(!err);
+  err = ::fsync(fd);
+  assert(!err);
+  err = TEMP_FAILURE_RETRY(::close(fd));
+  assert (!err); // this really can't fail, right? right?...
+  if (!append) {
+    err = ::rename(tfn, fn);
+    if (err < 0) {
       err = -errno;
       derr << __func__ << " failed to rename '" << tfn << "' -> '"
 	   << fn << "': " << cpp_strerror(err) << dendl;
-      return err;
+      assert(0 == "failed to rename");
     }
   }
-
-  return err;
-}
-
-int MonitorStore::write_bl_ss(bufferlist& bl, const char *a, const char *b, bool append)
-{
-  int err = write_bl_ss_impl(bl, a, b, append);
-  if (err)
-    derr << "write_bl_ss " << a << "/" << b << " got error " << cpp_strerror(err) << dendl;
-  assert(!err);  // for now
-  return 0;
 }
 
-int MonitorStore::put_bl_sn_map(const char *a,
+void MonitorStore::put_bl_sn_map(const char *a,
 				map<version_t,bufferlist>::iterator start,
 				map<version_t,bufferlist>::iterator end,
 				map<version_t,version_t> *gvmap)
 {
+  int err = 0;
+  int close_err = 0;
   version_t first = start->first;
   map<version_t,bufferlist>::iterator lastp = end;
   --lastp;
@@ -407,13 +413,11 @@ int MonitorStore::put_bl_sn_map(const char *a,
       last - first < (unsigned)g_conf->mon_sync_fs_threshold) {
     // just do them individually
     for (map<version_t,bufferlist>::iterator p = start; p != end; ++p) {
-      int err = put_bl_sn(p->second, a, p->first);
-      if (err < 0)
-	return err;
+      put_bl_sn(p->second, a, p->first);
       if (gvmap && gvmap->count(p->first) && (*gvmap)[p->first] > 0)
 	put_global_version(a, p->first, (*gvmap)[p->first]);
     }
-    return 0;
+    return;
   }
 
   // make sure dir exists
@@ -421,10 +425,10 @@ int MonitorStore::put_bl_sn_map(const char *a,
   snprintf(dfn, sizeof(dfn), "%s/%s", dir.c_str(), a);
   int r = ::mkdir(dfn, 0755);
   if ((r < 0) && (errno != EEXIST)) {
-    int err = -errno;
+    err = -errno;
     derr << __func__ << " failed to create dir " << dfn << ": "
 	 << cpp_strerror(err) << dendl;
-    return err;
+    assert(0 == "failed to create dir");
   }
 
   for (map<version_t,bufferlist>::iterator p = start; p != end; ++p) {
@@ -437,13 +441,14 @@ int MonitorStore::put_bl_sn_map(const char *a,
     if (fd < 0) {
       int err = -errno;
       derr << "failed to open " << tfn << ": " << cpp_strerror(err) << dendl;
-      return err;
+      assert(0 == "failed to open");
     }
 
-    int err = p->second.write_fd(fd);
-    ::close(fd);
+    err = p->second.write_fd(fd);
+    close_err = TEMP_FAILURE_RETRY(::close(fd));
+    assert (0 == close_err);
     if (err < 0)
-      return -errno;
+      assert(0 == "failed to write");
 
     // this doesn't try to be efficient.. too bad for you!  it may also
     // extend beyond commmitted, but that's okay; we only look at these
@@ -455,12 +460,13 @@ int MonitorStore::put_bl_sn_map(const char *a,
   // sync them all
   int dirfd = ::open(dir.c_str(), O_RDONLY);
   if (dirfd < 0) {
-    int err = -errno;
+    err = -errno;
     derr << "failed to open " << dir << ": " << cpp_strerror(err) << dendl;
-    return err;
+    assert(0 == "failed to open temp file");
   }
   sync_filesystem(dirfd);
-  ::close(dirfd);
+  close_err = TEMP_FAILURE_RETRY(::close(dirfd));
+  assert (0 == close_err);
     
   // rename them all into place
   for (map<version_t,bufferlist>::iterator p = start; p != end; ++p) {
@@ -469,23 +475,28 @@ int MonitorStore::put_bl_sn_map(const char *a,
     snprintf(fn, sizeof(fn), "%s/%llu", dfn, (long long unsigned)p->first);
     snprintf(tfn, sizeof(tfn), "%s.new", fn);
     
-    int err = ::rename(tfn, fn);
+    err = ::rename(tfn, fn);
     if (err < 0)
-      return -errno;
+      assert(0 == "failed to rename");
   }
     
   // fsync the dir (to commit the renames)
   dirfd = ::open(dir.c_str(), O_RDONLY);
   if (dirfd < 0) {
-    int err = -errno;
+    err = -errno;
     derr << __func__ << " failed to open " << dir
 	 << ": " << cpp_strerror(err) << dendl;
-    return err;
+    assert(0 == "failed to open dir");
   }
-  ::fsync(dirfd);
-  ::close(dirfd);
-
-  return 0;
+  err = ::fsync(dirfd);
+  if (err < 0) {
+    err = -errno;
+    derr << __func__ << " failed to fsync " << dir
+	 << ": " << cpp_strerror(err) << dendl;
+    assert(0 == "failed to fsync");
+  }
+  close_err = TEMP_FAILURE_RETRY(::close(dirfd));
+  assert (0 == close_err);
 }
 
 void MonitorStore::sync()
@@ -495,8 +506,9 @@ void MonitorStore::sync()
     int err = -errno;
     derr << __func__ << " failed to open " << dir
 	 << ": " << cpp_strerror(err) << dendl;
-    return;
+    assert(0 == "failed to open dir for syncing");
   }
   sync_filesystem(dirfd);
-  ::close(dirfd);
+  int close_err = TEMP_FAILURE_RETRY(::close(dirfd));
+  assert (0 == close_err);
 }
diff --git a/src/mon/MonitorStore.h b/src/mon/MonitorStore.h
index cf748360456..9d1efde3d53 100644
--- a/src/mon/MonitorStore.h
+++ b/src/mon/MonitorStore.h
@@ -18,16 +18,17 @@
 #include "include/types.h"
 #include "include/buffer.h"
 
+#include "common/compiler_extensions.h"
+
 #include <iosfwd>
 #include <string.h>
+#include <errno.h>
 
 class MonitorStore {
   string dir;
   int lock_fd;
 
-  int write_bl_ss_impl(bufferlist& bl, const char *a, const char *b,
-		       bool append);
-  int write_bl_ss(bufferlist& bl, const char *a, const char *b,
+  void write_bl_ss(bufferlist& bl, const char *a, const char *b,
 		  bool append);
 public:
   MonitorStore(const std::string &d) : dir(d), lock_fd(-1) { }
@@ -39,36 +40,44 @@ public:
   void sync();
 
   // ints (stored as ascii)
-  version_t get_int(const char *a, const char *b=0);
+  version_t get_int(const char *a, const char *b=0) WARN_UNUSED_RESULT;
   void put_int(version_t v, const char *a, const char *b=0);
 
-  version_t get_global_version(const char *a, version_t b);
+  version_t get_global_version(const char *a, version_t b) WARN_UNUSED_RESULT;
   void put_global_version(const char *a, version_t b, version_t gv);
 
   // buffers
   // ss and sn varieties.
   bool exists_bl_ss(const char *a, const char *b=0);
-  int get_bl_ss(bufferlist& bl, const char *a, const char *b);
-  int put_bl_ss(bufferlist& bl, const char *a, const char *b) {
-    return write_bl_ss(bl, a, b, false);
+  int get_bl_ss(bufferlist& bl, const char *a, const char *b) WARN_UNUSED_RESULT;
+  void get_bl_ss_safe(bufferlist& bl, const char *a, const char *b) {
+    int ret = get_bl_ss(bl, a, b);
+    assert (ret >= 0 || ret == -ENOENT);
+  }
+  void put_bl_ss(bufferlist& bl, const char *a, const char *b) {
+    write_bl_ss(bl, a, b, false);
   }
-  int append_bl_ss(bufferlist& bl, const char *a, const char *b) {
-    return write_bl_ss(bl, a, b, true);
+  void append_bl_ss(bufferlist& bl, const char *a, const char *b) {
+    write_bl_ss(bl, a, b, true);
   }
   bool exists_bl_sn(const char *a, version_t b) {
     char bs[20];
     snprintf(bs, sizeof(bs), "%llu", (unsigned long long)b);
     return exists_bl_ss(a, bs);
   }
-  int get_bl_sn(bufferlist& bl, const char *a, version_t b) {
+  int get_bl_sn(bufferlist& bl, const char *a, version_t b) WARN_UNUSED_RESULT {
     char bs[20];
     snprintf(bs, sizeof(bs), "%llu", (unsigned long long)b);
     return get_bl_ss(bl, a, bs);
   }
-  int put_bl_sn(bufferlist& bl, const char *a, version_t b) {
+  void get_bl_sn_safe(bufferlist& bl, const char *a, version_t b) {
+    int ret = get_bl_sn(bl, a, b);
+    assert(ret >= 0 || ret == -ENOENT);
+  }
+  void put_bl_sn(bufferlist& bl, const char *a, version_t b) {
     char bs[20];
     snprintf(bs, sizeof(bs), "%llu", (unsigned long long)b);
-    return put_bl_ss(bl, a, bs);
+    put_bl_ss(bl, a, bs);
   }
   /**
    * Put a whole set of values efficiently and safely.
@@ -77,16 +86,16 @@ public:
    * @param vals - map of int name -> values
    * @return 0 for success or negative error code
    */
-  int put_bl_sn_map(const char *a,
+  void put_bl_sn_map(const char *a,
 		    map<version_t,bufferlist>::iterator start,
 		    map<version_t,bufferlist>::iterator end,
 		    map<version_t,version_t> *gvmap);
 
-  int erase_ss(const char *a, const char *b);
-  int erase_sn(const char *a, version_t b) {
+  void erase_ss(const char *a, const char *b);
+  void erase_sn(const char *a, version_t b) {
     char bs[20];
     snprintf(bs, sizeof(bs), "%llu", (unsigned long long)b);
-    return erase_ss(a, bs);
+    erase_ss(a, bs);
   }
 
   /*
diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index e72929b7d43..94e08b87088 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -75,7 +75,7 @@ void OSDMonitor::create_initial()
   OSDMap newmap;
 
   bufferlist bl;
-  mon->store->get_bl_ss(bl, "mkfs", "osdmap");
+  mon->store->get_bl_ss_safe(bl, "mkfs", "osdmap");
   if (bl.length()) {
     newmap.decode(bl);
     newmap.set_fsid(mon->monmap->fsid);
@@ -1309,7 +1309,8 @@ void OSDMonitor::send_incremental(PaxosServiceMessage *req, epoch_t first)
   if (first < paxos->get_first_committed()) {
     first = paxos->get_first_committed();
     bufferlist bl;
-    mon->store->get_bl_sn(bl, "osdmap_full", first);
+    mon->store->get_bl_sn_safe(bl, "osdmap_full", first);
+    assert(bl.length());
     dout(20) << "send_incremental starting with base full " << first << " " << bl.length() << " bytes" << dendl;
     MOSDMap *m = new MOSDMap(osdmap.get_fsid());
     m->oldest_map = paxos->get_first_committed();
@@ -1336,7 +1337,8 @@ void OSDMonitor::send_incremental(epoch_t first, entity_inst_t& dest, bool oneti
   if (first < paxos->get_first_committed()) {
     first = paxos->get_first_committed();
     bufferlist bl;
-    mon->store->get_bl_sn(bl, "osdmap_full", first);
+    mon->store->get_bl_sn_safe(bl, "osdmap_full", first);
+    assert(bl.length());
     dout(20) << "send_incremental starting with base full " << first << " " << bl.length() << " bytes" << dendl;
     MOSDMap *m = new MOSDMap(osdmap.get_fsid());
     m->oldest_map = paxos->get_first_committed();
@@ -1701,7 +1703,7 @@ bool OSDMonitor::preprocess_command(MMonCommand *m)
       OSDMap *p = &osdmap;
       if (epoch) {
 	bufferlist b;
-	mon->store->get_bl_sn(b,"osdmap_full", epoch);
+	mon->store->get_bl_sn_safe(b,"osdmap_full", epoch);
 	if (!b.length()) {
 	  p = 0;
 	  r = -ENOENT;
diff --git a/src/mon/PGMonitor.cc b/src/mon/PGMonitor.cc
index dcf9380613b..6a6dd60b84b 100644
--- a/src/mon/PGMonitor.cc
+++ b/src/mon/PGMonitor.cc
@@ -579,7 +579,7 @@ void PGMonitor::check_osd_map(epoch_t epoch)
        e++) {
     dout(10) << "check_osd_map applying osdmap e" << e << " to pg_map" << dendl;
     bufferlist bl;
-    mon->store->get_bl_sn(bl, "osdmap", e);
+    mon->store->get_bl_sn_safe(bl, "osdmap", e);
     assert(bl.length());
     OSDMap::Incremental inc(bl);
     for (map<int32_t,uint32_t>::iterator p = inc.new_weight.begin();
diff --git a/src/mon/Paxos.cc b/src/mon/Paxos.cc
index 349908789e3..fbe141e3a84 100644
--- a/src/mon/Paxos.cc
+++ b/src/mon/Paxos.cc
@@ -73,7 +73,8 @@ void Paxos::collect(version_t oldpn)
   if (mon->store->exists_bl_sn(machine_name, last_committed+1)) {
     uncommitted_v = last_committed+1;
     uncommitted_pn = accepted_pn;
-    mon->store->get_bl_sn(uncommitted_value, machine_name, last_committed+1);
+    mon->store->get_bl_sn_safe(uncommitted_value, machine_name, last_committed+1);
+    assert(uncommitted_value.length());
     dout(10) << "learned uncommitted " << (last_committed+1)
 	     << " (" << uncommitted_value.length() << " bytes) from myself" 
 	     << dendl;
@@ -146,7 +147,7 @@ void Paxos::handle_collect(MMonPaxos *collect)
   //  (it'll be at last_committed+1)
   bufferlist bl;
   if (mon->store->exists_bl_sn(machine_name, last_committed+1)) {
-    mon->store->get_bl_sn(bl, machine_name, last_committed+1);
+    mon->store->get_bl_sn_safe(bl, machine_name, last_committed+1);
     assert(bl.length() > 0);
     dout(10) << " sharing our accepted but uncommitted value for " 
 	     << last_committed+1 << " (" << bl.length() << " bytes)" << dendl;
@@ -184,7 +185,8 @@ void Paxos::share_state(MMonPaxos *m, version_t peer_first_committed,
   // include incrementals
   for ( ; v <= last_committed; v++) {
     if (mon->store->exists_bl_sn(machine_name, v)) {
-      mon->store->get_bl_sn(m->values[v], machine_name, v);
+      mon->store->get_bl_sn_safe(m->values[v], machine_name, v);
+      assert(m->values[v].length());
       m->gv[v] = mon->store->get_global_version(machine_name, v);
       dout(10) << " sharing " << v << " (" 
 	       << m->values[v].length() << " bytes)" << dendl;
@@ -968,7 +970,7 @@ bool Paxos::is_readable(version_t v)
 
 bool Paxos::read(version_t v, bufferlist &bl)
 {
-  if (!mon->store->get_bl_sn(bl, machine_name, v))
+  if (mon->store->get_bl_sn(bl, machine_name, v) <= 0)
     return false;
   return true;
 }
@@ -1051,7 +1053,8 @@ void Paxos::stash_latest(version_t v, bufferlist& bl)
 version_t Paxos::get_stashed(bufferlist& bl)
 {
   bufferlist full;
-  if (mon->store->get_bl_ss(full, machine_name, "latest") <= 0) {
+  mon->store->get_bl_ss_safe(full, machine_name, "latest");
+  if (!full.length()) {
     dout(10) << "get_stashed not found" << dendl;
     return 0;
   }