public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] commands/mkswap01: Update wait_for_file function
Date: Fri, 26 Feb 2016 16:42:49 +0300	[thread overview]
Message-ID: <56D05659.4060205@oracle.com> (raw)
In-Reply-To: <1454082985-3549-1-git-send-email-shuang.qiu@oracle.com>

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.


On 01/29/2016 06:56 PM, shuang.qiu@oracle.com wrote:
> From: Shuang Qiu <shuang.qiu@oracle.com>
>
> 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.

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.

>
> Signed-off-by: Shuang Qiu <shuang.qiu@oracle.com>
> ---
>   testcases/commands/mkswap/mkswap01.sh |   25 +++++++++++++------------
>   1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/testcases/commands/mkswap/mkswap01.sh b/testcases/commands/mkswap/mkswap01.sh
> index ae4c98a..fdfc712 100755
> --- a/testcases/commands/mkswap/mkswap01.sh
> +++ b/testcases/commands/mkswap/mkswap01.sh
> @@ -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"

Since we rely on blkid for device availability checking, maybe change 
TWARN to TBROK here?


>   }
>
>   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"
>
>

  parent reply	other threads:[~2016-02-26 13:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-29 15:56 [LTP] [PATCH] commands/mkswap01: Update wait_for_file function shuang.qiu
2016-02-02 14:35 ` Cyril Hrubis
2016-02-03  1:28   ` Shuang Qiu
2016-02-04 13:17     ` Cyril Hrubis
2016-02-05  5:41       ` Shuang Qiu
2016-02-08 15:34         ` Cyril Hrubis
2016-02-17  8:30           ` Shuang Qiu
2016-02-25 16:51       ` Stanislav Kholmanskikh
2016-02-25 17:22         ` Stanislav Kholmanskikh
2016-03-02 13:42         ` Cyril Hrubis
2016-03-03  8:59           ` Stanislav Kholmanskikh
2016-03-03  9:43             ` Shuang Qiu
2016-03-03 15:55               ` Cyril Hrubis
2016-02-26 13:42 ` Stanislav Kholmanskikh [this message]
2016-03-02  6:20   ` Shuang Qiu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56D05659.4060205@oracle.com \
    --to=stanislav.kholmanskikh@oracle.com \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox