Blame SOURCES/coreutils-8.22-cp-sparsecorrupt.patch

dd59ef
From eea6b49210edf69682b2d0606bee17bbccb3765b Mon Sep 17 00:00:00 2001
dd59ef
From: Dmitry Monakhov <dmonakhov@openvz.org>
dd59ef
Date: Fri, 30 Oct 2015 22:04:46 +0000
dd59ef
Subject: [PATCH] copy: fix copying of extents beyond the apparent file size
dd59ef
dd59ef
fallocate can allocate extents beyond EOF via FALLOC_FL_KEEP_SIZE.
dd59ef
Where there is a gap (hole) between the extents, and EOF is within
dd59ef
that gap, the final hole wasn't reproduced, resulting in silent
dd59ef
data corruption in the copied file (size too small).
dd59ef
dd59ef
* src/copy.c (extent_copy): Ensure we don't process extents
dd59ef
beyond the apparent file size, since processing and allocating
dd59ef
those is not currently supported.
dd59ef
* tests/cp/fiemap-extents.sh: Renamed from tests/cp/fiemap-empty.sh
dd59ef
and re-enable parts checking the extents at and beyond EOF.
dd59ef
* tests/local.mk: Reference the renamed test.
dd59ef
Fixes http://bugs.gnu.org/21790
dd59ef
---
dd59ef
 src/copy.c                 |   19 ++++++++-
dd59ef
 tests/cp/fiemap-empty.sh   |  102 --------------------------------------------
dd59ef
 tests/cp/fiemap-extents.sh |   81 +++++++++++++++++++++++++++++++++++
dd59ef
 tests/local.mk             |    2 +-
dd59ef
 5 files changed, 100 insertions(+), 104 deletions(-)
dd59ef
 delete mode 100755 tests/cp/fiemap-empty.sh
dd59ef
 create mode 100755 tests/cp/fiemap-extents.sh
dd59ef
dd59ef
diff --git a/src/copy.c b/src/copy.c
dd59ef
index dc1cd29..6771bb5 100644
dd59ef
--- a/src/copy.c
dd59ef
+++ b/src/copy.c
dd59ef
@@ -432,6 +432,20 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
dd59ef
               ext_len = 0;
dd59ef
             }
dd59ef
 
dd59ef
+          /* Truncate extent to EOF.  Extents starting after EOF are
dd59ef
+             treated as zero length extents starting right after EOF.
dd59ef
+             Generally this will trigger with an extent starting after
dd59ef
+             src_total_size, and result in creating a hole or zeros until EOF.
dd59ef
+             Though in a file in which extents have changed since src_total_size
dd59ef
+             was determined, we might have an extent spanning that size,
dd59ef
+             in which case we'll only copy data up to that size.  */
dd59ef
+          if (src_total_size < ext_start + ext_len)
dd59ef
+            {
dd59ef
+              if (src_total_size < ext_start)
dd59ef
+                ext_start = src_total_size;
dd59ef
+              ext_len = src_total_size - ext_start;
dd59ef
+            }
dd59ef
+
dd59ef
           hole_size = ext_start - last_ext_start - last_ext_len;
dd59ef
 
dd59ef
           wrote_hole_at_eof = false;
dd59ef
@@ -495,14 +509,17 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
dd59ef
               off_t n_read;
dd59ef
               empty_extent = false;
dd59ef
               last_ext_len = ext_len;
dd59ef
+              bool read_hole;
dd59ef
 
dd59ef
               if ( ! sparse_copy (src_fd, dest_fd, buf, buf_size,
dd59ef
                                   sparse_mode == SPARSE_ALWAYS,
dd59ef
                                   src_name, dst_name, ext_len, &n_read,
dd59ef
-                                  &wrote_hole_at_eof))
dd59ef
+                                  &read_hole))
dd59ef
                 goto fail;
dd59ef
 
dd59ef
               dest_pos = ext_start + n_read;
dd59ef
+              if (n_read)
dd59ef
+                wrote_hole_at_eof = read_hole;
dd59ef
             }
dd59ef
 
dd59ef
           /* If the file ends with unwritten extents not accounted for in the
dd59ef
deleted file mode 100755
dd59ef
index b3b2cd7..0000000
dd59ef
--- a/tests/cp/fiemap-empty.sh
dd59ef
+++ /dev/null
dd59ef
@@ -1,101 +0,0 @@
dd59ef
-#!/bin/sh
dd59ef
-# Test cp reads unwritten extents efficiently
dd59ef
-
dd59ef
-# Copyright (C) 2011-2013 Free Software Foundation, Inc.
dd59ef
-
dd59ef
-# This program is free software: you can redistribute it and/or modify
dd59ef
-# it under the terms of the GNU General Public License as published by
dd59ef
-# the Free Software Foundation, either version 3 of the License, or
dd59ef
-# (at your option) any later version.
dd59ef
-
dd59ef
-# This program is distributed in the hope that it will be useful,
dd59ef
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
dd59ef
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
dd59ef
-# GNU General Public License for more details.
dd59ef
-
dd59ef
-# You should have received a copy of the GNU General Public License
dd59ef
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
dd59ef
-
dd59ef
-. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
dd59ef
-print_ver_ cp
dd59ef
-
dd59ef
-# FIXME: enable any part of this test that is still relevant,
dd59ef
-# or, if none are relevant (now that cp does not handle unwritten
dd59ef
-# extents), just remove the test altogether.
dd59ef
-skip_ 'disabled for now'
dd59ef
-
dd59ef
-touch fiemap_chk
dd59ef
-fiemap_capable_ fiemap_chk ||
dd59ef
-  skip_ 'this file system lacks FIEMAP support'
dd59ef
-rm fiemap_chk
dd59ef
-
dd59ef
-# TODO: rather than requiring $(fallocate), possible add
dd59ef
-# this functionality to truncate --alloc
dd59ef
-fallocate --help >/dev/null || skip_ 'The fallocate utility is required'
dd59ef
-fallocate -l 1 -n falloc.test ||
dd59ef
-  skip_ 'this file system lacks FALLOCATE support'
dd59ef
-rm falloc.test
dd59ef
-
dd59ef
-# Require more space than we'll actually use, so that
dd59ef
-# tests run in parallel do not run out of space.
dd59ef
-# Otherwise, with inadequate space, simply running the following
dd59ef
-# fallocate command would induce a temporary disk-full condition,
dd59ef
-# which would cause failure of unrelated tests run in parallel.
dd59ef
-require_file_system_bytes_free_ 800000000
dd59ef
-
dd59ef
-fallocate -l 600MiB space.test ||
dd59ef
-  skip_ 'this test needs at least 600MiB free space'
dd59ef
-
dd59ef
-# Disable this test on old BTRFS (e.g. Fedora 14)
dd59ef
-# which reports ordinary extents for unwritten ones.
dd59ef
-filefrag space.test || skip_ 'the 'filefrag' utility is missing'
dd59ef
-filefrag -v space.test | grep -F 'unwritten' > /dev/null ||
dd59ef
-  skip_ 'this file system does not report empty extents as "unwritten"'
dd59ef
-
dd59ef
-rm space.test
dd59ef
-
dd59ef
-# Ensure we read a large empty file quickly
dd59ef
-fallocate -l 300MiB empty.big || framework_failure_
dd59ef
-timeout 3 cp --sparse=always empty.big cp.test || fail=1
dd59ef
-test $(stat -c %s empty.big) = $(stat -c %s cp.test) || fail=1
dd59ef
-rm empty.big cp.test
dd59ef
-
dd59ef
-# Ensure we handle extents beyond file size correctly.
dd59ef
-# Note until we support fallocate, we will not maintain
dd59ef
-# the file allocation.  FIXME: amend this test when fallocate is supported.
dd59ef
-fallocate -l 10MiB -n unwritten.withdata || framework_failure_
dd59ef
-dd count=10 if=/dev/urandom conv=notrunc iflag=fullblock of=unwritten.withdata
dd59ef
-cp unwritten.withdata cp.test || fail=1
dd59ef
-test $(stat -c %s unwritten.withdata) = $(stat -c %s cp.test) || fail=1
dd59ef
-cmp unwritten.withdata cp.test || fail=1
dd59ef
-rm unwritten.withdata cp.test
dd59ef
-
dd59ef
-# The following to generate unaccounted extents followed by a hole, is not
dd59ef
-# supported by ext4 at least. The ftruncate discards all extents not
dd59ef
-# accounted for in the size.
dd59ef
-#  fallocate -l 10MiB -n unacc.withholes
dd59ef
-#  dd count=10 if=/dev/urandom conv=notrunc iflag=fullblock of=unacc.withholes
dd59ef
-#  truncate -s20M unacc.withholes
dd59ef
-
dd59ef
-# Ensure we handle a hole after empty extents correctly.
dd59ef
-# Since all extents are accounted for in the size,
dd59ef
-# we can maintain the allocation independently from
dd59ef
-# fallocate() support.
dd59ef
-fallocate -l 10MiB empty.withholes
dd59ef
-truncate -s 20M empty.withholes
dd59ef
-sectors_per_block=$(expr $(stat -c %o .) / 512)
dd59ef
-cp empty.withholes cp.test || fail=1
dd59ef
-test $(stat -c %s empty.withholes) = $(stat -c %s cp.test) || fail=1
dd59ef
-# These are usually equal but can vary by an IO block due to alignment
dd59ef
-alloc_diff=$(expr $(stat -c %b empty.withholes) - $(stat -c %b cp.test))
dd59ef
-alloc_diff=$(echo $alloc_diff | tr -d -- -) # abs()
dd59ef
-test $alloc_diff -le $sectors_per_block || fail=1
dd59ef
-# Again with SPARSE_ALWAYS
dd59ef
-cp --sparse=always empty.withholes cp.test || fail=1
dd59ef
-test $(stat -c %s empty.withholes) = $(stat -c %s cp.test) || fail=1
dd59ef
-# cp.test should take 0 space, but allowing for some systems
dd59ef
-# that store default extended attributes in data blocks
dd59ef
-test $(stat -c %b cp.test) -le $sectors_per_block || fail=1
dd59ef
-rm empty.withholes cp.test
dd59ef
-
dd59ef
-Exit $fail
dd59ef
diff --git a/tests/cp/fiemap-extents.sh b/tests/cp/fiemap-extents.sh
dd59ef
new file mode 100755
dd59ef
index 0000000..55ec5df
dd59ef
--- /dev/null
dd59ef
+++ b/tests/cp/fiemap-extents.sh
dd59ef
@@ -0,0 +1,81 @@
dd59ef
+#!/bin/sh
dd59ef
+# Test cp handles extents correctly
dd59ef
+
dd59ef
+# Copyright (C) 2011-2015 Free Software Foundation, Inc.
dd59ef
+
dd59ef
+# This program is free software: you can redistribute it and/or modify
dd59ef
+# it under the terms of the GNU General Public License as published by
dd59ef
+# the Free Software Foundation, either version 3 of the License, or
dd59ef
+# (at your option) any later version.
dd59ef
+
dd59ef
+# This program is distributed in the hope that it will be useful,
dd59ef
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
dd59ef
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
dd59ef
+# GNU General Public License for more details.
dd59ef
+
dd59ef
+# You should have received a copy of the GNU General Public License
dd59ef
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
dd59ef
+
dd59ef
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
dd59ef
+print_ver_ cp
dd59ef
+
dd59ef
+require_sparse_support_
dd59ef
+
dd59ef
+touch fiemap_chk
dd59ef
+fiemap_capable_ fiemap_chk ||
dd59ef
+  skip_ 'this file system lacks FIEMAP support'
dd59ef
+rm fiemap_chk
dd59ef
+
dd59ef
+fallocate --help >/dev/null || skip_ 'The fallocate utility is required'
dd59ef
+touch falloc.test || framework_failure_
dd59ef
+fallocate -l 1 -o 0 -n falloc.test ||
dd59ef
+  skip_ 'this file system lacks FALLOCATE support'
dd59ef
+rm falloc.test
dd59ef
+
dd59ef
+# We don't currently handle unwritten extents specially
dd59ef
+if false; then
dd59ef
+# Require more space than we'll actually use, so that
dd59ef
+# tests run in parallel do not run out of space.
dd59ef
+# Otherwise, with inadequate space, simply running the following
dd59ef
+# fallocate command would induce a temporary disk-full condition,
dd59ef
+# which would cause failure of unrelated tests run in parallel.
dd59ef
+require_file_system_bytes_free_ 800000000
dd59ef
+
dd59ef
+fallocate -l 600MiB space.test ||
dd59ef
+  skip_ 'this test needs at least 600MiB free space'
dd59ef
+
dd59ef
+# Disable this test on old BTRFS (e.g. Fedora 14)
dd59ef
+# which reports ordinary extents for unwritten ones.
dd59ef
+filefrag space.test || skip_ 'the 'filefrag' utility is missing'
dd59ef
+filefrag -v space.test | grep -F 'unwritten' > /dev/null ||
dd59ef
+  skip_ 'this file system does not report empty extents as "unwritten"'
dd59ef
+
dd59ef
+rm space.test
dd59ef
+
dd59ef
+# Ensure we read a large empty file quickly
dd59ef
+fallocate -l 300MiB empty.big || framework_failure_
dd59ef
+timeout 3 cp --sparse=always empty.big cp.test || fail=1
dd59ef
+test $(stat -c %s empty.big) = $(stat -c %s cp.test) || fail=1
dd59ef
+rm empty.big cp.test
dd59ef
+fi
dd59ef
+
dd59ef
+# Ensure we handle extents beyond file size correctly.
dd59ef
+# Note until we support fallocate, we will not maintain
dd59ef
+# the file allocation.  FIXME: amend this test if fallocate is supported.
dd59ef
+# Note currently this only uses fiemap logic when the allocation (-l)
dd59ef
+# is smaller than the size, thus identifying the file as sparse.
dd59ef
+# Note the '-l 1' case is an effective noop, and just checks
dd59ef
+# a file with a trailing hole is copied correctly.
dd59ef
+for sparse_mode in always auto never; do
dd59ef
+  for alloc in '-l 4MiB ' '-l 1MiB -o 4MiB' '-l 1'; do
dd59ef
+    dd count=10 if=/dev/urandom iflag=fullblock of=unwritten.withdata
dd59ef
+    truncate -s 2MiB unwritten.withdata || framework_failure_
dd59ef
+    fallocate $alloc -n unwritten.withdata || framework_failure_
dd59ef
+    cp --sparse=$sparse_mode unwritten.withdata cp.test || fail=1
dd59ef
+    test $(stat -c %s unwritten.withdata) = $(stat -c %s cp.test) || fail=1
dd59ef
+    cmp unwritten.withdata cp.test || fail=1
dd59ef
+    rm unwritten.withdata cp.test
dd59ef
+  done
dd59ef
+done
dd59ef
+
dd59ef
+Exit $fail
dd59ef
diff --git a/tests/local.mk b/tests/local.mk
dd59ef
index adf96f0..89fdbb0 100644
dd59ef
--- a/tests/local.mk
dd59ef
+++ b/tests/local.mk
dd59ef
@@ -446,7 +446,7 @@ all_tests =					\
dd59ef
   tests/cp/existing-perm-dir.sh			\
dd59ef
   tests/cp/existing-perm-race.sh		\
dd59ef
   tests/cp/fail-perm.sh				\
dd59ef
-  tests/cp/fiemap-empty.sh			\
dd59ef
+  tests/cp/fiemap-extents.sh			\
dd59ef
   tests/cp/fiemap-FMR.sh			\
dd59ef
   tests/cp/fiemap-perf.sh			\
dd59ef
   tests/cp/fiemap-2.sh				\
dd59ef
-- 
dd59ef
1.7.2.5
dd59ef