Re: BTRFS file clone support for cp

[Pádraig Brady]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:
 
 I am now convinced that cp's new behavior belongs on
 a separate option, --reflink (i.e., it should not be the default).
 Giuseppe, do you feel like adding that option and adjusting your
 test accordingly?
 
 I attached two separate patches, --reflink option and file-clone test.
 Last versions of btrfs have a bug (I asked on #btrfs and they confirmed
 it), btrfs doesn't use correctly all the free space available.  In fact
 I get ENOSPC while in reality only 54% is used.  Probably it is better
 to postpone the second patch inclusion after the bug is fixed.

That explains some of the weirdness I noticed.
Thanks for looking into it.
(there was also weird values for `du cloned_file`)

 Another note, I changed this line in the NEWS file:
 -  when both the source and destination are on the same btrfs partition.
 
 considering that BTRFS supports multiple devices I am not convinced that
 it is always true, I guess source and destination could be on different
 partitions, though I couldn't find a clear answer on the btrfs wiki to
 this question.

I would very much doubt one could clone outside a single file system.
If a file system can span multiple devices/partitions then maybe.
In any case would be better to say file system rather than partition,
as the latter is not directly related.

cheers,
Pádraig.

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 +  if (clone_file (dest_desc, source_desc))
 +{
 +  error (0, errno, _(cannot fstat %s), quote (dst_name));

 I prefer this diagnostic ;-)

  error (0, errno, _(failed to clone %s), quote (dst_name));

 Too wild copypaste :)

 I included your notes in the following patch.

Thanks.  I think this is finally ready.

Here are a few more small changes:
(alphabetize and change one-line description)

diff --git a/src/cp.c b/src/cp.c
index 635c7c7..ae1c1ce 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -192,11 +192,11 @@ Mandatory arguments to long options are mandatory for 
short options too.\n\
 ), stdout);
   fputs (_(\
   -R, -r, --recursive  copy directories recursively\n\
+  --reflinkperform a lightweight (CoW/clone) copy\n\
   --remove-destination remove each existing destination file before\n\
  attempting to open it (contrast with 
--force)\n\
 ), stdout);
   fputs (_(\
-  --reflinkclone the file if it is possible\n\
   --sparse=WHENcontrol creation of sparse files\n\
   --strip-trailing-slashes  remove any trailing slashes from each SOURCE\n\
  argument\n\

I did the same with the addition of REFLINK_OPTION here:

  enum
  {
COPY_CONTENTS_OPTION = CHAR_MAX + 1,
NO_PRESERVE_ATTRIBUTES_OPTION,
PARENTS_OPTION,
PRESERVE_ATTRIBUTES_OPTION,
REFLINK_OPTION,
SPARSE_OPTION,
STRIP_TRAILING_SLASHES_OPTION,
UNLINK_DEST_BEFORE_OPENING
  };

Plus, I adjusted the log message:

From a1d7469835371ded0ad8e3496bc5a5bebf94ccef Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 1 Aug 2009 19:36:48 +0200
Subject: [PATCH] cp: accept the --reflink option

* NEWS: Mention it.
* doc/coreutils.texi (cp invocation): Describe it.
* src/copy.h (struct cp_options) [reflink]: New member.
* src/copy.c (usage): Describe it.
(copy_reg): If reflink is true try to clone the file.
(main): Check for --reflink.
(cp_option_init): Initialize the new member.
* src/install.c (cp_option_init): Initialize the new member.
* src/mv.c (cp_option_init): Likewise.
* tests/cp/sparse: Add a new test case.
---
 NEWS   |4 
 doc/coreutils.texi |9 +
 src/copy.c |   16 ++--
 src/copy.h |3 +++
 src/cp.c   |   14 ++
 src/install.c  |1 +
 src/mv.c   |1 +
 tests/cp/sparse|4 
 8 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index c61f666..6df0d65 100644
--- a/NEWS
+++ b/NEWS
@@ -41,6 +41,10 @@ GNU coreutils NEWS-*- 
outline -*-

   chroot now accepts the options --userspec and --groups.

+  cp accepts a new option, --reflink: create a lightweight copy
+  using copy-on-write (COW).  This is currently supported only on
+  btrfs file systems.
+
   cp now preserves time stamps on symbolic links, when possible

   cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 90a54b2..bb1d87a 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7543,6 +7543,15 @@ cp invocation
 unless you also specify @option{-P}, as @acronym{POSIX} allows
 implementations that dereference symbolic links by default.

+...@item --reflink
+...@opindex --reflink
+Perform a lightweight, copy-on-write (COW) copy.
+Copying with this option can succeed only on some relatively new file systems.
+Once it has succeeded, beware that the source and destination files
+share the same disk data blocks as long as they remain unmodified.
+Thus, if a disk I/O error affects data blocks of one of the files,
+the other suffers the exact same fate.
+
 @item --remove-destination
 @opindex --remove-destination
 Remove each existing destination file before attempting to open it
diff --git a/src/copy.c b/src/copy.c
index 24b5f6b..bed90c4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -624,13 +624,16 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }

-  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
- If the operation is not supported or it fails then copy the file
- in the usual way.  */
-  bool copied = (x-sparse_mode == SPARSE_AUTO
-  clone_file (dest_desc, source_desc) == 0);
+  if (x-reflink)
+{
+  if (clone_file (dest_desc, source_desc))
+{
+  error (0, errno, _(failed to clone %s), quote (dst_name));
+  return_val = false;
+}
+  goto close_src_and_dst_desc;
+}

-  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
@@ -2232,6 +2235,7 @@ valid_options (const struct cp_options *co)
   assert (VALID_BACKUP_TYPE (co-backup_type));
   assert (VALID_SPARSE_MODE (co-sparse_mode));
   assert (!(co-hard_link  

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 I attached the cleaned patch, according to your advises.  If there are
 other things, I'll fix them.

Thanks!

...
 +...@item --reflink
 +...@opindex --reflink
 +Attempt a O(1) copy-on-write (COW) when the underlying file system
 +supports this operation instead of really copying the file.
 +The source and destination files share the same disk data blocks until
 +they are equal.  Changes done in a file are not visible in the other
 +one because shared blocks will be duplicated before these changes are
 +stored.

I'll adjust the wording above, to something like this:

Perform a lightweight, copy-on-write (COW) copy.
Copying with this option can succeed only on some relatively new file systems.
Once it has succeeded, beware that the source and destination files
share the same disk data blocks as long as they remain unmodified.
Thus, if a disk I/O error affects data blocks of one of the files,
the other suffers the exact same fate.

  @item --remove-destination
  @opindex --remove-destination
  Remove each existing destination file before attempting to open it
 diff --git a/src/copy.c b/src/copy.c
 index bbed336..5eae9f4 100644
 --- a/src/copy.c
 +++ b/src/copy.c
 @@ -610,13 +610,16 @@ copy_reg (char const *src_name, char const *dst_name,
goto close_src_and_dst_desc;
  }

 -  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
 - If the operation is not supported or it fails then copy the file
 - in the usual way.  */
 -  bool copied = (x-sparse_mode == SPARSE_AUTO
 -  clone_file (dest_desc, source_desc) == 0);
 +  if (x-reflink)
 +{
 +  if (clone_file (dest_desc, source_desc))
 +{
 +  error (0, errno, _(cannot fstat %s), quote (dst_name));

I prefer this diagnostic ;-)

 error (0, errno, _(failed to clone %s), quote (dst_name));

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Jim Meyering j...@meyering.net writes:

 +  if (clone_file (dest_desc, source_desc))
 +{
 +  error (0, errno, _(cannot fstat %s), quote (dst_name));

 I prefer this diagnostic ;-)

  error (0, errno, _(failed to clone %s), quote (dst_name));

Too wild copypaste :)  

I included your notes in the following patch.

Cheers,
Giuseppe


From 63c0a1840f236eebb9ba3a28d8f1e6242a7c5898 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 1 Aug 2009 19:36:48 +0200
Subject: [PATCH] cp: accept the --reflink option

* NEWS: Mention it.
* doc/coreutils.texi: Likewise.
* src/copy.h (struct cp_options): New member reflink.
* src/copy.c (usage): Likewise.
(copy_reg): If reflink is true try to clone the file.
(main): Check for --reflink.
(cp_option_init): By default set reflink to false.
* src/install.c (cp_option_init): By default set reflink to false.
* src/mv.c (cp_option_init): By default set reflink to false.
* tests/cp/sparse: Add a new test case.
---
 NEWS   |5 +++--
 doc/coreutils.texi |9 +
 src/copy.c |   16 ++--
 src/copy.h |3 +++
 src/cp.c   |   16 +++-
 src/install.c  |1 +
 src/mv.c   |1 +
 tests/cp/sparse|4 
 8 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index 80c60e2..94511b7 100644
--- a/NEWS
+++ b/NEWS
@@ -35,8 +35,9 @@ GNU coreutils NEWS-*- 
outline -*-
 
   chroot now accepts the options --userspec and --groups.
 
-  cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
-  when both the source and destination are on the same btrfs partition.
+  cp accepts a new option, --reflink: create a lightweight copy
+  using copy-on-write (COW).  This is currently supported only on
+  btrfs file systems.
 
   sort accepts a new option, --human-numeric-sort (-h): sort numbers
   while honoring human readable suffixes like KiB and MB etc.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index acec76e..a300d56 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7543,6 +7543,15 @@ Also, it is not portable to use @option{-R} to copy 
symbolic links
 unless you also specify @option{-P}, as @acronym{POSIX} allows
 implementations that dereference symbolic links by default.
 
+...@item --reflink
+...@opindex --reflink
+Perform a lightweight, copy-on-write (COW) copy.
+Copying with this option can succeed only on some relatively new file systems.
+Once it has succeeded, beware that the source and destination files
+share the same disk data blocks as long as they remain unmodified.
+Thus, if a disk I/O error affects data blocks of one of the files,
+the other suffers the exact same fate.
+
 @item --remove-destination
 @opindex --remove-destination
 Remove each existing destination file before attempting to open it
diff --git a/src/copy.c b/src/copy.c
index bbed336..3e83de3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -610,13 +610,16 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }
 
-  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
- If the operation is not supported or it fails then copy the file
- in the usual way.  */
-  bool copied = (x-sparse_mode == SPARSE_AUTO
-  clone_file (dest_desc, source_desc) == 0);
+  if (x-reflink)
+{
+  if (clone_file (dest_desc, source_desc))
+{
+  error (0, errno, _(failed to clone %s), quote (dst_name));
+  return_val = false;
+}
+  goto close_src_and_dst_desc;
+}
 
-  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
@@ -,6 +2225,7 @@ valid_options (const struct cp_options *co)
   assert (VALID_BACKUP_TYPE (co-backup_type));
   assert (VALID_SPARSE_MODE (co-sparse_mode));
   assert (!(co-hard_link  co-symbolic_link));
+  assert (!(co-reflink  co-sparse_mode != SPARSE_AUTO));
   return true;
 }
 
diff --git a/src/copy.h b/src/copy.h
index 8e0b408..ddf4f4e 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -219,6 +219,9 @@ struct cp_options
  such a symlink) and returns false.  */
   bool open_dangling_dest_symlink;
 
+  /* If true, attempt to clone the file instead of copying it.  */
+  bool reflink;
+
   /* This is a set of destination name/inode/dev triples.  Each such triple
  represents a file we have created corresponding to a source file name
  that was specified on the command line.  Use it to avoid clobbering
diff --git a/src/cp.c b/src/cp.c
index 8785076..635c7c7 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -78,7 +78,8 @@ enum
   PRESERVE_ATTRIBUTES_OPTION,
   SPARSE_OPTION,
   STRIP_TRAILING_SLASHES_OPTION,
-  UNLINK_DEST_BEFORE_OPENING
+  UNLINK_DEST_BEFORE_OPENING,
+  REFLINK_OPTION
 };
 
 /* True if the kernel is SELinux enabled.  */
@@ -121,6 +122,7 @@ static struct option const long_opts[] =
   {recursive, no_argument, NULL, 'R'},
   

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Hi Jim,

I attached the cleaned patch, according to your advises.  If there are
other things, I'll fix them.

Regards,
Giuseppe


Jim Meyering j...@meyering.net writes:

 How about this for NEWS:

   cp accepts a new option, --reflink: create a lightweight copy
   using copy-on-write (COW).  This is currently supported only on
   btrfs file systems.

 Typically, with a feature like this, if a tool is unable to provide
 the functionality implied by the new option, it gives a diagnostic
 and exits nonzero.  Otherwise, it would be far more work for an
 application to determine whether the option was honored.

 Deciding whether we also want an option saying
 copy-via-COW-if-possible-and-ignore-any-failure can wait,
 but I'm leaning away from it for now.

 
 With your change, the new reflink member may be used uninitialized
 via install and mv.  To avoid that, just initialize it to false in
 each cp_option_init.

 Also, please make cp give a diagnostic when --reflink is used with
 --sparse=never or --sparse=always.  Then, add this assertion in copy.c:

 assert ( ! (x-reflink  x-sparse_mode != SPARSE_AUTO));



From 75610aed5c325d14a3ef43fb2c319ace12f36c57 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 1 Aug 2009 19:36:48 +0200
Subject: [PATCH] cp: accept the --reflink option

* NEWS: Mention it.
* doc/coreutils.texi: Likewise.
* src/copy.h (struct cp_options): New member reflink.
* src/copy.c (usage): Likewise.
(copy_reg): If reflink is true try to clone the file.
(main): Check for --reflink.
(cp_option_init): By default set reflink to false.
* src/install.c (cp_option_init): By default set reflink to false.
* src/mv.c (cp_option_init): By default set reflink to false.
* tests/cp/sparse: Add a new test case.
---
 NEWS   |5 +++--
 doc/coreutils.texi |9 +
 src/copy.c |   16 ++--
 src/copy.h |3 +++
 src/cp.c   |   16 +++-
 src/install.c  |1 +
 src/mv.c   |1 +
 tests/cp/sparse|4 
 8 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/NEWS b/NEWS
index 80c60e2..94511b7 100644
--- a/NEWS
+++ b/NEWS
@@ -35,8 +35,9 @@ GNU coreutils NEWS-*- 
outline -*-
 
   chroot now accepts the options --userspec and --groups.
 
-  cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
-  when both the source and destination are on the same btrfs partition.
+  cp accepts a new option, --reflink: create a lightweight copy
+  using copy-on-write (COW).  This is currently supported only on
+  btrfs file systems.
 
   sort accepts a new option, --human-numeric-sort (-h): sort numbers
   while honoring human readable suffixes like KiB and MB etc.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index acec76e..03c9eb7 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7543,6 +7543,15 @@ Also, it is not portable to use @option{-R} to copy 
symbolic links
 unless you also specify @option{-P}, as @acronym{POSIX} allows
 implementations that dereference symbolic links by default.
 
+...@item --reflink
+...@opindex --reflink
+Attempt a O(1) copy-on-write (COW) when the underlying file system
+supports this operation instead of really copying the file.
+The source and destination files share the same disk data blocks until
+they are equal.  Changes done in a file are not visible in the other
+one because shared blocks will be duplicated before these changes are
+stored.
+
 @item --remove-destination
 @opindex --remove-destination
 Remove each existing destination file before attempting to open it
diff --git a/src/copy.c b/src/copy.c
index bbed336..5eae9f4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -610,13 +610,16 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }
 
-  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
- If the operation is not supported or it fails then copy the file
- in the usual way.  */
-  bool copied = (x-sparse_mode == SPARSE_AUTO
-  clone_file (dest_desc, source_desc) == 0);
+  if (x-reflink)
+{
+  if (clone_file (dest_desc, source_desc))
+{
+  error (0, errno, _(cannot fstat %s), quote (dst_name));
+  return_val = false;
+}
+  goto close_src_and_dst_desc;
+}
 
-  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
@@ -,6 +2225,7 @@ valid_options (const struct cp_options *co)
   assert (VALID_BACKUP_TYPE (co-backup_type));
   assert (VALID_SPARSE_MODE (co-sparse_mode));
   assert (!(co-hard_link  co-symbolic_link));
+  assert (!(co-reflink  co-sparse_mode != SPARSE_AUTO));
   return true;
 }
 
diff --git a/src/copy.h b/src/copy.h
index 8e0b408..ddf4f4e 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -219,6 +219,9 @@ struct cp_options
  such a symlink) and returns false.  */
   bool open_dangling_dest_symlink;
 
+  /* 

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 I am now convinced that cp's new behavior belongs on
 a separate option, --reflink (i.e., it should not be the default).
 Giuseppe, do you feel like adding that option and adjusting your
 test accordingly?

 I attached two separate patches, --reflink option and file-clone test.
 Last versions of btrfs have a bug (I asked on #btrfs and they confirmed
 it), btrfs doesn't use correctly all the free space available.  In fact
 I get ENOSPC while in reality only 54% is used.  Probably it is better
 to postpone the second patch inclusion after the bug is fixed.

 Another note, I changed this line in the NEWS file:
 -  when both the source and destination are on the same btrfs partition.

 considering that BTRFS supports multiple devices I am not convinced that
 it is always true, I guess source and destination could be on different
 partitions, though I couldn't find a clear answer on the btrfs wiki to
 this question.

Thanks for working on this.

I would be surprised if a cross-file-system COW is possible.
How about this for NEWS:

  cp accepts a new option, --reflink: create a lightweight copy
  using copy-on-write (COW).  This is currently supported only on
  btrfs file systems.

Typically, with a feature like this, if a tool is unable to provide
the functionality implied by the new option, it gives a diagnostic
and exits nonzero.  Otherwise, it would be far more work for an
application to determine whether the option was honored.

Deciding whether we also want an option saying
copy-via-COW-if-possible-and-ignore-any-failure can wait,
but I'm leaning away from it for now.


With your change, the new reflink member may be used uninitialized
via install and mv.  To avoid that, just initialize it to false in
each cp_option_init.

Also, please make cp give a diagnostic when --reflink is used with
--sparse=never or --sparse=always.  Then, add this assertion in copy.c:

assert ( ! (x-reflink  x-sparse_mode != SPARSE_AUTO));

From d110badaf7583acf957477bc7eda2e212b404343 Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Sat, 1 Aug 2009 19:36:48 +0200
 Subject: [PATCH 1/2] cp: accept the --reflink option

 * NEWS: Mention it.
 * doc/coreutils.texi: Likewise.
 * src/copy.h (struct cp_options): New member reflink.
 * src/copy.c (usage): Likewise.
 (copy_reg): If reflink is true try to clone the file.
 (main): Check for --reflink.
 (cp_option_init): By default set reflink to false.
 ---
  NEWS   |4 ++--
  doc/coreutils.texi |9 +
  src/copy.c |5 +++--
  src/copy.h |3 +++
  src/cp.c   |   10 +-


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Pádraig Brady wrote:
 Giuseppe Scrivano wrote:
 Pádraig Brady p...@draigbrady.com writes:

 +#expensive_
 That comment is just for testing I presume?
 Note you can run a single expensive test like:
 (cd tests  make check TESTS=cp/file-clone VERBOSE=yes 
 RUN_EXPENSIVE_TESTS=yes)

 sorry, yes I commented it out only for testing purpose.  If you think
 the rest is fine and want to push it, can you please amend my commit?

 Sure I'll fix it up from here.
 I'll not push yet though, until the interface settles down.
 I.E. we'll probably need to be calling {cp,ln} --reflink instead.

I am now convinced that cp's new behavior belongs on
a separate option, --reflink (i.e., it should not be the default).
Giuseppe, do you feel like adding that option and adjusting your
test accordingly?

Things to adjust, other than copy.c and the test:
  NEWS
  cp --help
  doc/coreutils.texi

For now, let's continue to use the ioctl,
but eventually we'll use the reflinkat syscall.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Jim Meyering j...@meyering.net writes:

 I am now convinced that cp's new behavior belongs on
 a separate option, --reflink (i.e., it should not be the default).
 Giuseppe, do you feel like adding that option and adjusting your
 test accordingly?

sure.


 Things to adjust, other than copy.c and the test:
   NEWS
   cp --help
   doc/coreutils.texi

 For now, let's continue to use the ioctl,
 but eventually we'll use the reflinkat syscall.

Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Jim Meyering j...@meyering.net writes:

 I am now convinced that cp's new behavior belongs on
 a separate option, --reflink (i.e., it should not be the default).
 Giuseppe, do you feel like adding that option and adjusting your
 test accordingly?

I attached two separate patches, --reflink option and file-clone test.
Last versions of btrfs have a bug (I asked on #btrfs and they confirmed
it), btrfs doesn't use correctly all the free space available.  In fact
I get ENOSPC while in reality only 54% is used.  Probably it is better
to postpone the second patch inclusion after the bug is fixed.

Another note, I changed this line in the NEWS file:
-  when both the source and destination are on the same btrfs partition.

considering that BTRFS supports multiple devices I am not convinced that
it is always true, I guess source and destination could be on different
partitions, though I couldn't find a clear answer on the btrfs wiki to
this question.


Any comment?

Thanks,
Giuseppe



From d110badaf7583acf957477bc7eda2e212b404343 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 1 Aug 2009 19:36:48 +0200
Subject: [PATCH 1/2] cp: accept the --reflink option

* NEWS: Mention it.
* doc/coreutils.texi: Likewise.
* src/copy.h (struct cp_options): New member reflink.
* src/copy.c (usage): Likewise.
(copy_reg): If reflink is true try to clone the file.
(main): Check for --reflink.
(cp_option_init): By default set reflink to false.
---
 NEWS   |4 ++--
 doc/coreutils.texi |9 +
 src/copy.c |5 +++--
 src/copy.h |3 +++
 src/cp.c   |   10 +-
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 80c60e2..8d6d7a6 100644
--- a/NEWS
+++ b/NEWS
@@ -35,8 +35,8 @@ GNU coreutils NEWS-*- 
outline -*-
 
   chroot now accepts the options --userspec and --groups.
 
-  cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
-  when both the source and destination are on the same btrfs partition.
+  cp accepts a new option, --reflink: attempt a copy-on-write (COW)
+  when the file system supports it.
 
   sort accepts a new option, --human-numeric-sort (-h): sort numbers
   while honoring human readable suffixes like KiB and MB etc.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index acec76e..03c9eb7 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7543,6 +7543,15 @@ Also, it is not portable to use @option{-R} to copy 
symbolic links
 unless you also specify @option{-P}, as @acronym{POSIX} allows
 implementations that dereference symbolic links by default.
 
+...@item --reflink
+...@opindex --reflink
+Attempt a O(1) copy-on-write (COW) when the underlying file system
+supports this operation instead of really copying the file.
+The source and destination files share the same disk data blocks until
+they are equal.  Changes done in a file are not visible in the other
+one because shared blocks will be duplicated before these changes are
+stored.
+
 @item --remove-destination
 @opindex --remove-destination
 Remove each existing destination file before attempting to open it
diff --git a/src/copy.c b/src/copy.c
index bbed336..02d36f3 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -610,10 +610,11 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }
 
-  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
+  /* Attempt a clone operation.  It is possible only when --sparse=auto
+ is in effect.
  If the operation is not supported or it fails then copy the file
  in the usual way.  */
-  bool copied = (x-sparse_mode == SPARSE_AUTO
+  bool copied = (x-reflink  x-sparse_mode == SPARSE_AUTO
   clone_file (dest_desc, source_desc) == 0);
 
   if (!copied)
diff --git a/src/copy.h b/src/copy.h
index 8e0b408..ddf4f4e 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -219,6 +219,9 @@ struct cp_options
  such a symlink) and returns false.  */
   bool open_dangling_dest_symlink;
 
+  /* If true, attempt to clone the file instead of copying it.  */
+  bool reflink;
+
   /* This is a set of destination name/inode/dev triples.  Each such triple
  represents a file we have created corresponding to a source file name
  that was specified on the command line.  Use it to avoid clobbering
diff --git a/src/cp.c b/src/cp.c
index 8785076..63c07d4 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -78,7 +78,8 @@ enum
   PRESERVE_ATTRIBUTES_OPTION,
   SPARSE_OPTION,
   STRIP_TRAILING_SLASHES_OPTION,
-  UNLINK_DEST_BEFORE_OPENING
+  UNLINK_DEST_BEFORE_OPENING,
+  REFLINK_OPTION
 };
 
 /* True if the kernel is SELinux enabled.  */
@@ -121,6 +122,7 @@ static struct option const long_opts[] =
   {recursive, no_argument, NULL, 'R'},
   {remove-destination, no_argument, NULL, UNLINK_DEST_BEFORE_OPENING},
   {sparse, required_argument, NULL, SPARSE_OPTION},
+  {reflink, no_argument, NULL, REFLINK_OPTION},
   

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Pádraig Brady p...@draigbrady.com writes:

 +#expensive_

 That comment is just for testing I presume?
 Note you can run a single expensive test like:
 (cd tests  make check TESTS=cp/file-clone VERBOSE=yes 
 RUN_EXPENSIVE_TESTS=yes)

sorry, yes I commented it out only for testing purpose.  If you think
the rest is fine and want to push it, can you please amend my commit?

Thanks,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Giuseppe Scrivano wrote:
 Pádraig Brady p...@draigbrady.com writes:
 
 +#expensive_
 That comment is just for testing I presume?
 Note you can run a single expensive test like:
 (cd tests  make check TESTS=cp/file-clone VERBOSE=yes 
 RUN_EXPENSIVE_TESTS=yes)
 
 sorry, yes I commented it out only for testing purpose.  If you think
 the rest is fine and want to push it, can you please amend my commit?

Sure I'll fix it up from here.
I'll not push yet though, until the interface settles down.
I.E. we'll probably need to be calling {cp,ln} --reflink instead.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Joel Becker wrote:

 On Wed, Jul 29, 2009 at 07:14:37PM +0100, Pádraig Brady wrote:
 Chris Mason wrote:
  On Wed, Jul 29, 2009 at 03:14:49PM +0100, Pádraig Brady wrote:
 
  We may need to play around with fallocate()
  if we want to get back to the original
  cp semantics of actually allocating space
  on the file system for the new file.
 
  Well, best to just use the original cp code.  I was talking with
  Giuseppe about this as well, I think we should the option to do regular
  cp via a flag.

 Right. Well we can turn off this cloning by doing --sparse={never,always}
 but that has side effects. If we need an option then maybe we should have
 it turn on cloning rather than restore default cp behaviour?
 The side effects I thought of earlier, of COW without corresponding 
 allocation
 were possible fragmentation on write or unexpected/mishandled ENOSPC.
 Also for endangered mechanical disks, subsequent processing could
 be slowed as the head seeks between the old and new data to be copied.
 Perhaps these are a small price to pay, especially considering that
 solid state disks will only be affected by the write()=ENOSPC issue.

 At the moment we have these linking options:

 cp -l, --link #for hardlinks
 cp -s, --symbolic-link #for symlinks

 So perhaps we should support:

 cp --link={soft,hard,cow}
 for symlink(), link() and reflink() respectively?
 I.E. link to the name, inode or extents respectively.

   I've cooked up 'ln -r' for reflinks, which works for ln(1) but
 not for cp(1).

Thanks.  I haven't looked, but after reading about the reflink syscall
[http://lwn.net/Articles/332802/] had come to the same conclusion:
this feature belongs with ln rather than with cp.

Besides, putting the new behavior on a new option avoids
the current semantic change we would otherwise induce in cp.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Joel Becker]

On Thu, Jul 30, 2009 at 09:39:17AM +0200, Jim Meyering wrote:
 Joel Becker wrote:
  I've cooked up 'ln -r' for reflinks, which works for ln(1) but
  not for cp(1).
 
 Thanks.  I haven't looked, but after reading about the reflink syscall
 [http://lwn.net/Articles/332802/] had come to the same conclusion:
 this feature belongs with ln rather than with cp.
 
 Besides, putting the new behavior on a new option avoids
 the current semantic change we would otherwise induce in cp.

Well, I don't see any reason cp(1) can't take advantage of
reflink(2).  I just think that cp(1) should look at reflink(2) as an
optimization, not a specific methodology.
What do I mean?  If you want to say I know what a reflink is,
and that's exactly what I want, you want ln -r.  But say you want a
cp --snap that tries to take a snapshot regardless of the backend.  It
could use reflink(2) on filesystems that support it, or perhaps a
passthrough call to the underlying storage, or who knows what.  I can
also imagine a cp --shallow that is if you can cow, do it, otherwise
do a normal cp.

Joel

-- 

I think it would be a good idea.  
- Mahatma Ghandi, when asked what he thought of Western
  civilization

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.bec...@oracle.com
Phone: (650) 506-8127


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:

 Hi,

 I cleaned a bit the Pádraig's example in a new test case.

 The second patch fixes a problem that I introduced with the commit
 e81c4d88c2fce526c02693d539e22c7468dc452b.

Thanks for the test.
We should be able to use it regardless of whether
the feature is added to cp or ln.  However, please
arrange to clean up (unmount) not just on normal
completion, but also when e.g., interrupted.

Note how that is done in the other tests
that mount/unmount file systems.  Here's one example:

  cp/cp-a-selinux:cleanup_() { cd /; umount $cwd/mnt; }


From 0348010828d201c0790c06e1427cc4b813c605db Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Wed, 29 Jul 2009 20:57:29 +0200
 Subject: [PATCH 2/2] tail: exit successfully on watched process death

 * src/tail.c (tail_forever_inotify): If a PID is specified and the
 watched process is death, exit with an EXIT_SUCCESS code.
 ---
  src/tail.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/src/tail.c b/src/tail.c
 index a73ffa2..5efaf57 100644
 --- a/src/tail.c
 +++ b/src/tail.c
 @@ -1280,7 +1280,7 @@ tail_forever_inotify (int wd, struct File_spec *f, 
 size_t n_files,
  {
/* See if the process we are monitoring is still alive.  */
if (kill (pid, 0) != 0  errno != EPERM)
 -break;
 +exit (EXIT_SUCCESS);

continue;
  }

Thanks!
I've added a test for this, too:

From 2ac2cace153dd19aaff5d3e0e6ce68d03146ca92 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Thu, 30 Jul 2009 10:27:51 +0200
Subject: [PATCH] tests: test for just-fixed tail --pid bug

* tests/tail-2/pid: Ensure tail exits successfully when PID dies.
---
 tests/tail-2/pid |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tests/tail-2/pid b/tests/tail-2/pid
index 9d33e4f..446c6db 100755
--- a/tests/tail-2/pid
+++ b/tests/tail-2/pid
@@ -65,4 +65,9 @@ if test -n $state; then
   kill $pid
 fi

+# Ensure that tail --pid=PID exits successfully when PID is dead.
+# Use an unlikely-to-be-live PID: 2^31-1
+getlimits_
+tail --pid=$INT_MAX -f /dev/null || fail=1
+
 Exit $fail
--
1.6.4.rc3.201.gd9d59


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Jim Meyering wrote:
 Joel Becker wrote:
 
 On Wed, Jul 29, 2009 at 07:14:37PM +0100, Pádraig Brady wrote:

 At the moment we have these linking options:

 cp -l, --link #for hardlinks
 cp -s, --symbolic-link #for symlinks

 So perhaps we should support:

 cp --link={soft,hard,cow}
 for symlink(), link() and reflink() respectively?
 I.E. link to the name, inode or extents respectively.

  I've cooked up 'ln -r' for reflinks, which works for ln(1) but
 not for cp(1).
 
 Thanks.  I haven't looked, but after reading about the reflink syscall
 [http://lwn.net/Articles/332802/] had come to the same conclusion:
 this feature belongs with ln rather than with cp.

Right. It definitely should be in ln anyway.

 Besides, putting the new behavior on a new option avoids
 the current semantic change we would otherwise induce in cp.

Yes doing reflink() in cp by default currently can
be problematic as discussed, especially on mechanical hard disks.
Though in future I can see most users of cp preferring
reflink() to be done, rather than read()/write(). Ponder...

In any case putting --link=cow or --reflink or whatever in cp
could be very useful for creating writeable snapshot branches.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Andi Kleen wrote:
 Jim Meyering j...@meyering.net writes:
 Thanks.  I haven't looked, but after reading about the reflink syscall
 [http://lwn.net/Articles/332802/] had come to the same conclusion:
 this feature belongs with ln rather than with cp.
 
 cp already has -l so it would make sense to extend that too.
 
 Besides, putting the new behavior on a new option avoids
 the current semantic change we would otherwise induce in cp.
 
 I don't see how semantics change in a user visible way.

I was thinking that doing reflink() in cp has the following
user visible advantages/disadvantages:

Advantages:
  very quick copy
  less space used

Disadvantages:
  disk head seeking deferred to modification process
  possible fragmentation on write
  possible ENOSPC on write

The disk head seeking issue will go away with time.
I'm not sure if the other disadvantages exist or whether
they could be alleviated with fallocate() or something.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Andi Kleen wrote:

 Jim Meyering j...@meyering.net writes:

 Thanks.  I haven't looked, but after reading about the reflink syscall
 [http://lwn.net/Articles/332802/] had come to the same conclusion:
 this feature belongs with ln rather than with cp.

 cp already has -l so it would make sense to extend that too.

Good point.

 Besides, putting the new behavior on a new option avoids
 the current semantic change we would otherwise induce in cp.

 I don't see how semantics change in a user visible way.

With classic cp, if I copy a 1GB non-sparse file and there's less
space than that available, cp fails with ENOSPC.
With this new feature, it succeeds even if there are
just a few blocks available.

Also, consider (buggy!) code that then depends on being able to modify
that file in-place, and that knows it doesn't need to check for ENOSPC.
Sure, they should always check for write failure, but still.  It is
a change.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Jim Meyering wrote:
 diff --git a/tests/tail-2/pid b/tests/tail-2/pid
 
 +# Ensure that tail --pid=PID exits successfully when PID is dead.
 +# Use an unlikely-to-be-live PID: 2^31-1
 +getlimits_
 +tail --pid=$INT_MAX -f /dev/null || fail=1

Probably should use $PID_T_MAX
You could speed up the test with -s.1 also
(after separate fix I'm sending applied).
Also could protect with timeout incase $PID_T_MAX does exist.
So something like:

getlimits_
timeout 1 tail -s.1 --pid=$PID_T_MAX -f /dev/null
ret=$?
test $ret = 124  skip_test_ pid $PID_T_MAX present
! test $ret = 0  fail=1

cheers,
Pádraig.

p.s. A lot of these tests could be changed to
using timeout I think. I'll have a look at that
at some stage.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Pádraig Brady wrote:
 Jim Meyering wrote:
 diff --git a/tests/tail-2/pid b/tests/tail-2/pid

 +# Ensure that tail --pid=PID exits successfully when PID is dead.
 +# Use an unlikely-to-be-live PID: 2^31-1
 +getlimits_
 +tail --pid=$INT_MAX -f /dev/null || fail=1

 Probably should use $PID_T_MAX

Definitely.

 You could speed up the test with -s.1 also

Yes, indeed.  Good one.

 (after separate fix I'm sending applied).
 Also could protect with timeout incase $PID_T_MAX does exist.
 So something like:

 getlimits_
 timeout 1 tail -s.1 --pid=$PID_T_MAX -f /dev/null
 ret=$?
 test $ret = 124  skip_test_ pid $PID_T_MAX present
 ! test $ret = 0  fail=1

Clearly better.
Thanks!

 cheers,
 Pádraig.

 p.s. A lot of these tests could be changed to
 using timeout I think. I'll have a look at that
 at some stage.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Andi Kleen]

Jim Meyering j...@meyering.net writes:

 Thanks.  I haven't looked, but after reading about the reflink syscall
 [http://lwn.net/Articles/332802/] had come to the same conclusion:
 this feature belongs with ln rather than with cp.

cp already has -l so it would make sense to extend that too.

 Besides, putting the new behavior on a new option avoids
 the current semantic change we would otherwise induce in cp.

I don't see how semantics change in a user visible way.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Tomasz Chmielewski]

Jim Meyering wrote:


With classic cp, if I copy a 1GB non-sparse file and there's less
space than that available, cp fails with ENOSPC.
With this new feature, it succeeds even if there are
just a few blocks available.


Is it good or bad?



Also, consider (buggy!) code that then depends on being able to modify
that file in-place, and that knows it doesn't need to check for ENOSPC.
Sure, they should always check for write failure, but still.  It is
a change.


On a multiuser system, that (buggy) tool would fail anyway if something 
else adds enough new data to the filesystem in the meantime.


But sure, it's a change.


--
Tomasz Chmielewski
http://wpkg.org


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Andi Kleen]

 
 With classic cp, if I copy a 1GB non-sparse file and there's less
 space than that available, cp fails with ENOSPC.
 With this new feature, it succeeds even if there are
 just a few blocks available.
 
 Also, consider (buggy!) code that then depends on being able to modify
 that file in-place, and that knows it doesn't need to check for ENOSPC.
 Sure, they should always check for write failure, but still.  It is
 a change.

Fair point, although I suspect there are cases where ENOSPC
on non extending write can already happen on specific file systems. e.g. on 
btrfs it might happen when the tree gets rebalanced? Or perhaps on nilfs2
when the garbage collector doesn't run in time. Wouldn't surprise 
me if there weren't more cases already.

-Andi

-- 
a...@linux.intel.com -- Speaking for myself only.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Ric Wheeler]

On 07/30/2009 04:40 AM, Pádraig Brady wrote:

Jim Meyering wrote:
   

Joel Becker wrote:

 

On Wed, Jul 29, 2009 at 07:14:37PM +0100, Pádraig Brady wrote:
   

At the moment we have these linking options:

cp -l, --link #for hardlinks
cp -s, --symbolic-link #for symlinks

So perhaps we should support:

cp --link={soft,hard,cow}
for symlink(), link() and reflink() respectively?
I.E. link to the name, inode or extents respectively.
 

I've cooked up 'ln -r' for reflinks, which works for ln(1) but
not for cp(1).
   

Thanks.  I haven't looked, but after reading about the reflink syscall
[http://lwn.net/Articles/332802/] had come to the same conclusion:
this feature belongs with ln rather than with cp.
 


Right. It definitely should be in ln anyway.

   

Besides, putting the new behavior on a new option avoids
the current semantic change we would otherwise induce in cp.
 


Yes doing reflink() in cp by default currently can
be problematic as discussed, especially on mechanical hard disks.
Though in future I can see most users of cp preferring
reflink() to be done, rather than read()/write(). Ponder...

   


I think that doing reflink by default would be a horrible idea - one 
good reason to copy a file is to increase your level of fault tolerance 
and reflink magically avoids that :-)


reflink is a neat feature, but should be used on purpose in my opinion,

ric


In any case putting --link=cow or --reflink or whatever in cp
could be very useful for creating writeable snapshot branches.

cheers,
Pádraig.
   




___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Ric Wheeler wrote:
 I think that doing reflink by default would be a horrible idea - one
 good reason to copy a file is to increase your level of fault
 tolerance and reflink magically avoids that :-)

Good point.
This would constitute another user-visible semantic change in cp:
a disk fault that affects any non-metadata block of a ref-linked file
affects both copies.

GNU cp will soon attempt this only when a --reflink option is specified.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Hi Pádraig,

thanks for the comments.

Pádraig Brady p...@draigbrady.com writes:

 # 300MB seems to be the minimum size for a btrfs with default
 parameters.

Actually, it seems the minimum space required is 256MB.  Using a 255MB
image I get: device btrfs.img is too small (must be at least 256 MB)


 # FIXME: use `truncate --allocate` when it becomes available, which
 # may allow unmarking this as an expensive test.

Are you sure that this feature will make the test less expensive?  Still
the test files must be written there, so in the best case (considering
the fallocate done in 0s) only the dd cost will be saved but still it
looks like an expensive test.

In the version I attached, I am using a sparse file (truncate --size)
and it seems to work fine.  Is it correct or am I missing something?

I haven't looked yet but probably there are other tests that can take
advantage of sparse files instead of using dd.

I am also considering the Jim's note doing the umount in the cleanup_
function.

Cheers,
Giuseppe


From 7add4b337b7db0a63bca0dd0fe0f146f175163f8 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Wed, 29 Jul 2009 20:31:20 +0200
Subject: [PATCH] tests: add a test for btrfs' copy-on-write file clone operation

* tests/Makefile.am: Consider the new test.
* tests/cp/file-clone: New file.
---
 tests/Makefile.am   |1 +
 tests/cp/file-clone |   58 +++
 2 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100755 tests/cp/file-clone

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 59737a0..9841aa3 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,6 +20,7 @@ EXTRA_DIST =  \
 
 root_tests =   \
   chown/basic  \
+  cp/file-clone\
   cp/cp-a-selinux  \
   cp/preserve-gid  \
   cp/special-bits  \
diff --git a/tests/cp/file-clone b/tests/cp/file-clone
new file mode 100755
index 000..c65b9cb
--- /dev/null
+++ b/tests/cp/file-clone
@@ -0,0 +1,58 @@
+#!/bin/sh
+# Make sure file-clone on a btrfs file system works properly.
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+
+
+if test $VERBOSE = yes; then
+  set -x
+  cp --version
+fi
+
+. $srcdir/test-lib.sh
+
+require_root_
+require_sparse_support_
+#expensive_
+
+cleanup_(){ umount btrfs; }
+
+fail=0
+
+mkfs.btrfs --version || skip_test_ btrfs userland tools not installed
+
+# 256MB seems to be the minimum size for a btrfs with default parameters.
+truncate --size=256M btrfs.img  || framework_failure
+
+mkfs.btrfs btrfs.img  || framework_failure
+
+mkdir btrfs || framework_failure
+
+mount -t btrfs -o loop btrfs.img btrfs || framework_failure
+
+dd bs=1M count=200 if=/dev/zero of=btrfs/alloc.test || framework_failure
+
+# If the file is cloned, only additional space for metadata is required.
+# Two 200MB files can be present even if the total file system space is 256MB.
+cp btrfs/alloc.test btrfs/clone.test || fail=1
+rm btrfs/clone.test
+
+# When --sparse={always,never} is used, the file is copied without any cloning.
+# Use --sparse=never to be sure the file is copied without holes and it is not
+# possible since there is not enough free space.
+cp --sparse=never btrfs/alloc.test btrfs/clone.test  fail=1
+
+Exit $fail
-- 
1.6.3.3


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Joel Becker]

On Thu, Jul 30, 2009 at 12:54:16PM +0200, Andi Kleen wrote:
  With classic cp, if I copy a 1GB non-sparse file and there's less
  space than that available, cp fails with ENOSPC.
  With this new feature, it succeeds even if there are
  just a few blocks available.
  
  Also, consider (buggy!) code that then depends on being able to modify
  that file in-place, and that knows it doesn't need to check for ENOSPC.
  Sure, they should always check for write failure, but still.  It is
  a change.
 
 Fair point, although I suspect there are cases where ENOSPC
 on non extending write can already happen on specific file systems. e.g. on 
 btrfs it might happen when the tree gets rebalanced? Or perhaps on nilfs2
 when the garbage collector doesn't run in time. Wouldn't surprise 
 me if there weren't more cases already.

In some sense, using btrfs, nilfs2i, ocfs2 with refcount trees
enabled, or any other CoW-ish filesystem is a tacit approval of the
delayed ENOSPC.  The same can be said of thin provisioning LUNs.
However, the other concerns are still valid.  A user invoking vanilla
cp(1) expects two independent storage regions for the data.
(Oh, and what about future support of de-duping in filesystems?
:-)

Joel

-- 

Anything that is too stupid to be spoken is sung.  
- Voltaire

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.bec...@oracle.com
Phone: (650) 506-8127


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Joel Becker wrote:
   In some sense, using btrfs, nilfs2i, ocfs2 with refcount trees
 enabled, or any other CoW-ish filesystem is a tacit approval of the
 delayed ENOSPC.  The same can be said of thin provisioning LUNs.
 However, the other concerns are still valid.  A user invoking vanilla
 cp(1) expects two independent storage regions for the data.
   (Oh, and what about future support of de-duping in filesystems?
 :-)

I maintain an app to de-dupe at http://www.pixelbeat.org/fslint/
and I'll be adding reflink support as soon as it becomes available.
From a filesystem point of view, one thing that would help speed
this up (and many other things like rsync etc.) would be to allow
one to associate say a sha-3 hash or whatever with the file, which
the filesystem would automatically clear when the file data changes.
So in general having a special set of extended attributes that
were auto cleared on file modification would be very useful for
lots of stuff.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Giuseppe Scrivano wrote:
 Hi Pádraig,
 
 thanks for the comments.
 
 Pádraig Brady p...@draigbrady.com writes:
 
 # 300MB seems to be the minimum size for a btrfs with default
 parameters.
 
 Actually, it seems the minimum space required is 256MB.  Using a 255MB
 image I get: device btrfs.img is too small (must be at least 256 MB)

I tried that with my version of btrfs but still got the too small error.
I probably did something silly.

 # FIXME: use `truncate --allocate` when it becomes available, which
 # may allow unmarking this as an expensive test.
 
 Are you sure that this feature will make the test less expensive?  Still
 the test files must be written there, so in the best case (considering
 the fallocate done in 0s) only the dd cost will be saved but still it
 looks like an expensive test.

Well with fallocate() I expect only the file system meta data
to be modified, thus obviating moving any zeros around.
But you're right that the mkfs will probably have to write lots of data.

 In the version I attached, I am using a sparse file (truncate --size)
 and it seems to work fine.  Is it correct or am I missing something?

Nope, good idea! I guess the mkfs writes all the blocks anyway
as mentioned above, but this is now done only once.

 I haven't looked yet but probably there are other tests that can take
 advantage of sparse files instead of using dd.
 
 I am also considering the Jim's note doing the umount in the cleanup_

I noticed that also but got distracted :p

 diff --git a/tests/cp/file-clone b/tests/cp/file-clone
 +
 +require_root_
 +require_sparse_support_
 +#expensive_

That comment is just for testing I presume?
Note you can run a single expensive test like:
(cd tests  make check TESTS=cp/file-clone VERBOSE=yes RUN_EXPENSIVE_TESTS=yes)

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Jim Meyering wrote:

 Andreas Schwab wrote:

 Jim Meyering j...@meyering.net writes:

 +#ifdef __linux__
 +# define BTRFS_IOC_CLONE 1074041865

 This is wrong, the actual value is architecture dependent.  You should
 use the _IOW macro instead.

 Good point.  Thanks!
 I've adjusted it.
 Here's the patch I'm considering now:

From 6ae1f09da25f6600f32dc540551a9479dd2e618e Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Sat, 25 Jul 2009 16:35:27 +0200
 Subject: [PATCH] cp: support btrfs' copy-on-write file clone operation

Giuseppe is going to write a root-only test to exercise this code,
so I've made one final adjustment to his patch (move decl of copied
down to definition) and have pushed this change:

From 45330176690b079ed47ac7c58f29a1b028f97b07 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 25 Jul 2009 16:35:27 +0200
Subject: [PATCH] cp: support btrfs' copy-on-write file clone operation

* src/copy.c [HAVE_SYS_IOCTL_H]: Include sys/ioctl.h.
(BTRFS_IOCTL_MAGIC, BTRFS_IOC_CLONE): Define.
(clone_file): New function.
(copy_reg): Use the btrfs clone operation if possible.
---
 src/copy.c |   28 
 1 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 4c8c432..bbed336 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -61,6 +61,10 @@
 # include verror.h
 #endif

+#if HAVE_SYS_IOCTL_H
+# include sys/ioctl.h
+#endif
+
 #ifndef HAVE_FCHOWN
 # define HAVE_FCHOWN false
 # define fchown(fd, uid, gid) (-1)
@@ -114,6 +118,23 @@ static bool owner_failure_ok (struct cp_options const *x);
 static char const *top_level_src_name;
 static char const *top_level_dst_name;

+/* Perform the O(1) btrfs clone operation, if possible.
+   Upon success, return 0.  Otherwise, return -1 and set errno.  */
+static inline int
+clone_file (int dest_fd, int src_fd)
+{
+#ifdef __linux__
+# undef BTRFS_IOCTL_MAGIC
+# define BTRFS_IOCTL_MAGIC 0x94
+# undef BTRFS_IOC_CLONE
+# define BTRFS_IOC_CLONE _IOW (BTRFS_IOCTL_MAGIC, 9, int)
+  return ioctl (dest_fd, BTRFS_IOC_CLONE, src_fd);
+#else
+  errno = ENOTSUP;
+  return -1;
+#endif
+}
+
 /* FIXME: describe */
 /* FIXME: rewrite this to use a hash table so we avoid the quadratic
performance hit that's probably noticeable only on trees deeper
@@ -589,6 +610,13 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }

+  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
+ If the operation is not supported or it fails then copy the file
+ in the usual way.  */
+  bool copied = (x-sparse_mode == SPARSE_AUTO
+  clone_file (dest_desc, source_desc) == 0);
+
+  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
--
1.6.4.rc3.201.gd9d59


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Jim Meyering wrote:

 Jim Meyering wrote:

 Andreas Schwab wrote:

 Jim Meyering j...@meyering.net writes:

 +#ifdef __linux__
 +# define BTRFS_IOC_CLONE 1074041865

 This is wrong, the actual value is architecture dependent.  You should
 use the _IOW macro instead.

 Good point.  Thanks!
 I've adjusted it.
 Here's the patch I'm considering now:

From 6ae1f09da25f6600f32dc540551a9479dd2e618e Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Sat, 25 Jul 2009 16:35:27 +0200
 Subject: [PATCH] cp: support btrfs' copy-on-write file clone operation

 Giuseppe is going to write a root-only test to exercise this code,
 so I've made one final adjustment to his patch (move decl of copied
 down to definition) and have pushed this change:

Now that I've pushed that, I realize it deserves a NEWS entry:

From 722d75b6091dd55b9254e7a49727ebe2f97d00a7 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Wed, 29 Jul 2009 12:25:33 +0200
Subject: [PATCH] maint: update NEWS

* NEWS (New features): Mention it.
---
 NEWS |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/NEWS b/NEWS
index e476f2b..80c60e2 100644
--- a/NEWS
+++ b/NEWS
@@ -35,6 +35,9 @@ GNU coreutils NEWS-*- 
outline -*-

   chroot now accepts the options --userspec and --groups.

+  cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
+  when both the source and destination are on the same btrfs partition.
+
   sort accepts a new option, --human-numeric-sort (-h): sort numbers
   while honoring human readable suffixes like KiB and MB etc.

--
1.6.4.rc3.201.gd9d59


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Chris Mason wrote:
 On Tue, Jul 28, 2009 at 10:06:35PM +0200, Giuseppe Scrivano wrote:

 I can't replicate it now, all tests I am doing report that blocks used
 before and after the clone are the same.  Probably yesterday the
 difference I noticed was in reality the original file flushed to the
 disk.
 
 The clone will use some additional space for the metadata required to
 point to the cloned blocks.  It isn't exactly O(1) it is O(metadata for
 the file).

Thanks for the clarification Chris.
So the just committed change in cp will
link the destination file to the extents of the source.

We may need to play around with fallocate()
if we want to get back to the original
cp semantics of actually allocating space
on the file system for the new file.

I'll test this when I get an up to date btrfs
and when the fallocate interface in glibc settles down.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Chris Mason wrote:
 On Wed, Jul 29, 2009 at 03:14:49PM +0100, Pádraig Brady wrote:

 We may need to play around with fallocate()
 if we want to get back to the original
 cp semantics of actually allocating space
 on the file system for the new file.
 
 Well, best to just use the original cp code.  I was talking with
 Giuseppe about this as well, I think we should the option to do regular
 cp via a flag.

Right. Well we can turn off this cloning by doing --sparse={never,always}
but that has side effects. If we need an option then maybe we should have
it turn on cloning rather than restore default cp behaviour?
The side effects I thought of earlier, of COW without corresponding allocation
were possible fragmentation on write or unexpected/mishandled ENOSPC.
Also for endangered mechanical disks, subsequent processing could
be slowed as the head seeks between the old and new data to be copied.
Perhaps these are a small price to pay, especially considering that
solid state disks will only be affected by the write()=ENOSPC issue.

At the moment we have these linking options:

cp -l, --link #for hardlinks
cp -s, --symbolic-link #for symlinks

So perhaps we should support:

cp --link={soft,hard,cow}
for symlink(), link() and reflink() respectively?
I.E. link to the name, inode or extents respectively.

 There will soon be a reflink system call that can be used on ocfs2 and
 btrfs as well.  Thanks for adding this to glibc!

I was thinking there would be a generic syscall for this.
So cp should call reflink() instead when it becomes available.

thanks for the info!
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Chris Mason]

On Tue, Jul 28, 2009 at 10:06:35PM +0200, Giuseppe Scrivano wrote:
 Hi Pádraig,
 
 
 Pádraig Brady p...@draigbrady.com writes:
 
  How different exactly?
  OK I tried this myself on F11 with inconclusive results.
 
 I can't replicate it now, all tests I am doing report that blocks used
 before and after the clone are the same.  Probably yesterday the
 difference I noticed was in reality the original file flushed to the
 disk.

The clone will use some additional space for the metadata required to
point to the cloned blocks.  It isn't exactly O(1) it is O(metadata for
the file).

 
 
  The above suggests that the clone does actually allocate space
  but btrfs isn't reporting it through statvfs correctly?
 
 The same message appeared here too some days ago, though I cloned only
 few Kb files, not much to fill the entire partition.
 
 
  If the clone does allocate space, then how can one
  clone without allocation which could be very useful
  for snapshotting for example?
 
 I don't know if snapshotting is handled in the same way as a clone,
 but in this case it seems more obvious to me that no additional space
 should be reported.

The COW for snapshotting and a clone are the same, but the way we get
there is a little different.  For a snapshot, we have two btree roots
pointing to the same nodes, and we've incremented the reference count on
each of the nodes they both point to.  No matter how big the subvolume
is, this will always be O(number of pointers in the root block).

Cloning a file is done by walking the file metadata and taking a
reference on each extent pointed to by the file.  The file data is never
read in, but all of the file metadata is read in.

 
 
  Also I tried the above twice and both times got:
  http://www.kerneloops.org/submitresult.php?number=578993
 
 I didn't get these errors.  I am using the btrfs git version.

These have been fixed.

-chris



___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Chris Mason]

On Wed, Jul 29, 2009 at 03:14:49PM +0100, Pádraig Brady wrote:
 Chris Mason wrote:
  On Tue, Jul 28, 2009 at 10:06:35PM +0200, Giuseppe Scrivano wrote:
 
  I can't replicate it now, all tests I am doing report that blocks used
  before and after the clone are the same.  Probably yesterday the
  difference I noticed was in reality the original file flushed to the
  disk.
  
  The clone will use some additional space for the metadata required to
  point to the cloned blocks.  It isn't exactly O(1) it is O(metadata for
  the file).
 
 Thanks for the clarification Chris.
 So the just committed change in cp will
 link the destination file to the extents of the source.
 
 We may need to play around with fallocate()
 if we want to get back to the original
 cp semantics of actually allocating space
 on the file system for the new file.

Well, best to just use the original cp code.  I was talking with
Giuseppe about this as well, I think we should the option to do regular
cp via a flag.

There will soon be a reflink system call that can be used on ocfs2 and
btrfs as well.  Thanks for adding this to glibc!

-chris


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Giuseppe Scrivano wrote:
 Hi,
 
 I cleaned a bit the Pádraig's example in a new test case.

tests, great!
comments below.

 The second patch fixes a problem that I introduced with the commit
 e81c4d88c2fce526c02693d539e22c7468dc452b.

I would have posted that patch in the other tail thread.
On momentary review it seems fine.


 diff --git a/tests/cp/file-clone b/tests/cp/file-clone

 +require_root_
 +require_sparse_support_

We probably want expensive_ for the moment
since we're moving 500M of data around

 +
 +fail=0
 +
 +mkfs.btrfs --version || skip_test_ btrfs userland tools not installed

Here, I'd add the comment:

# 300MB seems to be the minimum size for a btrfs with default parameters.
# FIXME: use `truncate --allocate` when it becomes available, which
# may allow unmarking this as an expensive test.

 +dd bs=1M count=300 if=/dev/zero of=btrfs.img  || framework_failure

Perhaps we could shrink everything by tweaking some
of the parameters to mkfs.btrfs ?

 +
 +mkfs.btrfs btrfs.img  || framework_failure
 +
 +mkdir btrfs || framework_failure
 +
 +mount -t btrfs -o loop btrfs.img btrfs || framework_failure
 +
 +dd bs=1M count=200 if=/dev/zero of=btrfs/alloc.test || (umount btrfs;
 +   framework_failure)

No need for a subshell above. The following is better I think:

dd bs=1M count=200 if=/dev/zero of=btrfs/alloc.test ||
  { umount btrfs; framework_failure; }

 +# If the file is cloned, only additional space for metadata is required.
 +# Two 200Mb files can be present even if the total file system space is 
 300Mb.

I usually use big B for Bytes and little b for bits.

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Joel Becker]

On Wed, Jul 29, 2009 at 07:14:37PM +0100, Pádraig Brady wrote:
 Chris Mason wrote:
  On Wed, Jul 29, 2009 at 03:14:49PM +0100, Pádraig Brady wrote:
 
  We may need to play around with fallocate()
  if we want to get back to the original
  cp semantics of actually allocating space
  on the file system for the new file.
 
  Well, best to just use the original cp code.  I was talking with
  Giuseppe about this as well, I think we should the option to do regular
  cp via a flag.
 
 Right. Well we can turn off this cloning by doing --sparse={never,always}
 but that has side effects. If we need an option then maybe we should have
 it turn on cloning rather than restore default cp behaviour?
 The side effects I thought of earlier, of COW without corresponding allocation
 were possible fragmentation on write or unexpected/mishandled ENOSPC.
 Also for endangered mechanical disks, subsequent processing could
 be slowed as the head seeks between the old and new data to be copied.
 Perhaps these are a small price to pay, especially considering that
 solid state disks will only be affected by the write()=ENOSPC issue.
 
 At the moment we have these linking options:
 
 cp -l, --link #for hardlinks
 cp -s, --symbolic-link #for symlinks
 
 So perhaps we should support:
 
 cp --link={soft,hard,cow}
 for symlink(), link() and reflink() respectively?
 I.E. link to the name, inode or extents respectively.

I've cooked up 'ln -r' for reflinks, which works for ln(1) but
not for cp(1).  I have a git tree with the (in-flux) code on
oss.oracle.com:

[View]
http://oss.oracle.com/git/?p=jlbec/reflink.git;a=summary
[Pull]
git://oss.oracle.com/git/jlbec/reflink.git master

This repository isn't designed to be an authorative patch for
coreutils.  Instead it provides a reflink(1) program that is actually ln
-r in disguise.  Later work would be to get coreutils updated
properly.

Joel

-- 

This is the end, beautiful friend.
 This is the end, my only friend the end
 Of our elaborate plans, the end
 Of everything that stands, the end
 No safety or surprise, the end
 I'll never look into your eyes again.

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.bec...@oracle.com
Phone: (650) 506-8127


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Andreas Schwab wrote:

 Jim Meyering j...@meyering.net writes:

 +#ifdef __linux__
 +# define BTRFS_IOC_CLONE 1074041865

 This is wrong, the actual value is architecture dependent.  You should
 use the _IOW macro instead.

Good point.  Thanks!
I've adjusted it.
Here's the patch I'm considering now:

From 6ae1f09da25f6600f32dc540551a9479dd2e618e Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 25 Jul 2009 16:35:27 +0200
Subject: [PATCH] cp: support btrfs' copy-on-write file clone operation

* src/copy.c [HAVE_SYS_IOCTL_H]: Include sys/ioctl.h.
(BTRFS_IOCTL_MAGIC, BTRFS_IOC_CLONE): Define.
(clone_file): New function.
(copy_reg): Use the btrfs clone operation if possible.
---
 src/copy.c |   30 ++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 4c8c432..ce4c67a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -61,6 +61,10 @@
 # include verror.h
 #endif

+#if HAVE_SYS_IOCTL_H
+# include sys/ioctl.h
+#endif
+
 #ifndef HAVE_FCHOWN
 # define HAVE_FCHOWN false
 # define fchown(fd, uid, gid) (-1)
@@ -114,6 +118,23 @@ static bool owner_failure_ok (struct cp_options const *x);
 static char const *top_level_src_name;
 static char const *top_level_dst_name;

+/* Perform the O(1) btrfs clone operation, if possible.
+   Upon success, return 0.  Otherwise, return -1 and set errno.  */
+static inline int
+clone_file (int dest_fd, int src_fd)
+{
+#ifdef __linux__
+# undef BTRFS_IOCTL_MAGIC
+# define BTRFS_IOCTL_MAGIC 0x94
+# undef BTRFS_IOC_CLONE
+# define BTRFS_IOC_CLONE _IOW (BTRFS_IOCTL_MAGIC, 9, int)
+  return ioctl (dest_fd, BTRFS_IOC_CLONE, src_fd);
+#else
+  errno = ENOTSUP;
+  return -1;
+#endif
+}
+
 /* FIXME: describe */
 /* FIXME: rewrite this to use a hash table so we avoid the quadratic
performance hit that's probably noticeable only on trees deeper
@@ -444,6 +465,7 @@ copy_reg (char const *src_name, char const *dst_name,
   struct stat sb;
   struct stat src_open_sb;
   bool return_val = true;
+  bool copied = false;

   source_desc = open (src_name,
  (O_RDONLY | O_BINARY
@@ -589,6 +611,14 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }

+  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
+ If the operation is not supported or it fails then copy the file
+ in the usual way.  */
+  if (x-sparse_mode == SPARSE_AUTO
+   clone_file (dest_desc, source_desc) == 0)
+copied = true;
+
+  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
--
1.6.2.5


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Hi Pádraig,


Pádraig Brady p...@draigbrady.com writes:

 How different exactly?
 OK I tried this myself on F11 with inconclusive results.

I can't replicate it now, all tests I am doing report that blocks used
before and after the clone are the same.  Probably yesterday the
difference I noticed was in reality the original file flushed to the
disk.


 The above suggests that the clone does actually allocate space
 but btrfs isn't reporting it through statvfs correctly?

The same message appeared here too some days ago, though I cloned only
few Kb files, not much to fill the entire partition.


 If the clone does allocate space, then how can one
 clone without allocation which could be very useful
 for snapshotting for example?

I don't know if snapshotting is handled in the same way as a clone,
but in this case it seems more obvious to me that no additional space
should be reported.


 Also I tried the above twice and both times got:
 http://www.kerneloops.org/submitresult.php?number=578993

I didn't get these errors.  I am using the btrfs git version.


Regards,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Pádraig Brady wrote:

 Giuseppe Scrivano wrote:
 Hello,

 the BTRFS file system, avaiable on Linux since its 2.6.29 version,
 supports file cloning.  This simple patch adds the support for this
 feature to the cp utility.

 Right, so using the COW feature of BTRFS to speed up copies,
 and reduce space by only writing blocks that need to be changed.
 Cool!

From deea0ee0c2a521aae5a89d8613f937707d8f0e7b Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Sat, 25 Jul 2009 16:35:27 +0200
 Subject: [PATCH] cp: support the btrfs file system clone operation.

 This could apply to `install` also.

 +#ifdef __linux__
 +  /* Try a btrfs clone operation.  If the operation is not supported
 + or it fails then copy the file in the usual way.  */
 +  if (!ioctl (dest_desc, 1074041865, source_desc))
 +copied = true;
 +#endif

 I'm a little worried about doing it unconditionally.
 At first I was worried about deferring the copy operation
 to the process that's going to be modifying the data.
 But then realised going to be reading and writing the data anyway.
 One caveat though is, with this scheme it could be reading from
 and writing to different parts of a mechanical disk which could
 be slower due to seeking.

 Another possible issue with this I can think of is
 depending on the modification pattern of the COW files,
 the modification processes could fragment the file or
 more seriously be given ENOSPC errors.

I hope btrfs takes care of this behind the scene.

How does the clone work wrt to space consumed, a la df?
If copying a 1GB file this way does not update usage
stats to reflect the additional 1GB of space used, ...

 One could alleviate the last issues by using fallocate()
 on the destination, and patch for which is already written
 and hopefully we can get in soon (we're waiting on some
 clarifcation of the fallocate() interface). Note the fallocate
 patch only calls fallocate() when creating holes is not attempted.
 I.E. --sparse=never or --sparse=auto when the disk usage indicates
 that no holes are present.

 So perhaps we should hold off on this patch until
 the fallocate one goes in also?

If all I need to clone a huge file is the inode (and no blocks),
then yes, we'll have to wait.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 Adding this optimization should not change the meaning of
 --sparse=always.

 So do you want to use it only when --sparse=auto is used?

 Precisely.

 I cleaned the patch a bit, the clone operation is done only when
 --sparse=auto is used.

Thanks.
Here's an adjusted version that moves the in-function #ifdefs into
a helper function and includes sys/ioctl.h whenever HAVE_SYS_IOCTL_H
is defined.  I'll adjust indentation in a separate patch.

From 2be19afe6a06bdf4172ab10ff657a32121960217 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 25 Jul 2009 16:35:27 +0200
Subject: [PATCH] cp: support btrfs' copy-on-write file clone operation

* src/copy.c [HAVE_SYS_IOCTL_H]: Include sys/ioctl.h.
(clone_file): New function.
(copy_reg): Use the btrfs clone operation if possible.
---
 src/copy.c |   27 +++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 4c8c432..34af802 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -61,6 +61,10 @@
 # include verror.h
 #endif

+#if HAVE_SYS_IOCTL_H
+# include sys/ioctl.h
+#endif
+
 #ifndef HAVE_FCHOWN
 # define HAVE_FCHOWN false
 # define fchown(fd, uid, gid) (-1)
@@ -114,6 +118,20 @@ static bool owner_failure_ok (struct cp_options const *x);
 static char const *top_level_src_name;
 static char const *top_level_dst_name;

+/* Perform the O(1) btrfs clone operation, if possible.
+   Upon success, return 0.  Otherwise, return -1 and set errno.  */
+static inline int
+clone_file (int dest_fd, int src_fd)
+{
+#ifdef __linux__
+# define BTRFS_IOC_CLONE 1074041865
+  return ioctl (dest_fd, BTRFS_IOC_CLONE, src_fd);
+#else
+  errno = ENOTSUP;
+  return -1;
+#endif
+}
+
 /* FIXME: describe */
 /* FIXME: rewrite this to use a hash table so we avoid the quadratic
performance hit that's probably noticeable only on trees deeper
@@ -444,6 +462,7 @@ copy_reg (char const *src_name, char const *dst_name,
   struct stat sb;
   struct stat src_open_sb;
   bool return_val = true;
+  bool copied = false;

   source_desc = open (src_name,
  (O_RDONLY | O_BINARY
@@ -589,6 +608,14 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }

+  /* If --sparse=auto is in effect, attempt a btrfs clone operation.
+ If the operation is not supported or it fails then copy the file
+ in the usual way.  */
+  if (x-sparse_mode == SPARSE_AUTO
+   clone_file (dest_desc, source_desc) == 0)
+copied = true;
+
+  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
--
1.6.4.rc3.195.g2b05


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Andreas Schwab]

Jim Meyering j...@meyering.net writes:

 +#ifdef __linux__
 +# define BTRFS_IOC_CLONE 1074041865

This is wrong, the actual value is architecture dependent.  You should
use the _IOW macro instead.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Jim Meyering j...@meyering.net writes:

 Another possible issue with this I can think of is
 depending on the modification pattern of the COW files,
 the modification processes could fragment the file or
 more seriously be given ENOSPC errors.

 I hope btrfs takes care of this behind the scene.

 How does the clone work wrt to space consumed, a la df?
 If copying a 1GB file this way does not update usage
 stats to reflect the additional 1GB of space used, ...

I tried to clone a big file and df reported a different used blocks
stat that it was before the clone operation.


Cheers,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 However, but what about cp's --sparse option?
 btrfs supports sparse files, so this new code will have to
 honor that.  The trouble is that there is currently no option
 to say preserve precisely and only whatever holes are present
 in src, which is the behavior I would expect from that ioctl.

 Yes, sparse files are maintained.

And if a file is only partially sparse, cloning it
produces a file with non-sparse runs of zero bytes.

 $ dd if=/dev/zero of=a bs=1 seek=200M count=0

 $ stat a
   File: `a'
   Size: 209715200 Blocks: 0  IO Block: 4096   regular file
   ...

 $ ./cp a b

 $ stat b
   File: `b'
   Size: 209715200 Blocks: 0  IO Block: 4096   regular file
   ...

 The special case to handle differently is --sparse=never.


 I am inclined to extend the definition of --sparse=auto (the default)
 to mean make as faithful a copy as possible (btrfs clone), and failing
 that, use the sparse-preserving heuristic.  Then we can use the new
 ioctl in the default case.  The down side of such a move is that it would
 induce a subtle change in behavior: whereas before, a partially-sparse
 file would be copied (assuming btrfs FS src and dest) to a fully-sparse
 destination, now, you'll get a mirror image, partially-sparse copy.

 Right, it will behave differently on different file systems.  What about
 --sparse=always?

If I specify --sparse=always, that means I expect all
long-enough sequences of zeros to be converted to holes in the copy.
Hence with --sparse=always, cp must not use the ioctl, which
would perform no transformation.

 But seeing as how btrfs is so new, there is little legacy to worry about.
 And besides, for a command whose job it to copy, a feature to permit
 more faithful copying is hard to turn down or relegate to non-default
 status.

 ..., a cloned partially-sparse file on btrfs takes less
 space than a fully-sparse copied file.

That is not true.  A fully-sparse file takes less space
than a partially-sparse one.

 IMHO use the btrfs clone method is to prefer in both cases:
 --sparse=auto and --sparse=always.  I think that in any case,
 considering a file system could not support sparse files, --sparse
 should be considered just a suggestion and not mandatory, and the
 final decision left to cp.

Adding this optimization should not change the meaning of
--sparse=always.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:
 ..., a cloned partially-sparse file on btrfs takes less
 space than a fully-sparse copied file.

 That is not true.  A fully-sparse file takes less space
 than a partially-sparse one.

 Sorry I wasn't clear, I wanted to say that a cloned file in any case
 take less space than really copying it.

That depends on what you mean by really copying it.
If you copy with --sparse=always and the file has at least
one non-sparse run of zeros, that result will be smaller
than the cloned file.

 Adding this optimization should not change the meaning of
 --sparse=always.

 So do you want to use it only when --sparse=auto is used?

Precisely.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Jim Meyering j...@meyering.net writes:

 Adding this optimization should not change the meaning of
 --sparse=always.

 So do you want to use it only when --sparse=auto is used?

 Precisely.

I cleaned the patch a bit, the clone operation is done only when
--sparse=auto is used.

Regards,
Giuseppe


From 747c96980acc25220cc436210403cdcaed6239c9 Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano gscriv...@gnu.org
Date: Sat, 25 Jul 2009 16:35:27 +0200
Subject: [PATCH] cp: support the btrfs file system clone operation.

* src/copy.c(copy_reg): Use the btrfs clone operation if it is possible.
---
 src/copy.c |   15 +++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 4c8c432..e0ddec5 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -61,6 +61,11 @@
 # include verror.h
 #endif
 
+#ifdef __linux__
+# include sys/ioctl.h
+# define BTRFS_IOC_CLONE 1074041865
+#endif
+
 #ifndef HAVE_FCHOWN
 # define HAVE_FCHOWN false
 # define fchown(fd, uid, gid) (-1)
@@ -444,6 +449,7 @@ copy_reg (char const *src_name, char const *dst_name,
   struct stat sb;
   struct stat src_open_sb;
   bool return_val = true;
+  bool copied = false;
 
   source_desc = open (src_name,
  (O_RDONLY | O_BINARY
@@ -589,6 +595,15 @@ copy_reg (char const *src_name, char const *dst_name,
   goto close_src_and_dst_desc;
 }
 
+#ifdef __linux__
+  /* Try a btrfs clone operation.  If the operation is not supported
+ or it fails then copy the file in the usual way.  */
+  if ((x-sparse_mode == SPARSE_AUTO)  !ioctl (dest_desc, BTRFS_IOC_CLONE,
+ source_desc))
+copied = true;
+#endif
+
+  if (!copied)
   {
 typedef uintptr_t word;
 off_t n_read_total = 0;
-- 
1.6.3.3




___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Pádraig Brady]

Giuseppe Scrivano wrote:
 Hello,
 
 the BTRFS file system, avaiable on Linux since its 2.6.29 version,
 supports file cloning.  This simple patch adds the support for this
 feature to the cp utility.

Right, so using the COW feature of BTRFS to speed up copies,
and reduce space by only writing blocks that need to be changed.
Cool!

From deea0ee0c2a521aae5a89d8613f937707d8f0e7b Mon Sep 17 00:00:00 2001
 From: Giuseppe Scrivano gscriv...@gnu.org
 Date: Sat, 25 Jul 2009 16:35:27 +0200
 Subject: [PATCH] cp: support the btrfs file system clone operation.

This could apply to `install` also.

 +#ifdef __linux__
 +  /* Try a btrfs clone operation.  If the operation is not supported
 + or it fails then copy the file in the usual way.  */
 +  if (!ioctl (dest_desc, 1074041865, source_desc))
 +copied = true;
 +#endif

I'm a little worried about doing it unconditionally.
At first I was worried about deferring the copy operation
to the process that's going to be modifying the data.
But then realised going to be reading and writing the data anyway.
One caveat though is, with this scheme it could be reading from
and writing to different parts of a mechanical disk which could
be slower due to seeking.

Another possible issue with this I can think of is
depending on the modification pattern of the COW files,
the modification processes could fragment the file or
more seriously be given ENOSPC errors.

One could alleviate the last issues by using fallocate()
on the destination, and patch for which is already written
and hopefully we can get in soon (we're waiting on some
clarifcation of the fallocate() interface). Note the fallocate
patch only calls fallocate() when creating holes is not attempted.
I.E. --sparse=never or --sparse=auto when the disk usage indicates
that no holes are present.

So perhaps we should hold off on this patch until
the fallocate one goes in also?

cheers,
Pádraig.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Hello,

 the BTRFS file system, avaiable on Linux since its 2.6.29 version,
 supports file cloning.  This simple patch adds the support for this
 feature to the cp utility.

Thanks!  I like it.
A few comments:

Doesn't that constant, 1074041865, have a symbolic name?
Maybe BTRFS_IOC_CLONE?

 Is there an easy and quick way to determine which file system is used by
 a file?  Probably it would be safer to add a guard around the ioctl
 call.

Before thinking about that, I'd like to know the approximate cost
of the failing ioctl, e.g., on a kernel with btrfs support and
on one without, in case that makes a difference.  I.e., what impact
would it have if left unprotected?  Does it make a measurable difference
when copying 20K empty files on a tmpfs file system?

If necessary, we can avoid almost all of the per-file ioctl calls
via a map that associates each distinct device number encountered
with a boolean is btrfs file system.  gnulib's fts does something
similar, but its goal is to determine whether a different FS-specific
performance optimization is likely to help.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Jim Meyering j...@meyering.net writes:

 Doesn't that constant, 1074041865, have a symbolic name?
 Maybe BTRFS_IOC_CLONE?

Yes, it is exactly BTRFS_IOC_CLONE and it is defined in the
fs/btrfs/ioctl.h file.


 Is there an easy and quick way to determine which file system is used by
 a file?  Probably it would be safer to add a guard around the ioctl
 call.

 Before thinking about that, I'd like to know the approximate cost
 of the failing ioctl, e.g., on a kernel with btrfs support and
 on one without, in case that makes a difference.  I.e., what impact
 would it have if left unprotected?  Does it make a measurable difference
 when copying 20K empty files on a tmpfs file system?

 If necessary, we can avoid almost all of the per-file ioctl calls
 via a map that associates each distinct device number encountered
 with a boolean is btrfs file system.  gnulib's fts does something
 similar, but its goal is to determine whether a different FS-specific
 performance optimization is likely to help.

I didn't notice any difference using a patched and an un-patched cp on
tmpfs.

To go more in detail I used this simple test:

  int src = open (foo, O_RDONLY);
  int dest = open (bar, O_WRONLY | O_CREAT, 0777);

  assert (src);
  assert (dest);

  for (i = 0; i  ITERATIONS; i++)
if (!ioctl (dest, 1074041865, src))
  error (1, 0, cannot be successful!);

  close (src);
  close (dest);


On BTRFS, it raises correctly the cannot be successful! message.


Either on EXT3 and TMPFS the result is similar.


With 10 iterations:

0.01user 0.00system 0:00.01elapsed 118%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+120minor)pagefaults 0swaps


With 1000 iterations:

0.68user 0.82system 0:01.50elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
0inputs+0outputs (0major+119minor)pagefaults 0swaps


Regards,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Jim Meyering]

Giuseppe Scrivano wrote:
 Jim Meyering j...@meyering.net writes:

 Doesn't that constant, 1074041865, have a symbolic name?
 Maybe BTRFS_IOC_CLONE?

 Yes, it is exactly BTRFS_IOC_CLONE and it is defined in the
 fs/btrfs/ioctl.h file.


 Is there an easy and quick way to determine which file system is used by
 a file?  Probably it would be safer to add a guard around the ioctl
 call.

 Before thinking about that, I'd like to know the approximate cost
 of the failing ioctl, e.g., on a kernel with btrfs support and
 on one without, in case that makes a difference.  I.e., what impact
 would it have if left unprotected?  Does it make a measurable difference
 when copying 20K empty files on a tmpfs file system?

 If necessary, we can avoid almost all of the per-file ioctl calls
 via a map that associates each distinct device number encountered
 with a boolean is btrfs file system.  gnulib's fts does something
 similar, but its goal is to determine whether a different FS-specific
 performance optimization is likely to help.

 I didn't notice any difference using a patched and an un-patched cp on
 tmpfs.
...
 With 1000 iterations:

 0.68user 0.82system 0:01.50elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k
 0inputs+0outputs (0major+119minor)pagefaults 0swaps

Good.  That seems negligible.

However, but what about cp's --sparse option?
btrfs supports sparse files, so this new code will have to
honor that.  The trouble is that there is currently no option
to say preserve precisely and only whatever holes are present
in src, which is the behavior I would expect from that ioctl.

I am inclined to extend the definition of --sparse=auto (the default)
to mean make as faithful a copy as possible (btrfs clone), and failing
that, use the sparse-preserving heuristic.  Then we can use the new
ioctl in the default case.  The down side of such a move is that it would
induce a subtle change in behavior: whereas before, a partially-sparse
file would be copied (assuming btrfs FS src and dest) to a fully-sparse
destination, now, you'll get a mirror image, partially-sparse copy.
But seeing as how btrfs is so new, there is little legacy to worry about.
And besides, for a command whose job it to copy, a feature to permit
more faithful copying is hard to turn down or relegate to non-default
status.


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils

Re: BTRFS file clone support for cp

[Giuseppe Scrivano]

Hi Jim,

Jim Meyering j...@meyering.net writes:

 However, but what about cp's --sparse option?
 btrfs supports sparse files, so this new code will have to
 honor that.  The trouble is that there is currently no option
 to say preserve precisely and only whatever holes are present
 in src, which is the behavior I would expect from that ioctl.

Yes, sparse files are maintained.

$ dd if=/dev/zero of=a bs=1 seek=200M count=0

$ stat a
  File: `a'
  Size: 209715200   Blocks: 0  IO Block: 4096   regular file
  ...

$ ./cp a b

$ stat b
  File: `b'
  Size: 209715200   Blocks: 0  IO Block: 4096   regular file
  ...

The special case to handle differently is --sparse=never.


 I am inclined to extend the definition of --sparse=auto (the default)
 to mean make as faithful a copy as possible (btrfs clone), and failing
 that, use the sparse-preserving heuristic.  Then we can use the new
 ioctl in the default case.  The down side of such a move is that it would
 induce a subtle change in behavior: whereas before, a partially-sparse
 file would be copied (assuming btrfs FS src and dest) to a fully-sparse
 destination, now, you'll get a mirror image, partially-sparse copy.

Right, it will behave differently on different file systems.  What about
--sparse=always?


 But seeing as how btrfs is so new, there is little legacy to worry about.
 And besides, for a command whose job it to copy, a feature to permit
 more faithful copying is hard to turn down or relegate to non-default
 status.

Moreover the principal reason to use sparse files is to use less space
on the file system, a cloned partially-sparse file on btrfs takes less
space than a fully-sparse copied file.
IMHO use the btrfs clone method is to prefer in both cases:
--sparse=auto and --sparse=always.  I think that in any case,
considering a file system could not support sparse files, --sparse
should be considered just a suggestion and not mandatory, and the
final decision left to cp.

Regards,
Giuseppe


___
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils