2011-06-28 14:44:59

by Allison Henderson

[permalink] [raw]
Subject: [PATCH 0/2v v7] XFS TESTS: ENOSPC Punch Hole Test

Hi All,

This is another set I sent out a while ago, but I didnt see it show up on the lists,
so I am resending this one too. The work in this patch is a continuation from a
previous patch set that has been partially accepted, so I thought I
should retain the v6.

This patch set adds the ENOSPC test that was used for ext4 punch hole testing.
This test will verify that a hole can be punched even when the disk is full.
Reserved blocks should be used to complete the operation when there is not blocks
to further fragment the file.

Because punching a hole does not always require extra blocks, there needs to
be serveal iterations of punching holes, and then filling the file system to 100%
usage before it is forced to grow the tree in order to handle the fragmentation.
The growing of the tree is what would cause ENOSPC if not for the use of reserved blocks.

I could use some opinions on this patch set becuase I am not sure if other filesystems
handle their punch holes in the same way. Although xfs appears to pass the test,
should this test be an ext4 only test? Thx!

Allison Henderson (2):
XFS TESTS: ENOSPC Punch Hole: Move su routines in 123 to common.rc
XFS TESTS: Add ENOSPC Hole Punch Test

123 | 24 --------
255 | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
255.out | 1 +
common.rc | 20 +++++++
group | 1 +
5 files changed, 200 insertions(+), 24 deletions(-)
create mode 100644 255
create mode 100644 255.out



2011-06-28 14:43:09

by Allison Henderson

[permalink] [raw]
Subject: [PATCH 1/2 v7] XFS TESTS: ENOSPC Punch Hole: Move su routines in 123 to common.rc

This patch moves the su routines in test 123 to common.rc
so that they can also be used in the new test 255

Signed-off-by: Allison Henderson <[email protected]>
---
:100755 100755 27c1e66... ee1194d... M 123
:100644 100644 680631d... 9d68574... M common.rc
123 | 24 ------------------------
common.rc | 20 ++++++++++++++++++++
2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/123 b/123
index 27c1e66..ee1194d 100755
--- a/123
+++ b/123
@@ -42,30 +42,6 @@ _cleanup()
_cleanup_testdir
}

-
-_filter_user_do()
-{
- perl -ne "
-s,.*Permission\sdenied.*,Permission denied,;
-s,.*no\saccess\sto\stty.*,,;
-s,.*no\sjob\scontrol\sin\sthis\sshell.*,,;
-s,^\s*$,,;
- print;"
-}
-
-
-
-
-_user_do()
-{
- if [ "$HOSTOS" == "IRIX" ]
- then
- echo $1 | /bin/bash "su $qa_user 2>&1" | _filter_user_do
- else
- echo $1 | su $qa_user 2>&1 | _filter_user_do
- fi
-}
-
# get standard environment, filters and checks
. ./common.rc
. ./common.filter
diff --git a/common.rc b/common.rc
index 680631d..9d68574 100644
--- a/common.rc
+++ b/common.rc
@@ -867,6 +867,26 @@ _require_user()
[ "$?" == "0" ] || _notrun "$qa_user user not defined."
}

+_filter_user_do()
+{
+ perl -ne "
+s,.*Permission\sdenied.*,Permission denied,;
+s,.*no\saccess\sto\stty.*,,;
+s,.*no\sjob\scontrol\sin\sthis\sshell.*,,;
+s,^\s*$,,;
+ print;"
+}
+
+_user_do()
+{
+ if [ "$HOSTOS" == "IRIX" ]
+ then
+ echo $1 | /bin/bash "su $qa_user 2>&1" | _filter_user_do
+ else
+ echo $1 | su $qa_user 2>&1 | _filter_user_do
+ fi
+}
+
# check that xfs_io, glibc, kernel, and filesystem all (!) support
# fallocate
#
--
1.7.1


2011-06-28 14:45:01

by Allison Henderson

[permalink] [raw]
Subject: [PATCH 2/2 v7] XFS TESTS: Add ENOSPC Hole Punch Test

This patch adds a new test 255 that tests that a hole can be punched even when the
disk is full. Reserved blocks should be used to allow a punch hole to proceed even
when there is not enough blocks to further fragment the file. To test this, the
file system is fragmented by punching holes in regular intervals and filling
the file system between punches. This will eventually force the file system to use
reserved blocks to proceed with the punch hole operation.

The work in this patch is a continuation from a previous patch set that has been
partially accepted.

Signed-off-by: Allison Henderson <[email protected]>
---
v5->v6

Test moved out of 252 and put in its own test 255

_fill_fs and _test_full_fs_punch have been moved from common.punch
to test 255 and modified to use the _user_do routines in common.rc

_fill_fs has been optimized to stop once files smaller than a block
cannot be created.

v6->v7
Fixed bad file add


:000000 100644 0000000... 3d39fdb... A 255
:000000 100644 0000000... 3525403... A 255.out
:100644 100644 1f86075... c045e70... M group
255 | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
255.out | 1 +
group | 1 +
3 files changed, 180 insertions(+), 0 deletions(-)

diff --git a/255 b/255
new file mode 100644
index 0000000..3d39fdb
--- /dev/null
+++ b/255
@@ -0,0 +1,178 @@
+#! /bin/bash
+# FS QA Test No. 255
+#
+# Test Full File System Hole Punching
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2011 IBM Corporation. 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
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ rm -f $tmp.*
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.punch
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+
+_require_xfs_io_falloc_punch
+_require_scratch
+_require_user
+
+testfile=$TEST_DIR/255.$$
+
+
+# _fill_fs()
+#
+# Fills a file system by repeatedly creating files in the given folder
+# starting with the given file size. Files are reduced in size when
+# they can no longer fit untill no more files can be created.
+#
+# This routine is used by _test_full_fs_punch to test that a hole may
+# still be punched when the disk is full by borrowing reserved blocks.
+# All files are created as a non root user to prevent reserved blocks
+# from being consumed.
+#
+_fill_fs() {
+ local file_size=$1
+ local dir=$2
+ local block_size=$3
+ local file_count=1
+ local bytes_written=0
+
+ if [ $# -ne 3 ]
+ then
+ echo "USAGE: _fill_fs filesize dir block size"
+ exit 1
+ fi
+
+ # Creation of files or folders
+ # must not be done as root or
+ # reserved blocks will be consumed
+ _user_do "mkdir -p $dir &> /dev/null"
+ if [ $? -ne 0 ] ; then
+ return 0
+ fi
+
+ if [ $file_size -lt $block_size ]
+ then
+ $file_size = $block_size
+ fi
+
+ while [ $file_size -ge $block_size ]
+ do
+ bytes_written=0
+ _user_do "$XFS_IO_PROG -F -f -c \"pwrite 0 $file_size\" $dir/$file_count.bin &> /dev/null"
+
+ if [ -f $dir/$file_count.bin ]
+ then
+ bytes_written=`$XFS_IO_PROG -F -c "stat" $dir/$file_count.bin | grep size | cut -d ' ' -f3`
+ fi
+
+ # If there was no room to make the file,
+ # then divide it in half, and keep going
+ if [ $bytes_written -lt $file_size ]
+ then
+ file_size=$(( $file_size / 2 ))
+ fi
+ file_count=$(( $file_count + 1 ))
+
+ done
+}
+
+# _test_full_fs_punch()
+#
+# This function will test that a hole may be punched
+# even when the file system is full. Reserved blocks
+# should be used to allow a punch hole to proceed even
+# when there is not enough blocks to further fragment the
+# file. To test this, this function will fragment the file
+# system by punching holes in regular intervals and filling
+# the file system between punches.
+#
+_test_full_fs_punch()
+{
+ local hole_len=$1 # The length of the holes to punch
+ local hole_interval=$2 # The interval between the holes
+ local iterations=$3 # The number of holes to punch
+ local file_name=$4 # File to punch holes in
+ local block_size=$5 # File system block size
+ local file_len=$(( $(( $hole_len + $hole_interval )) * $iterations ))
+ local path=`dirname $file_name`
+ local hole_offset=0
+
+ if [ $# -ne 5 ]
+ then
+ echo "USAGE: _test_full_fs_punch hole_len hole_interval iterations file_name block_size"
+ exit 1
+ fi
+
+ rm -f $file_name &> /dev/null
+
+ $XFS_IO_PROG -F -f -c "pwrite 0 $file_len" \
+ -c "fsync" $file_name &> /dev/null
+ chmod 666 $file_name
+
+ _fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size
+
+ for (( i=0; i<$iterations; i++ ))
+ do
+ # This part must not be done as root in order to
+ # test that reserved blocks are used when needed
+ _user_do "$XFS_IO_PROG -F -f -c \"fpunch $hole_offset $hole_len\" $file_name"
+ rc=$?
+ if [ $? -ne 0 ] ; then
+ echo Punch hole failed
+ break
+ fi
+
+ hole_offset=$(( $hole_offset + $hole_len + $hole_interval ))
+
+ _fill_fs $hole_len $path/fill.$i $block_size
+
+ done
+}
+
+# Make a small file system to fill
+umount $SCRATCH_DEV &> /dev/null
+_scratch_mkfs_sized $(( 1024 * 1024 * 1024 )) &> /dev/null
+_scratch_mount
+# Test must be able to write files with non-root permissions
+chmod 777 $SCRATCH_MNT
+
+block_size=`stat -f $SCRATCH_DEV | grep "Block size" | cut -d " " -f3`
+_test_full_fs_punch $(( $block_size * 2 )) $block_size 500 $SCRATCH_MNT/252.$$ $block_size
+
+status=0 ; exit
diff --git a/255.out b/255.out
new file mode 100644
index 0000000..3525403
--- /dev/null
+++ b/255.out
@@ -0,0 +1 @@
+QA output created by 255
diff --git a/group b/group
index 1f86075..c045e70 100644
--- a/group
+++ b/group
@@ -368,3 +368,4 @@ deprecated
252 auto quick prealloc
253 auto quick
254 auto quick
+255 auto quick
--
1.7.1


2011-07-01 14:30:19

by Allison Henderson

[permalink] [raw]
Subject: Re: [PATCH 0/2v v7] XFS TESTS: ENOSPC Punch Hole Test

On 06/28/2011 07:44 AM, Allison Henderson wrote:
> Hi All,
>
> This is another set I sent out a while ago, but I didnt see it show up on the lists,
> so I am resending this one too. The work in this patch is a continuation from a
> previous patch set that has been partially accepted, so I thought I
> should retain the v6.
>
> This patch set adds the ENOSPC test that was used for ext4 punch hole testing.
> This test will verify that a hole can be punched even when the disk is full.
> Reserved blocks should be used to complete the operation when there is not blocks
> to further fragment the file.
>
> Because punching a hole does not always require extra blocks, there needs to
> be serveal iterations of punching holes, and then filling the file system to 100%
> usage before it is forced to grow the tree in order to handle the fragmentation.
> The growing of the tree is what would cause ENOSPC if not for the use of reserved blocks.
>
> I could use some opinions on this patch set becuase I am not sure if other filesystems
> handle their punch holes in the same way. Although xfs appears to pass the test,
> should this test be an ext4 only test? Thx!
>

Hi All,

I just wanted to poke this thread before too much time goes by. This
patch was initially part of an earlier set that's already been picked
up, and it seemed like people were generally interested in it, so I
resubmitted it as it's own patch. Is there still an interest in this
patch set?

I have another set that now also needs to add a new test 255 (the fix
252 failure patch set). I am thinking that if people still want this
test, I could put this patch in with the other set so that they stack
properly. Thx!

Allison Henderson


2011-07-14 18:22:02

by Alex Elder

[permalink] [raw]
Subject: Re: [PATCH 1/2 v7] XFS TESTS: ENOSPC Punch Hole: Move su routines in 123 to common.rc

On Tue, 2011-06-28 at 07:45 -0700, Allison Henderson wrote:
> This patch moves the su routines in test 123 to common.rc
> so that they can also be used in the new test 255
>
> Signed-off-by: Allison Henderson <[email protected]>

This looks good.

Reviewed-by: Alex Elder <[email protected]>



2011-07-14 18:22:09

by Alex Elder

[permalink] [raw]
Subject: Re: [PATCH 2/2 v7] XFS TESTS: Add ENOSPC Hole Punch Test

On Tue, 2011-06-28 at 07:45 -0700, Allison Henderson wrote:
> This patch adds a new test 255 that tests that a hole can be punched even when the
> disk is full. Reserved blocks should be used to allow a punch hole to proceed even
> when there is not enough blocks to further fragment the file. To test this, the
> file system is fragmented by punching holes in regular intervals and filling
> the file system between punches. This will eventually force the file system to use
> reserved blocks to proceed with the punch hole operation.
>
> The work in this patch is a continuation from a previous patch set that has been
> partially accepted.
>
> Signed-off-by: Allison Henderson <[email protected]>

First of all, I renumbered the test 256 (and will do
that for you if necessary before committing the change).

Second, I can confirm that this new test passes for
XFS on my setup. I don't know what the threshold for
inclusion in the "quick" group is, but it's under a
minute so that's fine with me.

Third, maybe we can create a new group "punch" or
something like that to tag tests that exercise
hole punching functionality (tests 252, 255, and
now this one at least; maybe 175 and 176 too).

Finally, I found a bug and a few other spots that
really ought to be fixed but aren't really serious
problems. I also have a number of other things that
I commented on, but tried to make it clear where
they're just remarks to consider, not requests to
make a change.

-Alex

> ---
> v5->v6
>
> Test moved out of 252 and put in its own test 255
>
> _fill_fs and _test_full_fs_punch have been moved from common.punch
> to test 255 and modified to use the _user_do routines in common.rc
>
> _fill_fs has been optimized to stop once files smaller than a block
> cannot be created.
>
> v6->v7
> Fixed bad file add
>
>
> :000000 100644 0000000... 3d39fdb... A 255
> :000000 100644 0000000... 3525403... A 255.out
> :100644 100644 1f86075... c045e70... M group
> 255 | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 255.out | 1 +
> group | 1 +
> 3 files changed, 180 insertions(+), 0 deletions(-)
>
> diff --git a/255 b/255
> new file mode 100644
> index 0000000..3d39fdb
> --- /dev/null
> +++ b/255
> @@ -0,0 +1,178 @@
> +#! /bin/bash
> +# FS QA Test No. 255
> +#
> +# Test Full File System Hole Punching
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2011 IBM Corporation. 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
> [email protected]
> +
> +seq=`basename $0`
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + rm -f $tmp.*
> +}
> +
> +trap "_cleanup ; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +. ./common.punch
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +
> +_require_xfs_io_falloc_punch
> +_require_scratch
> +_require_user
> +
> +testfile=$TEST_DIR/255.$$
testfile=$TEST_DIR/$seq.$$

> +
> +

The _fill_fs function seems like it may be useful elsewhere,
so it may end up in common.rc eventually. (Easy enough to
move it there later though.)

> +# _fill_fs()
> +#
> +# Fills a file system by repeatedly creating files in the given folder
> +# starting with the given file size. Files are reduced in size when
> +# they can no longer fit untill no more files can be created.
> +#
> +# This routine is used by _test_full_fs_punch to test that a hole may
> +# still be punched when the disk is full by borrowing reserved blocks.
> +# All files are created as a non root user to prevent reserved blocks
> +# from being consumed.
> +#
> +_fill_fs() {
> + local file_size=$1
> + local dir=$2
> + local block_size=$3
> + local file_count=1
> + local bytes_written=0
> +
> + if [ $# -ne 3 ]
> + then
> + echo "USAGE: _fill_fs filesize dir block size"
> + exit 1
> + fi
> +
> + # Creation of files or folders
> + # must not be done as root or
> + # reserved blocks will be consumed
> + _user_do "mkdir -p $dir &> /dev/null"

I personally prefer seeing "> /dev/null 2>&1" rather
than "&> /dev/null", to make it that much more
explicit that both are being redirected. This is
purely a style thing, and I say this mainly to see
if anyone else has a preference one way or another.
(This appears several times in the file, so if you
do choose to change it, do so throughout.)

> + if [ $? -ne 0 ] ; then
> + return 0
> + fi
> +
> + if [ $file_size -lt $block_size ]
> + then
> + $file_size = $block_size
file_size=$block_size

(This one is a bug.)

> + fi
> +
> + while [ $file_size -ge $block_size ]
> + do
> + bytes_written=0
> + _user_do "$XFS_IO_PROG -F -f -c \"pwrite 0 $file_size\" $dir/$file_count.bin &> /dev/null"
> +

Try to split these long lines, e.g.:
_user_do "$XFS_IO_PROG -F -f -c \
\"pwrite 0 $file_size\" \
$dir/$file_count.bin &> /dev/null"

> + if [ -f $dir/$file_count.bin ]
> + then
> + bytes_written=`$XFS_IO_PROG -F -c "stat" $dir/$file_count.bin | grep size | cut -d ' ' -f3`

Personal preference again, but as long as we're using bash
and not sh, Use $(...) rather than `...` for sub-commands.

> + fi
> +
> + # If there was no room to make the file,
> + # then divide it in half, and keep going
> + if [ $bytes_written -lt $file_size ]
> + then
> + file_size=$(( $file_size / 2 ))
> + fi
> + file_count=$(( $file_count + 1 ))
> +
> + done
> +}
> +
> +# _test_full_fs_punch()
> +#
> +# This function will test that a hole may be punched
> +# even when the file system is full. Reserved blocks
> +# should be used to allow a punch hole to proceed even
> +# when there is not enough blocks to further fragment the
> +# file. To test this, this function will fragment the file
> +# system by punching holes in regular intervals and filling
> +# the file system between punches.
> +#
> +_test_full_fs_punch()
> +{
> + local hole_len=$1 # The length of the holes to punch
> + local hole_interval=$2 # The interval between the holes
> + local iterations=$3 # The number of holes to punch
> + local file_name=$4 # File to punch holes in
> + local block_size=$5 # File system block size
> + local file_len=$(( $(( $hole_len + $hole_interval )) * $iterations ))
> + local path=`dirname $file_name`
> + local hole_offset=0
> +
> + if [ $# -ne 5 ]
> + then
> + echo "USAGE: _test_full_fs_punch hole_len hole_interval iterations file_name block_size"
> + exit 1
> + fi
> +
> + rm -f $file_name &> /dev/null
> +
> + $XFS_IO_PROG -F -f -c "pwrite 0 $file_len" \
> + -c "fsync" $file_name &> /dev/null
> + chmod 666 $file_name
> +
> + _fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size

You are specifying a file size that's equal to the filesystem
size, which I think is guaranteed to fail its first pass. Maybe
start with half of the filesystem (or even 90% of it). (I
don't know whether this is a a real issue or not though.)

Also, you could maybe use a shell variable to represent the
filesystem size, since you use it twice in this script.

> +
> + for (( i=0; i<$iterations; i++ ))
> + do
> + # This part must not be done as root in order to
> + # test that reserved blocks are used when needed
> + _user_do "$XFS_IO_PROG -F -f -c \"fpunch $hole_offset $hole_len\" $file_name"
> + rc=$?
> + if [ $? -ne 0 ] ; then
> + echo Punch hole failed
> + break
> + fi
> +
> + hole_offset=$(( $hole_offset + $hole_len + $hole_interval ))
> +
> + _fill_fs $hole_len $path/fill.$i $block_size
> +
> + done
> +}
> +
> +# Make a small file system to fillkk
> +umount $SCRATCH_DEV &> /dev/null
> +_scratch_mkfs_sized $(( 1024 * 1024 * 1024 )) &> /dev/null
> +_scratch_mount
> +# Test must be able to write files with non-root permissions
> +chmod 777 $SCRATCH_MNT
> +
> +block_size=`stat -f $SCRATCH_DEV | grep "Block size" | cut -d " " -f3`
> +_test_full_fs_punch $(( $block_size * 2 )) $block_size 500 $SCRATCH_MNT/252.$$ $block_size
Use $SCRATCH_MNT/$seq.$$

...and possibly define this path in a variable
at the top of the file.

> +
> +status=0 ; exit
> diff --git a/255.out b/255.out
> new file mode 100644
> index 0000000..3525403
> --- /dev/null
> +++ b/255.out
> @@ -0,0 +1 @@
> +QA output created by 255
> diff --git a/group b/group
> index 1f86075..c045e70 100644
> --- a/group
> +++ b/group
> @@ -368,3 +368,4 @@ deprecated
> 252 auto quick prealloc
> 253 auto quick
> 254 auto quick
> +255 auto quick




2011-07-15 01:06:51

by Allison Henderson

[permalink] [raw]
Subject: Re: [PATCH 2/2 v7] XFS TESTS: Add ENOSPC Hole Punch Test

On 07/14/2011 11:22 AM, Alex Elder wrote:
> On Tue, 2011-06-28 at 07:45 -0700, Allison Henderson wrote:
>> This patch adds a new test 255 that tests that a hole can be punched even when the
>> disk is full. Reserved blocks should be used to allow a punch hole to proceed even
>> when there is not enough blocks to further fragment the file. To test this, the
>> file system is fragmented by punching holes in regular intervals and filling
>> the file system between punches. This will eventually force the file system to use
>> reserved blocks to proceed with the punch hole operation.
>>
>> The work in this patch is a continuation from a previous patch set that has been
>> partially accepted.
>>
>> Signed-off-by: Allison Henderson<[email protected]>
>
Hi Alex,

Thx for the review! :) Comments below:


> First of all, I renumbered the test 256 (and will do
> that for you if necessary before committing the change).
>
> Second, I can confirm that this new test passes for
> XFS on my setup. I don't know what the threshold for
> inclusion in the "quick" group is, but it's under a
> minute so that's fine with me.

As I recall it was pretty quick on xfs, but over a minute
for ext4, so I will pull off the quick tag.

>
> Third, maybe we can create a new group "punch" or
> something like that to tag tests that exercise
> hole punching functionality (tests 252, 255, and
> now this one at least; maybe 175 and 176 too).
>
I think that's a good idea, I will add that in the
next version.

> Finally, I found a bug and a few other spots that
> really ought to be fixed but aren't really serious
> problems. I also have a number of other things that
> I commented on, but tried to make it clear where
> they're just remarks to consider, not requests to
> make a change.

Ah, thank you for the careful eye, I will get those
fixed up.

>
> -Alex
>
>> ---
>> v5->v6
>>
>> Test moved out of 252 and put in its own test 255
>>
>> _fill_fs and _test_full_fs_punch have been moved from common.punch
>> to test 255 and modified to use the _user_do routines in common.rc
>>
>> _fill_fs has been optimized to stop once files smaller than a block
>> cannot be created.
>>
>> v6->v7
>> Fixed bad file add
>>
>>
>> :000000 100644 0000000... 3d39fdb... A 255
>> :000000 100644 0000000... 3525403... A 255.out
>> :100644 100644 1f86075... c045e70... M group
>> 255 | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 255.out | 1 +
>> group | 1 +
>> 3 files changed, 180 insertions(+), 0 deletions(-)
>>
>> diff --git a/255 b/255
>> new file mode 100644
>> index 0000000..3d39fdb
>> --- /dev/null
>> +++ b/255
>> @@ -0,0 +1,178 @@
>> +#! /bin/bash
>> +# FS QA Test No. 255
>> +#
>> +# Test Full File System Hole Punching
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2011 IBM Corporation. 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
>> [email protected]
>> +
>> +seq=`basename $0`
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1 # failure is the default!
>> +
>> +_cleanup()
>> +{
>> + rm -f $tmp.*
>> +}
>> +
>> +trap "_cleanup ; exit \$status" 0 1 2 3 15
>> +
>> +# get standard environment, filters and checks
>> +. ./common.rc
>> +. ./common.filter
>> +. ./common.punch
>> +
>> +# real QA test starts here
>> +_supported_fs generic
>> +_supported_os Linux
>> +
>> +_require_xfs_io_falloc_punch
>> +_require_scratch
>> +_require_user
>> +
>> +testfile=$TEST_DIR/255.$$
> testfile=$TEST_DIR/$seq.$$
>
>> +
>> +
>
> The _fill_fs function seems like it may be useful elsewhere,
> so it may end up in common.rc eventually. (Easy enough to
> move it there later though.)
>
In an earlier version of the patch, this function and _test_full_fs_punch()
were in common.punch, but we moved it here since non of the other punch hole
tests were using it. If you think other tests could make use of it though,
I think it's a good idea to put it in common.rc. That way people wont have
to worry about moving it, or wonder if something went wrong in the move.

>> +# _fill_fs()
>> +#
>> +# Fills a file system by repeatedly creating files in the given folder
>> +# starting with the given file size. Files are reduced in size when
>> +# they can no longer fit untill no more files can be created.
>> +#
>> +# This routine is used by _test_full_fs_punch to test that a hole may
>> +# still be punched when the disk is full by borrowing reserved blocks.
>> +# All files are created as a non root user to prevent reserved blocks
>> +# from being consumed.
>> +#
>> +_fill_fs() {
>> + local file_size=$1
>> + local dir=$2
>> + local block_size=$3
>> + local file_count=1
>> + local bytes_written=0
>> +
>> + if [ $# -ne 3 ]
>> + then
>> + echo "USAGE: _fill_fs filesize dir block size"
>> + exit 1
>> + fi
>> +
>> + # Creation of files or folders
>> + # must not be done as root or
>> + # reserved blocks will be consumed
>> + _user_do "mkdir -p $dir&> /dev/null"
>
> I personally prefer seeing "> /dev/null 2>&1" rather
> than "&> /dev/null", to make it that much more
> explicit that both are being redirected. This is
> purely a style thing, and I say this mainly to see
> if anyone else has a preference one way or another.
> (This appears several times in the file, so if you
> do choose to change it, do so throughout.)
>
Alrighty, I personally am not partial to one way over the
other, but if it helps make the code easier for people to
look at, I am happy to put it in :)

>> + if [ $? -ne 0 ] ; then
>> + return 0
>> + fi
>> +
>> + if [ $file_size -lt $block_size ]
>> + then
>> + $file_size = $block_size
> file_size=$block_size
>
> (This one is a bug.)

Got it :) I think the rest of what you have below are good modifications
to put in. I will start working on an update. Thx again for the review!

Allison Henderson

>
>> + fi
>> +
>> + while [ $file_size -ge $block_size ]
>> + do
>> + bytes_written=0
>> + _user_do "$XFS_IO_PROG -F -f -c \"pwrite 0 $file_size\" $dir/$file_count.bin&> /dev/null"
>> +
>
> Try to split these long lines, e.g.:
> _user_do "$XFS_IO_PROG -F -f -c \
> \"pwrite 0 $file_size\" \
> $dir/$file_count.bin&> /dev/null"
>
>> + if [ -f $dir/$file_count.bin ]
>> + then
>> + bytes_written=`$XFS_IO_PROG -F -c "stat" $dir/$file_count.bin | grep size | cut -d ' ' -f3`
>
> Personal preference again, but as long as we're using bash
> and not sh, Use $(...) rather than `...` for sub-commands.
>
>> + fi
>> +
>> + # If there was no room to make the file,
>> + # then divide it in half, and keep going
>> + if [ $bytes_written -lt $file_size ]
>> + then
>> + file_size=$(( $file_size / 2 ))
>> + fi
>> + file_count=$(( $file_count + 1 ))
>> +
>> + done
>> +}
>> +
>> +# _test_full_fs_punch()
>> +#
>> +# This function will test that a hole may be punched
>> +# even when the file system is full. Reserved blocks
>> +# should be used to allow a punch hole to proceed even
>> +# when there is not enough blocks to further fragment the
>> +# file. To test this, this function will fragment the file
>> +# system by punching holes in regular intervals and filling
>> +# the file system between punches.
>> +#
>> +_test_full_fs_punch()
>> +{
>> + local hole_len=$1 # The length of the holes to punch
>> + local hole_interval=$2 # The interval between the holes
>> + local iterations=$3 # The number of holes to punch
>> + local file_name=$4 # File to punch holes in
>> + local block_size=$5 # File system block size
>> + local file_len=$(( $(( $hole_len + $hole_interval )) * $iterations ))
>> + local path=`dirname $file_name`
>> + local hole_offset=0
>> +
>> + if [ $# -ne 5 ]
>> + then
>> + echo "USAGE: _test_full_fs_punch hole_len hole_interval iterations file_name block_size"
>> + exit 1
>> + fi
>> +
>> + rm -f $file_name&> /dev/null
>> +
>> + $XFS_IO_PROG -F -f -c "pwrite 0 $file_len" \
>> + -c "fsync" $file_name&> /dev/null
>> + chmod 666 $file_name
>> +
>> + _fill_fs $(( 1024 * 1024 * 1024 )) $path/fill $block_size
>
> You are specifying a file size that's equal to the filesystem
> size, which I think is guaranteed to fail its first pass. Maybe
> start with half of the filesystem (or even 90% of it). (I
> don't know whether this is a a real issue or not though.)
>
> Also, you could maybe use a shell variable to represent the
> filesystem size, since you use it twice in this script.
>
>> +
>> + for (( i=0; i<$iterations; i++ ))
>> + do
>> + # This part must not be done as root in order to
>> + # test that reserved blocks are used when needed
>> + _user_do "$XFS_IO_PROG -F -f -c \"fpunch $hole_offset $hole_len\" $file_name"
>> + rc=$?
>> + if [ $? -ne 0 ] ; then
>> + echo Punch hole failed
>> + break
>> + fi
>> +
>> + hole_offset=$(( $hole_offset + $hole_len + $hole_interval ))
>> +
>> + _fill_fs $hole_len $path/fill.$i $block_size
>> +
>> + done
>> +}
>> +
>> +# Make a small file system to fillkk
>> +umount $SCRATCH_DEV&> /dev/null
>> +_scratch_mkfs_sized $(( 1024 * 1024 * 1024 ))&> /dev/null
>> +_scratch_mount
>> +# Test must be able to write files with non-root permissions
>> +chmod 777 $SCRATCH_MNT
>> +
>> +block_size=`stat -f $SCRATCH_DEV | grep "Block size" | cut -d " " -f3`
>> +_test_full_fs_punch $(( $block_size * 2 )) $block_size 500 $SCRATCH_MNT/252.$$ $block_size
> Use $SCRATCH_MNT/$seq.$$
>
> ...and possibly define this path in a variable
> at the top of the file.
>
>> +
>> +status=0 ; exit
>> diff --git a/255.out b/255.out
>> new file mode 100644
>> index 0000000..3525403
>> --- /dev/null
>> +++ b/255.out
>> @@ -0,0 +1 @@
>> +QA output created by 255
>> diff --git a/group b/group
>> index 1f86075..c045e70 100644
>> --- a/group
>> +++ b/group
>> @@ -368,3 +368,4 @@ deprecated
>> 252 auto quick prealloc
>> 253 auto quick
>> 254 auto quick
>> +255 auto quick
>
>
>
> --
> 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