2024-01-16 17:31:32

by Jeff Layton

[permalink] [raw]
Subject: [PATCH fstests] common/rc: NFSv2/3 do not support negative timestamps

The NFSv2 and v3 protocols use unsigned values for timestamps. Fix
_require_negative_timestamps() to check the NFS version and _notrun if
it's 2 or 3.

Signed-off-by: Jeff Layton <[email protected]>
---
common/rc | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index a9e0ba7e22f1..d4ac0744fab0 100644
--- a/common/rc
+++ b/common/rc
@@ -2902,6 +2902,27 @@ _require_debugfs()
[ -d "$DEBUGFS_MNT/boot_params" ] || _notrun "Debugfs not mounted"
}

+#
+# Return the version of NFS in use on the mount on $1. Returns 0
+# if it's not NFS.
+#
+_nfs_version()
+{
+ local mountpoint=$1
+ local nfsvers=""
+
+ case "$FSTYP" in
+ nfs*)
+ nfsvers=`_mount | grep $1 | sed -n 's/^.*vers=\([0-9.]*\).*$/\1/p'`
+ ;;
+ *)
+ nfsvers="0"
+ ;;
+ esac
+
+ echo "$nfsvers"
+}
+
# The default behavior of SEEK_HOLE is to always return EOF.
# Filesystems that implement non-default behavior return the offset
# of holes with SEEK_HOLE. There is no way to query the filesystem
@@ -2925,7 +2946,7 @@ _fstyp_has_non_default_seek_data_hole()
nfs*)
# NFSv2, NFSv3, and NFSv4.0/4.1 only support default behavior of SEEK_HOLE,
# while NFSv4.2 supports non-default behavior
- local nfsvers=`_mount() | grep $TEST_DEV | sed -n 's/^.*vers=\([0-9.]*\).*$/\1/p'`
+ local nfsvers=$( _nfs_version "$TEST_DIR" )
[ "$nfsvers" = "4.2" ]
return $?
;;
@@ -5129,6 +5150,16 @@ _require_negative_timestamps() {
ceph|exfat)
_notrun "$FSTYP does not support negative timestamps"
;;
+ nfs*)
+ #
+ # NFSv2/3 timestamps use 32-bit unsigned values, and so
+ # cannot represent values prior to the epoch
+ #
+ local nfsvers=$( _nfs_version "$TEST_DIR" )
+ if [ "$nfsvers" = "2" -o "$nfsvers" = "3" ]; then
+ _notrun "$FSTYP version $nfsvers does not support negative timestamps"
+ fi
+ ;;
esac
}

--
2.43.0



2024-01-18 00:36:49

by David Disseldorp

[permalink] [raw]
Subject: Re: [PATCH fstests] common/rc: NFSv2/3 do not support negative timestamps

On Tue, 16 Jan 2024 12:31:27 -0500, Jeff Layton wrote:

> The NFSv2 and v3 protocols use unsigned values for timestamps. Fix
> _require_negative_timestamps() to check the NFS version and _notrun if
> it's 2 or 3.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> common/rc | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index a9e0ba7e22f1..d4ac0744fab0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2902,6 +2902,27 @@ _require_debugfs()
> [ -d "$DEBUGFS_MNT/boot_params" ] || _notrun "Debugfs not mounted"
> }
>
> +#
> +# Return the version of NFS in use on the mount on $1. Returns 0
> +# if it's not NFS.
> +#
> +_nfs_version()
> +{
> + local mountpoint=$1
> + local nfsvers=""
> +
> + case "$FSTYP" in
> + nfs*)
> + nfsvers=`_mount | grep $1 | sed -n 's/^.*vers=\([0-9.]*\).*$/\1/p'`
> + ;;
> + *)
> + nfsvers="0"
> + ;;
> + esac
> +
> + echo "$nfsvers"
> +}
> +
> # The default behavior of SEEK_HOLE is to always return EOF.
> # Filesystems that implement non-default behavior return the offset
> # of holes with SEEK_HOLE. There is no way to query the filesystem
> @@ -2925,7 +2946,7 @@ _fstyp_has_non_default_seek_data_hole()
> nfs*)
> # NFSv2, NFSv3, and NFSv4.0/4.1 only support default behavior of SEEK_HOLE,
> # while NFSv4.2 supports non-default behavior
> - local nfsvers=`_mount() | grep $TEST_DEV | sed -n 's/^.*vers=\([0-9.]*\).*$/\1/p'`
> + local nfsvers=$( _nfs_version "$TEST_DIR" )
> [ "$nfsvers" = "4.2" ]
> return $?
> ;;
> @@ -5129,6 +5150,16 @@ _require_negative_timestamps() {
> ceph|exfat)
> _notrun "$FSTYP does not support negative timestamps"
> ;;
> + nfs*)
> + #
> + # NFSv2/3 timestamps use 32-bit unsigned values, and so
> + # cannot represent values prior to the epoch
> + #
> + local nfsvers=$( _nfs_version "$TEST_DIR" )
> + if [ "$nfsvers" = "2" -o "$nfsvers" = "3" ]; then
> + _notrun "$FSTYP version $nfsvers does not support negative timestamps"
> + fi
> + ;;
> esac
> }

Nit: It looks like there's quite some overlap with the existing
_is_nfs_version() / _fs_options() helpers, but it's still an improvement
on what's currently there...
Reviewed-by: David Disseldorp <[email protected]>