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

[Pádraig Brady]

On 26/10/10 11:57, Jim Meyering wrote:
> Erik Auerswald wrote:
> 
>> Hi,
>>
>> On Tue, Oct 26, 2010 at 11:39:58AM +0200, Jim Meyering wrote:
>>> The cp/reflink-perm test fails on a btrfs file system:
>>>
>>>     echo > file2 # file with data
>>>     cp --reflink=auto --preserve --attributes-only file2 empty_copy || 
>>> fail=1
>>>     test -s empty_copy && fail=1
>>>
>>> because with --reflink=auto, the file contents are copied, too,
>>
>> Is this intentional? I would have expected that no data is copied, because
>> this is stated in the documentation. The --reflink option should change
>> behaviour of a data copy only, IMHO.
> 
> Thanks for the feedback.
> 
> I wondered the same thing and looked at this part of copy.c,
> but I didn't look carefully enough.
> 
> [data_copy_required defaults to true, but is set to false
>  with --attributes-only ]
> 
>   if (x->reflink_mode)
>     {
>       bool clone_ok = clone_file (dest_desc, source_desc) == 0;
>       if (clone_ok || x->reflink_mode == REFLINK_ALWAYS)
>         {
>           if (!clone_ok)
>             {
>               error (0, errno, _("failed to clone %s"), quote (dst_name));
>               return_val = false;
>               goto close_src_and_dst_desc;
>             }
>           data_copy_required = false;
>         }
>     }
> 
> Now, I think you're right.
> Perhaps, with --attributes-only cp should not even attempt
> the reflink copy, like this:
> 
> diff --git a/src/copy.c b/src/copy.c
> index df31592..f49722f 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -622,7 +622,7 @@ copy_reg (char const *src_name, char const *dst_name,
>        goto close_src_and_dst_desc;
>      }
> 
> -  if (x->reflink_mode)
> +  if (data_copy_required && x->reflink_mode)
>      {
>        bool clone_ok = clone_file (dest_desc, source_desc) == 0;
>        if (clone_ok || x->reflink_mode == REFLINK_ALWAYS)
> 

Well with --reflink=auto --preserve --attributes-only there is no data copy,
as only the inode is copied, using "reflink()" if possible, or falling back to
the traditional method.

Now I'm not sure what I was thinking exactly, but I think the test is valid.
Perhaps I was thinking of how to most efficiently/portably copy just the inode
(for the inplace script I was working on), with something like:

cp --attributes-only --version >/dev/null 2>&1 && attronly="--attributes-only"
cp -reflink=auto --version >/dev/null 2>&1 && reflink="-reflink=auto"
cp $reflink $attr-only --preserve orig new; : > new

But one could just as easily separate these with:

if cp --attributes-only --version >/dev/null 2>&1; then
  attronly="--attributes-only"
elif cp -reflink=auto --version >/dev/null 2>&1; then
  reflink="-reflink=auto"
fi
cp $reflink $attronly --preserve orig new; : > new

> Writing that makes me think that the combination
> of --attributes-only and --reflink=ANYTHING (or --reflink)
> should elicit an error.

So the question is, should --attributes-only override --reflink
or vice versa. I'm thinking for consistency with --link and --symbolic-link
that --reflink should override --attributes-only. If we don't error
then we should at least document:

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f28b894..2dc6c5a 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7408,7 +7408,8 @@ Equivalent to @option{-dR --preserve=all} with the 
reduced diagnostics.
 @opindex --attributes-only
 Preserve the specified attributes of the original files in the copy,
 but do not copy any data.  See the @option{--preserve} option for
-controlling which attributes to copy.
+controlling which attributes to copy.  This option is overridden by the
+@option{--link}, @option{--symbolic-link} and @option{--reflink} options.

On a related note, --link and --symbolic currently override --reflink.
Now combining --link and --symb gives an error, so for consistency
I guess we should error when any 2 of the 3 linking modes are specified.

So in summary error if any of --link, --symbolic-link,
--reflink or --attributes-only are combined.

cheers,
Pádraig.