Ticket #5082: add file cloning support for FreeBSD, Solaris and macOS#5083
Ticket #5082: add file cloning support for FreeBSD, Solaris and macOS#5083tuffnatty wants to merge 1 commit intoMidnightCommander:masterfrom
Conversation
|
Wow, is it possible to test it in some way? As you can see, we now have CI on FreeBSD. Maybe some way to check that |
|
I've added a very dumb test that seems to work on Linux as well as on FreeBSD. |
To me that looks fantastic, but can't you check that |
2a80fa9 to
e05e311
Compare
|
Hi. It took a bit of time... The new pull request adds macOS and Oracle Solaris support as well. On these platforms, the syscalls for file cloning respond for destination file creation, that is, they take in a path instead of a file descriptor, so the logic around the copy operation had to be adapted. |
zyv
left a comment
There was a problem hiding this comment.
Wow, this really looks fantastic. I'm much impressed. I wonder what the others think. I have but just a few minor comments.
8b2f8c0 to
e52f20c
Compare
|
I've added support for file cloning in append/reget modes. It's working on FreeBSD and Linux when the file offsets are block-aligned. I've also removed the extra #ifs by moving the corresponding checks to autoconf. |
|
The linter fails because I have touched a file with pre-existing invalid formatting. |
This looks good to me! I'm just left wondering whether this is a good idea to enable it by default. Are you planning to do anything else in this PR, or are you considering it ready for review now? |
|
I had to find and click the Submit review button as my previous review comments were in pending state. I don't think I have anything to add or remove at this moment. |
…macOS and Solaris Signed-off-by: Phil Krylov <phil@krylov.eu> Signed-off-by: Yury V. Zaytsev <yury@shurup.com>
236ce43 to
51a41a2
Compare
zyv
left a comment
There was a problem hiding this comment.
I've fixed the formatting error by adding a few hanging commas (in the same commit). Inasmuch as I'm concerned, this looks ready for merge. Thank you for your high-quality contribution! 👍
|
Thanks @zyv! |
Proposed changes
Use
copy_file_range(2)on FreeBSD for file cloning.It should resolve #5082 and probably #4991.
copy_file_range(2)also supports remote NFS operations, but it's out of scope for this change.Checklist
👉 Our coding style can be found here: https://midnight-commander.org/coding-style/ 👈
git commit --amend -smake indent && make check)I have not added any tests for the feature, as it would involve actions fairly complicated from the ops point of view (like, create a test ZFS filesystem), and the test infrastructure does not offer any support for superuser/VM operations. Butit's working locally, copy/move within one ZFS pool became instant, and the existing tests pass.