cp --reflink with Btrfs

cp --reflink with Btrfs

[Jason White]

I am testing a Btrfs root file system with Debian (kernel 2.6.32) under KVM.

jason@vrtl:~$ touch testfile
jason@vrtl:~$ cp --reflink testfile /tmp
cp: failed to clone `/tmp/testfile': Invalid argument

This is with GNU Coreutils 8.0 taken from debian Sid.

Is this a Coreutils issue, a Btrfs problem or something in my local
configuration?

All the other issues that I have encountered so far are known limitations, for
example, lack of Btrfs support in Grub 2 and the current inability of the
initrd scripts from Debian to recognize the file system type. All of this will
be fixed, no doubt, over time.

Also, if this is the wrong list for such questions, I would appreciate a
reference to the preferred mailing list.

Thank you for an excellent project which is very important to the future of
file systems under Linux.

Re: cp --reflink with Btrfs

[Josef Bacik]

On Sun, Dec 13, 2009 at 12:29:03AM +0000, Jason White wrote:
> I am testing a Btrfs root file system with Debian (kernel 2.6.32) under KVM.
> 
> jason@vrtl:~$ touch testfile
> jason@vrtl:~$ cp --reflink testfile /tmp
> cp: failed to clone `/tmp/testfile': Invalid argument
> 
> This is with GNU Coreutils 8.0 taken from debian Sid.
> 
> Is this a Coreutils issue, a Btrfs problem or something in my local
> configuration?
>

Try using bcp, if that works then its likely a problem with coreutils.

> All the other issues that I have encountered so far are known limitations, for
> example, lack of Btrfs support in Grub 2 and the current inability of the
> initrd scripts from Debian to recognize the file system type. All of this will
> be fixed, no doubt, over time.
> 

Yeah the initrd stuff is all debian "problems".  When I added btrfs support to
fedora there were alot of various utilities and such that needed to be changed
in order to accomodate it, so the same sort of thing likely needs to be done
with debian.  There are patches for Grub1 since thats what Fedora uses, I
suppose that there will be Grub2 patches in the future, but since the author of
the grub patches uses fedora and we're on grub1 it's all we care about :).
Thanks,

Josef

Re: cp --reflink with Btrfs

[Jason White]

Josef Bacik  <josef@redhat.com> wrote:
>On Sun, Dec 13, 2009 at 12:29:03AM +0000, Jason White wrote:
>> I am testing a Btrfs root file system with Debian (kernel 2.6.32) under KVM.
>> 
>> jason@vrtl:~$ touch testfile
>> jason@vrtl:~$ cp --reflink testfile /tmp
>> cp: failed to clone `/tmp/testfile': Invalid argument
>> 
>> This is with GNU Coreutils 8.0 taken from debian Sid.
>> 
>> Is this a Coreutils issue, a Btrfs problem or something in my local
>> configuration?
>>
>
>Try using bcp, if that works then its likely a problem with coreutils.

After reporting this to Debian and engaging on follow-up discussion, it turns
out that bcp copies the data if the ioctl() call to clone the file fails, as
can be seen from the Python code (which I should have read, but didn't...).

Unfortunately the ioctl() call is failing both in bcp and in cp --reflink.

Here's partial strace output from the latter.

cp --reflink testfile testfile2

open("testfile", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
open("testfile2", O_WRONLY|O_CREAT|O_EXCL, 0644) = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
ioctl(4, 0x40049409, 0x3)               = -1 EINVAL (Invalid argument)

Kernel 2.6.32 (debian Sid), x86-64 architecture.

Suggestions welcome.

Debian bug report:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=561225

Re: cp --reflink with Btrfs

[Li Dongyang]

Have a look at line 998, ioctl.c, inside btrfs_ioctl_clone(),
the src->i_size(the size of the testfile created by touch) is just 0, and this 
will cause btrfs_ioctl_clone just return -EINVAL.
I'm not sure if it makes sense to clone a file which actually doesn't have any 
data extents.

On Wednesday 16 December 2009 07:37:42 Jason White wrote:
> Josef Bacik  <josef@redhat.com> wrote:
> >On Sun, Dec 13, 2009 at 12:29:03AM +0000, Jason White wrote:
> >> I am testing a Btrfs root file system with Debian (kernel 2.6.32) under
> >> KVM.
> >>
> >> jason@vrtl:~$ touch testfile
> >> jason@vrtl:~$ cp --reflink testfile /tmp
> >> cp: failed to clone `/tmp/testfile': Invalid argument
> >>
> >> This is with GNU Coreutils 8.0 taken from debian Sid.
> >>
> >> Is this a Coreutils issue, a Btrfs problem or something in my local
> >> configuration?
> >
> >Try using bcp, if that works then its likely a problem with coreutils.
> 
> After reporting this to Debian and engaging on follow-up discussion, it
>  turns out that bcp copies the data if the ioctl() call to clone the file
>  fails, as can be seen from the Python code (which I should have read, but
>  didn't...).
> 
> Unfortunately the ioctl() call is failing both in bcp and in cp --reflink.
> 
> Here's partial strace output from the latter.
> 
> cp --reflink testfile testfile2
> 
> open("testfile", O_RDONLY)              = 3
> fstat(3, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> open("testfile2", O_WRONLY|O_CREAT|O_EXCL, 0644) = 4
> fstat(4, {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> ioctl(4, 0x40049409, 0x3)               = -1 EINVAL (Invalid argument)
> 
> Kernel 2.6.32 (debian Sid), x86-64 architecture.
> 
> Suggestions welcome.
> 
> Debian bug report:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=561225
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: cp --reflink with Btrfs

[Sage Weil]

On Wed, 16 Dec 2009, Li Dongyang wrote:

> Have a look at line 998, ioctl.c, inside btrfs_ioctl_clone(),
> the src->i_size(the size of the testfile created by touch) is just 0, and this 
> will cause btrfs_ioctl_clone just return -EINVAL.
> I'm not sure if it makes sense to clone a file which actually doesn't have any 
> data extents.

Probably not, but it seems more consistent to return success instead than 
-EINVAL.  Requiring the caller to check and special case empty files isn't 
very friendly...

sage

---
Subject: [PATCH] Btrfs: return success when cloning 0 byte range at eof

We currently return -EINVAL when cloning a zero byte range at EOF (most 
commonly when cloning a 0 byte file).  Return success instead, even though 
this is a no-op.

Signed-off-by: Sage Weil <sage@newdream.net>
---
 fs/btrfs/ioctl.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index cdbb054..1a964a4 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -994,8 +994,11 @@ static noinline long btrfs_ioctl_clone(struct file *file, unsigned long srcfd,
 	}
 
 	/* determine range to clone */
+	ret = 0;
+	if (off == src->i_size && len == 0)
+		goto out_unlock;
 	ret = -EINVAL;
-	if (off >= src->i_size || off + len > src->i_size)
+	if (off > src->i_size || off + len > src->i_size)
 		goto out_unlock;
 	if (len == 0)
 		olen = len = src->i_size - off;
-- 
1.6.5

Re: cp --reflink with Btrfs

[Andreas Barth]

* Sage Weil (sage@newdream.net) [091216 17:55]:
> On Wed, 16 Dec 2009, Li Dongyang wrote:
> 
> > Have a look at line 998, ioctl.c, inside btrfs_ioctl_clone(),
> > the src->i_size(the size of the testfile created by touch) is just 0, and this 
> > will cause btrfs_ioctl_clone just return -EINVAL.
> > I'm not sure if it makes sense to clone a file which actually doesn't have any 
> > data extents.
> 
> Probably not, but it seems more consistent to return success instead than 
> -EINVAL.  Requiring the caller to check and special case empty files isn't 
> very friendly...


Actually it makes lots of sense if trying to e.g. cow-copy an chroot
on an buildd, and there are some empty files inside of the linux
installation (which normally just are).

Can I hope on getting the patch (or another patch) incorporated into
the kernel?



Cheers,
Andi

Re: cp --reflink with Btrfs

[Chris Mason]

On Wed, Jan 27, 2010 at 11:53:22AM +0100, Andreas Barth wrote:
> * Sage Weil (sage@newdream.net) [091216 17:55]:
> > On Wed, 16 Dec 2009, Li Dongyang wrote:
> > 
> > > Have a look at line 998, ioctl.c, inside btrfs_ioctl_clone(),
> > > the src->i_size(the size of the testfile created by touch) is just 0, and this 
> > > will cause btrfs_ioctl_clone just return -EINVAL.
> > > I'm not sure if it makes sense to clone a file which actually doesn't have any 
> > > data extents.
> > 
> > Probably not, but it seems more consistent to return success instead than 
> > -EINVAL.  Requiring the caller to check and special case empty files isn't 
> > very friendly...
> 
> 
> Actually it makes lots of sense if trying to e.g. cow-copy an chroot
> on an buildd, and there are some empty files inside of the linux
> installation (which normally just are).
> 
> Can I hope on getting the patch (or another patch) incorporated into
> the kernel?

In this case the application is responsible for duplicating the file.
The clone ioctl doesn't actually clone any of the file metadata or
anything else.

-chris