[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, t
From: |
Erik Auerswald |
Subject: |
Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too) |
Date: |
Tue, 26 Oct 2010 13:38:23 +0200 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
Hi,
On Tue, Oct 26, 2010 at 12:57:08PM +0200, Jim Meyering wrote:
> Erik Auerswald wrote:
> > 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:
That's what I would expect.
> 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)
>
> Writing that makes me think that the combination
> of --attributes-only and --reflink=ANYTHING (or --reflink)
> should elicit an error.
This should be warning, to allow something like "alias cp=cp --reflink=never"
to work with --attributes-only. When --attributes-only is specified, all
modifiers for data copying (--sparse, --reflink) should be ignored and
might elicit a warning. A combination of --attributes-only with --link or
--symbolic-link could result in an error or be treated the same as --sparse
and --reflink.
I did not check how this is currently implemented, so no review of the
patch above...
Erik
--
Thanks to the virtue of me personally not caring one whit about
virtualization, I can stand back and just watch the fireworks.
-- Linus Torvalds
- [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Jim Meyering, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Erik Auerswald, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Jim Meyering, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Pádraig Brady, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Pádraig Brady, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Jim Meyering, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Pádraig Brady, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Jim Meyering, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Pádraig Brady, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too), Jim Meyering, 2010/10/26
- Re: [coreutils] cp/reflink-perm fails on btrfs (and probably on ocfs2, too),
Erik Auerswald <=