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: Thu, 11 Feb 2016 14:07:51 +1100 [thread overview]
Message-ID: <20160211030751.GE19486@dastard> (raw)
In-Reply-To: <20160210232826.GK26922@thunk.org>
On Wed, Feb 10, 2016 at 06:28:26PM -0500, Theodore Ts'o wrote:
> On Thu, Feb 11, 2016 at 10:07:00AM +1100, Dave Chinner wrote:
> > No, it's not really the options that are the problem here. The
> > problem is -o remount vs unmount/mount and what the test is actually
> > expecting.
> >
> > I'd say "_scratch_remount" should do "-o remount" unconditionally
> > (least surprise) and the current _scratch_remount should be changed
> > to something like _scratch_cycle_mount(). That way both can take
> > options, but it's clear they do different things. tmpfs can simply
> > implement them the same way.
>
> Well, I can do that, but it's going to be a huge patch --- the vast
> majority of the calls to _scratch_remount in the tree (over 100) would
> have to be changed to _scratch_cycle_mount, because they are just
> doing a _scratch_umount / _scratch_mount without taking any arguments
> to change the mount option.
Yup, but we do this sort of tree-wide cleanup fairly often if it
makes sense. In this case, it's just an initial patch taht
does sed -i -e 's/_scratch_remount/_scratch_cycle_mount/' ....
And, let's put things in context: changing 108 lines of code is a
pretty damn small patch in the greater scheme of things. Indeed,
it's smaller than most patches that add a new regression test.
A "huge" patch is something like the series Darrick posted earlier
in the week - something like 20 patches, including somewhere in the
order of 30 new tests, a couple of new binary test programs, a heap
of cleanups across all 80-90 existing reflink/dedupe tests, and a
bunch of bug fixes to go with them.
IOWs, s/_scratch_remount/_scratch_cycle_mount/ is the sort of
no-brainer change that takes less time to write, test and review
than it did for me to write this email....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
next prev parent reply other threads:[~2016-02-11 3: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
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 [this message]
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=20160211030751.GE19486@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.