[LTP] [PATCH 1/5] acl_test01: Fixes

[LTP] [PATCH 1/5] acl_test01: Fixes

[Stanislav Kholmanskikh]

Converted the test case to use tst_tmpdir instead of local juggling
with $TMP.

When we execute LTP, $PATH includes $LTPROOT/testcases/bin, so there
is no reason to set $TCbin explicitly. Therefore, removed $TCbin.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 runtest/admin_tools                 |    2 +-
 testcases/kernel/fs/acls/acl_test01 |   56 ++++++++++++++--------------------
 2 files changed, 24 insertions(+), 34 deletions(-)

diff --git a/runtest/admin_tools b/runtest/admin_tools
index 559757e..0a46468 100644
--- a/runtest/admin_tools
+++ b/runtest/admin_tools
@@ -6,4 +6,4 @@ cron_allow01 cron_allow01
 cron_dirs_checks01 cron_dirs_checks01
 at_deny01 at_deny01
 at_allow01 at_allow01
-acl_test01 export TCbin=$LTPROOT/testcases/bin;acl_test01
+acl_test01 acl_test01
diff --git a/testcases/kernel/fs/acls/acl_test01 b/testcases/kernel/fs/acls/acl_test01
index 328ff74..be88aa8 100755
--- a/testcases/kernel/fs/acls/acl_test01
+++ b/testcases/kernel/fs/acls/acl_test01
@@ -43,16 +43,13 @@ export TST_COUNT=1
 
 . test.sh
 
-TMP=${TMP:=/tmp}
-
 TEST_USER1="acltest1"
 TEST_USER1_GROUP="users"
 TEST_USER1_PASSWD="ltp_test_pass1"
-TEST_USER1_HOMEDIR="$TMP/tacl/mount-ext3/$TEST_USER1"
+TEST_USER1_HOMEDIR="mount-ext3/$TEST_USER1"
 
-FILE_ACL="$TMP/tacl/mount-ext3/test_file"
-FILE_ACL_LINK="$TMP/tacl/mount-ext3/test_file_link"
-TCbin=`pwd`
+FILE_ACL="mount-ext3/test_file"
+FILE_ACL_LINK="mount-ext3/test_file_link"
 
 COMMAND=$(command -v "getenforce" "setenforce" |wc -l)
 if [ $COMMAND -eq 2 ]; then
@@ -66,9 +63,6 @@ fi
 #-----------------------------------------------------------------------
 
 do_setup(){
-
-	tst_require_root
-
 	rm -f $FILE_ACL
 	rm -f $FILE_ACL_LINK
 
@@ -87,12 +81,12 @@ do_setup(){
 	rm -rf $TEST_USER1_HOMEDIR
 	userdel $TEST_USER1 > /dev/null 2>&1
 	sleep 1
-	useradd -d $TEST_USER1_HOMEDIR -m -g $TEST_USER1_GROUP $TEST_USER1 -s /bin/sh
+	useradd -d $(readlink -f $TEST_USER1_HOMEDIR) -m \
+	    -g $TEST_USER1_GROUP $TEST_USER1 -s /bin/sh
 
 	if [ $? -ne 0 ]; then
 		tst_brkm TBROK "Could not add test user $TEST_USER1."
 	fi
-
 }
 
 #-----------------------------------------------------------------------
@@ -100,13 +94,13 @@ do_setup(){
 #-----------------------------------------------------------------------
 
 do_cleanup() {
-	rm -rf $TEST_USER1_HOMEDIR
 	userdel $TEST_USER1 > /dev/null 2>&1
-	rm -f $FILE_ACL > /dev/null 2>&1
-	rm -f $FILE_ACL_LINK > /dev/null 2>&1
-	mount | grep "$TMP/tacl/mount-ext3" && umount -d $TMP/tacl/mount-ext3
+
+	MOUNT_POINT=$(readlink -f "mount-ext3")
+	mount | grep $MOUNT_POINT && umount -d $MOUNT_POINT
 	[ "x$LOOP_DEV" != x ] && losetup -d $LOOP_DEV
-	rm -rf $TMP/tacl
+
+	tst_rmdir
 
 	# We set it back to Enforcing.
 	if [ "$SELINUX" = "Enforcing" ]; then
@@ -126,18 +120,15 @@ then
 else
 	tst_require_root
 
-	if ! ( test -d $TMP/tacl || mkdir -m 777 $TMP/tacl) ; then
-		tst_brkm TBROK "Failed to create $TMP/tacl directory."
-	fi
-
-	trap do_cleanup EXIT
+	tst_tmpdir
+	TST_CLEANUP=do_cleanup
 
 	#	The  following  commands  can  be  used as an example of using
 	#	a loopback device.
 
-	dd if=/dev/zero of=$TMP/tacl/blkext3 bs=1k count=10240 && chmod 777 $TMP/tacl/blkext3
+	dd if=/dev/zero of=blkext3 bs=1k count=10240 && chmod 777 blkext3
 	if [ $? -ne 0 ] ; then
-		tst_brkm TBROK "Failed to create $TMP/tacl/blkext3"
+		tst_brkm TBROK "Failed to create blkext3"
 	fi
 
 	# Avoid hardcoded loopback device values (-f tries to find the first
@@ -146,7 +137,7 @@ else
 		tst_brkm TCONF "[ losetup.1 ] Failed to find an available loopback device -- is the required support compiled in your kernel?"
 	fi
 
-	if ! losetup $LOOP_DEV $TMP/tacl/blkext3 2>&1 > /dev/null; then
+	if ! losetup $LOOP_DEV blkext3 2>&1 > /dev/null; then
 		echo ""
 		tst_brkm TCONF "[ losetup.2 ] Failed to setup the device."
 	fi
@@ -154,8 +145,8 @@ else
 	mount | grep ext2
 	if [ $? -ne 0 ]; then
 		mkfs -t ext3 $LOOP_DEV #> /dev/null 2>&1
-		mkdir  -m 777 $TMP/tacl/mount-ext3
-		mount -t ext3 -o defaults,acl,user_xattr $LOOP_DEV $TMP/tacl/mount-ext3
+		mkdir -m 777 mount-ext3
+		mount -t ext3 -o defaults,acl,user_xattr $LOOP_DEV mount-ext3
 		if [ $? -ne 0 ]
 		then
 			tst_resm TCONF "[ mount ] Make sure that ACL (Access Control List)"
@@ -165,8 +156,8 @@ else
 	else
 
 		mkfs -t ext2 $LOOP_DEV
-		mkdir  -m 777 $TMP/tacl/mount-ext3
-		mount -t ext2 -o defaults,acl,user_xattr $LOOP_DEV $TMP/tacl/mount-ext3
+		mkdir -m 777 mount-ext3
+		mount -t ext2 -o defaults,acl,user_xattr $LOOP_DEV mount-ext3
 		if [ $? -ne 0 ]
 		then
 			tst_resm TCONF "FAILED: [ mount ] Make sure that ACL (Access Control List)"
@@ -188,7 +179,7 @@ then
 	setfacl -m u:$TEST_USER1:r $FILE_ACL
 
 	echo "Trying extended acls for files"
-	${TCbin}/acl_file_test $FILE_ACL
+	acl_file_test $FILE_ACL
 	if [ $? -ne 0 ]
 	then
 		tst_resm TFAIL "Extended acls for files."
@@ -197,7 +188,7 @@ then
 	fi
 
 	echo "Trying extended acls for file links"
-	${TCbin}/acl_link_test $FILE_ACL_LINK
+	acl_link_test $FILE_ACL_LINK
 	if [ $? -ne 0 ]
 	then
 		tst_resm TFAIL "Extended acls for links."
@@ -217,7 +208,7 @@ else
 	setfacl -mu:root:r $FILE_ACL
 
 	echo "Trying extended acls for files"
-	${TCbin}/acl_file_test $FILE_ACL
+	acl_file_test $FILE_ACL
 	if [ $? -ne 0 ]
 	then
 		tst_resm TFAIL "Extended acls for files."
@@ -226,7 +217,7 @@ else
 	fi
 
 	echo "Trying extended acls for file links"
-	${TCbin}/acl_link_test $FILE_ACL_LINK
+	acl_link_test $FILE_ACL_LINK
 	if [ $? -ne 0 ]
 	then
 		tst_resm TFAIL "Extended acls for links."
@@ -239,7 +230,6 @@ else
 
 	su $TEST_USER1 -c "$0"
 	echo ""
-	do_cleanup
 fi
 
 tst_exit
-- 
1.7.1


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 2/5] acl_test01: Verify the exit code of the second test case

[Stanislav Kholmanskikh]

Now we verify the exit code of the second test case. We did not
do this previously, and it was a bug.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/fs/acls/acl_test01 |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/testcases/kernel/fs/acls/acl_test01 b/testcases/kernel/fs/acls/acl_test01
index be88aa8..0f050b9 100755
--- a/testcases/kernel/fs/acls/acl_test01
+++ b/testcases/kernel/fs/acls/acl_test01
@@ -228,7 +228,8 @@ else
 	chown $TEST_USER1 $FILE_ACL
 	chown $TEST_USER1 $FILE_ACL_LINK
 
-	su $TEST_USER1 -c "$0"
+	su $TEST_USER1 -c "$0" &
+	tst_record_childstatus $!
 	echo ""
 fi
 
-- 
1.7.1


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 3/5] acl_test01: Hide unnecessary output

[Stanislav Kholmanskikh]

The output from 'grep' gives no value, so let's hide it.

'losetup -d' is also not needed, because the loop device is
detached by 'umount -d'. Therefore, removed 'losetup -d' to hide
warnings like:

loop: can't delete device /dev/loop1: No such device or address

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 testcases/kernel/fs/acls/acl_test01 |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/fs/acls/acl_test01 b/testcases/kernel/fs/acls/acl_test01
index 0f050b9..4dc15ef 100755
--- a/testcases/kernel/fs/acls/acl_test01
+++ b/testcases/kernel/fs/acls/acl_test01
@@ -97,8 +97,7 @@ do_cleanup() {
 	userdel $TEST_USER1 > /dev/null 2>&1
 
 	MOUNT_POINT=$(readlink -f "mount-ext3")
-	mount | grep $MOUNT_POINT && umount -d $MOUNT_POINT
-	[ "x$LOOP_DEV" != x ] && losetup -d $LOOP_DEV
+	mount | grep -q $MOUNT_POINT && umount -d $MOUNT_POINT
 
 	tst_rmdir
 
-- 
1.7.1


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [PATCH 4/5] tools/apicmd: Add tst_fs_type

[Stanislav Kholmanskikh]

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---
 doc/test-writing-guidelines.txt |   17 +++++++++++++++++
 tools/apicmds/.gitignore        |    1 +
 tools/apicmds/Makefile          |    2 +-
 tools/apicmds/ltpapicmd.c       |   17 +++++++++++++++++
 4 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 2c40e3d..e35ec34 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -1195,6 +1195,7 @@ Following functions similar to the LTP C interface are available.
 * tst_tmpdir()
 * tst_rmdir()
 * tst_fs_has_free()
+* tst_fs_type()
 
 There is one more function called 'tst_check_cmds()' that gets unspecified
 number of parameters and asserts that each parameter is a name of an
@@ -1249,6 +1250,22 @@ satisfied, 1 if not, and 2 on error.
 
 The second argument supports suffixes kB, MB and GB, the default unit is Byte.
 
+.tst_fs_type
+[source,sh]
+-------------------------------------------------------------------------------
+#!/bin/sh
+
+...
+
+# whether the current directory is on NFS
+if [ $(tst_fs_type .) = "NFS" ]; then
+	tst_brkm TCONF "Not supported on NFS"
+fi
+
+...
+-------------------------------------------------------------------------------
+
+
 2.3.3 Cleanup
 ^^^^^^^^^^^^^
 
diff --git a/tools/apicmds/.gitignore b/tools/apicmds/.gitignore
index b4f7922..c77ba02 100644
--- a/tools/apicmds/.gitignore
+++ b/tools/apicmds/.gitignore
@@ -11,3 +11,4 @@ tst_ncpus_max
 tst_res
 tst_resm
 tst_fs_has_free
+tst_fs_type
diff --git a/tools/apicmds/Makefile b/tools/apicmds/Makefile
index 9c9472d..e8ee78c 100644
--- a/tools/apicmds/Makefile
+++ b/tools/apicmds/Makefile
@@ -28,7 +28,7 @@ CPPFLAGS		+= -D_GNU_SOURCE
 
 MAKE_TARGETS		:= $(addprefix tst_,brk brkm exit flush kvercmp \
 			     kvercmp2 res resm ncpus ncpus_conf ncpus_max \
-			     get_unused_port fs_has_free)
+			     get_unused_port fs_has_free fs_type)
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
 
diff --git a/tools/apicmds/ltpapicmd.c b/tools/apicmds/ltpapicmd.c
index 5238328..ed59f6a 100644
--- a/tools/apicmds/ltpapicmd.c
+++ b/tools/apicmds/ltpapicmd.c
@@ -77,6 +77,7 @@
 #include "test.h"
 #include "usctest.h"
 #include "safe_macros.h"
+#include "tst_fs_type.h"
 
 char *TCID;			/* Name of the testcase */
 int TST_TOTAL;			/* Total number of testcases */
@@ -359,6 +360,20 @@ fs_has_free_err:
 	exit(2);
 }
 
+const char *apicmd_fs_type(int argc, char *argv[])
+{
+	long fs_type;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: tst_fs_type path\n");
+		exit(2);
+	}
+
+	fs_type = tst_fs_type(NULL, argv[0]);
+
+	return tst_fs_type_name(fs_type);
+}
+
 /*
  * Function:    main - entry point of this program
  *
@@ -449,6 +464,8 @@ int main(int argc, char *argv[])
 		printf("%u\n", apicmd_get_unused_port(argc, argv));
 	} else if (strcmp(cmd_name, "tst_fs_has_free") == 0) {
 		apicmd_fs_has_free(argc, argv);
+	} else if (strcmp(cmd_name, "tst_fs_type") == 0) {
+		printf("%s\n", apicmd_fs_type(argc, argv));
 	}
 
 	exit(0);
-- 
1.7.1


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

[LTP] [RFC PATCH 5/5] acl_test01: Wait until the loop device is detached

[Stanislav Kholmanskikh]

If the test is executed on NFS, it may print:

rm: cannot remove `/mnt/ltp-xeJYpESVKz/acltest01.fCogoo4gn0': Directory not empty

It seems the following happen:

1. After 'unmount -d' 'acltest01.fCogoo4gn0/blkext3' is still in use by udev.
   See kernel commit a1ecac3b0656a68259927c234e505804d33a7b83
   ("loop: Make explicit loop device destruction lazy")

2. Due to the "NFS silly rename" feature, command 'rm -f acltest01.fCogoo4gn0/blkext3'
   renames 'blkext3' to '.nfs*'

3. The removal of 'acltest01.fCogoo4gn0' directory fails, because it
   still contains this '.nfs*' file.

Intorucing a wait cycle should fix this problem.

Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
---

It's a follow up to:

http://sourceforge.net/p/ltp/mailman/message/32987268/

Indeed, 'sync' was wrong there.


 testcases/kernel/fs/acls/acl_test01 |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/testcases/kernel/fs/acls/acl_test01 b/testcases/kernel/fs/acls/acl_test01
index 4dc15ef..b4c2c70 100755
--- a/testcases/kernel/fs/acls/acl_test01
+++ b/testcases/kernel/fs/acls/acl_test01
@@ -99,6 +99,22 @@ do_cleanup() {
 	MOUNT_POINT=$(readlink -f "mount-ext3")
 	mount | grep -q $MOUNT_POINT && umount -d $MOUNT_POINT
 
+	# Cope with "NFS silly rename"
+	if [ $(tst_fs_type .) = "NFS" ]; then
+	    rm -f blkext3
+
+	    TIMEOUT=5
+	    while $(ls -la | grep -q .nfs); do
+		sleep 1
+
+		TIMEOUT=$(( $TIMEOUT - 1 ))
+		if [ $TIMEOUT -eq 0 ]; then
+		    tst_resm TBROK "The loop device is still in use"
+		    break
+		fi
+	    done
+	fi
+
 	tst_rmdir
 
 	# We set it back to Enforcing.
-- 
1.7.1


------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/5] acl_test01: Fixes

[Cyril Hrubis]

Hi!
> -
> -	tst_require_root

Why do we remove this? The test calls adduser and we need root for that
or don't we?

>  	rm -f $FILE_ACL
>  	rm -f $FILE_ACL_LINK
>  
> @@ -87,12 +81,12 @@ do_setup(){
>  	rm -rf $TEST_USER1_HOMEDIR
>  	userdel $TEST_USER1 > /dev/null 2>&1
>  	sleep 1
> -	useradd -d $TEST_USER1_HOMEDIR -m -g $TEST_USER1_GROUP $TEST_USER1 -s /bin/sh
> +	useradd -d $(readlink -f $TEST_USER1_HOMEDIR) -m \
> +	    -g $TEST_USER1_GROUP $TEST_USER1 -s /bin/sh
>  
>  	if [ $? -ne 0 ]; then
>  		tst_brkm TBROK "Could not add test user $TEST_USER1."
>  	fi
> -
>  }
>  
>  #-----------------------------------------------------------------------
> @@ -100,13 +94,13 @@ do_setup(){
>  #-----------------------------------------------------------------------
>  
>  do_cleanup() {
> -	rm -rf $TEST_USER1_HOMEDIR
>  	userdel $TEST_USER1 > /dev/null 2>&1
> -	rm -f $FILE_ACL > /dev/null 2>&1
> -	rm -f $FILE_ACL_LINK > /dev/null 2>&1
> -	mount | grep "$TMP/tacl/mount-ext3" && umount -d $TMP/tacl/mount-ext3
> +
> +	MOUNT_POINT=$(readlink -f "mount-ext3")
> +	mount | grep $MOUNT_POINT && umount -d $MOUNT_POINT
>  	[ "x$LOOP_DEV" != x ] && losetup -d $LOOP_DEV
> -	rm -rf $TMP/tacl
> +
> +	tst_rmdir
>  
>  	# We set it back to Enforcing.
>  	if [ "$SELINUX" = "Enforcing" ]; then
> @@ -126,18 +120,15 @@ then
>  else
>  	tst_require_root
>  
> -	if ! ( test -d $TMP/tacl || mkdir -m 777 $TMP/tacl) ; then
> -		tst_brkm TBROK "Failed to create $TMP/tacl directory."
> -	fi
> -
> -	trap do_cleanup EXIT
> +	tst_tmpdir
> +	TST_CLEANUP=do_cleanup
>  
>  	#	The  following  commands  can  be  used as an example of using
>  	#	a loopback device.
>  
> -	dd if=/dev/zero of=$TMP/tacl/blkext3 bs=1k count=10240 && chmod 777 $TMP/tacl/blkext3
> +	dd if=/dev/zero of=blkext3 bs=1k count=10240 && chmod 777 blkext3
>  	if [ $? -ne 0 ] ; then
> -		tst_brkm TBROK "Failed to create $TMP/tacl/blkext3"
> +		tst_brkm TBROK "Failed to create blkext3"
>  	fi
>  
>  	# Avoid hardcoded loopback device values (-f tries to find the first
> @@ -146,7 +137,7 @@ else
>  		tst_brkm TCONF "[ losetup.1 ] Failed to find an available loopback device -- is the required support compiled in your kernel?"
>  	fi
>  
> -	if ! losetup $LOOP_DEV $TMP/tacl/blkext3 2>&1 > /dev/null; then
> +	if ! losetup $LOOP_DEV blkext3 2>&1 > /dev/null; then
>  		echo ""
>  		tst_brkm TCONF "[ losetup.2 ] Failed to setup the device."
>  	fi

There is an in flight patch to add tst_acquire_device into test.sh as
well.

See patch by Zeng Linggang:

[PATCH v4 1/2] test.sh: Add tst_acquire_device() and tst_release_device()

The v4 is likely to be commited as it is, can you pretty please make use
of it here?

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 4/5] tools/apicmd: Add tst_fs_type

[Cyril Hrubis]

Hi!
This one is fine, acked.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 3/5] acl_test01: Hide unnecessary output

[Cyril Hrubis]

Hi!
This one is fine, acked.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH 5/5] acl_test01: Wait until the loop device is detached

[Cyril Hrubis]

Hi!
> If the test is executed on NFS, it may print:
> 
> rm: cannot remove `/mnt/ltp-xeJYpESVKz/acltest01.fCogoo4gn0': Directory not empty
> 
> It seems the following happen:
> 
> 1. After 'unmount -d' 'acltest01.fCogoo4gn0/blkext3' is still in use by udev.
>    See kernel commit a1ecac3b0656a68259927c234e505804d33a7b83
>    ("loop: Make explicit loop device destruction lazy")
> 
> 2. Due to the "NFS silly rename" feature, command 'rm -f acltest01.fCogoo4gn0/blkext3'
>    renames 'blkext3' to '.nfs*'
> 
> 3. The removal of 'acltest01.fCogoo4gn0' directory fails, because it
>    still contains this '.nfs*' file.
> 
> Intorucing a wait cycle should fix this problem.

Hmm, we may want to add this into test.sh because otherwise we will need
to carry this snippet of code in each testcase that works with loop
devices.

Would calling umount without -d and detach_device() (from tst_device.c
via apicmd) help here? (it seems to fix the very same problem, it this
function does not work, it needs to be fixed anyway)

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH 5/5] acl_test01: Wait until the loop device is detached

[Cyril Hrubis]

Hi!
> Hmm, we may want to add this into test.sh because otherwise we will need
> to carry this snippet of code in each testcase that works with loop
> devices.
> 
> Would calling umount without -d and detach_device() (from tst_device.c
> via apicmd) help here? (it seems to fix the very same problem, it this
> function does not work, it needs to be fixed anyway)

And we also have tst_umount() to cope with daemons that probe each newly
mounted fs, it would be a good idea to add it to apicmd and make use of
it as well.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 1/5] acl_test01: Fixes

[Cyril Hrubis]

Hi!
> This is because we call 'tst_require_root' twice. The first time is from 
> "FUNCTION MAIN:" and the second time when we execute do_setup.
> 
> So 'tst_require_root' in do_setup is redundant.

Ok.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/5] acl_test01: Verify the exit code of the second test case

[Cyril Hrubis]

Hi!
> Now we verify the exit code of the second test case. We did not
> do this previously, and it was a bug.
> 
> Signed-off-by: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> ---
>  testcases/kernel/fs/acls/acl_test01 |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/testcases/kernel/fs/acls/acl_test01 b/testcases/kernel/fs/acls/acl_test01
> index be88aa8..0f050b9 100755
> --- a/testcases/kernel/fs/acls/acl_test01
> +++ b/testcases/kernel/fs/acls/acl_test01
> @@ -228,7 +228,8 @@ else
>  	chown $TEST_USER1 $FILE_ACL
>  	chown $TEST_USER1 $FILE_ACL_LINK
>  
> -	su $TEST_USER1 -c "$0"
> +	su $TEST_USER1 -c "$0" &
> +	tst_record_childstatus $!

We should probably wrap the su into a script that makes sure that the
return value is compatible with LTP. Because it may set bogus test
return status if su reported anything else than 0 or 1.

Or we may add just another function into test.sh that would work
similariry to tst_record_childstatus but that will normalize the return
value. I.e. 0 == TPASS, anything else TFAIL.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [PATCH 2/5] acl_test01: Verify the exit code of the second test case

[Cyril Hrubis]

Hi!
> > We should probably wrap the su into a script that makes sure that the
> > return value is compatible with LTP. Because it may set bogus test
> > return status if su reported anything else than 0 or 1.
> 
> Ehm, but -c "$0" will execute the same script which, in turn, will exit 
> using tst_exit.

Yep, but the su man page specifies return values:

128 + signal_nr - command terminated by signal
127 - command cannot be executed (this one is actually handled in test.sh)
126 - command not found
255 - su was asked to terminate and had to kill the program

And if none of the abowe happens it retuns the return value from the
script.

So if the script gets killed by signal of failed to executed (or cannot
be found in $PATH) the return value will be wrong.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Re: [LTP] [RFC PATCH 5/5] acl_test01: Wait until the loop device is detached

[Cyril Hrubis]

Hi!
> I think it wouldn't. Please, see below.
> 
> ## Problem 1 ##
> Rapid execution of 'losetup /dev/loop0' file + 'losetup -d /dev/loop0' 
> may fail.
> 
> Reproducer:
> for i in `seq 20`; do
>      echo "attach"
>      losetup /dev/loop0 file1.img
>      echo "detach"
>      losetup -d /dev/loop0
> done
> 
> 1. Kernel versions without the above kernel patch should fail on the 
> "detach" phase, i.e. on
>         if (lo->lo_refcnt > 1)  /* we needed one fd for the ioctl */
>                 return -EBUSY;
> in loop_clr_fd().
> 
> However, I've not tested it by myself.
> 
> And Jan's commit:
> 
> commit 554b1793a1aa34f8918e8b1c04d8c6835f4a5710
> Author: Jan Stancek <jstancek@redhat.com>
> Date:   Tue Jan 20 14:35:49 2015 +0100
> 
>      tst_device: sleep/retry if LOOP_CLR_FD fails with EBUSY
> 
>      Rapid succession of device attach/detach may lead to EBUSY
>      from ioctl(LOOP_CLR_FD), because udev rules might still be
>      running.
> 
>      Sleep/retry for a short period if LOOP_CLR_FD fails with EBUSY.
> 
>      Signed-off-by: Jan Stancek <jstancek@redhat.com>
>      Acked-by: Cyril Hrubis <chrubis@suse.cz>
> 
> is to fix this scenario.

This has been added due to udev holding reference to the device and
preventing the detach. Looks like they "fixed" this in kernel with lazy
detach meanwhile...

> 2. However, newer kernels (i.e. with the above kernel patch) will fail 
> with EBUSY at the "attach" phase, because loop_clr_fd() will always 
> return 0 and mark the loop device for auto clearing, but loop_set_fd() 
> will go this path:
> 
>         error = -EBUSY;
>         if (lo->lo_state != Lo_unbound)
>                 goto out_putf;
> 
>         /* ... */
> 
> out_putf:
>         fput(file);
> out:
>         /* This is safe: open() is still holding a reference. */
>         module_put(THIS_MODULE);
>         return error;

Hmm, that shouldn't be needed, because the we do losetup -f to get free
loop device before the attach operation. If that returns a device that
hasn't been detached yet because of lazy detaching it's a kernel bug.

> ## Problem 2 - NFS specific ##
> 
> If we have the above kernel patch, even if 'losetup -d' executed without 
> problems, it doesn't mean that the backend file is not in use.
> 
> So we need to wait some undetermined time until udev finishes its 
> playing with the loop device, and only after that we can safely remove 
> the file without a risk to be affected by "NFS silly rename".
> 
> ##
> 
> I think that to nail down both the problems at once, we may need to do 
> something like this:
> 
> attach_device(const char *dev)
> {
>    /* a cycle to verify that dev is not in use.
>     * If it's in use after the cycle, call tst_brkm()
>     */
>    ioctl(dev_fd, LOOP_SET_FD);
>    /* handle ioctl errors */
> }
> 
> detach_device(const char *dev)
> {
>    ioctl(dev_fd, LOOP_CLR_FD);
>    /* ignore the return code */
> 
>    /* a cycle to verify that dev is not in use.
>     * If it's still in use after the cycle, call tst_brkm()
>     */
> }
> 
> ioctl(LOOP_GET_STATUS64) looks like a good way to check whether the loop 
> device is in use.
> 
> What do you think?


I think that we should do:

detach_device(..)
{
	while EBUSY:
		ioctl(fd, LOOP_CLR_FD);

	while not error:
		ioclt(LOOP_GET_STATUS);
		usleep();
}

Because if we ignore the return value from LOOP_CLR_FD we will break on
older kernels where we may get EBUSSY instead of lazy detach. And the
LOOP_GET_STATUS* ioctls should return error when device is not attached,
at least if I'm reading kernel code right.


-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list