On Tue, Dec 11, 2012 at 05:44:10PM +0100, Koen De Wit wrote:
> Tests included:
> - Creating a reflink and overwriting the original contents
> - Reflinking a directory tree
> - Moving/deleting reflinks
> - Diskspace consumption checks
> - Cross-filesystem copies (should fail)
> - Cross-subvolume copies
i'd say you've actually got several tests here, and that you
probably want to split them into multiple tests so that the
filesystem integrity can be checked after each test. i.e that
reflink hasn't silently corrupted the filesytsem structure...
> Signed-off-by: Koen De Wit <koen.de.wit@xxxxxxxxxx>
> ---
> 294 | 244
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 294.out | 89 +++++++++++++++++++++++
> group | 1 +
> 3 files changed, 334 insertions(+)
> create mode 100755 294
> create mode 100644 294.out
>
> diff --git a/294 b/294
> new file mode 100755
> index 0000000..7c422c8
> --- /dev/null
> +++ b/294
> @@ -0,0 +1,244 @@
> +#! /bin/bash
> +# FS QA Test No. 294
> +#
> +# Tests file clone functionality of btrfs ("reflinks"),
> +# including cross-subvolume copies.
More detail, please.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2012 Oracle. All Rights Reserved.
> +#
> +# 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.
> +#
> +# This program is distributed in the hope that it would 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, write the Free Software Foundation,
> +# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> +#-----------------------------------------------------------------------
> +#
> +# creator
> +owner=koen.de.wit@xxxxxxxxxx
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> + cd /
> + rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +
> +_require_scratch
You probably need to test for a cp binary that supports --reflink
here.
> +
> +testdir1=$TEST_DIR/test-$seq
> +rm -rf $testdir1
> +mkdir $testdir1
Better to leave the repvious version behind and create a new
directory altogether. i.e
THIS_TEST_DIR=$TEST_DIR/$seq.$$
> +testdir2=$SCRATCH_MNT/test-$seq
> +rm -rf $testdir2
> +mkdir $testdir2
You have to mkfs and mount the scratch device before you can use it.
Also, we tend to use upper case for variable names so they can be
easily seen in the code....
> +
> +# ----------------------------------
> +
> +# Simple test:
> +# - Reflink a file
> +# - Reflink the reflinked file
> +# - Modify the original file
> +# - Modify the reflinked file
> +
> +_catfiles() {
> + cat original copy1 copy2
> +}
No need for a wrapper like this. Just call it directly.
Also, I'd suggest that using larger files and dumping md5sum/sha1sum
to the output file is probably a better way to do this "are the
contents correct" check. it'll be consistent with how we check
data correctness in hole punch/zeroing tests.....
> +cd $testdir1
Please use absolute paths for output files rather than changing
directories.
> +echo "aaa" > original
use xfs_io to write a patterned file
> +cp --reflink original copy1
> +cp --reflink copy1 copy2
> +echo "--- Created file and 2 reflinked copies"
No need for all these text outputs into the output file. The comment
above is sufficient to understand the test.
> +_catfiles
> +echo "bbb" > original
And you can write a different pattern here with xfs_io.
> +echo "--- Changed original file"
> +_catfiles
> +echo "ccc" > copy1
> +echo "--- Changed first copy"
> +_catfiles
> +rm -rf original copy1 copy2
> +
> +# ----------------------------------
> +
> +# Reflinking directory tree:
> +# - Create directory and subdirectory, each having one file
> +# - Create 2 reflinked copies of the tree
> +# - Modify the original files
> +# - Modify one of the copies
new test, same comments as the last.
> +# ----------------------------------
> +
> +# Moving and deleting reflinks
> +# - Create a file and a reflink
> +# - Move both to a directory
> +# - Delete the original (moved) file, check that the copy still exists.
new test, same comments as the last.
> +# ----------------------------------
> +
> +# Diskspace consumption
> +# - Check that a reflink does not consume space
> +# - Check that a reflinks starts to consume space when the original file
> +# is modified
> +# - Check that diskspace is freed up after deleting all files
new test....
> +
> +_free() {
> + df -P $TEST_DIR | awk 'END {print $4}'
> +}
Perhaps you should copy this fucntion from test 015? i.e. use
_df_dir() and $AWK_PROG....
> +avail0=`_free`
free_before?
> +dd if=/dev/zero of=original bs=1MB count=10
xfs_io is preferable i.e.
$XFS_IO_PROG -f -c "pwrite 0 10m" -c "fsync" original
> +cp --reflink=always original copy1
> +cp --reflink=auto copy1 copy2
> +sleep 30
What's the point of sleeping here? If you are waiting for IO
completion, they shouldn't you run sync rather than sleeping?
> +avail1=`_free`
free_after?
> +echo "--- Created a 10MB file and two reflinks"
> +expr $avail0 - $avail1
So you are expecting metadata overhead to be identical on all
filesystem configurations?
> +# ----------------------------------
> +
> +# Cross-filesystem copy
> +# - Copy a file with the reflink=auto option.
> +# A normal copy should be created.
> +# - Copy a file with the reflink=always option. Should result in error,
> +# no file should be created.
New test.
> +_scratch_mkfs
> +_scratch_mount
> +mkdir $testdir2
> +
> +echo "aaa" > original
Where's this going? use of absolute paths makes it obvious where the
files exist...
> +cp --reflink=auto original $testdir2/copy
> +echo "--- Normal copy to second filesystem"
> +cat $testdir2/copy
> +rm -rf $testdir2
> +mkdir $testdir2
> +echo "--- Sparse copy to second filesystem (should fail)"
> +cp --reflink=always original $testdir2/copyfail 2>&1 | _filter_scratch
> +echo "--- Zero files should be created:"
> +ls $testdir2/copyfail | wc -l
> +
> +# ----------------------------------
> +
> +# Cross-subvolume copy
> +# - Create two subvolumes, mount one of them
> +# - Create a file on each (sub/root)volume,
> +# reflink them on the other volumes
> +# - Change the original files
> +# - Move and delete files
new test.
> +
> +_catfiles() {
> + for D in "test-$seq/" "$SCRATCH_MNT/" "subvol-$seq-2/"
> + do
> + cat $D/file1 $D/file2 $D/file3
> + done
> +}
BTW, defining the same function name in multiple places in one
script is not a nice thing to do. It's another good reason for
splitting these up into multiple separate tests.
> +
> +cd $TEST_DIR
> +_scratch_unmount
> +btrfs subvolume create subvol-$seq-1
> +btrfs subvolume create subvol-$seq-2
> +mount -t btrfs -o subvol=subvol-$seq-1 $TEST_DEV $SCRATCH_MNT
> +
> +echo "aaa" > test-$seq/file1
> +echo "bbb" > $SCRATCH_MNT/file2
> +echo "ccc" > subvol-$seq-2/file3
> +cp --reflink test-$seq/file1 subvol-$seq-1
> +cp --reflink test-$seq/file1 subvol-$seq-2
> +cp --reflink subvol-$seq-1/file2 test-$seq/
> +cp --reflink subvol-$seq-1/file2 subvol-$seq-2
> +cp --reflink subvol-$seq-2/file3 test-$seq/
> +cp --reflink subvol-$seq-2/file3 subvol-$seq-1
This is impossible to follow. Please create variables for holding
locations. e.g.
VOLUME_1=$TEST_DIR/test-#seq
VOLUME_2=$SCRATCH_MNT
SUBVOL_1=$TEST_DIR/subvol-$seq-1
SUBVOL_2=$TEST_DIR/subvol-$seq-2
> index dc8db65..b03001e 100644
> --- a/group
> +++ b/group
> @@ -410,3 +410,4 @@ deprecated
> 289 auto quick
> 290 auto rw prealloc quick ioctl
> 291 repair
> +294 auto
I'd add the rw group here as well...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
