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

mimic: core: should report EINVAL in ErasureCode::parse() if m<=0 #28995

Merged
merged 3 commits into from Jul 18, 2019
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
2 changes: 1 addition & 1 deletion qa/standalone/erasure-code/test-erasure-code.sh
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions qa/workunits/cephtool/test.sh
Expand Up @@ -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()
Expand Down
9 changes: 6 additions & 3 deletions src/erasure-code/ErasureCode.cc
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/erasure-code/ErasureCode.h
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/erasure-code/isa/ErasureCodeIsa.cc
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/erasure-code/jerasure/ErasureCodeJerasure.cc
Expand Up @@ -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;
}

Expand Down
6 changes: 5 additions & 1 deletion src/mon/OSDMonitor.cc
Expand Up @@ -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<int>(1, erasure_code->get_coding_chunk_count() - 1);
assert(*min_size <= *size);
assert(*min_size >= erasure_code->get_data_chunk_count());
}
}
break;
Expand Down
2 changes: 1 addition & 1 deletion src/test/erasure-code/TestErasureCodeLrc.cc
Expand Up @@ -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));
}
}

Expand Down