From ef9013445d560d46b82c696168e1b21accd75aaf Mon Sep 17 00:00:00 2001 From: xie xingguo Date: Thu, 22 Sep 2016 19:07:31 +0800 Subject: [PATCH] os/bluestore: drop the global bluestore_csum option We don't need this option as it can be properly covered by the bluestore_csum_type option. Signed-off-by: xie xingguo --- src/common/config_opts.h | 3 +-- src/os/bluestore/BlueStore.cc | 13 ++++-------- src/test/objectstore/store_test.cc | 33 +++++------------------------- 3 files changed, 10 insertions(+), 39 deletions(-) diff --git a/src/common/config_opts.h b/src/common/config_opts.h index 1ddfa4f644506..5f55e00602fd2 100644 --- a/src/common/config_opts.h +++ b/src/common/config_opts.h @@ -960,8 +960,7 @@ OPTION(bluestore_block_wal_path, OPT_STR, "") OPTION(bluestore_block_wal_size, OPT_U64, 96 * 1024*1024) // rocksdb wal OPTION(bluestore_block_wal_create, OPT_BOOL, false) OPTION(bluestore_block_preallocate_file, OPT_BOOL, false) //whether preallocate space if block/db_path/wal_path is file rather that block device. -OPTION(bluestore_csum, OPT_BOOL, true) -OPTION(bluestore_csum_type, OPT_STR, "crc32c") +OPTION(bluestore_csum_type, OPT_STR, "crc32c") // none|xxhash32|xxhash64|crc32c|crc32c_16|crc32c_8 OPTION(bluestore_min_csum_block, OPT_U32, 4096) OPTION(bluestore_max_csum_block, OPT_U32, 64*1024) OPTION(bluestore_min_alloc_size, OPT_U32, 0) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 1da5cc056e5e4..0ec048f6f29d3 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -2222,7 +2222,6 @@ BlueStore::~BlueStore() const char **BlueStore::get_tracked_conf_keys() const { static const char* KEYS[] = { - "bluestore_csum", "bluestore_csum_type", "bluestore_compression", "bluestore_compression_algorithm", @@ -2236,8 +2235,7 @@ const char **BlueStore::get_tracked_conf_keys() const void BlueStore::handle_conf_change(const struct md_config_t *conf, const std::set &changed) { - if (changed.count("bluestore_csum_type") || - changed.count("bluestore_csum")) { + if (changed.count("bluestore_csum_type")) { _set_csum(); } if (changed.count("bluestore_compression") || @@ -2303,12 +2301,9 @@ void BlueStore::_set_compression() void BlueStore::_set_csum() { csum_type = bluestore_blob_t::CSUM_NONE; - if (g_conf->bluestore_csum) { - int t = bluestore_blob_t::get_csum_string_type( - g_conf->bluestore_csum_type); - if (t > bluestore_blob_t::CSUM_NONE) - csum_type = t; - } + int t = bluestore_blob_t::get_csum_string_type(g_conf->bluestore_csum_type); + if (t > bluestore_blob_t::CSUM_NONE) + csum_type = t; dout(10) << __func__ << " csum_type " << bluestore_blob_t::get_csum_type_string(csum_type) diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc index 0248ae45c302d..e93db7b174d65 100644 --- a/src/test/objectstore/store_test.cc +++ b/src/test/objectstore/store_test.cc @@ -3995,9 +3995,8 @@ TEST_P(StoreTest, SyntheticMatrixCsumAlgorithm) { { "max_size", "1048576", 0 }, { "alignment", "16", 0 }, { "bluestore_min_alloc_size", "65536", 0 }, - { "bluestore_csum", "true", 0 }, { "bluestore_csum_type", "crc32c", "crc32c_16", "crc32c_8", "xxhash32", - "xxhash64", 0 }, + "xxhash64", "none", 0 }, { 0 }, }; do_matrix(m, store); @@ -4013,7 +4012,6 @@ TEST_P(StoreTest, SyntheticMatrixCsumVsCompression) { { "alignment", "512", 0 }, { "bluestore_min_alloc_size", "32768", "4096", 0 }, { "bluestore_compression", "force", "none", 0}, - { "bluestore_csum", "true", 0 }, { "bluestore_csum_type", "crc32c", 0 }, { "bluestore_default_buffered_read", "true", "false", 0 }, { 0 }, @@ -4061,7 +4059,7 @@ TEST_P(StoreTest, SyntheticMatrixNoCsum) { { "alignment", "512", 0 }, { "bluestore_min_alloc_size", "65536", "4096", 0 }, { "bluestore_compression", "force", "none", 0}, - { "bluestore_csum", "false", 0 }, + { "bluestore_csum_type", "none", 0}, { "bluestore_default_buffered_read", "true", "false", 0 }, { 0 }, }; @@ -5060,7 +5058,6 @@ TEST_P(StoreTest, TryMoveRename) { 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); @@ -5094,8 +5091,7 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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->set_val("bluestore_csum_type", "none"); g_conf->apply_changes(NULL); bufferlist in; @@ -5106,9 +5102,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { } { //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; @@ -5122,7 +5115,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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); @@ -5133,9 +5125,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { } { //'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; @@ -5148,8 +5137,7 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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->set_val("bluestore_csum_type", "none"); g_conf->apply_changes(NULL); ObjectStore::Transaction t2; @@ -5167,7 +5155,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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(); @@ -5181,9 +5168,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { } { //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; @@ -5198,8 +5182,7 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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->set_val("bluestore_csum_type", "none"); g_conf->apply_changes(NULL); ObjectStore::Transaction t2; @@ -5224,7 +5207,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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); @@ -5252,9 +5234,6 @@ TEST_P(StoreTest, BluestoreOnOffCSumTest) { 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( @@ -5369,7 +5348,6 @@ TEST_P(StoreTest, Many4KWritesTest) { TEST_P(StoreTest, Many4KWritesNoCSumTest) { if (string(GetParam()) != "bluestore") return; - g_conf->set_val("bluestore_csum", "false"); g_conf->set_val("bluestore_csum_type", "none"); g_ceph_context->_conf->apply_changes(NULL); store_statfs_t res_stat; @@ -5379,7 +5357,6 @@ TEST_P(StoreTest, Many4KWritesNoCSumTest) { ASSERT_LE(res_stat.stored, max_object); ASSERT_EQ(res_stat.allocated, max_object); - g_conf->set_val("bluestore_csum", "true"); g_conf->set_val("bluestore_csum_type", "crc32c"); } -- 2.39.5