sys/vfs: add vfs_mount_by_path()

[benpicco]

Contribution description

If auto-init of a default mount fs failed (e.g. because an SD card was not inserted on boot) or vfs_auto_mount was disabled, there is no way to mount such an auto-mount fs later manually.

Fix this by introducing a vfs_mount_by_path() function that takes the mount point of a pre-configured mount to trigger the mount procedure at run-time (e.g. when an SD card was inserted).

Testing procedure

I extended the vfs command with a mount sub-command.

vfs mount /

Issues/PRs references

[chrysn]

Generally looks good and straightforward.

An alternative way of identifying the mount point would be reconstructing the identifier (say, in a VFS_MOUNT_BY_AUTOMOUNT(littlefs2, 0); macro). Given that the boards would rather converge on the paths and not on the file systems and disambiguators, by_path is probably the better of these.

[chrysn]

Should be VFS_AUTO_MOUNT (it'd work just as well no matter whether these are defaults or set up by the application in a custom setup)

(also above in the corresponding help printf)

[benpicco]

How about we use XFA_LEN here instead?

[chrysn]

Does that work? I figure that if VFS_AUTO_MOUNT is not present, the relevant symbols are not even defined.

If it works, it's probably good enough (given that shell is not something present on the tiniest systems anyway). Note that MOUNTPOINTS_NUMOF gets its const value only really late, when the linker has assigned addresses and the symbols are substituted into the code -- I don't think that on NUMOF == 0, the code behind the check would even be recognized as dead code.

[benpicco]

If I remove the auto-mount:

--- a/boards/native/board_init.c
+++ b/boards/native/board_init.c
@@ -55,7 +55,7 @@ VFS_AUTO_MOUNT(littlefs, VFS_MTD(mtd0_dev), "/nvm", 0);
 #elif defined(MODULE_LITTLEFS2)
 
 #include "fs/littlefs2_fs.h"
-VFS_AUTO_MOUNT(littlefs2, VFS_MTD(mtd0_dev), "/nvm", 0);
+//VFS_AUTO_MOUNT(littlefs2, VFS_MTD(mtd0_dev), "/nvm", 0);
 
 /* spiffs support */
 #elif defined(MODULE_SPIFFS)

> vfs mount
vfs mount
vfs: unsupported sub-command "mount"

[chrysn]

Oh work in the sense of runtime it sure does; I was more wondering whether it still builds when VFS_AUTO_MOUNT is not an active module.

[benpicco]

This is examples/filesystem where vfs_auto_mount is not used

[chrysn]

Weird, looking into it to fix my understanding.

[chrysn]

Ah, found it: The XFA_INIT (and even its user auto_init_vfs) is in immaterial of whether the automounter is set, so whenever VFS is there, that array is too.

Conversely, this probably works even when VFS_DEFAULT is on but VFS_AUTO_MOUNT is off, which is not bad, just unexpected (and not even contradicting the documentation) -- because the macro named VFS_AUTO_MOUNT and the module VFS_AUTO_MOUNT do slightly different things.

[chrysn]

It's tangential (and not a proposal for addition here, although it might later share code), but are there any other operations that make sense over all auto-mountpoints? Seeing this in the PR list next to my "enumerate mount points" PR (#17660) makes me wonder if listing them too makes sense, or checking whether a mount is an automount.

vfs df (shifting towards a mixture of POSIX df and mount commands) could then list mountes, say whether they are automounted (which in hindsight is a confusing term, given that on Linux it means "detect insertion and mount then"), and also list (without showing their stats) potential mounts that are currently unmounted. (OK, maybe that would be more fitting to a no-further-arguments vfs mount that is a mixture of Linux's mount and cat /etc/fstab).

[benpicco]

I thought about that too, (e.g. vfs_get_mount_by_path() but then after the mount there is no need to differentiate between auto-mounted mounts anymore.

[chrysn]

after the mount there is no need to differentiate between auto-mounted mounts anymore.

Mh, not sure -- someone might want to unmount all automounted FSs before going into reboot or deeper sleep; but then again, that's all relatively custom anyway.

But then, so is mounting by a concrete path; maybe the "retry all previously failed automounts" would be a more precise fit for the initial use case you give?

It's all about going one-way from unmounted to mounted, after all -- the unmount that'd clear an SD card for removal would need application specific knowledge of what is actually safe to remove anyway.

[benpicco]

maybe the "retry all previously failed automounts" would be a more precise fit for the initial use case you give?

But then we'd need to keep track of which auto-mounts did fail 😉

[chrysn]

But then we'd need to keep track of which auto-mounts did fail 😉

The ones that are currently not mounted, no? (Granted, it's "or have been unmounted")

[benpicco]

Yea but it's much easier to use the "SD card inserted" GPIO interrupt and mount the path associated with the SD card then trying to (re-)mount all FSs on that event.

[chrysn]

I'm just trying to form a consistent full example that works without application specific knowledge of the mount points other than the path ... yeah, but probably there is: | Whenever an SD card is inserted, store your data on it; configure with | an LED number (defaulting to LED0) and a GPIO pin for SD card | insertion (not sure we have that accessible). Try mounting it (on | blink an error pattern if it fails), copy data over and unmount it | when done (blinking a "remove SD card now" pattern). OK, that works with the function as presented alone, will review the rest. (Still, there's nothing keeping others from going through, say the mount points, and listing them directly in a `cat /etc/fstab` style -- or checking whether a mount point is in that XFA; these may or may not later be useful as provided functions).

[chrysn]

For one minor thing I'd like your confirmation that you think it's good -- but once that's in or adjusted, ACK, please squash and go ahead.

[chrysn]

Suggested change

	XFA_USE_CONST(vfs_mount_t, vfs_mountpoints_xfa);
	XFA_USE(vfs_mount_t, vfs_mountpoints_xfa);

The array is init'd XFA_INIT (and not XFA_INIT_CONST), so I think it should be aligned type-wise (but I'm not sure -- it boils down to an extern being once used with const and once without, and I don't know whether the const is part of the type or just an effect here). I'm happy if you think it's fine. (It works, after all, and no Werrors show.)

[benpicco]

hmm I think it's good to use const here to promise that the array is not going to be modified by this file