xfs
[Top] [All Lists]

Re: [PATCH 1/6] xfstest: add fio git submodule

To: Dmitry Monakhov <dmonakhov@xxxxxxxxxx>
Subject: Re: [PATCH 1/6] xfstest: add fio git submodule
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 24 Sep 2012 23:53:34 +1000
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, linux-ext4@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, hch@xxxxxx
In-reply-to: <87txun1tqv.fsf@xxxxxxxxxx>
References: <a> <1348428276-13161-1-git-send-email-dmonakhov@xxxxxxxxxx> <505FD0A9.3090601@xxxxxxxxxx> <87lifzd9fl.fsf@xxxxxxxxxx> <20120924113718.GK20960@dastard> <87txun1tqv.fsf@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Sep 24, 2012 at 04:38:00PM +0400, Dmitry Monakhov wrote:
> On Mon, 24 Sep 2012 21:37:18 +1000, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> > On Mon, Sep 24, 2012 at 02:03:42PM +0400, Dmitry Monakhov wrote:
> > > On Sun, 23 Sep 2012 22:16:57 -0500, Eric Sandeen <sandeen@xxxxxxxxxx> 
> > > wrote:
> > > > On 9/23/12 2:24 PM, Dmitry Monakhov wrote:
> > > > > FIO is very flexible io generator, i would call it IO swiss knife.
> > > > > Currently we have tonnes of hardcoded application which reproduces
> > > > > some predefined scenario. This approach has obvious dissadvantages
> > > > > 1) Lack of flexability: once written it is hard to modify it in future
> > > > > 2) Code base is large, many routines written again and again
> > > > > 
> > > > > At the same time add new fio based tast is just add simle INI file.
> > > > > This greatly simplify code review. I do beleve that some day we will
> > > > > replace most of hardcoded io binaries with fio.
> > > > 
> > > > The submodule approach is interesting, but I wonder - we have quite a 
> > > > few
> > > > dependencies on other binaries already; what are the pros and cons of 
> > > > creating
> > > > this as a git submodule vs. simply expecting fio to be installed on the
> > > > system like any of the other dependencies we have today?
> > > Pro:
> > >  P1) allow to specify exact commit as a submodule HEAD this guarantee
> > >      that we will have known version and functionality regardless to
> > >      distribution package manager (which are known to be very 
> > > conservative)
> > 
> > You haven't provided a method to do this in this patch. All
> > you've provided is a submodule that tracks the fio tree head.
> > All this needs to be properly documented in the README file, at
> > minimum.
> > 
> > And conservative is good, too. I don't want tests to fail because of
> > rapid changes in the fio tree causing regressions in fio itself. The
> > tools that xfstests depends on need to be stable and relatively
> > unchanging, because we're not testing them - we're testing the
> > filesystem. The less the environemnt changes around the things we're
> > actually supposed to be regression testing, the better.
> Yes, but we do not have to advance submodule update unless we need it.
> Project may goes forward but we still can use old commit if needed.

Sure, but that them means we need to track fio closely enough and
commit changes to the upstream xfstests repository whenever someone
needs to move it forward. It's a centralised solution that doesn't
improve the workflow of significant users of xfstests.

Indeed, what happens if we take this and run it on an old distro or
platform that a current fio hasn't even been tested on (e.g. RHEL
5.x, SLES10.x, MIPSEL or SH)? i.e. what happens when the blessed
xfstests fio version doesn't even compile on the test target? It
gets messy in a hurry because the xfstests maintainers have to solve
that problem.

I *much* prefer to have external dependencies handled the same way
for all external tools and libraries: if the distro doesn't supply
it, then the user needs to download it, install it and get it
working themselves. If they don't install it or the installed
version is too old, then the tests get skipped. That moves the
burden of dealing with fio integration issues to the end user, not
onto the xfstests maintainers. End users are scalable, maintainers
are not.

> > >  P2) Prevent duplicating of source code (fsstress.c/aio-stress.c and
> > >      etc).  If some one want to add new feature to submodule he
> > >      simply push it to official submodule repo and move submodule HEAD
> > >      In that both parties(submodule maintainer and project maintainer)
> > >      will benefit because new features will be available to every
> > >      submodule user
> > > Cons:
> > >  C1) New dependencies
> > >  C2) Learn people how to work with submodules
> > > 
> > > I'll not assume (C2) as a serious argument because this is just one more
> > > git's command. For most users should just add new option to clone:
> > > git clone --recursive git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
> > 
> > Doesn't work for me. I keep local mirrors of all git trees that I
> > use regularly and update them by cron jobs so that I don't have to
> > go to the internet for every local tree that I clone or update.
> > 
> > That's particularly important for me because I'm a *long* way from
> > the US or Europe and cloning from scratch over the internet takes a
> > long time and suck up a lot of bandwidth. I don't even allow my test
> > machines to access the internet - they only know about the local
> > network and mirrors. I'd have to overide the fio submodule URL in
> > the xfstests repository on every test machine, and that gets messy
> > in a hurry.
> > 
> > Also, we distribute xfstests as a tarball, and there has been talk of
> > proper packaging (rpm/deb) as well. In those cases, the git
> > submodule approach does not work as we have to depend on the distro
> > supplied fio packages...
> Yes, if this is mandatory. it makes packaging harder but not too
> complex.

IMO, the submodule approach is an all-or-nothing approach that is
difficult to opt-out of or work around. Making it harder to maintain
a working test environment for a significant percentage of the
xfstests userbase is not a win, IMO.

> > > (C1) Is not big deal in case of Fio because we already depends from
> > > libaio.
> > 
> > There's also a fio version dependency. i.e. _require_fio has to
> > detect whether the currently installed fio is of sufficiently recent
> > version for the tests to run.
> > 
> > > (P2) Makes xfstest coverage larger because all new tests which use new 
> > >      submodules functionality will start to work by default (today it
> > >       silently ignored). As i already told fio is under rapid
> > >      development Jens Axboe does very good job so (P2) really works for
> > >      me, new features i need for xfstets was reviewed and accepted by Jens
> > > http://git.kernel.dk/?p=fio.git;a=commit;h=8b28bd41375930664a0ff9ff9b101a88ac416ac5
> > > http://git.kernel.dk/?p=fio.git;a=commit;h=9c25d2e3f498707c4fd5a4bb0adf9867ecb17768
> > > http://git.kernel.dk/?p=fio.git;a=commit;h=e615ceafbe3962a35b7a7e06a0c8f4e2c0652c65
> > 
> > For me, that's not a "pro" - that's a "con" as i explained above.
> > 
> > > > (I package fio for Fedora, is it not commonly available on other
> > > > distros?)
> > 
> > Available for Debian, which means all it's derivatives also have it.
> > 
> > In short, I'd prefer we continue to use package level dependencies
> > detected through configure/_require_foo infrastructure than using
> > source tree level dependencies. Package level dependencies are much,
> > much easier to manage for most people and don't require everyone to
> > have internet access on the machines that xfstests is being built
> > on....
> Ok i'll go back to _require_fio $VER approach, but it is reasonable to
> add prep script which will fetch or install all necessary packages
> so user can explicitly run it if internet is available.

I don't think that a "fetch and build this tool" script is really
something that is part of xfstests. Having the configure scripts
warn that the required version of fio was not found and giving a
pointer to the repository is consistent with the way we curently
handle missing external build dependencies. Yes, I dislike autoconf
at times, too, but I think it's a better solution to external
dependencies at the source level for xfstests than git
submodules.....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>