mon: do not reopen MonitorDBStore during startup

level doesn't seem to like this when it races with an internal compaction
attempt (see below).  Instead, let the store get opened by the ceph_mon
caller, and pull a bit of the logic into the caller to make the flow a
little easier to follow.

    -2> 2013-06-25 17:49:25.184490 7f4d439f8780 10 needs_conversion
    -1> 2013-06-25 17:49:25.184495 7f4d4065c700  5 asok(0x13b1460) entry start
     0> 2013-06-25 17:49:25.316908 7f4d3fe5b700 -1 *** Caught signal (Segmentation fault) **
 in thread 7f4d3fe5b700

 ceph version 0.64-667-g089cba8 (089cba8fc0e8ae8aef9a3111cba7342ecd0f8314)
 1: ceph-mon() [0x649f0a]
 2: (()+0xfcb0) [0x7f4d435dccb0]
 3: (leveldb::Table::BlockReader(void*, leveldb::ReadOptions const&, leveldb::Slice const&)+0x154) [0x806e54]
 4: ceph-mon() [0x808840]
 5: ceph-mon() [0x808b39]
 6: ceph-mon() [0x806540]
 7: (leveldb::DBImpl::DoCompactionWork(leveldb::DBImpl::CompactionState*)+0xdd) [0x7f363d]
 8: (leveldb::DBImpl::BackgroundCompaction()+0x2c0) [0x7f4210]
 9: (leveldb::DBImpl::BackgroundCall()+0x68) [0x7f4cc8]
 10: ceph-mon() [0x80b3af]
 11: (()+0x7e9a) [0x7f4d435d4e9a]
 12: (clone()+0x6d) [0x7f4d4196bccd]
 NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.

Signed-off-by: Sage Weil <sage@inktank.com>

src/ceph_mon.cc

@@ -289,26 +289,36 @@ int main(int argc, const char **argv)
   common_init_finish(g_ceph_context);
   global_init_chdir(g_ceph_context);
 
-  {
-    Monitor::StoreConverter converter(g_conf->mon_data);
-    int ret = converter.needs_conversion();
-    if (ret > 0) {
-      assert(!converter.convert());
-    } else if (ret < 0) {
-      derr << "found errors while attempting to convert the monitor store: "
+  MonitorDBStore *store = new MonitorDBStore(g_conf->mon_data);
+
+  Monitor::StoreConverter converter(g_conf->mon_data, store);
+  if (store->open(std::cerr) < 0) {
+    int ret = store->create_and_open(std::cerr);
+    if (ret < 0) {
+      derr << "failed to create new leveldb store" << dendl;
+      prefork.exit(1);
+    }
+
+    ret = converter.needs_conversion();
+    if (ret < 0) {
+      derr << "found errors while validating legacy unconverted monitor store: "
            << cpp_strerror(ret) << dendl;
       prefork.exit(1);
     }
-  }
-
-  MonitorDBStore *store = new MonitorDBStore(g_conf->mon_data);
-  err = store->open(std::cerr);
-  if (err < 0) {
-    cerr << argv[0] << ": error opening mon data store at '"
-         << g_conf->mon_data << "': " << cpp_strerror(err) << std::endl;
+    if (ret > 0) {
+      cout << "converting monitor store, please do not interrupt..." << std::endl;
+      int r = converter.convert();
+      if (r) {
+	derr << "failed to convert monitor store: " << cpp_strerror(r) << dendl;
+	prefork.exit(1);
+      }
+    }
+  } else if (converter.is_converting()) {
+    derr << "there is an on-going (maybe aborted?) conversion." << dendl;
+    derr << "you should check what happened" << dendl;
+    derr << "remove store.db to restart conversion" << dendl;
     prefork.exit(1);
   }
-  assert(err == 0);
 
   bufferlist magicbl;
   err = store->get(Monitor::MONITOR_NAME, "magic", magicbl);

src/mon/Monitor.cc

@@ -4341,46 +4341,39 @@ bool Monitor::StoreConverter::_check_gv_store()
 
 int Monitor::StoreConverter::needs_conversion()
 {
+  bufferlist magicbl;
   int ret = 0;
 
   dout(10) << __func__ << dendl;
   _init();
-  if (db->open(std::cerr) < 0) {
-    dout(1) << "unable to open monitor store at " << g_conf->mon_data << dendl;
-    dout(1) << "check for old monitor store format" << dendl;
-    int err = store->mount();
-    if (err < 0) {
-      if (err == -ENOENT) {
-        derr << "unable to mount monitor store: "
-             << cpp_strerror(err) << dendl;
-      } else {
-        derr << "it appears that another monitor is running: "
-             << cpp_strerror(err) << dendl;
-      }
-      ret = err;
-      goto out;
+
+  int err = store->mount();
+  if (err < 0) {
+    if (err == -ENOENT) {
+      derr << "unable to mount monitor store: "
+	   << cpp_strerror(err) << dendl;
+    } else {
+      derr << "it appears that another monitor is running: "
+	   << cpp_strerror(err) << dendl;
     }
-    assert(err == 0);
-    bufferlist magicbl;
-    if (store->exists_bl_ss("magic", 0)) {
-      if (_check_gv_store()) {
-	dout(1) << "found old GV monitor store format "
-		<< "-- should convert!" << dendl;
-	ret = 1;
-      } else {
-	dout(0) << "Existing monitor store has not been converted "
-		<< "to 0.52 (bobtail) format" << dendl;
-	assert(0 == "Existing store has not been converted to 0.52 format");
-      }
-    }
-    assert(!store->umount());
-  } else {
-    if (db->exists("mon_convert", "on_going")) {
-      ret = -EEXIST;
-      derr << "there is an on-going (maybe aborted?) conversion." << dendl;
-      derr << "you should check what happened" << dendl;
+    ret = err;
+    goto out;
+  }
+  assert(err == 0);
+
+  if (store->exists_bl_ss("magic", 0)) {
+    if (_check_gv_store()) {
+      dout(1) << "found old GV monitor store format "
+	      << "-- should convert!" << dendl;
+      ret = 1;
+    } else {
+      dout(0) << "Existing monitor store has not been converted "
+	      << "to 0.52 (bobtail) format" << dendl;
+      assert(0 == "Existing store has not been converted to 0.52 format");
     }
   }
+  assert(!store->umount());
+
 out:
   _deinit();
   return ret;

src/mon/Monitor.h

@@ -1447,7 +1447,7 @@ private:
 public:
   class StoreConverter {
     const string path;
-    boost::scoped_ptr<MonitorDBStore> db;
+    MonitorDBStore *db;
     boost::scoped_ptr<MonitorStore> store;
 
     set<version_t> gvs;
@@ -1457,8 +1457,8 @@ public:
     version_t highest_accepted_pn;
 
    public:
-    StoreConverter(const string &path)
-      : path(path), db(NULL), store(NULL),
+    StoreConverter(string path, MonitorDBStore *d)
+      : path(path), db(d), store(NULL),
 	highest_last_pn(0), highest_accepted_pn(0)
     { }
 
@@ -1471,20 +1471,20 @@ public:
     int needs_conversion();
     int convert();
 
+    bool is_converting() {
+      return db->exists("mon_convert", "on_going");
+    }
+
    private:
 
     bool _check_gv_store();
 
     void _init() {
-      MonitorDBStore *db_ptr = new MonitorDBStore(path);
-      db.reset(db_ptr);
-
       MonitorStore *store_ptr = new MonitorStore(path);
       store.reset(store_ptr);
     }
 
     void _deinit() {
-      db.reset(NULL);
       store.reset(NULL);
     }