commands/mkswap01: Update wait_for_file function
PATCH 1/1 commands/mkswap01: Update wait_for_file function
From: Shuang Qiu <[email protected]>
In commit a76b72ad31fa7bb22a09f323dadd5db7c00c7f56,it depends on the files under /dev/disk/by-* in wait_for_file function.But sometimes udev does not refresh automatically during runtime and the symbolic links will not appear. Update the function to use blkid instead.
--- testcases/commands/mkswap/mkswap01.sh | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
testcases/commands/mkswap/mkswap01.sh +13 -12
@@ -46,25 +46,26 @@ cleanup() tst_rmdir } -wait_for_file() +wait_for_device() { - local path="$1" + local token="$1" local retries=10 - if [ -z "$path" ]; then + if [ -z "$token" ]; then return fi while [ $retries -gt 0 ]; do - if [ -e "$path" ]; then + blkid -t "$token" $TST_DEVICE >/dev/null + if [ $? -eq 0 ]; then return fi - tst_resm TINFO "Waiting for $path to appear" + tst_resm TINFO "Waiting for device $token prepared" retries=$((retries - 1)) tst_sleep 10ms done - tst_resm TWARN "The file $path haven't appeared" + tst_resm TWARN "The device $token haven't prepared" } mkswap_verify()
@@ -72,7 +73,7 @@ mkswap_verify() local mkswap_op="$1" local op_arg="$2" local swapfile="$3" - local dev_file="$5" + local token="$5" local before=`awk '/SwapTotal/ {print $2}' /proc/meminfo`
@@ -84,7 +85,7 @@ mkswap_verify() local pagesize=$PAGE_SIZE fi - wait_for_file "$dev_file" + wait_for_device "$token" swapon $swapfile 2>/dev/null
@@ -138,7 +139,7 @@ mkswap_test() local op_arg="$2" local device="$3" local size="$4" - local dev_file="$5" + local token="$5" local mkswap_cmd="mkswap $mkswap_op $op_arg $TST_DEVICE $size"
@@ -156,7 +157,7 @@ mkswap_test() fi if [ -n "$device" ]; then - mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size" "$dev_file" + mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size" "$token" if [ $? -ne 0 ]; then tst_resm TFAIL "'${mkswap_cmd}' failed, not expected." return
@@ -173,9 +174,9 @@ 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" "ltp_testswap" "-L ltp_testswap" "" "/dev/disk/by-label/ltp_testswap" +mkswap_test "-L" "ltp_testswap" "-L ltp_testswap" "" "LABEL=ltp_testswap" mkswap_test "-v1" "" "$TST_DEVICE" -mkswap_test "-U" "$UUID" "-U $UUID" "" "/dev/disk/by-uuid/$UUID" +mkswap_test "-U" "$UUID" "-U $UUID" "" "UUID=$UUID" mkswap_test "-V" mkswap_test "-h"
Signature · 1 line
1.7.9.5
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Hi!
Quoted 3 lines
In commit a76b72ad31fa7bb22a09f323dadd5db7c00c7f56,it depends on the files under
/dev/disk/by-*inwait_for_filefunction.But sometimes udev does not refresh automatically during runtime and the symbolic links will not appear.
Isn't that udev bug?
Quoted 1 line
Update the function to use blkid instead.
Does actually the swapon in the verify function work? Since as far as I can tell it actually opens the symlink in question in order to get the swap device.
Have you tried stracing it in case that the code waiting for the device timeouted?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Hi Cyril,
Thanks for review.
Quoted 5 lines · Cyril Hrubis
Hi!
Quoted 3 lines
In commit a76b72ad31fa7bb22a09f323dadd5db7c00c7f56,it depends on the files under
/dev/disk/by-*inwait_for_filefunction.But sometimes udev does not refresh automatically during runtime and the symbolic links will not appear.Isn't that udev bug?
It does not look like a bug,restart udev(or run udevtrigger) or partprobe will create the symbolic link.
Quoted 8 lines
Quoted 2 lines
Update the function to use blkid instead.
Does actually the swapon in the verify function work? Since as far as I can tell it actually opens the symlink in question in order to get the swap device.
Have you tried stracing it in case that the code waiting for the device timeouted?
I tried it with UEK.Sometimes waiting for device timeout but swapon works,i.e.:
...
...
mkswap01 6 TINFO : Waiting for /dev/disk/by-label/ltp_testswap to appear mkswap01 6 TINFO : Waiting for /dev/disk/by-label/ltp_testswap to appear mkswap01 6 TINFO : Waiting for /dev/disk/by-label/ltp_testswap to appear mkswap01 6 TWARN : The file /dev/disk/by-label/ltp_testswap haven't appeared mkswap01 6 TPASS : 'mkswap -L ltp_testswap /dev/sda7 ' passed.
...
But we can find the label or UUID for device in blkid. Per my testing with blkid,swapon will not fail after checking blkid.
Thanks
Shuang
Quoted 1 line
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Hi!
Quoted 12 lines
Quoted 6 lines
Does actually the swapon in the verify function work? Since as far as I can tell it actually opens the symlink in question in order to get the swap device.
Have you tried stracing it in case that the code waiting for the device timeouted?
I tried it with UEK.Sometimes waiting for device timeout but swapon works,i.e.:
...
...
Quoted 6 lines
mkswap01 6 TINFO : Waiting for
/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TWARN : The file/dev/disk/by-label/ltp_testswaphaven't appeared mkswap01 6 TPASS : 'mkswap -Lltp_testswap/dev/sda7' passed.
...
That looks strange to me. Since if I strace 'swapon -L ltp_testswap' I got:
...stat("/dev/disk/by-label/ltp_testswap", 0x7ffef8293800) = -1 ENOENT (No such file or directory)
...
Then it proceeds to exit with error. So I'm geniuely confused.
Hmm but looking at blkid man page, it says that udev symlinks may depend on settings in /etc/blkid.conf. That may be the difference after all.
Looking at the manual page doing 'blkid -L ltp_testswap' and 'blkid -U $UUID' should be the only correct solution. Does that work for you?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Quoted 25 lines · Cyril Hrubis
Hi!
Quoted 12 lines
Quoted 6 lines
Does actually the swapon in the verify function work? Since as far as I can tell it actually opens the symlink in question in order to get the swap device.
Have you tried stracing it in case that the code waiting for the device timeouted?
I tried it with UEK.Sometimes waiting for device timeout but swapon works,i.e.:
...
...Quoted 6 lines
mkswap01 6 TINFO : Waiting for
/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TWARN : The file/dev/disk/by-label/ltp_testswaphaven't appeared mkswap01 6 TPASS : 'mkswap -Lltp_testswap/dev/sda7' passed.
...That looks strange to me. Since if I strace 'swapon -L
ltp_testswap' I got:...
stat("/dev/disk/by-label/ltp_testswap", 0x7ffef8293800)= -1 ENOENT (No such file or directory)
...
Quoted 7 lines
Then it proceeds to exit with error. So I'm geniuely confused.
Hmm but looking at blkid man page, it says that udev symlinks may depend on settings in
/etc/blkid.conf. That may be the difference after all.Looking at the manual page doing 'blkid -L
ltp_testswap' and 'blkid -U $UUID' should be the only correct solution. Does that work for you?
Yes.
But it does not support blkid -L/-U with lower version of blkid util(such like in RHEL5.11).
So I use "-t" option to search the label and UUID instead in the patch.
Thanks
Shuang
Quoted 1 line
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Hi!
Quoted 17 lines
Quoted 17 lines
Quoted 6 lines
mkswap01 6 TINFO : Waiting for
/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TWARN : The file/dev/disk/by-label/ltp_testswaphaven't appeared mkswap01 6 TPASS : 'mkswap -Lltp_testswap/dev/sda7' passed.
...That looks strange to me. Since if I strace 'swapon -L
ltp_testswap' I got:...
stat("/dev/disk/by-label/ltp_testswap", 0x7ffef8293800)= -1 ENOENT (No such file or directory)
...Then it proceeds to exit with error. So I'm geniuely confused.
Hmm but looking at blkid man page, it says that udev symlinks may depend on settings in
/etc/blkid.conf. That may be the difference after all.
Quoted 6 lines
Quoted 2 lines
Looking at the manual page doing 'blkid -L
ltp_testswap' and 'blkid -U $UUID' should be the only correct solution. Does that work for you?Yes.
But it does not support blkid -L/-U with lower version of blkid util(such like in RHEL5.11).
So I use "-t" option to search the label and UUID instead in the patch.
Looking around the backward compatible way should be:
blkid -l -t LABEL=foo
blkid -l -t UUID=foo
Does that work for you?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Quoted 24 lines · Cyril Hrubis
Hi!
Quoted 17 lines
Quoted 17 lines
Quoted 6 lines
mkswap01 6 TINFO : Waiting for
/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TWARN : The file/dev/disk/by-label/ltp_testswaphaven't appeared mkswap01 6 TPASS : 'mkswap -Lltp_testswap/dev/sda7' passed.
...That looks strange to me. Since if I strace 'swapon -L
ltp_testswap' I got:...
stat("/dev/disk/by-label/ltp_testswap", 0x7ffef8293800)= -1 ENOENT (No such file or directory)
...Then it proceeds to exit with error. So I'm geniuely confused.
Hmm but looking at blkid man page, it says that udev symlinks may depend on settings in
/etc/blkid.conf. That may be the difference after all.Quoted 6 lines
Quoted 2 lines
Looking at the manual page doing 'blkid -L
ltp_testswap' and 'blkid -U $UUID' should be the only correct solution. Does that work for you?Yes.
But it does not support blkid -L/-U with lower version of blkid util(such like in RHEL5.11).
So I use "-t" option to search the label and UUID instead in the patch.
Quoted 6 lines
Looking around the backward compatible way should be:
blkid -l -t LABEL=foo
blkid -l -t UUID=fooDoes that work for you?
According to the man page,-l option will look up only one device which matches -t NAME=value if there are multiple devices matched.And it will print all devices without -l option.
So I though just -t option is enough in this case.
But blkid -l -t also works.
Thanks
Shuang
Quoted 1 line
PATCH 1/1 commands/mkswap01: Update wait_for_file function
In reply to Cyril HrubisQuoted 20 lines · Cyril Hrubis
Hi!
Quoted 12 lines
Quoted 6 lines
Does actually the swapon in the verify function work? Since as far as I can tell it actually opens the symlink in question in order to get the swap device.
Have you tried stracing it in case that the code waiting for the device timeouted?
I tried it with UEK.Sometimes waiting for device timeout but swapon works,i.e.:
...
...Quoted 6 lines
mkswap01 6 TINFO : Waiting for
/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TWARN : The file/dev/disk/by-label/ltp_testswaphaven't appeared mkswap01 6 TPASS : 'mkswap -Lltp_testswap/dev/sda7' passed.
...
Quoted 7 lines
That looks strange to me. Since if I strace 'swapon -L
ltp_testswap' I got:...
stat("/dev/disk/by-label/ltp_testswap", 0x7ffef8293800)= -1 ENOENT (No such file or directory)
...Then it proceeds to exit with error. So I'm geniuely confused.
Hmm, in my case swapon -L test does not fail, if there is no /dev/disk/by-label/test:
root@skholman-m7 stas]# rm /dev/disk/by-label/test
rm: remove symbolic link `/dev/disk/by-label/test'? y [root@skholman-m7 stas]# BLKID_FILE=/tmp/none strace swapon -L test &> /tmp/o; echo $?
0
[root@skholman-m7 stas]# grep by-label /tmp/o stat64("/dev/disk/by-label/test", 0x7feff9b6130) = -1 ENOENT (No such file or directory)
[root@skholman-m7 stas]#
(but in the strace output I see that swapon tries to read some sectors from all available block devices)
Maybe the version of libblkid plays a role here.
Using 'blkid -t' simply makes wait_for_file/wait_for_device not fail if there is no /dev/disk/by-label/ symlink. I suppose 'blkid -t' could be substituted with /bin/true in my case.
Quoted 7 lines
Hmm but looking at blkid man page, it says that udev symlinks may depend on settings in
/etc/blkid.conf. That may be the difference after all.Looking at the manual page doing 'blkid -L
ltp_testswap' and 'blkid -U $UUID' should be the only correct solution. Does that work for you?
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Quoted 47 lines · Stanislav Kholmanskikh
Quoted 24 lines · Cyril Hrubis
Hi!
Quoted 12 lines
Quoted 6 lines
Does actually the swapon in the verify function work? Since as far as I can tell it actually opens the symlink in question in order to get the swap device.
Have you tried stracing it in case that the code waiting for the device timeouted?
I tried it with UEK.Sometimes waiting for device timeout but swapon works,i.e.:
...
...Quoted 7 lines
mkswap01 6 TINFO : Waiting for
/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TINFO : Waiting for/dev/disk/by-label/ltp_testswapto appear mkswap01 6 TWARN : The file/dev/disk/by-label/ltp_testswaphaven't appeared
mkswap01 6 TPASS : 'mkswap -Lltp_testswap/dev/sda7' passed.
...That looks strange to me. Since if I strace 'swapon -L
ltp_testswap' I got:Quoted 6 lines
...
stat("/dev/disk/by-label/ltp_testswap", 0x7ffef8293800)= -1 ENOENT (No such file or directory)
...Then it proceeds to exit with error. So I'm geniuely confused.
Hmm, in my case swapon -L test does not fail, if there is no
/dev/disk/by-label/test:root@skholman-m7 stas]# rm
/dev/disk/by-label/test
rm: remove symbolic link `/dev/disk/by-label/test'? y [root@skholman-m7 stas]# BLKID_FILE=/tmp/none strace swapon -L test &>/tmp/o; echo $?
0
[root@skholman-m7 stas]# grep by-label/tmp/ostat64("/dev/disk/by-label/test", 0x7feff9b6130)= -1 ENOENT (No such file or directory)
[root@skholman-m7 stas]#
Quoted 8 lines
(but in the strace output I see that swapon tries to read some sectors from all available block devices)
Maybe the version of libblkid plays a role here.
Using 'blkid -t' simply makes wait_for_file/wait_for_device not fail if there is no
/dev/disk/by-label/symlink. I suppose 'blkid -t' could be substituted with/bin/truein my case.
My system doesn't provide /etc/blkid.conf
Quoted 9 lines
Quoted 8 lines
Hmm but looking at blkid man page, it says that udev symlinks may depend on settings in
/etc/blkid.conf. That may be the difference after all.Looking at the manual page doing 'blkid -L
ltp_testswap' and 'blkid -U $UUID' should be the only correct solution. Does that work for you?
PATCH 1/1 commands/mkswap01: Update wait_for_file function
In reply to Stanislav KholmanskikhHi!
Quoted 13 lines
Hmm, in my case swapon -L test does not fail, if there is no
/dev/disk/by-label/test:root@skholman-m7 stas]# rm
/dev/disk/by-label/test
rm: remove symbolic link `/dev/disk/by-label/test'? y [root@skholman-m7 stas]# BLKID_FILE=/tmp/none strace swapon -L test &>/tmp/o; echo $?
0
[root@skholman-m7 stas]# grep by-label/tmp/ostat64("/dev/disk/by-label/test", 0x7feff9b6130)= -1 ENOENT (No such file or directory)
[root@skholman-m7 stas]#
Quoted 8 lines
(but in the strace output I see that swapon tries to read some sectors from all available block devices)
Maybe the version of libblkid plays a role here.
Using 'blkid -t' simply makes wait_for_file/wait_for_device not fail if there is no
/dev/disk/by-label/symlink. I suppose 'blkid -t' could be substituted with/bin/truein my case.
Unfortunately for me the swapon just looks for the symlink (util-linux-2.19.1) while blkid -t also reads first 512 bytes of each block device. So I guess that using blkid will not help in this case.
Then what about just changing the 'tst_resm TWARN "The file $path haven't appeared"' from TWARN to TINFO?
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Quoted 23 lines · Cyril Hrubis
Hi!
Quoted 13 lines
Hmm, in my case swapon -L test does not fail, if there is no
/dev/disk/by-label/test:root@skholman-m7 stas]# rm
/dev/disk/by-label/test
rm: remove symbolic link `/dev/disk/by-label/test'? y [root@skholman-m7 stas]# BLKID_FILE=/tmp/none strace swapon -L test &>/tmp/o; echo $?
0
[root@skholman-m7 stas]# grep by-label/tmp/ostat64("/dev/disk/by-label/test", 0x7feff9b6130)= -1 ENOENT (No such file or directory)
[root@skholman-m7 stas]#Quoted 8 lines
(but in the strace output I see that swapon tries to read some sectors from all available block devices)
Maybe the version of libblkid plays a role here.
Using 'blkid -t' simply makes wait_for_file/wait_for_device not fail if there is no
/dev/disk/by-label/symlink. I suppose 'blkid -t' could be substituted with/bin/truein my case.
Quoted 6 lines
Unfortunately for me the swapon just looks for the symlink (util-linux-2.19.1) while blkid -t also reads first 512 bytes of each block device. So I guess that using blkid will not help in this case.
Then what about just changing the '
tst_resmTWARN "The file $path haven't appeared"' from TWARN to TINFO?
Well, no objections then. Even if there is no symlink created in time we call swapon, swapon will not fail on our systems, but wait_for_file will still provide extra 10*10ms for the symlink to be created on your systems.
Shuang ?
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Quoted 33 lines · Stanislav Kholmanskikh
Quoted 23 lines · Cyril Hrubis
Hi!
Quoted 13 lines
Hmm, in my case swapon -L test does not fail, if there is no
/dev/disk/by-label/test:root@skholman-m7 stas]# rm
/dev/disk/by-label/test
rm: remove symbolic link `/dev/disk/by-label/test'? y [root@skholman-m7 stas]# BLKID_FILE=/tmp/none strace swapon -L test &>/tmp/o; echo $?
0
[root@skholman-m7 stas]# grep by-label/tmp/ostat64("/dev/disk/by-label/test", 0x7feff9b6130)= -1 ENOENT (No such file or directory)
[root@skholman-m7 stas]#Quoted 8 lines
(but in the strace output I see that swapon tries to read some sectors from all available block devices)
Maybe the version of libblkid plays a role here.
Using 'blkid -t' simply makes wait_for_file/wait_for_device not fail if there is no
/dev/disk/by-label/symlink. I suppose 'blkid -t' could be substituted with/bin/truein my case.Quoted 6 lines
Unfortunately for me the swapon just looks for the symlink (util-linux-2.19.1) while blkid -t also reads first 512 bytes of each block device. So I guess that using blkid will not help in this case.
Then what about just changing the '
tst_resmTWARN "The file $path haven't appeared"' from TWARN to TINFO?
Quoted 6 lines
Well, no objections then. Even if there is no symlink created in time we call swapon, swapon will not fail on our systems, but
wait_for_filewill still provide extra 10*10ms for the symlink to be created on your systems.Shuang ?
It is ok for me too.
I did not find any other issue if change TWARN to TINFO.
Thanks
Shuang
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Hi!
Fix pushed, thanks.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH 1/1 commands/mkswap01: Update wait_for_file function
In reply to [email protected]Hi!
Sorry for my spam in [1], [2]. My brain barely functioned on yesterday's evening. Now I understand that calling blkid is the right choice to check whether a block device exists. :-[
Both blkid, mkswap, swapon use libblkd (and take into account settings in blkid.conf), so if blkid finds the device, then mkswap, swapon will find it as well.
Quoted 6 lines · On 01/29/2016 06:56 PM, [email protected] wrote:
From: Shuang Qiu <[email protected]>
In commit a76b72ad31fa7bb22a09f323dadd5db7c00c7f56,it depends on the files under
/dev/disk/by-*inwait_for_filefunction.But sometimes udev does not refresh automatically during runtime and the symbolic links will not appear. Update the function to use blkid instead.
checkpatch.pl warns about the style of the commit reference.
Maybe add here that blkid (since it uses libblkid as mkswap, swapon does) is sufficient to check the availability of block devices, whereas explicit checking of /dev/disk/by-* should be avoided where possible (man 8 blkid) ?
Plus one more comment below.
Quoted 30 lines
Signed-off-by Shuang Qiu <[email protected]>--- testcases/commands/mkswap/mkswap01.sh | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)testcases/commands/mkswap/mkswap01.sh +5
-4@@ -46,25 +46,26 @@ cleanup() tst_rmdir } -wait_for_file() +wait_for_device() { - local path="$1" + local token="$1" local retries=10 - if [ -z "$path" ]; then + if [ -z "$token" ]; then return fi while [ $retries -gt 0 ]; do - if [ -e "$path" ]; then + blkid -t "$token" $TST_DEVICE >/dev/null + if [ $? -eq 0 ]; then
testcases/commands/mkswap/mkswap01.sh cleanup() · line 68
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh +2
-2return fi - tst_resm TINFO "Waiting for $path to appear" + tst_resm TINFO "Waiting for device $token prepared" retries=$((retries - 1)) tst_sleep 10ms done - tst_resm TWARN "The file $path haven't appeared" + tst_resm TWARN "The device $token haven't prepared"
Since we rely on blkid for device availability checking, maybe change TWARN to TBROK here?
Quoted 52 lines
testcases/commands/mkswap/mkswap01.sh +6
-6} mkswap_verify()@@ -72,7 +73,7 @@ mkswap_verify() local mkswap_op="$1" local op_arg="$2" local swapfile="$3" - local dev_file="$5" + local token="$5" local before=`awk '/SwapTotal/ {print $2}' /proc/meminfo`@@ -84,7 +85,7 @@ mkswap_verify() local pagesize=$PAGE_SIZE fi - wait_for_file "$dev_file" + wait_for_device "$token" swapon $swapfile 2>/dev/null@@ -138,7 +139,7 @@ mkswap_test() local op_arg="$2" local device="$3" local size="$4" - local dev_file="$5" + local token="$5" local mkswap_cmd="mkswap $mkswap_op $op_arg $TST_DEVICE $size"@@ -156,7 +157,7 @@ mkswap_test() fi if [ -n "$device" ]; then - mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size" "$dev_file" + mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size" "$token" if [ $? -ne 0 ]; then tst_resm TFAIL "'${mkswap_cmd}' failed, not expected." return@@ -173,9 +174,9 @@ 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" "ltp_testswap" "-L ltp_testswap" "" "/dev/disk/by-label/ltp_testswap" +mkswap_test "-L" "ltp_testswap" "-L ltp_testswap" "" "LABEL=ltp_testswap" mkswap_test "-v1" "" "$TST_DEVICE" -mkswap_test "-U" "$UUID" "-U $UUID" "" "/dev/disk/by-uuid/$UUID" +mkswap_test "-U" "$UUID" "-U $UUID" "" "UUID=$UUID" mkswap_test "-V" mkswap_test "-h"
PATCH 1/1 commands/mkswap01: Update wait_for_file function
Hi Stanislav,
Thanks for review.
I agree your comments and will submit another patch.
Thanks
Shuang
Quoted 133 lines · Stanislav Kholmanskikh
Hi!
Sorry for my spam in [1], [2]. My brain barely functioned on yesterday's evening. Now I understand that calling blkid is the right choice to check whether a block device exists. :-[
Both blkid, mkswap, swapon use libblkd (and take into account settings in blkid.conf), so if blkid finds the device, then mkswap, swapon will find it as well.
Quoted 9 lines · On 01/29/2016 06:56 PM, [email protected] wrote:
From: Shuang Qiu <[email protected]>
In commit a76b72ad31fa7bb22a09f323dadd5db7c00c7f56,it depends on the files
under/dev/disk/by-*inwait_for_filefunction.But sometimes udev does not
refresh automatically during runtime and the symbolic links will not appear.
Update the function to use blkid instead.checkpatch.pl warns about the style of the commit reference.
Maybe add here that blkid (since it uses libblkid as mkswap, swapon does) is sufficient to check the availability of block devices, whereas explicit checking of
/dev/disk/by-*should be avoided where possible (man 8 blkid) ?Plus one more comment below.
Quoted 31 lines
Signed-off-by Shuang Qiu <[email protected]>--- testcases/commands/mkswap/mkswap01.sh | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)testcases/commands/mkswap/mkswap01.sh +5
-4@@ -46,25 +46,26 @@ cleanup() tst_rmdir } -wait_for_file() +wait_for_device() { - local path="$1" + local token="$1" local retries=10 - if [ -z "$path" ]; then + if [ -z "$token" ]; then return fi while [ $retries -gt 0 ]; do - if [ -e "$path" ]; then + blkid -t "$token" $TST_DEVICE >/dev/null + if [ $? -eq 0 ]; thentestcases/commands/mkswap/mkswap01.sh cleanup() · line 68
Quoted 10 lines
testcases/commands/mkswap/mkswap01.sh +2
-2return fi - tst_resm TINFO "Waiting for $path to appear" + tst_resm TINFO "Waiting for device $token prepared" retries=$((retries - 1)) tst_sleep 10ms done - tst_resm TWARN "The file $path haven't appeared" + tst_resm TWARN "The device $token haven't prepared"Since we rely on blkid for device availability checking, maybe change TWARN to TBROK here?
Quoted 56 lines
testcases/commands/mkswap/mkswap01.sh +3
-4} mkswap_verify()@@ -72,7 +73,7 @@ mkswap_verify() local mkswap_op="$1" local op_arg="$2" local swapfile="$3" - local dev_file="$5" + local token="$5" local before=`awk '/SwapTotal/ {print $2}' /proc/meminfo`@@ -84,7 +85,7 @@ mkswap_verify() local pagesize=$PAGE_SIZE fi - wait_for_file "$dev_file" + wait_for_device "$token" swapon $swapfile 2>/dev/null@@ -138,7 +139,7 @@ mkswap_test() local op_arg="$2" local device="$3" local size="$4" - local dev_file="$5" + local token="$5" local mkswap_cmd="mkswap $mkswap_op $op_arg $TST_DEVICE $size"@@ -156,7 +157,7 @@ mkswap_test() fi if [ -n "$device" ]; then - mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size""$dev_file"
testcases/commands/mkswap/mkswap01.sh +1
-0+ mkswap_verify "$mkswap_op" "$op_arg" "$device" "$size" "$token" if [ $? -ne 0 ]; then tst_resm TFAIL "'${mkswap_cmd}' failed, not expected." return@@ -173,9 +174,9 @@ 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" "ltp_testswap" "-Lltp_testswap" "" "/dev/disk/by-label/ltp_testswap"
+mkswap_test "-L" "ltp_testswap" "-Lltp_testswap" "" "LABEL=ltp_testswap"
mkswap_test"-v1" "" "$TST_DEVICE"testcases/commands/mkswap/mkswap01.sh +1
-1-mkswap_test "-U" "$UUID" "-U $UUID" "" "/dev/disk/by-uuid/$UUID" +mkswap_test "-U" "$UUID" "-U $UUID" "" "UUID=$UUID" mkswap_test "-V" mkswap_test "-h"