On 8/14/13 1:47 PM, 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. 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. Myself and others within SGI have discussed these issues,
> 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.
> 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.
>
> 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.
> Also, tests are needed to validate the new functionality. While I
> expect that you will provide this support for v5 superblock and dir3
> filesystems, one of us at SGI will extend the support to v4 superblock
> and dir2 filesystems.
These requirements are very, very late in the process. Dave's work has
been discussed for a long time in public, with plenty of time for input
and cooperation and coordination.
If the type field is critical to SGI on V4 superblocks, I'd have expected
to see patches from SGI by now, either prior to, or in coordination with,
Dave's work. So it's hard for me to tell if this is a strategic requirement
for you, or an effort to delay the CRC work which you seem to be uneasy with.
Indeed, you floated this as a requirement many weeks (months?) ago, but as far
as I know, no actual work or patches or proposals to implement it have been
forthcoming from SGI.
I apologize for not being up to speed on the technical details of what it
might take, but I figure surely SGI is on top of it, if you're signing up
to do the work.
So I'd propose (and the guys in the trenches can bang this around) that if
you are serious about this, you commit to producing patches which address your
stated requirements without negatively affecting Dave's V5 design, with all
necessary retesting, any of Dave's outstanding patches rebased as necessary,
and everything ready for upstream integration on the original schedule.
You've had plenty of time to do this, but it's not been done AFAICT.
I think you need to get busy and back up your words with patches pronto, or drop
the above stated requirement; perhaps there is a way to add the support to V4
superblocks retroactively if you miss this window.
There may well be technical hurdles I'm not aware of, but I think we need to see
SGI's proposed implementation to be able to discuss that efficiently.
-Eric
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
