Thanks for your review. On 05/03/2011 03:47 PM, jeff.liu wrote: Hello All,
I'd like to introduce the ocfs2 reflink support to cp(1) when it was called with
"--reflink=[WHEN]".
With this patch, `cp' will try OCFS2 reflink first, if it fails with EEXIST, IMHO, it definitely
means the user is intended to perform reflink on OCFS2, but the destination file is already exists,
so set the retval = false and return, or try Btrfs clone again.
I have done some tests, includes reflink on OCFS2, reflink to an existing file, reflink files
cross-filesystems, and reflink attributes only, all works fine.
For the test automation, the existing reflink test are presume the tests running on either file
systems with Cow support IMO, maybe we can improve them with real filesystems on loop device?
Also, the old bug ticket for this topic will be closed at:
http://lists.gnu.org/archive/html/bug-coreutils/2010-04/msg00185.html
Thanks for doing this. It's a pity we've to try different file system specific CoW methods. Is there any news on the reflink system call? https://lkml.org/lkml/2009/9/14/532
Looks there is no update recently;
Hi Joel and Sunil, Would you please add some comments in this point.
I notice that xattrs are copied with REFLINK_ATTR_NONE.
xattrs will not copied if call ioctl(2) with REFLINK_ATTR_NONE,
reflink xattrs only if
REFLINK_ATTR_PRESERVE is specified, which were shown as following:
Firstly, invoking cp(1) without "-p" option:
jeff@jeff-laptop:~/opensource/coreutils$ setfacl -m user:jeff:rwx /ocfs2/test
jeff@jeff-laptop:~/opensource/coreutils$ ./src/cp --reflink=always
/ocfs2/test /ocfs2/test.reflink
jeff@jeff-laptop:~/opensource/coreutils$ ls -l /ocfs2/test.reflink
-rw-r-xr-- 1 jeff jeff 0 2011-08-21 21:16 /ocfs2/test.reflink
jeff@jeff-laptop:~/opensource/coreutils$ getfacl /ocfs2/test.reflink
getfacl: Removing leading '/' from absolute path names
# file: ocfs2/test.reflink
# owner: jeff
# group: jeff
user::rw-
group::r-x
other::r--
Next, try to copy xattrs too by specifying "-p":
--------------------------------------------------------------
jeff@jeff-laptop:~/opensource/coreutils$ ./src/cp --reflink=always -p
/ocfs2/test /ocfs2/test.reflink_with_xattr
jeff@jeff-laptop:~/opensource/coreutils$ getfacl /ocfs2/test.reflink_with_xattr
getfacl: Removing leading '/' from absolute path names
# file: ocfs2/test.reflink_with_xattr
# owner: jeff
# group: jeff
user::rw-
user:jeff:rwx
group::r--
mask::rwx
other::r--
Is that also the case for BTRFS clone?
Btrfs will not try to clone the xattrs by default too. In any case that's a change I'm not sure is desired, or worth noting at least. I don't understand this bit... + if (reflink_ok)
+ {
... + }
+ else
+ {
+ /* When the destination file is aready exists on OCFS2, the
+ above reflink should fails with EEXIST. If that happens,
+ we need not try btrfs clone again since it means the user
+ is definitely want a OCFS2 reflink. */
+ if (errno == EEXIST)
Should this not be errno != ENOTSUP
Thanks for pointing this out! I should checking errno != ENOTSUP here, then break if "--sparse=always" was specified, otherwise, let copy process go ahead. Or why not try to unlink() (dependent on -f, -n etc.).
At least shouldn't we be falling back to a regular copy when --reflink=auto
The current patch will fall back to a normal copy if reflink falled in both cases(ocfs2, btrfs), if cp(1) was invoked with "--reflink=auto". :-P.
Thanks, -Jeff + {
+ error (0, errno, _("failed to reflink ..."));
+ return_val = false;
+ goto close_src_desc;
+ }
cheers, Pádraig.
|