public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v4] commands/mkswap: Added new testcase to test mkswap(8).
Date: Thu, 19 Nov 2015 14:41:12 +0100	[thread overview]
Message-ID: <20151119134112.GD28969@rei.lan> (raw)
In-Reply-To: <1447837977-11734-1-git-send-email-fenggw-fnst@cn.fujitsu.com>

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

  reply	other threads:[~2015-11-19 13:41 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-05  3:03 [LTP] [PATCH] commands/mkswap: Added new testcase to test mkswap(8) Guangwen Feng
2015-11-05 11:41 ` Alexey Kodanev
2015-11-06  5:17   ` Guangwen Feng
2015-11-06 11:54     ` Alexey Kodanev
2015-11-09  8:02       ` [LTP] [PATCH v2] " Guangwen Feng
2015-11-11  4:50         ` Lei Li
2015-11-12  3:32           ` Guangwen Feng
2015-11-11  9:44         ` Alexey Kodanev
2015-11-12  3:36           ` Guangwen Feng
2015-11-12  8:23             ` [LTP] [PATCH v3] " Guangwen Feng
2015-11-16 16:31               ` Cyril Hrubis
2015-11-17  2:22                 ` Guangwen Feng
2015-11-18  9:12                   ` [LTP] [PATCH v4] " Guangwen Feng
2015-11-19 13:41                     ` Cyril Hrubis [this message]
2015-11-20  5:36                       ` Guangwen Feng
2015-11-20  6:04                         ` [LTP] [PATCH v5] " Guangwen Feng
2015-11-24 16:26                           ` Cyril Hrubis
2015-11-09  2:00 ` [LTP] [PATCH] " Lei Li
2015-11-09  6:54   ` Guangwen Feng
2015-11-11  3:45     ` Lei Li
2015-11-12  3:27       ` Guangwen Feng

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=20151119134112.GD28969@rei.lan \
    --to=chrubis@suse.cz \
    --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