From cce55f8f567e33e8c327621cea3b9a354c032939 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Mar 24 2023 15:19:48 +0000 Subject: Related: #2180056 - copy: fix --reflink=auto to fallback in more cases --- diff --git a/coreutils-9.2-cp-reflink.patch b/coreutils-9.2-cp-reflink.patch new file mode 100644 index 0000000..34718a7 --- /dev/null +++ b/coreutils-9.2-cp-reflink.patch @@ -0,0 +1,137 @@ +From af99eb5e5e89b67ecdc582d4face2a4614d5cda8 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?P=C3=A1draig=20Brady?= +Date: Thu, 23 Mar 2023 13:19:04 +0000 +Subject: [PATCH] copy: fix --reflink=auto to fallback in more cases + +On restricted systems like android or some containers, +FICLONE could return EPERM, EACCES, or ENOTTY, +which would have induced the command to fail to copy +rather than falling back to a more standard copy. + +* src/copy.c (is_terminal_failure): A new function refactored +from handle_clone_fail(). +(is_CLONENOTSUP): Merge in the handling of EACCES, ENOTTY, EPERM +as they also pertain to determination of whether cloning is supported +if we ever use this function in that context. +(handle_clone_fail): Use is_terminal_failure() in all cases, +so that we assume a terminal failure in less errno cases. +Addresses https://bugs.gnu.org/62404 + +Upstream-commit: 093a8b4bfaba60005f14493ce7ef11ed665a0176 +Signed-off-by: Kamil Dudka +--- + src/copy.c | 62 ++++++++++++++++++++++++++++++------------------------ + 1 file changed, 35 insertions(+), 27 deletions(-) + +diff --git a/src/copy.c b/src/copy.c +index 3919787..f8ba058 100644 +--- a/src/copy.c ++++ b/src/copy.c +@@ -278,15 +278,27 @@ create_hole (int fd, char const *name, bool punch_holes, off_t size) + } + + +-/* Whether the errno from FICLONE, or copy_file_range +- indicates operation is not supported for this file or file system. */ ++/* 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. */ ++ ++static bool ++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. */ + + static bool + is_CLONENOTSUP (int err) + { +- return err == ENOSYS || is_ENOTSUP (err) ++ return err == ENOSYS || err == ENOTTY || is_ENOTSUP (err) + || err == EINVAL || err == EBADF +- || err == EXDEV || err == ETXTBSY; ++ || err == EXDEV || err == ETXTBSY ++ || err == EPERM || err == EACCES; + } + + +@@ -339,20 +351,18 @@ 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, +- 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, ++ /* Consider operation unsupported only if no data copied. ++ For example, EPERM could occur if copy_file_range not 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, + in which case the (more accurate) error is still shown. */ +- if (errno == EPERM && *total_n_read == 0) ++ if (*total_n_read == 0 && is_CLONENOTSUP (errno)) + break; + + /* ENOENT was seen sometimes across CIFS shares, resulting in + no data being copied, but subsequent standard copies succeed. */ +- if (errno == ENOENT && *total_n_read == 0) ++ if (*total_n_read == 0 && errno == ENOENT) + break; + + if (errno == EINTR) +@@ -1172,17 +1182,15 @@ 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 (reflink_mode == REFLINK_ALWAYS || transient_failure) ++ /* 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. */ ++ bool report_failure = is_terminal_error (errno); ++ ++ if (reflink_mode == REFLINK_ALWAYS || report_failure) + error (0, errno, _("failed to clone %s from %s"), + quoteaf_n (0, dst_name), quoteaf_n (1, src_name)); + +@@ -1190,14 +1198,14 @@ handle_clone_fail (int dst_dirfd, char const* dst_relname, + but cloned no data. */ + if (new_dst /* currently not for fclonefileat(). */ + && reflink_mode == REFLINK_ALWAYS +- && ((! transient_failure) || lseek (dest_desc, 0, SEEK_END) == 0) ++ && ((! report_failure) || lseek (dest_desc, 0, SEEK_END) == 0) + && unlinkat (dst_dirfd, dst_relname, 0) != 0 && errno != ENOENT) + error (0, errno, _("cannot remove %s"), quoteaf (dst_name)); + +- if (! transient_failure) ++ if (! report_failure) + copy_debug.reflink = COPY_DEBUG_UNSUPPORTED; + +- if (reflink_mode == REFLINK_ALWAYS || transient_failure) ++ if (reflink_mode == REFLINK_ALWAYS || report_failure) + return false; + + return true; +-- +2.39.2 + diff --git a/coreutils.spec b/coreutils.spec index 4f25986..acd9614 100644 --- a/coreutils.spec +++ b/coreutils.spec @@ -17,6 +17,9 @@ Source106: coreutils-colorls.csh # cksum: fix reporting of failed checks (#2180056) Patch1: coreutils-9.2-cksum-check.patch +# copy: fix --reflink=auto to fallback in more cases (#2180056) +Patch2: coreutils-9.2-cp-reflink.patch + # do not make coreutils-single depend on /usr/bin/coreutils %global __requires_exclude ^%{_bindir}/coreutils$ @@ -256,6 +259,7 @@ rm -f $RPM_BUILD_ROOT%{_infodir}/dir %changelog * Fri Mar 24 2023 Kamil Dudka - 9.2-3 +- copy: fix --reflink=auto to fallback in more cases (#2180056) - cksum: fix reporting of failed checks (#2180056) * Wed Mar 22 2023 Kamil Dudka - 9.2-2