GNU bug report logs - #6131
[PATCH]: fiemap support for efficient sparse file copy

Previous Next

Full log

View this message in rfc822 format

From: "jeff.liu" <jeff.liu <at> oracle.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: Sunil Mushran <sunil.mushran <at> oracle.com>, Tao Ma <tao.ma <at> oracle.com>, bug-coreutils <at> gnu.org, Joel Becker <Joel.Becker <at> oracle.com>, Chris Mason <chris.mason <at> oracle.com>
Subject: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
Date: Tue, 25 May 2010 16:11:33 +0800

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 <jeff.liu <at> oracle.com>
>> 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 <jeff.liu <at> oracle.com>
>> ---
>>  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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.