This patch adds low level routines to common.punch
for populating test files and punching holes in them using
fallocate. When a hole is punched, file is then analyzed to
verify that the hole was punched correctly. These routines will
be used to test various corner cases in the new fallocate
punch hole tests.
Signed-off-by: Allison Henderson <[email protected]>
---
:100644 100644 e2da5d8... 778389a... M common.punch
common.punch | 393 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 393 insertions(+), 0 deletions(-)
diff --git a/common.punch b/common.punch
index e2da5d8..778389a 100644
--- a/common.punch
+++ b/common.punch
@@ -377,3 +377,396 @@ _test_generic_punch()
-c "$map_cmd -v" $testfile | $filter_cmd
[ $? -ne 0 ] && die_now
}
+
+
+#_fill_file()
+#
+#Fills a file with the given byte value up to the
+#given number of bytes.
+#
+# $1: The name of the file to fill
+# $2: The byte value to fill the file with
+# $3: The number of bytes to put in the file
+# $4: The block size used when copying in
+# "block" sized chunks of data
+_fill_file() {
+ local file_to_fill=$1
+ local byte_value=$2
+ local number_of_bytes=$3
+ local blk_size=$4
+
+ remaining_bytes=$number_of_bytes
+ blk=""
+
+ for (( i=0; i<blk_size; i++ ))
+ do
+ blk=$blk$byte_value
+ done
+
+ while [ $remaining_bytes -ge $blk_size ]
+ do
+ printf "$blk" >> $file_to_fill
+ remaining_bytes=$(($remaining_bytes - $blk_size))
+ done
+
+ for (( i=0; i<$remaining_bytes; i++ ))
+ do
+ printf "$byte_value" >> $file_to_fill
+ done
+
+
+}
+
+# _hole_test()
+#
+# Punches a hole in the test file.
+# The hole is the checked to make sure the correct number
+# of blocks are released and the associated extents are
+# removed
+#
+# Usage: _hole_test [options] file_name hole_offset hole_length
+# Options:
+# -c <length> Create a file of the given length full of data to punch a hole in
+# -p <length> Preallocate a file to the given length to punch a hole in
+# -s Sync the file before punching the hole
+# -m Remount the device before punching the hole
+#
+# This test is successful when the following conditions are met:
+# - ls shows that the number of blocks occupied by the file
+# has decreased by the number of blocks punched out.
+# - df shows number of available blocks has increased by the number
+# of blocks punched out
+# - File frag shows that the blocks punched out are absent from
+# the extents
+# - The test file contains zeros where the hole should be
+#
+_hole_test() {
+
+ err=0
+ sflag=
+ mflag=
+ pflag=
+ cflag=
+ lflag=
+ oflag=
+ OPTIND=1
+ while getopts 'smc:p:' OPTION
+ do
+ case $OPTION in
+ s) sflag=1
+ ;;
+ m) mflag=1
+ ;;
+ p) pflag="$OPTARG"
+ ;;
+ c) cflag="$OPTARG"
+ ;;
+ ?) echo Invalid flag
+ echo "Usage: $(basename $0): [options] file_name hole_offset hole_length" >&2
+ echo "Options:" >&2
+ echo "-c <length> Create a file of the given length full of data to punch a hole in" >&2
+ echo "-p <length> Preallocate a file to the given length to punch a hole in" >&2
+ echo "-s Sync the file before punching the hole" >&2
+ echo "-m Remount the device before punching the hole"
+ exit 1
+ ;;
+ esac
+ done
+ shift $(($OPTIND - 1))
+
+ local file_name=$1
+ local hole_offset=$2
+ local hole_length=$3
+ local hole_offset_orig=$hole_offset
+ local hole_length_orig=$hole_length
+
+ if [ "$cflag" ]; then
+ local file_length=$cflag
+ elif [ "$pflag" ]; then
+ local file_length=$pflag
+ else
+ local file_length=`ls -ls $file_name | cut -d ' ' -f6`
+ fi
+
+ # If the hole to punch out extends off the edge of the file,
+ # then we need to expect a smaller hole to be punched
+ if [ $hole_offset -ge $file_length ]; then
+ local hole_length=0
+ local hole_offset=$file_length
+ elif [ $(($hole_offset + $hole_length)) -gt $file_length ]; then
+ local hole_length=$(($file_length - $hole_offset))
+ fi
+
+ # locations to store hexdumps must not be in the filesystem
+ # or it will skew the results. When we measure used
+ # available blocks. Also, there may not be enough
+ # room to store them in the fs during ENOSPC tests
+ # So store the dumps in the cwd by stripping the path
+ expected=`echo $file_name.hexdump.expected | sed 's/.*\///'`
+ result=`echo $file_name.hexdump.result | sed 's/.*\///'`
+
+ #calculate the end points of the hole in blocks
+ local block_size=`stat -f $TEST_DEV | grep "Block size" | cut -d " " -f3`
+ local hole_first_blk=$(( $hole_offset / $block_size ))
+ if [ $(($hole_offset % $block_size)) != 0 ]
+ then
+ local hole_first_blk=$(( $hole_first_blk + 1 ))
+ fi
+
+ local hole_last_blk=$(( ($hole_offset + $hole_length) / $block_size ))
+
+ #calculate the length of the hole in blocks
+ if [ $hole_first_blk -gt $hole_last_blk ]
+ then
+ local hole_length_block=0
+ else
+ local hole_length_block=$(( $hole_last_blk - $hole_first_blk ))
+ fi
+
+ #record how many blocks free blocks the filesystem has
+ local fs_used_before=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | awk '{ print $3 }'`
+ local fs_avail_before=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | awk '{ print $4 }'`
+
+ rm $expected &> /dev/null
+ rm $result &> /dev/null
+ local total_hole_overlap=0
+
+ #if we are preallocating, then just fallocate the file
+ if [ "$pflag" ]; then
+ rm $file_name &> /dev/null
+
+ fallocate -o 0 -l $file_length $file_name
+
+ #get the hexdump, this is what the punch hole should look like
+ hexdump -C $file_name > $expected
+
+ #else if createing the file, manually construct the file
+ elif [ "$cflag" ]; then
+ rm $file_name &> /dev/null
+ #create a file full of data with zeros in the middle
+
+ #Fill in head of file
+ _fill_file $file_name "\xFF" $hole_offset $block_size
+
+ #Fill in hole with zeros
+ if [ $hole_length -gt 0 ]; then
+ (dd if=/dev/zero of=$file_name obs=1 ibs=1 seek=$(($hole_offset + $hole_length - 1)) \
+ count=1 conv=notrunc) &> /dev/null
+ fi
+
+ #Fill in tail of file
+ _fill_file $file_name "\xFF" $(( $file_length - $hole_offset - $hole_length )) $block_size
+
+ #get the hexdump, this is what the punch hole should look like
+ hexdump -C $file_name > $expected
+
+ #refill the file
+ rm $file_name
+ _fill_file $file_name "\xFF" $file_length $block_size
+
+ #else we are punching a hole in an existing file
+ else
+
+ local tmp_file=`echo $file_name.tmp | sed 's/.*\///'`
+
+ #make a back up of the test file
+ cp $file_name $tmp_file
+
+ #Fill in hole with zeros
+ if [ $hole_length -gt 0 ]; then
+ (dd if=/dev/zero of=$tmp_file obs=1 ibs=1 seek=$(($hole_offset)) \
+ count=$hole_length conv=notrunc) &> /dev/null
+ fi
+
+ #get the hexdump, this is what the punch hole should look like
+ hexdump -C $tmp_file > $expected
+ rm $tmp_file
+
+ # Now we need to check for existing holes
+ # Holes that are already punched out will need to be subtracted from the
+ # total blocks expected to be punched out
+ # Walk over the extents, to find existing holes.
+ if [ $hole_length_block -gt 0 ]; then
+ IFS="
+"
+ local prev_logical_blk=0
+ local prev_ext_len=0
+ local prev_last_logical_blk=0
+ extents=`filefrag -v $file_name | sed '1,3d' | sed '$d'`
+ for extent in $extents
+ do
+ #extract info from extent
+ ext_num=`echo $extent | cut -c1-4 | tr -d " "`
+ logical_blk=`echo $extent | cut -c5-12 | tr -d " "`
+ phys_blk=`echo $extent | cut -c13-21 | tr -d " "`
+ expct_phys_blk=`echo $extent | cut -c22-30 | tr -d " "`
+ ext_len=`echo $extent | cut -c31-37 | tr -d " "`
+ ext_flags=`echo $extent | cut -c38- | tr -d " "`
+ last_logical_blk=$(( $logical_blk + $ext_len ))
+
+ #calculate the space between this extent and the last
+ existing_hole_start=$prev_last_logical_blk
+ existing_hole_len=$(( $logical_blk - $prev_last_logical_blk ))
+ existing_hole_end=$(( $existing_hole_start + $existing_hole_len ))
+ hole_overlap=0;
+
+ #if there is a hole, check to see if it intersects with the new hole
+ if [ $existing_hole_len -gt 0 ]; then
+ if [[ $hole_first_blk -le $existing_hole_start && \
+ $existing_hole_start -lt $hole_last_blk ]]; then
+
+ if [[ $hole_first_blk -lt $existing_hole_end && \
+ $existing_hole_end -le $hole_last_blk ]] ;then
+
+ #The new hole contains an old hole
+ hole_overlap=$existing_hole_len;
+ else
+ #The new hole contains the start of an old hole
+ hole_overlap=$(( $hole_last_blk - $existing_hole_start ))
+ fi
+
+ elif [[ $hole_first_blk -lt $existing_hole_end && \
+ $existing_hole_end -le $hole_last_blk ]]; then
+
+ #The new hole contains the tail of an old hole
+ hole_overlap=$(( $existing_hole_end - $hole_first_blk ))
+
+ elif [[ $existing_hole_start -le $hole_first_blk && \
+ $hole_first_blk -lt $existing_hole_end ]] && \
+ [[ $existing_hole_start -lt $hole_last_blk && \
+ $hole_last_blk -le $existing_hole_end ]]; then
+
+ #The old hole contains the new hole
+ hole_overlap=$hole_length_block
+
+ fi
+
+ fi
+
+ #tally up the total number of already punched out blocks
+ total_hole_overlap=$(( $total_hole_overlap + $hole_overlap ))
+
+ prev_logical_blk=$logical_blk
+ prev_ext_len=$ext_len
+ prev_last_logical_blk=$last_logical_blk
+ done
+ fi
+
+
+ fi
+
+ if [ "$sflag" ]
+ then
+ sync
+ fi
+
+
+ if [ "$mflag" ]
+ then
+ umount $TEST_DEV
+ mount $TEST_DEV $TEST_DIR
+ fi
+
+ #record how many blocks free blocks the filesystem has
+ local fs_used_before=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | awk '{ print $3 }'`
+ local fs_avail_before=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | awk '{ print $4 }'`
+ local file_used_before=`ls -ls --block-size=$block_size $file_name | cut -d " " -f1`
+
+ #punch the hole
+ $XFS_IO_PROG -F -f -c "fpunch $hole_offset_orig $hole_length_orig " $file_name
+
+ #get the results
+ local fs_used_after=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | awk '{ print $3 }'`
+ local fs_avail_after=`df --block-size=$block_size $TEST_DEV | grep $TEST_DEV | awk '{ print $4 }'`
+ local file_used_after=`ls -ls --block-size=$block_size $file_name | cut -d " " -f1`
+ hexdump -C $file_name > $result #this must be collected last or it will skew the results
+
+
+ # verify results
+ #------------------------------------
+
+ # Does the file contain zeros where it should?
+ diff $expected $result
+ if [ "$?" -ne "0" ]; then
+ echo Punch hole failed: Expected file and resulting file differ
+ echo Check $expected vs $result
+ err=1
+ fi
+
+ # Has the number of used filesystem blocks decreased by the size of the hole?
+ if [ $(($fs_used_before - $fs_used_after)) != $(($hole_length_block - $total_hole_overlap)) ]; then
+ echo Punch hole failed: Filesystem shows an incorrect number of used blocks
+ echo Used blocks before: $fs_used_before
+ echo Used blocks after: $fs_used_after
+ echo Hole Length: $hole_length_block
+ echo Blocks already punched out: $total_hole_overlap
+ echo Total blocks that should be punched out from this hole: $hole_length_block
+ err=1
+ fi
+
+ # Has the number of free filesystem blocks increased by the size of the hole?
+ if [ $(($fs_avail_after - $fs_avail_before)) != $(($hole_length_block - $total_hole_overlap)) ]; then
+ echo Punch hole failed: Filesystem shows an incorrect number of available blocks
+ echo Available blocks before: $fs_avail_before
+ echo Available blocks after: $fs_avail_after
+ echo Hole Length: $hole_length_block
+ echo Blocks already punched out: $total_hole_overlap
+ echo Total blocks that should be punched out from this hole: $hole_length_block
+ err=1
+ fi
+
+ # Has the number of blocks the file occupies decreased by the size of the hole?
+ if [ $(($file_used_before - $file_used_after)) != $(($hole_length_block - $total_hole_overlap)) ]; then
+ echo Punch hole failed: File shows an incorrect number of used blocks
+ echo Used blocks before: $file_used_before
+ echo Used blocks after: $file_used_after
+ echo Hole Length: $hole_length_block
+ echo Blocks already punched out: $total_hole_overlap
+ echo Total blocks that should be punched out from this hole: $hole_length_block
+ err=1
+ fi
+
+
+ #Check the file extents. There should be no extent present where the hole is
+ if [ $hole_length_block -gt 0 ]; then
+ IFS="
+"
+
+ extents=`filefrag -v $file_name | sed '1,3d' | sed '$d'`
+ for extent in $extents
+ do
+ ext_num=`echo $extent | cut -c1-4 | tr -d " "`
+ logical_blk=`echo $extent | cut -c5-12 | tr -d " "`
+ phys_blk=`echo $extent | cut -c13-21 | tr -d " "`
+ expct_phys_blk=`echo $extent | cut -c22-30 | tr -d " "`
+ ext_len=`echo $extent | cut -c31-37 | tr -d " "`
+ ext_flags=`echo $extent | cut -c38- | tr -d " "`
+ last_logical_blk=$(( $logical_blk + $ext_len ))
+
+ #check to see if the extent intersects with the hole
+ if [[ $hole_first_blk -le $logical_blk && $logical_blk -lt $hole_last_blk ]] || \
+ [[ $hole_first_blk -lt $last_logical_blk && $last_logical_blk -le $hole_last_blk ]] || \
+ [[ $logical_blk -le $hole_first_blk && $hole_first_blk -lt $last_logical_blk ]] || \
+ [[ $logical_blk -lt $hole_last_blk && $hole_last_blk -le $last_logical_blk ]]
+ then
+
+ echo File fragmentation shows extents that should have been punched out
+ filefrag -v $file_name
+ echo Bad extent: $extent
+ echo Hole Start Block: $hole_first_blk End Block:$hole_last_blk Length:$hole_length_block
+ err=1
+ fi
+ done
+ fi
+
+ if [ $err -gt 0 ]; then
+ echo Hole Punch Test Failed
+ status=1
+ exit $err
+ else
+ rm $expected
+ rm $result
+ fi
+
+}
+
--
1.7.1
On Wed, May 18, 2011 at 01:58:40PM -0700, Allison Henderson wrote:
> This patch adds low level routines to common.punch
> for populating test files and punching holes in them using
> fallocate. When a hole is punched, file is then analyzed to
> verify that the hole was punched correctly. These routines will
> be used to test various corner cases in the new fallocate
> punch hole tests.
So what condition does this test cover that test 252 doesn't?
>
> Signed-off-by: Allison Henderson <[email protected]>
> ---
> :100644 100644 e2da5d8... 778389a... M common.punch
> common.punch | 393 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 393 insertions(+), 0 deletions(-)
>
> diff --git a/common.punch b/common.punch
> index e2da5d8..778389a 100644
> --- a/common.punch
> +++ b/common.punch
> @@ -377,3 +377,396 @@ _test_generic_punch()
> -c "$map_cmd -v" $testfile | $filter_cmd
> [ $? -ne 0 ] && die_now
> }
> +
> +
> +#_fill_file()
> +#
> +#Fills a file with the given byte value up to the
> +#given number of bytes.
> +#
> +# $1: The name of the file to fill
> +# $2: The byte value to fill the file with
> +# $3: The number of bytes to put in the file
> +# $4: The block size used when copying in
> +# "block" sized chunks of data
> +_fill_file() {
> + local file_to_fill=$1
> + local byte_value=$2
> + local number_of_bytes=$3
> + local blk_size=$4
> +
> + remaining_bytes=$number_of_bytes
> + blk=""
> +
> + for (( i=0; i<blk_size; i++ ))
> + do
> + blk=$blk$byte_value
> + done
> +
> + while [ $remaining_bytes -ge $blk_size ]
> + do
> + printf "$blk" >> $file_to_fill
> + remaining_bytes=$(($remaining_bytes - $blk_size))
> + done
> +
> + for (( i=0; i<$remaining_bytes; i++ ))
> + do
> + printf "$byte_value" >> $file_to_fill
> + done
> +
> +
> +}
Ummm, do you really need to reinvent the wheel?
$XFS_IO_PROG -F -f -c "pwrite -S $byte_value -b $blk_size 0 $number_of_bytes" $file_to_fill
> +
> +# _hole_test()
> +#
> +# Punches a hole in the test file.
> +# The hole is the checked to make sure the correct number
> +# of blocks are released and the associated extents are
> +# removed
> +#
> +# Usage: _hole_test [options] file_name hole_offset hole_length
> +# Options:
> +# -c <length> Create a file of the given length full of data to punch a hole in
rm -f $file_name
$XFS_IO_PROG -F -f -c "t $length" $file_name
> +# -p <length> Preallocate a file to the given length to punch a hole in
$XFS_IO_PROG -F -f -c "falloc 0 $length" $file_name
> +# -s Sync the file before punching the hole
$XFS_IO_PROG -F -f -c "fsync" $file_name
> +# -m Remount the device before punching the hole
....
> + umount $TEST_DEV
> + mount $TEST_DEV $TEST_DIR
If you need remounts during the test, it should be using the scratch
device, I think.
> +#
> +# This test is successful when the following conditions are met:
> +# - ls shows that the number of blocks occupied by the file
> +# has decreased by the number of blocks punched out.
There's no guarantee that a filesystem will punch the number of
blocks you expect.
> +# - df shows number of available blocks has increased by the number
> +# of blocks punched out
Ditto - indeed, punching blocks can lead to allocation for things
like extent tree splits because the number of extent records
increases.
> +# - File frag shows that the blocks punched out are absent from
> +# the extents
Probably the best method, though we tend to use the xfs_io output
because we already have a lot of parsing functions for it.
> +# - The test file contains zeros where the hole should be
> +#
> +_hole_test() {
> +
> + err=0
> + sflag=
> + mflag=
> + pflag=
> + cflag=
> + lflag=
> + oflag=
> + OPTIND=1
> + while getopts 'smc:p:' OPTION
> + do
> + case $OPTION in
> + s) sflag=1
> + ;;
> + m) mflag=1
> + ;;
> + p) pflag="$OPTARG"
> + ;;
> + c) cflag="$OPTARG"
> + ;;
> + ?) echo Invalid flag
> + echo "Usage: $(basename $0): [options] file_name hole_offset hole_length" >&2
> + echo "Options:" >&2
> + echo "-c <length> Create a file of the given length full of data to punch a hole in" >&2
> + echo "-p <length> Preallocate a file to the given length to punch a hole in" >&2
> + echo "-s Sync the file before punching the hole" >&2
> + echo "-m Remount the device before punching the hole"
> + exit 1
> + ;;
> + esac
> + done
> + shift $(($OPTIND - 1))
> +
> + local file_name=$1
> + local hole_offset=$2
> + local hole_length=$3
> + local hole_offset_orig=$hole_offset
> + local hole_length_orig=$hole_length
> +
> + if [ "$cflag" ]; then
> + local file_length=$cflag
> + elif [ "$pflag" ]; then
> + local file_length=$pflag
> + else
> + local file_length=`ls -ls $file_name | cut -d ' ' -f6`
> + fi
> +
> + # If the hole to punch out extends off the edge of the file,
> + # then we need to expect a smaller hole to be punched
> + if [ $hole_offset -ge $file_length ]; then
> + local hole_length=0
> + local hole_offset=$file_length
> + elif [ $(($hole_offset + $hole_length)) -gt $file_length ]; then
> + local hole_length=$(($file_length - $hole_offset))
> + fi
> +
> + # locations to store hexdumps must not be in the filesystem
> + # or it will skew the results. When we measure used
> + # available blocks. Also, there may not be enough
> + # room to store them in the fs during ENOSPC tests
> + # So store the dumps in the cwd by stripping the path
> + expected=`echo $file_name.hexdump.expected | sed 's/.*\///'`
> + result=`echo $file_name.hexdump.result | sed 's/.*\///'`
This is why you shoul dbe using the scratch device and using the
test device to store the results. No hoops to jump through, and the
result is s simpler test.
Oh, and all you do is run diff on the hexdump output of the files.
diff can check binary files just fine - why do you need the hexdump
output? i.e. create files with zeros where the holes will be on the
testdev, create new ones on the scratch dev, punch holes in them,
diff them. No need for hexdumps, and the diff output can be put
directly into the golden output. If we then dump the fiemap output
into the output file (appropriately filtered) no further validation
checks are needed to determine that the hole was punched in the
correct place....
Indeed, I've seen plenty of cases where same test case operating on
an existing file gives different results to operating on a newly
created file....
> +
> + #calculate the end points of the hole in blocks
> + local block_size=`stat -f $TEST_DEV | grep "Block size" | cut -d " " -f3`
> + local hole_first_blk=$(( $hole_offset / $block_size ))
> + if [ $(($hole_offset % $block_size)) != 0 ]
> + then
> + local hole_first_blk=$(( $hole_first_blk + 1 ))
> + fi
> +
> + local hole_last_blk=$(( ($hole_offset + $hole_length) / $block_size ))
That's making a big assumption about the granularity of hole
punching. It's invalid for certain XFS configurations - when using
per inode preferred allocation alignment or the real time device,
and hole punching follows those alignments.
....
I think maybe this test is trying to be too smart and do too much,
and the verbosity is hurting my eyes. I'm giving up here because I
think it overlaps greatly with test 252, and I can't see what
addition failures this test would actually detect that fsx and 252
wouldn't. If there are corner cases that 252 isn't covering that
this test does, then I think they should be added to 252....
--
Dave Chinner
[email protected]
On Thu, May 19, 2011 at 11:31:44AM +1000, Dave Chinner wrote:
> > +#
> > +# This test is successful when the following conditions are met:
> > +# - ls shows that the number of blocks occupied by the file
> > +# has decreased by the number of blocks punched out.
>
> There's no guarantee that a filesystem will punch the number of
> blocks you expect.
Cluster allocated filesystems like ocfs2 are certainly going to
punch some multiple of clusters, not exact blocks. Eg if the hole is
punched in the middle of a cluster.
Joel
--
Life's Little Instruction Book #464
"Don't miss the magic of the moment by focusing on what's
to come."
http://www.jlbec.org/
[email protected]
On 5/18/2011 6:31 PM, Dave Chinner wrote:
> On Wed, May 18, 2011 at 01:58:40PM -0700, Allison Henderson wrote:
>> This patch adds low level routines to common.punch
>> for populating test files and punching holes in them using
>> fallocate. When a hole is punched, file is then analyzed to
>> verify that the hole was punched correctly. These routines will
>> be used to test various corner cases in the new fallocate
>> punch hole tests.
>
> So what condition does this test cover that test 252 doesn't?
>
>>
>> Signed-off-by: Allison Henderson<[email protected]>
>> ---
>> :100644 100644 e2da5d8... 778389a... M common.punch
>> common.punch | 393 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 393 insertions(+), 0 deletions(-)
>>
>> diff --git a/common.punch b/common.punch
>> index e2da5d8..778389a 100644
>> --- a/common.punch
>> +++ b/common.punch
>> @@ -377,3 +377,396 @@ _test_generic_punch()
>> -c "$map_cmd -v" $testfile | $filter_cmd
>> [ $? -ne 0 ]&& die_now
>> }
>> +
>> +
>> +#_fill_file()
>> +#
>> +#Fills a file with the given byte value up to the
>> +#given number of bytes.
>> +#
>> +# $1: The name of the file to fill
>> +# $2: The byte value to fill the file with
>> +# $3: The number of bytes to put in the file
>> +# $4: The block size used when copying in
>> +# "block" sized chunks of data
>> +_fill_file() {
>> + local file_to_fill=$1
>> + local byte_value=$2
>> + local number_of_bytes=$3
>> + local blk_size=$4
>> +
>> + remaining_bytes=$number_of_bytes
>> + blk=""
>> +
>> + for (( i=0; i<blk_size; i++ ))
>> + do
>> + blk=$blk$byte_value
>> + done
>> +
>> + while [ $remaining_bytes -ge $blk_size ]
>> + do
>> + printf "$blk">> $file_to_fill
>> + remaining_bytes=$(($remaining_bytes - $blk_size))
>> + done
>> +
>> + for (( i=0; i<$remaining_bytes; i++ ))
>> + do
>> + printf "$byte_value">> $file_to_fill
>> + done
>> +
>> +
>> +}
>
> Ummm, do you really need to reinvent the wheel?
>
> $XFS_IO_PROG -F -f -c "pwrite -S $byte_value -b $blk_size 0 $number_of_bytes" $file_to_fill
>
>> +
>> +# _hole_test()
>> +#
>> +# Punches a hole in the test file.
>> +# The hole is the checked to make sure the correct number
>> +# of blocks are released and the associated extents are
>> +# removed
>> +#
>> +# Usage: _hole_test [options] file_name hole_offset hole_length
>> +# Options:
>> +# -c<length> Create a file of the given length full of data to punch a hole in
>
> rm -f $file_name
> $XFS_IO_PROG -F -f -c "t $length" $file_name
>
>> +# -p<length> Preallocate a file to the given length to punch a hole in
>
> $XFS_IO_PROG -F -f -c "falloc 0 $length" $file_name
>
>> +# -s Sync the file before punching the hole
>
> $XFS_IO_PROG -F -f -c "fsync" $file_name
>
>> +# -m Remount the device before punching the hole
> ....
>> + umount $TEST_DEV
>> + mount $TEST_DEV $TEST_DIR
>
> If you need remounts during the test, it should be using the scratch
> device, I think.
>
>> +#
>> +# This test is successful when the following conditions are met:
>> +# - ls shows that the number of blocks occupied by the file
>> +# has decreased by the number of blocks punched out.
>
> There's no guarantee that a filesystem will punch the number of
> blocks you expect.
>
>> +# - df shows number of available blocks has increased by the number
>> +# of blocks punched out
>
> Ditto - indeed, punching blocks can lead to allocation for things
> like extent tree splits because the number of extent records
> increases.
>
>> +# - File frag shows that the blocks punched out are absent from
>> +# the extents
>
> Probably the best method, though we tend to use the xfs_io output
> because we already have a lot of parsing functions for it.
>
>> +# - The test file contains zeros where the hole should be
>> +#
>> +_hole_test() {
>> +
>> + err=0
>> + sflag=
>> + mflag=
>> + pflag=
>> + cflag=
>> + lflag=
>> + oflag=
>> + OPTIND=1
>> + while getopts 'smc:p:' OPTION
>> + do
>> + case $OPTION in
>> + s) sflag=1
>> + ;;
>> + m) mflag=1
>> + ;;
>> + p) pflag="$OPTARG"
>> + ;;
>> + c) cflag="$OPTARG"
>> + ;;
>> + ?) echo Invalid flag
>> + echo "Usage: $(basename $0): [options] file_name hole_offset hole_length">&2
>> + echo "Options:">&2
>> + echo "-c<length> Create a file of the given length full of data to punch a hole in">&2
>> + echo "-p<length> Preallocate a file to the given length to punch a hole in">&2
>> + echo "-s Sync the file before punching the hole">&2
>> + echo "-m Remount the device before punching the hole"
>> + exit 1
>> + ;;
>> + esac
>> + done
>> + shift $(($OPTIND - 1))
>> +
>> + local file_name=$1
>> + local hole_offset=$2
>> + local hole_length=$3
>> + local hole_offset_orig=$hole_offset
>> + local hole_length_orig=$hole_length
>> +
>> + if [ "$cflag" ]; then
>> + local file_length=$cflag
>> + elif [ "$pflag" ]; then
>> + local file_length=$pflag
>> + else
>> + local file_length=`ls -ls $file_name | cut -d ' ' -f6`
>> + fi
>> +
>> + # If the hole to punch out extends off the edge of the file,
>> + # then we need to expect a smaller hole to be punched
>> + if [ $hole_offset -ge $file_length ]; then
>> + local hole_length=0
>> + local hole_offset=$file_length
>> + elif [ $(($hole_offset + $hole_length)) -gt $file_length ]; then
>> + local hole_length=$(($file_length - $hole_offset))
>> + fi
>> +
>> + # locations to store hexdumps must not be in the filesystem
>> + # or it will skew the results. When we measure used
>> + # available blocks. Also, there may not be enough
>> + # room to store them in the fs during ENOSPC tests
>> + # So store the dumps in the cwd by stripping the path
>> + expected=`echo $file_name.hexdump.expected | sed 's/.*\///'`
>> + result=`echo $file_name.hexdump.result | sed 's/.*\///'`
>
> This is why you shoul dbe using the scratch device and using the
> test device to store the results. No hoops to jump through, and the
> result is s simpler test.
>
> Oh, and all you do is run diff on the hexdump output of the files.
> diff can check binary files just fine - why do you need the hexdump
> output? i.e. create files with zeros where the holes will be on the
> testdev, create new ones on the scratch dev, punch holes in them,
> diff them. No need for hexdumps, and the diff output can be put
> directly into the golden output. If we then dump the fiemap output
> into the output file (appropriately filtered) no further validation
> checks are needed to determine that the hole was punched in the
> correct place....
>
> Indeed, I've seen plenty of cases where same test case operating on
> an existing file gives different results to operating on a newly
> created file....
>
>> +
>> + #calculate the end points of the hole in blocks
>> + local block_size=`stat -f $TEST_DEV | grep "Block size" | cut -d " " -f3`
>> + local hole_first_blk=$(( $hole_offset / $block_size ))
>> + if [ $(($hole_offset % $block_size)) != 0 ]
>> + then
>> + local hole_first_blk=$(( $hole_first_blk + 1 ))
>> + fi
>> +
>> + local hole_last_blk=$(( ($hole_offset + $hole_length) / $block_size ))
>
> That's making a big assumption about the granularity of hole
> punching. It's invalid for certain XFS configurations - when using
> per inode preferred allocation alignment or the real time device,
> and hole punching follows those alignments.
>
> ....
>
> I think maybe this test is trying to be too smart and do too much,
> and the verbosity is hurting my eyes. I'm giving up here because I
> think it overlaps greatly with test 252, and I can't see what
> addition failures this test would actually detect that fsx and 252
> wouldn't. If there are corner cases that 252 isn't covering that
> this test does, then I think they should be added to 252....
>
>
Hi there,
Thx for the all the reviewing on this one. Im not quite sure I agree
that the tests are analogous though. I did some poking around in
xfsprogs which I believe is what test 252 is using to do perform most of
its operations. I found the code where the hole gets punched, but I
didnt find any code that does any kind of analyzing to verify that the
hole was punched correctly. Maybe I over looked it? It kinda looks
like the hole gets punched and it just checks the return code (fpunch_f
in io/prealloc.c right?).
The reason this concerns me is that because a lot of the bugs that I
worked out during development did not show up in the form of a bad
return code or kernel oops. Initially the tests were not automated as
they are now. They would just perform the operations and print out info
about the file, the fs, fragmentation etc, and I would just go through
the raw output to make sure that every thing added up, as well as just
looking for anything that was out of the ordinary. To be honest, I feel
that I caught a lot more bugs before they started just with a careful
eye, than if I had just been watching return codes. The above routine
was meant to automate that work for xfstests, but sense I do not see
anything in xfstests or xfsprogs that is doing any kinda of analyzing, I
cannot say that I think removing this layer provides the same level of
verification.
Unfortunately it does sound like a lot of what is going would not work
on all file systems, but I would feel better if we at least kept the
hexdumps. The reason I'm diffing hexdumps in here is because some files
get quite large and can take a while to copy, but if they are full of
repeating data the hexdumps are small. They can be placed in the golden
output just the same I suppose. As much as I would like to include
output about the extents, I do not know how that would work since the
file may be inherently fragmented differently from test to test. Unless
maybe we did something where we just count up blocks from continuous
extents just to show where the holes are. What do you think? Thx!
Allison Henderson
On 05/19/2011 11:26 AM, Allison Henderson wrote:
> Thx for the all the reviewing on this one. Im not quite sure I agree that the tests are analogous though. I did some poking around in xfsprogs which I believe is what test 252 is using to do perform most of its operations. I found the code where the hole gets punched, but I didnt find any code that does any kind of analyzing to verify that the hole was punched correctly. Maybe I over looked it? It kinda looks like the hole gets punched and it just checks the return code (fpunch_f in io/prealloc.c right?).
>
Mark Fasheh wrote a test for the same for ocfs2 that might help. Check it out.
http://oss.oracle.com/git/?p=ocfs2-test.git;a=tree;f=programs/fill_verify_holes;h=0df38895e7ff6e66a9a558f35407699ebb40f790;hb=HEAD
On Thu, May 19, 2011 at 11:26:23AM -0700, Allison Henderson wrote:
> On 5/18/2011 6:31 PM, Dave Chinner wrote:
> >On Wed, May 18, 2011 at 01:58:40PM -0700, Allison Henderson wrote:
> >>This patch adds low level routines to common.punch
> >>for populating test files and punching holes in them using
> >>fallocate. When a hole is punched, file is then analyzed to
> >>verify that the hole was punched correctly. These routines will
> >>be used to test various corner cases in the new fallocate
> >>punch hole tests.
> >
> >So what condition does this test cover that test 252 doesn't?
....
> >....
> >
> >I think maybe this test is trying to be too smart and do too much,
> >and the verbosity is hurting my eyes. I'm giving up here because I
> >think it overlaps greatly with test 252, and I can't see what
> >addition failures this test would actually detect that fsx and 252
> >wouldn't. If there are corner cases that 252 isn't covering that
> >this test does, then I think they should be added to 252....
> >
> >
>
>
> Hi there,
>
> Thx for the all the reviewing on this one. Im not quite sure I
> agree that the tests are analogous though. I did some poking around
> in xfsprogs which I believe is what test 252 is using to do perform
> most of its operations. I found the code where the hole gets
> punched, but I didnt find any code that does any kind of analyzing
> to verify that the hole was punched correctly.
It's in the golden output - we use the "$map_cmd" output and filter
it to the normalise the data/unwritten/hole pattern, and if that
doesn't match the golden output, the test will fail. So it does
indeed test that the hole is exactly where we expect it to be, just
without needing the complex logic.
> Maybe I over looked
> it? It kinda looks like the hole gets punched and it just checks
> the return code (fpunch_f in io/prealloc.c right?).
For the immediate exectution of the punch, yes. That code doesn't do
the checking that there is a hole in the right place, though, which
is what the golden output checks at the end of the test do.
> The reason this concerns me is that because a lot of the bugs that I
> worked out during development did not show up in the form of a bad
> return code or kernel oops. Initially the tests were not automated
> as they are now. They would just perform the operations and print
> out info about the file, the fs, fragmentation etc, and I would just
> go through the raw output to make sure that every thing added up, as
> well as just looking for anything that was out of the ordinary. To
> be honest, I feel that I caught a lot more bugs before they started
> just with a careful eye, than if I had just been watching return
> codes. The above routine was meant to automate that work for
> xfstests, but sense I do not see anything in xfstests or xfsprogs
> that is doing any kinda of analyzing, I cannot say that I think
> removing this layer provides the same level of verification.
Looking through the raw output in an automated fashion is exactly
what the filter and golden image checks do.
> Unfortunately it does sound like a lot of what is going would not
> work on all file systems, but I would feel better if we at least
> kept the hexdumps. The reason I'm diffing hexdumps in here is
> because some files get quite large and can take a while to copy, but
> if they are full of repeating data the hexdumps are small.
You've got to read the files to produce hexdumps, md5sums or just
plain diff the binary files, so there's no saving in what you
propose anyway. IMO, it's just a poor way to compare two files. You
should have a golden image, and compare the test file against the
golden image. You don't need hexdump to do that, and the files. And
given the files are small, there is no reason not to copy stuff
around.
The only thing that 252 does not do is compare the file contents to
determine whether the zeros start and stop at the correct spot.
Realistically, fsx will do a much better job of testing these corner
cases from a data POV than any manual test could ever do. Hence
it's quite valid to make the assumption that we don't need to test
the data for zeros because we have other tests that do a better job
of it...
Remember that robust unit testing is not based around the concept of
"we need to check everything in one test". It's based around the
concept that a test should be as simple as possible and test one
thing. Then you write another simple test to cover a different
aspect of the same functionality, and the test suite as a whole then
covers all the functionality needing to be verified. fsx will cover
the "user data being zeroed correctly" cases, test 252 covers the
"hole being punched in the right place" cases. IOWs, there isn't One
True Test to test hole punching is working correctly - it's the
coverage provided by the entire suite that gives us the confidence
that hole punching is working correctly.
> They can
> be placed in the golden output just the same I suppose. As much as
> I would like to include output about the extents, I do not know how
> that would work since the file may be inherently fragmented
> differently from test to test.
That's the problem that the map output (extent) filtering fixes.
And FWIW, there's a bunch of interesting hole punching tests in the
DMAPI part of xfstests that is not normally run on mainline kernels
(no dmapi support) because punching out extents is a primary
functionality of a HSM and, as I've said before, a common place to
find data corruption bugs. IOWs, XFS has had hole punch test
coverage for a lot longer than the recent test cases have been
around. Those are tests 145, 161, 175, 176 and 185. If we want more
robust hole punch coverage, taking those and making them run without
needing dmapi or XFS specific interfaces would be a good place to
start. We don't need to re-invent the wheel just to have generic
tests....
Cheers,
Dave.
--
Dave Chinner
[email protected]
On 5/19/2011 4:56 PM, Dave Chinner wrote:
> On Thu, May 19, 2011 at 11:26:23AM -0700, Allison Henderson wrote:
>> On 5/18/2011 6:31 PM, Dave Chinner wrote:
>>> On Wed, May 18, 2011 at 01:58:40PM -0700, Allison Henderson wrote:
>>>> This patch adds low level routines to common.punch
>>>> for populating test files and punching holes in them using
>>>> fallocate. When a hole is punched, file is then analyzed to
>>>> verify that the hole was punched correctly. These routines will
>>>> be used to test various corner cases in the new fallocate
>>>> punch hole tests.
>>>
>>> So what condition does this test cover that test 252 doesn't?
> ....
>>> ....
>>>
>>> I think maybe this test is trying to be too smart and do too much,
>>> and the verbosity is hurting my eyes. I'm giving up here because I
>>> think it overlaps greatly with test 252, and I can't see what
>>> addition failures this test would actually detect that fsx and 252
>>> wouldn't. If there are corner cases that 252 isn't covering that
>>> this test does, then I think they should be added to 252....
>>>
>>>
>>
>>
>> Hi there,
>>
>> Thx for the all the reviewing on this one. Im not quite sure I
>> agree that the tests are analogous though. I did some poking around
>> in xfsprogs which I believe is what test 252 is using to do perform
>> most of its operations. I found the code where the hole gets
>> punched, but I didnt find any code that does any kind of analyzing
>> to verify that the hole was punched correctly.
>
> It's in the golden output - we use the "$map_cmd" output and filter
> it to the normalise the data/unwritten/hole pattern, and if that
> doesn't match the golden output, the test will fail. So it does
> indeed test that the hole is exactly where we expect it to be, just
> without needing the complex logic.
>
>> Maybe I over looked
>> it? It kinda looks like the hole gets punched and it just checks
>> the return code (fpunch_f in io/prealloc.c right?).
>
> For the immediate exectution of the punch, yes. That code doesn't do
> the checking that there is a hole in the right place, though, which
> is what the golden output checks at the end of the test do.
>
>> The reason this concerns me is that because a lot of the bugs that I
>> worked out during development did not show up in the form of a bad
>> return code or kernel oops. Initially the tests were not automated
>> as they are now. They would just perform the operations and print
>> out info about the file, the fs, fragmentation etc, and I would just
>> go through the raw output to make sure that every thing added up, as
>> well as just looking for anything that was out of the ordinary. To
>> be honest, I feel that I caught a lot more bugs before they started
>> just with a careful eye, than if I had just been watching return
>> codes. The above routine was meant to automate that work for
>> xfstests, but sense I do not see anything in xfstests or xfsprogs
>> that is doing any kinda of analyzing, I cannot say that I think
>> removing this layer provides the same level of verification.
>
> Looking through the raw output in an automated fashion is exactly
> what the filter and golden image checks do.
>
>> Unfortunately it does sound like a lot of what is going would not
>> work on all file systems, but I would feel better if we at least
>> kept the hexdumps. The reason I'm diffing hexdumps in here is
>> because some files get quite large and can take a while to copy, but
>> if they are full of repeating data the hexdumps are small.
>
> You've got to read the files to produce hexdumps, md5sums or just
> plain diff the binary files, so there's no saving in what you
> propose anyway. IMO, it's just a poor way to compare two files. You
> should have a golden image, and compare the test file against the
> golden image. You don't need hexdump to do that, and the files. And
> given the files are small, there is no reason not to copy stuff
> around.
>
> The only thing that 252 does not do is compare the file contents to
> determine whether the zeros start and stop at the correct spot.
> Realistically, fsx will do a much better job of testing these corner
> cases from a data POV than any manual test could ever do. Hence
> it's quite valid to make the assumption that we don't need to test
> the data for zeros because we have other tests that do a better job
> of it...
>
> Remember that robust unit testing is not based around the concept of
> "we need to check everything in one test". It's based around the
> concept that a test should be as simple as possible and test one
> thing. Then you write another simple test to cover a different
> aspect of the same functionality, and the test suite as a whole then
> covers all the functionality needing to be verified. fsx will cover
> the "user data being zeroed correctly" cases, test 252 covers the
> "hole being punched in the right place" cases. IOWs, there isn't One
> True Test to test hole punching is working correctly - it's the
> coverage provided by the entire suite that gives us the confidence
> that hole punching is working correctly.
>
>> They can
>> be placed in the golden output just the same I suppose. As much as
>> I would like to include output about the extents, I do not know how
>> that would work since the file may be inherently fragmented
>> differently from test to test.
>
> That's the problem that the map output (extent) filtering fixes.
>
> And FWIW, there's a bunch of interesting hole punching tests in the
> DMAPI part of xfstests that is not normally run on mainline kernels
> (no dmapi support) because punching out extents is a primary
> functionality of a HSM and, as I've said before, a common place to
> find data corruption bugs. IOWs, XFS has had hole punch test
> coverage for a lot longer than the recent test cases have been
> around. Those are tests 145, 161, 175, 176 and 185. If we want more
> robust hole punch coverage, taking those and making them run without
> needing dmapi or XFS specific interfaces would be a good place to
> start. We don't need to re-invent the wheel just to have generic
> tests....
>
> Cheers,
>
> Dave.
Hi Dave,
Thanks for all the explaining, I did not initially see how the tests
were verifying the holes, but I think that is a sufficient test then. I
am ok with just adding the non overlapping tests, and I will take a peek
at those older tests.
Also, there was one more test that I meant to be a part of this
collection, but I was not finished with it at the time I submitted the
patch for feedback. Basically it checks to see if a hole can still be
punched out when the disk is full. In ext4 this is allowable because
reserved space is used to allow the operation to proceed where it would
have otherwise failed. I'm not sure if this is also ext4 specific
though. Would this be another candidate for adding to 252? Thx!
Allison Henderson
On 5/19/2011 6:22 PM, Allison Henderson wrote:
> On 5/19/2011 4:56 PM, Dave Chinner wrote:
>> On Thu, May 19, 2011 at 11:26:23AM -0700, Allison Henderson wrote:
>>> On 5/18/2011 6:31 PM, Dave Chinner wrote:
>>>> On Wed, May 18, 2011 at 01:58:40PM -0700, Allison Henderson wrote:
>>>>> This patch adds low level routines to common.punch
>>>>> for populating test files and punching holes in them using
>>>>> fallocate. When a hole is punched, file is then analyzed to
>>>>> verify that the hole was punched correctly. These routines will
>>>>> be used to test various corner cases in the new fallocate
>>>>> punch hole tests.
>>>>
>>>> So what condition does this test cover that test 252 doesn't?
>> ....
>>>> ....
>>>>
>>>> I think maybe this test is trying to be too smart and do too much,
>>>> and the verbosity is hurting my eyes. I'm giving up here because I
>>>> think it overlaps greatly with test 252, and I can't see what
>>>> addition failures this test would actually detect that fsx and 252
>>>> wouldn't. If there are corner cases that 252 isn't covering that
>>>> this test does, then I think they should be added to 252....
>>>>
>>>>
>>>
>>>
>>> Hi there,
>>>
>>> Thx for the all the reviewing on this one. Im not quite sure I
>>> agree that the tests are analogous though. I did some poking around
>>> in xfsprogs which I believe is what test 252 is using to do perform
>>> most of its operations. I found the code where the hole gets
>>> punched, but I didnt find any code that does any kind of analyzing
>>> to verify that the hole was punched correctly.
>>
>> It's in the golden output - we use the "$map_cmd" output and filter
>> it to the normalise the data/unwritten/hole pattern, and if that
>> doesn't match the golden output, the test will fail. So it does
>> indeed test that the hole is exactly where we expect it to be, just
>> without needing the complex logic.
>>
>>> Maybe I over looked
>>> it? It kinda looks like the hole gets punched and it just checks
>>> the return code (fpunch_f in io/prealloc.c right?).
>>
>> For the immediate exectution of the punch, yes. That code doesn't do
>> the checking that there is a hole in the right place, though, which
>> is what the golden output checks at the end of the test do.
>>
>>> The reason this concerns me is that because a lot of the bugs that I
>>> worked out during development did not show up in the form of a bad
>>> return code or kernel oops. Initially the tests were not automated
>>> as they are now. They would just perform the operations and print
>>> out info about the file, the fs, fragmentation etc, and I would just
>>> go through the raw output to make sure that every thing added up, as
>>> well as just looking for anything that was out of the ordinary. To
>>> be honest, I feel that I caught a lot more bugs before they started
>>> just with a careful eye, than if I had just been watching return
>>> codes. The above routine was meant to automate that work for
>>> xfstests, but sense I do not see anything in xfstests or xfsprogs
>>> that is doing any kinda of analyzing, I cannot say that I think
>>> removing this layer provides the same level of verification.
>>
>> Looking through the raw output in an automated fashion is exactly
>> what the filter and golden image checks do.
>>
>>> Unfortunately it does sound like a lot of what is going would not
>>> work on all file systems, but I would feel better if we at least
>>> kept the hexdumps. The reason I'm diffing hexdumps in here is
>>> because some files get quite large and can take a while to copy, but
>>> if they are full of repeating data the hexdumps are small.
>>
>> You've got to read the files to produce hexdumps, md5sums or just
>> plain diff the binary files, so there's no saving in what you
>> propose anyway. IMO, it's just a poor way to compare two files. You
>> should have a golden image, and compare the test file against the
>> golden image. You don't need hexdump to do that, and the files. And
>> given the files are small, there is no reason not to copy stuff
>> around.
>>
>> The only thing that 252 does not do is compare the file contents to
>> determine whether the zeros start and stop at the correct spot.
>> Realistically, fsx will do a much better job of testing these corner
>> cases from a data POV than any manual test could ever do. Hence
>> it's quite valid to make the assumption that we don't need to test
>> the data for zeros because we have other tests that do a better job
>> of it...
>>
>> Remember that robust unit testing is not based around the concept of
>> "we need to check everything in one test". It's based around the
>> concept that a test should be as simple as possible and test one
>> thing. Then you write another simple test to cover a different
>> aspect of the same functionality, and the test suite as a whole then
>> covers all the functionality needing to be verified. fsx will cover
>> the "user data being zeroed correctly" cases, test 252 covers the
>> "hole being punched in the right place" cases. IOWs, there isn't One
>> True Test to test hole punching is working correctly - it's the
>> coverage provided by the entire suite that gives us the confidence
>> that hole punching is working correctly.
>>
>>> They can
>>> be placed in the golden output just the same I suppose. As much as
>>> I would like to include output about the extents, I do not know how
>>> that would work since the file may be inherently fragmented
>>> differently from test to test.
>>
>> That's the problem that the map output (extent) filtering fixes.
>>
>> And FWIW, there's a bunch of interesting hole punching tests in the
>> DMAPI part of xfstests that is not normally run on mainline kernels
>> (no dmapi support) because punching out extents is a primary
>> functionality of a HSM and, as I've said before, a common place to
>> find data corruption bugs. IOWs, XFS has had hole punch test
>> coverage for a lot longer than the recent test cases have been
>> around. Those are tests 145, 161, 175, 176 and 185. If we want more
>> robust hole punch coverage, taking those and making them run without
>> needing dmapi or XFS specific interfaces would be a good place to
>> start. We don't need to re-invent the wheel just to have generic
>> tests....
>>
>> Cheers,
>>
>> Dave.
>
>
> Hi Dave,
>
> Thanks for all the explaining, I did not initially see how the tests
> were verifying the holes, but I think that is a sufficient test then. I
> am ok with just adding the non overlapping tests, and I will take a peek
> at those older tests.
>
> Also, there was one more test that I meant to be a part of this
> collection, but I was not finished with it at the time I submitted the
> patch for feedback. Basically it checks to see if a hole can still be
> punched out when the disk is full. In ext4 this is allowable because
> reserved space is used to allow the operation to proceed where it would
> have otherwise failed. I'm not sure if this is also ext4 specific
> though. Would this be another candidate for adding to 252? Thx!
>
> Allison Henderson
>
Hi again,
I just didnt want this question to get washed away in the traffic. I am
working on an updated patch set, should I include the extra test case? Thx!
Allison Henderson
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 20, 2011 at 05:46:19PM -0700, Allison Henderson wrote:
> On 5/19/2011 6:22 PM, Allison Henderson wrote:
> >Also, there was one more test that I meant to be a part of this
> >collection, but I was not finished with it at the time I submitted the
> >patch for feedback. Basically it checks to see if a hole can still be
> >punched out when the disk is full. In ext4 this is allowable because
> >reserved space is used to allow the operation to proceed where it would
> >have otherwise failed. I'm not sure if this is also ext4 specific
> >though. Would this be another candidate for adding to 252? Thx!
>
> I just didnt want this question to get washed away in the traffic.
> I am working on an updated patch set, should I include the extra
> test case? Thx!
Yes, though probably not in the _generic_test_punch function. And
extra case specific to 252 that does something like:
umount SCRATCH_DEV
make a small filesystem
scratch_mount
prealloc to ENOSPC
punch
Cheers,
Dave.
--
Dave Chinner
[email protected]
On 5/20/2011 8:57 PM, Dave Chinner wrote:
> On Fri, May 20, 2011 at 05:46:19PM -0700, Allison Henderson wrote:
>> On 5/19/2011 6:22 PM, Allison Henderson wrote:
>>> Also, there was one more test that I meant to be a part of this
>>> collection, but I was not finished with it at the time I submitted the
>>> patch for feedback. Basically it checks to see if a hole can still be
>>> punched out when the disk is full. In ext4 this is allowable because
>>> reserved space is used to allow the operation to proceed where it would
>>> have otherwise failed. I'm not sure if this is also ext4 specific
>>> though. Would this be another candidate for adding to 252? Thx!
>>
>> I just didnt want this question to get washed away in the traffic.
>> I am working on an updated patch set, should I include the extra
>> test case? Thx!
>
> Yes, though probably not in the _generic_test_punch function. And
> extra case specific to 252 that does something like:
>
> umount SCRATCH_DEV
> make a small filesystem
> scratch_mount
> prealloc to ENOSPC
> punch
>
>
> Cheers,
>
> Dave.
Thx Dave, I will include include it once I get it working in the
xfstests frame work. The code for it though ended up being a little
more complex than what you have there. This one might be easier to talk
about with the code in front of us, but I will trying to sum it up quickly:
Because punching a hole does not always require extra blocks, it has to
go through a couple rounds of punching holes, and then "topping off" the
file system to 100% usage before it is forced to grow the tree in order
to deal with the fragmentation. The growing of the tree is what would
have triggered the ENOSPC on the next punch if not for the use of
reserved blocks. Before this feature was in place, this magic number
appeared to be about 333 iterations for ext4. Once we added it though,
it was able to run through an indefinite cycle of punching out every
other two blocks and topping off the fs (at least until it ran off the
edge of the file it is punching away at). The test case I have calls it
good after 500, but this may be something we may need to tweek in order
for it to be effective for other file systems too.
On a side note, I've hit a bit of a snag at the moment, because it
appears that test 252 hangs when run on ext4. It looks like the call to
get the fiemap doesnt come back for some reason, so I will need to
figure this out first, but when I get it all working, I will get the
updated set out to you asap. Thx for all your help, Dave. I really
appreciate all the thorough reviews! :)
Allison Henderson