Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/bluestore: proper handling for csum enable/disable settings #10431

Merged
merged 2 commits into from Aug 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/os/bluestore/BlueStore.cc
Expand Up @@ -3613,7 +3613,7 @@ int BlueStore::_do_read(
} else {
for (auto reg : b2r_it->second) {
// determine how much of the blob to read
uint64_t chunk_size = bptr->blob.get_chunk_size(block_size);
uint64_t chunk_size = bptr->blob.get_chunk_size(csum_type != bluestore_blob_t::CSUM_NONE, block_size);
uint64_t r_off = reg.blob_xoffset;
uint64_t r_len = reg.length;
unsigned front = r_off % chunk_size;
Expand Down Expand Up @@ -3692,7 +3692,7 @@ int BlueStore::_verify_csum(OnodeRef& o,
const bufferlist& bl) const
{
int bad;
int r = blob->verify_csum(blob_xoffset, bl, &bad);
int r = csum_type != bluestore_blob_t::CSUM_NONE ? blob->verify_csum(blob_xoffset, bl, &bad) :0;
if (r < 0) {
if (r == -1) {
vector<bluestore_pextent_t> pex;
Expand Down Expand Up @@ -5768,7 +5768,7 @@ void BlueStore::_do_write_small(
<< " bstart 0x" << std::hex << bstart << std::dec << dendl;

// can we pad our head/tail out with zeros?
uint64_t chunk_size = b->blob.get_chunk_size(block_size);
uint64_t chunk_size = b->blob.get_chunk_size(b->blob.has_csum(), block_size); //blob csum settings to be applied hence ignoring current config settings for csum enable/disable
uint64_t head_pad = P2PHASE(offset, chunk_size);
if (head_pad && o->onode.has_any_lextents(offset - head_pad, chunk_size)) {
head_pad = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/os/bluestore/bluestore_types.h
Expand Up @@ -328,8 +328,8 @@ struct bluestore_blob_t {
}

/// return chunk (i.e. min readable block) size for the blob
uint64_t get_chunk_size(uint64_t dev_block_size) {
return has_csum() ? MAX(dev_block_size, get_csum_chunk_size()) : dev_block_size;
uint64_t get_chunk_size(bool csum_enabled, uint64_t dev_block_size) {
return csum_enabled && has_csum() ? MAX(dev_block_size, get_csum_chunk_size()) : dev_block_size;
}
uint32_t get_csum_chunk_size() const {
return 1 << csum_chunk_order;
Expand Down
200 changes: 200 additions & 0 deletions src/test/objectstore/store_test.cc
Expand Up @@ -4913,6 +4913,206 @@ TEST_P(StoreTest, TryMoveRename) {
ASSERT_EQ(store->stat(cid, hoid2, &st), 0);
}

TEST_P(StoreTest, BluestoreOnOffCSumTest) {
if (string(GetParam()) != "bluestore")
return;
g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

ObjectStore::Sequencer osr("test");
int r;
coll_t cid;
ghobject_t hoid(hobject_t(sobject_t("Object 1", CEPH_NOSNAP)));
{
bufferlist in;
r = store->read(cid, hoid, 0, 5, in);
ASSERT_EQ(-ENOENT, r);
}
{
ObjectStore::Transaction t;
t.create_collection(cid, 0);
cerr << "Creating collection " << cid << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
{
//write with csum enabled followed by read with csum disabled
size_t block_size = 64*1024;
ObjectStore::Transaction t;
bufferlist bl, orig;
bl.append(std::string(block_size, 'a'));
orig = bl;
t.remove(cid, hoid);
t.set_alloc_hint(cid, hoid, 4*1024*1024, 1024*8, 0);
t.write(cid, hoid, 0, bl.length(), bl);
cerr << "Remove then create" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);

g_conf->set_val("bluestore_csum", "false");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

bufferlist in;
r = store->read(cid, hoid, 0, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));

}
{
//write with csum disabled followed by read with csum enabled
g_conf->set_val("bluestore_csum", "false");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

size_t block_size = 64*1024;
ObjectStore::Transaction t;
bufferlist bl, orig;
bl.append(std::string(block_size, 'a'));
orig = bl;
t.remove(cid, hoid);
t.set_alloc_hint(cid, hoid, 4*1024*1024, 1024*8, 0);
t.write(cid, hoid, 0, bl.length(), bl);
cerr << "Remove then create" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);

g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

bufferlist in;
r = store->read(cid, hoid, 0, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));
}
{
//'mixed' non-overlapping writes to the same blob
g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

ObjectStore::Transaction t;
bufferlist bl, orig;
size_t block_size = 8000;
bl.append(std::string(block_size, 'a'));
orig = bl;
t.remove(cid, hoid);
t.write(cid, hoid, 0, bl.length(), bl);
cerr << "Remove then create" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);

g_conf->set_val("bluestore_csum", "false");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

ObjectStore::Transaction t2;
t2.write(cid, hoid, block_size*2, bl.length(), bl);
cerr << "Append 'unprotected'" << std::endl;
r = apply_transaction(store, &osr, std::move(t2));
ASSERT_EQ(r, 0);

bufferlist in;
r = store->read(cid, hoid, 0, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));
in.clear();
r = store->read(cid, hoid, block_size*2, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));

g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);
in.clear();
r = store->read(cid, hoid, 0, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));
in.clear();
r = store->read(cid, hoid, block_size*2, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));
}
{
//partially blob overwrite under a different csum enablement mode
g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

ObjectStore::Transaction t;
bufferlist bl, orig, orig2;
size_t block_size0 = 0x10000;
size_t block_size = 9000;
size_t block_size2 = 5000;
bl.append(std::string(block_size0, 'a'));
t.remove(cid, hoid);
t.set_alloc_hint(cid, hoid, 4*1024*1024, 1024*8, 0);
t.write(cid, hoid, 0, bl.length(), bl);
cerr << "Remove then create" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);

g_conf->set_val("bluestore_csum", "false");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

ObjectStore::Transaction t2;
bl.clear();
bl.append(std::string(block_size, 'b'));
t2.write(cid, hoid, 0, bl.length(), bl);
t2.write(cid, hoid, block_size0, bl.length(), bl);
cerr << "Overwrite with unprotected data" << std::endl;
r = apply_transaction(store, &osr, std::move(t2));
ASSERT_EQ(r, 0);

orig = bl;
orig2 = bl;
orig.append( std::string(block_size0 - block_size, 'a'));

bufferlist in;
r = store->read(cid, hoid, 0, block_size0, in);
ASSERT_EQ((int)block_size0, r);
ASSERT_TRUE(bl_eq(orig, in));

r = store->read(cid, hoid, block_size0, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig2, in));

g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);

ObjectStore::Transaction t3;
bl.clear();
bl.append(std::string(block_size2, 'c'));
t3.write(cid, hoid, block_size0, bl.length(), bl);
cerr << "Overwrite with protected data" << std::endl;
r = apply_transaction(store, &osr, std::move(t3));
ASSERT_EQ(r, 0);

in.clear();
orig = bl;
orig.append( std::string(block_size - block_size2, 'b'));
r = store->read(cid, hoid, block_size0, block_size, in);
ASSERT_EQ((int)block_size, r);
ASSERT_TRUE(bl_eq(orig, in));
}

{
ObjectStore::Transaction t;
t.remove(cid, hoid);
t.remove_collection(cid);
cerr << "Cleaning" << std::endl;
r = apply_transaction(store, &osr, std::move(t));
ASSERT_EQ(r, 0);
}
g_conf->set_val("bluestore_csum", "true");
g_conf->set_val("bluestore_csum_type", "crc32c");
g_conf->apply_changes(NULL);
}

INSTANTIATE_TEST_CASE_P(
ObjectStore,
StoreTest,
Expand Down