Subject: Re: bug#6131: [PATCH]: fiemap support for efficient sparse file copy

From: "jeff.liu" <[email-masked]>
Date: Mon, 24 May 2010 17:34:01 +0800

jeff.liu wrote:
> Jim Meyering wrote:
>> jeff.liu wrote:
>>> Jim Meyering wrote:
>>>> jeff.liu wrote:
>>>> ...
>>>>>>> Subject: [PATCH 1/1] tests: add a new test for FIEMAP-copy
>>>>>>>
>>>>>>> * tests/cp/sparse-fiemap: Add a new test for FIEMAP-copy against a
>>>>>>> loopbacked ext4 partition.
>>>>>>> * tests/Makefile.am (sparse-fiemap): Reference the new test.
>>>> BTW, I've just made this additional change to your test,
>>>>
>>>> diff --git a/tests/cp/sparse-fiemap b/tests/cp/sparse-fiemap
>>>> index 6312a4c..bdc7ded 100755
>>>> --- a/tests/cp/sparse-fiemap
>>>> +++ b/tests/cp/sparse-fiemap
>>>> @@ -27,6 +27,7 @@ require_root_
>>>>  cwd=`pwd`
>>>>  cleanup_() { cd /; umount "$cwd/mnt"; }
>>>>
>>>> +skip=0
>> More fixes for the test:
>> - remove useless rm
>> - $sparse and $fiemap were not defined in that final test,
>>     so cd into mnt/ and just use the file names instead.
>> - I prefer $(...) to `...`, and it's portable in this context
>>
>> diff --git a/tests/cp/sparse-fiemap b/tests/cp/sparse-fiemap
>> index 32ca5fc..cec5224 100755
>> --- a/tests/cp/sparse-fiemap
>> +++ b/tests/cp/sparse-fiemap
>> @@ -40,18 +40,18 @@ test -s mnt/f                                  || skip=1
>>  test $skip = 1 &&
>>    skip_test_ "insufficient mount/ext4 support"
>>
>> -rm -f mnt/f
>> -
>>  # Create a 1TiB sparse file
>>  dd if=/dev/zero of=mnt/sparse bs=1k count=1 seek=1G || framework_failure
>>
>> +cd mnt || fail=1
>> +
>>  # It takes many minutes to copy this sparse file using the old method.
>>  # By contrast, it takes far less than 1 second using FIEMAP-copy.
>> -timeout 10 cp --sparse=always mnt/sparse mnt/sparse_fiemap || fail=1
>> +timeout 10 cp --sparse=always sparse fiemap || fail=1
>>
>>  # Ensure that the sparse file copied through fiemap has the same size
>>  # in bytes as the original.
>> -test `stat --printf %s $sparse` = `stat --printf %s $fiemap` || fail=1
>> +test $(stat --printf %s sparse) = $(stat --printf %s fiemap) || fail=1
>>
>>  # =================================================
>>  # Ensure that we exercise the FIEMAP-copying code enough
>> --
>> 1.7.1.262.g5ef3d
> 
> Thanks for the info.
> 
> For the point of *- I prefer $(...) to `...`, and it's portable in this context*
> 
> Could you check the tiny patch below, it does the same thing to improve the portability of
> 'tests/cp/sparse', I have referred to it as a model.
> 
> From 9fbffcd0f4d4706f4b88e7ac73ca13b62c789047 Mon Sep 17 00:00:00 2001
> From: Jie Liu <[email-masked]>
> Date: Mon, 24 May 2010 16:01:06 +0800
> Subject: [PATCH 1/1] cp: improve the portability of test
> 
> * tests/cp/sparse: improve the portability using shell constructs.
> 
> Signed-off-by: Jie Liu <[email-masked]>
> ---
>  tests/cp/sparse |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/cp/sparse b/tests/cp/sparse
> index 73c2924..dee6de2 100755
> --- a/tests/cp/sparse
> +++ b/tests/cp/sparse
> @@ -28,14 +28,14 @@ require_sparse_support_
>  # It has to be at least 128K in order to be sparse on some systems.
>  # Make its size one larger than 128K, in order to tickle the
>  # bug in coreutils-6.0.
> -size=`expr 128 \* 1024 + 1`
> +size=$((128 * 1024 + 1))
>  dd bs=1 seek=$size of=sparse < /dev/null 2> /dev/null || framework_failure
> 
> 
>  cp --sparse=always sparse copy || fail=1
> 
>  # Ensure that the copy has the same block count as the original.
> -test `stat --printf %b copy` -le `stat --printf %b sparse` || fail=1
> +test $(stat --printf %b copy) -le $(stat --printf %b sparse) || fail=1
> 
>  # Ensure that --sparse={always,never} with --reflink fail.
>  cp --sparse=always --reflink sparse copy && fail=1

Please ignore above patch, I just found another issue in 'tests/cp/sparse', the new created test
file also named to 'sparse', so when it running, the `cp/sparse' will be truncated to `expr 128 \*
1024 + 1`.

Below patch fix it to create a sparse file 'sparse1' instead(I can not find out a better name for
now), s/-le/=/ to compare the block count.

>From 0669ac6d0497a3c6abfc5d53202afc6bc47d0d07 Mon Sep 17 00:00:00 2001
From: Jie Liu <[email-masked]>
Date: Mon, 24 May 2010 17:29:27 +0800
Subject: [PATCH 1/1] cp: enhance the sparse file copy test

* tests/cp/sparse: fix sparse file name to 'sparse1', improve
the protability using shell constructs.

Signed-off-by: Jie Liu <[email-masked]>
---
 tests/cp/sparse |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/cp/sparse b/tests/cp/sparse
index 73c2924..cab8b9c 100755
--- a/tests/cp/sparse
+++ b/tests/cp/sparse
@@ -28,17 +28,17 @@ require_sparse_support_
 # It has to be at least 128K in order to be sparse on some systems.
 # Make its size one larger than 128K, in order to tickle the
 # bug in coreutils-6.0.
-size=`expr 128 \* 1024 + 1`
-dd bs=1 seek=$size of=sparse < /dev/null 2> /dev/null || framework_failure
+size=$((128 * 1024 + 1))
+dd bs=1 seek=$size of=sparse1 < /dev/null 2> /dev/null || framework_failure

-cp --sparse=always sparse copy || fail=1
+cp --sparse=always sparse1 copy || fail=1

 # Ensure that the copy has the same block count as the original.
-test `stat --printf %b copy` -le `stat --printf %b sparse` || fail=1
+test $(stat --printf %b copy) = $(stat --printf %b sparse1) || fail=1

 # Ensure that --sparse={always,never} with --reflink fail.
-cp --sparse=always --reflink sparse copy && fail=1
-cp --sparse=never --reflink sparse copy && fail=1
+cp --sparse=always --reflink sparse1 copy && fail=1
+cp --sparse=never --reflink sparse1 copy && fail=1

 Exit $fail
-- 
1.5.4.3

-- 
With Windows 7, Microsoft is asserting legal control over your computer and is using this power to
abuse computer users.