All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Tsvetkov <alexander.tsvetkov@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: fstests@vger.kernel.org, linux-ext4@vger.kernel.org
Subject: Re: max_dir_size_kb option list
Date: Mon, 15 Dec 2014 19:06:39 +0300	[thread overview]
Message-ID: <548F070F.2050808@oracle.com> (raw)
In-Reply-To: <20141212005517.GK24183@dastard>

[-- Attachment #1: Type: text/plain, Size: 6538 bytes --]

Hello Dave,

Thank you for the review, I've updated test according to your comments

On 12/12/2014 03:55 AM, Dave Chinner wrote:
> On Thu, Dec 11, 2014 at 03:06:42PM +0300, Alexander Tsvetkov wrote:
>> Hello,
>>
>> I've prepared test for xfstests suite that runs some checks for ext4
>> mount option max_dir_size_kb introduced in Linux Kernel 3.7, could
>> someone please look on it?
>>
>> Thanks,
>> Alexander Tsvetkov
>>  From 21b1da618d0fcb4cd4666d10c41583274ed4eeed Mon Sep 17 00:00:00 2001
>> From: Alexander Tsvetkov <alexander.tsvetkov@oracle.com>
>> Date: Wed, 10 Dec 2014 15:31:02 +0300
>> Subject: [PATCH] added test for max_dir_size_kb mount option
>>
>> ---
>>   tests/ext4/309     | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/ext4/309.out |   2 +
>>   tests/ext4/group   |   3 +-
>>   3 files changed, 217 insertions(+), 1 deletion(-)
>>   create mode 100755 tests/ext4/309
>>   create mode 100755 tests/ext4/309.out
>>
>> diff --git a/tests/ext4/309 b/tests/ext4/309
>> new file mode 100755
>> index 0000000..e2f4e43
>> --- /dev/null
>> +++ b/tests/ext4/309
>> @@ -0,0 +1,213 @@
>> +#! /bin/bash
>> +# FS QA Test
>> +#
>> +# Test for mount option max_dir_size_kb
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2014 Oracle and/or its affiliates.  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
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +tmp=/tmp/$$
>> +
>> +testdir=$SCRATCH_MNT/testdir
>> +testfile=$SCRATCH_MNT/testfile
> Do not call a file on the scratch device "testdir". That name has
> specific meaning: it's the mount point for the test device. Using
> the same variable name that differs only different syntax for
> somethingon the scratch device is not a good idea.
>
>
>> +sdir=`dirname $0`
>> +sdir=`cd "$sdir"; pwd`
> $here is already set to the current xfstests run location.
>
>> +echo "QA output created by $seq"
>> +echo "Silence is golden"
>> +rm -f $seqres.full
>> +
>> +status=1 # failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup() {
>> +   if [ ! -z SCRATCH_MNT ]; then
>> +      rm -fr $SCRATCH_MNT/test*
>> +      _scratch_unmount
>> +   fi
>> +}
> No. The harness will unmount the scratch device if it was mounted.
> Unless you need to do specific cleanup fo rthe test, do not modify
> the standard cleanup function. At minimum, it still needs to remove
> all the $tmp files created during the test run.
>
>
>> +
>> +_filter_error() {
>> +   sed -n -e "s/.*\($1\).*/\"\1\"/p"
>> +}
> "_" prefix is reserved for library functions.
>
> Comments are generally required to explain the intent of regexs that
> look like line noise.
>
> 8 space tabs.
>
>> +
>> +_clear_testdir() {
>> +   dirs="$testdir *$"
>> +   for i in $dirs; do
>> +      rm -fr $i/*
>> +   done
>> +}
> I thought this was going to clean up the TEST_DIR, but as per above,
> it's actually doing stuff to the scratch device. "remove_files()"
> is good enough....
>
>> +
>> +# $1 - device
>> +# $2 - options
>> +_make_ext4fs() {
>> +   device=$1
>> +   opts=$2
>> +   umount $device 1>/dev/null 2>&1
>> +   mkfs.ext4 $opts $device 1>/dev/null 2>&1
>> +}
> _mkfs_dev
>
>> +# $1 - options
>> +# $2 - mount point
>> +_make_loopfs() {
>> +   lpf=$testfile
>> +   dd if=/dev/zero of=$lpf bs=4k count=256 1>/dev/null 2>&1
>> +   loopdev=$(losetup -f)
>> +   losetup $loopdev $lpf
>> +   mkfs.ext4 -O ^dir_index,^has_journal $loopdev 1>/dev/null 2>&1
>> +   mount -t ext4 $1 $loopdev $2
>> +}
> no need to create a loop device. mount -o loop will do what you
> want. Also, $MKFS_EXT4_PROG (or whatever the var is) shoul dbe used.
> As should _mount. And 8 space tabs.
>
>
>> +
>> +# $1 - expected limit after items creation
>> +# $2 - command to create item
>> +# $3 - where to create (testdir by default)
>> +_create_items() {
>> +    limit=$1
>> +    create_cmd=$2
>> +    dir=${3:-$testdir}
>> +    sync
>> +    echo 3 > /proc/sys/vm/drop_caches
>> +    MAX_INUM=$(((limit*1024*2)/24))
>> +    for i in $(seq 0 $MAX_INUM); do
>> +       tmp_name=`mktemp -u`
applied
> We use $tmp as the location for temporary files in tests
mktemp is used to fill test directory up to limit
>
>> +       item=`basename $tmp_name`
>> +       if [ -e $dir/$item ]; then
>> +          continue
>> +       fi
> Why?
removed
>> +       create_cmd="$2 $dir/$item 2>$tmp.out 1>/dev/null"
>> +       eval "$create_cmd"
>> +       res=$?
>> +       if [ $res -ne 0 ]; then
>> +          _filter_error "No space left on device" < $tmp.out > $tmp.out2
>> +          if [ -s $tmp.out2 ]; then
>> +             cat $tmp.out2 | tr -d '\n' >> $seqres.full
>> +          else
>> +             echo "FAIL! expected ENOSPC" | tee -a $seqres.full
> _fail?
this output is supposed to make a test failure, but as I see _fail is rather
for test setup errors
>
>> +          fi
>> +          break
>> +      fi
> This all seems rather convoluted. You're creating a tmp file to
> capture the error, then if you get an ENOSPC error you dump it to
> the debug file, otherwise you dump an error message to main output
> to cause the test to eventually fail?
agree, I've simplified this part
>> +   done
>> +   size=$(stat -c %s $dir)
>> +   size=$((size/1024))
>> +   if [ $size -gt $limit ]; then
>> +      echo "FAIL! expected dir size: $limit, actually: $size" | tee -a $seqres.full
>> +   fi
>> +   rm -f $tmp*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +
>> +# real QA test starts here
>> +
>> +_supported_fs ext4
>> +_supported_os Linux
>> +_require_scratch
> _require_loop
>
>> +
>> +LIMIT1=8
>> +LIMIT2=16
>> +
>> +_make_ext4fs $SCRATCH_DEV "-O ^dir_index,^filetype"
> _scratch_mkfs -O ^dir_index,^filetype
>
applied
> Cheers,
>
> Dave.
Thanks,
Alexander Tsvetkov

[-- Attachment #2: max_dir_size_kb.patch --]
[-- Type: text/x-patch, Size: 6714 bytes --]

>From e30cd49f5ab84c029c0b376e702caeac42f59f49 Mon Sep 17 00:00:00 2001
From: Alexander Tsvetkov <alexander.tsvetkov@oracle.com>
Date: Mon, 15 Dec 2014 18:49:42 +0300
Subject: [PATCH] added test for max_dir_size_kb mount option

---
 tests/ext4/309     | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/309.out |   2 +
 tests/ext4/group   |   1 +
 3 files changed, 181 insertions(+)
 create mode 100755 tests/ext4/309
 create mode 100755 tests/ext4/309.out

diff --git a/tests/ext4/309 b/tests/ext4/309
new file mode 100755
index 0000000..34ceb2b
--- /dev/null
+++ b/tests/ext4/309
@@ -0,0 +1,178 @@
+#! /bin/bash
+# FS QA Test
+#
+# Test for mount option max_dir_size_kb
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Oracle and/or its affiliates.  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
+#-----------------------------------------------------------------------
+#
+
+seq=$(basename $0)
+seqres=$RESULT_DIR/$seq
+tmp=/tmp/$$
+
+testdir=$SCRATCH_MNT/dir.$seq
+testfile=$SCRATCH_MNT/testfile
+
+echo "QA output created by $seq"
+echo "Silence is golden"
+rm -f $seqres.full
+
+status=1 # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -rf $tmp.*
+}
+
+remove_files() 
+{
+   dirs="$testdir $*"
+   for i in $dirs; do
+      rm -fr $i/*
+   done
+}
+
+# $1 - expected limit after items creation
+# $2 - command to create item
+# $3 - where to create (testdir by default)
+_create_items() 
+{
+    limit=$1
+    dir=${2:-$testdir}
+    MKTEMP_OPT=""
+    [ "$3" = "mkdir" ] && MKTEMP_OPT="-d"
+    sync
+    echo 3 > /proc/sys/vm/drop_caches
+    MAX_INUM=$((limit * 1024 * 2 / 24))
+    for i in $(seq 0 $MAX_INUM); do
+       error=$(mktemp $MKTEMP_OPT --tmpdir=$dir 2>&1 >/dev/null)
+       res=$?
+       if [ $res -ne 0 ]; then
+          echo $error >> $seqres.full  
+          [[ ! $error =~ ^.*'No space left on device'$ ]] && echo "FAIL! expected ENOSPC" | tee -a $seqres.full
+          break
+      fi
+   done
+   size=$(stat -c %s $dir)
+   size=$((size / 1024))
+   if [ $size -gt $limit ]; then
+      echo "FAIL! expected dir size: $limit, actually: $size" | tee -a $seqres.full
+   fi
+}
+
+run_test()
+{
+   LIMIT1=$1
+   LIMIT2=$2
+   MKFS_OPT=$3
+
+   _scratch_unmount >/dev/null 2>&1
+   _scratch_mkfs $MKFS_OPT >>$seqres.full 2>&1
+   _scratch_mount -o max_dir_size_kb=$LIMIT1
+   mkdir $testdir
+
+   echo -e "\nExceed $LIMIT1 Kb limit with new files in testdir/: " >> $seqres.full
+   _create_items $LIMIT1
+
+   echo -e "\nRemount with $LIMIT1 Kb limit,\nnew item in testdir/ should result to ENOSPC: " >>$seqres.full
+   _scratch_mount "-o remount,max_dir_size_kb=$LIMIT1"
+   _create_items $LIMIT1
+
+   echo -e "\nExceed $LIMIT2 Kb limit with new files in testdir/: " >> $seqres.full
+   _scratch_mount "-o remount,max_dir_size_kb=$LIMIT2"
+   _create_items $LIMIT2
+
+   echo -e "\nExceed $LIMIT2 Kb limit with new files in testdir2/: " >> $seqres.full
+   mkdir $SCRATCH_MNT/testdir2 2>/dev/null
+   _create_items $LIMIT2 "$SCRATCH_MNT/testdir2"
+
+   echo -e "\nRemount with $LIMIT1 Kb limit,\nnew item in testdir/ should result to ENOSPC: " >> $seqres.full
+   _scratch_mount "-o remount,max_dir_size_kb=$LIMIT1"
+   _create_items $LIMIT2
+   echo -e "\nnew item in testdir2/ should result to ENOSPC: " >> $seqres.full
+   _create_items $LIMIT2 "$SCRATCH_MNT/testdir2"
+   remove_files "$SCRATCH_MNT/testdir2"
+   rmdir $testdir
+   mkdir $testdir
+   dd if=/dev/urandom of=$testfile bs=1 seek=4096 count=4096 > /dev/null 2>&1
+
+   echo -e "\nExceed $LIMIT1 Kb directory limit with new subdirectories: " >> $seqres.full
+   _create_items $LIMIT1 $testdir "mkdir"
+   remove_files
+
+   echo -e "\nCreate ext4 fs on testdir/subdir with $LIMIT2 Kb limit," >> $seqres.full
+   mkdir $testdir/subdir 2>/dev/null
+   umount $TEST_DEV 1>/dev/null 2>&1
+   _mkfs_dev $TEST_DEV $MKFS_OPT >>$seqres.full 2>&1
+   _mount -o max_dir_size_kb=$LIMIT2 $TEST_DEV $testdir/subdir
+
+   echo "exceed $LIMIT1 Kb limit of testdir/:" >> $seqres.full
+   _create_items $LIMIT1
+
+   echo -e "\nexceed $LIMIT2 Kb limit of testdir/subdir:" >> $seqres.full
+   _create_items $LIMIT2 "$testdir/subdir"
+
+   echo -e "\ntestdir/ limit $LIMIT2 Kb, testdir/subdir limit $LIMIT1 Kb," >> $seqres.full
+   umount $TEST_DEV 1>/dev/null 2>&1
+   _scratch_mount "-o remount,max_dir_size_kb=$LIMIT2"
+   _mount -o max_dir_size_kb=$LIMIT1 $TEST_DEV $testdir/subdir
+
+   echo "exceed new $LIMIT2 Kb limit of testdir/ with a set of files:" >> $seqres.full
+   _create_items $LIMIT2
+   echo -e "\nnew item in testdir/subdir should result to ENOSPC: " >> $seqres.full
+   _create_items $LIMIT2 "$testdir/subdir"
+
+   umount $TEST_DEV 1>/dev/null 2>&1
+   remove_files
+
+   echo -e "\nTestdir/ limit $LIMIT2 Kb, loop fs: testdir/subdir limit $LIMIT1 Kb," >> $seqres.full
+   mkdir $testdir/subdir
+
+   $MKFS_EXT4_PROG -F $MKFS_OPT $testfile 2m >> $seqres.full 2>&1
+   _mount -o loop,max_dir_size_kb=$LIMIT1 $testfile $testdir/subdir
+
+   echo "exceed $LIMIT1 Kb limit of testdir/subdir with a set of files:" >> $seqres.full
+   _create_items $LIMIT1 "$testdir/subdir"
+
+   echo -e "\nexceed $LIMIT2 Kb limit of testdir/ with a set of files:" >> $seqres.full
+   _create_items $LIMIT2
+
+   umount -d $testdir/subdir
+   remove_files
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+
+# real QA test starts here
+
+_supported_fs ext4
+_supported_os Linux
+_require_scratch
+_require_loop
+
+run_test 8 16
+run_test 4 32 "-O ^dir_index"
+run_test 5 11 "-b 1024" 
+
+# success, all done
+status=0
+exit 0
+
diff --git a/tests/ext4/309.out b/tests/ext4/309.out
new file mode 100755
index 0000000..56330d6
--- /dev/null
+++ b/tests/ext4/309.out
@@ -0,0 +1,2 @@
+QA output created by 309
+Silence is golden
diff --git a/tests/ext4/group b/tests/ext4/group
index e7f1f2a..78d643b 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -16,3 +16,4 @@
 306 auto rw resize quick
 307 auto ioctl rw
 308 auto ioctl rw prealloc quick
+309 auto
-- 
1.9.3


  reply	other threads:[~2014-12-15 16:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-11 12:06 max_dir_size_kb option list Alexander Tsvetkov
2014-12-12  0:12 ` Andreas Dilger
2014-12-15 16:06   ` Alexander Tsvetkov
2014-12-12  0:55 ` Dave Chinner
2014-12-15 16:06   ` Alexander Tsvetkov [this message]
2014-12-15 21:51     ` Dave Chinner
2014-12-16 15:42       ` Alexander Tsvetkov
2014-12-23 12:06         ` Alexander Tsvetkov
2014-12-24  0:34           ` Dave Chinner
2015-03-20 14:42             ` Alexander Tsvetkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=548F070F.2050808@oracle.com \
    --to=alexander.tsvetkov@oracle.com \
    --cc=david@fromorbit.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.