From: Qu Wenruo <quwenruo@cn.fujitsu.com>
To: Dave Chinner <david@fromorbit.com>, Eric Sandeen <sandeen@sandeen.net>
Cc: Eric Sandeen <sandeen@redhat.com>,
linux-btrfs <linux-btrfs@vger.kernel.org>,
xfs@oss.sgi.com
Subject: Re: [PATCH] xfstests: Add pairing mount options test
Date: Mon, 13 Jan 2014 10:26:05 +0800 [thread overview]
Message-ID: <52D34EBD.4020508@cn.fujitsu.com> (raw)
In-Reply-To: <20140113015239.GD3469@dastard>
On mon, 13 Jan 2014 12:52:39 +1100, Dave Chinner wrote:
> On Sun, Jan 12, 2014 at 07:35:44PM -0600, Eric Sandeen wrote:
>> On 1/12/14, 7:21 PM, Qu Wenruo wrote:
>>> On fri, 10 Jan 2014 10:15:37 -0600, Eric Sandeen wrote:
>>>> On 1/8/14, 12:30 AM, Qu Wenruo wrote:
>>>>> Test remount btrfs with different pairing options like barrier and no barrier.
>>>> It seems that while this tests that the remount succeeds, and that
>>>> the option string is present in /proc/mounts, it does not test that
>>>> the mount option is actually in effect.
>>> Yes, this is what the new test case is intended to do.
>>> This case was just a test case tests the mount options themselves
>>> to ensure all the pairing mount options works during remounting,
>>> since most pairing options are missing before.
>>>> I suppose for many of these options that would be hard to test; for
>>>> i.e. acl though it should be trivial.
>>>>
>>>> What do you think, is this enough to ensure that remount handling
>>>> is working as expected for all of these options?
>>> In my opinion, this test should just focuses on the remount handling and
>>> the pairing options.
>>> For the detailed function should be examineed in other test cases.
>> Except those won't test that a remount with those options actually *worked*;
>> in fact they don't do remount at all.
>>
>> In other words, all this does is test that an option flag was set or unset in
>> the superblock, but it doesn't really test whether the option has been
>> properly set up (or torn down) as a result.
>>
>> I won't say no to this, but it seems to be of somewhat limited use.
> What happens to the test when mount options are deprecated/removed?
> How are we going to handle the matrix of testable/untestable mount
> options across kernels with different mount option support?
>
> Cheers,
>
> Dave.
In my opinion,there may be two ways to deal it:
1) Introduce up_limit_kver and down_limit_kver to *every* mount option.
If needed also add deprecated flags.
Ithink this can handle the version related things quite well.
But for the test case, it may be overkilled.
Most codes will be focused on theversion handling even though the case only
tests the flags.
On the other hand, it would be quitenice if xfstests framework can
introduce such
version handling systems.
2) Manual editing after mount options change.
If any options become deprecated/removed, just remove them from
PAIRING_OPTIONS and HIDDEN_OPTIONS.
For the old kernels with does not support these new options, NG is what
it should be.
Since missing pairing options meaning that users can't trigger a funtion
without unmount,
NG inform users the fact.
(Recently, one user asked why there is no "barrier" mountoption in
btrfs. From the respect of users,
missing pairing mount options is a defeat)
This method will introduce more effort tomaintain the test case, but due
to the small codes and
relativly less changes in mount options, I consider it as an acceptable
method.
It would be quite nice if any one can provide any better idea.
Thanks
Qu
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2014-01-13 2:25 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-08 6:30 [PATCH] xfstests: Add pairing mount options test Qu Wenruo
2014-01-10 16:15 ` Eric Sandeen
2014-01-13 1:21 ` Qu Wenruo
2014-01-13 1:35 ` Eric Sandeen
2014-01-13 1:52 ` Dave Chinner
2014-01-13 2:26 ` Qu Wenruo [this message]
2014-01-13 3:26 ` Dave Chinner
2014-01-13 4:00 ` Qu Wenruo
2014-01-13 4:44 ` Eric Sandeen
2014-01-13 21:23 ` Dave Chinner
2014-01-13 1:55 ` Qu Wenruo
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=52D34EBD.4020508@cn.fujitsu.com \
--to=quwenruo@cn.fujitsu.com \
--cc=david@fromorbit.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=sandeen@redhat.com \
--cc=sandeen@sandeen.net \
--cc=xfs@oss.sgi.com \
/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;
as well as URLs for NNTP newsgroup(s).