Actions
Bug #38682
closedshould report EINVAL in ErasureCode::parse() if m<=0
Status:
Resolved
Priority:
High
Assignee:
-
Category:
Correctness/Safety
Target version:
-
% Done:
0%
Source:
Tags:
low-hanging-fruit
Backport:
luminous,mimic
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Component(RADOS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):
Description
MDS=0 MGR=1 OSD=4 MON=1 ../src/vstart.sh -n --without-dashboard --memstore -X ceph osd erasure-code-profile set isaprofile1 k=4 m=-1 ceph osd pool create ecpool1 4 4 erasure isaprofile1 rados -p ecpool1 put foo-obj Makefile
ceph version Development (no_version) nautilus (rc) 1: (()+0x2b70010) [0x563a4d1cc010] 2: (()+0x12730) [0x7fabf2945730] 3: (gsignal()+0x10b) [0x7fabf23d58bb] 4: (abort()+0x121) [0x7fabf23c0535] 5: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x36c) [0x563a4d235874] 6: (()+0x2bd9927) [0x563a4d235927] 7: (ECUtil::HashInfo::append(unsigned long, std::map<int, ceph::buffer::v14_2_0::list, std::less<int>, std::allocator<std::pair<int const, ceph::buffer::v14_2_0::list> > >&)+0xb1) [0x563a4cd32fe5] 8: (encode_and_write(pg_t, hobject_t const&, ECUtil::stripe_info_t const&, std::shared_ptr<ceph::ErasureCodeInterface>&, std::set<int, std::less<int>, std::allocator<int> > const&, unsigned long, ceph::buffer::v14_2_0::list, unsigned int, std::shared_ptr<ECUtil::HashInfo>, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge>&, std::map<shard_id_t, ObjectStore::Transaction, std::less<shard_id_t>, std::allocator<std::pair<shard_id_t const, ObjectStore::Transaction> > >*, DoutPrefixProvider*)+0x416) [0x563a4ce8923e] 9: (()+0x2833279) [0x563a4ce8f279] 10: (()+0x28347de) [0x563a4ce907de] 11: (ECTransaction::generate_transactions(ECTransaction::WritePlan&, std::shared_ptr<ceph::ErasureCodeInterface>&, pg_t, ECUtil::stripe_info_t const&, std::map<hobject_t, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge>, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge> > > > const&, std::vector<pg_log_entry_t, std::allocator<pg_log_entry_t> >&, std::map<hobject_t, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge>, std::less<hobject_t>, std::allocator<std::pair<hobject_t const, interval_map<unsigned long, ceph::buffer::v14_2_0::list, bl_split_merge> > > >*, std::map<shard_id_t, ObjectStore::Transaction, std::less<shard_id_t>, std::allocator<std::pair<shard_id_t const, ObjectStore::Transaction> > >*, std::set<hobject_t, std::less<hobject_t>, std::allocator<hobject_t> >*, std::set<hobject_t, std::less<hobject_t>, std::allocator<hobject_t> >*, DoutPrefixProvider*)+0x2e0) [0x563a4ce90352] 12: (ECBackend::try_reads_to_commit()+0x654) [0x563a4ce3bcf4] 13: (ECBackend::check_ops()+0x28) [0x563a4ce3d914] ...
Updated by Kefu Chai about 5 years ago
i think we can even go further -- to prevent user from creating a profile with m=0. technically, it's correct. but practically, it defeats the purpose of erasure by disabling the erasure codec from creating code chunks.
also, we should not use
err |= to_int("k", profile, &k, DEFAULT_K, ss);
for updating the errcode, as it assumes that to_int()
either returns 0 or a certain number which is identical to err
. in that case, would be better to just return a boolean.
Updated by Sage Weil about 5 years ago
- Subject changed from should report EINVAL in ErasureCode::parse() if m<0 to should report EINVAL in ErasureCode::parse() if m<=0
- Status changed from New to 12
- Priority changed from Normal to High
I agree that m=0 is useless--it's no better than num_rep=1... just a (much) more complicated code path and more corner cases to worry about.
Updated by Sage Weil about 5 years ago
- Status changed from 12 to Fix Under Review
- Backport set to luminous,mimic
Updated by Sage Weil about 5 years ago
- Status changed from Fix Under Review to Pending Backport
Updated by Nathan Cutler about 5 years ago
- Copied to Backport #38750: luminous: should report EINVAL in ErasureCode::parse() if m<=0 added
Updated by Nathan Cutler about 5 years ago
- Copied to Backport #38751: mimic: should report EINVAL in ErasureCode::parse() if m<=0 added
Updated by Nathan Cutler over 4 years ago
- Status changed from Pending Backport to Resolved
Actions