This change improves the CMAKE variable detection logic in do_cmake.sh
to allow users to override the CMAKE binary used for building.
Changes:
- Add conditional check to only set CMAKE if not already set
- Add detection for cmake version 4.x and later (prioritized)
- Maintain backward compatibility with cmake3 fallback
- Allow users to specify custom CMAKE path via environment variable
- Pass CMAKE_POLICY_VERSION_MINIMUM if set to submodule build steps for
older modules
This enables users to use a specific cmake binary by setting the CMAKE
environment variable before running the script:
CMAKE=/custom/path/to/cmake ./do_cmake.sh
Additionally, the script now automatically detects and uses cmake 4.x+
when available, falling back to cmake3 or cmake as needed. This supports
building Ceph with newer cmake versions while maintaining compatibility
with existing build environments.
Fixes: https://tracker.ceph.com/issues/73523
Signed-off-by: Edwin Rodriguez <edwin.rodriguez1@ibm.com>
the bump to arrow 17.0.0 broke the ubuntu jammy builds with:
In file included from /usr/include/thrift/transport/TTransport.h:25,
from /usr/include/thrift/protocol/TProtocol.h:28,
from /usr/include/thrift/TBase.h:24,
from /build/ceph-20.3.0-3599-g3d863d32/src/arrow/cpp/src/generated/parquet_types.h:14,
from /build/ceph-20.3.0-3599-g3d863d32/src/arrow/cpp/src/generated/parquet_constants.h:10,
from /build/ceph-20.3.0-3599-g3d863d32/src/arrow/cpp/src/generated/parquet_constants.cpp:7:
/usr/include/thrift/transport/TTransportException.h:23:10: fatal error: boost/numeric/conversion/cast.hpp: No such file or directory
23 | #include <boost/numeric/conversion/cast.hpp>
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
when comparing the gcc command line with arrow-15.0.0, the following argument
is no longer present:
> -isystem /build/ceph-20.3.0-3402-gb2db4947/obj-x86_64-linux-gnu/boost/include
arrow 17.0.0 seems to assume that thrift doesn't depend on boost anymore. a
comment in https://github.com/apache/arrow/issues/32266 claims that
> we don't need Boost with system Thrift 0.17.0 or later
but our jammy builds are stuck with libthrift-0.16.0. to reenable jammy builds,
instruct Arrow's cmake to use its bundled thrift dependency if our system thrift
version is < 0.17.0
Signed-off-by: Casey Bodley <cbodley@redhat.com>
when thrift headers are found, parse the version string into a
cmake variable and pass it to find_package_handle_standard_args
this will be needed by BuildArrow.cmake for some conditional logic
Signed-off-by: Casey Bodley <cbodley@redhat.com>
Arrow requires xsimd >= 9.0.1 according to arrow/cpp/thirdparty/versions.txt.
Previously, we unconditionally set -Dxsimd_SOURCE=BUNDLED, forcing the use
of Arrow's vendored xsimd regardless of system package availability.
This commit changes to -Dxsimd_SOURCE=AUTO, which allows Arrow's
resolve_dependency mechanism to automatically:
1. Try to find system xsimd package
2. Check if version >= 9.0.1
3. Use system version if found and sufficient
4. Fall back to bundled version otherwise
This reduces build time and dependencies on systems with sufficient xsimd,
while maintaining compatibility with older distributions.
Distribution availability:
- Ubuntu Noble (24.04): libxsimd-dev 12.1.1 (✓ will use system)
- Ubuntu Jammy (22.04): libxsimd-dev 7.6.0 (✗ will use bundled)
- Debian Trixie (13): libxsimd-dev 13.2.0 (✓ will use system)
- CentOS Stream 9: xsimd-devel 7.4.9 (✗ will use bundled)
Signed-off-by: Kefu Chai <k.chai@proxmox.com>
Add a pair of default test labels (`PurePython` and `PythonTox`) to the
tests generated by the add_tox_test cmake function. In addition, the
function can extend the set of labels applied with a new LABELS argument
to the cmake function.
Using the label can be done with something like:
```
ctest -L PythonTox -j8
```
Note that these tests are labeled PurePython because they do not
require C extensions to be built. The above example command can
be run successfully immediately after configuring, without having
to build the C++ sources first.
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add suppression rules for two categories of false positive warnings
encountered during ASan-enabled testing:
1. PyModule_ExecDef memory leaks: ASan incorrectly interprets Python's
module loading behavior as memory leaks when the interpreter loads
extension modules.
2. __cxa_throw interception failures: ASan's interceptor cannot properly
intercept exception handling when libstdc++.so is loaded after the
ASan shared library, causing CHECK failures.
3. ErasureCodePluginRegistry::load:
`ceph::ErasureCodePluginRegistry::load()` is known to leak, as we
don't free the memory allocated by the ec plugins which are
registered in the `ErasureCodePluginRegistry` singleton. this is a
known issue, but since the `ErasureCodePluginRegistry` instance is a
singleton. we can live with it. in this change, we add the rule to
suppress the leak report from LeakSanitizer. this rule also exist in
qa/valgrind.supp.
All warnings are confirmed false positives that should be suppressed
to reduce noise in test output.
Example warnings:
```
Direct leak of 3264 byte(s) in 1 object(s) allocated from:
#0 0x7f6027d20cb5 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67
#1 0x7f60277557ad (/usr/lib/libpython3.13.so.1.0+0x1557ad) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#2 0x7f6027756067 (/usr/lib/libpython3.13.so.1.0+0x156067) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#3 0x7f60278471a0 (/usr/lib/libpython3.13.so.1.0+0x2471a0) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#4 0x7f602774d031 (/usr/lib/libpython3.13.so.1.0+0x14d031) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#5 0x7b60234093bb in __Pyx_modinit_type_init_code.constprop.0 /home/kefu/dev/ceph/build/src/pybind/rados/rados.c:82066
#6 0x7b602340a826 in __pyx_pymod_exec_rados /home/kefu/dev/ceph/build/src/pybind/rados/rados.c:82755
#7 0x7f6027856777 in PyModule_ExecDef (/usr/lib/libpython3.13.so.1.0+0x256777) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#8 0x7f602785baa3 (/usr/lib/libpython3.13.so.1.0+0x25baa3) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#9 0x7f6027793df2 (/usr/lib/libpython3.13.so.1.0+0x193df2) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#10 0x7f6027777cbe in _PyEval_EvalFrameDefault (/usr/lib/libpython3.13.so.1.0+0x177cbe) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#11 0x7f60277957de (/usr/lib/libpython3.13.so.1.0+0x1957de) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#12 0x7f60277d11b9 in PyObject_CallMethodObjArgs (/usr/lib/libpython3.13.so.1.0+0x1d11b9) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#13 0x7f60277d0ee4 in PyImport_ImportModuleLevelObject (/usr/lib/libpython3.13.so.1.0+0x1d0ee4) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#14 0x7f6027779c0c in _PyEval_EvalFrameDefault (/usr/lib/libpython3.13.so.1.0+0x179c0c) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#15 0x7f602784e2c8 in PyEval_EvalCode (/usr/lib/libpython3.13.so.1.0+0x24e2c8) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#16 0x7f602788c88b (/usr/lib/libpython3.13.so.1.0+0x28c88b) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#17 0x7f602788985c (/usr/lib/libpython3.13.so.1.0+0x28985c) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#18 0x7f6027886f57 (/usr/lib/libpython3.13.so.1.0+0x286f57) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#19 0x7f6027886211 (/usr/lib/libpython3.13.so.1.0+0x286211) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#20 0x7f6027885b82 (/usr/lib/libpython3.13.so.1.0+0x285b82) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#21 0x7f6027883e50 in Py_RunMain (/usr/lib/libpython3.13.so.1.0+0x283e50) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#22 0x7f602783bbea in Py_BytesMain (/usr/lib/libpython3.13.so.1.0+0x23bbea) (BuildId: bea05fc2c8bd66145b159f10dcd810ebe813af39)
#23 0x7f6027227674 (/usr/lib/libc.so.6+0x27674) (BuildId: 4fe011c94a88e8aeb6f2201b9eb369f42b4a1e9e)
#24 0x7f6027227728 in __libc_start_main (/usr/lib/libc.so.6+0x27728) (BuildId: 4fe011c94a88e8aeb6f2201b9eb369f42b4a1e9e)
#25 0x55dae17e6044 in _start (/usr/bin/python3.13+0x1044) (BuildId: 8c0dc848f5b978c56ebeb07255bb332b4b37ae4e)
```
```
AddressSanitizer: CHECK failed: asan_interceptors.cpp:335 "((__interception::real___cxa_throw)) != (0)" (0x0, 0x0) (tid=3246455)
#0 0x7f345ea81979 in CheckUnwind ../../../../src/libsanitizer/asan/asan_rtl.cpp:69
#1 0x7f345eaa790d in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86
#2 0x7f345e9e1d54 in __interceptor___cxa_throw ../../../../src/libsanitizer/asan/asan_interceptors.cpp:335
#3 0x7f345e9e1d54 in __interceptor___cxa_throw ../../../../src/libsanitizer/asan/asan_interceptors.cpp:334
#4 0x7f3458623def in void boost::throw_exception<boost::bad_lexical_cast>(boost::bad_lexical_cast const&) /opt/ceph/include/boost/throw_exception.hpp:165
#5 0x7f345997ad3b in void boost::conversion::detail::throw_bad_cast<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, unsigned long>() /opt/ceph/include/boost/lexical_cast/bad_lexical_cast.hpp:93
#6 0x7f3459979d35 in unsigned long boost::lexical_cast<unsigned long, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) /opt/ceph/include/boost/lexical_cast.hpp:43`
```
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
LeakSanitizer prints matched suppressions by default, which causes CLI
tests to fail when built with LSan enabled. The tests compare actual
output against expected output, and the additional LSan messages break
this comparison.
Example failure:
```
/home/jenkins-build/build/workspace/ceph-pull-requests/src/test/cli/ceph-kvstore-tool/help.t: failed
--- /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/cli/ceph-kvstore-tool/help.t
+++ /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/cli/ceph-kvstore-tool/help.t.err
@@ -21,3 +21,9 @@
stats
histogram [prefix]
+ -----------------------------------------------------
+ Suppressions used:
+ count bytes template
+ 1 45 ^MallocExtension::Initialize
+ -----------------------------------------------------
+
```
Set LSAN_OPTIONS=print_suppressions=0 to prevent this output rather
than modifying test expectations, since the suppression messages are
not part of the actual tool output and would vary between builds.
This change should fix run_cli_tests failures when building with LeakSanitizer
See https://github.com/google/sanitizers/wiki/SanitizerCommonFlags
for the environment variables respected by LeakSanitizer.
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
Add Boost 1.88.0 to the supported versions list and update component
dependencies to eliminate build warnings.
This resolves the following warning when building with Boost 1.88.0:
```
-- Found Boost: /usr/include (found suitable version "1.88.0", minimum required is "1.73.0")
CMake Warning at cmake/modules/FindBoost.cmake:1413 (message):
New Boost version may have incorrect or missing dependencies and imported
targets
Call Stack (most recent call first):
cmake/modules/FindBoost.cmake:1538 (_Boost_COMPONENT_DEPENDENCIES)
cmake/modules/FindBoost.cmake:2157 (_Boost_MISSING_DEPENDENCIES)
src/CMakeLists.txt:461 (_find_package)
src/seastar/cmake/SeastarDependencies.cmake:136 (find_package)
src/seastar/CMakeLists.txt:395 (seastar_find_dependencies)
```
Boost 1.88.0 was released on April 3, 2025, and is already available
in some distributions. Since many distributions don't yet ship Boost's
native CMake configuration files, our vendored FindBoost.cmake module
needs updating to handle this version.
The component dependencies were updated following the scanning procedure
documented in the _Boost_COMPONENT_DEPENDENCIES() function. The change
will be upstreamed to CMake shortly.
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
Update our local FindBoost.cmake module to match CMake upstream's
latest version to properly handle Boost component dependencies.
While commit b446290f prevented warnings when checking Boost 1.87,
it failed to update the boost component dependency mappings. This
change synchronizes our module with upstream to ensure correct
dependency resolution.
Additionally, this prepares for Boost 1.88.0 support (released
April 3, 2025) which some distributions have already adopted.
Since CMake upstream hasn't yet added 1.88 support, this sync
provides the foundation for adding 1.88.0 compatibility in a
subsequent commit.
Changes made:
- Sync with upstream CMake FindBoost.cmake (commit 17726227)
(at 1772622772/Modules/FindBoost.cmake)
- Reapply local modifications from commit 06824bc1
Note: New dependencies can be scanned using:
```
cmake -DBOOST_DIR=/path/to/boost_1_88_0 -P
Utilities/Scripts/BoostScanDeps.cmake
```
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
cmake: adapt FindBoost.cmake to our needs
the vanilla FindBoost.cmake pulled from cmake has couple assumptions
which do not hold in our environment. so address them case by case.
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
Since upgrading minimum CMake version to 3.22.1 (commit 469d82a1), we can
now use find_program(REQUIRED) which was introduced in CMake 3.18.
This change replaces manual FATAL_ERROR checks with the REQUIRED option
and adds it to programs that are actually needed during the build. This
ensures the build fails early during configuration rather than later
during compilation when missing programs are invoked.
Changes:
- Replace find_program() + message(FATAL_ERROR) patterns with REQUIRED
- Add REQUIRED to programs that are used during build but previously
had no error checking
Reference: https://cmake.org/cmake/help/latest/command/find_program.html
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
Remove the c-ares::c-ares alias that was causing build failures after
bumping the minimum CMake version:
```
CMake Error at cmake/modules/Findc-ares.cmake:34 (add_library):
add_library cannot create ALIAS target "c-ares::c-ares" because another
target with the same name already exists.
Call Stack (most recent call first):
src/CMakeLists.txt:463 (_find_package)
src/seastar/cmake/SeastarDependencies.cmake:136 (find_package)
src/seastar/CMakeLists.txt:395 (seastar_find_dependencies)`
```
The alias was originally added for backward compatibility with Seastar,
but is no longer needed since the updated Seastar submodule no longer
references the c-ares::c-ares target.
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
Makes Catch2 available for C++ unit tests;
Adds GPM script availability to CMake.
Adds rudimentary test for rgw_hex.
Signed-off-by: Jesse F. Williamson <jfw@ibm.com>
Previously, the build process used `bootstrap.sh` to build the b2 tool,
which automatically selected the compiler based on the specified toolset.
This failed when the compiler executable had a version postfix (e.g.,
/usr/bin/clang++-19) without a symlink at the expected name, producing
errors like:
```
A C++11 capable compiler is required for building the B2 engine.
Toolset 'clang' does not appear to support C++11.
> clang++ -x c++ -std=c++11 -pthread check_clib.cpp check_cxx11.cpp
./tools/build/src/engine/build.sh: 120: clang++: not found
> clang++ -x c++ -std=c++11 check_clib.cpp check_cxx11.cpp
./tools/build/src/engine/build.sh: 120: clang++: not found
** Note, the C++11 capable compiler is _only_ required for building the B2
** engine. The B2 build system allows for using any C++ level and any other
** supported language and resource in your projects.
You can specify the toolset as the argument, i.e.:
./build.sh [options] gcc
```
The issue occurred because `bootstrap.sh` hardcodes the compiler name
based on the toolset (e.g., `clang++` for Clang) without supporting
postfixed versions.
This commit replaces the `bootstrap.sh` approach with an explicit build
command using Boost's `build.sh` script. We now:
1. Directly specify the full compiler path from CMake variables
2. Manually configure the build with `--cxx=...` and `--toolset=...`
3. Avoid reliance on symlinks or `bootstrap.sh`'s internal detection
This ensures the B2 engine is always built with the user-specified
compiler, even when installed with version postfixes.
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
In commit 986f6918, we updated the googletest submodule to silence a
CMake warning. However, this change broke the build due to a Clang issue
where it warns about deprecated declarations even from system headers
(see https://github.com/llvm/llvm-project/issues/76515). Since we use
`-Werror` in our compile options, these warnings become errors and fail
the build.
This change detects if we're affected by the specific combination of
compiler, standard library, and compile options that triggers this
issue. It then conditionally disables the `-Wdeprecated-declarations`
warning when building googletest to resolve build failures caused by
deprecated functions like `get_temporary_buffer<>` in the standard
library.
The build failure looks like:
```
FAILED: src/googletest/googletest/CMakeFiles/gtest.dir/src/gtest-all.cc.o
...
In file included from /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest-all.cc:38:
In file included from /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/include/gtest/gtest.h:55:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/memory:66:
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_tempbuf.h:263:8: error: 'get_temporary_buffer<testing::TestInfo *>' is deprecated [-Werror,-Wdeprecated-declarations]
263 | std::get_temporary_buffer<value_type>(_M_original_len));
| ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algo.h:4996:15: note: in instantiation of member function 'std::_Temporary_buffer<__gnu_cxx::__normal_iterator<testing::TestInfo **, std::vector<testing::TestInfo *>>, testing::TestInfo *>::_Temporary_buffer' requested here
4996 | _TmpBuf __buf(__first, (__last - __first + 1) / 2);
| ^
/usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algo.h:5070:23: note: in instantiation of function template specialization 'std::__stable_sort<__gnu_cxx::__normal_iterator<testing::TestInfo **, std::vector<testing::TestInfo *>>, __gnu_cxx::__ops::_Iter_comp_iter<(lambda at /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:3024:20)>>' requested here
5070 | _GLIBCXX_STD_A::__stable_sort(__first, __last,
| ^
/home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest.cc:3023:8: note: in instantiation of function template specializatio
```
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
OpenSSL 3.0 documentation recommends moving from the ENGINE API to the
Providers API. Recent distributions may compile OpenSSL without engine
support by default, necessitating more flexible configuration handling.
So, in this change:
- Add a CMake option `WITH_OPENSSL_ENGINE` to explicitly control engine support
- Respect `openssl_engine_opts` when engine support is enabled
- Provide clear error messaging when engine options are set but support is disabled
See also:
- OpenSSL 3.0 documentation:
https://wiki.openssl.org/index.php/OpenSSL_3.0#Engines_and_.22METHOD.22_APIs
Fixes: https://tracker.ceph.com/issues/68059
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
qatzip/configure.ac enables -Werror. However, newer compilers
(e.g clang 16) will not compile with the existing warnings
identified. The fixes for the warnings are not merged yet [1]
in the submodule. Until then, in order to allow for compiler
upgrade - we should disable the problematic errors.
[1] https://github.com/intel/QATzip/pull/119
Signed-off-by: Matan Breizman <mbreizma@redhat.com>
instead of building isa-l_crypto source files directly into the
ceph_crypto_isal plugin, use a cmake ExternalProject to build the
library with make and expose the static library as cmake target
ISAL::Crypto
Signed-off-by: Casey Bodley <cbodley@redhat.com>
This patch add UADK Acceleration for zlib compressor
[UADK: Userspace Acceleration Development Kit](https://github.com/Linaro/uadk)
1. Unity: one set of interfaces to support enc and comp acceleration
2. Efficiency: userspace zerocopy based on IOMMU & SVA(shared virtual address)
3. Security: the IOMMU limits the access rights and security boundaries of devices and processes
4. Compatibility: all acc devices can join this opensource project
Test on Kunpeng 920 platform, compression offload based on UadkAccel:
1. save almost 50% cpu usage compared with no-isal compression in RBD 4M workload
2. save almost 40% cpu usage compared with no-isal compression in RGW put op (4M) workload
3. lower cpu usage, better performance
Signed-off-by: Rongqi Sun <sunrongqi@huawei.com>
when enabled and found, the os_zfs_objs target would reference a missing
file filestore/ZFSFileStoreBackend.cc and cmake would fail to configure
i take this to mean that WITH_ZFS was accidentally left over from
filestore's removal. drop all references to WITH_ZFS, along with the
corresponding Findzfs.cmake script
Signed-off-by: Casey Bodley <cbodley@redhat.com>
If we are correctly configured for distributed mode, use the cluster's CPU count
instead of ours. If we are configured for sccache but without distributed mode,
inform the user but continue with normal job limits.
Signed-off-by: Zack Cerza <zack@redhat.com>
there are known leaks, which are tracked by qa/lsan.suppr, in Ceph.
so let's reuse it so we don't see them when running unit test with
ASan enabled.
see also https://clang.llvm.org/docs/AddressSanitizer.html#issue-suppression
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
turns out we have multiple copies of following symbol defined by rbd executable:
```
AddressSanitizer: odr-violation: global 'ceph::buffer::list::always_empty_bptr' at /home/jenkins-build/build/workspace/ceph-pull-requests/src/common/buffer.cc:1267:34
```
before addressing it. let's disable this warning.
Refs https://tracker.ceph.com/issues/65098
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
if sanitizers are enabled, we have to populate the required link flags
to python extensions's building workflow. otherwise ld would fail to
link like:
```
/usr/bin/ld: /home/jenkins-build/build/workspace/ceph-pull-requests/build/lib/libceph-common.so.2: undefined reference to `__asan_stack_free_10'
```
Signed-off-by: Kefu Chai <tchaikov@gmail.com>
