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

[jeff.liu]

Hi Jim,

Sorry, I have made a mistake in previous version, it should call 'quote_n' 
instead I think.

Or upon a clone failure, the diagnostic message was shown as:
jeff@jeff-wowo:/btrfs/coreutils$ ./src/cp --reflink=always /ocfs2/a /btrfs/b
./src/cp: failed to clone `/btrfs/b' from `/btrfs/b': Invalid cross-device link


Below patch can fix it.
diff --git a/src/copy.c b/src/copy.c
index 65566a0..801a474 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -975,7 +975,7 @@ copy_reg (char const *src_name, char const *dst_name,
           if (!clone_ok)
             {
               error (0, errno, _("failed to clone %s from %s"),
-                     quote (dst_name), quote (src_name));
+                     quote_n (0, dst_name), quote_n (1, src_name));
               return_val = false;
               goto close_src_and_dst_desc;
             }

then:
jeff@jeff-wowo:/btrfs/coreutils$ ./src/cp --reflink=always /ocfs2/a /btrfs/b
./src/cp: failed to clone `/btrfs/b' from `/ocfs2/a': Invalid cross-device link


Btw, I'd like to re-submit the ocfs2 relink patch soon, how about if I post it 
to this thread and
close the old ticket at:
http://lists.gnu.org/archive/html/bug-coreutils/2010-04/msg00185.html



Thanks,
-Jeff

jeff.liu wrote:
> Hi Jim,
> 
> Jim Meyering wrote:
>> jeff.liu wrote:
>>> 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.
>> Hi Jeff,
>>
>> Thanks for the patch.  Mentioning only one of source and destination,
>> I prefer to list the destination name, since it is more often relevant
>> than the source.  Sometimes it's useful to know both (e.g., EXDEV) so
>> I propose this instead:
> Exactly, it's more meaningful to list the destination file too.
> 
>> I changed the log message, too, but left your name as author,
> thank you.
> 
>> so I'll wait for an ACK from you before pushing.
> Acked.
> 
> 
> Regards,
> -Jeff
>>
>> From d5bd22605a9ff8289d6c02cd2d3e81d460bae873 Mon Sep 17 00:00:00 2001
>> From: Jeff Liu <address@hidden>
>> Date: Sun, 24 Apr 2011 16:25:31 +0200
>> Subject: [PATCH] copy: include both src and dest names in clone failure
>>  diagnostic
>>
>> * src/copy.c (copy_reg): Upon btrfs clone failure, print not just
>> the destination file name, but also the source file name.
>> That may be useful upon failure of a cross-device clone attempt.
>> ---
>>  src/copy.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/copy.c b/src/copy.c
>> index 6edf52e..65566a0 100644
>> --- a/src/copy.c
>> +++ b/src/copy.c
>> @@ -974,7 +974,8 @@ 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 from %s"),
>> +                     quote (dst_name), quote (src_name));
>>                return_val = false;
>>                goto close_src_and_dst_desc;
>>              }
>> --
>> 1.7.5.rc3.316.gd1ff9
> 
>