Ceph

Bug #13829

Updated by Kefu Chai over 8 years ago

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. 

 <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> 

 0.94.1 (e4bfad3a3c51054df7e537a724c8d0bf9be972ff) is not affected so I did a git bisect to find the commit that introduced this behavior. 

 <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> 

 That patch includes the following code to src/common/strtol.cc. 

 <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> 

 and this to @src/common/strtol.h@ 

 <pre> 
  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 } 
 </pre> 
 

 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.

Back