From: Kefu Chai Date: Tue, 25 May 2021 07:18:21 +0000 (+0800) Subject: os: let ObjectStore::create() return unique_ptr<> X-Git-Tag: v17.1.0~1806^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7e8ec0c8cae08c98f28848c0f0fe5d68b2909b97;p=ceph.git os: let ObjectStore::create() return unique_ptr<> instead of returning a raw pointer of ObjectStore, let `ObjectStore::create()` return a `std::unique_ptr`. less error prune this way. Signed-off-by: Kefu Chai --- diff --git a/src/ceph_osd.cc b/src/ceph_osd.cc index 333e0de9a107..b6278fa72bc4 100644 --- a/src/ceph_osd.cc +++ b/src/ceph_osd.cc @@ -323,11 +323,11 @@ int main(int argc, const char **argv) std::string journal_path = g_conf().get_val("osd_journal"); uint32_t flags = g_conf().get_val("osd_os_flags"); - ObjectStore *store = ObjectStore::create(g_ceph_context, - store_type, - data_path, - journal_path, - flags); + std::unique_ptr store = ObjectStore::create(g_ceph_context, + store_type, + data_path, + journal_path, + flags); if (!store) { derr << "unable to create object store" << dendl; forker.exit(-ENODEV); @@ -369,7 +369,7 @@ int main(int argc, const char **argv) forker.exit(-EINVAL); } - int err = OSD::mkfs(g_ceph_context, store, g_conf().get_val("fsid"), + int err = OSD::mkfs(g_ceph_context, store.release(), g_conf().get_val("fsid"), whoami, osdspec_affinity); if (err < 0) { derr << TEXT_RED << " ** ERROR: error creating empty object store in " @@ -438,7 +438,7 @@ int main(int argc, const char **argv) << " for object store " << data_path << dendl; flushjournal_out: - delete store; + store.reset(); forker.exit(err < 0 ? 1 : 0); } if (dump_journal) { @@ -477,7 +477,7 @@ flushjournal_out: uuid_d cluster_fsid, osd_fsid; ceph_release_t require_osd_release = ceph_release_t::unknown; int w; - int r = OSD::peek_meta(store, &magic, &cluster_fsid, &osd_fsid, &w, + int r = OSD::peek_meta(store.get(), &magic, &cluster_fsid, &osd_fsid, &w, &require_osd_release); if (r < 0) { derr << TEXT_RED << " ** ERROR: unable to open OSD superblock on " @@ -678,7 +678,7 @@ flushjournal_out: } osdptr = new OSD(g_ceph_context, - store, + store.release(), whoami, ms_cluster, ms_public, diff --git a/src/os/ObjectStore.cc b/src/os/ObjectStore.cc index dc606bd2cc7a..83288068199c 100644 --- a/src/os/ObjectStore.cc +++ b/src/os/ObjectStore.cc @@ -30,44 +30,45 @@ using std::string; -ObjectStore *ObjectStore::create(CephContext *cct, - const string& type, - const string& data, - const string& journal, - osflagbits_t flags) +std::unique_ptr ObjectStore::create( + CephContext *cct, + const string& type, + const string& data, + const string& journal, + osflagbits_t flags) { #ifndef WITH_SEASTAR if (type == "filestore") { - return new FileStore(cct, data, journal, flags); + return std::make_unique(cct, data, journal, flags); } if (type == "memstore") { - return new MemStore(cct, data); + return std::make_unique(cct, data); } #endif #if defined(WITH_BLUESTORE) if (type == "bluestore") { - return new BlueStore(cct, data); + return std::make_unique(cct, data); } #ifndef WITH_SEASTAR if (type == "random") { if (rand() % 2) { - return new FileStore(cct, data, journal, flags); + return std::make_unique(cct, data, journal, flags); } else { - return new BlueStore(cct, data); + return std::make_unique(cct, data); } } #endif #else #ifndef WITH_SEASTAR if (type == "random") { - return new FileStore(cct, data, journal, flags); + return std::make_unique(cct, data, journal, flags); } #endif #endif #ifndef WITH_SEASTAR if (type == "kstore" && cct->check_experimental_feature_enabled("kstore")) { - return new KStore(cct, data); + return std::make_unique(cct, data); } #endif return NULL; diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h index bba8627f5f2a..4fc921d9b61a 100644 --- a/src/os/ObjectStore.h +++ b/src/os/ObjectStore.h @@ -29,8 +29,9 @@ #include #include -#include #include +#include +#include #if defined(__APPLE__) || defined(__FreeBSD__) || defined(__sun) || defined(_WIN32) #include @@ -77,11 +78,12 @@ public: * @param journal path (or other descriptor) for journal (optional) * @param flags which filestores should check if applicable */ - static ObjectStore *create(CephContext *cct, - const std::string& type, - const std::string& data, - const std::string& journal, - osflagbits_t flags = 0); + static std::unique_ptr create( + CephContext *cct, + const std::string& type, + const std::string& data, + const std::string& journal, + osflagbits_t flags = 0); /** * probe a block device to learn the uuid of the owning OSD diff --git a/src/test/fio/fio_ceph_objectstore.cc b/src/test/fio/fio_ceph_objectstore.cc index 33f900b80fca..86d38919426a 100644 --- a/src/test/fio/fio_ceph_objectstore.cc +++ b/src/test/fio/fio_ceph_objectstore.cc @@ -408,10 +408,10 @@ Engine::Engine(thread_data* td) TracepointProvider::initialize(g_ceph_context); // create the ObjectStore - os.reset(ObjectStore::create(g_ceph_context, - g_conf().get_val("osd objectstore"), - g_conf().get_val("osd data"), - g_conf().get_val("osd journal"))); + os = ObjectStore::create(g_ceph_context, + g_conf().get_val("osd objectstore"), + g_conf().get_val("osd data"), + g_conf().get_val("osd journal")); if (!os) throw std::runtime_error("bad objectstore type " + g_conf()->osd_objectstore); diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index 50db5749bac4..176d1d48b846 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -16,9 +16,9 @@ #include #include #include +#include #include #include -#include #include #include #include @@ -6622,7 +6622,7 @@ INSTANTIATE_TEST_SUITE_P( #endif "kstore")); -void doMany4KWritesTest(boost::scoped_ptr& store, +void doMany4KWritesTest(ObjectStore* store, unsigned max_objects, unsigned max_ops, unsigned max_object_size, @@ -6634,7 +6634,7 @@ void doMany4KWritesTest(boost::scoped_ptr& store, coll_t cid(spg_t(pg_t(0,555), shard_id_t::NO_SHARD)); store_statfs_t res_stat; - SyntheticWorkloadState test_obj(store.get(), + SyntheticWorkloadState test_obj(store, &gen, &rng, cid, @@ -6675,7 +6675,7 @@ TEST_P(StoreTestSpecificAUSize, Many4KWritesTest) { StartDeferred(0x10000); const unsigned max_object = 4*1024*1024; - doMany4KWritesTest(store, 1, 1000, max_object, 4*1024, 0); + doMany4KWritesTest(store.get(), 1, 1000, max_object, 4*1024, 0); } TEST_P(StoreTestSpecificAUSize, Many4KWritesNoCSumTest) { @@ -6686,7 +6686,7 @@ TEST_P(StoreTestSpecificAUSize, Many4KWritesNoCSumTest) { g_ceph_context->_conf.apply_changes(nullptr); const unsigned max_object = 4*1024*1024; - doMany4KWritesTest(store, 1, 1000, max_object, 4*1024, 0 ); + doMany4KWritesTest(store.get(), 1, 1000, max_object, 4*1024, 0 ); } TEST_P(StoreTestSpecificAUSize, TooManyBlobsTest) { @@ -6694,7 +6694,7 @@ TEST_P(StoreTestSpecificAUSize, TooManyBlobsTest) { return; StartDeferred(0x10000); const unsigned max_object = 4*1024*1024; - doMany4KWritesTest(store, 1, 1000, max_object, 4*1024, 0); + doMany4KWritesTest(store.get(), 1, 1000, max_object, 4*1024, 0); } #if defined(WITH_BLUESTORE) diff --git a/src/test/objectstore/store_test_fixture.cc b/src/test/objectstore/store_test_fixture.cc index e343f96156da..2b26207d4e51 100644 --- a/src/test/objectstore/store_test_fixture.cc +++ b/src/test/objectstore/store_test_fixture.cc @@ -40,10 +40,10 @@ void StoreTestFixture::SetUp() } ASSERT_EQ(0, r); - store.reset(ObjectStore::create(g_ceph_context, - type, - data_dir, - string("store_test_temp_journal"))); + store = ObjectStore::create(g_ceph_context, + type, + data_dir, + "store_test_temp_journal"); if (!store) { cerr << __func__ << ": objectstore type " << type << " doesn't exist yet!" << std::endl; } @@ -113,10 +113,10 @@ void StoreTestFixture::CloseAndReopen() { EXPECT_EQ(0, r); ch.reset(nullptr); store.reset(nullptr); - store.reset(ObjectStore::create(g_ceph_context, - type, - data_dir, - string("store_test_temp_journal"))); + store = ObjectStore::create(g_ceph_context, + type, + data_dir, + "store_test_temp_journal"); if (!store) { cerr << __func__ << ": objectstore type " << type << " failed to reopen!" << std::endl; } diff --git a/src/test/objectstore/store_test_fixture.h b/src/test/objectstore/store_test_fixture.h index 3b7971d7cdee..3f25fd493d0d 100644 --- a/src/test/objectstore/store_test_fixture.h +++ b/src/test/objectstore/store_test_fixture.h @@ -1,6 +1,6 @@ #include #include -#include +#include #include #include "common/config_fwd.h" @@ -16,7 +16,7 @@ class StoreTestFixture : virtual public ::testing::Test { std::string orig_death_test_style; public: - boost::scoped_ptr store; + std::unique_ptr store; ObjectStore::CollectionHandle ch; explicit StoreTestFixture(const std::string& type) diff --git a/src/test/objectstore_bench.cc b/src/test/objectstore_bench.cc index b04e2d8c141e..bc25c5a8f7f4 100644 --- a/src/test/objectstore_bench.cc +++ b/src/test/objectstore_bench.cc @@ -207,11 +207,11 @@ int main(int argc, const char *argv[]) dout(0) << "repeats " << cfg.repeats << dendl; dout(0) << "threads " << cfg.threads << dendl; - auto os = std::unique_ptr( + auto os = ObjectStore::create(g_ceph_context, g_conf()->osd_objectstore, g_conf()->osd_data, - g_conf()->osd_journal)); + g_conf()->osd_journal); //Checking data folder: create if needed or error if it's not empty DIR *dir = ::opendir(g_conf()->osd_data.c_str()); diff --git a/src/test/osd/TestOSDScrub.cc b/src/test/osd/TestOSDScrub.cc index 45d79a183794..77caa4104a8b 100644 --- a/src/test/osd/TestOSDScrub.cc +++ b/src/test/osd/TestOSDScrub.cc @@ -55,7 +55,7 @@ public: TEST(TestOSDScrub, scrub_time_permit) { ceph::async::io_context_pool icp(1); - ObjectStore *store = ObjectStore::create(g_ceph_context, + std::unique_ptr store = ObjectStore::create(g_ceph_context, g_conf()->osd_objectstore, g_conf()->osd_data, g_conf()->osd_journal); @@ -68,7 +68,7 @@ TEST(TestOSDScrub, scrub_time_permit) { ms->bind(g_conf()->public_addr); MonClient mc(g_ceph_context, icp); mc.build_initial_monmap(); - TestOSDScrub* osd = new TestOSDScrub(g_ceph_context, store, 0, ms, ms, ms, ms, ms, ms, ms, &mc, "", "", icp); + TestOSDScrub* osd = new TestOSDScrub(g_ceph_context, store.release(), 0, ms, ms, ms, ms, ms, ms, ms, &mc, "", "", icp); // These are now invalid int err = g_ceph_context->_conf.set_val("osd_scrub_begin_hour", "24"); diff --git a/src/tools/ceph_objectstore_tool.cc b/src/tools/ceph_objectstore_tool.cc index 9a77a8df29da..1873b69a31ad 100644 --- a/src/tools/ceph_objectstore_tool.cc +++ b/src/tools/ceph_objectstore_tool.cc @@ -3513,7 +3513,7 @@ int main(int argc, char **argv) } } - ObjectStore *fs = ObjectStore::create(g_ceph_context, type, dpath, jpath, flags); + ObjectStore *fs = ObjectStore::create(g_ceph_context, type, dpath, jpath, flags).release(); if (fs == NULL) { cerr << "Unable to create store of type " << type << std::endl; return 1; @@ -3577,14 +3577,14 @@ int main(int argc, char **argv) target_type = string(bl.c_str(), bl.length() - 1); // drop \n } ::close(fd); - ObjectStore *targetfs = ObjectStore::create( + unique_ptr targetfs = ObjectStore::create( g_ceph_context, target_type, target_data_path, "", 0); if (targetfs == NULL) { cerr << "Unable to open store of type " << target_type << std::endl; return 1; } - int r = dup(dpath, fs, target_data_path, targetfs); + int r = dup(dpath, fs, target_data_path, targetfs.get()); if (r < 0) { cerr << "dup failed: " << cpp_strerror(r) << std::endl; return 1;