Thanks for installing that. I found the comments still a bit confusing
so I pushed the attached; hope it's OK.
It is annoying that in the common case where A is a regular file and B
does not exist but will be created on the same file system, the syscalls
start out:
openat(AT_FDCWD, "B", O_RDONLY|O_PATH|O_DIRECTORY) = -1 ENOENT
newfstatat(AT_FDCWD, "A", ...}, 0) = 0
openat(AT_FDCWD, "A", O_RDONLY) = 3
newfstatat(3, "", ..., AT_EMPTY_PATH) = 0
openat(AT_FDCWD, "B", O_WRONLY|O_CREAT|O_EXCL, 0775) = 4
ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP
newfstatat(4, "", ..., AT_EMPTY_PATH) = 0
They should be just:
openat(AT_FDCWD, "A", O_RDONLY) = 3
newfstatat(3, "", ..., AT_EMPTY_PATH) = 0
openat(AT_FDCWD, "B", O_WRONLY|O_CREAT|O_EXCL, 0775) = 4
ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = -1 EOPNOTSUPP
newfstatat(4, "", ..., AT_EMPTY_PATH) = 0
as there should be no need to stat the source twice, or to try to open
the destination twice. But this is a performance improvement for another
day.From 6272817de007d236f82616df5916be37333ba163 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 25 Mar 2023 09:30:22 -0700
Subject: [PATCH] cp: clarify commentary
* src/copy.c: Make comments a bit clearer.
---
src/copy.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index f8ba058d6..a8aa14920 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -278,9 +278,10 @@ create_hole (int fd, char const *name, bool punch_holes, off_t size)
}
-/* Whether the errno indicates the operation is a transient failure.
- I.e., a failure that would indicate the operation _is_ supported,
- but has failed in a terminal way. */
+/* Whether an errno value ERR, set by FICLONE or copy_file_range,
+ indicates that the copying operation has terminally failed, even
+ though it was invoked correctly (so that, e.g, EBADF cannot occur)
+ and even though !is_CLONENOTSUP (ERR). */
static bool
is_terminal_error (int err)
@@ -288,9 +289,9 @@ is_terminal_error (int err)
return err == EIO || err == ENOMEM || err == ENOSPC || err == EDQUOT;
}
-
-/* Whether the errno from FICLONE, or copy_file_range indicates
- the operation is not supported/allowed for this file or process. */
+/* Similarly, whether ERR indicates that the copying operation is not
+ supported or allowed for this file or process, even though the
+ operation was invoked correctly. */
static bool
is_CLONENOTSUP (int err)
@@ -1178,16 +1179,15 @@ fd_has_acl (int fd)
Return FALSE if it's a terminal failure for this file. */
static bool
-handle_clone_fail (int dst_dirfd, char const* dst_relname,
- char const* src_name, char const* dst_name,
+handle_clone_fail (int dst_dirfd, char const *dst_relname,
+ char const *src_name, char const *dst_name,
int dest_desc, bool new_dst, enum Reflink_type reflink_mode)
{
/* When the clone operation fails, report failure only with errno values
known to mean trouble when the clone is supported and called properly.
Do not report failure merely because !is_CLONENOTSUP (errno),
- as systems may yield oddball errno values here with FICLONE.
- Also is_CLONENOTSUP() is not appropriate for the range of errnos
- possible from fclonefileat(), so it's more consistent to avoid. */
+ as systems may yield oddball errno values here with FICLONE,
+ and is_CLONENOTSUP is not appropriate for fclonefileat. */
bool report_failure = is_terminal_error (errno);
if (reflink_mode == REFLINK_ALWAYS || report_failure)
--
2.37.2
