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