From d97e97b64b95cd3c6806ae1e15921fa7ede1da65 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 11 Mar 2019 12:52:23 -0500 Subject: [PATCH 1/3] mon/OSDMonitor: set ec min_size to k + min(1, m - 1) Signed-off-by: Sage Weil (cherry picked from commit 7f49be2104f0a5df894a53e200c431c527781f9c) --- src/mon/OSDMonitor.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 0c99b03871b73..c0bdb483d351b 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -6295,7 +6295,11 @@ int OSDMonitor::prepare_pool_size(const unsigned pool_type, err = get_erasure_code(erasure_code_profile, &erasure_code, ss); if (err == 0) { *size = erasure_code->get_chunk_count(); - *min_size = std::min(erasure_code->get_data_chunk_count() + 1, *size); + *min_size = + erasure_code->get_data_chunk_count() + + std::min(1, erasure_code->get_coding_chunk_count() - 1); + assert(*min_size <= *size); + assert(*min_size >= erasure_code->get_data_chunk_count()); } } break; From 365ec76cb84713d50a717636ed115bd098b13497 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Tue, 12 Mar 2019 08:57:21 -0500 Subject: [PATCH 2/3] erasure-code: ensure m >= 1 Fixes: http://tracker.ceph.com/issues/38682 Signed-off-by: Sage Weil (cherry picked from commit ab0a6528266ab30d442f351de97f5475399ff278) Conflicts: src/erasure-code/clay/ErasureCodeClay.cc - file does not exist in mimic --- qa/workunits/cephtool/test.sh | 10 ++++++++++ src/erasure-code/ErasureCode.cc | 9 ++++++--- src/erasure-code/ErasureCode.h | 2 +- src/erasure-code/isa/ErasureCodeIsa.cc | 2 +- src/erasure-code/jerasure/ErasureCodeJerasure.cc | 2 +- src/test/erasure-code/TestErasureCodeLrc.cc | 2 +- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/qa/workunits/cephtool/test.sh b/qa/workunits/cephtool/test.sh index 668778681404c..f6931fc9a16fd 100755 --- a/qa/workunits/cephtool/test.sh +++ b/qa/workunits/cephtool/test.sh @@ -2220,6 +2220,16 @@ function test_mon_osd_erasure_code() # clean up ceph osd erasure-code-profile rm fooprofile ceph osd erasure-code-profile rm barprofile + + # try weird k and m values + expect_false ceph osd erasure-code-profile set badk k=1 m=1 + expect_false ceph osd erasure-code-profile set badk k=1 m=2 + expect_false ceph osd erasure-code-profile set badk k=0 m=2 + expect_false ceph osd erasure-code-profile set badk k=-1 m=2 + expect_false ceph osd erasure-code-profile set badm k=2 m=0 + expect_false ceph osd erasure-code-profile set badm k=2 m=-1 + ceph osd erasure-code-profile set good k=2 m=1 + ceph osd erasure-code-profile rm good } function test_mon_osd_misc() diff --git a/src/erasure-code/ErasureCode.cc b/src/erasure-code/ErasureCode.cc index 45607bf2f4b2a..e474b70258da4 100644 --- a/src/erasure-code/ErasureCode.cc +++ b/src/erasure-code/ErasureCode.cc @@ -72,14 +72,17 @@ int ErasureCode::create_rule( return ruleid; } -int ErasureCode::sanity_check_k(int k, ostream *ss) +int ErasureCode::sanity_check_k_m(int k, int m, ostream *ss) { if (k < 2) { *ss << "k=" << k << " must be >= 2" << std::endl; return -EINVAL; - } else { - return 0; } + if (m < 1) { + *ss << "m=" << m << " must be >= 1" << std::endl; + return -EINVAL; + } + return 0; } int ErasureCode::chunk_index(unsigned int i) const diff --git a/src/erasure-code/ErasureCode.h b/src/erasure-code/ErasureCode.h index ec8dd31d5cfd5..97783a9929765 100644 --- a/src/erasure-code/ErasureCode.h +++ b/src/erasure-code/ErasureCode.h @@ -50,7 +50,7 @@ namespace ceph { CrushWrapper &crush, std::ostream *ss) const; - int sanity_check_k(int k, std::ostream *ss); + int sanity_check_k_m(int k, int m, std::ostream *ss); unsigned int get_coding_chunk_count() const override { return get_chunk_count() - get_data_chunk_count(); diff --git a/src/erasure-code/isa/ErasureCodeIsa.cc b/src/erasure-code/isa/ErasureCodeIsa.cc index 46c8e830173ae..ee6a31491be4f 100644 --- a/src/erasure-code/isa/ErasureCodeIsa.cc +++ b/src/erasure-code/isa/ErasureCodeIsa.cc @@ -325,7 +325,7 @@ int ErasureCodeIsaDefault::parse(ErasureCodeProfile &profile, int err = ErasureCode::parse(profile, ss); err |= to_int("k", profile, &k, DEFAULT_K, ss); err |= to_int("m", profile, &m, DEFAULT_M, ss); - err |= sanity_check_k(k, ss); + err |= sanity_check_k_m(k, m, ss); if (matrixtype == kVandermonde) { // these are verified safe values evaluated using the diff --git a/src/erasure-code/jerasure/ErasureCodeJerasure.cc b/src/erasure-code/jerasure/ErasureCodeJerasure.cc index ea76b22f00402..dbe0109da5840 100644 --- a/src/erasure-code/jerasure/ErasureCodeJerasure.cc +++ b/src/erasure-code/jerasure/ErasureCodeJerasure.cc @@ -66,7 +66,7 @@ int ErasureCodeJerasure::parse(ErasureCodeProfile &profile, chunk_mapping.clear(); err = -EINVAL; } - err |= sanity_check_k(k, ss); + err |= sanity_check_k_m(k, m, ss); return err; } diff --git a/src/test/erasure-code/TestErasureCodeLrc.cc b/src/test/erasure-code/TestErasureCodeLrc.cc index 21926af2b93eb..6257fbafaadd4 100644 --- a/src/test/erasure-code/TestErasureCodeLrc.cc +++ b/src/test/erasure-code/TestErasureCodeLrc.cc @@ -392,7 +392,7 @@ TEST(ErasureCodeLrc, layers_sanity_checks) " [ \"DD\", \"\" ], " "]"; profile["layers"] = description_string; - EXPECT_EQ(ERROR_LRC_MAPPING_SIZE, lrc.init(profile, &cerr)); + EXPECT_EQ(-EINVAL, lrc.init(profile, &cerr)); } } From 8afc1332ea7f0697e27b0830b0f6758c3d6d7eb2 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 13 Mar 2019 12:46:50 -0500 Subject: [PATCH 3/3] qa/standalone/erasure-code/test-erasure-code: adjust test to avoid m=0 _DD is k=2 m=0, which we don't allow. Switch it to cDD. I confess I don't fully understand why this was _DD to begin with, but I'm pretty sure mapping is there to control the order of results so that it can be mapped to the CRUSH rule output sanely, and the coding portion is not relevant to the test. Signed-off-by: Sage Weil (cherry picked from commit 52d5797c3d3d74dd88664cfe85e5d551e7b334a6) --- qa/standalone/erasure-code/test-erasure-code.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/standalone/erasure-code/test-erasure-code.sh b/qa/standalone/erasure-code/test-erasure-code.sh index ecdc25421ca7e..fb522bbd5b7c1 100755 --- a/qa/standalone/erasure-code/test-erasure-code.sh +++ b/qa/standalone/erasure-code/test-erasure-code.sh @@ -308,7 +308,7 @@ function TEST_chunk_mapping() { ceph osd erasure-code-profile set remap-profile \ plugin=lrc \ - layers='[ [ "_DD", "" ] ]' \ + layers='[ [ "cDD", "" ] ]' \ mapping='_DD' \ crush-steps='[ [ "choose", "osd", 0 ] ]' || return 1 ceph osd erasure-code-profile get remap-profile