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

[Deepa Dinamani]

>> > +_filesystem_timestamp_range()
>> > +{
>> > +   device=${1:-$TEST_DEV}
>> > +   case $FSTYP in
>> > +   ext4)   #dumpe2fs
>> > +   if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | 
>> > cut -d: -f2) -gt 128 ]; then
>> > +   echo "-2147483648 15032385535"
>> > +   else
>> > +   echo "-2147483648 2147483647"
>> > +   fi
>>
>> Do ext3 and ext2 follow the same config as ext4?
>
> Those two only support the second case with 128 byte inodes, but the same
> check should work on all three.
>
> I have an overview of the limits on https://kernelnewbies.org/y2038/vfs,
> though I'd probably check all of them again, as some of them turned out
> to be wrong. In particular, identifying whether the on-disk timestamps
> are meant to be signed or unsigned can be a matter of interpretation
> and there may be a specification that disagrees with the implementation.

Right now I've only added tests for a few filesystems.
The plan is to add more filesystems later after we agree on the test.

-Deepa
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

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

[Deepa Dinamani]

> Need an entry in .gitignore file too.

Will add this.

>>  SUBDIRS =
>>
>> diff --git a/src/y2038_futimens.c b/src/y2038_futimens.c
>> new file mode 100644
>> index 000..291e4fa
>> --- /dev/null
>> +++ b/src/y2038_futimens.c
>> @@ -0,0 +1,61 @@
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +int
>> +do_utime(int fd, long long time)
>> +{
>> + struct timespec t[2];
>> +
>> + /*
>> +  * Convert long long to timespec format.
>> +  * Seconds precision is assumed here.
>> +  */
>> + t[0].tv_sec = time;
>> + t[0].tv_nsec = 0;
>> + t[1].tv_sec = time;
>> + t[1].tv_nsec = 0;
>> +
>> + /* Call utimens to update time. */
>> + if (futimens(fd, t)) {
>> + perror("futimens");
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +int
>> +main(int argc, char **argv)
>> +{
>> + int fd;
>> + long long time;
>> +
>> + if(argc < 3) {
>> + fprintf(stderr, "Usage: %s filename timestamp\n"
>> + "Filename: file to be created or 
>> opened in current directory\n"
>> + "Timestamp: is seconds since 
>> 1970-01-01 00:00:00 UTC\n", argv[0]);
>> + exit(1);
>
> Seems there's no need for an extra level of indention :)

Ok, will fix this.

>> + }
>> +
>> + /* Create the file */
>> + fd = creat(argv[1], 0666);
>> + if(fd < 0) {
>> + perror("creat");
>> + exit(1);
>> + }
>> +
>> + /* Get the timestamp */
>> + time = strtoull(argv[2], NULL, 0);
>> + if (errno) {
>
> From errno(3), errno is never set to zero by any system call or library
> function, so errno isn't reset to zero on strtoull and this check always
> fails for me, with errno = ENOENT, because:
>
> ...
> access("/usr/share/dracut/modules.d/01fips", F_OK) = -1 ENOENT (No such file 
> or directory)
> ...
> write(4, "strtoull: No such file or direct"..., 36strtoull: No such file or 
> directory
>
> the errno was from access(2) call in my case.

Right. Man page for strtoull() says:

Since strtoul() can legitimately return 0 or ULONG_MAX (ULLONG_MAX for
strtoull()) on both success and failure, the calling program should
set errno to 0 before the call, and then determine if an error
occurred by checking whether errno has a nonzero value after the call.

I will set errno to 0 before strtoull().
Thanks.

>> diff --git a/tests/generic/390 b/tests/generic/390
>> new file mode 100755
>> index 000..f069988
>> --- /dev/null
>> +++ b/tests/generic/390
>> @@ -0,0 +1,238 @@
>> +#! /bin/bash
>> +# FS QA Test No. generic/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.
>
> Please use "./new" script to generate template, which contains all
> necessary initial setups and the copyright info.

Thanks, will do.

>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +#echo "output in $seqres.full"
>> +here=`pwd`
>> +
>> +# Get standard environment, filters and checks.
>> +. ./common/rc
>> +. ./common/filter
>> +. ./common/attr
>> +
>> +SRC_GROUPS=`find tests -not -path tests -type d -printf "%f "`
>
> What's this for? Seems it's not used in the test.

Yes, this is not required. Leftovers from some previous version of the test.
Thanks.

>> +
>> +# Prerequisites for the test run.
>> +_supported_fs generic
>> +_supported_os Linux
>> +_require_scratch
>
> Need to check the existence of y2038_futimens program, to make sure it's
> really built, e.g.
>
> _require_test_program "y2038_futimens"

Will add this


>> +Y2038_PROG=$here/src/y2038_futimens
>> +
>> +#initialize exit status
>> +status=0
>
> We use 1 as the default value of status, so you can just "exit" on
> failure (because trap will catch the signal and exit with $status again)
> and only set status=0 and exit when test passes. "./new" already sets it
> up for you :)

Will take care of this.

>> +
>> +# Generic test cleanup function.
>> +_cleanup()
>> +{
>> +# Remove any leftover files from last run.
>> +rm -f ${SCRATCH_MNT}/test_?
>
> No need to cleanup files on SCRATCH_DEV, it's meant to be mkfs'ed every
> time before using it.

Ok, makes sense. I will remove this clean up.

>> +#unmount and mount  $SCRATCH_DEV.
>> +_umount_mount_scratch_dev()
>
> There's a helper to do this: _scratch_cycle_mount, only that it doesn't
> change PWD. And if you don't want to write $SCRATCH_MNT again, this is
> what we do usually in fstests:
>
> testfile=$SCRATCH_MNT/
> ...
> do_test $testfile

Thanks, will change it similar to what you suggest.

>> +{
>> +#change directory so that you are not using 

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

[Arnd Bergmann]

On Thursday, November 24, 2016 6:40:55 PM CET Eryu Guan wrote:
> On Wed, Nov 23, 2016 at 04:52:02PM -0800, Deepa Dinamani wrote:
> > +_filesystem_timestamp_range()
> > +{
> > +   device=${1:-$TEST_DEV}
> > +   case $FSTYP in
> > +   ext4)   #dumpe2fs
> > +   if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | 
> > cut -d: -f2) -gt 128 ]; then
> > +   echo "-2147483648 15032385535"
> > +   else
> > +   echo "-2147483648 2147483647"
> > +   fi
> 
> Do ext3 and ext2 follow the same config as ext4?

Those two only support the second case with 128 byte inodes, but the same
check should work on all three.

I have an overview of the limits on https://kernelnewbies.org/y2038/vfs,
though I'd probably check all of them again, as some of them turned out
to be wrong. In particular, identifying whether the on-disk timestamps
are meant to be signed or unsigned can be a matter of interpretation
and there may be a specification that disagrees with the implementation.

Arnd
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

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

[Dave Chinner]

On Wed, Nov 23, 2016 at 04:52:02PM -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.

Thanks for doing this, Deepa. A few comments below - Eryu has
already given lots of feedback, so I won't repeat all that...

> Signed-off-by: Deepa Dinamani 
> ---
>  common/attr   |  27 ++
>  src/Makefile  |   2 +-
>  src/y2038_futimens.c  |  61 +
>  tests/generic/390 | 238 
> ++
>  tests/generic/390.out |   2 +
>  tests/generic/group   |   1 +
>  6 files changed, 330 insertions(+), 1 deletion(-)
>  create mode 100644 src/y2038_futimens.c
>  create mode 100755 tests/generic/390
>  create mode 100644 tests/generic/390.out
> 
> diff --git a/common/attr b/common/attr
> index ce2d76a..579dc9b 100644
> --- a/common/attr
> +++ b/common/attr
> @@ -56,6 +56,33 @@ _acl_get_max()
>   esac
>  }
>  
> +_filesystem_timestamp_range()
> +{
> + device=${1:-$TEST_DEV}
> + case $FSTYP in
> + ext4)   #dumpe2fs
> + 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"
> + ;;
> + esac
> +}

This is better off in common/rc right now - common/attr is for
extended attribute test code, not generic filesystem stuff.

> diff --git a/src/y2038_futimens.c b/src/y2038_futimens.c
> new file mode 100644
> index 000..291e4fa
> --- /dev/null
> +++ b/src/y2038_futimens.c
> @@ -0,0 +1,61 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +int
> +do_utime(int fd, long long time)
> +{
> + struct timespec t[2];
> +
> + /*
> +  * Convert long long to timespec format.
> +  * Seconds precision is assumed here.
> +  */
> + t[0].tv_sec = time;
> + t[0].tv_nsec = 0;
> + t[1].tv_sec = time;
> + t[1].tv_nsec = 0;
> +
> + /* Call utimens to update time. */
> + if (futimens(fd, t)) {
> + perror("futimens");
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> +int
> +main(int argc, char **argv)
> +{
> + int fd;
> + long long time;
> +
> + if(argc < 3) {
> + fprintf(stderr, "Usage: %s filename timestamp\n"
> + "Filename: file to be created or opened 
> in current directory\n"
> + "Timestamp: is seconds since 1970-01-01 
> 00:00:00 UTC\n", argv[0]);
> + exit(1);
> + }
> +
> + /* Create the file */
> + fd = creat(argv[1], 0666);
> + if(fd < 0) {
> + perror("creat");
> + exit(1);
> + }
> +
> + /* Get the timestamp */
> + time = strtoull(argv[2], NULL, 0);
> + if (errno) {
> + perror("strtoull");
> + exit(1);
> + }
> +
> + if (do_utime(fd, time))
> + return 1;
> +
> + return 0;
> +}

This might be useful to add to xfs_io rather than a one-off helper
for xfstest - that avoids the need to create files, and it can be
used to change times on existing files



> +_run_test_individual() #_run_individual_test(file, timestamp, update_time)
> +{
> +file=$1
> +timestamp=$2
> +update_time=$3

No need for comments after the function declaration - the prototype
is obvious from the local variable assignments

...

> +#Remove log from last run
> +rm -f $seqres.full
> +
> +#install cleaner
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
> +read tsmin tsmax <<<$(_filesystem_timestamp_range $SCRATCH_DEV)

This is all test setup preamble, so should be at the top.

> +if [ $tsmin -eq -1 -a $tsmax -eq -1 ]; then
> +_notrun "filesystem $FSTYP timestamp bounds are unknown"
> +fi

This should be in a _requires_timestamp_range() function, I think.

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

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

[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 
---
 common/attr   |  27 ++
 src/Makefile  |   2 +-
 src/y2038_futimens.c  |  61 +
 tests/generic/390 | 238 ++
 tests/generic/390.out |   2 +
 tests/generic/group   |   1 +
 6 files changed, 330 insertions(+), 1 deletion(-)
 create mode 100644 src/y2038_futimens.c
 create mode 100755 tests/generic/390
 create mode 100644 tests/generic/390.out

diff --git a/common/attr b/common/attr
index ce2d76a..579dc9b 100644
--- a/common/attr
+++ b/common/attr
@@ -56,6 +56,33 @@ _acl_get_max()
esac
 }
 
+_filesystem_timestamp_range()
+{
+   device=${1:-$TEST_DEV}
+   case $FSTYP in
+   ext4)   #dumpe2fs
+   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"
+   ;;
+   esac
+}
+
 _require_acl_get_max()
 {
if [ $(_acl_get_max) -eq 0 ]; then
diff --git a/src/Makefile b/src/Makefile
index dd51216..0b99ae4 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -21,7 +21,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize 
preallo_rw_pattern_reader \
stale_handle pwrite_mmap_blocked t_dir_offset2 seek_sanity_test \
seek_copy_test t_readdir_1 t_readdir_2 fsync-tester nsexec cloner \
renameat2 t_getcwd e4compact test-nextquota punch-alternating \
-   attr-list-by-handle-cursor-test listxattr
+   attr-list-by-handle-cursor-test listxattr y2038_futimens
 
 SUBDIRS =
 
diff --git a/src/y2038_futimens.c b/src/y2038_futimens.c
new file mode 100644
index 000..291e4fa
--- /dev/null
+++ b/src/y2038_futimens.c
@@ -0,0 +1,61 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int
+do_utime(int fd, long long time)
+{
+   struct timespec t[2];
+
+   /*
+* Convert long long to timespec format.
+* Seconds precision is assumed here.
+*/
+   t[0].tv_sec = time;
+   t[0].tv_nsec = 0;
+   t[1].tv_sec = time;
+   t[1].tv_nsec = 0;
+
+   /* Call utimens to update time. */
+   if (futimens(fd, t)) {
+   perror("futimens");
+   return 1;
+   }
+
+   return 0;
+}
+
+int
+main(int argc, char **argv)
+{
+   int fd;
+   long long time;
+
+   if(argc < 3) {
+   fprintf(stderr, "Usage: %s filename timestamp\n"
+   "Filename: file to be created or opened 
in current directory\n"
+   "Timestamp: is seconds since 1970-01-01 
00:00:00 UTC\n", argv[0]);
+   exit(1);
+   }
+
+   /* Create the file */
+   fd = creat(argv[1], 0666);
+   if(fd < 0) {
+   perror("creat");
+   exit(1);
+   }
+
+   /* Get the timestamp */
+   time = strtoull(argv[2], NULL, 0);
+   if (errno) {
+   perror("strtoull");
+   exit(1);
+   }
+
+   if (do_utime(fd, time))
+   return 1;
+
+   return 0;
+}
diff --git a/tests/generic/390 b/tests/generic/390
new file mode 100755
index 000..f069988
--- /dev/null
+++ b/tests/generic/390
@@ -0,0 +1,238 @@
+#! /bin/bash
+# FS QA Test No. generic/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.
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+#echo "output in $seqres.full"
+here=`pwd`
+
+# Get standard environment, filters and checks.
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+SRC_GROUPS=`find tests -not -path tests -type d -printf "%f "`
+
+# Prerequisites for the test run.
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+
+Y2038_PROG=$here/src/y2038_futimens
+
+#initialize exit status
+status=0
+
+# Generic test cleanup function.
+_cleanup()
+{
+# Remove any leftover files from last run.
+rm -f ${SCRATCH_MNT}/test_?
+}
+
+#unmount and mount  $SCRATCH_DEV.
+_umount_mount_scratch_dev()
+{
+#change directory so that you are not