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

From: "jeff.liu" <[email-masked]>
Date: Mon, 24 May 2010 16:27:36 +0800

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
-- 
1.5.4.3

Thanks,
-Jeff

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