os/bluestore: end scope of std::hex properly; convert csum error to EIO

Mark's comments:

This passed "ceph_test_objectstore --gtest_filter=*/2".
This PR did not appear to have a significant impact on performance tests.

Closes #10225

os/bluestore: end scope of std::hex properly

To avoid side-effects by accident.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
os/bluestore: convert csum error to EIO

The verify_csum() method either returns -1 or -EOPNOTSUPP, which
is not very proper and difficult for user understanding.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
os/bluestore: assert lextent is shared

Otherwise we are risking of accessing violation.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
os/bluestore: drop duplicated assignment of result code

These two methods never fail actually.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
os/bluestore: improve _do_read() a little

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
os/bluestore: assert decoding of shard of key to be successful

Otherwise we are risking of acessing null pointer.

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

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index afcba07ba7504e5dd9a3ab211716a5690ebdb5bd..361ebbabe93b8cc1e72bca4ac79506ab590e51c2 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -188,8 +188,10 @@ static const char *_key_decode_shard(const char *key, shard_id_t *pshard)
   } else {
     unsigned shard;
     int r = sscanf(key, "%x", &shard);
-    if (r < 1)
+    if (r < 1) {
+      assert(0 == "invalid shard of key");
       return NULL;
+    }
     *pshard = shard_id_t(shard);
   }
   return key + 2;
@@ -3480,6 +3482,15 @@ int BlueStore::_do_read(
   map<uint64_t,bluestore_lextent_t>::iterator ep, eend;
   int r = 0;
 
+  dout(20) << __func__ << " 0x" << std::hex << offset << "~" << length
+           << " size 0x" << o->onode.size << " (" << std::dec
+           << o->onode.size << ")" << dendl;
+  bl.clear();
+
+  if (offset >= o->onode.size) {
+    return r;
+  }
+
   // generally, don't buffer anything, unless the client explicitly requests
   // it.
   bool buffered = false;
@@ -3493,15 +3504,6 @@ int BlueStore::_do_read(
     buffered = true;
   }
 
-  dout(20) << __func__ << " 0x" << std::hex << offset << "~" << length
-          << " size 0x" << o->onode.size << " (" << std::dec
-          << o->onode.size << ")" << dendl;
-  bl.clear();
-
-  if (offset >= o->onode.size) {
-    return r;
-  }
-
   if (offset + length > o->onode.size) {
     length = o->onode.size - offset;
   }
@@ -3530,8 +3532,8 @@ int BlueStore::_do_read(
     Blob *bptr = c->get_blob(o, lp->second.blob);
     if (bptr == nullptr) {
       dout(20) << __func__ << "  missed blob " << lp->second.blob << dendl;
+      assert(bptr != nullptr);
     }
-    assert(bptr != nullptr);
     unsigned l_off = pos - lp->first;
     unsigned b_off = l_off + lp->second.offset;
     unsigned b_len = std::min(left, lp->second.length - l_off);
@@ -3620,7 +3622,7 @@ int BlueStore::_do_read(
        dout(20) << __func__ << "    region 0x" << std::hex
                 << reg.logical_offset
                 << ": 0x" << reg.blob_xoffset << "~" << reg.length
-                << " reading 0x" << r_off << "~" << r_len
+                << " reading 0x" << r_off << "~" << r_len << std::dec
                 << dendl;
 
        // read it
@@ -3634,7 +3636,7 @@ int BlueStore::_do_read(
          });
        int r = _verify_csum(o, &bptr->blob, r_off, bl);
        if (r < 0) {
-         return r;
+         return -EIO;
        }
        if (buffered) {
          bptr->bc.did_read(r_off, bl);
@@ -6556,7 +6558,6 @@ int BlueStore::_omap_rmkeys(TransContext *txc,
   __u32 num;
 
   if (!o->onode.omap_head) {
-    r = 0;
     goto out;
   }
   ::decode(num, p);
@@ -6569,7 +6570,6 @@ int BlueStore::_omap_rmkeys(TransContext *txc,
             << " <- " << key << dendl;
     txc->t->rmkey(PREFIX_OMAP, final_key);
   }
-  r = 0;
 
  out:
   dout(10) << __func__ << " " << c->cid << " " << o->oid << " = " << r << dendl;
@@ -6602,7 +6602,6 @@ int BlueStore::_omap_rmkey_range(TransContext *txc,
     dout(30) << __func__ << "  rm " << pretty_binary_string(it->key()) << dendl;
     it->next();
   }
-  r = 0;
 
  out:
   dout(10) << __func__ << " " << c->cid << " " << o->oid << " = " << r << dendl;
@@ -6688,6 +6687,7 @@ int BlueStore::_clone(TransContext *txc,
          p.second.blob = -moved_blobs[p.second.blob];
        }
        newo->onode.extent_map[p.first] = p.second;
+        assert(p.second.blob < 0);
        newo->bnode->blob_map.get(-p.second.blob)->blob.ref_map.get(
          p.second.offset,
          p.second.length);