On Wed, Aug 14, 2013 at 01:47:30PM -0500, Geoffrey Wehrman wrote:
> On Wed, Aug 14, 2013 at 05:50:42PM +1000, Dave Chinner wrote:
> | On Tue, Aug 13, 2013 at 10:42:32AM -0500, Mark Tinguely wrote:
> | > On 08/12/13 19:50, Dave Chinner wrote:
> | > >On Mon, Aug 12, 2013 at 08:25:23AM -0500, Mark Tinguely wrote:
> | > >>On 08/11/13 19:59, Dave Chinner wrote:
> | > >>>On Tue, Jul 30, 2013 at 02:10:32PM -0500, Mark Tinguely wrote:
> | > >>>>On 07/19/13 01:25, Dave Chinner wrote:
> | > >>>>>From: Dave Chinner<dchinner@xxxxxxxxxx>
> | > >>>>>
> | > >>>>>Add support for the file type field in directory entries so that
> | > >>>>>readdir can return the type of the inode the dirent points to to
> | > >>>>>userspace without first having to read the inode off disk.
> | .....
> | > >>>>>Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx>
> | > >>>>>---
> | > >>>>>
> | > >>>>
> | > >>>>>+static inline int xfs_sb_version_hasftype(struct xfs_sb *sbp)
> | > >>>>>+{
> | > >>>>>+ return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5&&
> | > >>>>>+ xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_FTYPE);
> | > >>>>> }
> | > >>>>>
> | > >>>>
> | > >>>>This feature should support inode version 2 and 3.
> | > >>>
> | > >>>Has nothing to do with the inode version number - it has to do with
> | > >>>the directory structure being modified.
> | > >>>
> | > >>>We're changing the directory structure for CRCs, and this builds on
> | > >>>top of that. It is essentially part of the V3 directory format, and
> | > >>>should be treated as such. Suggesting that we retrofit and support a
> | > >>>modified v2 directory format is close to insane - instead of only
> | > >>>having to test v2 vs v3 directory formats, you're suggesting we
> | > >>>support v2 vs v2+dtype vs v3 vs v3+dtype. We simply do not have the
> | > >>>resources to adequately test and support such an explosion of
> | > >>>filesystem configurations.
> | > >>>
> | > >>>We've had this discussion before - new on-disk features go into the
> | > >>>v5 superblock format - the v4 superblock format from this point
> | > >>>onwards is essentially legacy support from an upstream development
> | > >>>perspective.
> | ....
> | > >>>That said, there's nothing to stop anyone from backporting such a
> | > >>>feature to an older kernel and maintaining it themselves - it's open
> | > >>>source software. But the idea that development should be constrained
> | > >>>by having to support both old and new formats is wrong - the old v4
> | > >>>format should be considered stable and we need think very hard about
> | > >>>changing it at all now, especially as much of the development focus
> | > >>>is now shifting to taking advantage of the additions to the v5
> | > >>>format....
> | > >>
> | > >>I guess we need more time to argue this out. It is not going into
> | > >>Linux 3.12 as a crc feature only.
> | > >
> | > >Seriously?
> | >
> | > yes seriously.
> |
> | Great, another random roadblock from the XFS maintainers to deal
> | with.
>
> The addition of the file type field to directory entries is a great
> new feature. Your implementation of adding a "hidden" byte to the name
> field is especially clever.
Then go thank Christoph for that - I'm pretty sure he came up with
the idea one night when we drinking in a bar somewhere a few years
ago...
> This is a feature that can benefit both
> dir2 and dir3 format filesystems and is completely independent of your
> CRC and self describing metadata feature work. I understand that you
> are not interested in porting the capability to dir2 format filesystems
> yourself and do not have the resources to provide the associated testing
> and support.
Yes, I've been pretty clear about that.
> Myself and others within SGI have discussed these issues,
But not with the community.
So the SGI hivemind decided something behind closed doors, and
you're expecting us to just swallow that decision and not question
it?
> and we are willing to take on the work ourselves rather than have this
> feature go only into v5 superblock filesystems where the feature is only
> accessible to those who are willing to risk using experimental code.
Great. So Ben's going to merge the patches then?
> Given that SGI will be doing the work to support the file type field
> in dir2 format filesystems, it doesn't make sense to add the code to
> v5 filesystems until all of the work is complete as there could be
> additional considerations for the on disk changes.
<groan>
No, you're saying that merging is now dependent on whenever SGI gets
around to doing v4 support work?
So, what's the plan of action here? Who is going to do that work?
How long is it going to take them? Are we talking about a delay of
couple of days, a few weeks, or months?
But let's address the core premise being asserted here by the SGI
hivemind - that we can't add a v5 feature until it's also made for
v4 filesystems. I've said it time and time again: the v5 format
will not be compromised by having to cater for supporting v4
structures. I've been saying this for over 3 years. Indeed, in that
presentation at LCA 18 months ago: I said on slide 36:
" * No attempt to provide backwards or forwards
compatibility for format changes.
* Avoids compromising the new on-disk format design"
This still holds true. This directory format change is designed
specifically for v5 superblocks formats, and the process you are
insisting on is that that we should compromise the format change so
that we can support the same feature on v4 filesystems. This is the
complete antithesis of the design and architectural principles that
have been guiding our work over the past 3 years, and I'm not about
to compromise on it now because it's a matter of *principle*.
This should not come as a surprise to anyone at SGI, but we've been
open and clear about the processes that we have using to design new
features for a quite a long time now. If SGI disagrees with the
processes and principles we strive to acheive, then perhaps we need
to have a different discussion.....
> We also noted that this feature should not be added to the kernel until
> userspace code is available to support this feature. Specifically,
> xfs_repair needs to validate and if needed repair the the file type field.
Sure. I've been waiting for review of the code first so that any
nasty problems could be dealt with without having to double handle
the code changes in userspace. Instead of review, all I've got from
SGI is two weeks of silence and then a "get stuffed until you do v4
support as well" response.
I'll send the patches for repair support on top of that stack
tomorrow, and then this is not longer a barrier to merging it. Will
you merge it then?
> Also, tests are needed to validate the new functionality.
They can be written now that xfs_io readdir support hase been
merged. Though I've already done manual tests with the getdents
example code from the getdents(2) man page so I know it does
actually work correctly...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
