-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Workarounds for copy_file_range issues #75428
Conversation
…ttemted on a NFS mount under RHEL/CentOS 7. The syscall is supposed to return ENOSYS in most cases but when calling it on NFS it may leak through EOPNOTSUPP even though that's supposed to be handled by the kernel and not returned to userspace. Since it returns ENOSYS in some cases anyway this will trip the HAS_COPY_FILE_RANGE detection anyway, so treat EOPNOTSUPP as if it were a ENOSYS. https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/7.8_release_notes/deprecated_functionality#the_literal_copy_file_range_literal_call_has_been_disabled_on_local_file_systems_and_in_nfs https://bugzilla.redhat.com/show_bug.cgi?id=1783554
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @joshtriplett (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@bors r+ |
📌 Commit 1316c78 has been approved by |
⌛ Testing commit 1316c78 with merge c0e06fb4d88cf4063d23e407c155a2096b4ac611... |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-actions |
The bors error is not helpful, spurious failure? |
… on file size This solves several problems - race conditions where a file is truncated while copying from it. if we blindly trusted the file size this would lead to an infinite loop - proc files appearing empty to copy_file_range but not to read/write coreutils/coreutils@4b04a0c - copy_file_range returning 0 for some filesystems (overlay? bind mounts?) inside docker, again leading to an infinite loop
Updated to address an additional copy_file_range issue. |
ping @joshtriplett |
let copy_result = if has_copy_file_range { | ||
let bytes_to_copy = cmp::min(len - written, usize::MAX as u64) as usize; | ||
let bytes_to_copy = cmp::min(max_len - written, usize::MAX as u64) as usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to keep track of max_len - written
accurately at this point? It's probably okay just to pass a large constant size on every call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It avoids overflowing an u64. Plus I want to reuse the code in #75272 which will require exact max sizes
⌛ Testing commit 4ddedd5 with merge 596b8ed70633bf181a3b8966fb235e91a67ae146... |
💥 Test timed out |
@bors retry |
…triplett Workarounds for copy_file_range issues fixes rust-lang#75387 fixes rust-lang#75446
⌛ Testing commit 4ddedd5 with merge f9b36230f023ae6f396f3cc4284cd41a0583b819... |
💔 Test failed - checks-actions |
I don't see how linux-only code changes can make a mingw build fail. Doubly so when it's doc tests. |
@bors retry |
☀️ Test successful - checks-actions, checks-azure |
implement better availability probing for copy_file_range Followup to rust-lang#75428 (comment) Previously syscall detection was overly pessimistic. Any attempt to copy to an immutable file (EPERM) would disable copy_file_range support for the whole process. The change tries to copy_file_range on invalid file descriptors which will never run into the immutable file case and thus we can clearly distinguish syscall availability.
fixes #75387
fixes #75446