diff --git a/sys/include/vfs.h b/sys/include/vfs.h index 267a9d078f..1272be4f50 100644 --- a/sys/include/vfs.h +++ b/sys/include/vfs.h @@ -304,7 +304,7 @@ struct vfs_mount_struct { const vfs_file_system_t *fs; /**< The file system driver for the mount point */ const char *mount_point; /**< Mount point, e.g. "/mnt/cdrom" */ size_t mount_point_len; /**< Length of mount_point string (set by vfs_mount) */ - atomic_int open_files; /**< Number of currently open files */ + atomic_int open_files; /**< Number of currently open files and directories */ void *private_data; /**< File system driver private data, implementation defined */ }; @@ -660,23 +660,6 @@ struct vfs_file_system_ops { * @return <0 on error */ int (*statvfs) (vfs_mount_t *mountp, const char *restrict path, struct statvfs *restrict buf); - - /** - * @brief Get file system status of an open file - * - * @p path is only passed for consistency against the POSIX statvfs function. - * @c vfs_statvfs calls this function only when it has determined that - * @p path belongs to this file system. @p path is a file system relative - * path and does not necessarily name an existing file. - * - * @param[in] mountp file system mount to operate on - * @param[in] filp pointer to an open file on the file system being queried - * @param[out] buf pointer to statvfs struct to fill - * - * @return 0 on success - * @return <0 on error - */ - int (*fstatvfs) (vfs_mount_t *mountp, vfs_file_t *filp, struct statvfs *buf); }; /** @@ -732,6 +715,17 @@ int vfs_fstat(int fd, struct stat *buf); */ int vfs_fstatvfs(int fd, struct statvfs *buf); +/** + * @brief Get file system status of the file system containing an open directory + * + * @param[in] dirp pointer to open directory + * @param[out] buf pointer to statvfs struct to fill + * + * @return 0 on success + * @return <0 on error + */ +int vfs_dstatvfs(vfs_DIR *dirp, struct statvfs *buf); + /** * @brief Seek to position in file * @@ -904,7 +898,7 @@ int vfs_rename(const char *from_path, const char *to_path); /** * @brief Unmount a mounted file system * - * This will fail if there are any open files on the mounted file system + * This will fail if there are any open files or directories on the mounted file system * * @param[in] mountp pointer to the mount structure of the file system to unmount * @@ -1019,7 +1013,8 @@ int vfs_normalize_path(char *buf, const char *path, size_t buflen); * * Set @p cur to @c NULL to start from the beginning * - * @see @c sc_vfs.c (@c df command) for a usage example + * @deprecated This will become an internal-only function after the 2022.04 + * release, use @ref vfs_iterate_mount_dirs instead. * * @param[in] cur current iterator value * @@ -1028,6 +1023,37 @@ int vfs_normalize_path(char *buf, const char *path, size_t buflen); */ const vfs_mount_t *vfs_iterate_mounts(const vfs_mount_t *cur); +/** + * @brief Iterate through all mounted file systems by their root directories + * + * Unlike @ref vfs_iterate_mounts, this is thread safe, and allows thread safe + * access to the mount point's stats through @ref vfs_dstatvfs. If mounts or + * unmounts happen while iterating, this is guaranteed to report all file + * systems that stayed mounted, and may report any that are transiently + * mounted for up to as often as they are (re)mounted. Note that the volume + * being reported can not be unmounted as @p dir is an open directory. + * + * Zero-initialize @p dir to start. As long as @c true is returned, @p dir is a + * valid directory on which the user can call @ref vfs_readdir or @ref + * vfs_dstatvfs (or even peek at its `.mp` if they dare ignore the warning in + * @ref vfs_DIR). + * + * Users MUST NOT call @ref vfs_closedir if they intend to keep iterating, but + * MUST call it when aborting iteration. + * + * Note that this requires all enumerated file systems to support the `opendir` + * @ref vfs_dir_ops; any file system that does not support that will + * prematurely terminate the mount point enumeration. + * + * @see @c sc_vfs.c (@c df command) for a usage example + * + * @param[inout] dir The root directory of the discovered mount point + * + * @return @c true if another file system is mounted; @p dir then contains an open directory. + * @return @c false if the file system list is exhausted; @p dir is uninitialized then. + */ +bool vfs_iterate_mount_dirs(vfs_DIR *dir); + /** * @brief Get information about the file for internal purposes * diff --git a/sys/shell/commands/sc_vfs.c b/sys/shell/commands/sc_vfs.c index f04ec2b932..98c15b00c2 100644 --- a/sys/shell/commands/sc_vfs.c +++ b/sys/shell/commands/sc_vfs.c @@ -115,11 +115,11 @@ static int _errno_string(int err, char *buf, size_t buflen) } #undef _case_snprintf_errno_name -static void _print_df(const char *path) +static void _print_df(vfs_DIR *dir) { struct statvfs buf; - int res = vfs_statvfs(path, &buf); - printf("%-16s ", path); + int res = vfs_dstatvfs(dir, &buf); + printf("%-16s ", dir->mp->mount_point); if (res < 0) { char err[16]; _errno_string(res, err, sizeof(err)); @@ -136,13 +136,24 @@ static int _df_handler(int argc, char **argv) puts("Mountpoint Total Used Available Capacity"); if (argc > 1) { const char *path = argv[1]; - _print_df(path); + /* Opening a directory just to statfs is somewhat odd, but it is the + * easiest to support with a single _print_df function */ + vfs_DIR dir; + int res = vfs_opendir(&dir, path); + if (res == 0) { + _print_df(&dir); + vfs_closedir(&dir); + } else { + char err[16]; + _errno_string(res, err, sizeof(err)); + printf("Failed to open `%s`: %s\n", path, err); + } } else { /* Iterate through all mount points */ - const vfs_mount_t *it = NULL; - while ((it = vfs_iterate_mounts(it)) != NULL) { - _print_df(it->mount_point); + vfs_DIR it = { 0 }; + while (vfs_iterate_mount_dirs(&it)) { + _print_df(&it); } } return 0; diff --git a/sys/vfs/vfs.c b/sys/vfs/vfs.c index 7140ee388c..817f248c0b 100644 --- a/sys/vfs/vfs.c +++ b/sys/vfs/vfs.c @@ -216,15 +216,25 @@ int vfs_fstatvfs(int fd, struct statvfs *buf) } vfs_file_t *filp = &_vfs_open_files[fd]; memset(buf, 0, sizeof(*buf)); - if (filp->mp->fs->fs_op->fstatvfs == NULL) { - /* file system driver does not implement fstatvfs() */ - if (filp->mp->fs->fs_op->statvfs != NULL) { - /* Fall back to statvfs */ - return filp->mp->fs->fs_op->statvfs(filp->mp, "/", buf); - } + if (filp->mp->fs->fs_op->statvfs == NULL) { + /* file system driver does not implement statvfs() */ return -EINVAL; } - return filp->mp->fs->fs_op->fstatvfs(filp->mp, filp, buf); + return filp->mp->fs->fs_op->statvfs(filp->mp, "/", buf); +} + +int vfs_dstatvfs(vfs_DIR *dirp, struct statvfs *buf) +{ + DEBUG("vfs_dstatvfs: %p, %p\n", (void*)dirp, (void *)buf); + if (buf == NULL) { + return -EFAULT; + } + memset(buf, 0, sizeof(*buf)); + if (dirp->mp->fs->fs_op->statvfs == NULL) { + /* file system driver does not implement statvfs() */ + return -EINVAL; + } + return dirp->mp->fs->fs_op->statvfs(dirp->mp, "/", buf); } off_t vfs_lseek(int fd, off_t off, int whence) @@ -512,7 +522,7 @@ int vfs_mount(vfs_mount_t *mountp) } } } - /* insert last in list */ + /* Insert last in list. This property is relied on by vfs_iterate_mount_dirs. */ clist_rpush(&_vfs_mounts_list, &mountp->list_entry); mutex_unlock(&_mount_mutex); DEBUG("vfs_mount: mount done\n"); @@ -886,16 +896,62 @@ const vfs_mount_t *vfs_iterate_mounts(const vfs_mount_t *cur) /* empty list */ return NULL; } + node = node->next; } else { node = cur->list_entry.next; - if (node == _vfs_mounts_list.next) { + if (node == _vfs_mounts_list.next->next) { return NULL; } } return container_of(node, vfs_mount_t, list_entry); } +/* General implementation note: This heavily relies on the produced opened dir + * to lock keep the underlying mount point from closing. */ +bool vfs_iterate_mount_dirs(vfs_DIR *dir) +{ + /* This is NULL after the prescribed initialization, or a valid (and + * locked) mount point otherwise */ + vfs_mount_t *last_mp = dir->mp; + + /* This is technically violating vfs_iterate_mounts' API, as that says no + * mounts or unmounts on the chain while iterating. However, as we know + * that the current dir's mount point is still on, the equivalent procedure + * of starting a new round of `vfs_iterate_mounts` from NULL and calling it + * until it produces `last_mp` (all while holding _mount_mutex) would leave + * us with the very same situation as if we started iteration with last_mp. + * + * On the cast discarding const: vfs_iterate_mounts's type is more for + * public use */ + vfs_mount_t *next = (vfs_mount_t *)vfs_iterate_mounts(last_mp); + if (next == NULL) { + /* Ignoring errors, can't help with them */ + vfs_closedir(dir); + return false; + } + + /* Even if we held the mutex up to here (see above comment on the fiction + * of acquiring it, iterating to where we are, and releasing it again), + * we'd need to let go of it now to actually open the directory. This + * temporary count ensures that the file system will stick around for the + * directory open step that follows immediately */ + atomic_fetch_add(&next->open_files, 1); + + /* Ignoring errors, can't help with them */ + vfs_closedir(dir); + + int err = vfs_opendir(dir, next->mount_point); + /* No matter the success, the open_files lock has done its duty */ + atomic_fetch_sub(&next->open_files, 1); + + if (err != 0) { + DEBUG("vfs_iterate_mount opendir erred: vfs_opendir(\"%s\") = %d\n", next->mount_point, err); + return false; + } + return true; +} + const vfs_file_t *vfs_file_get(int fd) { if (_fd_is_valid(fd) == 0) { diff --git a/tests/vfs_iterate_mount/Makefile b/tests/vfs_iterate_mount/Makefile new file mode 100644 index 0000000000..c05a41a42b --- /dev/null +++ b/tests/vfs_iterate_mount/Makefile @@ -0,0 +1,6 @@ +include ../Makefile.tests_common + +USEMODULE += vfs +USEMODULE += constfs + +include $(RIOTBASE)/Makefile.include diff --git a/tests/vfs_iterate_mount/Makefile.ci b/tests/vfs_iterate_mount/Makefile.ci new file mode 100644 index 0000000000..b9ff275375 --- /dev/null +++ b/tests/vfs_iterate_mount/Makefile.ci @@ -0,0 +1,3 @@ +BOARD_INSUFFICIENT_MEMORY := \ + nucleo-l011k4 \ + # diff --git a/tests/vfs_iterate_mount/main.c b/tests/vfs_iterate_mount/main.c new file mode 100644 index 0000000000..6d415fa4bd --- /dev/null +++ b/tests/vfs_iterate_mount/main.c @@ -0,0 +1,124 @@ +/* + * Copyright (C) Christian Amsüss + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * Mount and unmount a few file systems, demonstrating that + * vfs_iterate_mount_dirs performs as advertised. + * + * @author Christian Amsüss + */ + +#include +#include +#include + +#include +#include + +static constfs_file_t constfs_files[1] = { + /* Not completely empty -- that'd be a hassle around empty arrays and + * their size */ + { + .path = "some-file", + .size = 0, + .data = (void*)"", + }, +}; + +static constfs_t constfs_desc = { + .nfiles = ARRAY_SIZE(constfs_files), + .files = constfs_files, +}; + +static vfs_mount_t mount1 = { + .fs = &constfs_file_system, + .mount_point = "/const1", + .private_data = &constfs_desc, +}; + +static vfs_mount_t mount2 = { + .fs = &constfs_file_system, + .mount_point = "/const2", + .private_data = &constfs_desc, +}; + +static vfs_mount_t mount3 = { + .fs = &constfs_file_system, + .mount_point = "/const3", + .private_data = &constfs_desc, +}; + +static vfs_mount_t mount4 = { + .fs = &constfs_file_system, + .mount_point = "/const4", + .private_data = &constfs_desc, +}; + +/* Crank the iterator, reporting "N%s" for the next entry, or "O\n" for the end + * of the iterator (avoiding the letter "E" which may be misread for an error + * in a casual look at the error output) */ +static void iter_and_report(vfs_DIR *iter) { + bool result = vfs_iterate_mount_dirs(iter); + if (result) { + printf("N(%s)", iter->mp->mount_point); + } else { + printf("O\n"); + /* Zero out so we're ready for next round immediately */ + memset(iter, 0, sizeof(*iter)); + } +} + +int main(void) { + int res = 0; + + vfs_DIR iter; + memset(&iter, 0, sizeof(iter)); + + res |= vfs_mount(&mount1); + res |= vfs_mount(&mount2); + res |= vfs_mount(&mount3); + res |= vfs_mount(&mount4); + assert(res == 0); + printf("Mounted 1234\n"); + + /* N1N2N3N4E */ + iter_and_report(&iter); + iter_and_report(&iter); + iter_and_report(&iter); + iter_and_report(&iter); + iter_and_report(&iter); + + /* N1N2, unmount 3, N4E */ + iter_and_report(&iter); + iter_and_report(&iter); + res |= vfs_umount(&mount3); + iter_and_report(&iter); + iter_and_report(&iter); + + /* N1, unmount 2, (3 is already unmounted), N4, mount 3 N3, unmount 1 and remount it at the end N1, O */ + /* It is OK that 1 is reported twice, because its first occurrence is its + * old mounting, and later it reappears */ + iter_and_report(&iter); + res |= vfs_umount(&mount2); + iter_and_report(&iter); + res |= vfs_mount(&mount3); + iter_and_report(&iter); + res |= vfs_umount(&mount1); + res |= vfs_mount(&mount1); + iter_and_report(&iter); + iter_and_report(&iter); + + /* This ensures we're not leaking locks */ + res |= vfs_umount(&mount1); + res |= vfs_umount(&mount3); + res |= vfs_umount(&mount4); + printf("All unmounted\n"); + + /* Only O */ + iter_and_report(&iter); +} diff --git a/tests/vfs_iterate_mount/tests/01-run.py b/tests/vfs_iterate_mount/tests/01-run.py new file mode 100755 index 0000000000..5a705e8d86 --- /dev/null +++ b/tests/vfs_iterate_mount/tests/01-run.py @@ -0,0 +1,23 @@ +#!/usr/bin/env python3 + +# Copyright (C) 2021 Christian Amsüss +# +# This file is subject to the terms and conditions of the GNU Lesser +# General Public License v2.1. See the file LICENSE in the top level +# directory for more details. + +import sys +from testrunner import run + + +def testfunc(child): + child.expect_exact("Mounted 1234") + child.expect_exact("N(/const1)N(/const2)N(/const3)N(/const4)O") + child.expect_exact("N(/const1)N(/const2)N(/const4)O") + child.expect_exact("N(/const1)N(/const4)N(/const3)N(/const1)O") + child.expect_exact("All unmounted") + child.expect_exact("O") + + +if __name__ == "__main__": + sys.exit(run(testfunc))