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.
