On 03/09/2026 09:04, Paul Eggert wrote:
On 2026-09-02 15:00, Pádraig Brady wrote:
On 02/09/2026 20:47, Paul Eggert wrote:
Don't we need to be careful with some errors as per
https://github.com/coreutils/coreutils/commit/f6c93f334
https://bugs.gnu.org/60489 ?
Thanks for bringing that up; I'd forgotten that. Unfortunately, though, bug#60489 seems
to have given us some incorrect advice. In it, Noah Misch wrote "One could argue
that ENOSPC also warrants termination, since no fallback reduces space usage." - but
in this thread people are saying that the fallback read+write can work even when
ioctl+FICLONE fails with ENOSPC.
Also, Noah's bug#60489 comments about EIO suggest that when EIO occurs, the
entire file or maybe even the entire file system (not just this open file
descriptor) is at risk of being inconsistent, which is a kernel bug that we
can't reasonably expect cp to work around.
All that being said, it does seem prudent to go back to having cp give up on
the file entirely (not just revert to read+write) if FICLONE fails with EIO. I
did that by installing the attached.
cool.
Also this removes partially cloned files on error,
rather than just empty files. ?
Why keep partially cloned files that we just created? ioctl + FICLONE is
supposed to be atomic, and if a kernel bug makes it non-atomic, the removal is
supposed to work around the bug. Why should the workaround go to the extra work
of checking that the file is empty before unlinking it? That loses the pretense
of atomicity that the workaround should strive for. And particularly with EIO,
the file is in a possibly-inconsistent state so leaving it around could be
hazardous.
agreed.
I'm a little worried about about giving worrying errors here
rather than an abstract "unsupported". Perhaps map is_CLONENOTSUP()
to "unsupported" and others to their errno?
--debug is *supposed* to worry people (:-), and I thought it better to put a
bit more info into the debugging output. But please feel free to adjust the
--debug diagnostics, though I'm not sure is_CLONENOTSUP is completely apropos
for doing so as is_CLONENOTSUP (which is not aptly named any more) is about
copy_file_range, not about ioctl with FICLONE.
agreed.
thanks!
Padraig
