Giuseppe Scrivano wrote:
> Jim Meyering 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 pat
Giuseppe Scrivano wrote:
> Jim Meyering 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_n
Jim Meyering 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 copy&paste :)
I
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 r
Hi Jim,
I attached the cleaned patch, according to your advises. If there are
other things, I'll fix them.
Regards,
Giuseppe
Jim Meyering writes:
> How about this for NEWS:
>
> cp accepts a new option, --reflink: create a lightweight copy
> using copy-on-write (COW). This is currently s
Giuseppe Scrivano wrote:
> Jim Meyering 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 patch
Jim Meyering 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
Jim Meyering 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:
>
Pádraig Brady wrote:
> Giuseppe Scrivano wrote:
>> Pádraig Brady 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, ye
Pádraig Brady wrote:
> Giuseppe Scrivano wrote:
>> Pádraig Brady 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, y
Giuseppe Scrivano wrote:
> Pádraig Brady 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
Pádraig Brady 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 r
Giuseppe Scrivano wrote:
> Hi Pádraig,
>
> thanks for the comments.
>
> Pádraig Brady 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 sma
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 vanill
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 (b
Hi Pádraig,
thanks for the comments.
Pádraig Brady 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: us
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 tha
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 s
>
> 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-
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
Jim Meyering 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
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
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 t
Andi Kleen wrote:
> Jim Meyering 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 t
Andi Kleen wrote:
> Jim Meyering 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
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={s
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 add
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 th
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 a
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
> >
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.
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.
Any comment?
Regards,
Giuseppe
>From 555192badb1a02dd730a3385e2540f48033b3de0 Mon Sep 17 00:00:00 2001
From: Giuseppe Scri
On Wed, Jul 29, 2009 at 12:10:14PM -0400, Chris Mason wrote:
> 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 bloc
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 yesterd
On Tue, Jul 28, 2009 at 10:06:35PM +0200, Giuseppe Scrivano wrote:
> Hi Pádraig,
>
>
> Pádraig Brady 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 t
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 origin
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 t
Jim Meyering wrote:
> Jim Meyering wrote:
>
>> Andreas Schwab wrote:
>>
>>> Jim Meyering 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. T
Jim Meyering wrote:
> Andreas Schwab wrote:
>
>> Jim Meyering 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
Hi Pádraig,
Pádraig Brady 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
Andreas Schwab wrote:
> Jim Meyering 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
Giuseppe Scrivano wrote:
> Jim Meyering 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 t
Jim Meyering 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 do
Jim Meyering 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 8
Giuseppe Scrivano wrote:
> Jim Meyering 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=
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,
>
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
Jim Meyering 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
>Fro
Giuseppe Scrivano wrote:
> Jim Meyering 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 clon
Jim Meyering 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 spac
Giuseppe Scrivano wrote:
> Jim Meyering 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
Hi Jim,
Jim Meyering 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 behav
Giuseppe Scrivano wrote:
> Jim Meyering 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
Jim Meyering 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
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?
55 matches
Mail list logo
