> As promised, here is a patch for Unionfs to use the new path_* VFS helpers > which now take a struct path *. I've used Al's vfs git tree plus all 12 > patches you've outlined (ten path_* patches + 2 others). I've run Unionfs > through my basic regression suite; I haven't done more extensive testing > yet. This patch can be used in -mm if/when your path_* patches make it > there (this patch will be a standalone one to be used on top of my > unionfs.git tree). Thanks. > Al, I'm not too pleased with the resulting additional code complexity to > Unionfs, due to these new helpers. See the diffstat below. These VFS > helpers have widened the gap between the f/s API and the VFS helpers': the > helpers now need a struct path/vfsmount, but the most ->ops that a > filesystem is being called with, don't have a path/vfsmount passed. I'm not too pleased either ;) What I think would be much cleaner if for example you had functions like this: void unionfs_lower_path_index(struct dentry *dentry, int bindex, struct path *path_out); Etc. If you are dealing with paths, you should not be handling vfsmounts separately. That part is ugly and just invitation for bugs. > You've said before that a filesystems has no business dealing with vfsmounts > (with the exception of follow_link). Fine. But the irony now is that the > new vfs helpers that a filesystems is likely to use, FORCE you to know a lot > more about vfsmounts! If indeed a filesystem shouldn't have to know or deal > with vfsmounts, then none of the VFS helpers should require a struct > vfsmount (or a struct path, or a nameidata, which embed a vfsmount+dentry > pair). Otherwise, let's admit that some filesystems have to know about > vfsmounts and hence let's pass them to all appropriate fs ->ops. How would that help? AFAICS the vfsmounts on the lower layers should have _absolutetly nothing_ to do with the vfsmount on the top layer. So no, passing down vfsmounts to the filesystem is wrong (with the exception of follow_link, and even in that case only very specialized filesystems like proc will ever have a valid use for it). > (BTW, if > that's done, then it'd also be helpful for a struct vfsmount to have a "void > *private" into which a stacked f/s can store "lower objects"). Oh, please! I think you are confused about the role that vfsmounts play in stacking. Vfsmounts place filesystems in the global and private mount namespaces. But each and every instance of that placement should behave exactly the same (modulo such flags as per-mount r/o, etc), and so stacking filesystems, just like any other filesystem, should have no need for the knowledge about where they are placed in the namespace. On the other hand, they _should_ have knowledge of the placement of the filesystems they are operating on. So when accessing the lower layers, they can conform to the same rules as anything else accessing the filesystems. I.e. they shouldn't meddle directly with the filesystem, without going through the namespace layer first. This is already enforced for dentry_open(), and I don't see the big issue with enfocing it for the rest of the vfs_* operations. Miklos -- 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
