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