From f578fd63ddde31c2134ec900a8a0a2207919ad94 Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Mon, 12 Sep 2016 14:13:42 +0800 Subject: [PATCH] os/ObjectStore: fix return code of collection_empty() method Signed-off-by: xie xingguo --- src/os/FuseStore.cc | 12 +++++++++--- src/os/ObjectStore.h | 5 +++-- src/os/bluestore/BlueStore.cc | 15 +++++++++------ src/os/bluestore/BlueStore.h | 2 +- src/os/filestore/FileStore.cc | 26 +++++++++++++++++--------- src/os/filestore/FileStore.h | 2 +- src/os/kstore/KStore.cc | 15 +++++++++------ src/os/kstore/KStore.h | 2 +- src/os/memstore/MemStore.cc | 8 ++++---- src/os/memstore/MemStore.h | 2 +- src/test/objectstore/store_test.cc | 12 ++++++++---- 11 files changed, 63 insertions(+), 38 deletions(-) diff --git a/src/os/FuseStore.cc b/src/os/FuseStore.cc index fd9dc9a2165c..c8c6c909e16f 100644 --- a/src/os/FuseStore.cc +++ b/src/os/FuseStore.cc @@ -1001,9 +1001,15 @@ static int os_unlink(const char *path) break; case FN_COLLECTION: - if (!fs->store->collection_empty(cid)) - return -ENOTEMPTY; - t.remove_collection(cid); + { + bool empty; + int r = fs->store->collection_empty(cid, &empty); + if (r < 0) + return r; + if (!empty) + return -ENOTEMPTY; + t.remove_collection(cid); + } break; case FN_OBJECT_DATA: diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h index 544f906831e3..c3e3fa5ba498 100644 --- a/src/os/ObjectStore.h +++ b/src/os/ObjectStore.h @@ -2164,9 +2164,10 @@ public: * is a collection empty? * * @param c collection - * @returns true if empty, false otherwise + * @param empty true if the specified collection is empty, false otherwise + * @returns 0 on success, negative error code on failure. */ - virtual bool collection_empty(const coll_t& c) = 0; + virtual int collection_empty(const coll_t& c, bool *empty) = 0; /** * return the number of significant bits of the coll_t::pgid. diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 025d60d1bfb6..1705e07e627b 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -5045,18 +5045,21 @@ bool BlueStore::collection_exists(const coll_t& c) return coll_map.count(c); } -bool BlueStore::collection_empty(const coll_t& cid) +int BlueStore::collection_empty(const coll_t& cid, bool *empty) { dout(15) << __func__ << " " << cid << dendl; vector ls; ghobject_t next; int r = collection_list(cid, ghobject_t(), ghobject_t::get_max(), true, 1, &ls, &next); - if (r < 0) - return false; // fixme? - bool empty = ls.empty(); - dout(10) << __func__ << " " << cid << " = " << (int)empty << dendl; - return empty; + if (r < 0) { + derr << __func__ << " collection_list returned: " << cpp_strerror(r) + << dendl; + return r; + } + *empty = ls.empty(); + dout(10) << __func__ << " " << cid << " = " << (int)(*empty) << dendl; + return 0; } int BlueStore::collection_bits(const coll_t& cid) diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index e50f899b8198..bcd22bac22d6 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -1541,7 +1541,7 @@ public: CollectionHandle open_collection(const coll_t &c) override; bool collection_exists(const coll_t& c) override; - bool collection_empty(const coll_t& c) override; + int collection_empty(const coll_t& c, bool *empty) override; int collection_bits(const coll_t& c) override; int collection_list(const coll_t& cid, ghobject_t start, ghobject_t end, diff --git a/src/os/filestore/FileStore.cc b/src/os/filestore/FileStore.cc index 15a4a3b15b07..ff9283148619 100644 --- a/src/os/filestore/FileStore.cc +++ b/src/os/filestore/FileStore.cc @@ -4681,14 +4681,17 @@ bool FileStore::collection_exists(const coll_t& c) return ret; } -bool FileStore::collection_empty(const coll_t& c) +int FileStore::collection_empty(const coll_t& c, bool *empty) { tracepoint(objectstore, collection_empty_enter, c.c_str()); dout(15) << "collection_empty " << c << dendl; Index index; int r = get_index(c, &index); - if (r < 0) - return false; + if (r < 0) { + derr << __func__ << " get_index returned: " << cpp_strerror(r) + << dendl; + return r; + } assert(NULL != index.index); RWLock::RLocker l((index.index)->access_lock); @@ -4697,12 +4700,14 @@ bool FileStore::collection_empty(const coll_t& c) r = index->collection_list_partial(ghobject_t(), ghobject_t::get_max(), true, 1, &ls, NULL); if (r < 0) { + derr << __func__ << " collection_list_partial returned: " + << cpp_strerror(r) << dendl; assert(!m_filestore_fail_eio || r != -EIO); - return false; + return r; } - bool ret = ls.empty(); - tracepoint(objectstore, collection_empty_exit, ret); - return ret; + *empty = ls.empty(); + tracepoint(objectstore, collection_empty_exit, *empty); + return 0; } int FileStore::collection_list(const coll_t& c, ghobject_t start, ghobject_t end, bool sort_bitwise, int max, @@ -4963,13 +4968,16 @@ int FileStore::_collection_hint_expected_num_objs(const coll_t& c, uint32_t pg_n dout(15) << __func__ << " collection: " << c << " pg number: " << pg_num << " expected number of objects: " << expected_num_objs << dendl; - if (!collection_empty(c) && !replaying) { + bool empty; + int ret = collection_empty(c, &empty); + if (ret < 0) + return ret; + if (!empty && !replaying) { dout(0) << "Failed to give an expected number of objects hint to collection : " << c << ", only empty collection can take such type of hint. " << dendl; return 0; } - int ret; Index index; ret = get_index(c, &index); if (ret < 0) diff --git a/src/os/filestore/FileStore.h b/src/os/filestore/FileStore.h index 1d7abd6d7335..dca849f28013 100644 --- a/src/os/filestore/FileStore.h +++ b/src/os/filestore/FileStore.h @@ -625,7 +625,7 @@ public: int list_collections(vector& ls, bool include_temp); int collection_stat(const coll_t& c, struct stat *st); bool collection_exists(const coll_t& c); - bool collection_empty(const coll_t& c); + int collection_empty(const coll_t& c, bool *empty); // omap (see ObjectStore.h for documentation) using ObjectStore::omap_get; diff --git a/src/os/kstore/KStore.cc b/src/os/kstore/KStore.cc index 31c6ebc497a8..abd322fd7edd 100755 --- a/src/os/kstore/KStore.cc +++ b/src/os/kstore/KStore.cc @@ -1375,18 +1375,21 @@ bool KStore::collection_exists(const coll_t& c) return coll_map.count(c); } -bool KStore::collection_empty(const coll_t& cid) +int KStore::collection_empty(const coll_t& cid, bool *empty) { dout(15) << __func__ << " " << cid << dendl; vector ls; ghobject_t next; int r = collection_list(cid, ghobject_t(), ghobject_t::get_max(), true, 1, &ls, &next); - if (r < 0) - return false; // fixme? - bool empty = ls.empty(); - dout(10) << __func__ << " " << cid << " = " << (int)empty << dendl; - return empty; + if (r < 0) { + derr << __func__ << " collection_list returned: " << cpp_strerror(r) + << dendl; + return r; + } + *empty = ls.empty(); + dout(10) << __func__ << " " << cid << " = " << (int)(*empty) << dendl; + return 0; } int KStore::collection_list( diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h index 534bc20e48f5..6230764dd813 100644 --- a/src/os/kstore/KStore.h +++ b/src/os/kstore/KStore.h @@ -459,7 +459,7 @@ public: int list_collections(vector& ls); bool collection_exists(const coll_t& c); - bool collection_empty(const coll_t& c); + int collection_empty(const coll_t& c, bool *empty); using ObjectStore::collection_list; int collection_list(const coll_t& cid, ghobject_t start, ghobject_t end, diff --git a/src/os/memstore/MemStore.cc b/src/os/memstore/MemStore.cc index f4b356b5926d..e0766b93b778 100644 --- a/src/os/memstore/MemStore.cc +++ b/src/os/memstore/MemStore.cc @@ -441,15 +441,15 @@ bool MemStore::collection_exists(const coll_t& cid) return coll_map.count(cid); } -bool MemStore::collection_empty(const coll_t& cid) +int MemStore::collection_empty(const coll_t& cid, bool *empty) { dout(10) << __func__ << " " << cid << dendl; CollectionRef c = get_collection(cid); if (!c) - return false; + return -ENOENT; RWLock::RLocker l(c->lock); - - return c->object_map.empty(); + *empty = c->object_map.empty(); + return 0; } int MemStore::collection_list(const coll_t& cid, ghobject_t start, ghobject_t end, diff --git a/src/os/memstore/MemStore.h b/src/os/memstore/MemStore.h index 9118db2cf540..090fc24ba98f 100644 --- a/src/os/memstore/MemStore.h +++ b/src/os/memstore/MemStore.h @@ -346,7 +346,7 @@ public: return get_collection(c); } bool collection_exists(const coll_t& c) override; - bool collection_empty(const coll_t& c) override; + int collection_empty(const coll_t& c, bool *empty) override; using ObjectStore::collection_list; int collection_list(const coll_t& cid, ghobject_t start, ghobject_t end, bool sort_bitwise, int max, diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index 12d5c8a890ba..919f6674497e 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -2029,8 +2029,10 @@ TEST_P(StoreTest, SimpleAttrTest) { ASSERT_EQ(r, 0); } { - bool r = store->collection_empty(cid); - ASSERT_TRUE(r); + bool empty; + int r = store->collection_empty(cid, &empty); + ASSERT_EQ(0, r); + ASSERT_TRUE(empty); } { bufferptr bp; @@ -2046,8 +2048,10 @@ TEST_P(StoreTest, SimpleAttrTest) { ASSERT_EQ(r, 0); } { - bool r = store->collection_empty(cid); - ASSERT_TRUE(!r); + bool empty; + int r = store->collection_empty(cid, &empty); + ASSERT_EQ(0, r); + ASSERT_TRUE(!empty); } { bufferptr bp; -- 2.47.3