https://tracker.ceph.com/https://tracker.ceph.com/favicon.ico2016-07-13T23:34:37ZCeph Ceph - Backport #16680: hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"https://tracker.ceph.com/issues/16680?journal_id=747552016-07-13T23:34:37ZBrad Hubbardbhubbard@redhat.com
<ul><li><strong>Copied from</strong> <i><a class="issue tracker-1 status-3 priority-5 priority-high3 closed" href="/issues/13829">Bug #13829</a>: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"</i> added</li></ul> Ceph - Backport #16680: hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"https://tracker.ceph.com/issues/16680?journal_id=747742016-07-14T03:08:41ZBrad Hubbardbhubbard@redhat.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Fix Under Review</i></li></ul><p><a class="external" href="https://github.com/ceph/ceph/pull/10291">https://github.com/ceph/ceph/pull/10291</a></p> Ceph - Backport #16680: hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"https://tracker.ceph.com/issues/16680?journal_id=747772016-07-14T03:28:30ZKefu Chaitchaikov@gmail.com
<ul><li><strong>Target version</strong> deleted (<del><i>v0.94.8</i></del>)</li></ul> Ceph - Backport #16680: hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"https://tracker.ceph.com/issues/16680?journal_id=747782016-07-14T03:30:06ZKefu Chaitchaikov@gmail.com
<ul><li><strong>Status</strong> changed from <i>Fix Under Review</i> to <i>In Progress</i></li></ul> Ceph - Backport #16680: hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"https://tracker.ceph.com/issues/16680?journal_id=748032016-07-14T17:36:14ZNathan Cutlerncutler@suse.cz
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/74803/diff?detail_id=71290">diff</a>)</li></ul><a name="original-description"></a>
<h3 >original description<a href="#original-description" class="wiki-anchor">¶</a></h3>
<p>In the filestore-config-ref we say...</p>
<blockquote>
<p>filestore merge threshold<br />Description: Min number of files in a subdir before merging into parent NOTE: A > negative value means to disable subdir merging</p>
</blockquote>
<p>But running the following on 0.94.3 (95cefea9fd9ab740263bf8bb4796fd864d9afe2b) gives an error.</p>
<pre>
# 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"
}
</pre>
<p>0.94.1 (e4bfad3a3c51054df7e537a724c8d0bf9be972ff) is not affected so I did a git bisect to find the commit that introduced this behavior.</p>
<pre>
$ 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
</pre>
<p>That patch includes the following code to src/common/strtol.cc.</p>
<pre>
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 }
</pre>
<p>and this to <code>src/common/strtol.h</code></p>
<pre>
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 }
</pre><br />Negative values fail both the test for a negative value (of course) and the tests<br />for uint64_t overflow.
<p>Working with kefu on a patch so assigning this to myself.</p> Ceph - Backport #16680: hammer: config set with negative value results in "error setting 'filestore_merge_threshold' to '-40': (22) Invalid argument"https://tracker.ceph.com/issues/16680?journal_id=765342016-08-12T11:14:19ZBrad Hubbardbhubbard@redhat.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>Target version</strong> set to <i>v0.94.8</i></li></ul>