Subject: bug#62404: --reflink=auto not falling back appropriately on older kernels

From: Paul Eggert <[email-masked]>
Date: Sat, 25 Mar 2023 13:34:40 -0700

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 <[email-masked]>
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