os/bluestore: remove duplicate sbid member for Blob, use one from shared_blob member

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 56e8bc258fa6b352568d1f14a2ec2cec6ff785ab..4473ba982ba97c3758bcdef55930f1e7c1174f97 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1814,7 +1814,7 @@ bool BlueStore::ExtentMap::encode_some(uint32_t offset, uint32_t length,
     denc_varint(0, bound); // logical_offset
     denc_varint(0, bound); // len
     denc_varint(0, bound); // blob_offset
-    p->blob->bound_encode(bound, struct_v, false);
+    p->blob->bound_encode(bound, struct_v, p->blob->shared_blob->sbid, false);
   }
 
   {
@@ -1866,7 +1866,7 @@ bool BlueStore::ExtentMap::encode_some(uint32_t offset, uint32_t length,
       }
       pos = p->logical_end();
       if (include_blob) {
-       p->blob->encode(app, struct_v, false);
+       p->blob->encode(app, struct_v, p->blob->shared_blob->sbid, false);
       }
     }
   }
@@ -1926,9 +1926,10 @@ void BlueStore::ExtentMap::decode_some(bufferlist& bl)
        assert(le->blob);
       } else {
        Blob *b = new Blob();
-       b->decode(p, struct_v, false);
+        uint64_t sbid = 0;
+       b->decode(p, struct_v, &sbid, false);
        blobs[n] = b;
-       onode->c->open_shared_blob(b);
+       onode->c->open_shared_blob(sbid, b);
        le->assign_blob(b);
       }
       // we build ref_map dynamically for non-spanning blobs
@@ -1951,7 +1952,7 @@ void BlueStore::ExtentMap::bound_encode_spanning_blobs(size_t& p)
   denc_varint((uint32_t)0, key_size);
   p += spanning_blob_map.size() * key_size;
   for (const auto& i : spanning_blob_map) {
-    i.second->bound_encode(p, struct_v, true);
+    i.second->bound_encode(p, struct_v, i.second->shared_blob->sbid, true);
   }
 }
 
@@ -1963,7 +1964,7 @@ void BlueStore::ExtentMap::encode_spanning_blobs(
   denc_varint(spanning_blob_map.size(), p);
   for (auto& i : spanning_blob_map) {
     denc_varint(i.second->id, p);
-    i.second->encode(p, struct_v, true);
+    i.second->encode(p, struct_v, i.second->shared_blob->sbid, true);
   }
 }
 
@@ -1980,8 +1981,9 @@ void BlueStore::ExtentMap::decode_spanning_blobs(
     BlobRef b(new Blob());
     denc_varint(b->id, p);
     spanning_blob_map[b->id] = b;
-    b->decode(p, struct_v, true);
-    c->open_shared_blob(b);
+    uint64_t sbid = 0;
+    b->decode(p, struct_v, &sbid, true);
+    c->open_shared_blob(sbid, b);
   }
 }
 
@@ -2313,7 +2315,7 @@ BlueStore::Collection::Collection(BlueStore *ns, Cache *c, coll_t cid)
 {
 }
 
-void BlueStore::Collection::open_shared_blob(BlobRef b)
+void BlueStore::Collection::open_shared_blob(uint64_t sbid, BlobRef b)
 {
   assert(!b->shared_blob);
   const bluestore_blob_t& blob = b->get_blob();
@@ -2322,14 +2324,14 @@ void BlueStore::Collection::open_shared_blob(BlobRef b)
     return;
   }
 
-  b->shared_blob = shared_blob_set.lookup(blob.sbid);
+  b->shared_blob = shared_blob_set.lookup(sbid);
   if (b->shared_blob) {
-    dout(10) << __func__ << " sbid 0x" << std::hex << blob.sbid << std::dec
+    dout(10) << __func__ << " sbid 0x" << std::hex << sbid << std::dec
             << " had " << *b->shared_blob << dendl;
   } else {
-    b->shared_blob = new SharedBlob(blob.sbid, cache);
+    b->shared_blob = new SharedBlob(sbid, cache);
     shared_blob_set.add(b->shared_blob.get());
-    dout(10) << __func__ << " sbid 0x" << std::hex << blob.sbid << std::dec
+    dout(10) << __func__ << " sbid 0x" << std::hex << sbid << std::dec
             << " opened " << *b->shared_blob << dendl;
   }
 }
@@ -2354,7 +2356,7 @@ void BlueStore::Collection::load_shared_blob(SharedBlobRef sb)
   sb->loaded = true;
 }
 
-void BlueStore::Collection::make_blob_shared(BlobRef b)
+void BlueStore::Collection::make_blob_shared(uint64_t sbid, BlobRef b)
 {
   dout(10) << __func__ << " " << *b << dendl;
   bluestore_blob_t& blob = b->dirty_blob();
@@ -2365,7 +2367,7 @@ void BlueStore::Collection::make_blob_shared(BlobRef b)
 
   // update shared blob
   b->shared_blob->loaded = true;  // we are new and therefore up to date
-  b->shared_blob->sbid = blob.sbid;
+  b->shared_blob->sbid = sbid;
   shared_blob_set.add(b->shared_blob.get());
   for (auto p : blob.extents) {
     if (p.is_valid()) {
@@ -4435,18 +4437,18 @@ int BlueStore::fsck(bool deep)
          }
        }
        if (blob.is_shared()) {
-         if (blob.sbid > blobid_max) {
+         if (i.first->shared_blob->sbid > blobid_max) {
            derr << __func__ << " " << oid << " blob " << blob
-                << " sbid " << blob.sbid << " > blobid_max "
+                << " sbid " << i.first->shared_blob->sbid << " > blobid_max "
                 << blobid_max << dendl;
            ++errors;
-         } else if (blob.sbid == 0) {
+         } else if (i.first->shared_blob->sbid == 0) {
             derr << __func__ << " " << oid << " blob " << blob
                  << " marked as shared but has uninitialized sbid"
                  << dendl;
             ++errors;
           }
-         sb_info_t& sbi = sb_info[blob.sbid];
+         sb_info_t& sbi = sb_info[i.first->shared_blob->sbid];
          sbi.sb = i.first->shared_blob;
          sbi.oids.push_back(oid);
          sbi.compressed = blob.is_compressed();
@@ -8676,8 +8678,7 @@ int BlueStore::_do_clone_range(
       const bluestore_blob_t& blob = e.blob->get_blob();
       // make sure it is shared
       if (!blob.is_shared()) {
-       e.blob->dirty_blob().sbid = _assign_blobid(txc);
-       c->make_blob_shared(e.blob);
+       c->make_blob_shared(_assign_blobid(txc), e.blob);
        dirtied_oldo = true;  // fixme: overkill
       } else if (!e.blob->shared_blob->loaded) {
        c->load_shared_blob(e.blob->shared_blob);

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index d1673edfc5bd881946248567539212b4e25e2040..b92777f5cd2be960eae691e38864e7ba15455c5d 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -528,22 +528,31 @@ public:
       }
     }
 #else
-    void bound_encode(size_t& p, uint64_t struct_v, bool include_ref_map) const {
+    void bound_encode(size_t& p, uint64_t struct_v, uint64_t sbid, bool include_ref_map) const {
       denc(blob, p, struct_v);
+      if (blob.is_shared()) {
+        denc(sbid, p);
+      }
       if (include_ref_map) {
         ref_map.bound_encode(p);
       }
     }
-    void encode(bufferlist::contiguous_appender& p, uint64_t struct_v,
+    void encode(bufferlist::contiguous_appender& p, uint64_t struct_v, uint64_t sbid,
                bool include_ref_map) const {
       denc(blob, p, struct_v);
+      if (blob.is_shared()) {
+        denc(sbid, p);
+      }
       if (include_ref_map) {
         ref_map.encode(p);
       }
     }
-    void decode(bufferptr::iterator& p, uint64_t struct_v,
+    void decode(bufferptr::iterator& p, uint64_t struct_v, uint64_t* sbid,
                bool include_ref_map) {
       denc(blob, p, struct_v);
+      if (blob.is_shared()) {
+        denc(*sbid, p);
+      }
       if (include_ref_map) {
         ref_map.decode(p);
       }
@@ -1105,9 +1114,9 @@ public:
     //  open = SharedBlob is instantiated
     //  shared = blob_t shared flag is set; SharedBlob is hashed.
     //  loaded = SharedBlob::shared_blob_t is loaded from kv store
-    void open_shared_blob(BlobRef b);
+    void open_shared_blob(uint64_t sbid, BlobRef b);
     void load_shared_blob(SharedBlobRef sb);
-    void make_blob_shared(BlobRef b);
+    void make_blob_shared(uint64_t sbid, BlobRef b);
 
     BlobRef new_blob() {
       BlobRef b = new Blob;

diff --git a/src/os/bluestore/bluestore_types.cc b/src/os/bluestore/bluestore_types.cc
index 0781c1135ac5b26f07744605709adab618bcf43e..01630cbc1df87a7e7d2a371a0c7ba47d86e376b2 100644 (file)
--- a/src/os/bluestore/bluestore_types.cc
+++ b/src/os/bluestore/bluestore_types.cc
@@ -400,7 +400,6 @@ void bluestore_blob_t::dump(Formatter *f) const
     f->dump_object("extent", p);
   }
   f->close_section();
-  f->dump_unsigned("shared_blob_id", sbid);
   f->dump_unsigned("compressed_length_original", compressed_length_orig);
   f->dump_unsigned("compressed_length", compressed_length);
   f->dump_unsigned("flags", flags);
@@ -434,9 +433,6 @@ void bluestore_blob_t::generate_test_instances(list<bluestore_blob_t*>& ls)
 ostream& operator<<(ostream& out, const bluestore_blob_t& o)
 {
   out << "blob(" << o.extents;
-  if (o.sbid) {
-    out << " sbid 0x" << std::hex << o.sbid << std::dec;
-  }
   if (o.is_compressed()) {
     out << " clen 0x" << std::hex
        << o.compressed_length_orig

diff --git a/src/os/bluestore/bluestore_types.h b/src/os/bluestore/bluestore_types.h
index 1cb41596477951910b2fba5fd1b095e82cba3be9..2e684791bf511544a965245c271f8390a6d0ad96 100644 (file)
--- a/src/os/bluestore/bluestore_types.h
+++ b/src/os/bluestore/bluestore_types.h
@@ -280,7 +280,6 @@ struct bluestore_blob_t {
   static string get_flags_string(unsigned flags);
 
   vector<bluestore_pextent_t> extents;///< raw data position on device
-  uint64_t sbid = 0;                  ///< shared blob id (if shared)
   uint32_t compressed_length_orig = 0;///< original length of compressed blob if any
   uint32_t compressed_length = 0;     ///< compressed length if any
   uint32_t flags = 0;                 ///< FLAG_*
@@ -301,7 +300,6 @@ struct bluestore_blob_t {
     assert(struct_v == 1);
     denc(extents, p);
     denc_varint(flags, p);
-    denc_varint(sbid, p);
     denc_varint_lowz(compressed_length_orig, p);
     denc_varint_lowz(compressed_length, p);
     denc(csum_type, p);
@@ -315,9 +313,6 @@ struct bluestore_blob_t {
     assert(struct_v == 1);
     denc(extents, p);
     denc_varint(flags, p);
-    if (is_shared()) {
-      denc_varint(sbid, p);
-    }
     if (is_compressed()) {
       denc_varint_lowz(compressed_length_orig, p);
       denc_varint_lowz(compressed_length, p);
@@ -338,9 +333,6 @@ struct bluestore_blob_t {
     assert(struct_v == 1);
     denc(extents, p);
     denc_varint(flags, p);
-    if (is_shared()) {
-      denc_varint(sbid, p);
-    }
     if (is_compressed()) {
       denc_varint_lowz(compressed_length_orig, p);
       denc_varint_lowz(compressed_length, p);

diff --git a/src/test/objectstore/test_bluestore_types.cc b/src/test/objectstore/test_bluestore_types.cc
index ca3bcaad6b5e508076de60186d098b4a463fb9ab..4c63bef32c72591728451ca19ebc2126acb47c18 100644 (file)
--- a/src/test/objectstore/test_bluestore_types.cc
+++ b/src/test/objectstore/test_bluestore_types.cc
@@ -688,7 +688,6 @@ TEST(Blob, put_ref)
     b.extents.push_back(bluestore_pextent_t(0x40118000, 0x7000));
     b.set_flag(bluestore_blob_t::FLAG_SHARED);
     b.init_csum(Checksummer::CSUM_CRC32C, 12, 0x1e000);
-    b.sbid = 0xcf92e;
 
     cout << "before: " << B << std::endl;
     vector<bluestore_pextent_t> r;