Backport #16680
hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"
Release:
hammer
Crash signature (v1):
Crash signature (v2):
Related issues
History
#1 Updated by Brad Hubbard over 7 years ago
- Copied from Bug #13829: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument" added
#2 Updated by Brad Hubbard over 7 years ago
- Status changed from New to Fix Under Review
#3 Updated by Kefu Chai over 7 years ago
- Target version deleted (
v0.94.8)
#4 Updated by Kefu Chai over 7 years ago
- Status changed from Fix Under Review to In Progress
#5 Updated by Nathan Cutler over 7 years ago
- Description updated (diff)
original description¶
In the filestore-config-ref we say...
filestore merge threshold
Description: Min number of files in a subdir before merging into parent NOTE: A > negative value means to disable subdir merging
But running the following on 0.94.3 (95cefea9fd9ab740263bf8bb4796fd864d9afe2b) gives an error.
# ceph --admin-daemon /var/run/ceph/ceph-osd.312.asok config set filestore_merge_threshold -40 { "error": "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument" }
0.94.1 (e4bfad3a3c51054df7e537a724c8d0bf9be972ff) is not affected so I did a git bisect to find the commit that introduced this behavior.
$ git bisect bad 9b947fa320b77e0055a581005353c2561a12a198 is the first bad commit commit 9b947fa320b77e0055a581005353c2561a12a198 Author: Kefu Chai <kchai@redhat.com> Date: Wed Apr 29 15:41:08 2015 +0800 common/config: detect overflow of int values * #include "strtol.h" in strtol.cc, to ensure the function defintions are consistent. * add a test accordingly * fix the testcase of converting 1024E. * do not accept integers overflow after adding SI suffix * do not accept integers underflow (i.e. negative values) Fixes: #11484
That patch includes the following code to src/common/strtol.cc.
163 long long r_ll = strict_strtoll(v, 10, err); 164 165 if (r_ll < 0) { 166 *err = "strict_sistrtoll: value should not be negative"; 167 return 0; 168 } 169 170 uint64_t r = r_ll; 171 if (err->empty() && m > 0) { 172 if (r > (std::numeric_limits<uint64_t>::max() >> m)) { 173 *err = "strict_sistrtoll: value seems to be too large"; 174 return 0; 175 } 176 r <<= m; 177 }
and this to src/common/strtol.h
34 template <typename Target> 35 Target strict_si_cast(const char *str, std::string *err) { 36 uint64_t ret = strict_sistrtoll(str, err); 37 if (!err->empty()) 38 return ret; 39 if (ret > (uint64_t)std::numeric_limits<Target>::max()) { 40 err->append("The option value '"); 41 err->append(str); 42 err->append("' seems to be too large"); 43 return 0; 44 } 45 return ret; 46 }
Negative values fail both the test for a negative value (of course) and the tests
for uint64_t overflow.
Working with kefu on a patch so assigning this to myself.
#6 Updated by Brad Hubbard over 7 years ago
- Status changed from In Progress to Resolved
- Target version set to v0.94.8