Re: cp --reflink with Btrfs
On Wed, Jan 27, 2010 at 11:53:22AM +0100, Andreas Barth wrote: * Sage Weil (s...@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 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cp --reflink with Btrfs
* Sage Weil (s...@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 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cp --reflink with Btrfs
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 s...@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 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cp --reflink with Btrfs
Josef Bacik jo...@redhat.com wrote:
On Sun, Dec 13, 2009 at 12:29:03AM +, Jason White wrote:
I am testing a Btrfs root file system with Debian (kernel 2.6.32) under KVM.
ja...@vrtl:~$ touch testfile
ja...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cp --reflink with Btrfs
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 jo...@redhat.com wrote:
On Sun, Dec 13, 2009 at 12:29:03AM +, Jason White wrote:
I am testing a Btrfs root file system with Debian (kernel 2.6.32) under
KVM.
ja...@vrtl:~$ touch testfile
ja...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-btrfs in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: cp --reflink with Btrfs
On Sun, Dec 13, 2009 at 12:29:03AM +, Jason White wrote: I am testing a Btrfs root file system with Debian (kernel 2.6.32) under KVM. ja...@vrtl:~$ touch testfile ja...@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 -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
