GNU bug report logs - #6048
[coreutils] Re: Feature: add ocfs2 reflink to cp(1)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 6048 in the body.
You can then email your comments to 6048 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "jeff.liu" <jeff.liu <at> oracle.com>
To: coreutils <at> gnu.org
Cc: Sunil Mushran <sunil.mushran <at> oracle.com>, bug-coreutils <at> gnu.org,
	Jim Meyering <jim <at> meyering.net>, Joel Becker <Joel.Becker <at> oracle.com>,
	Chris Mason <chris.mason <at> oracle.com>,
	Pádraig Brady <P <at> draigBrady.com>,
	Tao Ma <tao.ma <at> oracle.com>
Subject: Re: [coreutils] Re: Feature: add ocfs2 reflink to cp(1)
Date: Tue, 27 Apr 2010 15:41:07 +0800

> Joel Becker wrote:
>> On Sun, Apr 04, 2010 at 11:44:48PM +0800, jeff.liu wrote:
>>> +/* This function detects whether the source and destination files
>>> +   are all resides on OCFS2.  If true, then perform the OCFS2 reflink
>>> +   operation.  Otherwise, performing btrfs clone operation.  */

>> 	I don't know that the correct behavior is btrfs' when ocfs2 is
>> not detected.
> Looks I should not implement 'is_ocfs2_file()' here, it add a bit extra indistinct noise.
> Now my idea is just performing either btrfs clone or ocfs2 reflink operation if cp(1) invoked with
> '--reflink=[WHEN]'.
> If both of them failed no matter what the reason is, fall back to normal copy process when the
> reflink mode is 'REFLINK_AUTO', or abort.
> 
>>> +/* Perform the OCFS2 CoW reflink operation if possible.
>>> +   We do not attempt to preserve security attributes for a reference
>>> +   counted link.  Instead, let 'x->preserve_xxxx' to process them if
>>> +   they are the user expected.
>>> +   Upon success, return 0, Otherwise, return -1 and set errno.  */
>>> +static inline int
>>> +reflink_file (char const *src_name, char const *dst_name,
>>> +              int src_fd, bool *new_dst)
>>> +{
>>> +#ifdef __linux__
>>> +# ifndef REFLINK_ATTR_NONE
>>> +#  define REFLINK_ATTR_NONE 0
>>> +# endif
>> 	If '-p' was specified, you should honor it with
>> REFLINK_ATTR_PRESERVE.
> Thanks for pointing this out.  At first, I think the file attributes preserve should managed through
> normal copy,  its wrong, I will fix it in the next patch submit.
> 
>> Joel
>>
> 
> Thanks,
> -Jeff
> 

Hello All,

This is the revised version to add ocfs2 reflink feature to cp(1) based on Joel's comments.

Changes:
1. Remove 'is_ocfs2_file()' function, it does not need to pre-check the file system type for reflink
operation.  If ocfs2 reflink ioctl(2) failed due to this issue or any other reason, proceed to do
btrfs clone operation.
2. Do perserve attriutes if `cp' invoked with '-p' opition.  In this case,'mode,ownership,timestamp'
are reflinked to destination file; security_context will be reflinked as well if possible.
Also, 'xattrs' are reflinked always no matter preserve_attr or preserve_none specified.
3. From the code layout point of view, I separated ocfs2 reflink and btrfs clone into 2 steps.
the reason is, ocfs2 ioctl(2) play with the destination path name instead of a file descriptor.
So I place its code block at the top of '*new_dst' check, otherwise, we have to unlink(2) the
'dst_desc' when trying ocfs2 reflink ioctl(2).


Tests against preserve option:
=============================

ACL:
---
jeff <at> jeff-laptop:~/opensource_dev/coreutils$ setfacl -m user:oracle:r-- /ocfs2/a
jeff <at> jeff-laptop:~/opensource_dev/coreutils$ ls -l /ocfs2/a
-rw-r--r--+ 1 jeff jeff 5 Apr 27 14:04 /ocfs2/a
jeff <at> jeff-laptop:~/opensource_dev/coreutils$ getfacl /ocfs2/a
getfacl: Removing leading '/' from absolute path names
# file: ocfs2/a
# owner: jeff
# group: jeff
user::rw-
user:oracle:r--
group::r--
mask::r--
other::r--

jeff <at> jeff-laptop:~/opensource_dev/coreutils$ ./src/cp --reflink=always -p /ocfs2/a /ocfs2/c
jeff <at> jeff-laptop:~/opensource_dev/coreutils$ ls -l /ocfs2/c
-rw-r--r--+ 1 jeff jeff 5 Apr 27 14:04 /ocfs2/c
jeff <at> jeff-laptop:~/opensource_dev/coreutils$ getfacl /ocfs2/c
getfacl: Removing leading '/' from absolute path names
# file: ocfs2/c
# owner: jeff
# group: jeff
user::rw-
user:oracle:r--
group::r--
mask::r--
other::r--

mode,ownership,timestamp:
-------------------------
root <at> jeff-laptop:/home/jeff/opensource_dev/coreutils# ./src/cp --reflink=always -p /ocfs2/a
/ocfs2/a_preserve_ownership
root <at> jeff-laptop:/home/jeff/opensource_dev/coreutils# ls -l /ocfs2/a
-rw-r--r--+ 1 jeff jeff 5 Apr 27 14:04 /ocfs2/a
root <at> jeff-laptop:/home/jeff/opensource_dev/coreutils# ls -l /ocfs2/a_preserve_ownership
-rw-r--r--+ 1 jeff jeff 5 Apr 27 14:04 /ocfs2/a_preserve_ownership

Security context:
-----------------
root <at> jeff-laptop:~# chcon oracle:object_r:tmp_t:s0 /ocfs2/a
root <at> jeff-laptop:~# ls -l /ocfs2/a
-rw-r--r--+ 1 jeff jeff 5 Apr 27 14:04 /ocfs2/a
root <at> jeff-laptop:~# ls -Z /ocfs2/a |grep oracle:object_r:tmp_t:s0
oracle:object_r:tmp_t:s0 /ocfs2/a

jeff <at> jeff-laptop:~/opensource_dev/coreutils$ ./src/cp --reflink=always -p /ocfs2/a
/ocfs2/e_preserve_security
-rw-r--r--+ 1 jeff jeff 5 Apr 27 14:04 /ocfs2/e_preserve_security
jeff <at> jeff-laptop:~/opensource_dev/coreutils$ ls -Z /ocfs2/e_preserve_security |grep
oracle:object_r:tmp_t:s0
oracle:object_r:tmp_t:s0 /ocfs2/e_preserve_security


Would you guys please review the patch which shown as following?  I will continue to write the test
script if you guys like the current implemention.

Any comments are appreciated!

From 96d1d86b82ec101caeebe2c51fad6b2a52d093f5 Mon Sep 17 00:00:00 2001
From: Jie Liu <jeff.liu <at> oracle.com>
Date: Tue, 27 Apr 2010 15:37:17 +0800
Subject: [PATCH 1/1] Add ocfs2 reflink to cp(1), it share the same '--reflink=[WHEN]' option
 with btrfs clone operation.

If cp(1) invoked with '-p' option, and the underlaying file system is OCFS2 for
both source and destination files, it will try to preserve the default
attributes(i.e. 'mode,ownership,timestamp'), xattr as well as security context if possbile.

Signed-off-by: Jie Liu <jeff.liu <at> oracle.com>
---
 src/copy.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 76 insertions(+), 1 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index d18fd45..d2ba306 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -54,6 +54,12 @@
 #include "areadlink.h"
 #include "yesno.h"

+#if HAVE_SYS_VFS_H
+# include <sys/vfs.h>
+#else
+# include <sys/statfs.h>
+#endif
+
 #if USE_XATTR
 # include <attr/error_context.h>
 # include <attr/libattr.h>
@@ -132,6 +138,47 @@ utimens_symlink (char const *file, struct timespec const *timespec)
   return err;
 }

+/* Perform the OCFS2 CoW reflink ioctl(2) operation if possible.
+   When using '-p' option, the file's default attributes(i.e. mode,timestamp,
+   ownership and security context if possbile) are reflinked to the destination
+   file as well.  We will then skip over the standard preserve process for such
+   attributes.  Also, 'xattrs' are reflinked always even if 'REFLINK_ATTR_NONE'.
+   Upon success, return 0, Otherwise, return -1 and set errno.  */
+static inline int
+reflink_file (char const *src_name, char const *dst_name,
+              bool preserve_attrs, int src_fd)
+{
+#ifdef __linux__
+# ifndef REFLINK_ATTR_NONE
+#  define REFLINK_ATTR_NONE 0
+# endif
+# ifndef REFLINK_ATTR_PRESERVE
+#  define REFLINK_ATTR_PRESERVE 1
+# endif
+# ifndef OCFS2_IOC_REFLINK
+  struct reflink_arguments {
+   uint64_t old_path;
+   uint64_t new_path;
+   uint64_t preserve;
+  };
+#  define OCFS2_IOC_REFLINK _IOW ('o', 4, struct reflink_arguments)
+# endif
+  struct reflink_arguments args = {
+    .old_path = (unsigned long) src_name,
+    .new_path = (unsigned long) dst_name,
+    .preserve = preserve_attrs ? REFLINK_ATTR_PRESERVE : REFLINK_ATTR_NONE,
+  };
+  return ioctl (src_fd, OCFS2_IOC_REFLINK, &args);
+#else
+  (void) src_name;
+  (void) dst_name;
+  (void) preserve_attrs;
+  (void) src_fd;
+  errno = ENOTSUP;
+  return -1;
+#endif
+}
+
 /* Perform the O(1) btrfs clone operation, if possible.
    Upon success, return 0.  Otherwise, return -1 and set errno.  */
 static inline int
@@ -514,6 +561,31 @@ copy_reg (char const *src_name, char const *dst_name,
       goto close_src_desc;
     }

+  bool reflink_ok = false;
+  if (x->reflink_mode)
+    {
+      /* When cp is invoked with '--reflink=[WHEN]', try to do OCFS2 reflink ioctl(2)
+         first, if it fails, then try Btrfs clone once again at the later time.
+         The reason to perform those operations separately is because `cp' will create
+         the destination file if it is a '*new_dst'.  In this case, we have to unlink(2)
+         it firstly, otherwise, OCFS2 reflink will fail with 'EEXIST'.  But if the destnation
+         file is not a '*new_dst', just let reflink ioctl(2) fails and go ahead.  */
+      bool preserve_attributes = (x->preserve_ownership
+                                  && x->preserve_mode
+                                  && x->preserve_timestamps);
+      reflink_ok = reflink_file (src_name, dst_name, preserve_attributes,
+                                 source_desc) == 0;
+      if (reflink_ok)
+        {
+          *new_dst = false;
+
+          /* Skip over the standard attributes preserve process
+             if reflink succeeds and they are already reflinked.  */
+          if (preserve_attributes)
+            goto close_src_desc;
+        }
+    }
+
   /* The semantics of the following open calls are mandated
      by the specs for both cp and mv.  */
   if (! *new_dst)
@@ -634,8 +706,11 @@ copy_reg (char const *src_name, char const *dst_name,
       goto close_src_and_dst_desc;
     }

-  if (x->reflink_mode)
+  if (x->reflink_mode && ! reflink_ok)
     {
+      /* It seems that the preceeding OCFS2 reflink fails, perform Btrfs
+         clone now, if it fails again and `cp' is invoked with '--reflink=auto',
+         fall back to a standard copy.  */
       bool clone_ok = clone_file (dest_desc, source_desc) == 0;
       if (clone_ok || x->reflink_mode == REFLINK_ALWAYS)
         {
-- 
1.5.4.3




Best Regards,
-Jeff

Message #10 received at 6048 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Kai Petzke <petzke <at> teltarif.de>
Cc: 6048 <at> debbugs.gnu.org, Georg Schmitz <schmitz <at> teltarif.de>
Subject: Re: bug#11443: linux "cp" and ocfs2 reflink/clone/fastcopy/copy on
	write
Date: Mon, 14 May 2012 14:19:42 +0100

[Message part 1 (text/plain, inline)]

I've merged this back to bug 6048

On 05/09/2012 03:28 PM, Kai Petzke wrote:
> Hello,
> 
> 
> there has been work by others about adding support for the OCFS2 "reflink" ioctl() call, which is similiar to the btrfs "clone" call, and creates a copy-on-write copy of the original, thus allowing to "copy" even gigabyte sized files within a tiny fraction of a second, and without using much additional file system space. See:
>     http://lists.gnu.org/archive/html/coreutils/2011-08/msg00046.html
>     http://lists.gnu.org/archive/html/bug-coreutils/2010-04/msg00185.html
> 
> I have updated those patches to work against coreutils 8.16, removed those bugs, that I spotted. In particular, if the destination file exists, the "reflink" ist automatically tried again after removing it, and if not all attributes are copied, it is made sure, that the following open() system call does not truncate the just created copy.
> 
> I strongly suggest including that patch in the coreutils package,

I'm less enthused about adding this as it doesn't fit very cleanly.

> even though the interface to use to different system calls to achieve the same thing is awkward.
> But, as laid out in the comments in the source, btrfs clone and ocfs2 reflink are semantically
> quite different, so that unifying them into one on the kernel side is not likely to happen, soon, if it happens at all.

That would be unfortunate. Hopefully a generic reflink() call can be sorted out.

> If users don't use the --reflink option of "cp", the additional code makes no difference, so it doesn't hurt.

Fair point.

> And if users use "--reflink" on either of the supported file systems, they get a huge advantage out of it!

I really dislike that xattrs are copied unconditionally.
It might be best to auto clear xattrs after the "reflink", if possible?

cheers,
Pádraig.

[coreutils-8.16-ocfs2-reflink.diff (text/plain, attachment)]

Message #13 received at 6048 <at> debbugs.gnu.org (full text, mbox):

From: Assaf Gordon <assafgordon <at> gmail.com>
To: 6048 <at> debbugs.gnu.org
Subject: Re: bug#6048: bug#11443: linux "cp" and ocfs2
 reflink/clone/fastcopy/copy on write
Date: Wed, 10 Oct 2018 10:53:55 -0600

(triaging old bugs)

On 14/05/12 07:19 AM, Pádraig Brady wrote:
> On 05/09/2012 03:28 PM, Kai Petzke wrote:

>> there has been work by others about adding support for the OCFS2
>> "reflink" ioctl() call, which is similiar to the btrfs "clone"
>> call, and creates a copy-on-write copy of the original, thus
>> allowing to "copy" even gigabyte sized files within a tiny fraction
>> of a second, and without using much additional file system space.
>> See:
[...]

>> even though the interface to use to different system calls to achieve the same thing is awkward.
>> But, as laid out in the comments in the source, btrfs clone and ocfs2 reflink are semantically
>> quite different, so that unifying them into one on the kernel side is not likely to happen, soon, if it happens at all.
> 
> That would be unfortunate. Hopefully a generic reflink() call can be sorted out.

In 2014 we had this commit:
https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=b47231be6813e6cb5305266e391b4bb745f27f13
====
commit b47231be6813e6cb5305266e391b4bb745f27f13
Author: David Sterba <dsterba <at> suse.cz>
Date:   Wed Sep 24 11:15:05 2014 +0100

    mv: use reflink=auto mode by default

    On some filesystems (BTRFS), moving a file within the filesystem may
    cross subvolume boundaries and we can use a lightweight reflink copy,
    similar to what cp(1) can do, which is faster than a full file copy.
====

Does this 'reflink=auto' mode addresses OCFS2 as well?
I believe so, but want to double-check.

If there are no objections, I'll mark this as "fixed/done" in couple of 
days.

regards,
 - assaf

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.