On Thu, 2024-11-14 at 12:29 +0100, Christian Brauner wrote: > On Wed, Nov 13, 2024 at 08:45:06AM -0500, Jeff Layton wrote: > > On Wed, 2024-11-13 at 12:27 +0100, Karel Zak wrote: > > > On Tue, Nov 12, 2024 at 02:39:21PM GMT, Christian Brauner wrote: > > > > On Mon, 11 Nov 2024 10:09:54 -0500, Jeff Layton wrote: > > > > > Meta has some internal logging that scrapes /proc/self/mountinfo today. > > > > > I'd like to convert it to use listmount()/statmount(), so we can do a > > > > > better job of monitoring with containers. We're missing some fields > > > > > though. This patchset adds them. > > > > > > > > > > > > > > > > > > Applied to the vfs.misc branch of the vfs/vfs.git tree. > > > > Patches in the vfs.misc branch should appear in linux-next soon. > > > > > > Jeff, thank you for this! > > > > > > I have already implemented support for statmount() and listmount() in > > > libmount (PR: https://github.com/util-linux/util-linux/pull/3092). The > > > only remaining issue was the mount source and incomplete file system > > > type. > > > > > > > Unfortunately, I think we might be missing something else: > > > > The mountinfo (and "mounts") file generator calls show_sb_opts() which > > generates some strings from the sb->s_flags field and then calls > > security_sb_show_options(). statmount() will give you the s_flags field > > (or an equivalent), but it doesn't give you the security options > > string. So, those aren't currently visible from statmount(). > > > > How should we expose those? Should we create a new statmount string > > field and populate it, or is it better to just tack them onto the end > > of the statmount.mnt_opts string? > > I'm leaning towards using a separate field because mnt_opts/opts_array > is about filesystem specific mount options whereas the security mount > options are somewhat generic. So it's easy to tell them apart. Ordinarily, I might agree, but we're now growing a new mount option field that has them separated by NULs. Will we need two extra fields for this? One comma-separated, and one NUL separated? /proc/#/mountinfo and mounts prepend these to the output of ->show_options, so the simple solution would be to just prepend those there instead of adding a new field. FWIW, only SELinux has any extra mount options to show here. Tough call -- anyone else have opinions? -- Jeff Layton <jlayton@xxxxxxxxxx>
