[PATCH v5 0/2] kselftest: tmpfs: Add ksft macros and skip if no root

[PATCH v5 0/2] kselftest: tmpfs: Add ksft macros and skip if no root

[Shivam Chaudhary]

This version 5 patch series replace direct error handling methods with ksft
macros, which provide better reporting.Currently, when the tmpfs test runs,
it does not display any output if it passes,and if it fails
(particularly when not run as root),it simply exits without any warning or
message.

This series of patch adds:

1. Add 'ksft_print_header()' and 'ksft_set_plan()'
   to structure test outputs more effectively.

2. Error if not run as root.

3. Replace direct error handling with 'ksft_test_result_*',
   'ksft_exit_fail_msg' macros for better reporting.

v4->v5:
         - Remove unnecessary pass messages.
         - Remove unnecessary use of KSFT_SKIP.
         - Add appropriate use of ksft_exit_fail_msg.

v4 1/2: https://lore.kernel.org/all/20241105202639.1977356-2-cvam0000@gmail.com/
v4 2/2: https://lore.kernel.org/all/20241105202639.1977356-3-cvam0000@gmail.com/

v3->v4:
         - Start a patchset
         - Split patch into smaller patches to make it easy to review.
  Patch1 Replace  'ksft_test_result_skip' with 'KSFT_SKIP' during root run check.
  Patch2 Replace  'ksft_test_result_fail' with 'KSFT_SKIP' where fail does not make sense,
         or failure could be due to not unsupported APIs with appropriate warnings.


v3: https://lore.kernel.org/all/20241028185756.111832-1-cvam0000@gmail.com/

v2->v3:
        - Remove extra ksft_set_plan()
        - Remove function for unshare()
        - Fix the comment style
v2: https://lore.kernel.org/all/20241026191621.2860376-1-cvam0000@gmail.com/

v1->v2:
        - Make the commit message more clear.
v1: https://lore.kernel.org/all/20241024200228.1075840-1-cvam0000@gmail.com/T/#u


thanks
Shivam

Shivam Chaudhary (2):
  selftests: tmpfs: Add Test-fail if not run as root
  selftests: tmpfs: Add kselftest support to tmpfs

 .../selftests/tmpfs/bug-link-o-tmpfile.c      | 60 ++++++++++++-------
 1 file changed, 37 insertions(+), 23 deletions(-)

-- 
2.45.2

[PATCH v5 1/2] selftests: tmpfs: Add Test-fail if not run as root

[Shivam Chaudhary]

Add 'ksft_exit_fail_msg()', if  not run as root, with an appropriate
Warning.

Add 'ksft_print_header()' and 'ksft_set_plan()' to structure test
outputs more effectively.

Test logs:

Before Change:
- Without root
 error: unshare, errno 1

- With root
 No, output

After Change:
- Without root
TAP version 13
1..1
Bail out! Error : Need to run as root# Planned tests != run tests (1 != 0)

- With root
TAP version 13
1..1

Signed-off-by: Shivam Chaudhary <cvam0000@gmail.com>
---
 tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
index b5c3ddb90942..f0b36e7a152e 100644
--- a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
+++ b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
@@ -23,10 +23,21 @@
 #include <sys/mount.h>
 #include <unistd.h>
 
+#include "../kselftest.h"
+
 int main(void)
 {
 	int fd;
 
+	/* Setting up kselftest framework */
+	ksft_print_header();
+	ksft_set_plan(1);
+
+	/* Check if test is run as root */
+	if (geteuid()) {
+		ksft_exit_fail_msg("Error : Need to run as root");
+	}
+
 	if (unshare(CLONE_NEWNS) == -1) {
 		if (errno == ENOSYS || errno == EPERM) {
 			fprintf(stderr, "error: unshare, errno %d\n", errno);
-- 
2.45.2

[PATCH v5 2/2] selftests: tmpfs: Add kselftest support to tmpfs

[Shivam Chaudhary]

Replace direct error handling with 'ksft_test_result_*',
'ksft_exit_fail_msg' macros for better reporting.

Test logs:

Before change:
- Without root
 error: unshare, errno 1

- With root
 No, output

After change:
- Without root
TAP version 13
1..1
Bail out! Error : Need to run as root# Planned tests != run tests (1 != 0)
Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0

- With root
TAP version 13
1..1
ok 1 Test : Success
Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0

Signed-off-by: Shivam Chaudhary <cvam0000@gmail.com>
---
 .../selftests/tmpfs/bug-link-o-tmpfile.c      | 49 ++++++++++---------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
index f0b36e7a152e..c3a8c9c50c8a 100644
--- a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
+++ b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
@@ -40,39 +40,42 @@ int main(void)
 
 	if (unshare(CLONE_NEWNS) == -1) {
 		if (errno == ENOSYS || errno == EPERM) {
-			fprintf(stderr, "error: unshare, errno %d\n", errno);
-			return 4;
+			ksft_test_result_skip("unshare() error: unshare, errno %d\n", errno);	
 		}
-		fprintf(stderr, "error: unshare, errno %d\n", errno);
-		return 1;
-	}
-	if (mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL) == -1) {
-		fprintf(stderr, "error: mount '/', errno %d\n", errno);
-		return 1;
-	}
+		else{
+			ksft_exit_fail_msg("unshare() error: unshare, errno %d\n", errno);
+		}
+	} 
+	
+	if (mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) == -1) {
+		ksft_exit_fail_msg("mount() error: Root filesystem private mount: Fail %d\n", errno);
+	} 
+
 
 	/* Our heroes: 1 root inode, 1 O_TMPFILE inode, 1 permanent inode. */
 	if (mount(NULL, "/tmp", "tmpfs", 0, "nr_inodes=3") == -1) {
-		fprintf(stderr, "error: mount tmpfs, errno %d\n", errno);
-		return 1;
-	}
+		ksft_exit_fail_msg("mount() error: Mounting tmpfs on /tmp: Fail %d\n", errno);
+	} 
 
-	fd = openat(AT_FDCWD, "/tmp", O_WRONLY|O_TMPFILE, 0600);
+	fd = openat(AT_FDCWD, "/tmp", O_WRONLY | O_TMPFILE, 0600);
 	if (fd == -1) {
-		fprintf(stderr, "error: open 1, errno %d\n", errno);
-		return 1;
-	}
+		ksft_exit_fail_msg("openat() error: Open first temporary file: Fail %d\n", errno);
+	} 
+
 	if (linkat(fd, "", AT_FDCWD, "/tmp/1", AT_EMPTY_PATH) == -1) {
-		fprintf(stderr, "error: linkat, errno %d\n", errno);
-		return 1;
-	}
+		ksft_exit_fail_msg("linkat() error: Linking the temporary file: Fail %d\n", errno);
+		/* Ensure fd is closed on failure */
+		close(fd); 
+	} 
 	close(fd);
 
-	fd = openat(AT_FDCWD, "/tmp", O_WRONLY|O_TMPFILE, 0600);
+
+	fd = openat(AT_FDCWD, "/tmp", O_WRONLY | O_TMPFILE, 0600);
 	if (fd == -1) {
-		fprintf(stderr, "error: open 2, errno %d\n", errno);
-		return 1;
-	}
+		ksft_exit_fail_msg("openat() error: Opening the second temporary file: Fail %d\n", errno);
+	} 
 
+    ksft_test_result_pass("Test : Success\n");
+	ksft_exit_pass();
 	return 0;
 }
-- 
2.45.2

Re: [PATCH v5 1/2] selftests: tmpfs: Add Test-fail if not run as root

[Shuah Khan]

On 11/12/24 07:30, Shivam Chaudhary wrote:
> Add 'ksft_exit_fail_msg()', if  not run as root, with an appropriate
> Warning.

This should be a skip - not a fail.

> 
> Add 'ksft_print_header()' and 'ksft_set_plan()' to structure test
> outputs more effectively.
> 
> Test logs:
> 
> Before Change:
> - Without root
>   error: unshare, errno 1
> 
> - With root
>   No, output
> 
> After Change:
> - Without root
> TAP version 13
> 1..1
> Bail out! Error : Need to run as root# Planned tests != run tests (1 != 0)
> 
> - With root
> TAP version 13
> 1..1
> 
> Signed-off-by: Shivam Chaudhary <cvam0000@gmail.com>
> ---
>   tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
> index b5c3ddb90942..f0b36e7a152e 100644
> --- a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
> +++ b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
> @@ -23,10 +23,21 @@
>   #include <sys/mount.h>
>   #include <unistd.h>
>   
> +#include "../kselftest.h"
> +
>   int main(void)
>   {
>   	int fd;
>   
> +	/* Setting up kselftest framework */
> +	ksft_print_header();
> +	ksft_set_plan(1);
> +
> +	/* Check if test is run as root */
> +	if (geteuid()) {
> +		ksft_exit_fail_msg("Error : Need to run as root");

Use ksft_skip call.

> +	}
> +
>   	if (unshare(CLONE_NEWNS) == -1) {
>   		if (errno == ENOSYS || errno == EPERM) {
>   			fprintf(stderr, "error: unshare, errno %d\n", errno);

thanks,
-- Shuah

Re: [PATCH v5 0/2] kselftest: tmpfs: Add ksft macros and skip if no root

[Shuah Khan]

On 11/12/24 07:30, Shivam Chaudhary wrote:
> This version 5 patch series replace direct error handling methods with ksft
> macros, which provide better reporting.Currently, when the tmpfs test runs,
> it does not display any output if it passes,and if it fails
> (particularly when not run as root),it simply exits without any warning or
> message.
> 
> This series of patch adds:
> 
> 1. Add 'ksft_print_header()' and 'ksft_set_plan()'
>     to structure test outputs more effectively.
> 
> 2. Error if not run as root.

This should be a skip - not a fail.

> 
> 3. Replace direct error handling with 'ksft_test_result_*',
>     'ksft_exit_fail_msg' macros for better reporting.


> 
> v4->v5:
>           - Remove unnecessary pass messages.
>           - Remove unnecessary use of KSFT_SKIP.
>           - Add appropriate use of ksft_exit_fail_msg.
> 
> v4 1/2: https://lore.kernel.org/all/20241105202639.1977356-2-cvam0000@gmail.com/
> v4 2/2: https://lore.kernel.org/all/20241105202639.1977356-3-cvam0000@gmail.com/
> 
> v3->v4:
>           - Start a patchset
>           - Split patch into smaller patches to make it easy to review.
>    Patch1 Replace  'ksft_test_result_skip' with 'KSFT_SKIP' during root run check.
>    Patch2 Replace  'ksft_test_result_fail' with 'KSFT_SKIP' where fail does not make sense,
>           or failure could be due to not unsupported APIs with appropriate warnings.
> 
> 
> v3: https://lore.kernel.org/all/20241028185756.111832-1-cvam0000@gmail.com/
> 
> v2->v3:
>          - Remove extra ksft_set_plan()
>          - Remove function for unshare()
>          - Fix the comment style
> v2: https://lore.kernel.org/all/20241026191621.2860376-1-cvam0000@gmail.com/
> 
> v1->v2:
>          - Make the commit message more clear.
> v1: https://lore.kernel.org/all/20241024200228.1075840-1-cvam0000@gmail.com/T/#u
> 
> 
> thanks
> Shivam
> 
> Shivam Chaudhary (2):
>    selftests: tmpfs: Add Test-fail if not run as root
>    selftests: tmpfs: Add kselftest support to tmpfs
> 
>   .../selftests/tmpfs/bug-link-o-tmpfile.c      | 60 ++++++++++++-------
>   1 file changed, 37 insertions(+), 23 deletions(-)
> 

thanks,
-- Shuah

Re: [PATCH v5 2/2] selftests: tmpfs: Add kselftest support to tmpfs

[Shuah Khan]

On 11/12/24 07:30, Shivam Chaudhary wrote:
> Replace direct error handling with 'ksft_test_result_*',
> 'ksft_exit_fail_msg' macros for better reporting.
> 

Don't fail the test - skip is the right exit code in this case.

> Test logs:
> 
> Before change:
> - Without root
>   error: unshare, errno 1
> 
> - With root
>   No, output
> 
> After change:
> - Without root
> TAP version 13
> 1..1
> Bail out! Error : Need to run as root# Planned tests != run tests (1 != 0)

This should be a skip -- see above.

> Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> - With root
> TAP version 13
> 1..1
> ok 1 Test : Success
> Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> 
> Signed-off-by: Shivam Chaudhary <cvam0000@gmail.com>
> ---
>   .../selftests/tmpfs/bug-link-o-tmpfile.c      | 49 ++++++++++---------
>   1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
> index f0b36e7a152e..c3a8c9c50c8a 100644
> --- a/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
> +++ b/tools/testing/selftests/tmpfs/bug-link-o-tmpfile.c
> @@ -40,39 +40,42 @@ int main(void)
>   
>   	if (unshare(CLONE_NEWNS) == -1) {
>   		if (errno == ENOSYS || errno == EPERM) {
> -			fprintf(stderr, "error: unshare, errno %d\n", errno);
> -			return 4;
> +			ksft_test_result_skip("unshare() error: unshare, errno %d\n", errno);

This looks right.

	
>   		}
> -		fprintf(stderr, "error: unshare, errno %d\n", errno);
> -		return 1;
> -	}
> -	if (mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL) == -1) {
> -		fprintf(stderr, "error: mount '/', errno %d\n", errno);
> -		return 1;
> -	}
> +		else{
> +			ksft_exit_fail_msg("unshare() error: unshare, errno %d\n", errno);
> +		}
> +	}
> +	
> +	if (mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) == -1) {
> +		ksft_exit_fail_msg("mount() error: Root filesystem private mount: Fail %d\n", errno);
> +	}
> +
>   
>   	/* Our heroes: 1 root inode, 1 O_TMPFILE inode, 1 permanent inode. */
>   	if (mount(NULL, "/tmp", "tmpfs", 0, "nr_inodes=3") == -1) {
> -		fprintf(stderr, "error: mount tmpfs, errno %d\n", errno);
> -		return 1;
> -	}
> +		ksft_exit_fail_msg("mount() error: Mounting tmpfs on /tmp: Fail %d\n", errno);
> +	}
>   
> -	fd = openat(AT_FDCWD, "/tmp", O_WRONLY|O_TMPFILE, 0600);
> +	fd = openat(AT_FDCWD, "/tmp", O_WRONLY | O_TMPFILE, 0600);
>   	if (fd == -1) {
> -		fprintf(stderr, "error: open 1, errno %d\n", errno);
> -		return 1;
> -	}
> +		ksft_exit_fail_msg("openat() error: Open first temporary file: Fail %d\n", errno);
> +	}
> +
>   	if (linkat(fd, "", AT_FDCWD, "/tmp/1", AT_EMPTY_PATH) == -1) {
> -		fprintf(stderr, "error: linkat, errno %d\n", errno);
> -		return 1;
> -	}
> +		ksft_exit_fail_msg("linkat() error: Linking the temporary file: Fail %d\n", errno);
> +		/* Ensure fd is closed on failure */
> +		close(fd);
> +	}
>   	close(fd);
>   
> -	fd = openat(AT_FDCWD, "/tmp", O_WRONLY|O_TMPFILE, 0600);
> +
> +	fd = openat(AT_FDCWD, "/tmp", O_WRONLY | O_TMPFILE, 0600);
>   	if (fd == -1) {
> -		fprintf(stderr, "error: open 2, errno %d\n", errno);
> -		return 1;
> -	}
> +		ksft_exit_fail_msg("openat() error: Opening the second temporary file: Fail %d\n", errno);
> +	}
>   
> +    ksft_test_result_pass("Test : Success\n");
> +	ksft_exit_pass();
>   	return 0;
>   }

The rest looks good.

thanks,
-- Shuah