On Fri, Sep 21, 2018 at 04:07:55PM +0100, David Howells wrote: > Miklos Szeredi <mszeredi@xxxxxxxxxx> wrote: > > > What happens if we introduce new flags for fsmount(2) and are already out > > of flags for mount(2)? I see a big mess that way. > > > > So let's instead start a clean new set, to be used in the new API. If I may chime in. I think this is a really good idea. > > If we must. But let's not call them just M_* please. Let's call them > MOUNT_ATTR_* or something. > > > The MS_RELATIME flag was accepted but ignored. Simply leave this out of > > the new set, since "relatime" is the default. > > Can we make RELATIME, STRICTATIME and NOATIME an enum rather than individual > flags? > > #define MOUNT_ATTR_RDONLY 0x01 > #define MOUNT_ATTR_NOSUID 0x02 > #define MOUNT_ATTR_NODEV 0x04 > #define MOUNT_ATTR_NOEXEC 0x08 > #define MOUNT_ATTR_RELATIME 0x00 > #define MOUNT_ATTR_NOATIME 0x10 > #define MOUNT_ATTR_STRICTATIME 0x20 > #define MOUNT_ATTR_ATIME_MASK 0x30 > #define MOUNT_ATTR_NODIRATIME 0x40 > > We can also use these for a mount_setattr() syscall: So a - maybe stupid - question I had when recently working on a patch was how to add new mount properties in the new api. What I would expect is that once the new mount API has landed the old mount() syscall should not see any new features added since I take it that we want userspace to very slowly over time have sufficient incentive to only use the new mount API. So from reading the patch I got the impression that superblock mount options passed via fsconfig() are passed as strings like "ro" and are translated into approriate objects (e.g. flags etc.) by the kernel. That seems like a future proof mechanism to extend mount options for userspace without having to worry about exceeding any integer types in the future. Maybe this would make sense for non-superblock options as well? I can see that this is less performant that checking flags and string parsing is a thing that is not particularly nice but it would be one option to solve the running out of flags problem. > > mount_setattr(int dfd, const char *path, unsigned int atflags, > unsigned int attr_values, > unsigned int attr_mask); If we go with the flag arguments wouldn't it make sense to use a larger integer type? > > where atflags can potentially include AT_RECURSIVE. Very much in favor of having this operate recursively! Christian
