Skip to content

Commit

Permalink
os/bluestore: remove duplicate sbid member for Blob, use one from sha…
Browse files Browse the repository at this point in the history 
…red_blob member

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
  • Loading branch information
Igor Fedotov committed Dec 19, 2016
1 parent a6158d5 commit 7d1f46b
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 39 deletions.
43 changes: 22 additions & 21 deletions src/os/bluestore/BlueStore.cc
View file
Expand Up @@ -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);
}

{
Expand Down Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}

Expand All @@ -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);
}
}

Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
}
Expand All @@ -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();
Expand All @@ -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()) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 14 additions & 5 deletions src/os/bluestore/BlueStore.h
View file
Expand Up @@ -528,22 +528,31 @@ class BlueStore : public ObjectStore,
}
}
#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);
}
Expand Down Expand Up @@ -1105,9 +1114,9 @@ class BlueStore : public ObjectStore,
// 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;
Expand Down
4 changes: 0 additions & 4 deletions src/os/bluestore/bluestore_types.cc
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions src/os/bluestore/bluestore_types.h
View file
Expand Up @@ -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_*
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/test/objectstore/test_bluestore_types.cc
View file
Expand Up @@ -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;
Expand Down

0 comments on commit 7d1f46b

Please sign in to comment.