On Mon, May 22, 2017 at 04:50:56PM +0100, David Howells wrote:
>
> Here are a set of patches to create a superblock configuration context
> prior to setting up a new mount, populating it with the parsed
> options/binary data, creating the superblock and then effecting the mount.
>
> This allows namespaces and other information to be conveyed through the
> mount procedure. It also allows extra error information to be returned
> (so many things can go wrong during a mount that a small integer isn't
> really sufficient to convey the issue).
>
> This also allows Miklós Szeredi's idea of doing:
>
> fd = fsopen("nfs");
> write(fd, "option=val", ...);
> fsmount(fd, "/mnt");
>
> that he presented at LSF-2017 to be implemented (see the relevant patches
> in the series), to which I can add:
>
> read(fd, error_buffer, ...);
>
> to read back any error message. I didn't use netlink as that would make it
> depend on CONFIG_NET and would introduce network namespacing issues.
Random notes:
* "sb_config" looks rather odd in the current variant; mount_context,
perhaps? Or fs_context, for that matter... Anyway, that's trivial.
* if NFS folks want to play with EXPORT_SYMBOL_GPL, fine, but any
EXPORT_SYMBOL_GPL in vfs proper is a mistake. If it's an interface that
makes sense, just export it; if it's a vewwwy, vewwwy pwiwate interface
for some specific module - let's figure out how to deal with that layering
violation rather than exporting it at all.
* what the hell is ms_flags thing doing in __vfs_new_sb_config()?
It's a really vile mix of unrelated flags and operations we had in existing
mount(2) ABI. With MS_KERNMOUNT thrown into that loo^Wmix. Sure, we need
to parse the garbage fed to mount(2). And we need to pass that garbage to
"legacy" types as well, but let's not inflict it upon the new mechanisms.
* what's wrong with simple_pin_fs() as it is? You keep
vfs_kern_mount() anyway, so...
* vfs_new_sb_config(): please, move dealing with name into the caller.
Then you would be able to use it more than once.
* submount side of that thing: do we ever want a type different from
that of src_sb, and how the fuck would methods know what to do with it?
* remounts: where (if anywhere) do you call ->validate() for those,
and if you do not, WTF is this
+ if (cfg->sc.purpose == SB_CONFIG_FOR_REMOUNT)
+ return 0;
for? You know, the only place that ever looks at ->purpose...
* docs need to be brought in sync with code - 'purpose' is called 'mount_type'
in those, which is especially unpleasant since you do introduce a field called just
that - NFS-only and in NFS-private part.
* you don't need to register filesystem to use kern_mount()
* locking inode in fsmount(2). What for?
* ->sb_mountpoint(). YALinuxSadoMasochismHook. Not called on normal
mount(2) pathway. Yuck...
* could you split whitespace parts off? Minor, but...
* I'd like to see ipc/mqueue.c dealt with as well; feels like procfs
counterpart might have too much open-coded. This would show what might be
folded into saner helpers...
