From ab90dd977e3ac3ccf91c1e154e24a09ca3729207 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <P@draigBrady.com>
Date: Thu, 23 Mar 2023 13:19:04 +0000
Subject: [PATCH] copy: --reflink=auto fallback to standard copy in more cases

* src/copy.c (is_transient_failure): A new function refactored
from handle_clone_fail().
(is_CLONENOTSUP): Merge in the handling of EPERM as that also
pertains to determination of whether cloning is supported
if we ever use this function in that context.
(handle_clone_fail): Use is_transient_failure() in all cases,
so that we assume a terminal failure in less errno cases.
* NEWS: Mention the bug fix.
Addresses https://bugs.gnu.org/62404
---
 NEWS       |  8 ++++++++
 src/copy.c | 41 +++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/NEWS b/NEWS
index 030f0e543..73801d4bf 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,14 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  cp --relink=auto (the default), mv, and install will
+  fall back to a standard copy in more cases.
+  Previously copies could fail with "Permission denied" errors etc.
+  on older kernels or unprivileged containers on ZFS etc.
+  [issue introduced in coreutils-9.2]
+
 
 * Noteworthy changes in release 9.2 (2023-03-20) [stable]
 
diff --git a/src/copy.c b/src/copy.c
index 39197872c..19a605bd5 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -278,15 +278,28 @@ create_hole (int fd, char const *name, bool punch_holes, off_t size)
 }
 
 
+/* Whether the errno indicates operation is a transient failure.
+   I.e., a failure that would indicate the operation _is_ supported,
+   but has failed in a terminal way.  */
+
+static bool
+is_transient_failure (int err)
+{
+  return err == EIO || err == ENOMEM
+         || err == ENOSPC || err == EDQUOT;
+}
+
+
 /* Whether the errno from FICLONE, or copy_file_range
    indicates operation is not supported for this file or file system.  */
 
 static bool
-is_CLONENOTSUP (int err)
+is_CLONENOTSUP (int err, bool partial)
 {
   return err == ENOSYS || is_ENOTSUP (err)
          || err == EINVAL || err == EBADF
-         || err == EXDEV || err == ETXTBSY;
+         || err == EXDEV || err == ETXTBSY
+         || (err == EPERM && ! partial);
 }
 
 
@@ -339,15 +352,13 @@ sparse_copy (int src_fd, int dest_fd, char **abuf, size_t buf_size,
           {
             copy_debug.offload = COPY_DEBUG_UNSUPPORTED;
 
-            if (is_CLONENOTSUP (errno))
-              break;
-
-            /* copy_file_range might not be enabled in seccomp filters,
+            /* Consider EPERM if not copied any data yet.  EPERM could occur
+               if copy_file_range not be enabled in seccomp filters,
                so retry with a standard copy.  EPERM can also occur
                for immutable files, but that would only be in the edge case
                where the file is made immutable after creating/truncating,
                in which case the (more accurate) error is still shown.  */
-            if (errno == EPERM && *total_n_read == 0)
+            if (is_CLONENOTSUP (errno, *total_n_read))
               break;
 
             /* ENOENT was seen sometimes across CIFS shares, resulting in
@@ -1172,15 +1183,13 @@ 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)
 {
-  /* If the clone operation is creating the destination,
-     then don't try and cater for all non transient file system errors,
-     and instead only cater for specific transient errors.  */
-  bool transient_failure;
-  if (dest_desc < 0) /* currently for fclonefileat().  */
-    transient_failure = errno == EIO || errno == ENOMEM
-                        || errno == ENOSPC || errno == EDQUOT;
-  else /* currently for FICLONE.  */
-    transient_failure = ! is_CLONENOTSUP (errno);
+  /* If the clone operation is not creating the destination (i.e., FICLONE)
+     then we could possibly be more restrictive with errors deemed non terminal.
+     However doing that like in the following
+     would be more coupled to disparate errno handling on various systems.
+     if (0 <= dest_desc)
+       transient_failure = ! is_CLONENOTSUP (errno, false);  */
+  bool transient_failure = is_transient_failure (errno);
 
   if (reflink_mode == REFLINK_ALWAYS || transient_failure)
     error (0, errno, _("failed to clone %s from %s"),
-- 
2.26.2