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

Previous Next

Full log

Message #56 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: "jeff.liu" <jeff.liu <at> oracle.com>
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: Re: bug#6131: [PATCH]: fiemap support for efficient sparse file copy
Date: Tue, 25 May 2010 07:50:27 +0200

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 $(...).

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

Previous Next

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