Re: [RFC][PATCH 11/11] xfs: use common file type conversion

[Dave Chinner <david@fromorbit.com>]

On Tue, Dec 20, 2016 at 07:20:22AM +0200, Amir Goldstein wrote:
> On Tue, Dec 20, 2016 at 2:28 AM, Darrick J. Wong
> <darrick.wong@oracle.com> wrote:
> > On Mon, Dec 19, 2016 at 10:11:08PM +0200, Amir Goldstein wrote:
> > Note: We did this all this work a short time ago so that ext4 and XFS
> > could present the same FSGETXATTR ioctl to user programs (major benefit)
> > but it was kind of a pain to get right.  I don't see an upside to ceding
> > control of this part of the disk format to the VFS.
> >
> 
> The answer "this is not wanted for XFS" is perfectly valid :)
> The common implementation can be used by the small fs, which never
> want anymore than the basic ext2 implementation.
> 
> However, since the DT_ values and  XFS_DIR3_FT_ values of 0..7
> are already carved in stone, as long as comments in both common code
> and libxfs code makes that clear, I see no harm in using the common
> implementation in xfs, besides the need to yank more code from fs.h to
> libxfs, but it's your decision.

It's a philoophical and architectural issue. We currently have a
distinct separation between generic functionality and per-filesystem
specific definitions. The on-disk definitions are owned by the
filesystem not the generic code, and the generic code /never/ sees
what the filesystem stores on disk. THe VFS is supposed to be
completely independent of what the filesystems store on disk - it's
an abstract concept - so that it can morph into whatever is required
to support the functionality the different filesystem provides.

The way we normally handle this is a method callout of some kind
into the filesystem to do the filesystem specific function, and
if there are multiple filesystems that do the same thing, they use a
common function. So that part of the patchset - providing common
helpers to inode mode/filesytem d_type conversion - is fine.

The part that isn't fine, IMO, is defining the filesystem d_type
values in generic code. They should be defined by the filesystem and
passed to the generic conversion functions as a constant. It may
require a different structure to do this cleanly (i.e. something
other than a sparse array keyed on S_IFMT), but I think that having
the VFS define on-disk formats like this is a slippery slope that
only leads to long term pain and ossification.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com