Re: [PATCH] copy: adjust fiemap handling of sparse files

[Jim Meyering]

Pádraig Brady wrote:
> On 18/03/11 13:19, Pádraig Brady wrote:
>> Bah humbug. Looks like there is no such issue.
>> This actually seems like an issue in a coreutils test script,
>> which made it seem like the SYNC done by `filefrag -vs` was ineffective.
>
> Proposed fix attached.

Thanks!  Here's a new version of your patch:

I've adjusted your new function to modify the actual arrays,
which lets us simplify the caller.
I've also added two die message "$ME: " prefixes I'd forgotten.

Is filefrag's -s option portable enough for us to rely on it?

>From 477316a020fb1e5d4f870700562dbc829e2920a5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Sat, 19 Mar 2011 01:22:37 +0000
Subject: [PATCH] tests: fix the sparse-fiemap test

* tests/filefrag-extent-compare: Merge adjacent extents in
each list before processing, so we correctly account for
split extents in either list.
* tests/cp/sparse-fiemap: Remove the explicit syncing,
which was only changing the way extents were arranged,
and thus working around the extent comparison issue
that was seen on ext4 loop back.
---
 tests/cp/sparse-fiemap        |   11 ++++----
 tests/filefrag-extent-compare |   53 ++++++++++++++++++++++++-----------------
 2 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/tests/cp/sparse-fiemap b/tests/cp/sparse-fiemap
index a2460a0..5f0beb7 100755
--- a/tests/cp/sparse-fiemap
+++ b/tests/cp/sparse-fiemap
@@ -69,12 +69,11 @@ for i in $(seq 1 2 21); do
           -e 'for (1..'$j') { sysseek (*F, $n, 1)' \
           -e '&& syswrite (*F, chr($_)x$n) or die "$!"}' > j1 || fail=1

-    # Note the explicit fdatasync is used here as
-    # it was seen that `filefrag -s` (FIEMAP_FLAG_SYNC) was
-    # ineffective on ext4 loopback on Linux 2.6.35.10-72.fc14.i686
-    dd if=/dev/null of=j1 conv=notrunc,fdatasync
+    # Note there is an implicit sync performed by cp to
+    # work arounds bugs in EXT4 and BTRFS before Linux 2.6.38
+    # Note also the -s parameter to the second filefrag below
+    # for the same reasons.
     cp --sparse=always j1 j2 || fail=1
-    dd if=/dev/null of=j2 conv=notrunc,fdatasync

     cmp j1 j2 || fail=1
     if ! filefrag -v j1 | grep -F extent >/dev/null; then
@@ -98,7 +97,7 @@ for i in $(seq 1 2 21); do

       # exclude the physical block numbers; they always differ
       filefrag -v j1 > ff1 || framework_failure
-      filefrag -v j2 > ff2 || framework_failure
+      filefrag -vs j2 > ff2 || framework_failure
       { f ff1; f ff2; } | $PERL $abs_top_srcdir/tests/filefrag-extent-compare 
||
         fail=1
     fi
diff --git a/tests/filefrag-extent-compare b/tests/filefrag-extent-compare
index 3c095d5..2c33584 100644
--- a/tests/filefrag-extent-compare
+++ b/tests/filefrag-extent-compare
@@ -28,30 +28,39 @@ my @b;
 foreach my $i (0..@A/2-1) { $a[$i] = { L_BLK => $A[2*$i], LEN => $A[2*$i+1] } 
};
 foreach my $i (0..@B/2-1) { $b[$i] = { L_BLK => $B[2*$i], LEN => $B[2*$i+1] } 
};

+# Merge adjacent extents in array E.
+sub merge_extents($)
+{
+  my ($e) = @_;
+
+  my $i = 0;
+  while (1)
+    {
+      !defined $e->[$i+1]
+        and last;
+      $e->[$i]->{L_BLK} + $e->[$i]->{LEN} != $e->[$i+1]->{L_BLK}
+        and ++$i, next;
+
+      $e->[$i]->{LEN} += $e->[$i+1]->{LEN};
+      # Remove $e->[$i+1]
+      splice @$e, $i+1, 1;
+    }
+}
+
+merge_extents \@a;
+merge_extents \@b;
+
+@a == @b
+  or die "$ME: extent counts differ, even after adjustment\n";
+
 my $i = 0;
-my $j = 0;
-while (1)
-  {
-    !defined $a[$i] && !defined $b[$j]
-      and exit 0;
-    defined $a[$i] && defined $b[$j]
-      or die "\@a and \@b have different lengths, even after adjustment\n";
-    ($a[$i]->{L_BLK} == $b[$j]->{L_BLK}
-     && $a[$i]->{LEN} == $b[$j]->{LEN})
-      and next;
-    ($a[$i]->{LEN} < $b[$j]->{LEN}
-     && exists $a[$i+1] && $a[$i]->{LEN} + $a[$i+1]->{LEN} == $b[$j]->{LEN})
-      and ++$i, next;
-    exists $b[$j+1] && $a[$i]->{LEN} == $b[$i]->{LEN} + $b[$i+1]->{LEN}
-      and ++$j, next;
-    die "differing extent:\n"
-      . "  [$i]=$a[$i]->{L_BLK} $a[$i]->{LEN}\n"
-      . "  [$j]=$b[$j]->{L_BLK} $b[$j]->{LEN}\n"
-  }
-continue
+while (defined $a[$i])
   {
-    ++$i;
-    ++$j;
+    $a[$i]->{L_BLK} == $b[$i]->{L_BLK} && $a[$i]->{LEN} == $b[$i]->{LEN}
+      or die "$ME: differing extent:\n"
+        . "  [$i]=$a[$i]->{L_BLK} $a[$i]->{LEN}\n"
+          . "  [$i]=$b[$i]->{L_BLK} $b[$i]->{LEN}\n";
+    $i++;
   }

 ### Setup "GNU" style for perl-mode and cperl-mode.
--
1.7.4.1.494.g5ddab