osd: kill META_COLL constant; use named ctor

Note: this kills a subtle bug *somewhere* where meta coll_t's pgid field
is 0.0 vs 0.0s0 (it appears the static META_COLL gets 0.0s0).  Statics are
bad news so kill.

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

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 3fe505a6a24161fc082ec033980b77b108bc97b8..17367e820424a1aad4c18d47084a34c0d6f312ea 100644 (file)
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -140,8 +140,6 @@
 #define tracepoint(...)
 #endif
 
-static coll_t META_COLL;
-
 #define dout_subsys ceph_subsys_osd
 #undef dout_prefix
 #define dout_prefix _prefix(_dout, whoami, get_osdmap_epoch())
@@ -1101,8 +1099,8 @@ bool OSDService::_get_map_bl(epoch_t e, bufferlist& bl)
   bool found = map_bl_cache.lookup(e, &bl);
   if (found)
     return true;
-  found = store->read(
-    META_COLL, OSD::get_osdmap_pobject_name(e), 0, 0, bl) >= 0;
+  found = store->read(coll_t::meta(),
+                     OSD::get_osdmap_pobject_name(e), 0, 0, bl) >= 0;
   if (found)
     _add_map_bl(e, bl);
   return found;
@@ -1114,8 +1112,8 @@ bool OSDService::get_inc_map_bl(epoch_t e, bufferlist& bl)
   bool found = map_bl_inc_cache.lookup(e, &bl);
   if (found)
     return true;
-  found = store->read(
-    META_COLL, OSD::get_inc_osdmap_pobject_name(e), 0, 0, bl) >= 0;
+  found = store->read(coll_t::meta(),
+                     OSD::get_inc_osdmap_pobject_name(e), 0, 0, bl) >= 0;
   if (found)
     _add_map_inc_bl(e, bl);
   return found;
@@ -1323,7 +1321,7 @@ int OSD::mkfs(CephContext *cct, ObjectStore *store, const string &dev,
 
     OSDSuperblock sb;
     bufferlist sbbl;
-    ret = store->read(META_COLL, OSD_SUPERBLOCK_POBJECT, 0, 0, sbbl);
+    ret = store->read(coll_t::meta(), OSD_SUPERBLOCK_POBJECT, 0, 0, sbbl);
     if (ret >= 0) {
       dout(0) << " have superblock" << dendl;
       if (whoami != sb.whoami) {
@@ -1353,8 +1351,8 @@ int OSD::mkfs(CephContext *cct, ObjectStore *store, const string &dev,
       ::encode(sb, bl);
 
       ObjectStore::Transaction t;
-      t.create_collection(META_COLL);
-      t.write(META_COLL, OSD_SUPERBLOCK_POBJECT, 0, bl.length(), bl);
+      t.create_collection(coll_t::meta());
+      t.write(coll_t::meta(), OSD_SUPERBLOCK_POBJECT, 0, bl.length(), bl);
       ret = store->apply_transaction(t);
       if (ret) {
        derr << "OSD::mkfs: error while writing OSD_SUPERBLOCK_POBJECT: "
@@ -1810,10 +1808,10 @@ int OSD::init()
   }
 
   // make sure snap mapper object exists
-  if (!store->exists(META_COLL, OSD::make_snapmapper_oid())) {
+  if (!store->exists(coll_t::meta(), OSD::make_snapmapper_oid())) {
     dout(10) << "init creating/touching snapmapper object" << dendl;
     ObjectStore::Transaction t;
-    t.touch(META_COLL, OSD::make_snapmapper_oid());
+    t.touch(coll_t::meta(), OSD::make_snapmapper_oid());
     r = store->apply_transaction(t);
     if (r < 0)
       goto out;
@@ -2443,13 +2441,13 @@ void OSD::write_superblock(ObjectStore::Transaction& t)
 
   bufferlist bl;
   ::encode(superblock, bl);
-  t.write(META_COLL, OSD_SUPERBLOCK_POBJECT, 0, bl.length(), bl);
+  t.write(coll_t::meta(), OSD_SUPERBLOCK_POBJECT, 0, bl.length(), bl);
 }
 
 int OSD::read_superblock()
 {
   bufferlist bl;
-  int r = store->read(META_COLL, OSD_SUPERBLOCK_POBJECT, 0, 0, bl);
+  int r = store->read(coll_t::meta(), OSD_SUPERBLOCK_POBJECT, 0, 0, bl);
   if (r < 0)
     return r;
 
@@ -2867,10 +2865,10 @@ void OSD::load_pgs()
   }
 
   // clean up old infos object?
-  if (has_upgraded && store->exists(META_COLL, OSD::make_infos_oid())) {
+  if (has_upgraded && store->exists(coll_t::meta(), OSD::make_infos_oid())) {
     dout(1) << __func__ << " removing legacy infos object" << dendl;
     ObjectStore::Transaction t;
-    t.remove(META_COLL, OSD::make_infos_oid());
+    t.remove(coll_t::meta(), OSD::make_infos_oid());
     int r = store->apply_transaction(t);
     if (r != 0) {
       derr << __func__ << ": apply_transaction returned "
@@ -5044,9 +5042,9 @@ void OSD::do_command(Connection *con, ceph_tid_t tid, vector<string>& cmd, buffe
       object_t oid(nm);
       hobject_t soid(sobject_t(oid, 0));
       ObjectStore::Transaction *t = new ObjectStore::Transaction;
-      t->write(META_COLL, soid, 0, bsize, bl);
+      t->write(coll_t::meta(), soid, 0, bsize, bl);
       store->queue_transaction_and_cleanup(NULL, t);
-      cleanupt->remove(META_COLL, soid);
+      cleanupt->remove(coll_t::meta(), soid);
     }
     store->sync_and_flush();
     utime_t end = ceph_clock_now(cct);
@@ -6071,7 +6069,7 @@ void OSD::handle_osd_map(MOSDMap *m)
        last_marked_full = e;
 
       hobject_t fulloid = get_osdmap_pobject_name(e);
-      t.write(META_COLL, fulloid, 0, bl.length(), bl);
+      t.write(coll_t::meta(), fulloid, 0, bl.length(), bl);
       pin_map_bl(e, bl);
       pinned_maps.push_back(add_map(o));
       continue;
@@ -6082,7 +6080,7 @@ void OSD::handle_osd_map(MOSDMap *m)
       dout(10) << "handle_osd_map  got inc map for epoch " << e << dendl;
       bufferlist& bl = p->second;
       hobject_t oid = get_inc_osdmap_pobject_name(e);
-      t.write(META_COLL, oid, 0, bl.length(), bl);
+      t.write(coll_t::meta(), oid, 0, bl.length(), bl);
       pin_map_inc_bl(e, bl);
 
       OSDMap *o = new OSDMap;
@@ -6128,7 +6126,7 @@ void OSD::handle_osd_map(MOSDMap *m)
 
 
       hobject_t fulloid = get_osdmap_pobject_name(e);
-      t.write(META_COLL, fulloid, 0, fbl.length(), fbl);
+      t.write(coll_t::meta(), fulloid, 0, fbl.length(), fbl);
       pin_map_bl(e, fbl);
       pinned_maps.push_back(add_map(o));
       continue;
@@ -6154,8 +6152,8 @@ void OSD::handle_osd_map(MOSDMap *m)
          service.map_cache.cached_key_lower_bound()));
     for (epoch_t e = superblock.oldest_map; e < min; ++e) {
       dout(20) << " removing old osdmap epoch " << e << dendl;
-      t.remove(META_COLL, get_osdmap_pobject_name(e));
-      t.remove(META_COLL, get_inc_osdmap_pobject_name(e));
+      t.remove(coll_t::meta(), get_osdmap_pobject_name(e));
+      t.remove(coll_t::meta(), get_inc_osdmap_pobject_name(e));
       superblock.oldest_map = e+1;
       num++;
       if (num >= cct->_conf->osd_target_transaction_size &&

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 767f607ec322d56e3332ef494e0431afcb8709e8..17e7ab93ce92df05d081345d1bc52b6df9b682d9 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -56,8 +56,6 @@
 #undef dout_prefix
 #define dout_prefix _prefix(_dout, this)
 
-static coll_t META_COLL;
-
 // prefix pgmeta_oid keys with _ so that PGLog::read_log() can
 // easily skip them
 const string infover_key("_infover");
@@ -2606,8 +2604,8 @@ void PG::upgrade(ObjectStore *store)
   pg_log.mark_log_for_rewrite();
   hobject_t log_oid(OSD::make_pg_log_oid(pg_id));
   hobject_t biginfo_oid(OSD::make_pg_biginfo_oid(pg_id));
-  t.remove(META_COLL, log_oid);
-  t.remove(META_COLL, biginfo_oid);
+  t.remove(coll_t::meta(), log_oid);
+  t.remove(coll_t::meta(), biginfo_oid);
   t.collection_rmattr(coll, "info");
 
   t.touch(coll, pgmeta_oid);
@@ -2778,7 +2776,7 @@ epoch_t PG::peek_map_epoch(ObjectStore *store,
     keys.clear();
     values.clear();
     keys.insert(ek);
-    store->omap_get_values(META_COLL, legacy_infos_oid, keys, &values);
+    store->omap_get_values(coll_t::meta(), legacy_infos_oid, keys, &values);
     assert(values.size() == 1);
     bufferlist::iterator p = values[ek].begin();
     ::decode(cur_epoch, p);
@@ -2966,7 +2964,7 @@ int PG::read_info(
   keys.insert(k);
   keys.insert(bk);
   values.clear();
-  store->omap_get_values(META_COLL, infos_oid, keys, &values);
+  store->omap_get_values(coll_t::meta(), infos_oid, keys, &values);
   assert(values.size() == 2);
 
   p = values[k].begin();
@@ -2989,7 +2987,7 @@ void PG::read_state(ObjectStore *store, bufferlist &bl)
   ostringstream oss;
   pg_log.read_log(store,
                  coll,
-                 info_struct_v < 8 ? META_COLL : coll,
+                 info_struct_v < 8 ? coll_t::meta() : coll,
                  info_struct_v < 8 ? OSD::make_pg_log_oid(pg_id) : pgmeta_oid,
                  info, oss);
   if (oss.str().length())

diff --git a/src/osd/PGLog.cc b/src/osd/PGLog.cc
index 657d32450dcc81fea1d565104a303475ecf158b3..0347a779e9e59ec1149809249f73b23fdd7c2d9b 100644 (file)
--- a/src/osd/PGLog.cc
+++ b/src/osd/PGLog.cc
@@ -22,8 +22,6 @@
 
 #define dout_subsys ceph_subsys_osd
 
-static coll_t META_COLL;
-
 //////////////////// PGLog::IndexedLog ////////////////////
 
 void PGLog::IndexedLog::advance_rollback_info_trimmed_to(

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index de39af2bf95b413627c87020549574bd4461634a..2ccbcd12670df0268619828cd121af87585f7c6e 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -515,6 +515,14 @@ public:
     calc_str();
   }
 
+  // named constructors
+  static coll_t meta() {
+    return coll_t();
+  }
+  static coll_t pg(spg_t p) {
+    return coll_t(p);
+  }
+
   const std::string& to_str() const {
     return _str;
   }

diff --git a/src/test/objectstore/TestObjectStoreState.cc b/src/test/objectstore/TestObjectStoreState.cc
index 4a16026981744c511675fa9c531dd847b527395a..a52227f87c4825accec74770a1ec690d601dbf43 100644 (file)
--- a/src/test/objectstore/TestObjectStoreState.cc
+++ b/src/test/objectstore/TestObjectStoreState.cc
@@ -29,8 +29,6 @@
 #undef dout_prefix
 #define dout_prefix *_dout << "ceph_test_objectstore_state "
 
-const coll_t TestObjectStoreState::META_COLL;
-
 void TestObjectStoreState::init(int colls, int objs)
 {
   dout(5) << "init " << colls << " colls " << objs << " objs" << dendl;
@@ -38,7 +36,7 @@ void TestObjectStoreState::init(int colls, int objs)
   ObjectStore::Transaction *t;
   t = new ObjectStore::Transaction;
 
-  t->create_collection(META_COLL);
+  t->create_collection(coll_t::meta());
   m_store->apply_transaction(*t);
 
   wait_for_ready();
@@ -59,7 +57,7 @@ void TestObjectStoreState::init(int colls, int objs)
     ::encode(num_objs, hint);
     t->collection_hint(entry->m_coll, ObjectStore::Transaction::COLL_HINT_EXPECTED_NUM_OBJECTS, hint);
     dout(5) << "give collection hint, number of objects per collection: " << num_objs << dendl;
-    t->touch(META_COLL, entry->m_meta_obj);
+    t->touch(coll_t::meta(), entry->m_meta_obj);
 
     for (int i = 0; i < objs; i++) {
       hobject_t *obj = entry->touch_obj(i + baseid);

diff --git a/src/test/objectstore/TestObjectStoreState.h b/src/test/objectstore/TestObjectStoreState.h
index a3338cb6bd07171c6a1befe5e4d7546c42d0e101..cfffcb155963ca265f04fec26edd921df5723568 100644 (file)
--- a/src/test/objectstore/TestObjectStoreState.h
+++ b/src/test/objectstore/TestObjectStoreState.h
@@ -56,10 +56,6 @@ public:
     hobject_t *get_obj_at(int pos, bool remove, int *key = NULL);
   };
 
-  /* kept in upper case for consistency with coll_t's */
-  static const coll_t META_COLL;
-  static const coll_t TEMP_COLL;
-
  protected:
   boost::shared_ptr<ObjectStore> m_store;
   map<int, coll_entry_t*> m_collections;

diff --git a/src/test/objectstore/workload_generator.cc b/src/test/objectstore/workload_generator.cc
index 4331d98e30e25005476490180eee7acecc006372..d50a1e896873e8c12d27850c0f78a4d7f9713731 100644 (file)
--- a/src/test/objectstore/workload_generator.cc
+++ b/src/test/objectstore/workload_generator.cc
@@ -336,13 +336,13 @@ void WorkloadGenerator::do_append_log(ObjectStore::Transaction *t,
   hobject_t log_obj = entry->m_meta_obj;
 
   dout(2) << __func__ << " coll " << entry->m_coll << " "
-      << META_COLL << " /" << log_obj << " (" << bl.length() << ")" << dendl;
+      << coll_t::meta() << " /" << log_obj << " (" << bl.length() << ")" << dendl;
 
   if (m_do_stats && (stat != NULL))
       stat->written_data += bl.length();
 
   uint64_t s = pg_log_size[entry->m_coll];
-  t->write(META_COLL, log_obj, s, bl.length(), bl);
+  t->write(coll_t::meta(), log_obj, s, bl.length(), bl);
   pg_log_size[entry->m_coll] += bl.length();
 }
 
@@ -362,7 +362,7 @@ void WorkloadGenerator::do_destroy_collection(ObjectStore::Transaction *t,
   }
 
   t->remove_collection(entry->m_coll);
-  t->remove(META_COLL, entry->m_meta_obj);
+  t->remove(coll_t::meta(), entry->m_meta_obj);
 }
 
 TestObjectStoreState::coll_entry_t
@@ -379,8 +379,8 @@ TestObjectStoreState::coll_entry_t
 
   dout(2) << __func__ << " id " << entry->m_id << " coll " << entry->m_coll << dendl;
   t->create_collection(entry->m_coll);
-  dout(2) << __func__ << " meta " << META_COLL << "/" << entry->m_meta_obj << dendl;
-  t->touch(META_COLL, entry->m_meta_obj);
+  dout(2) << __func__ << " meta " << coll_t::meta() << "/" << entry->m_meta_obj << dendl;
+  t->touch(coll_t::meta(), entry->m_meta_obj);
   return entry;
 }
 

diff --git a/src/tools/ceph_objectstore_tool.cc b/src/tools/ceph_objectstore_tool.cc
index 42f5279631a04f405ac0c942b20aef6cd32f3ea7..1d7b9b1551a60c4bb423b8b97bca7f9e9a13f634 100644 (file)
--- a/src/tools/ceph_objectstore_tool.cc
+++ b/src/tools/ceph_objectstore_tool.cc
@@ -40,8 +40,6 @@
 namespace po = boost::program_options;
 using namespace std;
 
-static coll_t META_COLL;
-
 #ifdef INTERNAL_TEST
 CompatSet get_test_compat_set() {
   CompatSet::FeatureSet ceph_osd_feature_compat;
@@ -348,7 +346,7 @@ int get_log(ObjectStore *fs, __u8 struct_ver,
     ostringstream oss;
     assert(struct_ver > 0);
     PGLog::read_log(fs, coll,
-                   struct_ver >= 8 ? coll : META_COLL,
+                   struct_ver >= 8 ? coll : coll_t::meta(),
                    struct_ver >= 8 ? pgid.make_pgmeta_oid() : log_oid,
                    info, divergent_priors, log, missing, oss);
     if (debug && oss.str().size())
@@ -490,10 +488,10 @@ int mark_pg_for_removal(ObjectStore *fs, spg_t pgid, ObjectStore::Transaction *t
     bufferlist one;
     one.append('1');
     t->collection_setattr(coll, "remove", one);
-    cout << "remove " << META_COLL << " " << log_oid.hobj.oid << std::endl;
-    t->remove(META_COLL, log_oid);
-    cout << "remove " << META_COLL << " " << biginfo_oid.oid << std::endl;
-    t->remove(META_COLL, biginfo_oid);
+    cout << "remove " << coll_t::meta() << " " << log_oid.hobj.oid << std::endl;
+    t->remove(coll_t::meta(), log_oid);
+    cout << "remove " << coll_t::meta() << " " << biginfo_oid.oid << std::endl;
+    t->remove(coll_t::meta(), biginfo_oid);
   } else {
     // new omap key
     cout << "setting '_remove' omap key" << std::endl;
@@ -715,7 +713,7 @@ int ObjectStoreTool::export_files(ObjectStore *store, coll_t coll)
 int get_osdmap(ObjectStore *store, epoch_t e, OSDMap &osdmap, bufferlist& bl)
 {
   bool found = store->read(
-      META_COLL, OSD::get_osdmap_pobject_name(e), 0, 0, bl) >= 0;
+      coll_t::meta(), OSD::get_osdmap_pobject_name(e), 0, 0, bl) >= 0;
   if (!found) {
     cerr << "Can't find OSDMap for pg epoch " << e << std::endl;
     return -ENOENT;
@@ -2068,7 +2066,7 @@ int main(int argc, char **argv)
   bufferlist bl;
   OSDSuperblock superblock;
   bufferlist::iterator p;
-  ret = fs->read(META_COLL, OSD_SUPERBLOCK_POBJECT, 0, 0, bl);
+  ret = fs->read(coll_t::meta(), OSD_SUPERBLOCK_POBJECT, 0, 0, bl);
   if (ret < 0) {
     cerr << "Failure to read OSD superblock: " << cpp_strerror(ret) << std::endl;
     goto out;
@@ -2254,7 +2252,7 @@ int main(int argc, char **argv)
       ObjectStore::Transaction t;
       bl.clear();
       ::encode(superblock, bl);
-      t.write(META_COLL, OSD_SUPERBLOCK_POBJECT, 0, bl.length(), bl);
+      t.write(coll_t::meta(), OSD_SUPERBLOCK_POBJECT, 0, bl.length(), bl);
       ret = fs->apply_transaction(t);
       if (ret < 0) {
         cerr << "Error writing OSD superblock: " << cpp_strerror(ret) << std::endl;