[PATCH v2 0/3] CIFS support for XFS test suite

[PATCH v2 0/3] CIFS support for XFS test suite

[Pavel Shilovsky]

These are patches that adds CIFS support to XFS tests.

The first patch changes generic/013 test to use mount options provided for $TEST_DEV for remount. The second patch adds "-cifs" command line argument and CIFS specific variables. The third patch disables tests that need POSIX behavior from filesystem for unix cifs mounts.

With these patches applied, most of generic/shared $TEST_DEV tests pass for the recent kernel cifs module.

Pavel Shilovsky (3):
  generic/013: encapsulate remount during cleanup
  common: add cifs support
  cifs: skip tests that need POSIX support for nounix mounts

 README            |  5 +++--
 check             |  2 ++
 common/config     | 31 +++++++++++++++++++++++++++----
 common/rc         | 39 +++++++++++++++++++++++++++++++++++++++
 tests/generic/005 |  1 +
 tests/generic/013 |  2 +-
 tests/generic/023 |  1 +
 tests/generic/024 |  1 +
 tests/generic/025 |  1 +
 tests/generic/131 |  1 +
 10 files changed, 77 insertions(+), 7 deletions(-)

-- 
1.9.1

[PATCH v2 1/3] generic/013: encapsulate remount during cleanup

[Pavel Shilovsky]

The existing code calls remount for $TEST_DEV with constantly defined
mount options. This can fail if a user specifies different mount options.
Fix this by using new _test_remount() call that remounts $TEST_DEV.

Reviewed-by: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 common/rc         | 6 ++++++
 tests/generic/013 | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index 16da898..d9c9995 100644
--- a/common/rc
+++ b/common/rc
@@ -218,6 +218,12 @@ _test_mount()
     _mount -t $FSTYP $TEST_OPTIONS $TEST_FS_MOUNT_OPTS $SELINUX_MOUNT_OPTIONS $* $TEST_DEV $TEST_DIR
 }
 
+_test_remount()
+{
+    $UMOUNT_PROG $TEST_DEV
+    _test_mount
+}
+
 _scratch_mkfs_options()
 {
     _scratch_options mkfs
diff --git a/tests/generic/013 b/tests/generic/013
index 93d9904..534c9f0 100755
--- a/tests/generic/013
+++ b/tests/generic/013
@@ -35,7 +35,7 @@ _cleanup()
 {
     cd /
     # we might get here with a RO FS
-    mount -o remount,rw $TEST_DEV >/dev/null 2>&1
+    _test_remount
     # now remove fsstress directory.
     # N.B. rm(1) on IRIX can find problems when building up a long pathname
     # such that what it has is greater the 1024 chars and will
-- 
1.9.1

[PATCH v2 2/3] common: add cifs support

[Pavel Shilovsky]

Pass -cifs argument from the command line to enable cifs testing
for $TEST_DEV.

Reviewed-by: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 README        |  5 +++--
 check         |  2 ++
 common/config | 31 +++++++++++++++++++++++++++----
 common/rc     | 26 ++++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/README b/README
index b299c8f..95a81c6 100644
--- a/README
+++ b/README
@@ -91,14 +91,15 @@ Running tests:
     - By default the tests suite will run xfs tests:
     - ./check '*/001' '*/002' '*/003'
     - ./check '*/06?'
-    - You can explicitly specify NFS, otherwise the filesystem type will be
-      autodetected from $TEST_DEV:
+    - You can explicitly specify NFS or CIFS, otherwise the filesystem type will
+      be autodetected from $TEST_DEV:
       ./check -nfs [test(s)]
     - Groups of tests maybe ran by: ./check -g [group(s)]
       See the 'group' file for details on groups
     - for udf tests: ./check -udf [test(s)]
       Running all the udf tests: ./check -udf -g udf
     - for running nfs tests: ./check -nfs [test(s)]
+    - for running cifs/smb3 tests: ./check -cifs [test(s)]
     - To randomize test order: ./check -r [test(s)]
 
     
diff --git a/check b/check
index 77c6559..42a1ac2 100755
--- a/check
+++ b/check
@@ -68,6 +68,7 @@ usage()
 
 check options
     -nfs                test NFS
+    -cifs               test CIFS
     -tmpfs              test TMPFS
     -l			line mode diff
     -udiff		show unified diff (default)
@@ -205,6 +206,7 @@ while [ $# -gt 0 ]; do
 	-\? | -h | --help) usage ;;
 
 	-nfs)	FSTYP=nfs ;;
+	-cifs)	FSTYP=cifs ;;
 	-tmpfs)	FSTYP=tmpfs ;;
 
 	-g)	group=$2 ; shift ;
diff --git a/common/config b/common/config
index 10cc6fe..fc21b37 100644
--- a/common/config
+++ b/common/config
@@ -206,6 +206,7 @@ case "$HOSTOS" in
         export MKFS_UDF_PROG="`set_prog_path mkfs_udf`"
         export XFS_FSR_PROG="`set_prog_path /usr/etc/fsr_xfs`"
         export MKFS_NFS_PROG="false"
+        export MKFS_CIFS_PROG="false"
         ;;
     Linux)
         export MKFS_XFS_PROG="`set_prog_path mkfs.xfs`"
@@ -215,6 +216,7 @@ case "$HOSTOS" in
         export BTRFS_SHOW_SUPER_PROG="`set_prog_path btrfs-show-super`"
         export XFS_FSR_PROG="`set_prog_path xfs_fsr`"
         export MKFS_NFS_PROG="false"
+        export MKFS_CIFS_PROG="false"
         ;;
 esac
 
@@ -238,6 +240,9 @@ _mount_opts()
 	nfs)
 		export MOUNT_OPTIONS=$NFS_MOUNT_OPTIONS
 		;;
+	cifs)
+		export MOUNT_OPTIONS=$CIFS_MOUNT_OPTIONS
+		;;
 	ext2|ext3|ext4|ext4dev)
 		# acls & xattrs aren't turned on by default on ext$FOO
 		export MOUNT_OPTIONS="-o acl,user_xattr $EXT_MOUNT_OPTIONS"
@@ -259,6 +264,17 @@ _mount_opts()
 	esac
 }
 
+_test_mount_opts()
+{
+	case $FSTYP in
+	cifs)
+		export TEST_FS_MOUNT_OPTS=$CIFS_MOUNT_OPTIONS
+		;;
+	*)
+		;;
+	esac
+}
+
 _mkfs_opts()
 {
 	case $FSTYP in
@@ -273,6 +289,9 @@ _mkfs_opts()
 	nfs)
 		export MKFS_OPTIONS=$NFS_MKFS_OPTIONS
 		;;
+	cifs)
+		export MKFS_OPTIONS=$CIFS_MKFS_OPTIONS
+		;;
 	reiserfs)
 		export MKFS_OPTIONS="$REISERFS_MKFS_OPTIONS -q"
 		;;
@@ -366,6 +385,7 @@ get_next_config() {
 
 	local OLD_FSTYP=$FSTYP
 	local OLD_MOUNT_OPTIONS=$MOUNT_OPTIONS
+	local OLD_TEST_FS_MOUNT_OPTS=$TEST_FS_MOUNT_OPTS
 	local OLD_MKFS_OPTIONS=$MKFS_OPTIONS
 	local OLD_FSCK_OPTIONS=$FSCK_OPTIONS
 
@@ -382,10 +402,12 @@ get_next_config() {
 
 	if [ ! -z "$OLD_FSTYP" ] && [ $OLD_FSTYP != $FSTYP ]; then
 		[ -z "$MOUNT_OPTIONS" ] && _mount_opts
+		[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
 		[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 		[ -z "$FSCK_OPTIONS" ] && _fsck_opts
 	else
 		[ -z "$MOUNT_OPTIONS" ] && export MOUNT_OPTIONS=$OLD_MOUNT_OPTIONS
+		[ -z "$TEST_FS_MOUNT_OPTS" ] && export TEST_FS_MOUNT_OPTS=$OLD_TEST_FS_MOUNT_OPTS
 		[ -z "$MKFS_OPTIONS" ] && export MKFS_OPTIONS=$OLD_MKFS_OPTIONS
 		[ -z "$FSCK_OPTIONS" ] && export FSCK_OPTIONS=$OLD_FSCK_OPTIONS
 	fi
@@ -408,9 +430,9 @@ get_next_config() {
 		exit 1
 	fi
 
-	echo $TEST_DEV | grep -q ":" > /dev/null 2>&1
+	echo $TEST_DEV | grep -qE ":|//" > /dev/null 2>&1
 	if [ ! -b "$TEST_DEV" -a "$?" != "0" ]; then
-		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a block device or a NFS filesystem"
+		echo "common/config: Error: \$TEST_DEV ($TEST_DEV) is not a block device or a network filesystem"
 		exit 1
 	fi
 
@@ -431,9 +453,9 @@ get_next_config() {
 		export SCRATCH_DEV_NOT_SET=true
 	fi
 
-	echo $SCRATCH_DEV | grep -q ":" > /dev/null 2>&1
+	echo $SCRATCH_DEV | grep -qE ":|//" > /dev/null 2>&1
 	if [ ! -z "$SCRATCH_DEV" -a ! -b "$SCRATCH_DEV" -a "$?" != "0" ]; then
-		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a block device or a NFS filesystem"
+		echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) is not a block device or a network filesystem"
 		exit 1
 	fi
 
@@ -457,6 +479,7 @@ if [ -z "$CONFIG_INCLUDED" ]; then
 	FSTYP=${FSTYP:=xfs}
 	export FSTYP
 	[ -z "$MOUNT_OPTIONS" ] && _mount_opts
+	[ -z "$TEST_FS_MOUNT_OPTS" ] && _test_mount_opts
 	[ -z "$MKFS_OPTIONS" ] && _mkfs_opts
 	[ -z "$FSCK_OPTIONS" ] && _fsck_opts
 fi
diff --git a/common/rc b/common/rc
index d9c9995..8b427fc 100644
--- a/common/rc
+++ b/common/rc
@@ -107,6 +107,8 @@ case "$FSTYP" in
 	 ;;
     nfs)
 	 ;;
+    cifs)
+	 ;;
 esac
 
 # make sure we have a standard umask
@@ -503,6 +505,9 @@ _test_mkfs()
     nfs*)
 	# do nothing for nfs
 	;;
+    cifs)
+	# do nothing for cifs
+	;;
     udf)
         $MKFS_UDF_PROG $MKFS_OPTIONS $* $TEST_DEV > /dev/null
 	;;
@@ -524,6 +529,9 @@ _scratch_mkfs()
     nfs*)
 	# do nothing for nfs
 	;;
+    cifs)
+	# do nothing for cifs
+	;;
     udf)
         $MKFS_UDF_PROG $MKFS_OPTIONS $* $SCRATCH_DEV > /dev/null
 	;;
@@ -973,6 +981,9 @@ _require_scratch()
 	nfs*)
                  _notrun "requires a scratch device"
 		 ;;
+	cifs)
+		_notrun "requires a scratch device"
+		;;
 	tmpfs)
 		if [ -z "$SCRATCH_DEV" -o ! -d "$SCRATCH_MNT" ];
 		then
@@ -1022,6 +1033,15 @@ _require_test()
 	nfs*)
                  _notrun "requires a test device"
 		 ;;
+	cifs)
+		echo $TEST_DEV | grep -q "//" > /dev/null 2>&1
+		if [ -z "$TEST_DEV" -o "$?" != "0" ]; then
+			_notrun "this test requires a valid \$TEST_DEV"
+		fi
+		if [ ! -d "$TEST_DIR" ]; then
+		     _notrun "this test requires a valid \$TEST_DIR"
+		fi
+		;;
 	tmpfs)
 		if [ -z "$TEST_DEV" -o ! -d "$TEST_DIR" ];
 		then
@@ -1812,6 +1832,9 @@ _check_test_fs()
     nfs)
 	# no way to check consistency for nfs
 	;;
+    cifs)
+	# no way to check consistency for cifs
+	;;
     udf)
 	# do nothing for now
 	;;
@@ -1850,6 +1873,9 @@ _check_scratch_fs()
     nfs*)
 	# Don't know how to check an NFS filesystem, yet.
 	;;
+    cifs)
+	# Don't know how to check a CIFS filesystem, yet.
+	;;
     btrfs)
 	_check_btrfs_filesystem $device
 	;;
-- 
1.9.1

[PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Pavel Shilovsky]

CIFS/SMB protocol without POSIX extensions doesn't support operations
with symbolic links and advisory byte-range locks from the same process.
Add a check for nounix mounts and use it in generic tests that
require such operations.

Reviewed-by: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 common/rc         | 7 +++++++
 tests/generic/005 | 1 +
 tests/generic/023 | 1 +
 tests/generic/024 | 1 +
 tests/generic/025 | 1 +
 tests/generic/131 | 1 +
 6 files changed, 12 insertions(+)

diff --git a/common/rc b/common/rc
index 8b427fc..191f7ff 100644
--- a/common/rc
+++ b/common/rc
@@ -2374,6 +2374,13 @@ _require_btrfs_fs_feature()
 		_notrun "Feature $feat not supported by the available btrfs version"
 }
 
+_require_test_posix_ext()
+{
+	[ "$FSTYP" != "cifs" ] && return 0
+	cat /proc/mounts | grep $TEST_DEV | grep cifs | grep -q nounix && \
+		_notrun "Require POSIX extensions enabled"
+}
+
 _get_total_inode()
 {
 	if [ -z "$1" ]; then
diff --git a/tests/generic/005 b/tests/generic/005
index d78e43f..0c2b51f 100755
--- a/tests/generic/005
+++ b/tests/generic/005
@@ -67,6 +67,7 @@ _touch()
 # real QA test starts here
 _supported_fs generic
 _require_test
+_require_test_posix_ext
 
 # IRIX UDF does not support symlinks
 if [ $FSTYP == 'udf' ]; then
diff --git a/tests/generic/023 b/tests/generic/023
index 114485c..91b8a37 100755
--- a/tests/generic/023
+++ b/tests/generic/023
@@ -45,6 +45,7 @@ _supported_os Linux
 
 _require_test
 _requires_renameat2
+_require_test_posix_ext
 
 # real QA test starts here
 
diff --git a/tests/generic/024 b/tests/generic/024
index 8945191..1248e78 100755
--- a/tests/generic/024
+++ b/tests/generic/024
@@ -45,6 +45,7 @@ _supported_os Linux
 
 _require_test
 _requires_renameat2
+_require_test_posix_ext
 
 rename_dir=$TEST_DIR/$$
 mkdir $rename_dir
diff --git a/tests/generic/025 b/tests/generic/025
index 6b6c8ab..d06136c 100755
--- a/tests/generic/025
+++ b/tests/generic/025
@@ -45,6 +45,7 @@ _supported_os Linux
 
 _require_test
 _requires_renameat2
+_require_test_posix_ext
 
 rename_dir=$TEST_DIR/$$
 mkdir $rename_dir
diff --git a/tests/generic/131 b/tests/generic/131
index b4e3ff0..9736963 100755
--- a/tests/generic/131
+++ b/tests/generic/131
@@ -45,6 +45,7 @@ _cleanup()
 _supported_fs generic
 _supported_os Linux
 _require_test
+_require_test_posix_ext
 
 TESTFILE=$TEST_DIR/lock_file
 
-- 
1.9.1

Re: [PATCH v2 0/3] CIFS support for XFS test suite

[Pavel Shilovsky]

2014-08-27 0:28 GMT+04:00 Pavel Shilovsky <pshilovsky-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
> These are patches that adds CIFS support to XFS tests.
>
> The first patch changes generic/013 test to use mount options provided for $TEST_DEV for remount. The second patch adds "-cifs" command line argument and CIFS specific variables. The third patch disables tests that need POSIX behavior from filesystem for unix cifs mounts.
>
> With these patches applied, most of generic/shared $TEST_DEV tests pass for the recent kernel cifs module.
>
> Pavel Shilovsky (3):
>   generic/013: encapsulate remount during cleanup
>   common: add cifs support
>   cifs: skip tests that need POSIX support for nounix mounts

Patches are also available in a git repository:
http://git.altlinux.org/people/piastry/public/?p=xfstests.git;a=shortlog;h=refs/heads/v2

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Christoph Hellwig]

On Wed, Aug 27, 2014 at 12:28:38AM +0400, Pavel Shilovsky wrote:
> CIFS/SMB protocol without POSIX extensions doesn't support operations
> with symbolic links and advisory byte-range locks from the same process.
> Add a check for nounix mounts and use it in generic tests that
> require such operations.

+_require_test_posix_ext seems very cifs specific.  Can you take
a look at the tests and see what posix feature they require and
add features based on that?  Let's have a quick discussion here on the
requirements of the tests before even writing the code.

> diff --git a/tests/generic/005 b/tests/generic/005
> index d78e43f..0c2b51f 100755
> --- a/tests/generic/005
> +++ b/tests/generic/005
> @@ -67,6 +67,7 @@ _touch()
>  # real QA test starts here
>  _supported_fs generic
>  _require_test
> +_require_test_posix_ext
>  
>  # IRIX UDF does not support symlinks
>  if [ $FSTYP == 'udf' ]; then

this suggest 005 needs symlinks and plain cifs doesn't support them.
We should also fold this test for IRIX udf into the _requires_symlink
tests.

> diff --git a/tests/generic/023 b/tests/generic/023
> index 114485c..91b8a37 100755
> --- a/tests/generic/023
> +++ b/tests/generic/023
> @@ -45,6 +45,7 @@ _supported_os Linux
>  
>  _require_test
>  _requires_renameat2
> +_require_test_posix_ext


023-025 just require a working renameat2, and nothing in Posix.  What's
the problem for cifs here?

> diff --git a/tests/generic/131 b/tests/generic/131
> index b4e3ff0..9736963 100755
> --- a/tests/generic/131
> +++ b/tests/generic/131
> @@ -45,6 +45,7 @@ _cleanup()
>  _supported_fs generic
>  _supported_os Linux
>  _require_test
> +_require_test_posix_ext
>  
>  TESTFILE=$TEST_DIR/lock_file

131 tests fcntl style file locking, so we should test for that.

Re: [PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Steve French]

On Wed, Aug 27, 2014 at 10:58 AM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> On Wed, Aug 27, 2014 at 12:28:38AM +0400, Pavel Shilovsky wrote:
>> CIFS/SMB protocol without POSIX extensions doesn't support operations
>> with symbolic links and advisory byte-range locks from the same process.
>> Add a check for nounix mounts and use it in generic tests that
>> require such operations.
>
> +_require_test_posix_ext seems very cifs specific.  Can you take
> a look at the tests and see what posix feature they require and
> add features based on that?  Let's have a quick discussion here on the
> requirements of the tests before even writing the code.

Do you know a more standardized way to test if symlink support is available?
We don't really have the same thing as the FS and Device capability ioctls
that Microsoft offers, but even for them, they don't export whether symlink
support is enabled as a volume or export property.
Presumably we could try to create one and look for the rc EOPNOTSUPP), but
it seems simple enough for now to check on mount looking for
fs-specific features
as Pavel has done, or to always enable the symlink tests for cifs (and
smb3 after
we have added it).




>
> this suggest 005 needs symlinks and plain cifs doesn't support them.
> We should also fold this test for IRIX udf into the _requires_symlink
> tests.

Plain cifs does support emulated symlinks two ways but they aren't enabled by
default in mount.  At least one of these also needs to be added to SMB3, and
when I looked at this in the past it looked fairly easy. When support for
"mfsymlinks" (apple-style emulated symlinks) or for creating "NFS-reparse point
symlinks" (Windows style NFS server symlinks)  is added
for SMB3 we could change the way the cifs check is done.  In the long run I
would prefer that we run the cifs and smb3 xfstest runs with symlink
dependent tests enabled.

>> diff --git a/tests/generic/023 b/tests/generic/023
>> index 114485c..91b8a37 100755
>> --- a/tests/generic/023
>> +++ b/tests/generic/023
>> @@ -45,6 +45,7 @@ _supported_os Linux
>>
>>  _require_test
>>  _requires_renameat2
>> +_require_test_posix_ext
>
>
> 023-025 just require a working renameat2, and nothing in Posix.  What's
> the problem for cifs here?




-- 
Thanks,

Steve

Re: [PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Christoph Hellwig]

On Wed, Aug 27, 2014 at 11:17:25AM -0500, Steve French wrote:
> Do you know a more standardized way to test if symlink support is available?
> We don't really have the same thing as the FS and Device capability ioctls
> that Microsoft offers, but even for them, they don't export whether symlink
> support is enabled as a volume or export property.
> Presumably we could try to create one and look for the rc EOPNOTSUPP), but
> it seems simple enough for now to check on mount looking for
> fs-specific features
> as Pavel has done, or to always enable the symlink tests for cifs (and
> smb3 after
> we have added it).

There's not good way to check for it except for trying.  So a simple
routine that creates a symlink on $TEST_DIR and checks if that works
would be the way to go.

Re: [PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Pavel Shilovsky]

> 27 авг. 2014 г., в 19:58, Christoph Hellwig <hch@infradead.org> написал(а):
> 
>> On Wed, Aug 27, 2014 at 12:28:38AM +0400, Pavel Shilovsky wrote:
>> CIFS/SMB protocol without POSIX extensions doesn't support operations
>> with symbolic links and advisory byte-range locks from the same process.
>> Add a check for nounix mounts and use it in generic tests that
>> require such operations.
> 
> +_require_test_posix_ext seems very cifs specific.  Can you take
> a look at the tests and see what posix feature they require and
> add features based on that?  Let's have a quick discussion here on the
> requirements of the tests before even writing the code.
> 

Agree.

>> diff --git a/tests/generic/005 b/tests/generic/005
>> index d78e43f..0c2b51f 100755
>> --- a/tests/generic/005
>> +++ b/tests/generic/005
>> @@ -67,6 +67,7 @@ _touch()
>> # real QA test starts here
>> _supported_fs generic
>> _require_test
>> +_require_test_posix_ext
>> 
>> # IRIX UDF does not support symlinks
>> if [ $FSTYP == 'udf' ]; then
> 
> this suggest 005 needs symlinks and plain cifs doesn't support them.
> We should also fold this test for IRIX udf into the _requires_symlink
> tests.
> 
>> diff --git a/tests/generic/023 b/tests/generic/023
>> index 114485c..91b8a37 100755
>> --- a/tests/generic/023
>> +++ b/tests/generic/023
>> @@ -45,6 +45,7 @@ _supported_os Linux
>> 
>> _require_test
>> _requires_renameat2
>> +_require_test_posix_ext
> 
> 
> 023-025 just require a working renameat2, and nothing in Posix.  What's
> the problem for cifs here?

These tests try to create symlinks and then rename them.

> 
>> diff --git a/tests/generic/131 b/tests/generic/131
>> index b4e3ff0..9736963 100755
>> --- a/tests/generic/131
>> +++ b/tests/generic/131
>> @@ -45,6 +45,7 @@ _cleanup()
>> _supported_fs generic
>> _supported_os Linux
>> _require_test
>> +_require_test_posix_ext
>> 
>> TESTFILE=$TEST_DIR/lock_file
> 
> 131 tests fcntl style file locking, so we should test for that.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


So for these tests we need two check functions: _require_symlink for 005, 023, 024, 025 and _require_fcntl for 131. Right?

Re: [PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Christoph Hellwig]

On Wed, Aug 27, 2014 at 11:49:56PM +0400, Pavel Shilovsky wrote:
> So for these tests we need two check functions: _require_symlink for 005,
> 023, 024, 025 and _require_fcntl for 131. Right?

More or less. I'd call the second one _require_fcntl_locks as fcntl
is a multiplexer for tons of unrelated functionality.

Re: [PATCH v2 3/3] cifs: skip tests that need POSIX support for nounix mounts

[Pavel Shilovsky]

2014-08-28 2:50 GMT+04:00 Christoph Hellwig <hch@infradead.org>:
> On Wed, Aug 27, 2014 at 11:49:56PM +0400, Pavel Shilovsky wrote:
>> So for these tests we need two check functions: _require_symlink for 005,
>> 023, 024, 025 and _require_fcntl for 131. Right?
>
> More or less. I'd call the second one _require_fcntl_locks as fcntl
> is a multiplexer for tons of unrelated functionality.

Ok, let's name it this way.

What's about the 1st and 2nd patches? I am going to repost the series
with a new version of the 3rd patch (according to our discussion) and
it'll be better to incorporate their possible changes as well.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH v2 1/3] generic/013: encapsulate remount during cleanup

[Christoph Hellwig]

On Wed, Aug 27, 2014 at 12:28:36AM +0400, Pavel Shilovsky wrote:
> The existing code calls remount for $TEST_DEV with constantly defined
> mount options. This can fail if a user specifies different mount options.
> Fix this by using new _test_remount() call that remounts $TEST_DEV.

Looks technically correct, but I still wonder why it's needed at all,
I can't see anything in the test that would remount the filesystem
read-only.  git history isn't a help here as the remount goes back to
the first public revision.

Re: [PATCH v2 2/3] common: add cifs support

[Christoph Hellwig]

On Wed, Aug 27, 2014 at 12:28:37AM +0400, Pavel Shilovsky wrote:
> +    - You can explicitly specify NFS or CIFS, otherwise the filesystem type will
> +      be autodetected from $TEST_DEV:
>        ./check -nfs [test(s)]

Can you take care of mentioning tmpfs as well, looks like it didn't get
added here when support was added.

>  	;;
> +    cifs)
> +	# Don't know how to check a CIFS filesystem, yet.
> +	;;

Will we ever find a way to sanity check a cifs fs? :)

Otherwise looks fine to me.

Re: [PATCH v2 1/3] generic/013: encapsulate remount during cleanup

[Pavel Shilovsky]

>> 28 авг. 2014 г., в 21:08, Christoph Hellwig <hch@infradead.org> написал(а):
>> 
>> On Wed, Aug 27, 2014 at 12:28:36AM +0400, Pavel Shilovsky wrote:
>> The existing code calls remount for $TEST_DEV with constantly defined
>> mount options. This can fail if a user specifies different mount options.
>> Fix this by using new _test_remount() call that remounts $TEST_DEV.
> 
> Looks technically correct, but I still wonder why it's needed at all,
> I can't see anything in the test that would remount the filesystem
> read-only.  git history isn't a help here as the remount goes back to
> the first public revision.

Anyway I think it won't hurt to apply this patch at first and then probably remove this part at all.

--
Best regards,
Pavel Shilovsky.

Re: [PATCH v2 2/3] common: add cifs support

[Pavel Shilovsky]

>> 28 авг. 2014 г., в 21:18, Christoph Hellwig <hch@infradead.org> написал(а):
>> 
>> On Wed, Aug 27, 2014 at 12:28:37AM +0400, Pavel Shilovsky wrote:
>> +    - You can explicitly specify NFS or CIFS, otherwise the filesystem type will
>> +      be autodetected from $TEST_DEV:
>>      ./check -nfs [test(s)]
> 
> Can you take care of mentioning tmpfs as well, looks like it didn't get
> added here when support was added.

Ok, will add it.

> 
>>   ;;
>> +    cifs)
>> +    # Don't know how to check a CIFS filesystem, yet.
>> +    ;;
> 
> Will we ever find a way to sanity check a cifs fs? :)
> 
> Otherwise looks fine to me.

Probably some day we will be able to run fsck checks on the remote server through a specific rpc.

Thank you for the review - will repost the series.

--
Best regards,
Pavel Shilovsky.