Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too)

[jeff.liu]

Hi Jim and Pádraig,

Is it better to adjust the error string from "failed to clone dst_name" to 
"failed to clone
src_name" when clone operation fails? or maybe it would confuse the user IMHO.

If it make sense, one line patch fix it as below:

>From ffb747674f1318a1fec2c2db335d5f29c310fada Mon Sep 17 00:00:00 2001
From: Jie Liu <address@hidden>
Date: Sun, 24 Apr 2011 17:17:40 +0800
Subject: [PATCH 1/1] copy: adjust error string from dst_name to src_name if 
clone failed

* src/copy.c: Print "faild to clone src_name" as the error string if
  Btrfs clone operation fails.

Signed-off-by: Jie Liu <address@hidden>
---
 src/copy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 6edf52e..1ac1f95 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -974,7 +974,7 @@ copy_reg (char const *src_name, char const *dst_name,
         {
           if (!clone_ok)
             {
-              error (0, errno, _("failed to clone %s"), quote (dst_name));
+              error (0, errno, _("failed to clone %s"), quote (src_name));
               return_val = false;
               goto close_src_and_dst_desc;
             }
-- 
1.5.4.3


Thanks,
-Jeff


Jim Meyering wrote:
> Pádraig Brady wrote:
>> Actually I remember now why we didn't want --reflink
>> mutually exclusive with other options.
>> It's so it can be used as configuration to change
>> the data copy method employed. I.E. it operates
>> on much the same level as --sparse in that the
>> results are mostly transparent to the user.
>> I.E. cp could be aliased like this to minimize
>> the amount of space that cp uses:
>>
>> alias cp='cp --reflink=auto --sparse=always'
>>
>> So therefore I've changed the patch as per the attached.
>> I'll apply in a few hours.
>>
>> >From 3dc499ba27b1b0b6365179cb6d7bc9e5f6456600 Mon Sep 17 00:00:00 2001
>> Subject: [PATCH] cp: make --attributes-only override --reflink completely
>>
>> * doc/coreutils.texi (cp invocation): Change the description slightly
>> so as users might not immediately discount using this option.
>> Mention that --reflink is overridden by the other linking options and
>> --attributes-only, and give an example where this might be useful.
>> * src/copy.c (copy_internal): Bypass the reflink if
>> --attributes-only is specifed.
>> * tests/cp/reflink-perm: Ensure both --reflink modes are
>> overridden by --attributes-only.
>> * NEWS: Mention the change in behavior.
> 
> Glad you caught that.  Thanks!
>