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

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

[Guangwen Feng]

Test mkswap(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                      |   1 +
 testcases/commands/mkswap/Makefile    |  24 +++++
 testcases/commands/mkswap/mkswap01.sh | 196 ++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)
 create mode 100644 testcases/commands/mkswap/Makefile
 create mode 100755 testcases/commands/mkswap/mkswap01.sh

diff --git a/runtest/commands b/runtest/commands
index 6c0485b..ab600dc 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
 mkfs01_msdos mkfs01.sh -f msdos
 mkfs01_vfat mkfs01.sh -f vfat
 mkfs01_ntfs mkfs01.sh -f ntfs
+mkswap01 mkswap01.sh
diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
new file mode 100644
index 0000000..39651c0
--- /dev/null
+++ b/testcases/commands/mkswap/Makefile
@@ -0,0 +1,24 @@
+#
+#    commands/mkswap 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		:= mkswap01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkswap/mkswap01.sh b/testcases/commands/mkswap/mkswap01.sh
new file mode 100755
index 0000000..e0c1a08
--- /dev/null
+++ b/testcases/commands/mkswap/mkswap01.sh
@@ -0,0 +1,196 @@
+#!/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 mkswap command with some basic options.
+#
+
+TCID=mkswap01
+TST_TOTAL=10
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds losetup free mkswap
+
+	tst_tmpdir
+
+	TST_CLEANUP=cleanup
+
+	tst_acquire_device
+
+	local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
+		cut -d '(' -f2 | cut -d ')' -f1`
+	DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
+
+	PAGE_SIZE=`getconf PAGE_SIZE`
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkswap_verify()
+{
+	local ret=0
+
+	local before=`free | grep "Swap" | awk '{print $2}'`
+
+	if [ -z "$4" ]; then
+		local swapsize=$DEVICE_SIZE
+	else
+		local swapsize=$4
+	fi
+
+	if [ "$1" = "-p" ]; then
+		local pagesize=$2
+	else
+		local pagesize=$PAGE_SIZE
+	fi
+
+	if [ "$1" = "-L" ]; then
+		local swap_op="-L"
+		local swapfile=$2
+	elif [ "$1" = "-U" ]; then
+		local swap_op="-U"
+		local swapfile=$2
+	else
+		local swap_op=""
+		local swapfile=$3
+	fi
+
+	swapon $swap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "can not do swapon on $swapfile."
+		if [ $pagesize -ne $PAGE_SIZE ]; then
+			return $ret
+		fi
+
+		if [ $swapsize -gt $DEVICE_SIZE ]; then
+			return $ret
+		fi
+	fi
+
+	local after=`free | grep "Swap" | awk '{print $2}'`
+
+	local est=16
+	if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
+		[ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
+		ret=1
+	fi
+
+	swapoff $swap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "can not do swapoff on $swapfile."
+	fi
+
+	return $ret
+}
+
+mkswap_test()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local device=$3
+	local size=$4
+
+	local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
+
+	${mkswap_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option|invalid option|Usage" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkswap_cmd}' not supported."
+			return
+		else
+			tst_resm TFAIL "'${mkswap_cmd}' failed."
+			cat temp
+			return
+		fi
+	fi
+
+	if [ -n "$device" ]; then
+		mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkswap_cmd}' passed."
+}
+
+test1()
+{
+	mkswap_test "" "" "$TST_DEVICE"
+}
+
+test2()
+{
+	mkswap_test "" "" "$TST_DEVICE" "10000"
+}
+
+test3()
+{
+	mkswap_test "-f" "" "$TST_DEVICE" "30000"
+}
+
+test4()
+{
+	mkswap_test "-c" "" "$TST_DEVICE"
+}
+
+test5()
+{
+	mkswap_test "-p" "2048" "$TST_DEVICE"
+}
+
+test6()
+{
+	mkswap_test "-L" "testswap" "$TST_DEVICE"
+}
+
+test7()
+{
+	mkswap_test "-v1" "" "$TST_DEVICE"
+}
+
+test8()
+{
+	mkswap_test "-U" "ffffffff-ffff-ffff-ffff-ffffffffffff" "$TST_DEVICE"
+}
+
+test9()
+{
+	mkswap_test "-V"
+}
+
+test10()
+{
+	mkswap_test "-h"
+}
+
+setup
+
+for i in $(seq 1 ${TST_TOTAL})
+do
+	test$i
+done
+
+tst_exit
-- 
1.8.4.2

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

[Alexey Kodanev]

Hi,
On 11/05/2015 06:03 AM, Guangwen Feng wrote:
> Test mkswap(8) command with some basic options.
>
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>   runtest/commands                      |   1 +
>   testcases/commands/mkswap/Makefile    |  24 +++++
>   testcases/commands/mkswap/mkswap01.sh | 196 ++++++++++++++++++++++++++++++++++
>   3 files changed, 221 insertions(+)
>   create mode 100644 testcases/commands/mkswap/Makefile
>   create mode 100755 testcases/commands/mkswap/mkswap01.sh
>
> diff --git a/runtest/commands b/runtest/commands
> index 6c0485b..ab600dc 100644
> --- a/runtest/commands
> +++ b/runtest/commands
> @@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
>   mkfs01_msdos mkfs01.sh -f msdos
>   mkfs01_vfat mkfs01.sh -f vfat
>   mkfs01_ntfs mkfs01.sh -f ntfs
> +mkswap01 mkswap01.sh
> diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
> new file mode 100644
> index 0000000..39651c0
> --- /dev/null
> +++ b/testcases/commands/mkswap/Makefile
> @@ -0,0 +1,24 @@
> +#
> +#    commands/mkswap testcases Makefile.

There is no need for such a comment.

> +#
> +#
> +
> +setup()
> +{
> +	tst_require_root
> +
> +	tst_check_cmds losetup free mkswap
> +
> +	tst_tmpdir
> +
> +	TST_CLEANUP=cleanup
> +
> +	tst_acquire_device
> +
> +	local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
> +		cut -d '(' -f2 | cut -d ')' -f1`

Isn't it easier to get path with $(losetup -l | grep tmp | awk '{print $6}')

> +	DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
> +
> +	PAGE_SIZE=`getconf PAGE_SIZE`
> +}
> +
> +cleanup()
> +{
> +	tst_release_device
> +
> +	tst_rmdir
> +}
> +
> +mkswap_verify()
> +{
> +	local ret=0
> +
> +	local before=`free | grep "Swap" | awk '{print $2}'`
> +
> +	if [ -z "$4" ]; then
> +		local swapsize=$DEVICE_SIZE
> +	else
> +		local swapsize=$4
> +	fi
> +
> +	if [ "$1" = "-p" ]; then
> +		local pagesize=$2

I would name each parameter ($1,$2,...) in the begining of the function 
as of make them more informative.
Or use "while getopts; do".

> +
> +mkswap_test()
> +{
> +	local mkswap_op=$1
> +	local op_arg=$2
> +	local device=$3
> +	local size=$4
> +
> +	local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
> +
> +	${mkswap_cmd} >temp 2>&1
> +	if [ $? -ne 0 ]; then
> +		grep -q -E "unknown option|invalid option|Usage" temp
> +		if [ $? -eq 0 ]; then
> +			tst_resm TCONF "'${mkswap_cmd}' not supported."
> +			return
> +		else

Don't need 'else' after return.

> +			tst_resm TFAIL "'${mkswap_cmd}' failed."
> +			cat temp
> +			return
> +		fi
> +	fi
> +
> +	if [ -n "$device" ]; then
> +		mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
> +		if [ $? -ne 0 ]; then
> +			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
> +			return
> +		fi
> +	fi
> +
> +	tst_resm TPASS "'${mkswap_cmd}' passed."
> +}
> +
> +test1()
> +{
> +	mkswap_test "" "" "$TST_DEVICE"
> +}

May be it would be better to call mkswap_test() without such wrappers 
and the for loop in the end.

Best regards,
Alexey

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

[Guangwen Feng]

Hi!
Thanks for your review!

On 2015/11/05 19:41, Alexey Kodanev wrote:
> Hi,
> On 11/05/2015 06:03 AM, Guangwen Feng wrote:
>> Test mkswap(8) command with some basic options.
>>
>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> ---
>>   runtest/commands                      |   1 +
>>   testcases/commands/mkswap/Makefile    |  24 +++++
>>   testcases/commands/mkswap/mkswap01.sh | 196 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 221 insertions(+)
>>   create mode 100644 testcases/commands/mkswap/Makefile
>>   create mode 100755 testcases/commands/mkswap/mkswap01.sh
>>
>> diff --git a/runtest/commands b/runtest/commands
>> index 6c0485b..ab600dc 100644
>> --- a/runtest/commands
>> +++ b/runtest/commands
>> @@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
>>   mkfs01_msdos mkfs01.sh -f msdos
>>   mkfs01_vfat mkfs01.sh -f vfat
>>   mkfs01_ntfs mkfs01.sh -f ntfs
>> +mkswap01 mkswap01.sh
>> diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
>> new file mode 100644
>> index 0000000..39651c0
>> --- /dev/null
>> +++ b/testcases/commands/mkswap/Makefile
>> @@ -0,0 +1,24 @@
>> +#
>> +#    commands/mkswap testcases Makefile.
> 
> There is no need for such a comment.
> 

OK, got it.

>> +#
>> +#
>> +
>> +setup()
>> +{
>> +    tst_require_root
>> +
>> +    tst_check_cmds losetup free mkswap
>> +
>> +    tst_tmpdir
>> +
>> +    TST_CLEANUP=cleanup
>> +
>> +    tst_acquire_device
>> +
>> +    local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
>> +        cut -d '(' -f2 | cut -d ')' -f1`
> 
> Isn't it easier to get path with $(losetup -l | grep tmp | awk '{print $6}')
> 

Yes, it's easier to use losetup -l, but RHEL5.11GA does not support -l option,
so I would use losetup -a here.

>> +    DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
>> +
>> +    PAGE_SIZE=`getconf PAGE_SIZE`
>> +}
>> +
>> +cleanup()
>> +{
>> +    tst_release_device
>> +
>> +    tst_rmdir
>> +}
>> +
>> +mkswap_verify()
>> +{
>> +    local ret=0
>> +
>> +    local before=`free | grep "Swap" | awk '{print $2}'`
>> +
>> +    if [ -z "$4" ]; then
>> +        local swapsize=$DEVICE_SIZE
>> +    else
>> +        local swapsize=$4
>> +    fi
>> +
>> +    if [ "$1" = "-p" ]; then
>> +        local pagesize=$2
> 
> I would name each parameter ($1,$2,...) in the begining of the function as of make them more informative.
> Or use "while getopts; do".
> 

Indeed, it needs to be more informative.
I will name them, thanks.

>> +
>> +mkswap_test()
>> +{
>> +    local mkswap_op=$1
>> +    local op_arg=$2
>> +    local device=$3
>> +    local size=$4
>> +
>> +    local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
>> +
>> +    ${mkswap_cmd} >temp 2>&1
>> +    if [ $? -ne 0 ]; then
>> +        grep -q -E "unknown option|invalid option|Usage" temp
>> +        if [ $? -eq 0 ]; then
>> +            tst_resm TCONF "'${mkswap_cmd}' not supported."
>> +            return
>> +        else
> 
> Don't need 'else' after return.
> 

OK, got it.

>> +            tst_resm TFAIL "'${mkswap_cmd}' failed."
>> +            cat temp
>> +            return
>> +        fi
>> +    fi
>> +
>> +    if [ -n "$device" ]; then
>> +        mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
>> +        if [ $? -ne 0 ]; then
>> +            tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
>> +            return
>> +        fi
>> +    fi
>> +
>> +    tst_resm TPASS "'${mkswap_cmd}' passed."
>> +}
>> +
>> +test1()
>> +{
>> +    mkswap_test "" "" "$TST_DEVICE"
>> +}
> 
> May be it would be better to call mkswap_test() without such wrappers and the for loop in the end.
> 

Sorry, do you mean just call mkswap_test() straightforward with different arguments for all these tests?

Best Regards,
Guangwen Feng

> Best regards,
> Alexey
> .
> 

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

[Alexey Kodanev]

Hi,

On 11/06/2015 08:17 AM, Guangwen Feng wrote:

>>> +            tst_resm TFAIL "'${mkswap_cmd}' failed."
>>> +            cat temp
>>> +            return
>>> +        fi
>>> +    fi
>>> +
>>> +    if [ -n "$device" ]; then
>>> +        mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
>>> +        if [ $? -ne 0 ]; then
>>> +            tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
>>> +            return
>>> +        fi
>>> +    fi
>>> +
>>> +    tst_resm TPASS "'${mkswap_cmd}' passed."
>>> +}
>>> +
>>> +test1()
>>> +{
>>> +    mkswap_test "" "" "$TST_DEVICE"
>>> +}
>> May be it would be better to call mkswap_test() without such wrappers and the for loop in the end.
>>
> Sorry, do you mean just call mkswap_test() straightforward with different arguments for all these tests?

Right, the same way it is done inside each test*().

Best regards,
Alexey

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

[Lei Li]

> +setup()
> +{
> +	tst_require_root
> +
> +	tst_check_cmds losetup free mkswap
> +
> +	tst_tmpdir
> +
> +	TST_CLEANUP=cleanup
> +
> +	tst_acquire_device
> +
> +	local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
> +		cut -d '(' -f2 | cut -d ')' -f1`
> +	DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
Hi!
	This testcase uses a loop device instead of a block device, but 
tst_acquire_device may offer you a block device, /opt/ltp/runltp -b 
/dev/sdb1 for example, then, "path" may be invilidate, and syntax error 
occurs in the line :

DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))

So, examination whether the "path" is validate is necessary.

> +
> +	PAGE_SIZE=`getconf PAGE_SIZE`
> +}
> +
> +cleanup()
> +{
> +	tst_release_device
> +
> +	tst_rmdir
> +}
> +
> +mkswap_verify()
> +{
> +	local ret=0
> +
> +	local before=`free | grep "Swap" | awk '{print $2}'`
> +
> +	if [ -z "$4" ]; then
> +		local swapsize=$DEVICE_SIZE
> +	else
> +		local swapsize=$4
> +	fi
> +
> +	if [ "$1" = "-p" ]; then
> +		local pagesize=$2
> +	else
> +		local pagesize=$PAGE_SIZE
> +	fi
> +
> +	if [ "$1" = "-L" ]; then
> +		local swap_op="-L"
> +		local swapfile=$2
> +	elif [ "$1" = "-U" ]; then
> +		local swap_op="-U"
> +		local swapfile=$2
> +	else
> +		local swap_op=""
> +		local swapfile=$3
> +	fi
> +
> +	swapon $swap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "can not do swapon on $swapfile."
> +		if [ $pagesize -ne $PAGE_SIZE ]; then
> +			return $ret
> +		fi
> +
> +		if [ $swapsize -gt $DEVICE_SIZE ]; then
> +			return $ret
> +		fi
> +	fi
> +
> +	local after=`free | grep "Swap" | awk '{print $2}'`
> +
> +	local est=16
> +	if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
> +		[ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
> +		ret=1
> +	fi

How do you get this formula? I'm a newbie here :-)

> +
> +	swapoff $swap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "can not do swapoff on $swapfile."
> +	fi
> +
> +	return $ret
> +}
> +
> +mkswap_test()
> +{
> +	local mkswap_op=$1
> +	local op_arg=$2
> +	local device=$3
> +	local size=$4
> +
> +	local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
> +
> +	${mkswap_cmd} >temp 2>&1
> +	if [ $? -ne 0 ]; then
> +		grep -q -E "unknown option|invalid option|Usage" temp
> +		if [ $? -eq 0 ]; then
> +			tst_resm TCONF "'${mkswap_cmd}' not supported."
> +			return
> +		else
> +			tst_resm TFAIL "'${mkswap_cmd}' failed."
> +			cat temp
> +			return
> +		fi
> +	fi
> +
> +	if [ -n "$device" ]; then
> +		mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
> +		if [ $? -ne 0 ]; then
> +			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
> +			return
> +		fi
> +	fi
> +
> +	tst_resm TPASS "'${mkswap_cmd}' passed."
> +}
> +
> +test1()
> +{
> +	mkswap_test "" "" "$TST_DEVICE"
> +}
> +
> +test2()
> +{
> +	mkswap_test "" "" "$TST_DEVICE" "10000"
> +}
> +
> +test3()
> +{
> +	mkswap_test "-f" "" "$TST_DEVICE" "30000"
> +}
The size of loop device is 20000 currently, but there is no guarantee 
that value will not be changed in the future. "10000" and "30000" may be 
inappropriate at that time.In my opinion, it is more reasonable like this:

test2()
{
	mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
}

test3()
{
	mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
}
> +
> +test4()
> +{
> +	mkswap_test "-c" "" "$TST_DEVICE"
> +}
> +
> +test5()
> +{
> +	mkswap_test "-p" "2048" "$TST_DEVICE"
> +}
> +
> +test6()
> +{
> +	mkswap_test "-L" "testswap" "$TST_DEVICE"
> +}
> +
> +test7()
> +{
> +	mkswap_test "-v1" "" "$TST_DEVICE"
> +}
> +
> +test8()
> +{
> +	mkswap_test "-U" "ffffffff-ffff-ffff-ffff-ffffffffffff" "$TST_DEVICE"
> +}
> +
> +test9()
> +{
> +	mkswap_test "-V"
> +}
> +
> +test10()
> +{
> +	mkswap_test "-h"
> +}
> +
> +setup
> +
> +for i in $(seq 1 ${TST_TOTAL})
> +do
> +	test$i
> +done
> +
> +tst_exit
>

-- 
Rock Lee
rocklee_104@outlook.com

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

[Guangwen Feng]

Hi!
Thanks for review!

On 2015/11/09 10:00, Lei Li wrote:
>> +setup()
>> +{
>> +    tst_require_root
>> +
>> +    tst_check_cmds losetup free mkswap
>> +
>> +    tst_tmpdir
>> +
>> +    TST_CLEANUP=cleanup
>> +
>> +    tst_acquire_device
>> +
>> +    local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
>> +        cut -d '(' -f2 | cut -d ')' -f1`
>> +    DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
> Hi!
>     This testcase uses a loop device instead of a block device, but tst_acquire_device may offer you a block device, /opt/ltp/runltp -b /dev/sdb1 for example, then, "path" may be invilidate, and syntax error occurs in the line :
> 
> DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
> 
> So, examination whether the "path" is validate is necessary.
> 

Indeed, I missed the condition of block device.
I think I could use DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024)) instead of via path.

>> +
>> +    PAGE_SIZE=`getconf PAGE_SIZE`
>> +}
>> +
>> +cleanup()
>> +{
>> +    tst_release_device
>> +
>> +    tst_rmdir
>> +}
>> +
>> +mkswap_verify()
>> +{
>> +    local ret=0
>> +
>> +    local before=`free | grep "Swap" | awk '{print $2}'`
>> +
>> +    if [ -z "$4" ]; then
>> +        local swapsize=$DEVICE_SIZE
>> +    else
>> +        local swapsize=$4
>> +    fi
>> +
>> +    if [ "$1" = "-p" ]; then
>> +        local pagesize=$2
>> +    else
>> +        local pagesize=$PAGE_SIZE
>> +    fi
>> +
>> +    if [ "$1" = "-L" ]; then
>> +        local swap_op="-L"
>> +        local swapfile=$2
>> +    elif [ "$1" = "-U" ]; then
>> +        local swap_op="-U"
>> +        local swapfile=$2
>> +    else
>> +        local swap_op=""
>> +        local swapfile=$3
>> +    fi
>> +
>> +    swapon $swap_op $swapfile 2>/dev/null
>> +    if [ $? -ne 0 ]; then
>> +        tst_resm TINFO "can not do swapon on $swapfile."
>> +        if [ $pagesize -ne $PAGE_SIZE ]; then
>> +            return $ret
>> +        fi
>> +
>> +        if [ $swapsize -gt $DEVICE_SIZE ]; then
>> +            return $ret
>> +        fi
>> +    fi
>> +
>> +    local after=`free | grep "Swap" | awk '{print $2}'`
>> +
>> +    local est=16
>> +    if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
>> +        [ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
>> +        ret=1
>> +    fi
> 
> How do you get this formula? I'm a newbie here :-)
> 

I just compare device size used for swap with the actual increment of swap.
The increment should be equal to the device size minus a page size, but in some cases, the result fluctuates.

>> +
>> +    swapoff $swap_op $swapfile 2>/dev/null
>> +    if [ $? -ne 0 ]; then
>> +        tst_resm TINFO "can not do swapoff on $swapfile."
>> +    fi
>> +
>> +    return $ret
>> +}
>> +
>> +mkswap_test()
>> +{
>> +    local mkswap_op=$1
>> +    local op_arg=$2
>> +    local device=$3
>> +    local size=$4
>> +
>> +    local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
>> +
>> +    ${mkswap_cmd} >temp 2>&1
>> +    if [ $? -ne 0 ]; then
>> +        grep -q -E "unknown option|invalid option|Usage" temp
>> +        if [ $? -eq 0 ]; then
>> +            tst_resm TCONF "'${mkswap_cmd}' not supported."
>> +            return
>> +        else
>> +            tst_resm TFAIL "'${mkswap_cmd}' failed."
>> +            cat temp
>> +            return
>> +        fi
>> +    fi
>> +
>> +    if [ -n "$device" ]; then
>> +        mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
>> +        if [ $? -ne 0 ]; then
>> +            tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
>> +            return
>> +        fi
>> +    fi
>> +
>> +    tst_resm TPASS "'${mkswap_cmd}' passed."
>> +}
>> +
>> +test1()
>> +{
>> +    mkswap_test "" "" "$TST_DEVICE"
>> +}
>> +
>> +test2()
>> +{
>> +    mkswap_test "" "" "$TST_DEVICE" "10000"
>> +}
>> +
>> +test3()
>> +{
>> +    mkswap_test "-f" "" "$TST_DEVICE" "30000"
>> +}
> The size of loop device is 20000 currently, but there is no guarantee that value will not be changed in the future. "10000" and "30000" may be inappropriate at that time.In my opinion, it is more reasonable like this:
> 
> test2()
> {
>     mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
> }
> 
> test3()
> {
>     mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
> }

Yes, it's more reasonably, thanks.

>> +
>> +test4()
>> +{
>> +    mkswap_test "-c" "" "$TST_DEVICE"
>> +}
>> +
>> +test5()
>> +{
>> +    mkswap_test "-p" "2048" "$TST_DEVICE"
>> +}
>> +
>> +test6()
>> +{
>> +    mkswap_test "-L" "testswap" "$TST_DEVICE"
>> +}
>> +
>> +test7()
>> +{
>> +    mkswap_test "-v1" "" "$TST_DEVICE"
>> +}
>> +
>> +test8()
>> +{
>> +    mkswap_test "-U" "ffffffff-ffff-ffff-ffff-ffffffffffff" "$TST_DEVICE"
>> +}
>> +
>> +test9()
>> +{
>> +    mkswap_test "-V"
>> +}
>> +
>> +test10()
>> +{
>> +    mkswap_test "-h"
>> +}
>> +
>> +setup
>> +
>> +for i in $(seq 1 ${TST_TOTAL})
>> +do
>> +    test$i
>> +done
>> +
>> +tst_exit
>>
> 

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

[Guangwen Feng]

Test mkswap(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                      |   1 +
 testcases/commands/mkswap/Makefile    |  22 +++++
 testcases/commands/mkswap/mkswap01.sh | 155 ++++++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+)
 create mode 100644 testcases/commands/mkswap/Makefile
 create mode 100755 testcases/commands/mkswap/mkswap01.sh

diff --git a/runtest/commands b/runtest/commands
index 6c0485b..ab600dc 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
 mkfs01_msdos mkfs01.sh -f msdos
 mkfs01_vfat mkfs01.sh -f vfat
 mkfs01_ntfs mkfs01.sh -f ntfs
+mkswap01 mkswap01.sh
diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
new file mode 100644
index 0000000..5c8bd9b
--- /dev/null
+++ b/testcases/commands/mkswap/Makefile
@@ -0,0 +1,22 @@
+#
+#    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		:= mkswap01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkswap/mkswap01.sh b/testcases/commands/mkswap/mkswap01.sh
new file mode 100755
index 0000000..d983f0f
--- /dev/null
+++ b/testcases/commands/mkswap/mkswap01.sh
@@ -0,0 +1,155 @@
+#!/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 mkswap command with some basic options.
+#
+
+TCID=mkswap01
+TST_TOTAL=10
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds blockdev free mkswap
+
+	tst_tmpdir
+
+	TST_CLEANUP=cleanup
+
+	tst_acquire_device
+
+	DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024))
+
+	PAGE_SIZE=`getconf PAGE_SIZE`
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkswap_verify()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local device=$3
+	local size=$4
+
+	local ret=0
+
+	local before=`free | grep "Swap" | awk '{print $2}'`
+
+	if [ -z "$size" ]; then
+		local swapsize=$DEVICE_SIZE
+	else
+		local swapsize=$size
+	fi
+
+	if [ "$mkswap_op" = "-p" ]; then
+		local pagesize=$op_arg
+	else
+		local pagesize=$PAGE_SIZE
+	fi
+
+	if [ "$mkswap_op" = "-L" ]; then
+		local swap_op="-L"
+		local swapfile=$op_arg
+	elif [ "$mkswap_op" = "-U" ]; then
+		local swap_op="-U"
+		local swapfile=$op_arg
+	else
+		local swap_op=""
+		local swapfile=$device
+	fi
+
+	swapon $swap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "can not do swapon on $swapfile."
+		if [ $pagesize -ne $PAGE_SIZE ]; then
+			return $ret
+		fi
+
+		if [ $swapsize -gt $DEVICE_SIZE ]; then
+			return $ret
+		fi
+	fi
+
+	local after=`free | grep "Swap" | awk '{print $2}'`
+
+	local est=16
+	if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
+		[ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
+		ret=1
+	fi
+
+	swapoff $swap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "can not do swapoff on $swapfile."
+	fi
+
+	return $ret
+}
+
+mkswap_test()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local device=$3
+	local size=$4
+
+	local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
+
+	${mkswap_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option|invalid option|Usage" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkswap_cmd}' not supported."
+			return
+		fi
+
+		tst_resm TFAIL "'${mkswap_cmd}' failed."
+		cat temp
+		return
+	fi
+
+	if [ -n "$device" ]; then
+		mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkswap_cmd}' passed."
+}
+
+setup
+
+mkswap_test "" "" "$TST_DEVICE"
+mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
+mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
+mkswap_test "-c" "" "$TST_DEVICE"
+mkswap_test "-p" "2048" "$TST_DEVICE"
+mkswap_test "-L" "testswap" "$TST_DEVICE"
+mkswap_test "-v1" "" "$TST_DEVICE"
+mkswap_test "-U" "ffffffff-ffff-ffff-ffff-ffffffffffff" "$TST_DEVICE"
+mkswap_test "-V"
+mkswap_test "-h"
+
+tst_exit
-- 
1.8.4.2

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

[Lei Li]

> Hi!
> Thanks for review!
>
> On 2015/11/09 10:00, Lei Li wrote:
>>> +setup()
>>> +{
>>> +    tst_require_root
>>> +
>>> +    tst_check_cmds losetup free mkswap
>>> +
>>> +    tst_tmpdir
>>> +
>>> +    TST_CLEANUP=cleanup
>>> +
>>> +    tst_acquire_device
>>> +
>>> +    local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
>>> +        cut -d '(' -f2 | cut -d ')' -f1`
>>> +    DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
>> Hi!
>>      This testcase uses a loop device instead of a block device, but tst_acquire_device may offer you a block device, /opt/ltp/runltp -b /dev/sdb1 for example, then, "path" may be invilidate, and syntax error occurs in the line :
>>
>> DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
>>
>> So, examination whether the "path" is validate is necessary.
>>
>
> Indeed, I missed the condition of block device.
> I think I could use DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024)) instead of via path.
>
>>> +
>>> +    PAGE_SIZE=`getconf PAGE_SIZE`
>>> +}
>>> +
>>> +cleanup()
>>> +{
>>> +    tst_release_device
>>> +
>>> +    tst_rmdir
>>> +}
>>> +
>>> +mkswap_verify()
>>> +{
>>> +    local ret=0
>>> +
>>> +    local before=`free | grep "Swap" | awk '{print $2}'`
>>> +
>>> +    if [ -z "$4" ]; then
>>> +        local swapsize=$DEVICE_SIZE
>>> +    else
>>> +        local swapsize=$4
>>> +    fi
>>> +
>>> +    if [ "$1" = "-p" ]; then
>>> +        local pagesize=$2
>>> +    else
>>> +        local pagesize=$PAGE_SIZE
>>> +    fi
>>> +
>>> +    if [ "$1" = "-L" ]; then
>>> +        local swap_op="-L"
>>> +        local swapfile=$2
>>> +    elif [ "$1" = "-U" ]; then
>>> +        local swap_op="-U"
>>> +        local swapfile=$2
>>> +    else
>>> +        local swap_op=""
>>> +        local swapfile=$3
>>> +    fi
>>> +
>>> +    swapon $swap_op $swapfile 2>/dev/null
>>> +    if [ $? -ne 0 ]; then
>>> +        tst_resm TINFO "can not do swapon on $swapfile."
>>> +        if [ $pagesize -ne $PAGE_SIZE ]; then
>>> +            return $ret
>>> +        fi
>>> +
>>> +        if [ $swapsize -gt $DEVICE_SIZE ]; then
>>> +            return $ret
>>> +        fi
>>> +    fi
>>> +
>>> +    local after=`free | grep "Swap" | awk '{print $2}'`
>>> +
>>> +    local est=16
>>> +    if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
>>> +        [ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
>>> +        ret=1
>>> +    fi
>>
>> How do you get this formula? I'm a newbie here :-)
>>
>
> I just compare device size used for swap with the actual increment of swap.
> The increment should be equal to the device size minus a page size, but in some cases, the result fluctuates.

Hi
   I read the code of mkswap, find nothing about the fluctuation but
the exact value(as you mentioned above, device size minus a page size).
And the fluctuation didn't show up in all of my test result.Would you
please give me some proof about the fluctuation ?

Thanks.
-- 
Rock Lee
rocklee_104@outlook.com

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

[Lei Li]

> +mkswap_test "" "" "$TST_DEVICE"
> +mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
> +mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
> +mkswap_test "-c" "" "$TST_DEVICE"
> +mkswap_test "-p" "2048" "$TST_DEVICE"
> +mkswap_test "-L" "testswap" "$TST_DEVICE"
> +mkswap_test "-v1" "" "$TST_DEVICE"
> +mkswap_test "-U" "ffffffff-ffff-ffff-ffff-ffffffffffff" "$TST_DEVICE"

Hi again :-)
	This testcase changes UUID of a block device and will never
restore it. Swapon error occurs when there are two "ffffffff-
ffff-ffff-ffff-ffffffffffff" UUID in the system, for example, run this
testcase to test a block device firstly and then, test a loop device.

Maybe you should restore the UUID of the block device in cleanup().

Thanks
-- 
Rock Lee
rocklee_104@outlook.com

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

[Alexey Kodanev]

Hi,
On 11/09/2015 11:02 AM, Guangwen Feng wrote:
> Test mkswap(8) command with some basic options.
>
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>   runtest/commands                      |   1 +
>   testcases/commands/mkswap/Makefile    |  22 +++++
>   testcases/commands/mkswap/mkswap01.sh | 155 ++++++++++++++++++++++++++++++++++
>   3 files changed, 178 insertions(+)
>   create mode 100644 testcases/commands/mkswap/Makefile
>   create mode 100755 testcases/commands/mkswap/mkswap01.sh
...
> ...
> +
> +mkswap_verify()
> +{
> +	local mkswap_op=$1
> +	local op_arg=$2
> +	local device=$3
> +	local size=$4
> +
> +	local ret=0
> +
> +	local before=`free | grep "Swap" | awk '{print $2}'`
> +
> +	if [ -z "$size" ]; then
> +		local swapsize=$DEVICE_SIZE
> +	else
> +		local swapsize=$size
> +	fi

No need to check it here if you define single variable, i.e. 
size=${4:-$DEVICE_SIZE}

> +
> +	if [ "$mkswap_op" = "-p" ]; then
> +		local pagesize=$op_arg
> +	else
> +		local pagesize=$PAGE_SIZE
> +	fi
> +
> +	if [ "$mkswap_op" = "-L" ]; then
> +		local swap_op="-L"
> +		local swapfile=$op_arg
> +	elif [ "$mkswap_op" = "-U" ]; then
> +		local swap_op="-U"

'swap_op' var is redundant, instead just set "mkswap_op=" in the else 
branch below.

> +		local swapfile=$op_arg
> +	else
> +		local swap_op=""
> +		local swapfile=$device
> +	fi
> +

> +	swapon $swap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "can not do swapon on $swapfile."
> +		if [ $pagesize -ne $PAGE_SIZE ]; then
> +			return $ret
> +		fi
> +
> +		if [ $swapsize -gt $DEVICE_SIZE ]; then
> +			return $ret
> +		fi
> +	fi
> +

There are two conditions that returns 0 when swapon command fails. If it 
is expected result, could you provide more informative tst_resm message 
in each case. Other cases should return 1, right? But the test proceeds 
further.

Thanks,
Alexey

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

[Guangwen Feng]

Hi!

On 2015/11/11 11:45, Lei Li wrote:
>> Hi!
>> Thanks for review!
>>
>> On 2015/11/09 10:00, Lei Li wrote:
>>>> +setup()
>>>> +{
>>>> +    tst_require_root
>>>> +
>>>> +    tst_check_cmds losetup free mkswap
>>>> +
>>>> +    tst_tmpdir
>>>> +
>>>> +    TST_CLEANUP=cleanup
>>>> +
>>>> +    tst_acquire_device
>>>> +
>>>> +    local path=`losetup -a | grep "$TMPDIR"  | awk '{print $3}' | \
>>>> +        cut -d '(' -f2 | cut -d ')' -f1`
>>>> +    DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
>>> Hi!
>>>      This testcase uses a loop device instead of a block device, but tst_acquire_device may offer you a block device, /opt/ltp/runltp -b /dev/sdb1 for example, then, "path" may be invilidate, and syntax error occurs in the line :
>>>
>>> DEVICE_SIZE=$((`ls -l $path | awk '{print $5}'`/1024))
>>>
>>> So, examination whether the "path" is validate is necessary.
>>>
>>
>> Indeed, I missed the condition of block device.
>> I think I could use DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024)) instead of via path.
>>
>>>> +
>>>> +    PAGE_SIZE=`getconf PAGE_SIZE`
>>>> +}
>>>> +
>>>> +cleanup()
>>>> +{
>>>> +    tst_release_device
>>>> +
>>>> +    tst_rmdir
>>>> +}
>>>> +
>>>> +mkswap_verify()
>>>> +{
>>>> +    local ret=0
>>>> +
>>>> +    local before=`free | grep "Swap" | awk '{print $2}'`
>>>> +
>>>> +    if [ -z "$4" ]; then
>>>> +        local swapsize=$DEVICE_SIZE
>>>> +    else
>>>> +        local swapsize=$4
>>>> +    fi
>>>> +
>>>> +    if [ "$1" = "-p" ]; then
>>>> +        local pagesize=$2
>>>> +    else
>>>> +        local pagesize=$PAGE_SIZE
>>>> +    fi
>>>> +
>>>> +    if [ "$1" = "-L" ]; then
>>>> +        local swap_op="-L"
>>>> +        local swapfile=$2
>>>> +    elif [ "$1" = "-U" ]; then
>>>> +        local swap_op="-U"
>>>> +        local swapfile=$2
>>>> +    else
>>>> +        local swap_op=""
>>>> +        local swapfile=$3
>>>> +    fi
>>>> +
>>>> +    swapon $swap_op $swapfile 2>/dev/null
>>>> +    if [ $? -ne 0 ]; then
>>>> +        tst_resm TINFO "can not do swapon on $swapfile."
>>>> +        if [ $pagesize -ne $PAGE_SIZE ]; then
>>>> +            return $ret
>>>> +        fi
>>>> +
>>>> +        if [ $swapsize -gt $DEVICE_SIZE ]; then
>>>> +            return $ret
>>>> +        fi
>>>> +    fi
>>>> +
>>>> +    local after=`free | grep "Swap" | awk '{print $2}'`
>>>> +
>>>> +    local est=16
>>>> +    if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
>>>> +        [ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
>>>> +        ret=1
>>>> +    fi
>>>
>>> How do you get this formula? I'm a newbie here :-)
>>>
>>
>> I just compare device size used for swap with the actual increment of swap.
>> The increment should be equal to the device size minus a page size, but in some cases, the result fluctuates.
> 
> Hi
>   I read the code of mkswap, find nothing about the fluctuation but
> the exact value(as you mentioned above, device size minus a page size).
> And the fluctuation didn't show up in all of my test result.Would you
> please give me some proof about the fluctuation ?
> 

Yes, in most of my environment, there is no fluctuation but the exact value,
but for example, on my RHEL5.11GA, the result is not exactly equal to the value somehow,
maybe the result is slightly different in some kernels.

Best Regards,
Guangwen Feng

> Thanks.

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

[Guangwen Feng]

Hi!

On 2015/11/11 12:50, Lei Li wrote:
>> +mkswap_test "" "" "$TST_DEVICE"
>> +mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
>> +mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
>> +mkswap_test "-c" "" "$TST_DEVICE"
>> +mkswap_test "-p" "2048" "$TST_DEVICE"
>> +mkswap_test "-L" "testswap" "$TST_DEVICE"
>> +mkswap_test "-v1" "" "$TST_DEVICE"
>> +mkswap_test "-U" "ffffffff-ffff-ffff-ffff-ffffffffffff" "$TST_DEVICE"
> 
> Hi again :-)
>     This testcase changes UUID of a block device and will never
> restore it. Swapon error occurs when there are two "ffffffff-
> ffff-ffff-ffff-ffffffffffff" UUID in the system, for example, run this
> testcase to test a block device firstly and then, test a loop device.
> 

Indeed, just like you mentioned, it need to be fixed, thanks!

Best Regards,
Guangwen Feng

> Maybe you should restore the UUID of the block device in cleanup().
> 
> Thanks

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

[Guangwen Feng]

Hi!

I will modify the test case according to your suggestion, thanks!

Best Regards,
Guangwen Feng

On 2015/11/11 17:44, Alexey Kodanev wrote:
> Hi,
> On 11/09/2015 11:02 AM, Guangwen Feng wrote:
>> Test mkswap(8) command with some basic options.
>>
>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> ---
>>   runtest/commands                      |   1 +
>>   testcases/commands/mkswap/Makefile    |  22 +++++
>>   testcases/commands/mkswap/mkswap01.sh | 155 ++++++++++++++++++++++++++++++++++
>>   3 files changed, 178 insertions(+)
>>   create mode 100644 testcases/commands/mkswap/Makefile
>>   create mode 100755 testcases/commands/mkswap/mkswap01.sh
> ...
>> ...
>> +
>> +mkswap_verify()
>> +{
>> +    local mkswap_op=$1
>> +    local op_arg=$2
>> +    local device=$3
>> +    local size=$4
>> +
>> +    local ret=0
>> +
>> +    local before=`free | grep "Swap" | awk '{print $2}'`
>> +
>> +    if [ -z "$size" ]; then
>> +        local swapsize=$DEVICE_SIZE
>> +    else
>> +        local swapsize=$size
>> +    fi
> 
> No need to check it here if you define single variable, i.e. size=${4:-$DEVICE_SIZE}
> 
>> +
>> +    if [ "$mkswap_op" = "-p" ]; then
>> +        local pagesize=$op_arg
>> +    else
>> +        local pagesize=$PAGE_SIZE
>> +    fi
>> +
>> +    if [ "$mkswap_op" = "-L" ]; then
>> +        local swap_op="-L"
>> +        local swapfile=$op_arg
>> +    elif [ "$mkswap_op" = "-U" ]; then
>> +        local swap_op="-U"
> 
> 'swap_op' var is redundant, instead just set "mkswap_op=" in the else branch below.
> 
>> +        local swapfile=$op_arg
>> +    else
>> +        local swap_op=""
>> +        local swapfile=$device
>> +    fi
>> +
> 
>> +    swapon $swap_op $swapfile 2>/dev/null
>> +    if [ $? -ne 0 ]; then
>> +        tst_resm TINFO "can not do swapon on $swapfile."
>> +        if [ $pagesize -ne $PAGE_SIZE ]; then
>> +            return $ret
>> +        fi
>> +
>> +        if [ $swapsize -gt $DEVICE_SIZE ]; then
>> +            return $ret
>> +        fi
>> +    fi
>> +
> 
> There are two conditions that returns 0 when swapon command fails. If it is expected result, could you provide more informative tst_resm message in each case. Other cases should return 1, right? But the test proceeds further.
> 
> Thanks,
> Alexey
> .
> 

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

[Guangwen Feng]

Test mkswap(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                      |   1 +
 testcases/commands/mkswap/Makefile    |  22 +++++
 testcases/commands/mkswap/mkswap01.sh | 158 ++++++++++++++++++++++++++++++++++
 3 files changed, 181 insertions(+)
 create mode 100644 testcases/commands/mkswap/Makefile
 create mode 100755 testcases/commands/mkswap/mkswap01.sh

diff --git a/runtest/commands b/runtest/commands
index 6c0485b..ab600dc 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
 mkfs01_msdos mkfs01.sh -f msdos
 mkfs01_vfat mkfs01.sh -f vfat
 mkfs01_ntfs mkfs01.sh -f ntfs
+mkswap01 mkswap01.sh
diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
new file mode 100644
index 0000000..5c8bd9b
--- /dev/null
+++ b/testcases/commands/mkswap/Makefile
@@ -0,0 +1,22 @@
+#
+#    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		:= mkswap01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkswap/mkswap01.sh b/testcases/commands/mkswap/mkswap01.sh
new file mode 100755
index 0000000..dc4e8ee
--- /dev/null
+++ b/testcases/commands/mkswap/mkswap01.sh
@@ -0,0 +1,158 @@
+#!/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 mkswap command with some basic options.
+#
+
+TCID=mkswap01
+TST_TOTAL=10
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds blkid blockdev free mkswap
+
+	tst_tmpdir
+
+	TST_CLEANUP=cleanup
+
+	tst_acquire_device
+
+	UUID=`uuidgen`
+
+	DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024))
+
+	PAGE_SIZE=`getconf PAGE_SIZE`
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkswap_verify()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local device=$3
+	local size=$4
+
+	local ret=0
+
+	local before=`free | grep "Swap" | awk '{print $2}'`
+
+	local swapsize=${4:-$DEVICE_SIZE}
+
+	if [ "$mkswap_op" = "-p" ]; then
+		local pagesize=$op_arg
+	else
+		local pagesize=$PAGE_SIZE
+	fi
+
+	if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then
+		local swapfile=$op_arg
+	else
+		mkswap_op=
+		local swapfile=$device
+	fi
+
+	swapon $mkswap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "Can not do swapon on $swapfile."
+		if [ $pagesize -ne $PAGE_SIZE ]; then
+			tst_resm TINFO "Page size specified by 'mkswap -p' \
+is not equal to system's page size."
+			tst_resm TINFO "Swapon failed expectedly."
+			return $ret
+		fi
+
+		if [ $swapsize -gt $DEVICE_SIZE ]; then
+			tst_resm TINFO "Device size specified by 'mkswap' \
+greater than real size."
+			tst_resm TINFO "Swapon failed expectedly."
+			return $ret
+		fi
+
+		tst_resm TINFO "Swapon failed unexpectedly."
+		return 1
+	fi
+
+	local after=`free | grep "Swap" | awk '{print $2}'`
+
+	local est=16
+	if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
+		[ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
+		ret=1
+	fi
+
+	swapoff $mkswap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "Can not do swapoff on $swapfile."
+	fi
+
+	return $ret
+}
+
+mkswap_test()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local device=$3
+	local size=$4
+
+	local mkswap_cmd="mkswap $mkswap_op $op_arg $device $size"
+
+	${mkswap_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option|invalid option|Usage" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkswap_cmd}' not supported."
+			return
+		fi
+
+		tst_resm TFAIL "'${mkswap_cmd}' failed."
+		cat temp
+		return
+	fi
+
+	if [ -n "$device" ]; then
+		mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkswap_cmd}' passed."
+}
+
+setup
+
+mkswap_test "" "" "$TST_DEVICE"
+mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
+mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
+mkswap_test "-c" "" "$TST_DEVICE"
+mkswap_test "-p" "2048" "$TST_DEVICE"
+mkswap_test "-L" "testswap" "$TST_DEVICE"
+mkswap_test "-v1" "" "$TST_DEVICE"
+mkswap_test "-U" "$UUID" "$TST_DEVICE"
+mkswap_test "-V"
+mkswap_test "-h"
+
+tst_exit
-- 
1.8.4.2

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

[Cyril Hrubis]

Hi!
> +TCID=mkswap01
> +TST_TOTAL=10
> +. test.sh
> +
> +setup()
> +{
> +	tst_require_root
> +
> +	tst_check_cmds blkid blockdev free mkswap
                        ^
			uuidgen is missing here

> +	tst_tmpdir
> +
> +	TST_CLEANUP=cleanup
> +
> +	tst_acquire_device
> +
> +	UUID=`uuidgen`
> +
> +	DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024))
> +
> +	PAGE_SIZE=`getconf PAGE_SIZE`
> +}
> +
> +cleanup()
> +{
> +	tst_release_device
> +
> +	tst_rmdir
> +}
> +
> +mkswap_verify()
> +{
> +	local mkswap_op=$1
> +	local op_arg=$2
> +	local device=$3
> +	local size=$4
> +
> +	local ret=0
> +
> +	local before=`free | grep "Swap" | awk '{print $2}'`

We can do better with:

awk '/SwapTotal/ {print $2}' /proc/meminfo

No need to call free and no need to use grep :).

> +	local swapsize=${4:-$DEVICE_SIZE}
> +
> +	if [ "$mkswap_op" = "-p" ]; then
> +		local pagesize=$op_arg
> +	else
> +		local pagesize=$PAGE_SIZE
> +	fi
> +
> +	if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then
> +		local swapfile=$op_arg
> +	else
> +		mkswap_op=
> +		local swapfile=$device
> +	fi

Can you actually pass the device for the test instead of passing
$TST_DEVICE which is kind of pointles when you can use $TST_DEVICE in
the mkswap_test() for the mkswap command?

> +	swapon $mkswap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "Can not do swapon on $swapfile."
> +		if [ $pagesize -ne $PAGE_SIZE ]; then
> +			tst_resm TINFO "Page size specified by 'mkswap -p' \
> +is not equal to system's page size."
> +			tst_resm TINFO "Swapon failed expectedly."
> +			return $ret
                              ^
                        Just do return 0 here, it's more confusing as
			it should be as this is.
> +		fi
> +
> +		if [ $swapsize -gt $DEVICE_SIZE ]; then
> +			tst_resm TINFO "Device size specified by 'mkswap' \
> +greater than real size."
> +			tst_resm TINFO "Swapon failed expectedly."
> +			return $ret

Here as well.

> +		fi
> +
> +		tst_resm TINFO "Swapon failed unexpectedly."
> +		return 1
> +	fi
> +
> +	local after=`free | grep "Swap" | awk '{print $2}'`

Here you can simplify the command as well.

> +	local est=16

You shoud intialize the ret=0 here.

> +	if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
> +		[ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
> +		ret=1
> +	fi

What is the story behind the est? It should probably be worth comment
when the size could be 16kB off or at least on which system does this
happens.

And I would compute the difference only once and store it to a variable,
but that is minor.

> +	swapoff $mkswap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "Can not do swapoff on $swapfile."
> +	fi
> +
> +	return $ret

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[Guangwen Feng]

Hi!

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

Best Regards,
Guangwen Feng

On 2015/11/17 00:31, Cyril Hrubis wrote:
> Hi!
>> +TCID=mkswap01
>> +TST_TOTAL=10
>> +. test.sh
>> +
>> +setup()
>> +{
>> +	tst_require_root
>> +
>> +	tst_check_cmds blkid blockdev free mkswap
>                         ^
> 			uuidgen is missing here
> 
>> +	tst_tmpdir
>> +
>> +	TST_CLEANUP=cleanup
>> +
>> +	tst_acquire_device
>> +
>> +	UUID=`uuidgen`
>> +
>> +	DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024))
>> +
>> +	PAGE_SIZE=`getconf PAGE_SIZE`
>> +}
>> +
>> +cleanup()
>> +{
>> +	tst_release_device
>> +
>> +	tst_rmdir
>> +}
>> +
>> +mkswap_verify()
>> +{
>> +	local mkswap_op=$1
>> +	local op_arg=$2
>> +	local device=$3
>> +	local size=$4
>> +
>> +	local ret=0
>> +
>> +	local before=`free | grep "Swap" | awk '{print $2}'`
> 
> We can do better with:
> 
> awk '/SwapTotal/ {print $2}' /proc/meminfo
> 
> No need to call free and no need to use grep :).
> 
>> +	local swapsize=${4:-$DEVICE_SIZE}
>> +
>> +	if [ "$mkswap_op" = "-p" ]; then
>> +		local pagesize=$op_arg
>> +	else
>> +		local pagesize=$PAGE_SIZE
>> +	fi
>> +
>> +	if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then
>> +		local swapfile=$op_arg
>> +	else
>> +		mkswap_op=
>> +		local swapfile=$device
>> +	fi
> 
> Can you actually pass the device for the test instead of passing
> $TST_DEVICE which is kind of pointles when you can use $TST_DEVICE in
> the mkswap_test() for the mkswap command?
> 
>> +	swapon $mkswap_op $swapfile 2>/dev/null
>> +	if [ $? -ne 0 ]; then
>> +		tst_resm TINFO "Can not do swapon on $swapfile."
>> +		if [ $pagesize -ne $PAGE_SIZE ]; then
>> +			tst_resm TINFO "Page size specified by 'mkswap -p' \
>> +is not equal to system's page size."
>> +			tst_resm TINFO "Swapon failed expectedly."
>> +			return $ret
>                               ^
>                         Just do return 0 here, it's more confusing as
> 			it should be as this is.
>> +		fi
>> +
>> +		if [ $swapsize -gt $DEVICE_SIZE ]; then
>> +			tst_resm TINFO "Device size specified by 'mkswap' \
>> +greater than real size."
>> +			tst_resm TINFO "Swapon failed expectedly."
>> +			return $ret
> 
> Here as well.
> 
>> +		fi
>> +
>> +		tst_resm TINFO "Swapon failed unexpectedly."
>> +		return 1
>> +	fi
>> +
>> +	local after=`free | grep "Swap" | awk '{print $2}'`
> 
> Here you can simplify the command as well.
> 
>> +	local est=16
> 
> You shoud intialize the ret=0 here.
> 
>> +	if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \
>> +		[ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then
>> +		ret=1
>> +	fi
> 
> What is the story behind the est? It should probably be worth comment
> when the size could be 16kB off or at least on which system does this
> happens.
> 
> And I would compute the difference only once and store it to a variable,
> but that is minor.
> 
>> +	swapoff $mkswap_op $swapfile 2>/dev/null
>> +	if [ $? -ne 0 ]; then
>> +		tst_resm TINFO "Can not do swapoff on $swapfile."
>> +	fi
>> +
>> +	return $ret
> 
> Otherwise it looks good.
> 

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

[Guangwen Feng]

Test mkswap(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                      |   1 +
 testcases/commands/mkswap/Makefile    |  22 +++++
 testcases/commands/mkswap/mkswap01.sh | 162 ++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 testcases/commands/mkswap/Makefile
 create mode 100755 testcases/commands/mkswap/mkswap01.sh

diff --git a/runtest/commands b/runtest/commands
index 6c0485b..ab600dc 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
 mkfs01_msdos mkfs01.sh -f msdos
 mkfs01_vfat mkfs01.sh -f vfat
 mkfs01_ntfs mkfs01.sh -f ntfs
+mkswap01 mkswap01.sh
diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
new file mode 100644
index 0000000..5c8bd9b
--- /dev/null
+++ b/testcases/commands/mkswap/Makefile
@@ -0,0 +1,22 @@
+#
+#    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		:= mkswap01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkswap/mkswap01.sh b/testcases/commands/mkswap/mkswap01.sh
new file mode 100755
index 0000000..9c11032
--- /dev/null
+++ b/testcases/commands/mkswap/mkswap01.sh
@@ -0,0 +1,162 @@
+#!/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 mkswap command with some basic options.
+#
+
+TCID=mkswap01
+TST_TOTAL=10
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds uuidgen blkid blockdev mkswap
+
+	tst_tmpdir
+
+	TST_CLEANUP=cleanup
+
+	tst_acquire_device
+
+	UUID=`uuidgen`
+
+	DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024))
+
+	PAGE_SIZE=`getconf PAGE_SIZE`
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkswap_verify()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local size=$3
+
+	local before=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
+
+	local swapsize=${3:-$DEVICE_SIZE}
+
+	if [ "$mkswap_op" = "-p" ]; then
+		local pagesize=$op_arg
+	else
+		local pagesize=$PAGE_SIZE
+	fi
+
+	if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then
+		local swapfile=$op_arg
+	else
+		mkswap_op=
+		local swapfile=$TST_DEVICE
+	fi
+
+	swapon $mkswap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "Can not do swapon on $swapfile."
+		if [ $pagesize -ne $PAGE_SIZE ]; then
+			tst_resm TINFO "Page size specified by 'mkswap -p' \
+is not equal to system's page size."
+			tst_resm TINFO "Swapon failed expectedly."
+			return 0
+		fi
+
+		if [ $swapsize -gt $DEVICE_SIZE ]; then
+			tst_resm TINFO "Device size specified by 'mkswap' \
+greater than real size."
+			tst_resm TINFO "Swapon failed expectedly."
+			return 0
+		fi
+
+		tst_resm TINFO "Swapon failed unexpectedly."
+		return 1
+	fi
+
+	local after=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
+
+	local diff=$((after-before))
+	local filesize=$((swapsize-pagesize/1024))
+
+	local ret=0
+
+	# In general, the swap increment by doing swapon should be equal to
+	# the device size minus a page size, however for some kernels, the
+	# increment we get is not exactly equal to that value, but is equal
+	# to the value minus an extra page size, e.g. on RHEL5.11GA.
+	if [ $diff -ne $filesize ] && \
+		[ $diff -ne $((filesize-pagesize/1024)) ]; then
+		ret=1
+	fi
+
+	swapoff $mkswap_op $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "Can not do swapoff on $swapfile."
+	fi
+
+	return $ret
+}
+
+mkswap_test()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local size=$4
+
+	local mkswap_cmd="mkswap $mkswap_op $op_arg $TST_DEVICE $size"
+
+	${mkswap_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option|invalid option|Usage" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkswap_cmd}' not supported."
+			return
+		fi
+
+		tst_resm TFAIL "'${mkswap_cmd}' failed."
+		cat temp
+		return
+	fi
+
+	if [ -n $3 ]; then
+		mkswap_verify "$mkswap_op" "$op_arg" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkswap_cmd}' passed."
+}
+
+setup
+
+mkswap_test "" "" "$TST_DEVICE"
+mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
+mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
+mkswap_test "-c" "" "$TST_DEVICE"
+mkswap_test "-p" "2048" "$TST_DEVICE"
+mkswap_test "-L" "testswap" "$TST_DEVICE"
+mkswap_test "-v1" "" "$TST_DEVICE"
+mkswap_test "-U" "$UUID" "$TST_DEVICE"
+mkswap_test "-V"
+mkswap_test "-h"
+
+tst_exit
-- 
1.8.4.2

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

[Cyril Hrubis]

Hi!
> +	if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then
> +		local swapfile=$op_arg
> +	else
> +		mkswap_op=
> +		local swapfile=$TST_DEVICE
> +	fi

What I asked you to do is to pass either the label or uuid for -L and -U
as $3 and $TST_DEVICE otherwise to the mkswap_test function. Then we can
just pass it in mkswap_test to the verify function so that we do not
have to do this ugly if. Ok?

Looking at the code we would have to actually pass "-L label" and
"-U uuid" or $TST_DEVICE. But that would be way nicer than spreading ifs
arounds.

> +	swapon $mkswap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "Can not do swapon on $swapfile."
> +		if [ $pagesize -ne $PAGE_SIZE ]; then
> +			tst_resm TINFO "Page size specified by 'mkswap -p' \
> +is not equal to system's page size."
> +			tst_resm TINFO "Swapon failed expectedly."
> +			return 0
> +		fi
> +
> +		if [ $swapsize -gt $DEVICE_SIZE ]; then
> +			tst_resm TINFO "Device size specified by 'mkswap' \
> +greater than real size."
> +			tst_resm TINFO "Swapon failed expectedly."
> +			return 0
> +		fi
> +
> +		tst_resm TINFO "Swapon failed unexpectedly."
> +		return 1
> +	fi
> +
> +	local after=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
> +
> +	local diff=$((after-before))
> +	local filesize=$((swapsize-pagesize/1024))
> +
> +	local ret=0
> +
> +	# In general, the swap increment by doing swapon should be equal to
> +	# the device size minus a page size, however for some kernels, the
> +	# increment we get is not exactly equal to that value, but is equal
> +	# to the value minus an extra page size, e.g. on RHEL5.11GA.
> +	if [ $diff -ne $filesize ] && \
> +		[ $diff -ne $((filesize-pagesize/1024)) ]; then
> +		ret=1
> +	fi

This looks good now, thanks for the detailed description.

> +	swapoff $mkswap_op $swapfile 2>/dev/null
> +	if [ $? -ne 0 ]; then
> +		tst_resm TINFO "Can not do swapoff on $swapfile."

                         I would change this to TWARN so that we make
			 sure that tester would notice
			 the failure in test results.

> +	fi
> +
> +	return $ret
> +}


The rest looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

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

[Guangwen Feng]

Hi!

On 2015/11/19 21:41, Cyril Hrubis wrote:
> Hi!
>> +	if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then
>> +		local swapfile=$op_arg
>> +	else
>> +		mkswap_op=
>> +		local swapfile=$TST_DEVICE
>> +	fi
> 
> What I asked you to do is to pass either the label or uuid for -L and -U
> as $3 and $TST_DEVICE otherwise to the mkswap_test function. Then we can
> just pass it in mkswap_test to the verify function so that we do not
> have to do this ugly if. Ok?
> 
> Looking at the code we would have to actually pass "-L label" and
> "-U uuid" or $TST_DEVICE. But that would be way nicer than spreading ifs
> arounds.
> 

OK, I got it now, thanks!
Sorry for my misunderstanding...

Best Regards,
Guangwen Feng

>> +	swapon $mkswap_op $swapfile 2>/dev/null
>> +	if [ $? -ne 0 ]; then
>> +		tst_resm TINFO "Can not do swapon on $swapfile."
>> +		if [ $pagesize -ne $PAGE_SIZE ]; then
>> +			tst_resm TINFO "Page size specified by 'mkswap -p' \
>> +is not equal to system's page size."
>> +			tst_resm TINFO "Swapon failed expectedly."
>> +			return 0
>> +		fi
>> +
>> +		if [ $swapsize -gt $DEVICE_SIZE ]; then
>> +			tst_resm TINFO "Device size specified by 'mkswap' \
>> +greater than real size."
>> +			tst_resm TINFO "Swapon failed expectedly."
>> +			return 0
>> +		fi
>> +
>> +		tst_resm TINFO "Swapon failed unexpectedly."
>> +		return 1
>> +	fi
>> +
>> +	local after=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
>> +
>> +	local diff=$((after-before))
>> +	local filesize=$((swapsize-pagesize/1024))
>> +
>> +	local ret=0
>> +
>> +	# In general, the swap increment by doing swapon should be equal to
>> +	# the device size minus a page size, however for some kernels, the
>> +	# increment we get is not exactly equal to that value, but is equal
>> +	# to the value minus an extra page size, e.g. on RHEL5.11GA.
>> +	if [ $diff -ne $filesize ] && \
>> +		[ $diff -ne $((filesize-pagesize/1024)) ]; then
>> +		ret=1
>> +	fi
> 
> This looks good now, thanks for the detailed description.
> 
>> +	swapoff $mkswap_op $swapfile 2>/dev/null
>> +	if [ $? -ne 0 ]; then
>> +		tst_resm TINFO "Can not do swapoff on $swapfile."
> 
>                          I would change this to TWARN so that we make
> 			 sure that tester would notice
> 			 the failure in test results.
> 
>> +	fi
>> +
>> +	return $ret
>> +}
> 
> 
> The rest looks good to me.
> 

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

[Guangwen Feng]

Test mkswap(8) command with some basic options.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 runtest/commands                      |   1 +
 testcases/commands/mkswap/Makefile    |  22 +++++
 testcases/commands/mkswap/mkswap01.sh | 156 ++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 testcases/commands/mkswap/Makefile
 create mode 100755 testcases/commands/mkswap/mkswap01.sh

diff --git a/runtest/commands b/runtest/commands
index 6c0485b..ab600dc 100644
--- a/runtest/commands
+++ b/runtest/commands
@@ -38,3 +38,4 @@ mkfs01_minix mkfs01.sh -f minix
 mkfs01_msdos mkfs01.sh -f msdos
 mkfs01_vfat mkfs01.sh -f vfat
 mkfs01_ntfs mkfs01.sh -f ntfs
+mkswap01 mkswap01.sh
diff --git a/testcases/commands/mkswap/Makefile b/testcases/commands/mkswap/Makefile
new file mode 100644
index 0000000..5c8bd9b
--- /dev/null
+++ b/testcases/commands/mkswap/Makefile
@@ -0,0 +1,22 @@
+#
+#    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		:= mkswap01.sh
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/commands/mkswap/mkswap01.sh b/testcases/commands/mkswap/mkswap01.sh
new file mode 100755
index 0000000..2a0b3c7
--- /dev/null
+++ b/testcases/commands/mkswap/mkswap01.sh
@@ -0,0 +1,156 @@
+#!/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 mkswap command with some basic options.
+#
+
+TCID=mkswap01
+TST_TOTAL=10
+. test.sh
+
+setup()
+{
+	tst_require_root
+
+	tst_check_cmds uuidgen blkid blockdev mkswap
+
+	tst_tmpdir
+
+	TST_CLEANUP=cleanup
+
+	tst_acquire_device
+
+	UUID=`uuidgen`
+
+	DEVICE_SIZE=$((`blockdev --getsize64 $TST_DEVICE`/1024))
+
+	PAGE_SIZE=`getconf PAGE_SIZE`
+}
+
+cleanup()
+{
+	tst_release_device
+
+	tst_rmdir
+}
+
+mkswap_verify()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local swapfile=$3
+
+	local before=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
+
+	local swapsize=${4:-$DEVICE_SIZE}
+
+	if [ "$mkswap_op" = "-p" ]; then
+		local pagesize=$op_arg
+	else
+		local pagesize=$PAGE_SIZE
+	fi
+
+	swapon $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TINFO "Can not do swapon on $swapfile."
+		if [ $pagesize -ne $PAGE_SIZE ]; then
+			tst_resm TINFO "Page size specified by 'mkswap -p' \
+is not equal to system's page size."
+			tst_resm TINFO "Swapon failed expectedly."
+			return 0
+		fi
+
+		if [ $swapsize -gt $DEVICE_SIZE ]; then
+			tst_resm TINFO "Device size specified by 'mkswap' \
+greater than real size."
+			tst_resm TINFO "Swapon failed expectedly."
+			return 0
+		fi
+
+		tst_resm TINFO "Swapon failed unexpectedly."
+		return 1
+	fi
+
+	local after=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
+
+	local diff=$((after-before))
+	local filesize=$((swapsize-pagesize/1024))
+
+	local ret=0
+
+	# In general, the swap increment by doing swapon should be equal to
+	# the device size minus a page size, however for some kernels, the
+	# increment we get is not exactly equal to that value, but is equal
+	# to the value minus an extra page size, e.g. on RHEL5.11GA.
+	if [ $diff -ne $filesize ] && \
+		[ $diff -ne $((filesize-pagesize/1024)) ]; then
+		ret=1
+	fi
+
+	swapoff $swapfile 2>/dev/null
+	if [ $? -ne 0 ]; then
+		tst_resm TWARN "Can not do swapoff on $swapfile."
+	fi
+
+	return $ret
+}
+
+mkswap_test()
+{
+	local mkswap_op=$1
+	local op_arg=$2
+	local device=$3
+	local size=$4
+
+	local mkswap_cmd="mkswap $mkswap_op $op_arg $TST_DEVICE $size"
+
+	${mkswap_cmd} >temp 2>&1
+	if [ $? -ne 0 ]; then
+		grep -q -E "unknown option|invalid option|Usage" temp
+		if [ $? -eq 0 ]; then
+			tst_resm TCONF "'${mkswap_cmd}' not supported."
+			return
+		fi
+
+		tst_resm TFAIL "'${mkswap_cmd}' failed."
+		cat temp
+		return
+	fi
+
+	if [ -n "$device" ]; then
+		mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size"
+		if [ $? -ne 0 ]; then
+			tst_resm TFAIL "'${mkswap_cmd}' failed, not expected."
+			return
+		fi
+	fi
+
+	tst_resm TPASS "'${mkswap_cmd}' passed."
+}
+
+setup
+
+mkswap_test "" "" "$TST_DEVICE"
+mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))"
+mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))"
+mkswap_test "-c" "" "$TST_DEVICE"
+mkswap_test "-p" "2048" "$TST_DEVICE"
+mkswap_test "-L" "testswap" "-L testswap"
+mkswap_test "-v1" "" "$TST_DEVICE"
+mkswap_test "-U" "$UUID" "-U $UUID"
+mkswap_test "-V"
+mkswap_test "-h"
+
+tst_exit
-- 
1.8.4.2

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

[Cyril Hrubis]

Hi!
> +mkswap_test "-L" "testswap" "-L testswap"

I've changed the label to "ltp_testswap" to be extra sure that it does
not collide with anything on the system and pushed the patch, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz