On 7/6/22 06:17, Pádraig Brady wrote:
This will usually work, but there are cases where this may lose data,
as previously discussed at:
https://bugzilla.redhat.com/show_bug.cgi?id=921708
http://lists.gnu.org/archive/html/coreutils/2013-03/msg00056.html
I'm not sure cp can robustly clean up in this situation?
Thanks for pointing me to those old discussions. As I understand it, the
worry is that FICLONE will only partly succeed, causing the destination
file to contain some (but not all) the input data, and then if we remove
the output file we'll lose the newly-made partial clone. I don't know
whether FICLONE can do that, but it sounds like a reasonable worry.
If that understanding is correct, then the attached further patch should
suffice, so I boldly installed it.
From 123ed2df4c23e12b08e1d18245f3a0b47508496f Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 6 Jul 2022 14:29:12 -0500
Subject: [PATCH] =?UTF-8?q?cp:=20don=E2=80=99t=20remove=20nonempty=20clone?=
=?UTF-8?q?d=20dest?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This follows up on comments by Pádraig Brady (bug#56391).
* src/copy.c (copy_reg): When --reflink=always removes a file
due to an FICLONE failure, do not remove a nonempty file.
---
NEWS | 3 +++
src/copy.c | 12 ++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/NEWS b/NEWS
index a3a55541e..b4e3cf83a 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ GNU coreutils NEWS -*- outline -*-
** Changes in behavior
+ 'cp --reflink=always A B' no longer leaves behind a newly created
+ empty file B merely because copy-on-write clones are not supported.
+
'ls -v' and 'sort -V' go back to sorting ".0" before ".A",
reverting to the behavior in coreutils-9.0 and earlier.
This behavior is now documented.
diff --git a/src/copy.c b/src/copy.c
index eaed148b4..e465271ef 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1279,9 +1279,17 @@ copy_reg (char const *src_name, char const *dst_name,
{
error (0, errno, _("failed to clone %s from %s"),
quoteaf_n (0, dst_name), quoteaf_n (1, src_name));
- if (*new_dst && unlinkat (dst_dirfd, dst_relname, 0) != 0
- && errno != ENOENT)
+
+ /* Remove the destination if cp --reflink=always created it
+ but cloned no data. If clone_file failed with
+ EOPNOTSUPP, EXDEV or EINVAL no data were copied so do not
+ go to the expense of lseeking. */
+ if (*new_dst
+ && (is_ENOTSUP (errno) || errno == EXDEV || errno == EINVAL
+ || 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));
+
return_val = false;
goto close_src_and_dst_desc;
}
--
2.36.1
