FS/XFS testing framework
 help / color / mirror / Atom feed
From: "Lukáš Czerner" <lczerner@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: fstests@vger.kernel.org, fdmanana@gmail.com
Subject: Re: [PATCH 2/2] generic/017: Do not create file systems with different block sizes
Date: Tue, 24 Jun 2014 06:50:15 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LFD.2.00.1406240643160.2556@localhost.localdomain> (raw)
In-Reply-To: <20140624043835.GF9508@dastard>

On Tue, 24 Jun 2014, Dave Chinner wrote:

> Date: Tue, 24 Jun 2014 14:38:35 +1000
> From: Dave Chinner <david@fromorbit.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: fstests@vger.kernel.org, fdmanana@gmail.com
> Subject: Re: [PATCH 2/2] generic/017: Do not create file systems with
>     different block sizes
> 
> On Mon, Jun 23, 2014 at 04:54:15PM +0200, Lukas Czerner wrote:
> > User takes care about specifying mkfs options he wishes to test and the
> > test itself should not change it if it's not strictly necessary for the
> > test itself.
> > 
> > In this case it is not necessary and we should only test configuration
> > provided by the user. Moreover if the block size was already specified
> > some mkfs utilities does not handle multiple of the same parameters and
> > the mkfs utility fails making it re-try with only provided options
> > (ignoring what user specified), which is wrong.
> 
> I disagree strongly with this justification. The test is perfectly
> fine: it is allowed to do whatever it wants with mkfs and mount
> options.
> 
> It is up to the implementations of the specific FSTYP mkfs
> implementation called from _scratch_mkfs to handle
> conflicting/unsupported options sanely, not the test....

I agree, the test should not care about those options at all. That's
why I removed it. What was done in this test was entirely
unnecessary and was not done in any other similar test before.

Yes, we can test all block size in every test from now on, but
what's the point ? We have a config file and the user can set
whatever block size he wants to test. I do not dispute that there is
a time when we really want to set a specific mkfs options in the
test, but this is not the case at all.

Moreover, if I want to test specific mkfs configuration the current
approach might not work as it will replace the options.

-Lukas

> 
> > In this case it's also a problem for btrfs file system which does not
> > support block size < page size.
> 
> i.e. _scratch_mkfs_btrfs needs to either filter that out or
> _fail/_not_run the test if it can't....
> 
> Cheers,
> 
> Dave.
> 

  reply	other threads:[~2014-06-24  4:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-23 14:54 [PATCH 1/2] xfstests: Introduce get_block_size() helper Lukas Czerner
2014-06-23 14:54 ` [PATCH 2/2] generic/017: Do not create file systems with different block sizes Lukas Czerner
2014-06-23 18:12   ` Filipe David Manana
2014-06-24  4:38   ` Dave Chinner
2014-06-24  4:50     ` Lukáš Czerner [this message]
2014-06-24  5:48       ` Dave Chinner
2014-06-24  6:17         ` Lukáš Czerner
2014-06-24  6:45           ` Dave Chinner
2014-06-24  9:08             ` Lukáš Czerner
2014-06-24  8:40     ` Filipe David Manana
2014-06-23 18:12 ` [PATCH 1/2] xfstests: Introduce get_block_size() helper Filipe David Manana
2014-06-24  4:28 ` Dave Chinner

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=alpine.LFD.2.00.1406240643160.2556@localhost.localdomain \
    --to=lczerner@redhat.com \
    --cc=david@fromorbit.com \
    --cc=fdmanana@gmail.com \
    --cc=fstests@vger.kernel.org \
    /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