Page MenuHomeFreeBSD

vfs mount: Consistently use ENODEV internally for an invalid fstype
ClosedPublic

Authored by jhb on Oct 23 2023, 3:47 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 10:38 AM
Unknown Object (File)
Sep 17 2024, 11:22 AM
Unknown Object (File)
Sep 16 2024, 11:30 AM
Unknown Object (File)
Sep 7 2024, 10:26 AM
Unknown Object (File)
Sep 4 2024, 10:48 PM
Unknown Object (File)
Aug 6 2024, 2:21 PM
Unknown Object (File)
Aug 5 2024, 9:34 PM
Unknown Object (File)
Aug 1 2024, 2:33 AM
Subscribers

Details

Summary

Change vfs_byname_kld to always return an error value of ENODEV to
indicate an unsupported fstype leaving ENOENT to indicate errors such
as a missing mount point or invalid path. This allows nmount(2) to
better distinguish these cases and avoid treating a missing device
node as an invalid fstype after commit 6e8272f317b8.

While here, change mount(2) to return EINVAL instead of ENODEV for an
invalid fstype to match nmount(2).

PR: 274600

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jhb requested review of this revision.Oct 23 2023, 3:47 AM
sys/kern/vfs_init.c
153

Do we want to translate all errors to ENODEV, or just ENOENT?

sys/kern/vfs_init.c
153

I think all. The call in ZFS ignores *error and always returns ENODEV already. I think from the caller's perspective it's still true after a failed kldload that vfs_byname() would return NULL and that's the main error we want to communicate here.

What are the user-facing errors in various cases after this?

What are the user-facing errors in various cases after this?

Using an invalid path like /dev/foobar now returns ENOENT again as it did before 6e8272f317b8. Trying to use an invalid fstype that fails to kldload still returns EINVAL from nmount(2) (a change made in 6e8272f317b8) and mount(2) now returns EINVAL for that case instead of ENOENT. Before 6e8272f317b8 the invalid fstype case probably returned ENOENT but might have possibly returned ENODEV. The invalid fstype case is not documented in the mount(2) manpage, but the ENOENT error is, so for the documented ENOENT case this commit fixes a regression in 6e8272f317b8.

Thanks for the information and apologies for the mess.
For me the case where the filesystem name was wrong and mount complained about file not existing was more common and more annoying since it was easy to see that the designated mount point clearly existed.

I'd vote for pushing this into the upcoming release.

This revision is now accepted and ready to land.Oct 25 2023, 4:01 PM

@jhb do you think the following is related?

root@precision:~ # gpart show da0
=>       63  500118129  da0  MBR  (238G)
         63       1985       - free -  (993K)
       2048   67108864    1  fat32lba  [active]  (32G)
   67110912  433007280       - free -  (206G)

root@precision:~ # mount /dev/da0s1 /tmp/empty
mount: /dev/da0s1: No such file or directory
root@precision:~ # mount -t msdosfs /dev/da0s1 /tmp/empty
root@precision:~ #
This comment was removed by pstef.

@jhb do you think the following is related?

root@precision:~ # gpart show da0
=>       63  500118129  da0  MBR  (238G)
         63       1985       - free -  (993K)
       2048   67108864    1  fat32lba  [active]  (32G)
   67110912  433007280       - free -  (206G)

root@precision:~ # mount /dev/da0s1 /tmp/empty
mount: /dev/da0s1: No such file or directory
root@precision:~ # mount -t msdosfs /dev/da0s1 /tmp/empty
root@precision:~ #

Maybe? It would be helpful to know what the old/expected behavior is for comparison. It seems that ffs_mount is returning ENOENT when it sees an invalid superblock or the like which used to get translated into EINVAL? Probably it should be returning a better errno value than ENOENT.