os: get rid of the Transaction::decode_bp()

`os::Transaction::decode_bp()` has only one user: `_setattrs()`
of `BlueStore`. It uses that for optimization purposes: keeping
up contigous space instead of potentially fragmented `bufferlist`
that would require rectifying memcpy later.
The problem is `_setattrs()` also needs to avoid keeping large
raw buffers with only small subset being referenced. It achieves
this by copying the data if `bufferptr:::is_partial()` returns
`true`. However, this means the memcpy happens virtually always
as it's hard to even imagine the `val`, decoded from the wire,
can fulfill the 0 waste requirement.
Therefore the optimization doesn't make sense; it only imposes
costs in terms of complexity breaking the symmetry between encode
and decode in `os::Transation` (there is no `encode_bp()`).

This commit kills the optimization and simplifies `os::Transaction`.

Signed-off-by: Radoslaw Zarzynski <[email protected]>

Author: rzarzynski

src/os/Transaction.h

@@ -711,10 +711,6 @@ class Transaction {
       decode(s, data_bl_p);
       return s;
     }
-    void decode_bp(ceph::buffer::ptr& bp) {
-	using ceph::decode;
-      decode(bp, data_bl_p);
-    }
     void decode_bl(ceph::buffer::list& bl) {
 	using ceph::decode;
       decode(bl, data_bl_p);

src/os/bluestore/BlueStore.cc

@@ -15690,9 +15690,9 @@ void BlueStore::_txc_add_transaction(TransContext *txc, Transaction *t)
     case Transaction::OP_SETATTR:
       {
         string name = i.decode_string();
-        bufferptr bp;
-        i.decode_bp(bp);
-	r = _setattr(txc, c, o, name, bp);
+        bufferlist bl;
+        i.decode_bl(bl);
+	r = _setattr(txc, c, o, name, bl);
       }
       break;
 
@@ -17733,18 +17733,22 @@ int BlueStore::_setattr(TransContext *txc,
 			CollectionRef& c,
 			OnodeRef& o,
 			const string& name,
-			bufferptr& val)
+			bufferlist& val)
 {
   dout(15) << __func__ << " " << c->cid << " " << o->oid
 	   << " " << name << " (" << val.length() << " bytes)"
 	   << dendl;
   int r = 0;
-  if (val.is_partial()) {
-    auto& b = o->onode.attrs[name.c_str()] = bufferptr(val.c_str(),
-						       val.length());
+  if (!val.length()) {
+    auto& b = o->onode.attrs[name.c_str()] = bufferptr("", 0);
     b.reassign_to_mempool(mempool::mempool_bluestore_cache_meta);
-  } else {
-    auto& b = o->onode.attrs[name.c_str()] = val;
+  } 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);
   }
   txc->write_onode(o);

src/os/bluestore/BlueStore.h

@@ -3862,7 +3862,7 @@ class BlueStore : public ObjectStore,
 	       CollectionRef& c,
 	       OnodeRef& o,
 	       const std::string& name,
-	       ceph::buffer::ptr& val);
+	       ceph::buffer::list& val);
   int _setattrs(TransContext *txc,
 		CollectionRef& c,
 		OnodeRef& o,