linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: "Amir G." <amir73il@users.sourceforge.net>
Cc: "Ted Ts'o" <tytso@mit.edu>, Andreas Dilger <adilger@dilger.ca>,
	Dave Chinner <david@fromorbit.com>,
	xfs@oss.sgi.com, linux-ext4@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, sergey57@gmail.com,
	Amir Goldstein <amir73il@users.sf.net>
Subject: Re: [PATCH v2] xfstests: add support for ext4dev FSTYP
Date: Fri, 03 Jun 2011 00:06:09 -0500	[thread overview]
Message-ID: <4DE86BC1.4080008@redhat.com> (raw)
In-Reply-To: <BANLkTikiE1v6r7sYKSsR_pgqM-dJzQKrzQ@mail.gmail.com>

On 6/2/11 11:59 PM, Amir G. wrote:
> On Fri, Jun 3, 2011 at 3:36 AM, Ted Ts'o <tytso@mit.edu> wrote:
>> On Thu, Jun 02, 2011 at 11:22:53AM -0600, Andreas Dilger wrote:
>>> On 2011-06-02, at 8:59 AM, Eric Sandeen wrote:
>>>> I don't really mind adding ext4dev to FSTYP case statements, it
>>>> -is- something which blkid could, in theory, still return, and
>>>> making xfstests cope with that and try to invoke fsck -t ext4dev
>>>> doesn't bother me too much.  It is sadly an fs type embedded into
>>>> a few tools.
>>>
>>> I'm perfectly OK with using ext4dev as a filesystem type that allows testing
>>> changes to ext4 on a system that is already running ext4 as the root fs.
>>
>> My take on this is that way too much time has been spent this subject.
> 
> No doubt.
> 
>> Being able to use ext4dev is useful, and given that we have all of
>> this support in our existing system tools, why not use it to make ext4
>> development more efficient/easy?  As a bonus you can build the ext4dev
>> as a module, and that means you the compile/edit/debug cycle can be
>> much faster since you can avoid doing a reboot, for those
>> circumstances where using KVM is not possible/convenient.  Personally,
>> I normally use KVM these days, but I can imagine situations where
>> using ext4dev would be a better way to go.  For example, I'd probably
>> use KVM on my laptop, but for testing on production servers in a data
>> center, I'd probably use ext4dev, for a variety of local deployment
>> considerations that's not worth going into here.
>>
>> That being said, whether or not we modify xfstests seems to be a moot
>> point.  In order for me to do my bigalloc development, I've been
>> patching common.rc so that "/sbin/mkfs.$FSTYP" --> "mkfs.$FSTYP" and
>> "/sbin/fsck -t $FSTYP" --> "fsck.$FSTYP".  It's a 3 line change.  Not
>> a big deal.  I've been making this change using /bin/ed after
>> installing xfstests.  So if the XFS folks want to veto this change ---
>> who cares?  It's not hard to make the change locally in order to make
>> xfstests.
>>
>> On the other hand, given that xfstests is using "mkfs.$FSTYP", I don't
>> see why it's so important that it clings to "fsck -t $FSTYP" instead
>> of using "fsck.$FSTYP".  There's no real benefit to calling the fsck
>> driver; it's just an extra fork and exec, and xfstests is being
>> inconsistent by insisting on the use of the fsck driver, but not using
>> the mkfs driver.
>>
>> But that being said, hacking xfstests is not hard, and if Dave and/or
>> Eric feels strongly about resisting this change, it's not worth a lot
>> of time, one way or another....
>>
>>                                                - Ted
>>
> 
> I blame only myself for not presenting the case correctly.
> I made it sound like I am trying to push my own private hack upstream.
> Actually, all 10 people involved in snapshot development clone my xfstests
> tree from github, so we have no real need for the upstream change.
> The reason I was pushing upstream is because I found this feature
> so useful, I thought other developers may enjoy it as well.
> 
> Anyone on on this thread not having used ext4dev by next LSF
> can come to me to claim his beer ;-)

mmm I like beer, I'll see you then!  ;)

-Eric (tucking this email away for future reference... ;)

> Amir.


  reply	other threads:[~2011-06-03  5:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-01 12:56 [PATCH v2] xfstests: add support for ext4dev FSTYP amir73il
2011-06-01 23:28 ` Dave Chinner
2011-06-02  2:16   ` Amir G.
2011-06-02  2:33     ` Amir G.
2011-06-02  3:08       ` Dave Chinner
2011-06-02  3:49         ` Amir G.
2011-06-02  6:40           ` Dave Chinner
2011-06-02  7:11             ` Amir G.
2011-06-02 12:10               ` Lukas Czerner
2011-06-02 13:17                 ` Amir G.
2011-06-02 14:44                   ` Lukas Czerner
2011-06-02  7:16       ` Amir G.
2011-06-02 14:59         ` Eric Sandeen
2011-06-02 17:22           ` Andreas Dilger
2011-06-03  0:36             ` Ted Ts'o
2011-06-03  3:26               ` Eric Sandeen
2011-06-03  4:59               ` Amir G.
2011-06-03  5:06                 ` Eric Sandeen [this message]
2011-06-03 17:21                   ` Amir G.
2011-06-03  2:01           ` 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=4DE86BC1.4080008@redhat.com \
    --to=sandeen@redhat.com \
    --cc=adilger@dilger.ca \
    --cc=amir73il@users.sf.net \
    --cc=amir73il@users.sourceforge.net \
    --cc=david@fromorbit.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=sergey57@gmail.com \
    --cc=tytso@mit.edu \
    --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).