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

2011-01-25 Thread jeff.liu
Jim Meyering wrote:
> jeff.liu wrote:
>> Jim Meyering wrote:
>>> jeff.liu wrote:
 AFAICS, the tests passed on all filesystems except ext4,
>>> Really?
>>> The vast majority of my testing is with ext4 on Fedora 14, and I have seen
>>> no failure -- otherwise I would have mentioned that as a known problem.
>> I have mentioned this issue at:
>> http://osdir.com/ml/bug-coreutils-gnu/2010-09/msg00092.html
>>
>> "make test against cp/sparse-fiemap failed at the extent compare
>> stage, but the file content is
>> identical to each other by comparing those two files "j1/j2" manually.
>> Is it make sense if we verify them through diff(1) since the testing
>> file is in small size?"
> 
> No.  The whole point of the test is to verify that the extents have
> been preserved in the copy.  Diff doesn't know about extents.
> 
>>> What type of system/kernel are you using?
>> 2.6.33-RC3 && 2.6.36
>>> Was your ext4 partition created long ago?  With what options?
>> fiemap copy works well if run `cp' against physical ext4 partition.
>>> Did "make check" fail?  If so, please provide details.
>> Yeah, I will show the detail of 'make check' at below.
> 
> What version of filefrag are you using?
Mine comes from E2fsprogs version 1.41.12 shipped with ubuntu8.0.4.
I updated the filefrag(8) to the upstream one but still no luck. :(
the kernel I have tried are 2.6.28/2.6.33-RC3/2.6.36.

> Mine comes from e2fsprogs-1.41.12-6.fc14.x86_64
> 
>> btw, I just checked out the new branch and tried to compile it but ran
>> into an error:
>> date.c:30:28: error: parse-datetime.h: No such file or directory
>> date.c: In function 'batch_convert':
>> date.c:284: warning: implicit declaration of function 'parse_datetime'
>>
>> I guess 'parse-datetime.h' is shipped with gnulib? For now, I can not
>> pull the latest gnulib code
>> since the remote host does not response.
> 
> Did you run ./bootstrap ?
sure, yesterday it failed due to my proxy issue.


Thanks,
-Jeff
> That is a requirement whenever the coreutils
> pulls in a change to the gnulib submodule.
> 
>> For a quick reply, I ran 'make check' against the previous code base
>> before your latest commit.
>>
>> sudo make check TESTS=cp/sparse-fiemap VERBOSE=yes
> ...
>> + filefrag -v j2
> ...
>> + awk '/^ *[0-9]/ {printf "%d %d ", $2 ,NF < 5 ? $NF : $5 } END {print ""}'
>> @a and @b have different lengths, even after adjustment
>> + fail=1
> 
> 
> 






bug#7915: Possible off-by-1 in wc

2011-01-25 Thread Eric Blake
On 01/25/2011 06:18 PM, Sune Mølgaard wrote:
> sune@jekaterina:~$ echo Å|wc -m
> 2
> sune@jekaterina:~$ echo Å|wc -c
> 3
> sune@jekaterina:~$
> 
> Expected: 1 and 2 respectively.

Thanks for the report.  However, this is not a bug - echo is outputting
a newline (which is a second character, and third byte given the
encoding of your chosen character).

To see the difference, try:

$ echo Å | od -tx1z
000 c3 85 0a >...<
003
$ printf Å | od -tx1z
000 c3 85>..<
002

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


bug#7915: Possible off-by-1 in wc

2011-01-25 Thread Eric Blake
[Let's keep the list in the loop]

On 01/25/2011 07:04 PM, Sune Mølgaard wrote:
>> Thanks for the report.  However, this is not a bug - echo is outputting
>> a newline (which is a second character, and third byte given the
>> encoding of your chosen character).
>>
>> To see the difference, try:
>>
>> $ echo Å | od -tx1z
>> $ printf Å | od -tx1z
> 
> Thanks for the swift reply - I figured it might be something like that,
> but would it be possible to make an option to strip things like that, or
> is that a silly question?

An option to which program, echo or wc?

For wc, it's not possible.  How is wc supposed to know whether the input
has an extra trailing newline that should be ignored?  Fix the input
side of the equation, rather than adding complexity to wc.

For echo, the option is already (non-portably) there, in the form of
'echo -n' (won't work everywhere), or portably by using printf instead
of echo (as was the case in my example).

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


bug#7915: Possible off-by-1 in wc

2011-01-25 Thread Sune Mølgaard

sune@jekaterina:~$ echo Å|wc -m
2
sune@jekaterina:~$ echo Å|wc -c
3
sune@jekaterina:~$

Expected: 1 and 2 respectively.

wc is the one in Ubuntu Linux 10.10

Best regards,

Sune Mølgaard

--
It is wonderful how much time good people spend fighting the devil. If 
they would only expend the same amount of energy loving their fellow 
men, the devil would die in his own tracks of ennui.

- Helen Keller





bug#7877: sleep 5 -4

2011-01-25 Thread Jim Meyering
Paul Eggert wrote:
> OK, so it's late, but I can't resist:

You obviously need to, er... sleep.

> First, 'sleep' does accept one number that's negative
> in an IEEE-754 sense, namely, "sleep -- -0.0".
>
> Second, due to rounding error, 'sleep' does accept some
> numbers that are negative in a mathematical sense, e.g.,
> "sleep -- -1e1000" works.
>
> Third, there's nothing intrinsically wrong with 'sleep'
> accepting negative numbers.  All that POSIX
> requires is that 'sleep' must sleep for at *least* the
> amount of time specified.  So, if "sleep -- -1.0"
> is treated like "sleep 0", then it's conforming to
> that requirement of POSIX (obviously it doesn't
> conform to the other requirement that the operand be
> a nonnegative decimal integer; but it's a valid extension
> that is consistent with POSIX).

Maybe there is a use (albeit far-fetched) for negative numbers.
Let's say you want to sleep for 5 seconds less than a day.
Which would you prefer?
This:
sleep -- 1d -5s
or this:
sleep -- $((24*3600 - 5))s

> (Have I written enough to tempt you to extend 'sleep'
> to allow negative numbers?  :-)

I would accept the change.
Do you feel like writing it?





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

2011-01-25 Thread jeff.liu
Jim Meyering wrote:
> jeff.liu wrote:
>> AFAICS, the tests passed on all filesystems except ext4,
> 
> Really?
> The vast majority of my testing is with ext4 on Fedora 14, and I have seen
> no failure -- otherwise I would have mentioned that as a known problem.

I have mentioned this issue at:
http://osdir.com/ml/bug-coreutils-gnu/2010-09/msg00092.html
"make test against cp/sparse-fiemap failed at the extent compare stage, but the 
file content is
identical to each other by comparing those two files "j1/j2" manually.
Is it make sense if we verify them through diff(1) since the testing file is in 
small size?"

> 
> What type of system/kernel are you using?
2.6.33-RC3 && 2.6.36
> Was your ext4 partition created long ago?  With what options?
fiemap copy works well if run `cp' against physical ext4 partition.
> Did "make check" fail?  If so, please provide details.
Yeah, I will show the detail of 'make check' at below.

btw, I just checked out the new branch and tried to compile it but ran into an 
error:
date.c:30:28: error: parse-datetime.h: No such file or directory
date.c: In function 'batch_convert':
date.c:284: warning: implicit declaration of function 'parse_datetime'

I guess 'parse-datetime.h' is shipped with gnulib? For now, I can not pull the 
latest gnulib code
since the remote host does not response.

For a quick reply, I ran 'make check' against the previous code base before 
your latest commit.

sudo make check TESTS=cp/sparse-fiemap VERBOSE=yes

tests/cp/sparse-fiemap.log:
===

...
+ mount -oloop blob mnt
+ cd mnt
+ echo test
+ test -s f
+ test 0 = 1
+ dd if=/dev/zero of=sparse bs=1k count=1 seek=1G
1+0 records in
1+0 records out
1024 bytes (1.0 kB) copied, 4.6234e-05 s, 22.1 MB/s
+ timeout 10 cp --sparse=always sparse fiemap
++ stat --printf %s sparse
++ stat --printf %s fiemap
+ test 1099511628800 = 1099511628800
+ perl -e 1
++ seq 1 2 21
+ for i in '$(seq 1 2 21)'
+ for j in 1 2 31 100
+ perl -e 'BEGIN { $n = 1 * 1024; *F = *STDOUT }' -e 'for (1..1) { sysseek (*F, 
$n, 1)' -e '&&
syswrite (*F, chr($_)x$n) or die "$!"}'
+ cp --sparse=always j1 j2
+ cmp j1 j2
+ filefrag -v j1
+ grep extent
j1: 2 extents found
+ filefrag -v j1
+ filefrag -v j2
+ f ff1
+ perl /home/jeff/opensource_dev/fiemap_copy/tests/filefrag-extent-compare
+ awk '/^ *[0-9]/ {printf "%d %d ", $2 ,NF < 5 ? $NF : $5 } END {print ""}'
+ sed 's/ [a-z,][a-z,]*$//' ff1
+ f ff2
+ sed 's/ [a-z,][a-z,]*$//' ff2
+ awk '/^ *[0-9]/ {printf "%d %d ", $2 ,NF < 5 ? $NF : $5 } END {print ""}'
@a and @b have different lengths, even after adjustment
+ fail=1
+ break
+ test 1 = 1
+ break
+ Exit 1
+ set +e
+ exit 1
+ exit 1
+ remove_tmp_
+ __st=1
+ cleanup_
+ cd /
+ umount /home/jeff/opensource_dev/fiemap_copy/tests/gt-sparse-fiemap.cXgC/mnt
+ cd /home/jeff/opensource_dev/fiemap_copy/tests
+ chmod -R u+rwx 
/home/jeff/opensource_dev/fiemap_copy/tests/gt-sparse-fiemap.cXgC
+ rm -rf /home/jeff/opensource_dev/fiemap_copy/tests/gt-sparse-fiemap.cXgC
+ exit 1


Thanks,
-Jeff
> If something else failed, please give me enough information
> to reproduce it.
> 
>> but the result is ok by comparing the file
>> contents, can we take this risk?
> 
>> Is this patchset acceptable to merge into the next official release?
> 
> An ext4 failure sounds ominous.
> 
>> Another thing is to add solaris SEEK_DATA support to extent_scan.c as
>> we discussed before, not sure
>> if anyone working on this now. If not, I will take some time to follow
>> up but have to delay about 2
>> weeks since I will on vacation for the chinese new year start from next week.
>>
>> Btw, do you have plan to post extent_scan module to gnulib upstream?
>> so that other file archive
>> projects(like tar(1)) can benefit from it.
> 
> I do not plan to do that right away.
> 
>> Any thing I can do for this patchset please just let me know. :)
> 
> 
> 






bug#7877: sleep 5 -4

2011-01-25 Thread jidanni
> "PE" == Paul Eggert  writes:
PE> (Have I written enough to tempt ... to extend 'sleep'
PE> to allow negative numbers?  :-)
Right you are young man.
We here at NerdLabs already use
$ sleep -- -100
to give us a few moments to go back and correct errors.
But due to National Security, that's all I can say, except that one
shouldn't make assumptions about what future generations might want to do...





bug#7877: sleep 5 -4

2011-01-25 Thread Paul Eggert
OK, so it's late, but I can't resist:

First, 'sleep' does accept one number that's negative
in an IEEE-754 sense, namely, "sleep -- -0.0".

Second, due to rounding error, 'sleep' does accept some
numbers that are negative in a mathematical sense, e.g.,
"sleep -- -1e1000" works.

Third, there's nothing intrinsically wrong with 'sleep'
accepting negative numbers.  All that POSIX
requires is that 'sleep' must sleep for at *least* the
amount of time specified.  So, if "sleep -- -1.0"
is treated like "sleep 0", then it's conforming to
that requirement of POSIX (obviously it doesn't
conform to the other requirement that the operand be
a nonnegative decimal integer; but it's a valid extension
that is consistent with POSIX).

(Have I written enough to tempt you to extend 'sleep'
to allow negative numbers?  :-)





bug#7877: sleep 5 -4

2011-01-25 Thread Jim Meyering
jida...@jidanni.org wrote:
> $ sleep 5 -4
> sleep: invalid option -- '4'
> $ sleep -- 5 -4
> sleep: invalid time interval `-4'
>
> No fair prejudicing negative numbers.
>
> At least document it.
> 'However, GNU `sleep' accepts arbitrary floating point numbers (using a
> period before any fractional digits).' is what it says on Debian.

Hi Dan,

GNU sleep requires nonnegative numbers, so I've mentioned that
in the texinfo documentation.  I nearly made the s/arbitrary/nonnegative/
change to the sentence in both --help and coreutils.texi:

Unlike most implementations that require NUMBER be an integer,
here NUMBER may be an arbitrary floating point number.

But that seemed counterproductive, since we're making the contrast
between integer and FP.  Adding "nonnegative" there would detract.

Other implementations accept only a single argument,
so negative numbers would not make sense.


>From 228b7b3c85b85a14b702aac1642b931798916809 Mon Sep 17 00:00:00 2001
From: Jim Meyering 
Date: Tue, 25 Jan 2011 09:22:37 +0100
Subject: [PATCH] doc: mention that each sleep argument must be nonnegative

* doc/coreutils.texi (sleep invocation): Document that arguments
must be nonnegative.  Reported by Dan Jacobson.
---
 doc/coreutils.texi |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index ebe379e..398e20c 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -15674,7 +15674,7 @@ sleep invocation
 @end example

 @cindex time units
-Each argument is a number followed by an optional unit; the default
+Each argument is a nonnegative number followed by an optional unit; the default
 is seconds.  The units are:

 @table @samp
--
1.7.3.5.38.gb312b





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

2011-01-25 Thread Jim Meyering
jeff.liu wrote:
> AFAICS, the tests passed on all filesystems except ext4,

Really?
The vast majority of my testing is with ext4 on Fedora 14, and I have seen
no failure -- otherwise I would have mentioned that as a known problem.

What type of system/kernel are you using?
Was your ext4 partition created long ago?  With what options?
Did "make check" fail?  If so, please provide details.
If something else failed, please give me enough information
to reproduce it.

> but the result is ok by comparing the file
> contents, can we take this risk?

> Is this patchset acceptable to merge into the next official release?

An ext4 failure sounds ominous.

> Another thing is to add solaris SEEK_DATA support to extent_scan.c as
> we discussed before, not sure
> if anyone working on this now. If not, I will take some time to follow
> up but have to delay about 2
> weeks since I will on vacation for the chinese new year start from next week.
>
> Btw, do you have plan to post extent_scan module to gnulib upstream?
> so that other file archive
> projects(like tar(1)) can benefit from it.

I do not plan to do that right away.

> Any thing I can do for this patchset please just let me know. :)