From: Dave Chinner <david@fromorbit.com>
To: Theodore Ts'o <tytso@mit.edu>
Cc: fstests@vger.kernel.org, hughd@google.com, Junho Ryu <jayr@google.com>
Subject: Re: [PATCH 05/12] xfstests: do not unmount tmpfs during remount
Date: Wed, 10 Feb 2016 17:07:16 +1100 [thread overview]
Message-ID: <20160210060716.GV19486@dastard> (raw)
In-Reply-To: <1455069001-17846-6-git-send-email-tytso@mit.edu>
On Tue, Feb 09, 2016 at 08:49:54PM -0500, Theodore Ts'o wrote:
> From: Junho Ryu <jayr@google.com>
>
> Several tests unmount then re-mount the scratch filesystem, to check
> that the content is unchanged; but unmounting a tmpfs is designed to
> lose its content, which causes such tests to fail unnecessarily.
>
> Signed-off-by: Hugh Dickins <hughd@google.com>
> Signed-off-by: Junho Ryu <jayr@google.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
....
> _scratch_remount()
> {
> - _scratch_unmount
> - _scratch_mount
> + case $FSTYP in
> + tmpfs)
> + OPTS="$@"
> + if test -n "$OPTS"; then
> + OPTS=$(echo $OPTS | sed -e 's/-o /-o remount,/')
> + mount $OPTS $SCRATCH_MNT
> + fi
> + ;;
> + *)
> + _scratch_unmount
> + _scratch_mount "$@"
> + ;;
> + esac
> }
If really don't like the different definitions of "remount" being
used here, especially now that new parameters are being passed in.
i.e.
> --- a/tests/generic/003
> +++ b/tests/generic/003
> @@ -108,8 +108,7 @@ _compare_stat_times NNN "$file1_stat_after_first_access" \
> "$file1_stat_after_second_access" "after accessing file1 second time"
>
> # Remounting with nodiratime option
> -_scratch_unmount
> -_scratch_mount "-o nodiratime"
> +_scratch_remount "-o nodiratime"
This makes me go "no, that can't work, nodiratime is not an option
that we allow on remount."
So, at minimum, the name of the helper needs to get changed so that
it doesn't imply that a "-o remount" with new options is being
done...
> _require_scratch
> -_scratch_mkfs >/dev/null 2>&1
> -
> -_umount_mount()
> -{
> - CWD=`pwd`
> - cd /
> - # pipe error into /dev/null, in case not mounted (after _require_scratch)
> - _scratch_unmount 2>/dev/null
> - _scratch_mount
> - cd "$CWD"
> -}
> -
> -_umount_mount
> +_scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed"
> +_scratch_mount > /dev/null 2>&1 || _fail "mount failed"
Please don't add _fail to mkfs/mount like this, especially where the
test doesn't already have them.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2016-02-10 6:07 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-10 1:49 [PATCH 00/12] xfstests: fix up various tmpfs failures Theodore Ts'o
2016-02-10 1:49 ` [PATCH 01/12] check: avoid error messages of tests/$FS does not exist Theodore Ts'o
2016-02-10 5:45 ` Dave Chinner
2016-02-10 15:34 ` Theodore Ts'o
2016-02-10 16:28 ` Christoph Hellwig
2016-02-10 23:17 ` Theodore Ts'o
2016-02-10 22:19 ` Dave Chinner
2016-02-10 23:32 ` Theodore Ts'o
2016-02-10 1:49 ` [PATCH 02/12] common: _scratch_mkfs_sized() for tmpfs Theodore Ts'o
2016-02-10 6:00 ` Dave Chinner
2016-02-10 15:58 ` Theodore Ts'o
2016-02-10 22:37 ` Dave Chinner
2016-02-10 1:49 ` [PATCH 03/12] generic: use mount point instead of device name Theodore Ts'o
2016-02-10 1:49 ` [PATCH 04/12] generic: add _require_odirect to three more tests Theodore Ts'o
2016-02-10 9:15 ` Eryu Guan
2016-02-10 16:11 ` Theodore Ts'o
2016-02-10 22:51 ` Dave Chinner
2016-02-10 23:21 ` Theodore Ts'o
2016-02-10 1:49 ` [PATCH 05/12] xfstests: do not unmount tmpfs during remount Theodore Ts'o
2016-02-10 6:07 ` Dave Chinner [this message]
2016-02-10 16:07 ` Theodore Ts'o
2016-02-10 18:04 ` Theodore Ts'o
2016-02-10 23:07 ` Dave Chinner
2016-02-10 23:28 ` Theodore Ts'o
2016-02-11 3:07 ` Dave Chinner
2016-02-11 15:25 ` Theodore Ts'o
2016-02-11 17:36 ` Darrick J. Wong
2016-02-10 1:49 ` [PATCH 06/12] generic: do not unmount before calling _check_scratch_fs() Theodore Ts'o
2016-02-10 1:49 ` [PATCH 07/12] generic: require fiemap for generic/009 Theodore Ts'o
2016-02-10 1:49 ` [PATCH 08/12] xfstests: fix generic/312 on tmpfs, ignore /proc/partitions Theodore Ts'o
2016-02-10 5:54 ` Dave Chinner
2016-02-10 23:39 ` Theodore Ts'o
2016-02-11 2:53 ` Dave Chinner
2016-02-10 1:49 ` [PATCH 09/12] xfstests: generic/079 requires chattr, not xattrs Theodore Ts'o
2016-02-10 9:09 ` Eryu Guan
2016-02-10 16:09 ` Theodore Ts'o
2016-02-10 1:49 ` [PATCH 10/12] generic: disable generic/027 for tmpfs Theodore Ts'o
2016-02-10 5:58 ` Dave Chinner
2016-02-10 15:48 ` Theodore Ts'o
2016-02-10 23:13 ` Dave Chinner
2016-02-10 1:50 ` [PATCH 11/12] xfstests: add executable permission to tests Theodore Ts'o
2016-02-10 9:07 ` Eryu Guan
2016-02-10 1:50 ` [PATCH 12/12] xfstests: increase tmpfs memory size Theodore Ts'o
2016-02-10 2:10 ` [PATCH 00/12] xfstests: fix up various tmpfs failures Theodore Ts'o
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=20160210060716.GV19486@dastard \
--to=david@fromorbit.com \
--cc=fstests@vger.kernel.org \
--cc=hughd@google.com \
--cc=jayr@google.com \
--cc=tytso@mit.edu \
/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