search

arvidn / libtorrent

an efficient feature complete C++ bittorrent implementation
http://libtorrent.org
Other
5.19k stars 995 forks source link

copy_file_range not used by musl builds #7669

Closed calebj closed 3 months ago

calebj commented 4 months ago

Please provide the following information

libtorrent version (or branch): 2.0.10

platform/architecture: Linux 6.1.15 x86_64 (Debian host, Alpine container)

compiler and compiler version: g++ 13.2.1

please describe what symptom you see, what you would expect to see instead and how to reproduce it.

I noticed that my Deluge container wasn't using copy_file_range on my NFS setup for reflinks and server-side copy between pools. This was verified by comparing the pool IO to the NFS traffic rate. The host in question is able to do copy offload, so I know it should work.

After some digging, I found that it was being cut out by the glibc version check in https://github.com/arvidn/libtorrent/blob/74bc93a37a5e31c78f0aa02037a68fb9ac5deb41/include/libtorrent/config.hpp#L189-L191 because the container in question is based on alpine, which uses musl instead of glibc.

I know this call and getrandom (but not execinfo) are currently implemented in musl, but the developers are opinionated about using preprocessor feature checks, so this approach is only valid for glibc.

After building the libtorrent apk without this check and dropping it into the container image, the offload works as expected. So that's definitely the culprit. But what's the best way to allow musl builds to use it, while protecting against undefined references with older musl and glibc? I have yet to try it, but could all of these checks in config.hpp be changed to use CMake CheckFunctionExists() instead?

arvidn commented 4 months ago

perhaps the check could be flipped around. when on linux, we assume copy_file_range() exists, unless you're on an old version of glibc

calebj commented 4 months ago

That would fix it. The syscall wrapper was added to musl in the now-EOL v1.1.24, released October 2019. Everything from before that would break, but v1.1.x is EOL so that should be fine.