os/ObjectStore: fix return code of collection_empty() method

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

diff --git a/src/os/FuseStore.cc b/src/os/FuseStore.cc
index fd9dc9a2165ccbfbee8d1e5fd46c21547639f4f7..c8c6c909e16f7cbeb761554f906b20849bee6f00 100644 (file)
--- 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 544f906831e35f53438ae170d65ccea2a7406a7b..c3e3fa5ba498635e8fca95057826c43ccac2101c 100644 (file)
--- 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 025d60d1bfb6ab2f327c2ff86a729e1442ec91c6..1705e07e627bb1f44e3e2d399d39556d2dd3ca50 100644 (file)
--- 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<ghobject_t> 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 e50f899b81988c202293d0e04aec29cc9ae5423a..bcd22bac22d66e03ecddb0a7ed5d3fc56c6b8f48 100644 (file)
--- 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 15a4a3b15b07753410091a8e78736d019229c304..ff928314861995d4c13eadf87a2c6ec2be8d5c6e 100644 (file)
--- 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 1d7abd6d73355f5765c787206b260d417a4e34a6..dca849f280131ed265fb7f1648c53d6ff6732cd0 100644 (file)
--- a/src/os/filestore/FileStore.h
+++ b/src/os/filestore/FileStore.h
@@ -625,7 +625,7 @@ public:
   int list_collections(vector<coll_t>& 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 31c6ebc497a85bb93924e7ef2044745273fc1392..abd322fd7edd592de645f373633816f6ab0bfe0e 100755 (executable)
--- 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<ghobject_t> 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 534bc20e48f5bedc11bce0543c834a840383f7d4..6230764dd813def9450c306d905a5660faec29fb 100644 (file)
--- a/src/os/kstore/KStore.h
+++ b/src/os/kstore/KStore.h
@@ -459,7 +459,7 @@ public:
 
   int list_collections(vector<coll_t>& 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 f4b356b5926d943fab49a3c72ecedaadf8ef7190..e0766b93b7785588902e6d2b8ce8da4a20d9f072 100644 (file)
--- 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 9118db2cf5403403edb1cdcd57a61ea2cda7a072..090fc24ba98f916a970c5a0c1d91fa1c7f0a6a04 100644 (file)
--- 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 12d5c8a890babc8f0c76d3e8e8659b1841677133..919f6674497e4365824b06758fe76eb68af571ec 100644 (file)
--- 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;