zram/zram_lib.sh: Check fielsystem support more throughly

PATCH New Xiao Yang 2 participants 10 messages Jul 11, 2018 – Jul 23, 2018 Library code

Share & tools

Xiao Yang

author

2018-07-11 08:55 UTC #0 lore raw

PATCH 1/1 zram/zram_lib.sh: Check fielsystem support more throughly

When mkfs command supports a specified filesystem and kernel doesn't support it, mkfs can format zram device to the filesystem successfully, but mount will fail with "unknown filesystem type".

For example, running zram01 got the following error when only mkfs supported btrfs filesystem:
---------------------------------------------------------------------------------
mount: /tmp/ltp-aJSv2PMZGl/zram01.46mBiYaa3M/zram3: unknown filesystem type 'btrfs'.
---------------------------------------------------------------------------------

We should add a fielsystem support check for both kernel and mkfs command, and use ext2 fielsystem by default if either of them doesn't support a specified filesystem.

Signed-off-by Xiao Yang <[email protected]>

---
 testcases/kernel/device-drivers/zram/zram_lib.sh | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

testcases/kernel/device-drivers/zram/zram_lib.sh +16 -2

index 224b407..d6ce74a 100755
@@ -193,13 +193,27 @@ zram_swapoff()
 	tst_resm TPASS "swapoff completed"
 }
 
+zram_check_fs_support()
+{
+	tst_check_cmds which modprobe
+	local filesystem=$1
+
+	# Check if mkfs command supports fs
+	which mkfs.$filesystem > /dev/null 2>&1 || return 1
+
+	# Check if kernel supports fs
+	if ! grep -qw $filesystem /proc/filesystems; then
+		modprobe $filesystem > /dev/null 2>&1 || return 1
+	fi
+}
+
 zram_makefs()
 {
-	tst_check_cmds mkfs which
+	tst_check_cmds mkfs
 	local i=0
 	for fs in $zram_filesystems; do
 		# if requested fs not supported default it to ext2
-		which mkfs.$fs > /dev/null 2>&1 || fs=ext2
+		zram_check_fs_support $fs || fs=ext2
 
 		tst_resm TINFO "make $fs filesystem on /dev/zram$i"
 		mkfs.$fs /dev/zram$i > err.log 2>&1

Signature · 1 line

1.8.3.1

Cyril Hrubis

2018-07-18 12:46 UTC #1 lore raw

PATCH 1/1 zram/zram_lib.sh: Check fielsystem support more throughly

Hi!

testcases/kernel/device-drivers/zram/zram_lib.sh zram_swapoff() · line 205

Quoted 10 lines

testcases/kernel/device-drivers/zram/zram_lib.sh +10 -0

+zram_check_fs_support()
+{
+	tst_check_cmds which modprobe
+	local filesystem=$1
+
+	# Check if mkfs command supports fs
+	which mkfs.$filesystem > /dev/null 2>&1 || return 1
+
+	# Check if kernel supports fs
+	if ! grep -qw $filesystem /proc/filesystems; then

Actually grepping /proc/filesystems is not enough to figure out if filesystem is supported or not, since for example ext2 and ext3 are handled with ext4 these days.

But happily we do already have all the code in tst_supported_fs_types.c in the test library, we all that is needed to to do is to create a C helper on the top of tst_get_supported_fs_types() function that returns array of supported filesystems. So we can write a small helper binary on the top of that.

I would be for adding a tst_supported_fs binary into the testcases/lib/ that would, without any parameter print the list of supported filesystems into the stdout. and if passed a fs type as a parameter would use return value to indicate if the filesystem is supported or not.

Quoted 24 lines

testcases/kernel/device-drivers/zram/zram_lib.sh +6 -2

+		modprobe $filesystem > /dev/null 2>&1 || return 1
+	fi
+}
+
 zram_makefs()
 {
-	tst_check_cmds mkfs which
+	tst_check_cmds mkfs
 	local i=0
 	for fs in $zram_filesystems; do
 		# if requested fs not supported default it to ext2
-		which mkfs.$fs > /dev/null 2>&1 || fs=ext2
+		zram_check_fs_support $fs || fs=ext2
 
 		tst_resm TINFO "make $fs filesystem on /dev/zram$i"
 		mkfs.$fs /dev/zram$i > err.log 2>&1

Signature · 7 lines

1.8.3.1




-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Signature · 2 lines

Cyril Hrubis
[email protected]

Xiao Yang

author

2018-07-19 07:31 UTC #2 lore raw

PATCH v2 1/2 lib: Factor out is_supported() && Add tst_supported_fs for shell

1) Factor out is_supported() and rename it as tst_fs_is_supported(), so that some tests can check a specified filesystem by it.

2) Shell tests can check a specified filesystem or all supported fielsystems by tst_supported_fs binary.

Signed-off-by Xiao Yang <[email protected]>

---
 include/tst_fs.h                 |  8 ++++++
 lib/tst_supported_fs_types.c     |  6 ++--
 testcases/lib/Makefile           |  2 +-
 testcases/lib/tst_supported_fs.c | 61 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 testcases/lib/tst_supported_fs.c

include/tst_fs.h +8 -0

index 73916a6..599f28f 100644
@@ -49,6 +49,8 @@ enum {
 	TST_GB = 1073741824,
 };
 
+extern const char *const fs_type_whitelist[];
+
 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB

@@ -147,6 +149,12 @@ int tst_get_path(const char *prog_name, char *buf, size_t buf_len);
 int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
 
 /*
+ * Return 1 if a specified fiilsystem is supported
+ * Return 0 if a specified fiilsystem isn't supported
+ */
+int tst_fs_is_supported(const char *fs_type);
+
+/*
  * Returns NULL-terminated array of kernel-supported filesystems.
  */
 const char **tst_get_supported_fs_types(void);

lib/tst_supported_fs_types.c +3 -3

index a23b1ed..c7123b2 100644
@@ -25,7 +25,7 @@
 #include "tst_test.h"
 #include "tst_fs.h"
 
-static const char *const fs_type_whitelist[] = {
+const char *const fs_type_whitelist[] = {
 	"ext2",
 	"ext3",
 	"ext4",

@@ -100,7 +100,7 @@ static int has_kernel_support(const char *fs_type)
 	return 1;
 }
 
-static int is_supported(const char *fs_type)
+int tst_fs_is_supported(const char *fs_type)
 {
 	return has_kernel_support(fs_type) && has_mkfs(fs_type);
 }

@@ -110,7 +110,7 @@ const char **tst_get_supported_fs_types(void)
 	unsigned int i, j = 0;
 
 	for (i = 0; fs_type_whitelist[i]; i++) {
-		if (is_supported(fs_type_whitelist[i]))
+		if (tst_fs_is_supported(fs_type_whitelist[i]))
 			fs_types[j++] = fs_type_whitelist[i];
 	}

testcases/lib/Makefile +1 -1

index 398150a..14352d5 100644
@@ -26,7 +26,7 @@ include $(top_srcdir)/include/mk/testcases.mk
 
 INSTALL_TARGETS		:= *.sh
 
-MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
+MAKE_TARGETS		:= tst_supported_fs tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk

testcases/lib/tst_supported_fs.c new file +61 -0

61 lines not on this page · raw message

Signature · 1 line

1.8.3.1

Xiao Yang

author

2018-07-19 07:31 UTC #3 lore raw

PATCH v2 2/2 zram/zram_lib.sh: Apply tst_supported_fs for shell

When mkfs command supports a specified filesystem and kernel doesn't support it, mkfs can format zram device to the filesystem successfully, but mount will fail with "unknown filesystem type".

We should check for both kernel and mkfs command by tst_supported_fs, and use ext2 fielsystem by default if either of them doesn't support a specified filesystem.

Signed-off-by Xiao Yang <[email protected]>

---
 testcases/kernel/device-drivers/zram/zram_lib.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

testcases/kernel/device-drivers/zram/zram_lib.sh +3 -2

index 224b407..0203c4d 100755
@@ -195,11 +195,12 @@ zram_swapoff()
 
 zram_makefs()
 {
-	tst_check_cmds mkfs which
+	tst_check_cmds mkfs
 	local i=0
 	for fs in $zram_filesystems; do
 		# if requested fs not supported default it to ext2
-		which mkfs.$fs > /dev/null 2>&1 || fs=ext2
+		tst_supported_fs $fs > /dev/null 2>&1
+		[ $? -eq 0 ] && fs=ext2
 
 		tst_resm TINFO "make $fs filesystem on /dev/zram$i"
 		mkfs.$fs /dev/zram$i > err.log 2>&1

Signature · 1 line

1.8.3.1

Cyril Hrubis

2018-07-19 12:27 UTC #4 lore raw

PATCH v2 2/2 zram/zram_lib.sh: Apply tst_supported_fs for shell

Hi!

testcases/kernel/device-drivers/zram/zram_lib.sh zram_swapoff() · line 203

Quoted 4 lines

testcases/kernel/device-drivers/zram/zram_lib.sh +2 -1

 		# if requested fs not supported default it to ext2
-		which mkfs.$fs > /dev/null 2>&1 || fs=ext2
+		tst_supported_fs $fs > /dev/null 2>&1
+		[ $? -eq 0 ] && fs=ext2

Should just:

	tst_supported_fs $fs || fs=ext2

suffice here?

(assuming that tst_supported_fs returns 0 on success)

Signature · 2 lines

Cyril Hrubis
[email protected]

Cyril Hrubis

2018-07-19 12:21 UTC #5 lore raw

PATCH v2 1/2 lib: Factor out is_supported() && Add tst_supported_fs for shell

In reply to Xiao Yang

Hi!

include/tst_fs.h enum { · line 52

Quoted 9 lines

include/tst_fs.h +1 -0

index 73916a6..599f28f 100644
@@ -49,6 +49,8 @@ enum {
 	TST_GB = 1073741824,
 };
 
+extern const char *const fs_type_whitelist[];

I do not think that we have to check the fs type against the whitelist before we call tst_fs_is_supported() function. If we pass invalid fs we will end up with "Filesystem foo is not supported" message anyways.

Quoted 86 lines

include/tst_fs.h +6 -0

 /*
  * @path: path is the pathname of any file within the mounted file system
  * @mult: mult should be TST_KB, TST_MB or TST_GB

@@ -147,6 +149,12 @@ int tst_get_path(const char *prog_name, char *buf, size_t buf_len);
 int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
 
 /*
+ * Return 1 if a specified fiilsystem is supported
+ * Return 0 if a specified fiilsystem isn't supported
+ */
+int tst_fs_is_supported(const char *fs_type);
+
+/*
  * Returns NULL-terminated array of kernel-supported filesystems.
  */
 const char **tst_get_supported_fs_types(void);

lib/tst_supported_fs_types.c +3 -3

index a23b1ed..c7123b2 100644
@@ -25,7 +25,7 @@
 #include "tst_test.h"
 #include "tst_fs.h"
 
-static const char *const fs_type_whitelist[] = {
+const char *const fs_type_whitelist[] = {
 	"ext2",
 	"ext3",
 	"ext4",

@@ -100,7 +100,7 @@ static int has_kernel_support(const char *fs_type)
 	return 1;
 }
 
-static int is_supported(const char *fs_type)
+int tst_fs_is_supported(const char *fs_type)
 {
 	return has_kernel_support(fs_type) && has_mkfs(fs_type);
 }

@@ -110,7 +110,7 @@ const char **tst_get_supported_fs_types(void)
 	unsigned int i, j = 0;
 
 	for (i = 0; fs_type_whitelist[i]; i++) {
-		if (is_supported(fs_type_whitelist[i]))
+		if (tst_fs_is_supported(fs_type_whitelist[i]))
 			fs_types[j++] = fs_type_whitelist[i];
 	}

testcases/lib/Makefile +1 -1

index 398150a..14352d5 100644
@@ -26,7 +26,7 @@ include $(top_srcdir)/include/mk/testcases.mk
 
 INSTALL_TARGETS		:= *.sh
 
-MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
+MAKE_TARGETS		:= tst_supported_fs tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk

testcases/lib/tst_supported_fs.c new file +20 -0

new file mode 100644
index 0000000..df9ea48
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2018 FUJITSU LIMITED. All rights reserved.
+ * Author: Xiao Yang <[email protected]>
+ */
+
+#include <errno.h>
+#include <stdio.h>
+#include <string.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_fs.h"
+
+static void usage(void)
+{
+	printf("Usage: tst_supported_fs [fs_type]\n");
+	printf("   If a specified fs_type is supported, return 1\n");
+	printf("   If a specified fs_type isn't supported, return 0\n");
+	printf("   If a fs_type isn't specified, print the list of supported filesystems\n");

testcases/lib/tst_supported_fs.c line 30

Quoted 10 lines

testcases/lib/tst_supported_fs.c new file +10 -0

+	printf("   fs_type - a specified filesystem type\n");
+}
+
+int main(int argc, char *argv[])
+{
+	const char *const *filesystems;
+	int i, valid_flag = 0;
+
+	if (argc > 2) {
+		printf("Can't specify multiple fs_type\n");

Error messages and help should rather go to stderr.

testcases/lib/tst_supported_fs.c line 37

Quoted 7 lines

testcases/lib/tst_supported_fs.c new file +7 -0

+		usage();
+		return 2;
+	}
+
+	if (argv[1] && !strcmp(argv[1], "-h")) {
+		usage();
+		return 2;

This should probably be return 0 but that is minor.

Quoted 8 lines

testcases/lib/tst_supported_fs.c new file +8 -0

+	}
+
+	if (argv[1]) {
+		for (i = 0; fs_type_whitelist[i]; i++) {
+			if (!strcmp(argv[1], fs_type_whitelist[i])) {
+				valid_flag = 1;
+				break;
+			}

testcases/lib/tst_supported_fs.c line 55

Quoted 10 lines

testcases/lib/tst_supported_fs.c new file +10 -0

+		}
+
+		if (valid_flag)
+			return tst_fs_is_supported(argv[1]);
+
+		printf("An invalid specified fs_type is %s\n", argv[1]);
+		return 2;
+	}
+
+	printf("List of supported fs_type:\n");

This message here would make it harder to use the helper actually, since I would expect that we would like to use this as:

	for i in $(tst_supported_fs); do
		# Run the actual test for each filesystem
	done

testcases/lib/tst_supported_fs.c line 60

Quoted 5 lines

testcases/lib/tst_supported_fs.c new file +5 -0

+	filesystems = tst_get_supported_fs_types();
+	for (i = 0; filesystems[i]; i++)
+		printf("%s\n", filesystems[i]);
+
+	return 2;

This should probably be return 0 as well.

Quoted 6 lines

testcases/lib/tst_supported_fs.c new file +1 -0
+}
Signature · 1 line
1.8.3.1

Signature · 2 lines

Cyril Hrubis
[email protected]

Xiao Yang

author

2018-07-20 10:40 UTC #6 lore raw

PATCH v3 1/2 lib: Factor out is_supported() && Add tst_supported_fs

1) Factor out is_supported() and rename it as tst_fs_is_supported(), so that some tests can check a specified filesystem by it.

2) Add tst_supported_fs binary for shell tests to check a specified filesystem or all supported fielsystems.

PS: hint messages from tst_supported_fs binary is targeted as stderr,
so we can filter them by "2> /dev/null".

Signed-off-by Xiao Yang <[email protected]>

---
 include/tst_fs.h                 |  6 +++++
 lib/tst_supported_fs_types.c     |  4 ++--
 testcases/lib/Makefile           |  2 +-
 testcases/lib/tst_supported_fs.c | 47 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 testcases/lib/tst_supported_fs.c

include/tst_fs.h +6 -0

index 73916a6..8d3f1cf 100644
@@ -147,6 +147,12 @@ int tst_get_path(const char *prog_name, char *buf, size_t buf_len);
 int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
 
 /*
+ * Return 1 if a specified fiilsystem is supported
+ * Return 0 if a specified fiilsystem isn't supported
+ */
+int tst_fs_is_supported(const char *fs_type);
+
+/*
  * Returns NULL-terminated array of kernel-supported filesystems.
  */
 const char **tst_get_supported_fs_types(void);

lib/tst_supported_fs_types.c +2 -2

index a23b1ed..b7e647f 100644
@@ -100,7 +100,7 @@ static int has_kernel_support(const char *fs_type)
 	return 1;
 }
 
-static int is_supported(const char *fs_type)
+int tst_fs_is_supported(const char *fs_type)
 {
 	return has_kernel_support(fs_type) && has_mkfs(fs_type);
 }

@@ -110,7 +110,7 @@ const char **tst_get_supported_fs_types(void)
 	unsigned int i, j = 0;
 
 	for (i = 0; fs_type_whitelist[i]; i++) {
-		if (is_supported(fs_type_whitelist[i]))
+		if (tst_fs_is_supported(fs_type_whitelist[i]))
 			fs_types[j++] = fs_type_whitelist[i];
 	}

testcases/lib/Makefile +1 -1

index 398150a..14352d5 100644
@@ -26,7 +26,7 @@ include $(top_srcdir)/include/mk/testcases.mk
 
 INSTALL_TARGETS		:= *.sh
 
-MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
+MAKE_TARGETS		:= tst_supported_fs tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
 
 include $(top_srcdir)/include/mk/generic_leaf_target.mk

testcases/lib/tst_supported_fs.c new file +47 -0

47 lines not on this page · raw message

Signature · 1 line

1.8.3.1

Xiao Yang

author

2018-07-20 10:40 UTC #7 lore raw

PATCH v3 2/2 zram/zram_lib.sh: Apply tst_supported_fs for shell

When mkfs command supports a specified filesystem and kernel doesn't support it, mkfs can format zram device to the filesystem successfully, but mount will fail with "unknown filesystem type".

We should check both kernel and mkfs command by tst_supported_fs binary, and use ext2 fielsystem by default if either of them doesn't support a specified filesystem.

Signed-off-by Xiao Yang <[email protected]>

---
 testcases/kernel/device-drivers/zram/zram_lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

testcases/kernel/device-drivers/zram/zram_lib.sh +2 -2

index 224b407..36d1603 100755
@@ -195,11 +195,11 @@ zram_swapoff()
 
 zram_makefs()
 {
-	tst_check_cmds mkfs which
+	tst_check_cmds mkfs
 	local i=0
 	for fs in $zram_filesystems; do
 		# if requested fs not supported default it to ext2
-		which mkfs.$fs > /dev/null 2>&1 || fs=ext2
+		tst_supported_fs $fs 2> /dev/null || fs=ext2
 
 		tst_resm TINFO "make $fs filesystem on /dev/zram$i"
 		mkfs.$fs /dev/zram$i > err.log 2>&1

Signature · 1 line

1.8.3.1

Cyril Hrubis

2018-07-23 14:52 UTC #8 lore raw

PATCH v3 1/2 lib: Factor out is_supported() && Add tst_supported_fs

In reply to Xiao Yang

Hi!
Both pushed, thanks.

Signature · 2 lines

Cyril Hrubis
[email protected]

Cyril Hrubis

2018-07-19 12:27 UTC #9 lore raw

PATCH v2 1/2 lib: Factor out is_supported() && Add tst_supported_fs for shell

In reply to Xiao Yang

Hi!

testcases/lib/tst_supported_fs.c line 19

Quoted 3 lines

testcases/lib/tst_supported_fs.c new file +3 -0

+	printf("Usage: tst_supported_fs [fs_type]\n");
+	printf("   If a specified fs_type is supported, return 1\n");
+	printf("   If a specified fs_type isn't supported, return 0\n");

Also this is a bit confusing since in shell 0 == true, hence it would be a bit better to return !tst_fs_is_supported(argv[1]);

Signature · 2 lines

Cyril Hrubis
[email protected]