xfs
[Top] [All Lists]

Re: [PATCH v2] xfstests: add support for ext4dev FSTYP

To: "Amir G." <amir73il@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH v2] xfstests: add support for ext4dev FSTYP
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 2 Jun 2011 13:08:02 +1000
Cc: xfs@xxxxxxxxxxx, sandeen@xxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, sergey57@xxxxxxxxx, Amir Goldstein <amir73il@xxxxxxxxxxxx>
In-reply-to: <BANLkTimbPWfOJKq6er4mnSYNPcx6VHLcrw@xxxxxxxxxxxxxx>
References: <1306933012-8666-1-git-send-email-amir73il@xxxxxxxxxxxxxxxxxxxxx> <20110601232804.GL32466@dastard> <BANLkTi=sV5=PyZvNSd=DGNW-V84=27d7Yw@xxxxxxxxxxxxxx> <BANLkTimbPWfOJKq6er4mnSYNPcx6VHLcrw@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Thu, Jun 02, 2011 at 05:33:34AM +0300, Amir G. wrote:
> On Thu, Jun 2, 2011 at 5:16 AM, Amir G. <amir73il@xxxxxxxxxxxxxxxxxxxxx> 
> wrote:
> > On Thu, Jun 2, 2011 at 2:28 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> >> On Wed, Jun 01, 2011 at 03:56:52PM +0300, amir73il@xxxxxxxxxxxxxxxxxxxxx 
> >> wrote:
> >>> From: Amir Goldstein <amir73il@xxxxxxxxxxxx>
> >>>
> >>> From: Amir Goldstein <amir73il@xxxxxxxxxxxx>
> >>>
> >>> blkid knows to identify the ext4dev FSTYP of a partition that was
> >>> formatted with mkfs.ext4dev.
> >>> quota tools and various util-linux utils are also aware of ext4dev,
> >>> so ext4dev shares the same capabilities as ext4.
> >>>
> >>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxxxxx>
> >>> Tested-by: Sergey Ivanov <sergey57@xxxxxxxxx>
> >>> ---
> >>> ext4dev is used to test experimental ext4 code in mutual existance
> >>> with production ext4 code on the same system.
> >>>
> >>> Specifically, ext4 snapshots code is available for testing as a
> >>> stand-alone ext4dev module for Fedora 15 and Ubuntu 11.4
> >>> (see http://next3.sf.net).
> >>>
> >>> v1 -> v2:
> >>> - undo change of fsck -t $FSTYP to fsck.$FSTYP
> >>>
> >>>  common.defrag |    2 +-
> >>>  common.quota  |    4 ++--
> >>>  common.rc     |   10 +++++-----
> >>>  3 files changed, 8 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/common.defrag b/common.defrag
> >>> index 1bcf01d..4850803 100644
> >>> --- a/common.defrag
> >>> +++ b/common.defrag
> >>> @@ -26,7 +26,7 @@ _require_defrag()
> >>>      xfs)
> >>>          DEFRAG_PROG=/usr/sbin/xfs_fsr
> >>>       ;;
> >>> -    ext4)
> >>> +    ext4|ext4dev)
> >>>          DEFRAG_PROG=/usr/bin/e4defrag
> >>>       ;;
> >>>      *)
> >>> diff --git a/common.quota b/common.quota
> >>> index 3c87ce1..b6d5f16 100644
> >>> --- a/common.quota
> >>> +++ b/common.quota
> >>> @@ -29,7 +29,7 @@ _require_quota()
> >>>      [ -n $QUOTA_PROG ] || _notrun "Quota user tools not installed"
> >>>
> >>>      case $FSTYP in
> >>> -    ext2|ext3|ext4|reiserfs)
> >>> +    ext2|ext3|ext4|ext4dev|reiserfs)
> >>>       if [ ! -d /proc/sys/fs/quota ]; then
> >>>           _notrun "Installed kernel does not support quotas"
> >>>       fi
> >>> @@ -237,7 +237,7 @@ _check_quota_usage()
> >>>       # Sync to get delalloc to disk
> >>>       sync
> >>>       VFS_QUOTA=0
> >>> -     if [ $FSTYP = "ext2" -o $FSTYP = "ext3" -o $FSTYP = "ext4" -o 
> >>> $FSTYP = "reiserfs" ]; then
> >>> +     if [ $FSTYP = "ext2" -o $FSTYP = "ext3" -o $FSTYP = "ext4" -o 
> >>> $FSTYP = "ext4dev" -o $FSTYP = "reiserfs" ]; then
> >>>               VFS_QUOTA=1
> >>>               quotaon -f -u -g $SCRATCH_MNT 2>/dev/null
> >>>       fi
> >>
> >> Perhaps this should be changes to a case statement?
> >>
> >
> > you're making me go to v3 in such a trivial patch, but ok, I'll do it ;-)
> >
> 
> I rechecked the fsck -t ext4dev vs. fsck.ext4dev.
> fsck -t ext4dev doesn't work for me :-(
> Sergey has a newer version of  util-linux-ng
> see:
> 
> amir@qalab:~/xfstests$ sudo fsck -t ext4dev -nf /dev/sda5
> fsck from util-linux-ng 2.17.2
> e2fsck 1.41.14 (22-Dec-2010)
> /dev/sda5 has unsupported feature(s): FEATURE_C7 FEATURE_C8 FEATURE_R7
> e2fsck: Get a newer version of e2fsck!
> amir@qalab:~/xfstests$ sudo fsck.ext4dev -nf /dev/sda5
> e2fsck 1.41.14-next3-1.0.13-7 (24-May-2011)
> Checking snapshots: 1,done
> Pass 1: Checking inodes, blocks, and sizes
> Pass 2: Checking directory structure
> Pass 3: Checking directory connectivity
> Pass 4: Checking reference counts
> Pass 5: Checking group summary information
> /dev/sda5: 6596/6561792 files (3.6% non-contiguous), 1242522/26216064 blocks
> amir@qalab:~/xfstests$
> 
> What do you thing, Dave?
> Should xfstests rely on a non-buggy generic fsck util,

For filessytems that use the generic fsck multiplexor, yes.

> or just
> implement it's own
> non-buggy generic fsck (invoke fsck.$FSTYP directly)

In general, no. XFS is a special case in that fsck.xfs is a no-op -
it does no checking at all and only returns values needed for init
scripts to work correctly. xfs_repair/xfs_check are for checking the
filesystem...

> I am running a recent system (Ubuntu 11.4) and I don't thing that upgrading
> util-linux should be a requirement for xfstests to work.

We do not try to support every buggy piece of crap out there - if a
newer version of util-linux has already fixed the problem, then use
that and we don't need to do anything special with xfstests at all.
If you've got bleeding edge filesystem code that requires using a
ext4dev fstyp and a new ext4 userspace, then I think that requiring
you to use a non-buggy util-linux is not a big deal....

---

Personally I think that ext4dev shouldn't be supported at all. A
special fstyp iwhile ext4 was being developed was, IMO, a stupid
thing to do in the first place, and I was happy when it died. It
should not be resurrected and propagated.

xfstests assumes that you are using a userspace that is current with
the version of the filesystem the kernel supports. If you are
running a development/special branch of ext4, then you need to be
running a userspace that understands it completely. If all you are
doing with the ext4dev fstyp is trying to vector to a different fsck
program that supports a new set of feature bits, then IMO you are
doing it all wrong.

Fundamentally, the filesystem is either ext4 or it isn't. If the
features are never going to make it into mainline ext4, then you
need a completely different fstype and full userspace support for
that fstype. Once you have that, you can add the fstype support to
xfstests. However, just using a different fstyp just to set a
certain set of feature flags is, again IMO, a pretty stupid way of
going about this.

Cheers,

Dave.

-- 
Dave Chinner
david@xxxxxxxxxxxxx

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