commands/mkswap: Added new testcase to test mkswap(8).
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Test mkswap(8) command with some basic options.
--- 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
runtest/commands +1 -0
@@ -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
testcases/commands/mkswap/Makefile new file +24 -0
@@ -0,0 +1,24 @@ +# +# commands/mkswap testcases Makefile. +# +# Copyright (c) 2015 Fujitsu Ltd. +# Author:Guangwen Feng <[email protected]> +# +# 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
testcases/commands/mkswap/mkswap01.sh new file +196 -0
196 lines not on this page · raw message
Signature · 1 line
1.8.4.2
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi,
Quoted 26 lines · Guangwen Feng
Test mkswap(8) command with some basic options.
Signed-off-by Guangwen Feng <[email protected]>--- 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.shruntest/commands +1
-0@@ -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.shtestcases/commands/mkswap/Makefile new file +0
-0
testcases/commands/mkswap/Makefile line 2
Quoted 2 lines
testcases/commands/mkswap/Makefile new file +2
-0@@ -0,0 +1,24 @@ +# +# commands/mkswap testcases Makefile.
There is no need for such a comment.
Quoted 7 lines
testcases/commands/mkswap/Makefile new file +7
-0+# +# + +setup() +{ + tst_require_root +
testcases/commands/mkswap/Makefile line 19
Quoted 10 lines
testcases/commands/mkswap/Makefile new file +10
-0+ 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}')
Quoted 16 lines
testcases/commands/mkswap/Makefile new file +16
-0+ 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 +
testcases/commands/mkswap/Makefile
Quoted 10 lines
testcases/commands/mkswap/Makefile new file +10
-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".
Quoted 7 lines
testcases/commands/mkswap/Makefile new file +7
-0+ +mkswap_test() +{ + local mkswap_op=$1 + local op_arg=$2 + local device=$3 + local size=$4
testcases/commands/mkswap/Makefile
Quoted 10 lines
testcases/commands/mkswap/Makefile new file +10
-0+ + 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.
Quoted 11 lines
testcases/commands/mkswap/Makefile new file +11
-0+ 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
testcases/commands/mkswap/Makefile
Quoted 10 lines
testcases/commands/mkswap/Makefile new file +10
-0+ 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
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Thanks for your review!
Quoted 24 lines · Alexey Kodanev
Hi,
Quoted 22 lines · Guangwen Feng
Test mkswap(8) command with some basic options.
Signed-off-by Guangwen Feng <[email protected]>--- 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.shruntest/commands +1
-0@@ -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.shtestcases/commands/mkswap/Makefile new file +0
-0
Quoted 9 lines
testcases/commands/mkswap/Makefile line 2
Quoted 6 lines
index 0000000..39651c0
testcases/commands/mkswap/Makefile new file +2
-0@@ -0,0 +1,24 @@ +# +# commands/mkswap testcases Makefile.There is no need for such a comment.
OK, got it.
Quoted 11 lines
Quoted 11 lines
testcases/commands/mkswap/Makefile new file +11
-0+# +# + +setup() +{ + tst_require_root + + tst_check_cmds losetup free mkswap + + tst_tmpdir +
Quoted 9 lines
testcases/commands/mkswap/Makefile line 19
Quoted 6 lines
testcases/commands/mkswap/Makefile new file +6
-0+ 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.
Quoted 21 lines
Quoted 21 lines
testcases/commands/mkswap/Makefile new file +21
-0+ 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
Quoted 9 lines
testcases/commands/mkswap/Makefile
Quoted 5 lines
testcases/commands/mkswap/Makefile new file +5
-0+ local swapsize=$4 + fi + + if [ "$1" = "-p" ]; then + local pagesize=$2I 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.
Quoted 11 lines
Quoted 11 lines
testcases/commands/mkswap/Makefile new file +11
-0+ +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
Quoted 9 lines
testcases/commands/mkswap/Makefile
Quoted 6 lines
testcases/commands/mkswap/Makefile new file +6
-0+ 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 + elseDon't need 'else' after return.
OK, got it.
Quoted 15 lines
Quoted 15 lines
testcases/commands/mkswap/Makefile new file +15
-0+ 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."
Quoted 9 lines
testcases/commands/mkswap/Makefile
Quoted 6 lines
testcases/commands/mkswap/Makefile new file +6
-0+} + +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
Quoted 4 lines
Best regards,
Alexey
.
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi,
Quoted 23 lines · Guangwen Feng
Quoted 14 lines
Quoted 14 lines
testcases/commands/mkswap/mkswap01.sh new file +14
-0+ 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 +Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh line 142
Quoted 7 lines
testcases/commands/mkswap/mkswap01.sh new file +7
-0+ 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.
Quoted 1 line
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
PATCH v2 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Test mkswap(8) command with some basic options.
--- 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
runtest/commands +1 -0
@@ -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
testcases/commands/mkswap/Makefile new file +22 -0
@@ -0,0 +1,22 @@ +# +# Copyright (c) 2015 Fujitsu Ltd. +# Author:Guangwen Feng <[email protected]> +# +# 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
testcases/commands/mkswap/mkswap01.sh new file +155 -0
155 lines not on this page · raw message
Signature · 1 line
1.8.4.2
PATCH v2 1/1 commands/mkswap: Added new testcase to test mkswap(8).
testcases/commands/mkswap/mkswap01.sh line 151
Quoted 8 lines
testcases/commands/mkswap/mkswap01.sh new file +8
-0+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
Signature · 2 lines
Rock Lee [email protected]
PATCH v2 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Quoted 6 lines · Lei Li
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+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"
Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh line 151
Quoted 2 lines
testcases/commands/mkswap/mkswap01.sh new file +2
-0+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
Quoted 3 lines
Maybe you should restore the UUID of the block device in
cleanup().Thanks
PATCH v2 1/1 commands/mkswap: Added new testcase to test mkswap(8).
In reply to Guangwen FengHi,
Quoted 10 lines · Guangwen Feng
Test mkswap(8) command with some basic options.
Signed-off-by Guangwen Feng <[email protected]>--- 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
...
Quoted 8 lines
...
testcases/commands/mkswap/mkswap01.sh new file +7
-0+ +mkswap_verify() +{ + local mkswap_op=$1 + local op_arg=$2 + local device=$3 + local size=$4
testcases/commands/mkswap/mkswap01.sh line 62
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ + 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}
testcases/commands/mkswap/mkswap01.sh line 74
Quoted 12 lines
testcases/commands/mkswap/mkswap01.sh new file +12
-0+ + 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.
Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh new file +8
-0+ local swapfile=$op_arg + else + local swap_op="" + local swapfile=$device + fi + + swapon $swap_op $swapfile 2>/dev/null + if [ $? -ne 0 ]; then
testcases/commands/mkswap/mkswap01.sh line 92
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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
PATCH v2 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
I will modify the test case according to your suggestion, thanks!
Best Regards,
Guangwen Feng
Quoted 75 lines · Alexey Kodanev
Hi,
Quoted 10 lines · Guangwen Feng
Test mkswap(8) command with some basic options.
Signed-off-by Guangwen Feng <[email protected]>--- 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...
Quoted 8 lines
...
testcases/commands/mkswap/mkswap01.sh new file +7
-0+ +mkswap_verify() +{ + local mkswap_op=$1 + local op_arg=$2 + local device=$3 + local size=$4testcases/commands/mkswap/mkswap01.sh line 62
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ + local ret=0 + + local before=`free | grep "Swap" | awk '{print $2}'` + + if [ -z "$size" ]; then + local swapsize=$DEVICE_SIZE + else + local swapsize=$size + fiNo need to check it here if you define single variable, i.e. size=${4:-$DEVICE_SIZE}
testcases/commands/mkswap/mkswap01.sh line 74
Quoted 12 lines
testcases/commands/mkswap/mkswap01.sh new file +12
-0+ + 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.Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh new file +8
-0+ local swapfile=$op_arg + else + local swap_op="" + local swapfile=$device + fi + + swapon $swap_op $swapfile 2>/dev/null + if [ $? -ne 0 ]; thentestcases/commands/mkswap/mkswap01.sh line 92
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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_resmmessage in each case. Other cases should return 1, right? But the test proceeds further.Thanks,
Alexey
.
PATCH v3 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Test mkswap(8) command with some basic options.
--- 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
runtest/commands +1 -0
@@ -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
testcases/commands/mkswap/Makefile new file +22 -0
@@ -0,0 +1,22 @@ +# +# Copyright (c) 2015 Fujitsu Ltd. +# Author:Guangwen Feng <[email protected]> +# +# 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
testcases/commands/mkswap/mkswap01.sh new file +158 -0
158 lines not on this page · raw message
Signature · 1 line
1.8.4.2
PATCH v3 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
testcases/commands/mkswap/mkswap01.sh line 27
Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh new file +9
-0+TCID=mkswap01 +TST_TOTAL=10 +. test.sh + +setup() +{ + tst_require_root + + tst_check_cmds blkid blockdev free mkswap
^
uuidgen is missing here
Quoted 20 lines
testcases/commands/mkswap/mkswap01.sh new file +20
-0+ 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 +} +
testcases/commands/mkswap/mkswap01.sh line 58
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+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 :).
Quoted 4 lines
testcases/commands/mkswap/mkswap01.sh new file +4
-0+ local swapsize=${4:-$DEVICE_SIZE} + + if [ "$mkswap_op" = "-p" ]; then + local pagesize=$op_arg
testcases/commands/mkswap/mkswap01.sh line 73
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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?
testcases/commands/mkswap/mkswap01.sh line 82
Quoted 8 lines
testcases/commands/mkswap/mkswap01.sh new file +8
-0+ 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.
testcases/commands/mkswap/mkswap01.sh line 89
Quoted 7 lines
testcases/commands/mkswap/mkswap01.sh new file +7
-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 $ret
Here as well.
testcases/commands/mkswap/mkswap01.sh line 96
Quoted 7 lines
testcases/commands/mkswap/mkswap01.sh new file +7
-0+ 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.
testcases/commands/mkswap/mkswap01.sh line 98
Quoted 1 line
testcases/commands/mkswap/mkswap01.sh new file +1
-0+ local est=16
You shoud intialize the ret=0 here.
testcases/commands/mkswap/mkswap01.sh line 102
Quoted 4 lines
testcases/commands/mkswap/mkswap01.sh new file +4
-0+ 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.
testcases/commands/mkswap/mkswap01.sh line 109
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+ 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.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v3 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Thanks for your review!
I will modify the test case according to your suggestion.
Best Regards,
Guangwen Feng
Quoted 125 lines · Cyril Hrubis
Hi!
testcases/commands/mkswap/mkswap01.sh line 27
Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh new file +9
-0+TCID=mkswap01 +TST_TOTAL=10 +. test.sh + +setup() +{ + tst_require_root + + tst_check_cmds blkid blockdev free mkswap^
uuidgen is missing hereQuoted 20 lines
testcases/commands/mkswap/mkswap01.sh new file +20
-0+ 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 +} +testcases/commands/mkswap/mkswap01.sh line 58
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+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/meminfoNo need to call free and no need to use grep :).
Quoted 4 lines
testcases/commands/mkswap/mkswap01.sh new file +4
-0+ local swapsize=${4:-$DEVICE_SIZE} + + if [ "$mkswap_op" = "-p" ]; then + local pagesize=$op_argtestcases/commands/mkswap/mkswap01.sh line 73
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ else + local pagesize=$PAGE_SIZE + fi + + if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then + local swapfile=$op_arg + else + mkswap_op= + local swapfile=$device + fiCan 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?testcases/commands/mkswap/mkswap01.sh line 82
Quoted 8 lines
testcases/commands/mkswap/mkswap01.sh new file +8
-0+ 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.testcases/commands/mkswap/mkswap01.sh line 89
Quoted 7 lines
testcases/commands/mkswap/mkswap01.sh new file +7
-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 $retHere as well.
testcases/commands/mkswap/mkswap01.sh line 96
Quoted 7 lines
testcases/commands/mkswap/mkswap01.sh new file +7
-0+ 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.
testcases/commands/mkswap/mkswap01.sh line 98
Quoted 1 line
testcases/commands/mkswap/mkswap01.sh new file +1
-0+ local est=16You shoud intialize the ret=0 here.
testcases/commands/mkswap/mkswap01.sh line 102
Quoted 4 lines
testcases/commands/mkswap/mkswap01.sh new file +4
-0+ if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \ + [ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then + ret=1 + fiWhat 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.
testcases/commands/mkswap/mkswap01.sh line 109
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+ swapoff $mkswap_op $swapfile 2>/dev/null + if [ $? -ne 0 ]; then + tst_resm TINFO "Can not do swapoff on $swapfile." + fi + + return $retOtherwise it looks good.
PATCH v4 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Test mkswap(8) command with some basic options.
--- 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
runtest/commands +1 -0
@@ -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
testcases/commands/mkswap/Makefile new file +22 -0
@@ -0,0 +1,22 @@ +# +# Copyright (c) 2015 Fujitsu Ltd. +# Author:Guangwen Feng <[email protected]> +# +# 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
testcases/commands/mkswap/mkswap01.sh new file +162 -0
162 lines not on this page · raw message
Signature · 1 line
1.8.4.2
PATCH v4 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
testcases/commands/mkswap/mkswap01.sh line 70
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+ 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.
Quoted 26 lines
testcases/commands/mkswap/mkswap01.sh new file +26
-0+ 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)) +
testcases/commands/mkswap/mkswap01.sh line 107
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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.
testcases/commands/mkswap/mkswap01.sh line 111
Quoted 3 lines
testcases/commands/mkswap/mkswap01.sh new file +3
-0+ 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.
testcases/commands/mkswap/mkswap01.sh line 115
Quoted 4 lines
testcases/commands/mkswap/mkswap01.sh new file +4
-0+ fi + + return $ret +}
The rest looks good to me.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v4 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Quoted 13 lines · Cyril Hrubis
Hi!
testcases/commands/mkswap/mkswap01.sh line 70
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+ if [ "$mkswap_op" = "-L" ] || [ "$mkswap_op" = "-U" ]; then + local swapfile=$op_arg + else + mkswap_op= + local swapfile=$TST_DEVICE + fiWhat 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_testfunction. Then we can just pass it inmkswap_testto the verify function so that we do not have to do this ugly if. Ok?
Quoted 4 lines
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
Quoted 55 lines
Quoted 26 lines
testcases/commands/mkswap/mkswap01.sh new file +26
-0+ 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)) +testcases/commands/mkswap/mkswap01.sh line 107
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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 + fiThis looks good now, thanks for the detailed description.
testcases/commands/mkswap/mkswap01.sh line 111
Quoted 3 lines
testcases/commands/mkswap/mkswap01.sh new file +3
-0+ 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.testcases/commands/mkswap/mkswap01.sh line 115
Quoted 4 lines
testcases/commands/mkswap/mkswap01.sh new file +4
-0+ fi + + return $ret +}The rest looks good to me.
PATCH v5 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Test mkswap(8) command with some basic options.
--- 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
runtest/commands +1 -0
@@ -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
testcases/commands/mkswap/Makefile new file +22 -0
@@ -0,0 +1,22 @@ +# +# Copyright (c) 2015 Fujitsu Ltd. +# Author:Guangwen Feng <[email protected]> +# +# 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
testcases/commands/mkswap/mkswap01.sh new file +156 -0
156 lines not on this page · raw message
Signature · 1 line
1.8.4.2
PATCH v5 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Quoted 1 line
+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.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
In reply to Guangwen FengQuoted 5 lines
testcases/commands/mkswap/mkswap01.sh new file +5
-0+setup() +{ + tst_require_root + + tst_check_cmds losetup free mkswap
testcases/commands/mkswap/mkswap01.sh line 37
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ + 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, buttst_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.
Quoted 49 lines
testcases/commands/mkswap/mkswap01.sh new file +49
-0+ + 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
testcases/commands/mkswap/mkswap01.sh line 96
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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 :-)
Quoted 46 lines
testcases/commands/mkswap/mkswap01.sh new file +46
-0+ + 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" +}
testcases/commands/mkswap/mkswap01.sh line 152
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ +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))"
}Quoted 45 lines
testcases/commands/mkswap/mkswap01.sh new file +44
-0+ +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
Signature · 2 lines
Rock Lee [email protected]
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Thanks for review!
Quoted 18 lines · Lei Li
Quoted 5 lines
testcases/commands/mkswap/mkswap01.sh new file +5
-0+setup() +{ + tst_require_root + + tst_check_cmds losetup free mkswaptestcases/commands/mkswap/mkswap01.sh line 37
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ + 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, buttst_acquire_devicemay offer you a block device,/opt/ltp/runltp-b/dev/sdb1for example, then, "path" may be invilidate, and syntax error occurs in the line :
Quoted 4 lines
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.
Quoted 53 lines
Quoted 53 lines
testcases/commands/mkswap/mkswap01.sh new file +53
-0+ + 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}'`
Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh line 96
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+ + local est=16 + if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \ + [ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then + ret=1 + fiHow 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.
Quoted 59 lines
Quoted 46 lines
testcases/commands/mkswap/mkswap01.sh new file +46
-0+ + 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" +}testcases/commands/mkswap/mkswap01.sh line 152
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ +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()
Quoted 8 lines
{ mkswap_test "" "" "$TST_DEVICE" "$((DEVICE_SIZE-10000))" }
test3(){ mkswap_test "-f" "" "$TST_DEVICE" "$((DEVICE_SIZE+10000))" }
Yes, it's more reasonably, thanks.
Quoted 46 lines
Quoted 45 lines
testcases/commands/mkswap/mkswap01.sh new file +44
-0+ +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
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Quoted 93 lines
Hi!
Thanks for review!Quoted 18 lines · Lei Li
Quoted 5 lines
testcases/commands/mkswap/mkswap01.sh new file +5
-0+setup() +{ + tst_require_root + + tst_check_cmds losetup free mkswaptestcases/commands/mkswap/mkswap01.sh line 37
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ + 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, buttst_acquire_devicemay offer you a block device,/opt/ltp/runltp-b/dev/sdb1for example, then, "path" may be invilidate, and syntax error occurs in the line :Quoted 4 lines
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.Quoted 53 lines
Quoted 53 lines
testcases/commands/mkswap/mkswap01.sh new file +53
-0+ + 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}'`Quoted 9 lines
testcases/commands/mkswap/mkswap01.sh line 96
Quoted 6 lines
testcases/commands/mkswap/mkswap01.sh new file +6
-0+ + local est=16 + if [ $((after-before)) -lt $((swapsize-pagesize/1024-est)) ] || \ + [ $((after-before)) -gt $((swapsize-pagesize/1024+est)) ]; then + ret=1 + fiHow do you get this formula? I'm a newbie here :-)
Quoted 2 lines
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.
Signature · 2 lines
Rock Lee [email protected]
PATCH 1/1 commands/mkswap: Added new testcase to test mkswap(8).
Hi!
Quoted 93 lines · Lei Li
Quoted 93 lines
Hi!
Thanks for review!Quoted 18 lines · Lei Li
Quoted 5 lines
testcases/commands/mkswap/mkswap01.sh new file +5
-0+setup() +{ + tst_require_root + + tst_check_cmds losetup free mkswaptestcases/commands/mkswap/mkswap01.sh line 37
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ + 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, buttst_acquire_devicemay offer you a block device,/opt/ltp/runltp-b/dev/sdb1for example, then, "path" may be invilidate, and syntax error occurs in the line :Quoted 4 lines
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.Quoted 62 lines
Quoted 49 lines
testcases/commands/mkswap/mkswap01.sh new file +49
-0+ + 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 $rettestcases/commands/mkswap/mkswap01.sh line 96
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh new file +10
-0+ 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 + fiHow do you get this formula? I'm a newbie here :-)
Quoted 9 lines
Quoted 2 lines
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 exactvalue(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
Quoted 1 line
Thanks.