On Wed, Mar 06, 2024 at 10:35:02AM -0600, Eric Sandeen wrote: > On 3/6/24 6:17 AM, Christian Brauner wrote: > > On Wed, Mar 06, 2024 at 01:13:05PM +0100, Miklos Szeredi wrote: > >> On Wed, 6 Mar 2024 at 11:57, Christian Brauner <brauner@xxxxxxxxxx> wrote: > >> > >>> There's a tiny wrinkle though. We currently have no way of letting > >>> userspace know whether a filesystem supports the new mount API or not > >>> (see that mount option probing systemd does we recently discussed). So > >>> if say mount(8) remounts debugfs with mount options that were ignored in > >>> the old mount api that are now rejected in the new mount api users now > >>> see failures they didn't see before. > > Oh, right - the problem is the new mount API rejects unknown options > internally, right? > > >>> For the user it's completely intransparent why that failure happens. For > >>> them nothing changed from util-linux's perspective. So really, we should > >>> probably continue to ignore old mount options for backward compatibility. > >> > >> The reject behavior could be made conditional on e.g. an fsopen() flag. > > > > and fspick() which I think is more relevant. > > > >> > >> I.e. FSOPEN_REJECT_UNKNOWN would make unknown options be always > >> rejected. Without this flag fsconfig(2) would behave identically > >> before/after the conversion. > > > > Yeah, that would work. That would only make sense if we make all > > filesystems reject unknown mount options by default when they're > > switched to the new mount api imho. When we recognize the request comes > > from the old mount api fc->oldapi we continue ignoring as we did before. > > If it comes from the new mount api we reject unless > > FSOPEN/FSPICK_REJECT_UKNOWN was specified. I actually did misparse that I now realize. I read that as "ignore unknown" instead of "reject unknown". > > Ok, good point. Just thinking out loud, I guess an fsopen/fspick flag does > make more sense than i.e. each filesystem deciding whether it should reject > unknown options in its ->init_fs_context(), for consistency? Yes, I think so. The interesting case for util-linux according to Karel was remounting where mount(8) wants to gather all options from fstab and mountinfo, add new options from the command line and send it to the kernel without having to care about filesystems specific options that cannot be changed on remount. However, other users that do use the api programatically do care about this. They want to get an error when changing a mount property doesn't work. I think doing this on a per-fs basis just leads to more inconsistency. I'd rather have this be something we enforce on a higher level if we do it at all. > > Right now it looks like the majority of filesystems do reject unknown > options internally, already. Yeah, it's mostly pseudo fses that don't, I reckon. > > (To muddy the waters more, other inconsistencies I've thought about are > re: how the fileystem handles remount. For example, which options are > remountable and which are not, and should non-remountable options fail? Yes, they should but similar to fsopen() we should have an fspick() flag. This was what I mentioned earlier in my response to Miklos. But I'm not yet clear whether FSOPEN/FSPICK_IGNORE_UNKNOWN wouldn't make more sense than FSOPEN/FSPICK_REJECT_UNKNOWN. IOW, invert the logic. Because as you said most filesystems do already reject unknown mount options and it's a few that don't. So I think we should focus on the remount case and for that I think we want FSOPEN_IGNORE_UNKNOWN otherwise default to rejecting unknown options if coming from the new mount api? I'm not sure. > Also whether the filesystem internally preserves the original set of > options and applies the new set as a delta, or whether it treats the > new set as the exact set of options requested post-remount, but that's > probably a topic for another day.) For vfs mount properties it's a delta in the new mount api. The old mount api didn't have a concept of add or subtract. If you had a read-only mount and you wanted to also make it noexec then you'd have to specify "ro" again otherwise the mount would be made rw. mount(8) hides that behavior by retrieving the current mountflags from mountinfo and adding it to the remount call if the old mount api is used. mount_setattr() does that directly in the kernel and always does a delta. For filesystem specific properties it's probably irrelevant because remount already is effectively a delta for most filesystems. IOW, you don't suddenly get "usrquota" unset because you've changed the "dax" property on your xfs filesystem which would be worrisome. :) The thing is though that changing one mount option might implicitly change other mount options. But that's something that only the filesystem should decide. So I don't think this is something we need to worry about? The way I see mount(8) currently doing it is to change: (1) filesystem specific mount properties via fspick()+fsconfig() (2) generic mount properties via mount_setattr() during a remount call.
