Re: removing fiemap logic from copy.c

2021-05-09 Thread Pádraig Brady

On 02/05/2021 22:50, Pádraig Brady wrote:

FYI tomorrow I'm going to push a change to
remove the fiemap extent handling logic from cp/mv/install etc.
(Assuming that ok with folks of course).

This is only used on 10 year old linux kernels,
(where SEEK_HOLE is not available).

Also with fiemap we perform an fsync before each copy,
which is best avoided.

The patch is largish, but boring as just removes lines,
so not attaching here.

Will reply with a link when pushed.


As part of this, we should improve the tests on systems
that don't support fiemap, but where we will be using
the new SEEK_DATA logic.

I'll apply the attached to do that before removing the fiemap logic.

cheers,
Pádraig
>From 9c404d16208a5ae86b0876f9f2102c7205812b8a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sun, 9 May 2021 14:29:01 +0100
Subject: [PATCH] tests: ensure we test SEEK_DATA where used

fiemap is no longer the default copy implementation,
so check for SEEK_DATA support instead as that's preferred.
This will ensure better test coverage on systems without fiemap.

* init.cfg: Replace fiemap_capable_ with seek_data_capable_.
This is best supported with python 3 so prefer that.
* tests/seek-data-capable: A new test script checking for
SEEK_DATA support on the passed file name,
called from seek_data_capable_.
* tests/fiemap-capable: Remove no longer used probing script.
* tests/cp/fiemap-perf.sh: Renamed to tests/cp/sparse-perf.sh
* tests/cp/fiemap-2.sh: Renamed to tests/cp/sparse-2.sh
* tests/cp/fiemap-extents.sh: Renamed to tests/cp/sparse-extents.sh
* tests/cp/sparse-fiemap.sh: Renamed to tests/cp/sparse-extents-2.sh
* tests/cp/fiemap-FMR.sh: Renamed to tests/cp/copy-FMR.sh
* tests/local.mk: Reference the renamed tests.
---
 init.cfg  | 16 +
 tests/cp/{fiemap-FMR.sh => copy-FMR.sh}   |  1 +
 tests/cp/{fiemap-2.sh => sparse-2.sh} |  9 +++--
 .../{sparse-fiemap.sh => sparse-extents-2.sh} | 12 +++
 .../{fiemap-extents.sh => sparse-extents.sh}  |  9 +++--
 tests/cp/{fiemap-perf.sh => sparse-perf.sh}   | 20 +++
 tests/fiemap-capable  | 16 -
 tests/local.mk| 11 +++
 tests/seek-data-capable   | 33 +++
 9 files changed, 66 insertions(+), 61 deletions(-)
 rename tests/cp/{fiemap-FMR.sh => copy-FMR.sh} (95%)
 rename tests/cp/{fiemap-2.sh => sparse-2.sh} (88%)
 rename tests/cp/{sparse-fiemap.sh => sparse-extents-2.sh} (91%)
 rename tests/cp/{fiemap-extents.sh => sparse-extents.sh} (93%)
 rename tests/cp/{fiemap-perf.sh => sparse-perf.sh} (64%)
 delete mode 100644 tests/fiemap-capable
 create mode 100644 tests/seek-data-capable

diff --git a/init.cfg b/init.cfg
index 4ad2ecd11..568946015 100644
--- a/init.cfg
+++ b/init.cfg
@@ -530,15 +530,17 @@ require_kill_group_()
 }
 
 # Return nonzero if the specified path is on a file system for
-# which FIEMAP support exists.  Note some file systems (like ext3 and btrfs)
-# only support FIEMAP for files, not directories.
-fiemap_capable_()
+# which SEEK_DATA support exists.
+seek_data_capable_()
 {
-  if ! python < /dev/null; then
-warn_ 'fiemap_capable_: python missing: assuming not fiemap capable'
-return 1
+  { python3 < /dev/null && PYTHON_=python3; } ||
+  { python  < /dev/null && PYTHON_=python; }
+
+  if x"$PYTHON_" = x; then
+  warn_ 'seek_data_capable_: python missing: assuming not SEEK_DATA capable'
+  return 1
   fi
-  python "$abs_srcdir"/tests/fiemap-capable "$@"
+  $PYTHON_ "$abs_srcdir"/tests/seek-data-capable "$@"
 }
 
 # Skip the current test if "." lacks d_type support.
diff --git a/tests/cp/fiemap-FMR.sh b/tests/cp/copy-FMR.sh
similarity index 95%
rename from tests/cp/fiemap-FMR.sh
rename to tests/cp/copy-FMR.sh
index 7bbb88a2c..1ecdbe44d 100755
--- a/tests/cp/fiemap-FMR.sh
+++ b/tests/cp/copy-FMR.sh
@@ -22,6 +22,7 @@ print_ver_ cp
 require_valgrind_
 require_perl_
 
+# Trigger FMR in fiemap logic from v8.11..v8.19
 $PERL -e 'for (1..600) { sysseek (*STDOUT, 4096, 1)' \
   -e '&& syswrite (*STDOUT, "a" x 1024) or die "$!"}' > j || fail=1
 valgrind --quiet --error-exitcode=3 cp --reflink=never j j2 || fail=1
diff --git a/tests/cp/fiemap-2.sh b/tests/cp/sparse-2.sh
similarity index 88%
rename from tests/cp/fiemap-2.sh
rename to tests/cp/sparse-2.sh
index e6b80ebd3..05aa7f980 100755
--- a/tests/cp/fiemap-2.sh
+++ b/tests/cp/sparse-2.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# Exercise a few more corners of the fiemap-copying code.
+# Exercise a few more corners of the copying code.
 
 # Copyright (C) 2011-2021 Free Software Foundation, Inc.
 
@@ -19,10 +19,9 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ cp
 
-# Require a fiemap-enabled FS.
-touch fiemap_chk # check a file rather than current dir for best coverage
-fiemap_capable_ fiemap_chk \
-  || skip_ "this file system lacks FIEMAP support"
+touch sparse_chk
+seek_data_capable_ sparse_chk \
+  || skip_ "this 

Re: removing fiemap logic from copy.c

2021-05-02 Thread Jim Meyering
On Sun, May 2, 2021 at 2:50 PM Pádraig Brady  wrote:
> FYI tomorrow I'm going to push a change to
> remove the fiemap extent handling logic from cp/mv/install etc.
> (Assuming that ok with folks of course).
>
> This is only used on 10 year old linux kernels,
> (where SEEK_HOLE is not available).
>
> Also with fiemap we perform an fsync before each copy,
> which is best avoided.
>
> The patch is largish, but boring as just removes lines,
> so not attaching here.

That code brings back not-so-fond memories.
Good riddance :-)



removing fiemap logic from copy.c

2021-05-02 Thread Pádraig Brady

FYI tomorrow I'm going to push a change to
remove the fiemap extent handling logic from cp/mv/install etc.
(Assuming that ok with folks of course).

This is only used on 10 year old linux kernels,
(where SEEK_HOLE is not available).

Also with fiemap we perform an fsync before each copy,
which is best avoided.

The patch is largish, but boring as just removes lines,
so not attaching here.

Will reply with a link when pushed.

cheers,
Pádraig