Re: [PATCH 51/50] xfs: add xfs sb v4 support for dirent filetype field

[Ben Myers <bpm@sgi.com>]

Hi Dave,

On Wed, Aug 21, 2013 at 10:06:24AM +1000, Dave Chinner wrote:
> On Tue, Aug 20, 2013 at 09:29:17AM -0500, Mark Tinguely wrote:
> > I repeat, if you have technical concerns for the feature's
> > implementation and its impact on v4 filesystems because it uses
> > common directory code, then it should be held back for more testing.
> 
> I missed this comment. Mark, I'm really concerned that SGI is taking
> the stance that the dtype code is fully working unless otherwise
> proven to have problems.  That is a dangerous approach to take for
> new code and new on-disk formats - it should be considered with
> suspicion and paranoia until enough testing has been done to negate
> those concerns.
> 
> The reason I only proposed this for v5 superblocks is to enable
> wider testing and get us to the point where we are not concerned
> anymore about it before we say it is ready for production
> deployment.
> 
> I have technical concerns that arise once the feature bit it
> enabled, not when it is disabled. Those technical concerns center
> around off-by-one and alignment issues as a result of increasing the
> dirent size when the feature bit is enabled - they pack differently
> into the directory structure and hence will exercise allocation,
> freespace and logging differently.
> 
> See my previous comments about how hard the directory code is to
> test and validate - that's why I want to enable in V5 first so we
> can shake out problems over a wider (but still constrained) user
> base that understand that EXPERIMENTAL means that they might still
> be corruption bugs lurking.

I understand the sentiment that it would be nice to get this into v5 for some
early initial testing.  However, we agreed to take in the crc work as
experimental on the condition that it does not regress v4 superblocks, and with
the knowledge that it might take awhile to be completed.  It's still unfinished
and that's ok.  We knew that was coming.  But this was an agreement made for
one feature only.  We did not agree that the v5 superblock would become a
dumping ground for unrelated and incomplete features to get early testing.

> Again, as I've said all along - enabling the feature on v4
> filesystems is not a technical problem - it's a process and support
> problem. If I thought that this code was ready for widespread
> production deployment then I would have no hesitation to add v4
> support, but it's simply not at that stage yet. We need wider test
> and deployment coverage to get the new feature to that stage.
>
> Which leads to the "then it should be held back for more testing"
> comment. We've discussed this before - almost a year ago now - when
> SGI stated that they wouldn't accept any new code in the xfsdev tree
> unless it was proven to be regression free. That was unacceptable
> then and to apply it to the v5 dirent code is no different.
> 
> We need wider testing of these changes before it is production
> ready, and so holding it back until it's proven to be OK for
> production deployment in v4 filesystems is placing us in a catch-22
> and as such is a similarly an unacceptable outcome.

If this needs more testing I'm all for it.  We should make it a Kconfig option
marked as experimental in it's own right, finish the userspace work, and then
set about pulling it in.  Marking the feature bit as experimental in mkfs with
a warning also seems like an good idea to me...  And if you're that concerned
about it then I'd really like to see them both.  But marking it experimental
doesn't magically mean that we'll pull in another incomplete feature.  My
impression is we're likely to go to -rc7, so I think chances are good this work
can be finished in time for 3.12.

Regards,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs