Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

copy_file_range not used by musl builds #7669

Open
calebj opened this issue May 9, 2024 · 2 comments
Open

copy_file_range not used by musl builds #7669

calebj opened this issue May 9, 2024 · 2 comments

Comments

@calebj
Copy link

calebj commented May 9, 2024

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

#if defined __GLIBC__ && (__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 27))
#define TORRENT_HAS_COPY_FILE_RANGE 1
#endif
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
Copy link
Owner

arvidn commented May 11, 2024

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
Copy link
Author

calebj commented May 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants