Merge pull request ceph#63839 from aainscow/setattr_fix

bluestore: Fix _setattr() with rare memory alignments

Reviewed-by: [email protected]
Reviewed-by: [email protected]

Author: aainscow

src/os/bluestore/BlueStore.cc

@@ -18039,6 +18039,8 @@ int BlueStore::_remove(TransContext *txc,
   return r;
 }
 
+
+
 int BlueStore::_setattr(TransContext *txc,
 			CollectionRef& c,
 			OnodeRef& o,
@@ -18049,18 +18051,17 @@ int BlueStore::_setattr(TransContext *txc,
 	   << " " << name << " (" << val.length() << " bytes)"
 	   << dendl;
   int r = 0;
-  if (!val.length()) {
-    auto& b = o->onode.attrs[name.c_str()] = bufferptr("", 0);
-    b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
-  } else if (!val.is_contiguous()) {
-    val.rebuild();
-    auto& b = o->onode.attrs[name.c_str()] = val.front();
-    b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
-  } else if (val.front().is_partial()) {
-    val.rebuild();
-    auto& b = o->onode.attrs[name.c_str()] = val.front();
-    b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
+  auto& b = o->onode.attrs[name.c_str()];
+  if (val.length() == 0) {
+    b = bufferptr("", 0);
+  } else {
+    if (!val.is_contiguous() || val.front().is_partial()) {
+      val.rebuild();
+    }
+    b = val.front();
   }
+  b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
+
   txc->write_onode(o);
   dout(10) << __func__ << " " << c->cid << " " << o->oid
 	   << " " << name << " (" << val.length() << " bytes)"

src/test/objectstore/store_test.cc

@@ -3019,9 +3019,13 @@ TEST_P(StoreTest, SimpleAttrTest) {
   int r;
   coll_t cid;
   ghobject_t hoid(hobject_t(sobject_t("attr object 1", CEPH_NOSNAP)));
-  bufferlist val, val2;
+  bufferlist val, val2, val_exactly_10, empty_bl;
   val.append("value");
   val.append("value2");
+  bufferptr bp = bufferptr("0123456789abcdef", 0x10);
+  ASSERT_EQ(bp.length(), 0x10);
+  val_exactly_10.append(bp);
+  ASSERT_EQ(val_exactly_10.get_num_buffers(), 1);
   {
     auto ch = store->open_collection(cid);
     ASSERT_FALSE(ch);
@@ -3049,6 +3053,8 @@ TEST_P(StoreTest, SimpleAttrTest) {
     t.touch(cid, hoid);
     t.setattr(cid, hoid, "foo", val);
     t.setattr(cid, hoid, "bar", val2);
+    t.setattr(cid, hoid, "tiramisu", val_exactly_10);
+    t.setattr(cid, hoid, "empty", empty_bl);
     r = queue_transaction(store, ch, std::move(t));
     ASSERT_EQ(r, 0);
   }
@@ -3069,10 +3075,48 @@ TEST_P(StoreTest, SimpleAttrTest) {
     bl.append(bp);
     ASSERT_TRUE(bl_eq(val, bl));
 
+    r = store->getattr(ch, hoid, "tiramisu", bp);
+    ASSERT_EQ(0, r);
+    bufferlist bl1;
+    bl1.append(bp);
+    ASSERT_TRUE(bl_eq(val_exactly_10, bl1));
+
+    r = store->getattr(ch, hoid, "empty", bp);
+    ASSERT_EQ(0, r);
+    EXPECT_EQ(bp.length(), 0);
+
     map<string,bufferptr,less<>> bm;
     r = store->getattrs(ch, hoid, bm);
     ASSERT_EQ(0, r);
+  }
+  ch.reset();
+  EXPECT_EQ(store->umount(), 0);
+  EXPECT_EQ(store->mount(), 0);
+  ch = store->open_collection(cid);
+  {
+    bufferptr bp;
+    r = store->getattr(ch, hoid, "nofoo", bp);
+    ASSERT_EQ(-ENODATA, r);
 
+    r = store->getattr(ch, hoid, "foo", bp);
+    ASSERT_EQ(0, r);
+    bufferlist bl;
+    bl.append(bp);
+    ASSERT_TRUE(bl_eq(val, bl));
+
+    r = store->getattr(ch, hoid, "tiramisu", bp);
+    ASSERT_EQ(0, r);
+    bufferlist bl1;
+    bl1.append(bp);
+    ASSERT_TRUE(bl_eq(val_exactly_10, bl1));
+
+    r = store->getattr(ch, hoid, "empty", bp);
+    ASSERT_EQ(0, r);
+    EXPECT_EQ(bp.length(), 0);
+
+    map<string,bufferptr,less<>> bm;
+    r = store->getattrs(ch, hoid, bm);
+    ASSERT_EQ(0, r);
   }
   {
     ObjectStore::Transaction t;
@@ -10692,8 +10736,11 @@ TEST_P(StoreTest, BluestorePerPoolOmapFixOnMount)
     t.omap_setheader(cid, oid, h);
     t.touch(cid, oid2);
     t.omap_setheader(cid, oid2, h);
+    C_SaferCond c;
+    t.register_on_commit(&c);
     int r = queue_transaction(store, ch, std::move(t));
     ASSERT_EQ(r, 0);
+    c.wait();
   }
 
   // inject legacy omaps