diff mbox series

generic/402: Make timestamp range check conditional

Message ID 20191223051622.7975-1-deepa.kernel@gmail.com (mailing list archive)
State New, archived
Headers show
Series generic/402: Make timestamp range check conditional | expand

Commit Message

Deepa Dinamani Dec. 23, 2019, 5:16 a.m. UTC
Addition of fs-specific timestamp range checking was added
in 188d20bcd1eb ("vfs: Add file timestamp range support").

Add a check for whether the kernel supports the limits check
before running the associated test.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 common/rc         | 11 +++++++++++
 tests/generic/402 |  3 +++
 2 files changed, 14 insertions(+)

Comments

Amir Goldstein Dec. 23, 2019, 6:36 a.m. UTC | #1
On Mon, Dec 23, 2019 at 7:16 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> Addition of fs-specific timestamp range checking was added
> in 188d20bcd1eb ("vfs: Add file timestamp range support").
>
> Add a check for whether the kernel supports the limits check
> before running the associated test.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
>  common/rc         | 11 +++++++++++
>  tests/generic/402 |  3 +++
>  2 files changed, 14 insertions(+)
>
> diff --git a/common/rc b/common/rc
> index 816588d6..472db995 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1981,6 +1981,17 @@ _run_aiodio()
>      return $status
>  }
>
> +_require_kernel_timestamp_range()
> +{
> +       # 128-byte inodes do not have room for extended timestamp
> +       MKFS_OPTIONS=-I128 _scratch_mkfs_ext4 &>> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> +
> +       mount -t ext4 ${SCRATCH_DEV} ${SCRATCH_MNT}
> +       _check_dmesg_for "ext4 filesystem being mounted at ${SCRATCH_MNT} supports timestamps until 2038" || \
> +               _notrun "Kernel does not support timestamp limits"
> +       umount ${SCRATCH_MNT}
> +}
> +

Deepa,

Thank you for following up.
I am not sure if mkfs.ext4 of scratch partition in a generic test is going to be
very popular - let's see what others have to say.
You can certainly now do that without checking that  ${SCRATCH_DEV} is
a blockdev which is not the case for overlay and networking filesystems.

Why did you choose not to use a loop mounted ext2 for the check as
I suggested?
You can use _require_loop() and _require_ext2() inside the check.
In any case, please also check for failure to mount.

Thanks,
Amir.
Deepa Dinamani Dec. 24, 2019, 1:15 a.m. UTC | #2
On Sun, Dec 22, 2019 at 10:36 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Dec 23, 2019 at 7:16 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > Addition of fs-specific timestamp range checking was added
> > in 188d20bcd1eb ("vfs: Add file timestamp range support").
> >
> > Add a check for whether the kernel supports the limits check
> > before running the associated test.
> >
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > ---
> >  common/rc         | 11 +++++++++++
> >  tests/generic/402 |  3 +++
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/common/rc b/common/rc
> > index 816588d6..472db995 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1981,6 +1981,17 @@ _run_aiodio()
> >      return $status
> >  }
> >
> > +_require_kernel_timestamp_range()
> > +{
> > +       # 128-byte inodes do not have room for extended timestamp
> > +       MKFS_OPTIONS=-I128 _scratch_mkfs_ext4 &>> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> > +
> > +       mount -t ext4 ${SCRATCH_DEV} ${SCRATCH_MNT}
> > +       _check_dmesg_for "ext4 filesystem being mounted at ${SCRATCH_MNT} supports timestamps until 2038" || \
> > +               _notrun "Kernel does not support timestamp limits"
> > +       umount ${SCRATCH_MNT}
> > +}
> > +
>
> Deepa,
>
> Thank you for following up.
> I am not sure if mkfs.ext4 of scratch partition in a generic test is going to be
> very popular - let's see what others have to say.
> You can certainly now do that without checking that  ${SCRATCH_DEV} is
> a blockdev which is not the case for overlay and networking filesystems.

 _require_block_device() should alleviate this concern. But, I think
making it a loop back device is a good idea.

I meant to check when mkfs.ext4 would fail, but I forgot. I will
change this in v2.

> Why did you choose not to use a loop mounted ext2 for the check as
> I suggested?
> You can use _require_loop() and _require_ext2() inside the check.
> In any case, please also check for failure to mount.

I did not not see anybody creating ext2 filesystem, and I thought that
finding a kernel supporting ext4 was a lot more likely. We can add a
_require_ext4() along the lines of _ext2_reqire() if really necessary.
We should also add "_require_command "$MKFS_EXT4_PROG" mkfs.ext4".

I think it does not matter which filesystem we use unless we get the
warning. Let me know if I missed something.

I will pick one of the two: ext4 (with small inode) or ext2 and do a
loop back device instead for v2. I will also add the suggested-by that
I forgot on this patch.

-Deepa
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 816588d6..472db995 100644
--- a/common/rc
+++ b/common/rc
@@ -1981,6 +1981,17 @@  _run_aiodio()
     return $status
 }
 
+_require_kernel_timestamp_range()
+{
+	# 128-byte inodes do not have room for extended timestamp
+	MKFS_OPTIONS=-I128 _scratch_mkfs_ext4 &>> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
+
+	mount -t ext4 ${SCRATCH_DEV} ${SCRATCH_MNT}
+	_check_dmesg_for "ext4 filesystem being mounted at ${SCRATCH_MNT} supports timestamps until 2038" || \
+		_notrun "Kernel does not support timestamp limits"
+	umount ${SCRATCH_MNT}
+}
+
 _require_timestamp_range()
 {
 	local device=${1:-$TEST_DEV}
diff --git a/tests/generic/402 b/tests/generic/402
index 0392c258..2be94c54 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -30,6 +30,7 @@  rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_check_dmesg
 _require_xfs_io_command utimes
 
 # Compare file timestamps obtained from stat
@@ -79,6 +80,8 @@  run_test()
 	done
 }
 
+_require_kernel_timestamp_range
+
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
 _require_timestamp_range $SCRATCH_DEV