Re: [Y2038] [PATCH v2] generic/390: Add tests for inode timestamp policy

2016-12-30 Thread Deepa Dinamani 
On Wed, Dec 28, 2016 at 3:05 AM, Eryu Guan  wrote:
> On Sat, Dec 24, 2016 at 07:38:13PM -0800, Deepa Dinamani wrote:
>> The test helps to validate clamping and mount behaviors
>> according to supported file system timestamp ranges.
>>
>> Note that the test can fail on 32-bit systems for a
>> few file systems. This will be corrected when vfs is
>> transitioned to use 64-bit timestamps.
>>
>> Signed-off-by: Deepa Dinamani 
>> ---
>> The branch of the kernel tree can be located at
>>
>> https://github.com/deepa-hub/vfs refs/heads/vfs_timestamp_policy
>>
>> The xfs_io patch to add utimes is at
>>
>> https://www.spinics.net/lists/linux-xfs/msg02952.html
>
> Thanks for this info! I built your test kernel and applied the xfs_io
> patch, and I got test failure on XFS, test passed on ext4 (256 inode
> size) without problems, is this expected?

Yes, this is expected.
Since kernel does not have actual limits for xfs filled in.
Although I need to fix the if condition(-gt needs to change to -ge) to
fail on mount here.

>> + f2fs)
>> + echo "-2147483648 2147483647"
>> + ;;
>> + *)
>> + echo "-1 -1"
>> + _notrun "filesystem $FSTYP timestamp bounds are unknown"
>
> This "_notrun" doesn't belong here. I think we can introduce a
> _require_y2038 rule. e.g.

Ok, I will merge this with require_y2038_sysfs() like what you suggest below.

> _require_y2038()
> {
> local device=${1:-$TEST_DEV}
> local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
> if [ ! -ne $sysfsdir ]; then
> _notrun "no kernel support for y2038 sysfs switch"
> fi
>
> local tsmin tsmax
> read tsmin tsmax <<<$(_filesystem_timestamp_range $device)
> if [ $tsmin -eq -1 -a $tsmax -eq -1 ]; then
> _notrun "filesystem $FSTYP timestamp bounds are unknown"
> fi
> }
>
> Then
>
> _scratch_mkfs
> _require_y2038 $SCRATCH_DEV
>
>> + ;;
>> + esac
>> +}
>> +
>>  # indicate whether YP/NIS is active or not
>>  #
>>  _yp_active()
>> @@ -2070,6 +2109,9 @@ _require_xfs_io_command()
>>   echo $testio | egrep -q "Inappropriate ioctl" && \
>>   _notrun "xfs_io $command support is missing"
>>   ;;
>> + "utimes" )
>> + testio=`$XFS_IO_PROG -f -c "utimes" 0 0 0 0 $testfile 2>&1`
>> + ;;
>>   *)
>>   testio=`$XFS_IO_PROG -c "$command help" 2>&1`
>>   esac
>> diff --git a/tests/generic/390 b/tests/generic/390
>> new file mode 100755
>> index 000..8ccadad
>> + stat_timestamp=`stat -c"%X;%Y" $file`
>> +
>> + prev_timestamp="$timestamp;$timestamp"
>> + if [ $prev_timestamp != $stat_timestamp ]; then
>> + echo "$prev_timestamp != $stat_timestamp" | tee -a $seqres.full
>> + exit
>
> No need to exit here. We prefer continuing the test in fstests when such
> test failure happens, it enlarges the test coverage and exercises some
> error paths. One exception is that when continuing the test could result
> in blocking all subsequent tests, we should exit early, one example
> provided later.

Ok, will continue here.

>> +{
>> + file=$1
>> + timestamp=$2
>> + update_time=$3
>> +
>> + #check if the time needs update
>> + if [ $update_time -eq 1 ]; then
>> + echo "Updating file: $file to timestamp `date -d @$timestamp`" 
>>  >> $seqres.full
>> + $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file
>> + if [ $? -ne 0 ]; then
>> + echo "Failed to update times on $file" | tee -a 
>> $seqres.full
>> + exit
>
> Same here.

Will do.

>> + #initialization iterator
>> + n=1
>> +
>> + for TIME in "${TIMESTAMPS[@]}"
>> + do
>> + #Run the test
>> + run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time
>> +
>> + #update iterator
>> + ((n++))
>
> Seems the comments here are not necessary, initialize the iterator, run
> the test and update iterator are all obvious.

Will remove comments.

> And we prefer this code style in fstests:
> for TIME in "${TIMESTAMPS[@]}"; do
> ...
> done
>

Will follow this style.
Is there a coding style guide or some recognized good example test?

>> + done
>> +}
>> +
>> +_scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
>
> Let test continue here on mkfs and mount failure, test harness will
> capture these failures too, and running tests in these failure
> conditions sometimes reveal interesting bugs :)
>
> One example that we should exit on mkfs & mount failure is that we're
> testing ENOSPC and going to fulfill the test filesystem, and we'll eat
> all free space on root fs if we failed to mount the test device, and all
> subsequent tests are blocked because of ENOSPC on root fs.

In this test, I want to check for mount failure as that is expected behavior.

Re: [Y2038] [PATCH v2] generic/390: Add tests for inode timestamp policy

2016-12-28 Thread Eryu Guan 
On Sat, Dec 24, 2016 at 07:38:13PM -0800, Deepa Dinamani wrote:
> The test helps to validate clamping and mount behaviors
> according to supported file system timestamp ranges.
> 
> Note that the test can fail on 32-bit systems for a
> few file systems. This will be corrected when vfs is
> transitioned to use 64-bit timestamps.
> 
> Signed-off-by: Deepa Dinamani 
> ---
> The branch of the kernel tree can be located at
> 
> https://github.com/deepa-hub/vfs refs/heads/vfs_timestamp_policy
> 
> The xfs_io patch to add utimes is at
> 
> https://www.spinics.net/lists/linux-xfs/msg02952.html

Thanks for this info! I built your test kernel and applied the xfs_io
patch, and I got test failure on XFS, test passed on ext4 (256 inode
size) without problems, is this expected?

[root@localhost xfstests]# uname -a
Linux localhost 4.9.0-rc6-next-20161123.y2038+ #14 SMP Wed Dec 28 16:01:54 CST 
2016 x86_64 x86_64 x86_64 GNU/Linux
[root@localhost xfstests]# diff -u tests/generic/402.out
/root/workspace/xfstests/results//xfs_4k/generic/402.out.bad
--- tests/generic/402.out   2016-12-28 16:16:23.773711175 +0800
+++ /root/workspace/xfstests/results//xfs_4k/generic/402.out.bad 2016-12-28 
16:43:54.825058119 +0800
@@ -1,2 +1,2 @@
 QA output created by 402
 -y2038 inode timestamp tests completed successfully
 +2147483647;2147483647 != 2147483648;2147483648

seqres.full shows:

meta-data=/dev/mapper/systemvg-testlv2 isize=256agcount=16, agsize=245696 
blks
 =   sectsz=512   attr=2, projid32bit=1
 =   crc=0finobt=0, sparse=0, rmapbt=0
data =   bsize=4096   blocks=3931136, imaxpct=25
 =   sunit=64 swidth=192 blks
naming   =version 2  bsize=4096   ascii-ci=0 ftype=1
log  =internal log   bsize=4096   blocks=2560, version=2
 =   sectsz=512   sunit=64 blks, lazy-count=1
realtime =none   extsz=4096   blocks=0, rtextents=0
min supported timestamp -2147483648 Sat Dec 14 04:45:52 CST 1901
max supported timestamp 2147483647 Tue Jan 19 11:14:07 CST 2038
min timestamp that needs to be supported by fs for rw mount is 2147483647 Tue 
Jan 19 11:14:07 CST 2038
sysctl filesystem timestamp check is on
In memory timestamps update test start
Updating file: /mnt/testarea/scratch/test_1 to timestamp Sat Dec 14 04:45:52 
CST 1901
Checking file: /mnt/testarea/scratch/test_1 Updated timestamp is Sat Dec 14 
04:45:52 CST 1901
Updating file: /mnt/testarea/scratch/test_2 to timestamp Thu Jan  1 08:00:00 
CST 1970
Checking file: /mnt/testarea/scratch/test_2 Updated timestamp is Thu Jan  1 
08:00:00 CST 1970
Updating file: /mnt/testarea/scratch/test_3 to timestamp Tue Jan 19 11:14:07 
CST 2038
Checking file: /mnt/testarea/scratch/test_3 Updated timestamp is Tue Jan 19 
11:14:07 CST 2038
Updating file: /mnt/testarea/scratch/test_4 to timestamp Tue Jan 19 11:14:08 
CST 2038
Checking file: /mnt/testarea/scratch/test_4 Updated timestamp is Tue Jan 19 
11:14:07 CST 2038
2147483647;2147483647 != 2147483648;2147483648

> 
> Changes since v1:
> * Use xfs_io utimes command
> * Updated error handling
> * Reorganized code according to review comments
> 
>  common/rc |  42 +++
>  tests/generic/390 | 197 
> ++
>  tests/generic/390.out |   2 +
>  tests/generic/group   |   1 +
>  4 files changed, 242 insertions(+)
>  create mode 100755 tests/generic/390
>  create mode 100644 tests/generic/390.out
> 
> diff --git a/common/rc b/common/rc
> index e3b54ec..93c6e65 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1960,6 +1960,45 @@ _run_aiodio()
>  return $status
>  }
>  
> +# this test requires y2038 sysfs switch support
> +#
> +_require_y2038_sysfs()
> +{
> + sysfsdir=/proc/sys/fs/fs-timestamp-check-on
> +
> + if [ ! -e $sysfsdir ]; then
> + _notrun "no kernel support for y2038 sysfs switch"
> + fi
> +}
> +
> +_filesystem_timestamp_range()
> +{
> + device=${1:-$TEST_DEV}
> + case $FSTYP in
> + ext4)
> + if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | 
> cut -d: -f2) -gt 128 ]; then
> + echo "-2147483648 15032385535"
> + else
> + echo "-2147483648 2147483647"
> + fi
> + ;;
> +
> + xfs)
> + echo "-2147483648 2147483647"
> + ;;
> + jfs)
> + echo "0 4294967295"
> + ;;
> + f2fs)
> + echo "-2147483648 2147483647"
> + ;;
> + *)
> + echo "-1 -1"
> + _notrun "filesystem $FSTYP timestamp bounds are unknown"

This "_notrun" doesn't belong here. I think we can introduce a
_require_y2038 rule. e.g.

_require_y2038()
{
local device=${1:-$TEST_DEV}
local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
if [ ! -ne $sysfsdir ]; then

[Y2038] [PATCH v2] generic/390: Add tests for inode timestamp policy

2016-12-24 Thread Deepa Dinamani 
The test helps to validate clamping and mount behaviors
according to supported file system timestamp ranges.

Note that the test can fail on 32-bit systems for a
few file systems. This will be corrected when vfs is
transitioned to use 64-bit timestamps.

Signed-off-by: Deepa Dinamani 
---
The branch of the kernel tree can be located at

https://github.com/deepa-hub/vfs refs/heads/vfs_timestamp_policy

The xfs_io patch to add utimes is at

https://www.spinics.net/lists/linux-xfs/msg02952.html

Changes since v1:
* Use xfs_io utimes command
* Updated error handling
* Reorganized code according to review comments

 common/rc |  42 +++
 tests/generic/390 | 197 ++
 tests/generic/390.out |   2 +
 tests/generic/group   |   1 +
 4 files changed, 242 insertions(+)
 create mode 100755 tests/generic/390
 create mode 100644 tests/generic/390.out

diff --git a/common/rc b/common/rc
index e3b54ec..93c6e65 100644
--- a/common/rc
+++ b/common/rc
@@ -1960,6 +1960,45 @@ _run_aiodio()
 return $status
 }
 
+# this test requires y2038 sysfs switch support
+#
+_require_y2038_sysfs()
+{
+   sysfsdir=/proc/sys/fs/fs-timestamp-check-on
+
+   if [ ! -e $sysfsdir ]; then
+   _notrun "no kernel support for y2038 sysfs switch"
+   fi
+}
+
+_filesystem_timestamp_range()
+{
+   device=${1:-$TEST_DEV}
+   case $FSTYP in
+   ext4)
+   if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | 
cut -d: -f2) -gt 128 ]; then
+   echo "-2147483648 15032385535"
+   else
+   echo "-2147483648 2147483647"
+   fi
+   ;;
+
+   xfs)
+   echo "-2147483648 2147483647"
+   ;;
+   jfs)
+   echo "0 4294967295"
+   ;;
+   f2fs)
+   echo "-2147483648 2147483647"
+   ;;
+   *)
+   echo "-1 -1"
+   _notrun "filesystem $FSTYP timestamp bounds are unknown"
+   ;;
+   esac
+}
+
 # indicate whether YP/NIS is active or not
 #
 _yp_active()
@@ -2070,6 +2109,9 @@ _require_xfs_io_command()
echo $testio | egrep -q "Inappropriate ioctl" && \
_notrun "xfs_io $command support is missing"
;;
+   "utimes" )
+   testio=`$XFS_IO_PROG -f -c "utimes" 0 0 0 0 $testfile 2>&1`
+   ;;
*)
testio=`$XFS_IO_PROG -c "$command help" 2>&1`
esac
diff --git a/tests/generic/390 b/tests/generic/390
new file mode 100755
index 000..8ccadad
--- /dev/null
+++ b/tests/generic/390
@@ -0,0 +1,197 @@
+#! /bin/bash
+# FS QA Test 390
+#
+# Tests to verify policy for filesystem timestamps for
+# supported ranges:
+# 1. Verify filesystem rw mount according to sysctl
+# timestamp_supported.
+# 2. Verify timestamp clamping for timestamps beyond max
+# timestamp supported.
+#
+# Exit status 1: either or both tests above fail.
+# Exit status 0: both the above tests pass.
+#
+#---
+# Copyright (c) 2016 Deepa Dinamani.  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
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+trap "exit \$status" 0 1 2 3 15
+
+# Get standard environment, filters and checks.
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# Prerequisites for the test run.
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command utimes
+_require_y2038_sysfs
+
+# Compare file timestamps obtained from stat
+# with a given timestamp.
+check_stat()
+{
+   file=$1
+   timestamp=$2
+
+   stat_timestamp=`stat -c"%X;%Y" $file`
+
+   prev_timestamp="$timestamp;$timestamp"
+   if [ $prev_timestamp != $stat_timestamp ]; then
+   echo "$prev_timestamp != $stat_timestamp" | tee -a $seqres.full
+   exit
+   fi
+}
+
+run_test_individual()
+{
+   file=$1
+   timestamp=$2
+   update_time=$3
+
+   #check if the time needs update
+   if [