lkml.org 
[lkml]   [2017]   [May]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]
    On Wed, May 17, 2017 at 1:31 PM, David Howells <dhowells@redhat.com> wrote:
    > Miklos Szeredi <mszeredi@redhat.com> wrote:
    >
    >> > (b) is internal-only at the moment, used by NFS submounts as triggered by
    >> > automounts. There isn't currently any way to supply mount options to this.
    >>
    >> And all blockdev based fs.
    >
    > I see what you're getting at. In which case there are more cases:
    >
    > (a) new mount, new sb struct with no source (eg. procfs, sysfs, tmpfs)
    > (b) new mount, new sb struct, params loaded from filesystem data (eg. bdev)
    > (c) new mount, new sb struct, params derived from parent (eg. NFS automount)
    > (d) new mount, shared extant sb struct
    > (e) remount
    >
    > In the case of (d) where we're attempting to make another mount for an extant
    > super_block struct and we need to check the consistency of the parameters.

    Yes. Current behavior seems to just ignore given options (except
    MS_RDONLY) in that case, so we need to keep that possibility.

    Also I think it would be good to allow selecting when superblock is created:

    - non-exclusive create: if exists return it, if not create it
    - exclusive create: only create if non-existent
    - non-create: only return if exists

    >
    >> > Ah - but some of these options have to be set *inside* sget() or before the
    >> > superblock becomes live, even the ones that can be changed in-flight.
    >>
    >> That would be the "???" category. Any concrete examples?
    >
    > NFS is a good example. You need parameters that indicate the server to talk
    > to and specify I/O parameters before you even get the superblock as you have
    > to talk to the server first. I think this is particularly true of NFSv2/3
    > where you need to talk to mountd.
    >
    > This would also be true of AFS. There you have to access the network to look
    > up the volume ID before you can call sget() as the volume ID is part of the
    > index key to the set of super_block structs.
    >
    > Further, some of these values (I/O parameters in NFS's case, for example) form
    > part of the super_block struct index key, so you have to set those inside
    > sget()'s set callback.

    So what I propose is:

    1) call ->parse_option()

    would get indication what we are trying to do (find and/or
    create and/or reconfig)

    this step is optional, the the filesystem type could possibly be
    enough for the following steps

    2) call ->get_tree()

    pass sc containing parsed options and flags controlling the
    creation of the superblock (create/exclusive)

    this step is optional, not called if we are given an sb to work
    with (i.e. only reconfig)

    3) call ->reconfig()

    pass sc containing parsed options

    this step is optional, we might be instructed just to find or
    create the sb

    >
    >> >> Also I think silently ignoring options is not always the right answer.
    >> >
    >> > Example?
    >>
    >> mount /dev/sda -oacl /mnt
    >> mount /dev/sda -onoacl /mnt2
    >
    > So you'd like to give an error or a warning if ACLs are not supported, either
    > by the filesystem or the kernel as a whole?

    What I was getting at is that the second mount will ignore the "noacl"
    option. It's not something we apparently care much about (but will
    definitely want to keep as back-compat thing for the mount(2)
    interface). But for the new interface I think we need something less
    crazy. One solution would be the exclusive create, which doesn't have
    this problem. Maybe that's enough; not sure if we need anything more
    sophisticated.

    >> You are thinking on the wrong level. Of course mount(2) needs to
    >> handle MS_NOSUID et al. But it's doing it now, and it isn't parsing
    >> "nosuid", just translating MS_NOSUID to MNT_NOSUID.
    >
    > Ummm... That's done by the parser in this case, so effectively it is.

    Where exactly? You are not touching do_mount(), which is where the
    MS_*** -> MNT_*** translation is done.


    >> For the fsopen() case you won't need to parse "nosuid" because that's a flag
    >> for fsmount().
    >
    > Whilst this is true, that means that the parser has to operate differently in
    > the mount(2) and fsopen(2) cases - which I was trying to avoid.

    I don't get it. We never passed MNT_* options as strings to the
    kernel. That was parsed by mount(8) and translated to MS_* flags.
    So how would mount(2) and fsopen(2) need to operate differently
    regarding parsing MNT_* options, when we want neither to do it?

    >> The only thing fsmount() should take from the sc is the root_dentry.
    >> It should be equivalent to what currently is a bind mount, except it
    >> should be able to fully configure the new mount.
    >
    > It needs to take the device name as well. I wonder if it would be possible to
    > store the device name on the superblock and then leave a path-in-mount in the

    Ah, mnt_devname. The device name as just a special type of option and
    as such should be stored in the superblock.

    > vfsmount struct to fabricate a <source>:/<path> later. Though this would
    > change the behaviour if someone did:
    >
    > mknod /dev/foo b 8 1
    > mknod /dev/bar b 8 1
    > mount /dev/foo /mnt/foo
    > mount /dev/bar /mnt/bar
    >
    > as /proc/mounts would now show /dev/foo for /mnt/bar.

    I'd very much hope this doesn't introduce regressions, but if it did,
    then we'd have to go back to using mnt_devname...

    > Also, I guess the subtype should be wangled in the superblock-getting code
    > (vfs_get_tree() as of patch 21) rather than in do_new_mount_sc(). If I do
    > that, then it may be that do_new_mount_sc() only needs the root dentry pointer
    > and not the sb_config pointer (except for error string passing).

    Would be nice.

    And hopefully error string passing can be made generic and be moved
    out of this set.

    Thanks,
    Miklos

    \
     
     \ /
      Last update: 2017-05-18 10:10    [W:3.267 / U:1.744 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site