From: Kefu Chai <kchai@redhat.com>
Date: Thu, 30 Jul 2015 14:19:41 +0000 (+0800)
Subject: os/FileStore: getattr() should return 0 if success
X-Git-Tag: v9.1.0~455^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F5422%2Fhead;p=ceph.git

os/FileStore: getattr() should return 0 if success

* fix the test of ObjectStore/StoreTest.SimpleCloneTest, the expected
  return value of getattr() should be 0. otherwise it otherwise it fails
  on objectstore backends of "memstore" and "keyvaluestore".
* change FileStore::getattr() to return 0 on success
* tighten related tests in ceph_test_objectstore, to test the return value
  of getattr() using ASSERT_EQ(r, 0) instead of ASSERT_GE(r, 0)

Signed-off-by: Kefu Chai <kchai@redhat.com>
---

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index a9f4470e140f..841de3f39649 100644
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -4072,7 +4072,7 @@ int FileStore::getattr(coll_t cid, const ghobject_t& oid, const char *name, buff
     return -EIO;
   } else {
     tracepoint(objectstore, getattr_exit, r);
-    return r;
+    return r < 0 ? r : 0;
   }
 }
 
diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
index 90c368bef680..6d1654cd9c91 100644
--- a/src/test/objectstore/store_test.cc
+++ b/src/test/objectstore/store_test.cc
@@ -428,24 +428,29 @@ TEST_P(StoreTest, SimpleCloneTest) {
     cerr << "Clone object and rm attr" << std::endl;
     r = store->apply_transaction(t);
     ASSERT_EQ(r, 0);
+
     r = store->read(cid, hoid, 10, 5, newdata);
     ASSERT_EQ(r, 5);
     ASSERT_TRUE(newdata.contents_equal(large));
+
     newdata.clear();
     r = store->read(cid, hoid2, 10, 5, newdata);
     ASSERT_EQ(r, 5);
     ASSERT_TRUE(newdata.contents_equal(small));
+
     r = store->getattr(cid, hoid2, "attr2", attr);
-    ASSERT_EQ(r, 5);
+    ASSERT_EQ(r, 0);
     ASSERT_TRUE(attr.contents_equal(small));
+
     attr.clear();
     r = store->getattr(cid, hoid2, "attr3", attr);
-    ASSERT_EQ(r, 6);
+    ASSERT_EQ(r, 0);
     ASSERT_TRUE(attr.contents_equal(xlarge));
+
     attr.clear();
     r = store->getattr(cid, hoid, "attr1", attr);
-    ASSERT_EQ(r, 5);
-    ASSERT_TRUE(attr.contents_equal(small));
+    ASSERT_EQ(r, 0);
+    ASSERT_TRUE(attr.contents_equal(large));
   }
   {
     ObjectStore::Transaction t;
@@ -952,7 +957,7 @@ public:
 
     bufferlist bl;
     r = store->getattr(cid, obj, it->first, bl);
-    ASSERT_TRUE(r >= 0);
+    ASSERT_EQ(r, 0);
     ASSERT_TRUE(it->second.contents_equal(bl));
   }
 
@@ -1650,7 +1655,7 @@ TEST_P(StoreTest, XattrTest) {
   ASSERT_EQ(r, -ENODATA);
 
   r = store->getattr(cid, hoid, "attr3", bp);
-  ASSERT_GE(r, 0);
+  ASSERT_EQ(r, 0);
   bufferlist bl2;
   bl2.push_back(bp);
   ASSERT_TRUE(bl2 == attrs["attr3"]);
@@ -1859,7 +1864,7 @@ TEST_P(StoreTest, MoveRename) {
     ASSERT_TRUE(newdata.contents_equal(data));
     bufferlist newattr;
     r = store->getattr(cid, oid, "attr", newattr);
-    ASSERT_GE(r, 0);
+    ASSERT_EQ(r, 0);
     ASSERT_TRUE(newattr.contents_equal(attr));
     set<string> keys;
     keys.insert("omap_key");