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@xxxxxxxxxx> 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@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
