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

[jeff.liu]

Jim Meyering wrote:
> jeff.liu wrote:
> ...
>> 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 <address@hidden>
>> 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 <address@hidden>
>> ---
>>  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))
> 
> Thank you, but I will not use this patch.
> 
> First, $((...)) is *not* portable.
> Note that while $(...) is an improvement in coreutils tests, in most projects
> converting `...` to $(...) would represent a portability *regression*.
> It happens to be acceptable in coreutils tests (and thus preferred by me)
> because coreutils ensures that tests are run using a shell that is
> modern enough to accept $(...).
Thanks for the clarification.
> 
> Second, while I prefer $(...), it's not worth converting them
> one by one.  There are over 500+ uses in tests/.
> 
>> +dd bs=1 seek=$size of=sparse1 < /dev/null 2> /dev/null || framework_failure
> 
> Are you worried about tests running in parallel, and thus
> this test's "sparse" file colliding with the one by
> the same name in the fiemap test?
No.
At first, I took a look at the file 'cp/sparse', I consider it could be 
truncated by
the following DD(1) operation, because the target name also named as 'sparse' 
which is same as
'cp/sparse', if the test run at the same physical directory(i.e. tests/cp/)

++ expr 128 '*' 1024 + 1
+ size=131073
+ dd bs=1 seek=131073 of=sparse
+ cp --sparse=always sparse copy
++ stat --printf %b copy
++ stat --printf %b sparse
+ test 8 -le 8

but by checking through the run log file, looks all the tests are safely run at 
their own sub-dirs,
Sorry for my take for granted for this point.
+++ /home/jeff/opensource_dev/coreutils/src/mktemp -d
--tmp=/home/jeff/opensource_dev/coreutils/tests cu-sparse.XXXXXXXXXX

> That's not a problem, since each is run in its own
> separate subdirectory, via the machinery in test-lib.sh.
> 
>> -cp --sparse=always sparse copy || fail=1
>> +cp --sparse=always sparse1 copy || fail=1
> 
> 
> 

Thanks,
-Jeff

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