[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: cp/reflink support for OCFS2

From: Jeff liu
Subject: Re: cp/reflink support for OCFS2
Date: Sun, 21 Aug 2011 21:47:59 +0800

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
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:

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?

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

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

 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.


+            {
+              error (0, errno, _("failed to reflink ..."));
+              return_val = false;
+              goto close_src_desc;
+            }


reply via email to

[Prev in Thread] Current Thread [Next in Thread]