statmount: requesting more information: statfs, devname, label

statmount: requesting more information: statfs, devname, label

[Thiago Macieira]

[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]

Hello Miklos & others

Thank you for the listmount() & statmount() API. I found it very easy to use 
and definitely easier than parsing of /proc/self/mountinfo. I am missing three 
pieces of information from statmount(), two of which I can get from elsewhere 
but the third is a showstopper.

The showstopper is the lack of mnt_devname anywhere in the output. It is 
present in mountinfo and even in the older /etc/mtab and I couldn't find a way 
to convert back from the device's major/minor to a string form. Scanning /dev 
will not work because the process in question may not have a populated /dev 
and even if it does, it would be wasteful to scan /dev for all devices. 
Moreover, given symlinks for device-mapper, /dev/dm-1 isn't as descriptive as 
/dev/mapper/system-root or /dev/system/root.

Is there a chance of getting this in a new version of the kernel?

Of the two others, we can get via other system calls, but would be nice if 
statmount() also provided it.

First, the information provided by statfs(), which is the workaround. It's 
easy to call statfs() with the returned mount point path, though that causes a 
minor race.

The second is the filesystem label. The workaround for this is opening the 
mount point and issuing ioctl(FS_IOC_GETFSLABEL), but that again introduces a 
minor race and also requires that the ability to open() the path in question. 
The second fallback to that is to scan /dev/disks/by-label, which is populated 
by udev/udisks/systemd.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Principal Engineer - Intel DCAI Fleet Engineering and Quality

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5152 bytes --]

Re: statmount: requesting more information: statfs, devname, label

[Christian Brauner]

On Wed, May 29, 2024 at 03:36:39PM -0300, Thiago Macieira wrote:
> Hello Miklos & others
> 
> Thank you for the listmount() & statmount() API. I found it very easy to use 
> and definitely easier than parsing of /proc/self/mountinfo. I am missing three 
> pieces of information from statmount(), two of which I can get from elsewhere 
> but the third is a showstopper.
> 
> The showstopper is the lack of mnt_devname anywhere in the output. It is 
> present in mountinfo and even in the older /etc/mtab and I couldn't find a way 
> to convert back from the device's major/minor to a string form. Scanning /dev 
> will not work because the process in question may not have a populated /dev 
> and even if it does, it would be wasteful to scan /dev for all devices. 
> Moreover, given symlinks for device-mapper, /dev/dm-1 isn't as descriptive as 
> /dev/mapper/system-root or /dev/system/root.
> 
> Is there a chance of getting this in a new version of the kernel?
> 
> Of the two others, we can get via other system calls, but would be nice if 
> statmount() also provided it.
> 
> First, the information provided by statfs(), which is the workaround. It's 
> easy to call statfs() with the returned mount point path, though that causes a 
> minor race.
> 
> The second is the filesystem label. The workaround for this is opening the 
> mount point and issuing ioctl(FS_IOC_GETFSLABEL), but that again introduces a 
> minor race and also requires that the ability to open() the path in question. 
> The second fallback to that is to scan /dev/disks/by-label, which is populated 
> by udev/udisks/systemd.

I think that mnt_devname makes sense!
I don't like the other additions because they further blur the
distinction between mount and filesystem information.

Re: statmount: requesting more information: statfs, devname, label

[Miklos Szeredi]

On Thu, 30 May 2024 at 09:16, Christian Brauner <brauner@kernel.org> wrote:
>
> On Wed, May 29, 2024 at 03:36:39PM -0300, Thiago Macieira wrote:

> > First, the information provided by statfs(), which is the workaround. It's
> > easy to call statfs() with the returned mount point path, though that causes a
> > minor race.

It's easy enough to shove struct statfs fields into struct statmount.
I didn't do that for the first version to minimise the size of the
patch but I think it makes sense.  Even Linus suggested that this
syscall should be an extended statfs(2), which implies that statfs
info should be included.

> > The second is the filesystem label. The workaround for this is opening the
> > mount point and issuing ioctl(FS_IOC_GETFSLABEL), but that again introduces a
> > minor race and also requires that the ability to open() the path in question.
> > The second fallback to that is to scan /dev/disks/by-label, which is populated
> > by udev/udisks/systemd.

FS_IOC_GETFSLABEL seems to be implemented only by a handful of
filesystems.   I don't really undestand how this label thing works...

> I think that mnt_devname makes sense!
> I don't like the other additions because they further blur the
> distinction between mount and filesystem information.

mnt_devname is exactly that: filesystem information (don't let it fool
you that it's in struct mount, that's just an historical accident).
It's just a special option that customarily refers to a device path,
but in general is very much filesystem specific.

I don't think we've promised that statmount(2) will only return mount
information.  In fact it does already return a fair amount of
filesystem info as well.  Note, stat/statx also return various bits
and pieces, some inode some mount and some fs specific.

I'd put fs options in there too, but that was something Christian
disliked.  We should have a discussion about how to retrieve options,
and maybe the other things like label, devname, etc will fall out of
that too.

Thanks,
Miklos

Re: statmount: requesting more information: statfs, devname, label

[Christian Brauner]

> I'd put fs options in there too, but that was something Christian
> disliked.  We should have a discussion about how to retrieve options,
> and maybe the other things like label, devname, etc will fall out of
> that too.

Let me rephrase what I mean as I probably wasn't clear enough in my
first mail. My objection is mostly that I don't want us to start putting
stuff in there that's not generic. And yes, some statfs() fields might
make sense to put in statmount(). But I think stuff like "f_files" or
"f_ffree" is really something that's misplaced in statmount().

Re: statmount: requesting more information: statfs, devname, label

[Thiago Macieira]

[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]

On Thursday 30 May 2024 07:42:09 GMT-3 Christian Brauner wrote:
> Let me rephrase what I mean as I probably wasn't clear enough in my
> first mail. My objection is mostly that I don't want us to start putting
> stuff in there that's not generic. And yes, some statfs() fields might
> make sense to put in statmount(). But I think stuff like "f_files" or
> "f_ffree" is really something that's misplaced in statmount().

I'll take what I can get. My class's API exposes the total size of the 
filesystem (f_blocks), the bytes available and free (f_bfree and f_bavail), the 
block size and whether the filesystem is read-only. For the latter, I don't 
know if it's the same as MOUNT_ATTR_RDONLY.

Keeping statfs() is not a problem. My biggest problem is matching the mounted 
filesystems from either mountinfo or listmounts() with what is actually visible 
in the system. For example, what happens if you mount something that makes a 
mountpoint invisible? Both df and the Qt class hide the entry too, but it was 
guess work until the mount IDs were exposed in statx().

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Principal Engineer - Intel DCAI Fleet Engineering and Quality

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5152 bytes --]

Re: statmount: requesting more information: statfs, devname, label

[Thiago Macieira]

[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]

On Thursday 30 May 2024 05:25:39 GMT-3 Miklos Szeredi wrote:
> > > The second is the filesystem label. The workaround for this is opening
> > > the
> > > mount point and issuing ioctl(FS_IOC_GETFSLABEL), but that again
> > > introduces a minor race and also requires that the ability to open()
> > > the path in question. The second fallback to that is to scan
> > > /dev/disks/by-label, which is populated by udev/udisks/systemd.
> 
> FS_IOC_GETFSLABEL seems to be implemented only by a handful of
> filesystems.   I don't really undestand how this label thing works...

Nor I. It's one of the btrfs ioctl calls that became generic, like FICLONE, so 
it's not surprising that it isn't supported for all filesystems. For the rest, 
I guess udev/udisks knows the filesystem superblock header format and reads the 
label off it, because it seems to know the labels for FAT filesystems despite 
the system call not offering it.

openat(AT_FDCWD, "/boot/efi", O_RDONLY|O_CLOEXEC) = 4
statfs("/boot/efi", {f_type=MSDOS_SUPER_MAGIC, ...}) = 0
ioctl(4, FS_IOC_GETFSLABEL, 0x7ffe6f557110) = -1 ENOTTY

My code to decode this via /dev/disks/by-label is a best effort that seems to 
work for everything that doesn't support the ioctl. Fortunately, statmount() 
does give me the actual device's major/minor, no the anonymous IDs that some 
filesystems (like btrfs) use for subvolumes, so it's actually easier to scan 
with statmount() information.

> > I think that mnt_devname makes sense!
> > I don't like the other additions because they further blur the
> > distinction between mount and filesystem information.
> 
> mnt_devname is exactly that: filesystem information (don't let it fool
> you that it's in struct mount, that's just an historical accident).
> It's just a special option that customarily refers to a device path,
> but in general is very much filesystem specific.

Ah, good point. So here's even a stronger reason: if I had a remote FS, I'd 
probably want both the devname and the mount options, neither of which are in 
statmount() right now.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Principal Engineer - Intel DCAI Fleet Engineering and Quality

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5152 bytes --]