[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

Test mkfs(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                  |  10 ++
 testcases/commands/mkfs/Makefile  |  24 ++++
 testcases/commands/mkfs/mkfs01.sh | 236 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 270 insertions(+)
 create mode 100644 testcases/commands/mkfs/Makefile
 create mode 100755 testcases/commands/mkfs/mkfs01.sh

diff --git a/runtest/commands b/runtest/commands
index e5260ba..6c0485b 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -28,3 +28,13 @@ df01_xfs df01.sh -f xfs
 df01_vfat df01.sh -f vfat
 df01_exfat df01.sh -f exfat
 df01_ntfs df01.sh -f ntfs
+mkfs01 mkfs01.sh
+mkfs01_ext2 mkfs01.sh -f ext2
+mkfs01_ext3 mkfs01.sh -f ext3
+mkfs01_ext4 mkfs01.sh -f ext4
+mkfs01_xfs mkfs01.sh -f xfs
+mkfs01_btrfs mkfs01.sh -f btrfs
+mkfs01_minix mkfs01.sh -f minix
+mkfs01_msdos mkfs01.sh -f msdos
+mkfs01_vfat mkfs01.sh -f vfat
+mkfs01_ntfs mkfs01.sh -f ntfs
diff --git a/testcases/commands/mkfs/Makefile b/testcases/commands/mkfs/Makefile
new file mode 100644
index 0000000..6099017
--- /dev/null
+++ b/testcases/commands/mkfs/Makefile
@@ -0,0 +1,24 @@
+#
+#    commands/mkfs testcases Makefile.
+#
+#    Copyright (c) 2015 Fujitsu Ltd.
+#    Author:Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+#
+#    This program is free software; you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License as published by
+#    the Free Software Foundation; either version 2 of the License, or
+#    (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+
+top_srcdir		?= ../../..
+
+include $(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_TARGETS		:= mkfs01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkfs/mkfs01.sh b/testcases/commands/mkfs/mkfs01.sh
new file mode 100755
index 0000000..3d2f92e
--- /dev/null
+++ b/testcases/commands/mkfs/mkfs01.sh
@@ -0,0 +1,236 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Fujitsu Ltd.
+# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+# the GNU General Public License for more details.
+#
+# Test mkfs command with some basic options.
+#
+
+TCID=mkfs01
+TST_TOTAL=5
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds blkid df
+
+	if [ -n "$FS_TYPE" ]; then
+		tst_check_cmds mkfs.${FS_TYPE}
+	fi
+
+	tst_tmpdir
+
+	TST_CLEANUP="cleanup"
+
+	ROD_SILENT mkdir -p mntpoint
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkfs_mount()
+{
+	mount ${TST_DEVICE} mntpoint
+	local ret=$?
+	if [ $ret -eq 32 ]; then
+		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
+	fi
+
+	if [ $ret -ne 0 ]; then
+		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
+	fi
+}
+
+mkfs_umount()
+{
+	grep -q ${TST_DEVICE} /proc/self/mounts
+	if [ $? -eq 0 ]; then
+		umount ${TST_DEVICE}
+		if [ $? -ne 0 ];then
+			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
+		fi
+	else
+		tst_resm TINFO "${TST_DEVICE} is not mounted"
+	fi
+}
+
+usage()
+{
+	cat << EOF
+	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
+
+	OPTIONS
+		-f	Specify the type of filesystem to be built. If not
+			specified, the default filesystem type (currently ext2)
+			is used.
+		-h	Display help text and exit.
+
+EOF
+	tst_brkm TCONF "Display help text or unknown options"
+}
+
+mkfs_verify()
+{
+	if [ -z "$1" ]; then
+		blkid | grep "$2" | grep -q "ext2"
+	else
+		blkid | grep "$2" | grep -q "$1"
+	fi
+}
+
+mkfs_check()
+{
+	mkfs_mount
+	local blocknum=`df -aT | grep "$1" | awk '{print $3}'`
+	mkfs_umount
+
+	if [ $blocknum -gt "$2" ]; then
+		return 1
+	fi
+}
+
+mkfs_test()
+{
+	local mkfs_op=$1
+	local fs_type=$2
+	local fs_op=$3
+	local device=$4
+	local size=$5
+
+	if [ -n "$fs_type" ]; then
+		mkfs_op="-t"
+	fi
+
+	if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
+		fs_op+=" -f"
+	fi
+
+	local mkfs_cmd="mkfs $mkfs_op $fs_type $fs_op $device $size"
+	mkfs_cmd=`echo "$mkfs_cmd" | sed 's/\s\+/ /g'`
+
+	echo ${fs_op} | grep -q "\-c"
+	if [ $? -eq 0 ] && [ "$fs_type" = "ntfs" ]; then
+		tst_resm TCONF "'${mkfs_cmd}' not supported."
+		return 32
+	fi
+
+	if [ -n "$size" ]; then
+		if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
+			tst_resm TCONF "'${mkfs_cmd}' not supported."
+			return 32
+		fi
+	fi
+
+	${mkfs_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option | invalid option" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkfs_cmd}' not supported."
+			return 32
+		else
+			tst_resm TFAIL "'${mkfs_cmd}' failed."
+			cat temp
+			return 1
+		fi
+	fi
+
+	if [ -n "$device" ]; then
+		mkfs_verify "$fs_type" "$device"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
+			return 1
+		fi
+	fi
+
+	if [ -n "$size" ]; then
+		mkfs_check "$device" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
+			return 1
+		fi
+	fi
+
+	tst_resm TPASS "'${mkfs_cmd}' passed."
+}
+
+test1()
+{
+	tst_acquire_device
+
+	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE"
+
+	tst_release_device
+}
+
+test2()
+{
+	tst_acquire_device
+
+	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE" "10000"
+
+	tst_release_device
+}
+
+test3()
+{
+	tst_acquire_device
+
+	mkfs_test "$MKFS_OP" "$FS_TYPE" "-c" "$TST_DEVICE"
+
+	tst_release_device
+}
+
+test4()
+{
+	mkfs_test "-V"
+}
+
+test5()
+{
+	mkfs_test "-h"
+}
+
+MKFS_OP=""
+FS_TYPE=""
+FS_OP=""
+
+if [ $# -ne 0 ]; then
+	while getopts f:h OPTION; do
+		case $OPTION in
+		f)
+			FS_TYPE=$OPTARG;;
+		h)
+			usage;;
+		?)
+			usage;;
+		esac
+	done
+
+	if [ -z "$FS_TYPE" ]; then
+		usage
+	fi
+fi
+
+setup
+for i in $(seq 1 ${TST_TOTAL})
+do
+	test$i
+done
+
+tst_exit
-- 
1.8.4.2

[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[Cyril Hrubis]

> +#!/bin/sh
> +#
> +# Copyright (c) 2015 Fujitsu Ltd.
> +# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> +# the GNU General Public License for more details.
> +#
> +# Test mkfs command with some basic options.
> +#
> +
> +TCID=mkfs01
> +TST_TOTAL=5
> +. test.sh
> +
> +setup()
> +{
> +	tst_require_root
> +
> +	tst_check_cmds blkid df
> +
> +	if [ -n "$FS_TYPE" ]; then
> +		tst_check_cmds mkfs.${FS_TYPE}
> +	fi
> +
> +	tst_tmpdir
> +
> +	TST_CLEANUP="cleanup"
> +
> +	ROD_SILENT mkdir -p mntpoint
> +}
> +
> +cleanup()
> +{
> +	tst_release_device
> +
> +	tst_rmdir
> +}
> +
> +mkfs_mount()
> +{
> +	mount ${TST_DEVICE} mntpoint
> +	local ret=$?
> +	if [ $ret -eq 32 ]; then
> +		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
> +	fi
> +
> +	if [ $ret -ne 0 ]; then
> +		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
> +	fi
> +}
> +
> +mkfs_umount()
> +{
> +	grep -q ${TST_DEVICE} /proc/self/mounts
                                 ^
                            Why not just /proc/mounts?

> +	if [ $? -eq 0 ]; then
> +		umount ${TST_DEVICE}
> +		if [ $? -ne 0 ];then
> +			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
> +		fi
> +	else
> +		tst_resm TINFO "${TST_DEVICE} is not mounted"
> +	fi
> +}
> +
> +usage()
> +{
> +	cat << EOF
> +	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
> +
> +	OPTIONS
> +		-f	Specify the type of filesystem to be built. If not
> +			specified, the default filesystem type (currently ext2)
> +			is used.
> +		-h	Display help text and exit.
> +
> +EOF
> +	tst_brkm TCONF "Display help text or unknown options"
                 ^
		I would rather make this TWARN, because when runtest entries
                became corrupted somehow the test would start reporting TCONF
                And that may end up being undetected for quite some time.

> +}
> +
> +mkfs_verify()
> +{
> +	if [ -z "$1" ]; then
> +		blkid | grep "$2" | grep -q "ext2"
> +	else
> +		blkid | grep "$2" | grep -q "$1"
> +	fi

You can actually pass the device to blkid as a parameter instead of grepping it
in the output.

> +}
> +
> +mkfs_check()
> +{
> +	mkfs_mount
> +	local blocknum=`df -aT | grep "$1" | awk '{print $3}'`

You can pass the mount point to the df instead of greping the device.

> +	mkfs_umount
> +
> +	if [ $blocknum -gt "$2" ]; then
> +		return 1
> +	fi

So the size we get as $2 is in kB and df -T reports 1k blocks shouldn't these
be equal, or are there any reserved block in play?

> +}
> +
> +mkfs_test()
> +{
> +	local mkfs_op=$1
> +	local fs_type=$2
> +	local fs_op=$3
> +	local device=$4
> +	local size=$5
> +
> +	if [ -n "$fs_type" ]; then
> +		mkfs_op="-t"

Why are you adding the flags by the tiniest bits?

Why don't you rather do:

	mkfs_op="-t $fs_type"

Which is far more logical.

> +	fi
> +
> +	if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
> +		fs_op+=" -f"

This is bashism. You have to do fs_op="$fs_op -f" instead.

> +	fi
> +
> +	local mkfs_cmd="mkfs $mkfs_op $fs_type $fs_op $device $size"
> +	mkfs_cmd=`echo "$mkfs_cmd" | sed 's/\s\+/ /g'`

This is just cosmetic, isn't it? I would just omit it.

> +	echo ${fs_op} | grep -q "\-c"
> +	if [ $? -eq 0 ] && [ "$fs_type" = "ntfs" ]; then
> +		tst_resm TCONF "'${mkfs_cmd}' not supported."
> +		return 32

		This is fairly misleading. It would be better to use return 0
                in case when you just need to exit the function. Because returning
                different values and ingoring them later is misleading.

> +	fi
> +
> +	if [ -n "$size" ]; then
> +		if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
> +			tst_resm TCONF "'${mkfs_cmd}' not supported."
> +			return 32

                        Here as well.

> +		fi
> +	fi
> +
> +	${mkfs_cmd} >temp 2>&1
> +	if [ $? -ne 0 ]; then
> +		grep -q -E "unknown option | invalid option" temp
> +		if [ $? -eq 0 ]; then
> +			tst_resm TCONF "'${mkfs_cmd}' not supported."
> +			return 32
> +		else
> +			tst_resm TFAIL "'${mkfs_cmd}' failed."
> +			cat temp
> +			return 1
> +		fi
> +	fi
> +
> +	if [ -n "$device" ]; then
> +		mkfs_verify "$fs_type" "$device"
> +		if [ $? -ne 0 ]; then
> +			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
> +			return 1
> +		fi

We should name the function better, mkfs_verify_type() comes to mind.

> +	fi
> +
> +	if [ -n "$size" ]; then
> +		mkfs_check "$device" "$size"
> +		if [ $? -ne 0 ]; then
> +			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
> +			return 1
> +		fi
> +	fi

Here as well, mkfs_verify_size would be better.

> +	tst_resm TPASS "'${mkfs_cmd}' passed."
> +}
> +
> +test1()
> +{
> +	tst_acquire_device
> +
> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE"
> +
> +	tst_release_device
> +}
> +
> +test2()
> +{
> +	tst_acquire_device
> +
> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE" "10000"

Can you just pass empty string when it should be empty instead of passing empty
global variable?

It's more confusing than it could be this way.

> +	tst_release_device
> +}
> +
> +test3()
> +{
> +	tst_acquire_device
> +
> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "-c" "$TST_DEVICE"
> +
> +	tst_release_device
> +}

Why don't we acquire device once in the setup and release it in the cleanup?

> +test4()
> +{
> +	mkfs_test "-V"
> +}
> +
> +test5()
> +{
> +	mkfs_test "-h"
> +}
> +
> +MKFS_OP=""
> +FS_TYPE=""
> +FS_OP=""
> +
> +if [ $# -ne 0 ]; then
> +	while getopts f:h OPTION; do
> +		case $OPTION in
> +		f)
> +			FS_TYPE=$OPTARG;;
> +		h)
> +			usage;;
> +		?)
> +			usage;;
> +		esac
> +	done
> +
> +	if [ -z "$FS_TYPE" ]; then
> +		usage
> +	fi
> +fi

Eh, why just not:

while getopts f:h OPTION; do
        case $OPTION in
        f)
                FS_TYPE=$OPTARG;;
        h)
                usage;;
        ?)
                usage;;
        esac
done

> +setup
> +for i in $(seq 1 ${TST_TOTAL})
> +do
> +	test$i
> +done
> +
> +tst_exit
> -- 
> 1.8.4.2
> 
> 
> -- 
> Mailing list info: http://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

Hi!

Thanks for your review.
I will modify the test case according to your suggestion.

Best Regards,
Guangwen Feng

On 2015/11/02 23:28, Cyril Hrubis wrote:
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2015 Fujitsu Ltd.
>> +# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
>> +# the GNU General Public License for more details.
>> +#
>> +# Test mkfs command with some basic options.
>> +#
>> +
>> +TCID=mkfs01
>> +TST_TOTAL=5
>> +. test.sh
>> +
>> +setup()
>> +{
>> +	tst_require_root
>> +
>> +	tst_check_cmds blkid df
>> +
>> +	if [ -n "$FS_TYPE" ]; then
>> +		tst_check_cmds mkfs.${FS_TYPE}
>> +	fi
>> +
>> +	tst_tmpdir
>> +
>> +	TST_CLEANUP="cleanup"
>> +
>> +	ROD_SILENT mkdir -p mntpoint
>> +}
>> +
>> +cleanup()
>> +{
>> +	tst_release_device
>> +
>> +	tst_rmdir
>> +}
>> +
>> +mkfs_mount()
>> +{
>> +	mount ${TST_DEVICE} mntpoint
>> +	local ret=$?
>> +	if [ $ret -eq 32 ]; then
>> +		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
>> +	fi
>> +
>> +	if [ $ret -ne 0 ]; then
>> +		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
>> +	fi
>> +}
>> +
>> +mkfs_umount()
>> +{
>> +	grep -q ${TST_DEVICE} /proc/self/mounts
>                                  ^
>                             Why not just /proc/mounts?
> 
>> +	if [ $? -eq 0 ]; then
>> +		umount ${TST_DEVICE}
>> +		if [ $? -ne 0 ];then
>> +			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
>> +		fi
>> +	else
>> +		tst_resm TINFO "${TST_DEVICE} is not mounted"
>> +	fi
>> +}
>> +
>> +usage()
>> +{
>> +	cat << EOF
>> +	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
>> +
>> +	OPTIONS
>> +		-f	Specify the type of filesystem to be built. If not
>> +			specified, the default filesystem type (currently ext2)
>> +			is used.
>> +		-h	Display help text and exit.
>> +
>> +EOF
>> +	tst_brkm TCONF "Display help text or unknown options"
>                  ^
> 		I would rather make this TWARN, because when runtest entries
>                 became corrupted somehow the test would start reporting TCONF
>                 And that may end up being undetected for quite some time.
> 
>> +}
>> +
>> +mkfs_verify()
>> +{
>> +	if [ -z "$1" ]; then
>> +		blkid | grep "$2" | grep -q "ext2"
>> +	else
>> +		blkid | grep "$2" | grep -q "$1"
>> +	fi
> 
> You can actually pass the device to blkid as a parameter instead of grepping it
> in the output.
> 
>> +}
>> +
>> +mkfs_check()
>> +{
>> +	mkfs_mount
>> +	local blocknum=`df -aT | grep "$1" | awk '{print $3}'`
> 
> You can pass the mount point to the df instead of greping the device.
> 
>> +	mkfs_umount
>> +
>> +	if [ $blocknum -gt "$2" ]; then
>> +		return 1
>> +	fi
> 
> So the size we get as $2 is in kB and df -T reports 1k blocks shouldn't these
> be equal, or are there any reserved block in play?
> 
>> +}
>> +
>> +mkfs_test()
>> +{
>> +	local mkfs_op=$1
>> +	local fs_type=$2
>> +	local fs_op=$3
>> +	local device=$4
>> +	local size=$5
>> +
>> +	if [ -n "$fs_type" ]; then
>> +		mkfs_op="-t"
> 
> Why are you adding the flags by the tiniest bits?
> 
> Why don't you rather do:
> 
> 	mkfs_op="-t $fs_type"
> 
> Which is far more logical.
> 
>> +	fi
>> +
>> +	if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
>> +		fs_op+=" -f"
> 
> This is bashism. You have to do fs_op="$fs_op -f" instead.
> 
>> +	fi
>> +
>> +	local mkfs_cmd="mkfs $mkfs_op $fs_type $fs_op $device $size"
>> +	mkfs_cmd=`echo "$mkfs_cmd" | sed 's/\s\+/ /g'`
> 
> This is just cosmetic, isn't it? I would just omit it.
> 
>> +	echo ${fs_op} | grep -q "\-c"
>> +	if [ $? -eq 0 ] && [ "$fs_type" = "ntfs" ]; then
>> +		tst_resm TCONF "'${mkfs_cmd}' not supported."
>> +		return 32
> 
> 		This is fairly misleading. It would be better to use return 0
>                 in case when you just need to exit the function. Because returning
>                 different values and ingoring them later is misleading.
> 
>> +	fi
>> +
>> +	if [ -n "$size" ]; then
>> +		if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
>> +			tst_resm TCONF "'${mkfs_cmd}' not supported."
>> +			return 32
> 
>                         Here as well.
> 
>> +		fi
>> +	fi
>> +
>> +	${mkfs_cmd} >temp 2>&1
>> +	if [ $? -ne 0 ]; then
>> +		grep -q -E "unknown option | invalid option" temp
>> +		if [ $? -eq 0 ]; then
>> +			tst_resm TCONF "'${mkfs_cmd}' not supported."
>> +			return 32
>> +		else
>> +			tst_resm TFAIL "'${mkfs_cmd}' failed."
>> +			cat temp
>> +			return 1
>> +		fi
>> +	fi
>> +
>> +	if [ -n "$device" ]; then
>> +		mkfs_verify "$fs_type" "$device"
>> +		if [ $? -ne 0 ]; then
>> +			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
>> +			return 1
>> +		fi
> 
> We should name the function better, mkfs_verify_type() comes to mind.
> 
>> +	fi
>> +
>> +	if [ -n "$size" ]; then
>> +		mkfs_check "$device" "$size"
>> +		if [ $? -ne 0 ]; then
>> +			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
>> +			return 1
>> +		fi
>> +	fi
> 
> Here as well, mkfs_verify_size would be better.
> 
>> +	tst_resm TPASS "'${mkfs_cmd}' passed."
>> +}
>> +
>> +test1()
>> +{
>> +	tst_acquire_device
>> +
>> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE"
>> +
>> +	tst_release_device
>> +}
>> +
>> +test2()
>> +{
>> +	tst_acquire_device
>> +
>> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE" "10000"
> 
> Can you just pass empty string when it should be empty instead of passing empty
> global variable?
> 
> It's more confusing than it could be this way.
> 
>> +	tst_release_device
>> +}
>> +
>> +test3()
>> +{
>> +	tst_acquire_device
>> +
>> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "-c" "$TST_DEVICE"
>> +
>> +	tst_release_device
>> +}
> 
> Why don't we acquire device once in the setup and release it in the cleanup?
> 
>> +test4()
>> +{
>> +	mkfs_test "-V"
>> +}
>> +
>> +test5()
>> +{
>> +	mkfs_test "-h"
>> +}
>> +
>> +MKFS_OP=""
>> +FS_TYPE=""
>> +FS_OP=""
>> +
>> +if [ $# -ne 0 ]; then
>> +	while getopts f:h OPTION; do
>> +		case $OPTION in
>> +		f)
>> +			FS_TYPE=$OPTARG;;
>> +		h)
>> +			usage;;
>> +		?)
>> +			usage;;
>> +		esac
>> +	done
>> +
>> +	if [ -z "$FS_TYPE" ]; then
>> +		usage
>> +	fi
>> +fi
> 
> Eh, why just not:
> 
> while getopts f:h OPTION; do
>         case $OPTION in
>         f)
>                 FS_TYPE=$OPTARG;;
>         h)
>                 usage;;
>         ?)
>                 usage;;
>         esac
> done
> 
>> +setup
>> +for i in $(seq 1 ${TST_TOTAL})
>> +do
>> +	test$i
>> +done
>> +
>> +tst_exit
>> -- 
>> 1.8.4.2
>>
>>
>> -- 
>> Mailing list info: http://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

On 2015/11/02 23:28, Cyril Hrubis wrote:
>> +#!/bin/sh
>> +#
>> +# Copyright (c) 2015 Fujitsu Ltd.
>> +# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
>> +# the GNU General Public License for more details.
>> +#
>> +# Test mkfs command with some basic options.
>> +#
>> +
>> +TCID=mkfs01
>> +TST_TOTAL=5
>> +. test.sh
>> +
>> +setup()
>> +{
>> +	tst_require_root
>> +
>> +	tst_check_cmds blkid df
>> +
>> +	if [ -n "$FS_TYPE" ]; then
>> +		tst_check_cmds mkfs.${FS_TYPE}
>> +	fi
>> +
>> +	tst_tmpdir
>> +
>> +	TST_CLEANUP="cleanup"
>> +
>> +	ROD_SILENT mkdir -p mntpoint
>> +}
>> +
>> +cleanup()
>> +{
>> +	tst_release_device
>> +
>> +	tst_rmdir
>> +}
>> +
>> +mkfs_mount()
>> +{
>> +	mount ${TST_DEVICE} mntpoint
>> +	local ret=$?
>> +	if [ $ret -eq 32 ]; then
>> +		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
>> +	fi
>> +
>> +	if [ $ret -ne 0 ]; then
>> +		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
>> +	fi
>> +}
>> +
>> +mkfs_umount()
>> +{
>> +	grep -q ${TST_DEVICE} /proc/self/mounts
>                                  ^
>                             Why not just /proc/mounts?
> 
>> +	if [ $? -eq 0 ]; then
>> +		umount ${TST_DEVICE}
>> +		if [ $? -ne 0 ];then
>> +			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
>> +		fi
>> +	else
>> +		tst_resm TINFO "${TST_DEVICE} is not mounted"
>> +	fi
>> +}
>> +
>> +usage()
>> +{
>> +	cat << EOF
>> +	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
>> +
>> +	OPTIONS
>> +		-f	Specify the type of filesystem to be built. If not
>> +			specified, the default filesystem type (currently ext2)
>> +			is used.
>> +		-h	Display help text and exit.
>> +
>> +EOF
>> +	tst_brkm TCONF "Display help text or unknown options"
>                  ^
> 		I would rather make this TWARN, because when runtest entries
>                 became corrupted somehow the test would start reporting TCONF
>                 And that may end up being undetected for quite some time.
> 
>> +}
>> +
>> +mkfs_verify()
>> +{
>> +	if [ -z "$1" ]; then
>> +		blkid | grep "$2" | grep -q "ext2"
>> +	else
>> +		blkid | grep "$2" | grep -q "$1"
>> +	fi
> 
> You can actually pass the device to blkid as a parameter instead of grepping it
> in the output.
> 
>> +}
>> +
>> +mkfs_check()
>> +{
>> +	mkfs_mount
>> +	local blocknum=`df -aT | grep "$1" | awk '{print $3}'`
> 
> You can pass the mount point to the df instead of greping the device.
> 
>> +	mkfs_umount
>> +
>> +	if [ $blocknum -gt "$2" ]; then
>> +		return 1
>> +	fi
> 
> So the size we get as $2 is in kB and df -T reports 1k blocks shouldn't these
> be equal, or are there any reserved block in play?
> 

Indeed, "$blocknum" here is equal to struct statfs'f_blocks, which denotes total
data blocks in filesystem(df uses statfs(2) to get this info) and does not contain
some metadata in filesytem, e.g. ext3's journal space size. 

Best Regards,
Guangwen Feng

>> +}
>> +
>> +mkfs_test()
>> +{
>> +	local mkfs_op=$1
>> +	local fs_type=$2
>> +	local fs_op=$3
>> +	local device=$4
>> +	local size=$5
>> +
>> +	if [ -n "$fs_type" ]; then
>> +		mkfs_op="-t"
> 
> Why are you adding the flags by the tiniest bits?
> 
> Why don't you rather do:
> 
> 	mkfs_op="-t $fs_type"
> 
> Which is far more logical.
> 
>> +	fi
>> +
>> +	if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
>> +		fs_op+=" -f"
> 
> This is bashism. You have to do fs_op="$fs_op -f" instead.
> 
>> +	fi
>> +
>> +	local mkfs_cmd="mkfs $mkfs_op $fs_type $fs_op $device $size"
>> +	mkfs_cmd=`echo "$mkfs_cmd" | sed 's/\s\+/ /g'`
> 
> This is just cosmetic, isn't it? I would just omit it.
> 
>> +	echo ${fs_op} | grep -q "\-c"
>> +	if [ $? -eq 0 ] && [ "$fs_type" = "ntfs" ]; then
>> +		tst_resm TCONF "'${mkfs_cmd}' not supported."
>> +		return 32
> 
> 		This is fairly misleading. It would be better to use return 0
>                 in case when you just need to exit the function. Because returning
>                 different values and ingoring them later is misleading.
> 
>> +	fi
>> +
>> +	if [ -n "$size" ]; then
>> +		if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
>> +			tst_resm TCONF "'${mkfs_cmd}' not supported."
>> +			return 32
> 
>                         Here as well.
> 
>> +		fi
>> +	fi
>> +
>> +	${mkfs_cmd} >temp 2>&1
>> +	if [ $? -ne 0 ]; then
>> +		grep -q -E "unknown option | invalid option" temp
>> +		if [ $? -eq 0 ]; then
>> +			tst_resm TCONF "'${mkfs_cmd}' not supported."
>> +			return 32
>> +		else
>> +			tst_resm TFAIL "'${mkfs_cmd}' failed."
>> +			cat temp
>> +			return 1
>> +		fi
>> +	fi
>> +
>> +	if [ -n "$device" ]; then
>> +		mkfs_verify "$fs_type" "$device"
>> +		if [ $? -ne 0 ]; then
>> +			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
>> +			return 1
>> +		fi
> 
> We should name the function better, mkfs_verify_type() comes to mind.
> 
>> +	fi
>> +
>> +	if [ -n "$size" ]; then
>> +		mkfs_check "$device" "$size"
>> +		if [ $? -ne 0 ]; then
>> +			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
>> +			return 1
>> +		fi
>> +	fi
> 
> Here as well, mkfs_verify_size would be better.
> 
>> +	tst_resm TPASS "'${mkfs_cmd}' passed."
>> +}
>> +
>> +test1()
>> +{
>> +	tst_acquire_device
>> +
>> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE"
>> +
>> +	tst_release_device
>> +}
>> +
>> +test2()
>> +{
>> +	tst_acquire_device
>> +
>> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "$FS_OP" "$TST_DEVICE" "10000"
> 
> Can you just pass empty string when it should be empty instead of passing empty
> global variable?
> 
> It's more confusing than it could be this way.
> 
>> +	tst_release_device
>> +}
>> +
>> +test3()
>> +{
>> +	tst_acquire_device
>> +
>> +	mkfs_test "$MKFS_OP" "$FS_TYPE" "-c" "$TST_DEVICE"
>> +
>> +	tst_release_device
>> +}
> 
> Why don't we acquire device once in the setup and release it in the cleanup?
> 
>> +test4()
>> +{
>> +	mkfs_test "-V"
>> +}
>> +
>> +test5()
>> +{
>> +	mkfs_test "-h"
>> +}
>> +
>> +MKFS_OP=""
>> +FS_TYPE=""
>> +FS_OP=""
>> +
>> +if [ $# -ne 0 ]; then
>> +	while getopts f:h OPTION; do
>> +		case $OPTION in
>> +		f)
>> +			FS_TYPE=$OPTARG;;
>> +		h)
>> +			usage;;
>> +		?)
>> +			usage;;
>> +		esac
>> +	done
>> +
>> +	if [ -z "$FS_TYPE" ]; then
>> +		usage
>> +	fi
>> +fi
> 
> Eh, why just not:
> 
> while getopts f:h OPTION; do
>         case $OPTION in
>         f)
>                 FS_TYPE=$OPTARG;;
>         h)
>                 usage;;
>         ?)
>                 usage;;
>         esac
> done
> 
>> +setup
>> +for i in $(seq 1 ${TST_TOTAL})
>> +do
>> +	test$i
>> +done
>> +
>> +tst_exit
>> -- 
>> 1.8.4.2
>>
>>
>> -- 
>> Mailing list info: http://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[Cyril Hrubis]

Hi!
> >> +	mkfs_umount
> >> +
> >> +	if [ $blocknum -gt "$2" ]; then
> >> +		return 1
> >> +	fi
> > 
> > So the size we get as $2 is in kB and df -T reports 1k blocks shouldn't these
> > be equal, or are there any reserved block in play?
> > 
> 
> Indeed, "$blocknum" here is equal to struct statfs'f_blocks, which denotes total
> data blocks in filesystem(df uses statfs(2) to get this info) and does not contain
> some metadata in filesytem, e.g. ext3's journal space size. 

Ok, then we should check for lower bound as well. Something as blocknum
is greater than 90% of size or similar.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

Test mkfs(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                  |  10 ++
 testcases/commands/mkfs/Makefile  |  24 +++++
 testcases/commands/mkfs/mkfs01.sh | 217 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 251 insertions(+)
 create mode 100644 testcases/commands/mkfs/Makefile
 create mode 100755 testcases/commands/mkfs/mkfs01.sh

diff --git a/runtest/commands b/runtest/commands
index e5260ba..6c0485b 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -28,3 +28,13 @@ df01_xfs df01.sh -f xfs
 df01_vfat df01.sh -f vfat
 df01_exfat df01.sh -f exfat
 df01_ntfs df01.sh -f ntfs
+mkfs01 mkfs01.sh
+mkfs01_ext2 mkfs01.sh -f ext2
+mkfs01_ext3 mkfs01.sh -f ext3
+mkfs01_ext4 mkfs01.sh -f ext4
+mkfs01_xfs mkfs01.sh -f xfs
+mkfs01_btrfs mkfs01.sh -f btrfs
+mkfs01_minix mkfs01.sh -f minix
+mkfs01_msdos mkfs01.sh -f msdos
+mkfs01_vfat mkfs01.sh -f vfat
+mkfs01_ntfs mkfs01.sh -f ntfs
diff --git a/testcases/commands/mkfs/Makefile b/testcases/commands/mkfs/Makefile
new file mode 100644
index 0000000..6099017
--- /dev/null
+++ b/testcases/commands/mkfs/Makefile
@@ -0,0 +1,24 @@
+#
+#    commands/mkfs testcases Makefile.
+#
+#    Copyright (c) 2015 Fujitsu Ltd.
+#    Author:Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+#
+#    This program is free software; you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License as published by
+#    the Free Software Foundation; either version 2 of the License, or
+#    (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+
+top_srcdir		?= ../../..
+
+include $(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_TARGETS		:= mkfs01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkfs/mkfs01.sh b/testcases/commands/mkfs/mkfs01.sh
new file mode 100755
index 0000000..c1337a6
--- /dev/null
+++ b/testcases/commands/mkfs/mkfs01.sh
@@ -0,0 +1,217 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Fujitsu Ltd.
+# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+# the GNU General Public License for more details.
+#
+# Test mkfs command with some basic options.
+#
+
+TCID=mkfs01
+TST_TOTAL=5
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds blkid df
+
+	if [ -n "$FS_TYPE" ]; then
+		tst_check_cmds mkfs.${FS_TYPE}
+	fi
+
+	tst_tmpdir
+
+	tst_acquire_device
+
+	TST_CLEANUP="cleanup"
+
+	ROD_SILENT mkdir -p mntpoint
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkfs_mount()
+{
+	mount ${TST_DEVICE} mntpoint
+	local ret=$?
+	if [ $ret -eq 32 ]; then
+		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
+	fi
+
+	if [ $ret -ne 0 ]; then
+		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
+	fi
+}
+
+mkfs_umount()
+{
+	grep -q ${TST_DEVICE} /proc/mounts
+	if [ $? -eq 0 ]; then
+		umount ${TST_DEVICE}
+		if [ $? -ne 0 ];then
+			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
+		fi
+	else
+		tst_resm TINFO "${TST_DEVICE} is not mounted"
+	fi
+}
+
+usage()
+{
+	cat << EOF
+	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
+
+	OPTIONS
+		-f	Specify the type of filesystem to be built. If not
+			specified, the default filesystem type (currently ext2)
+			is used.
+		-h	Display help text and exit.
+
+EOF
+	tst_brkm TWARN "Display help text or unknown options"
+}
+
+mkfs_verify_type()
+{
+	if [ -z "$1" ]; then
+		blkid $2 | grep -q "ext2"
+	else
+		blkid $2 | grep -q "$1"
+	fi
+}
+
+mkfs_verify_size()
+{
+	mkfs_mount
+	local blocknum=`df -aT mntpoint | tail -n1 | awk '{print $3}'`
+	mkfs_umount
+
+	if [ $blocknum -gt "$1" ]; then
+		return 1
+	fi
+}
+
+mkfs_test()
+{
+	local mkfs_op=$1
+	local fs_type=$2
+	local fs_op=$3
+	local device=$4
+	local size=$5
+
+	if [ -n "$fs_type" ]; then
+		mkfs_op="-t $fs_type"
+	fi
+
+	if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
+		fs_op="$fs_op -f"
+	fi
+
+	local mkfs_cmd="mkfs $mkfs_op $fs_op $device $size"
+
+	echo ${fs_op} | grep -q "\-c"
+	if [ $? -eq 0 ] && [ "$fs_type" = "ntfs" ]; then
+		tst_resm TCONF "'${mkfs_cmd}' not supported."
+		return
+	fi
+
+	if [ -n "$size" ]; then
+		if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
+			tst_resm TCONF "'${mkfs_cmd}' not supported."
+			return
+		fi
+	fi
+
+	${mkfs_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option | invalid option" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkfs_cmd}' not supported."
+			return
+		else
+			tst_resm TFAIL "'${mkfs_cmd}' failed."
+			cat temp
+			return
+		fi
+	fi
+
+	if [ -n "$device" ]; then
+		mkfs_verify_type "$fs_type" "$device"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	if [ -n "$size" ]; then
+		mkfs_verify_size "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkfs_cmd}' passed."
+}
+
+test1()
+{
+	mkfs_test "" "$FS_TYPE" "" "$TST_DEVICE"
+}
+
+test2()
+{
+	mkfs_test "" "$FS_TYPE" "" "$TST_DEVICE" "10000"
+}
+
+test3()
+{
+	mkfs_test "" "$FS_TYPE" "-c" "$TST_DEVICE"
+}
+
+test4()
+{
+	mkfs_test "-V"
+}
+
+test5()
+{
+	mkfs_test "-h"
+}
+
+FS_TYPE=""
+
+while getopts f:h OPTION; do
+	case $OPTION in
+	f)
+		FS_TYPE=$OPTARG;;
+	h)
+		usage;;
+	?)
+		usage;;
+	esac
+done
+
+setup
+for i in $(seq 1 ${TST_TOTAL})
+do
+	test$i
+done
+
+tst_exit
-- 
1.8.4.2

[LTP] [PATCH] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

On 2015/11/03 19:39, Cyril Hrubis wrote:
> Hi!
>>>> +	mkfs_umount
>>>> +
>>>> +	if [ $blocknum -gt "$2" ]; then
>>>> +		return 1
>>>> +	fi
>>>
>>> So the size we get as $2 is in kB and df -T reports 1k blocks shouldn't these
>>> be equal, or are there any reserved block in play?
>>>
>>
>> Indeed, "$blocknum" here is equal to struct statfs'f_blocks, which denotes total
>> data blocks in filesystem(df uses statfs(2) to get this info) and does not contain
>> some metadata in filesytem, e.g. ext3's journal space size. 
> 
> Ok, then we should check for lower bound as well. Something as blocknum
> is greater than 90% of size or similar.
> 

OK, I see, thanks!
Please ignore the v2 I just sent, I will email a v3 with lower bound check.

Best Regards,
Guangwen Feng

[LTP] [PATCH v2] commands/mkfs: Added new testcase to test mkfs(8) command.

[=?unknown-8bit?b?5p2O56OK?=]

> Test mkfs(8) command with some basic options.
>
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>   runtest/commands                  |  10 ++
>   testcases/commands/mkfs/Makefile  |  24 +++++
>   testcases/commands/mkfs/mkfs01.sh | 217 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 251 insertions(+)
>   create mode 100644 testcases/commands/mkfs/Makefile
>   create mode 100755 testcases/commands/mkfs/mkfs01.sh
>
> diff --git a/runtest/commands b/runtest/commands
> index e5260ba..6c0485b 100644
> --- a/runtest/commands
> +++ b/runtest/commands
> @@ -28,3 +28,13 @@ df01_xfs df01.sh -f xfs
>   df01_vfat df01.sh -f vfat
>   df01_exfat df01.sh -f exfat
>   df01_ntfs df01.sh -f ntfs
> +mkfs01 mkfs01.sh
> +mkfs01_ext2 mkfs01.sh -f ext2
> +mkfs01_ext3 mkfs01.sh -f ext3
> +mkfs01_ext4 mkfs01.sh -f ext4
> +mkfs01_xfs mkfs01.sh -f xfs
> +mkfs01_btrfs mkfs01.sh -f btrfs
> +mkfs01_minix mkfs01.sh -f minix
> +mkfs01_msdos mkfs01.sh -f msdos
> +mkfs01_vfat mkfs01.sh -f vfat
> +mkfs01_ntfs mkfs01.sh -f ntfs
> diff --git a/testcases/commands/mkfs/Makefile b/testcases/commands/mkfs/Makefile
> new file mode 100644
> index 0000000..6099017
> --- /dev/null
> +++ b/testcases/commands/mkfs/Makefile
> @@ -0,0 +1,24 @@
> +#
> +#    commands/mkfs testcases Makefile.
> +#
> +#    Copyright (c) 2015 Fujitsu Ltd.
> +#    Author:Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> +#
> +#    This program is free software; you can redistribute it and/or modify
> +#    it under the terms of the GNU General Public License as published by
> +#    the Free Software Foundation; either version 2 of the License, or
> +#    (at your option) any later version.
> +#
> +#    This program is distributed in the hope that it will be useful,
> +#    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +#    GNU General Public License for more details.
> +#
> +
> +top_srcdir		?= ../../..
> +
> +include $(top_srcdir)/include/mk/env_pre.mk
> +
> +INSTALL_TARGETS		:= mkfs01.sh
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/commands/mkfs/mkfs01.sh b/testcases/commands/mkfs/mkfs01.sh
> new file mode 100755
> index 0000000..c1337a6
> --- /dev/null
> +++ b/testcases/commands/mkfs/mkfs01.sh
> @@ -0,0 +1,217 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2015 Fujitsu Ltd.
> +# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> +# the GNU General Public License for more details.
> +#
> +# Test mkfs command with some basic options.
> +#
> +
> +TCID=mkfs01
> +TST_TOTAL=5
> +. test.sh
> +
> +setup()
> +{
> +	tst_require_root
> +
> +	tst_check_cmds blkid df
> +
> +	if [ -n "$FS_TYPE" ]; then
> +		tst_check_cmds mkfs.${FS_TYPE}
> +	fi
> +
> +	tst_tmpdir
> +
> +	tst_acquire_device
> +
> +	TST_CLEANUP="cleanup"
> +
> +	ROD_SILENT mkdir -p mntpoint
> +}
> +
> +cleanup()
> +{
> +	tst_release_device
> +
> +	tst_rmdir
> +}
> +
> +mkfs_mount()
> +{
> +	mount ${TST_DEVICE} mntpoint
> +	local ret=$?
> +	if [ $ret -eq 32 ]; then
> +		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
> +	fi
> +
> +	if [ $ret -ne 0 ]; then
> +		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
> +	fi
> +}
> +
> +mkfs_umount()
> +{
> +	grep -q ${TST_DEVICE} /proc/mounts
> +	if [ $? -eq 0 ]; then
> +		umount ${TST_DEVICE}
> +		if [ $? -ne 0 ];then
> +			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
> +		fi
> +	else
> +		tst_resm TINFO "${TST_DEVICE} is not mounted"
> +	fi
> +}
> +
> +usage()
> +{
> +	cat << EOF
> +	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
> +
> +	OPTIONS
> +		-f	Specify the type of filesystem to be built. If not
> +			specified, the default filesystem type (currently ext2)
> +			is used.
> +		-h	Display help text and exit.
> +
> +EOF
> +	tst_brkm TWARN "Display help text or unknown options"
> +}
> +
> +mkfs_verify_type()
> +{
> +	if [ -z "$1" ]; then
> +		blkid $2 | grep -q "ext2"
> +	else
> +		blkid $2 | grep -q "$1"
> +	fi
> +}

Hi, how about:
mkfs_verify_type()
{
	if [ -z "$1" ]; then
		blkid $2 -t TYPE="ext2"
	else
		blkid $2 -t TYPE="$1"
	fi
}

-- 
Rock Lee

[LTP] [PATCH v2] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

Hi!

>> +mkfs_verify_type()
>> +{
>> +    if [ -z "$1" ]; then
>> +        blkid $2 | grep -q "ext2"
>> +    else
>> +        blkid $2 | grep -q "$1"
>> +    fi
>> +}
> 
> Hi, how about:
> mkfs_verify_type()
> {
>     if [ -z "$1" ]; then
>         blkid $2 -t TYPE="ext2"
>     else
>         blkid $2 -t TYPE="$1"
>     fi
> }
> 

It looks better since it's more logical? thanks!

[LTP] [PATCH v3] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

Test mkfs(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                  |  10 ++
 testcases/commands/mkfs/Makefile  |  24 ++++
 testcases/commands/mkfs/mkfs01.sh | 234 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 268 insertions(+)
 create mode 100644 testcases/commands/mkfs/Makefile
 create mode 100755 testcases/commands/mkfs/mkfs01.sh

diff --git a/runtest/commands b/runtest/commands
index e5260ba..6c0485b 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -28,3 +28,13 @@ df01_xfs df01.sh -f xfs
 df01_vfat df01.sh -f vfat
 df01_exfat df01.sh -f exfat
 df01_ntfs df01.sh -f ntfs
+mkfs01 mkfs01.sh
+mkfs01_ext2 mkfs01.sh -f ext2
+mkfs01_ext3 mkfs01.sh -f ext3
+mkfs01_ext4 mkfs01.sh -f ext4
+mkfs01_xfs mkfs01.sh -f xfs
+mkfs01_btrfs mkfs01.sh -f btrfs
+mkfs01_minix mkfs01.sh -f minix
+mkfs01_msdos mkfs01.sh -f msdos
+mkfs01_vfat mkfs01.sh -f vfat
+mkfs01_ntfs mkfs01.sh -f ntfs
diff --git a/testcases/commands/mkfs/Makefile b/testcases/commands/mkfs/Makefile
new file mode 100644
index 0000000..6099017
--- /dev/null
+++ b/testcases/commands/mkfs/Makefile
@@ -0,0 +1,24 @@
+#
+#    commands/mkfs testcases Makefile.
+#
+#    Copyright (c) 2015 Fujitsu Ltd.
+#    Author:Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+#
+#    This program is free software; you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License as published by
+#    the Free Software Foundation; either version 2 of the License, or
+#    (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+
+top_srcdir		?= ../../..
+
+include $(top_srcdir)/include/mk/env_pre.mk
+
+INSTALL_TARGETS		:= mkfs01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkfs/mkfs01.sh b/testcases/commands/mkfs/mkfs01.sh
new file mode 100755
index 0000000..2cf9d6a
--- /dev/null
+++ b/testcases/commands/mkfs/mkfs01.sh
@@ -0,0 +1,234 @@
+#!/bin/sh
+#
+# Copyright (c) 2015 Fujitsu Ltd.
+# Author: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
+# the GNU General Public License for more details.
+#
+# Test mkfs command with some basic options.
+#
+
+TCID=mkfs01
+TST_TOTAL=5
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds blkid df
+
+	if [ -n "$FS_TYPE" ]; then
+		tst_check_cmds mkfs.${FS_TYPE}
+	fi
+
+	tst_tmpdir
+
+	tst_acquire_device
+
+	TST_CLEANUP="cleanup"
+
+	ROD_SILENT mkdir -p mntpoint
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkfs_mount()
+{
+	mount ${TST_DEVICE} mntpoint
+	local ret=$?
+	if [ $ret -eq 32 ]; then
+		tst_brkm TCONF "Cannot mount ${FS_TYPE}, missing driver?"
+	fi
+
+	if [ $ret -ne 0 ]; then
+		tst_brkm TBROK "Failed to mount device: mount exit = $ret"
+	fi
+}
+
+mkfs_umount()
+{
+	grep -q ${TST_DEVICE} /proc/mounts
+	if [ $? -eq 0 ]; then
+		umount ${TST_DEVICE}
+		if [ $? -ne 0 ];then
+			tst_resm TWARN "'umount ${TST_DEVICE}' failed"
+		fi
+	else
+		tst_resm TINFO "${TST_DEVICE} is not mounted"
+	fi
+}
+
+usage()
+{
+	cat << EOF
+	usage: $0 [-f <ext2|ext3|ext4|vfat|...>]
+
+	OPTIONS
+		-f	Specify the type of filesystem to be built. If not
+			specified, the default filesystem type (currently ext2)
+			is used.
+		-h	Display help text and exit.
+
+EOF
+	tst_brkm TWARN "Display help text or unknown options"
+}
+
+mkfs_verify_type()
+{
+	if [ -z "$1" ]; then
+		blkid $2 -t TYPE="ext2" >/dev/null
+	else
+		if [ "$1" = "msdos" ]; then
+			blkid $2 -t SEC_TYPE="$1" >/dev/null
+		else
+			blkid $2 -t TYPE="$1" >/dev/null
+		fi
+	fi
+}
+
+mkfs_verify_size()
+{
+	mkfs_mount
+	local blocknum=`df -B 1k mntpoint | tail -n1 | awk '{print $2}'`
+	mkfs_umount
+
+	if [ $blocknum -gt "$2" ]; then
+		return 1
+	fi
+
+	# Size argument in mkfs.ntfs denotes number-of-sectors which is 512bytes,
+	# 1k-block size should be devided by this argument for ntfs verification.
+	if [ "$1" = "ntfs" ]; then
+		local rate=1024/512
+		if [ $blocknum -lt "$(($2/rate*9/10))" ]; then
+			return 1
+		fi
+	else
+		if [ $blocknum -lt "$(($2*9/10))" ]; then
+			return 1
+		fi
+	fi
+}
+
+mkfs_test()
+{
+	local mkfs_op=$1
+	local fs_type=$2
+	local fs_op=$3
+	local device=$4
+	local size=$5
+
+	if [ -n "$fs_type" ]; then
+		mkfs_op="-t $fs_type"
+	fi
+
+	if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
+		fs_op="$fs_op -f"
+	fi
+
+	local mkfs_cmd="mkfs $mkfs_op $fs_op $device $size"
+
+	echo ${fs_op} | grep -q "\-c"
+	if [ $? -eq 0 ] && [ "$fs_type" = "ntfs" ]; then
+		tst_resm TCONF "'${mkfs_cmd}' not supported."
+		return
+	fi
+
+	if [ -n "$size" ]; then
+		if [ "$fs_type" = "xfs" ] || [ "$fs_type" = "btrfs" ]; then
+			tst_resm TCONF "'${mkfs_cmd}' not supported."
+			return
+		fi
+	fi
+
+	${mkfs_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option | invalid option" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkfs_cmd}' not supported."
+			return
+		else
+			tst_resm TFAIL "'${mkfs_cmd}' failed."
+			cat temp
+			return
+		fi
+	fi
+
+	if [ -n "$device" ]; then
+		mkfs_verify_type "$fs_type" "$device"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	if [ -n "$size" ]; then
+		mkfs_verify_size "$fs_type" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkfs_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkfs_cmd}' passed."
+}
+
+test1()
+{
+	mkfs_test "" "$FS_TYPE" "" "$TST_DEVICE"
+}
+
+test2()
+{
+	mkfs_test "" "$FS_TYPE" "" "$TST_DEVICE" "16000"
+}
+
+test3()
+{
+	mkfs_test "" "$FS_TYPE" "-c" "$TST_DEVICE"
+}
+
+test4()
+{
+	mkfs_test "-V"
+}
+
+test5()
+{
+	mkfs_test "-h"
+}
+
+FS_TYPE=""
+
+while getopts f:h OPTION; do
+	case $OPTION in
+	f)
+		FS_TYPE=$OPTARG;;
+	h)
+		usage;;
+	?)
+		usage;;
+	esac
+done
+
+setup
+for i in $(seq 1 ${TST_TOTAL})
+do
+	test$i
+done
+
+tst_exit
-- 
1.8.4.2

[LTP] [PATCH v3] commands/mkfs: Added new testcase to test mkfs(8) command.

[Cyril Hrubis]

Hi!
> +mkfs_verify_size()
> +{
> +	mkfs_mount
> +	local blocknum=`df -B 1k mntpoint | tail -n1 | awk '{print $2}'`
> +	mkfs_umount
> +
> +	if [ $blocknum -gt "$2" ]; then
> +		return 1
> +	fi
> +
> +	# Size argument in mkfs.ntfs denotes number-of-sectors which is 512bytes,
> +	# 1k-block size should be devided by this argument for ntfs verification.
> +	if [ "$1" = "ntfs" ]; then
> +		local rate=1024/512
> +		if [ $blocknum -lt "$(($2/rate*9/10))" ]; then
> +			return 1
> +		fi
> +	else
> +		if [ $blocknum -lt "$(($2*9/10))" ]; then
> +			return 1
> +		fi
> +	fi

I've added explicit return 0 here, since otherwise the retul value would
be set by the last executed command (which would be the either umount
or mount above...).


And pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] commands/mkfs: Added new testcase to test mkfs(8) command.

[Guangwen Feng]

On 2015/11/04 21:22, Cyril Hrubis wrote:
> Hi!
>> +mkfs_verify_size()
>> +{
>> +	mkfs_mount
>> +	local blocknum=`df -B 1k mntpoint | tail -n1 | awk '{print $2}'`
>> +	mkfs_umount
>> +
>> +	if [ $blocknum -gt "$2" ]; then
>> +		return 1
>> +	fi
>> +
>> +	# Size argument in mkfs.ntfs denotes number-of-sectors which is 512bytes,
>> +	# 1k-block size should be devided by this argument for ntfs verification.
>> +	if [ "$1" = "ntfs" ]; then
>> +		local rate=1024/512
>> +		if [ $blocknum -lt "$(($2/rate*9/10))" ]; then
>> +			return 1
>> +		fi
>> +	else
>> +		if [ $blocknum -lt "$(($2*9/10))" ]; then
>> +			return 1
>> +		fi
>> +	fi
> 
> I've added explicit return 0 here, since otherwise the retul value would
> be set by the last executed command (which would be the either umount
> or mount above...).
> 

Indeed, got it.
Many thanks!

Best Regards,
Guangwen Feng

> 
> And pushed, thanks.
>