From 3b587d9268c799e9237089e0be986ced975d2d9b Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Sun, 26 Feb 2023 23:37:30 +0100 Subject: [PATCH 1/3] sys/vfs: add force option to vfs_umount() --- sys/include/vfs.h | 6 ++++-- sys/shell/cmds/vfs.c | 4 ++-- sys/vfs/vfs.c | 8 ++++---- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/sys/include/vfs.h b/sys/include/vfs.h index a7df6110f8..e7009022c7 100644 --- a/sys/include/vfs.h +++ b/sys/include/vfs.h @@ -969,11 +969,12 @@ int vfs_mount_by_path(const char *path); * @note This assumes mount points have been configured with @ref VFS_AUTO_MOUNT. * * @param[in] path Path of the pre-configured mount point + * @param[in] force Unmount the filesystem even if there are still open files * * @return 0 on success * @return <0 on error */ -int vfs_unmount_by_path(const char *path); +int vfs_unmount_by_path(const char *path, bool force); /** * @brief Rename a file @@ -996,11 +997,12 @@ int vfs_rename(const char *from_path, const char *to_path); * 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 + * @param[in] force Unmount the filesystem even if there are still open files * * @return 0 on success * @return <0 on error */ -int vfs_umount(vfs_mount_t *mountp); +int vfs_umount(vfs_mount_t *mountp, bool force); /** * @brief Unlink (delete) a file from a mounted file system diff --git a/sys/shell/cmds/vfs.c b/sys/shell/cmds/vfs.c index 4a0407a4aa..1f289d0171 100644 --- a/sys/shell/cmds/vfs.c +++ b/sys/shell/cmds/vfs.c @@ -191,7 +191,7 @@ static int _umount_handler(int argc, char **argv) return -1; } - int res = vfs_unmount_by_path(argv[1]); + int res = vfs_unmount_by_path(argv[1], false); if (res < 0) { puts(tiny_strerror(res)); } @@ -207,7 +207,7 @@ static int _remount_handler(int argc, char **argv) return -1; } - vfs_unmount_by_path(argv[1]); + vfs_unmount_by_path(argv[1], false); int res = vfs_mount_by_path(argv[1]); if (res < 0) { puts(tiny_strerror(res)); diff --git a/sys/vfs/vfs.c b/sys/vfs/vfs.c index 3c4f65b99f..e48bbe6f5d 100644 --- a/sys/vfs/vfs.c +++ b/sys/vfs/vfs.c @@ -547,7 +547,7 @@ int vfs_mount(vfs_mount_t *mountp) return 0; } -int vfs_umount(vfs_mount_t *mountp) +int vfs_umount(vfs_mount_t *mountp, bool force) { DEBUG("vfs_umount: %p\n", (void *)mountp); int ret = check_mount(mountp); @@ -564,7 +564,7 @@ int vfs_umount(vfs_mount_t *mountp) return -EINVAL; } DEBUG("vfs_umount: -> \"%s\" open=%d\n", mountp->mount_point, atomic_load(&mountp->open_files)); - if (atomic_load(&mountp->open_files) > 0) { + if (atomic_load(&mountp->open_files) > 0 && !force) { mutex_unlock(&_mount_mutex); return -EBUSY; } @@ -1204,11 +1204,11 @@ int vfs_mount_by_path(const char *path) return -ENOENT; } -int vfs_unmount_by_path(const char *path) +int vfs_unmount_by_path(const char *path, bool force) { for (unsigned i = 0; i < MOUNTPOINTS_NUMOF; ++i) { if (strcmp(path, vfs_mountpoints_xfa[i].mount_point) == 0) { - return vfs_umount(&vfs_mountpoints_xfa[i]); + return vfs_umount(&vfs_mountpoints_xfa[i], force); } } From c19c25e078f1bdbb455bae65d2a5b4cf77d5b372 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Sun, 26 Feb 2023 23:48:33 +0100 Subject: [PATCH 2/3] tests: adapt to vfs_umount() API change --- tests/devfs/main.c | 2 +- tests/pkg_fatfs_vfs/main.c | 20 +++++++++---------- tests/pkg_littlefs/main.c | 8 ++++---- tests/pkg_littlefs2/main.c | 8 ++++---- tests/pkg_spiffs/main.c | 10 +++++----- tests/pkg_tinyvcdiff/main.c | 2 +- tests/unittests/tests-vfs/tests-vfs-dir-ops.c | 2 +- .../unittests/tests-vfs/tests-vfs-file-ops.c | 2 +- .../tests-vfs/tests-vfs-file-system-ops.c | 6 +++--- .../tests-vfs/tests-vfs-mount-constfs.c | 12 +++++------ tests/vfs_iterate_mount/main.c | 12 +++++------ 11 files changed, 42 insertions(+), 42 deletions(-) diff --git a/tests/devfs/main.c b/tests/devfs/main.c index d1448454ed..475f28a3b5 100644 --- a/tests/devfs/main.c +++ b/tests/devfs/main.c @@ -136,7 +136,7 @@ static void test_devfs_mount_open(void) res = devfs_unregister(&_mock_devfs_node); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_devfs_mount); + res = vfs_umount(&_test_devfs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); } diff --git a/tests/pkg_fatfs_vfs/main.c b/tests/pkg_fatfs_vfs/main.c index 5ff51f4fbb..d25272b761 100644 --- a/tests/pkg_fatfs_vfs/main.c +++ b/tests/pkg_fatfs_vfs/main.c @@ -90,7 +90,7 @@ static void test_format(void) static void test_mount(void) { print_test_result("test_mount__mount", vfs_mount(&_test_vfs_mount) == 0); - print_test_result("test_mount__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_mount__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_open(void) @@ -113,7 +113,7 @@ static void test_open(void) print_test_result("test_open__open_rw", fd >= 0); print_test_result("test_open__close_rw", vfs_close(fd) == 0); - print_test_result("test_open__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_open__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_rw(void) @@ -193,7 +193,7 @@ static void test_rw(void) (strncmp(buf, test_txt3, sizeof(test_txt3)) == 0)); print_test_result("test_rw__close_rwc", vfs_close(fd) == 0); - print_test_result("test_rw__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_rw__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_dir(void) @@ -216,7 +216,7 @@ static void test_dir(void) print_test_result("test_dir__readdir3", vfs_readdir(&dir, &entry2) == 0); print_test_result("test_dir__closedir", vfs_closedir(&dir) == 0); - print_test_result("test_dir__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_dir__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_rename(void) @@ -243,7 +243,7 @@ static void test_rename(void) print_test_result("test_rename__readdir3", vfs_readdir(&dir, &entry2) == 0); print_test_result("test_rename__closedir", vfs_closedir(&dir) == 0); - print_test_result("test_rename__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_rename__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_unlink(void) @@ -257,7 +257,7 @@ static void test_unlink(void) print_test_result("test_unlink__opendir", vfs_opendir(&dir, MNT_PATH) == 0); print_test_result("test_unlink__readdir", vfs_readdir(&dir, &entry) == 0); print_test_result("test_unlink__closedir", vfs_closedir(&dir) == 0); - print_test_result("test_unlink__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_unlink__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_mkrmdir(void) @@ -281,7 +281,7 @@ static void test_mkrmdir(void) vfs_opendir(&dir, MNT_PATH"/"DIR_NAME) < 0); print_test_result("test_mkrmdir__umount", - vfs_umount(&_test_vfs_mount) == 0); + vfs_umount(&_test_vfs_mount, false) == 0); } static void test_create(void) @@ -305,7 +305,7 @@ static void test_create(void) print_test_result("test_create__write_wo2", nw == sizeof(test_txt)); print_test_result("test_create__close_wo2", vfs_close(fd) == 0); - print_test_result("test_create__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_create__umount", vfs_umount(&_test_vfs_mount, false) == 0); } static void test_fstat(void) @@ -328,7 +328,7 @@ static void test_fstat(void) print_test_result("test_stat__stat", vfs_fstat(fd, &stat_buf) == 0); print_test_result("test_stat__close", vfs_close(fd) == 0); print_test_result("test_stat__size", stat_buf.st_size == sizeof(test_txt)); - print_test_result("test_stat__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_stat__umount", vfs_umount(&_test_vfs_mount, false) == 0); } #if defined(MODULE_NEWLIB) || defined(MODULE_PICOLIBC) @@ -393,7 +393,7 @@ static void test_libc(void) } print_test_result("test_libc__remove", remove(FULL_FNAME2) == 0); - print_test_result("test_libc__umount", vfs_umount(&_test_vfs_mount) == 0); + print_test_result("test_libc__umount", vfs_umount(&_test_vfs_mount, false) == 0); } #endif diff --git a/tests/pkg_littlefs/main.c b/tests/pkg_littlefs/main.c index b9669b3d06..1586148ba5 100644 --- a/tests/pkg_littlefs/main.c +++ b/tests/pkg_littlefs/main.c @@ -142,13 +142,13 @@ static void test_littlefs_teardown(void) vfs_unlink("/test-littlefs/test1.txt"); vfs_unlink("/test-littlefs/a/test2.txt"); vfs_rmdir("/test-littlefs/a"); - vfs_umount(&_test_littlefs_mount); + vfs_umount(&_test_littlefs_mount, false); } static void tests_littlefs_format(void) { int res; - vfs_umount(&_test_littlefs_mount); + vfs_umount(&_test_littlefs_mount, false); res = mtd_erase(_dev, 0, _dev->page_size * _dev->pages_per_sector * _dev->sector_count); TEST_ASSERT_EQUAL_INT(0, res); @@ -162,7 +162,7 @@ static void tests_littlefs_format(void) res = vfs_mount(&_test_littlefs_mount); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_littlefs_mount); + res = vfs_umount(&_test_littlefs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); /* 2. format a valid file system */ @@ -173,7 +173,7 @@ static void tests_littlefs_format(void) static void tests_littlefs_mount_umount(void) { int res; - res = vfs_umount(&_test_littlefs_mount); + res = vfs_umount(&_test_littlefs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); res = vfs_mount(&_test_littlefs_mount); diff --git a/tests/pkg_littlefs2/main.c b/tests/pkg_littlefs2/main.c index 01716715b0..df33d559fb 100644 --- a/tests/pkg_littlefs2/main.c +++ b/tests/pkg_littlefs2/main.c @@ -142,13 +142,13 @@ static void test_littlefs_teardown(void) vfs_unlink("/test-littlefs/test1.txt"); vfs_unlink("/test-littlefs/a/test2.txt"); vfs_rmdir("/test-littlefs/a"); - vfs_umount(&_test_littlefs_mount); + vfs_umount(&_test_littlefs_mount, false); } static void tests_littlefs_format(void) { int res; - vfs_umount(&_test_littlefs_mount); + vfs_umount(&_test_littlefs_mount, false); res = mtd_erase(_dev, 0, _dev->page_size * _dev->pages_per_sector * _dev->sector_count); TEST_ASSERT_EQUAL_INT(0, res); @@ -162,7 +162,7 @@ static void tests_littlefs_format(void) res = vfs_mount(&_test_littlefs_mount); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_littlefs_mount); + res = vfs_umount(&_test_littlefs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); /* 2. format a valid file system */ @@ -173,7 +173,7 @@ static void tests_littlefs_format(void) static void tests_littlefs_mount_umount(void) { int res; - res = vfs_umount(&_test_littlefs_mount); + res = vfs_umount(&_test_littlefs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); res = vfs_mount(&_test_littlefs_mount); diff --git a/tests/pkg_spiffs/main.c b/tests/pkg_spiffs/main.c index 883d57fc2a..e66a383f9f 100644 --- a/tests/pkg_spiffs/main.c +++ b/tests/pkg_spiffs/main.c @@ -142,7 +142,7 @@ static void test_spiffs_teardown(void) vfs_unlink("/test-spiffs/test0.txt"); vfs_unlink("/test-spiffs/test1.txt"); vfs_unlink("/test-spiffs/a/test2.txt"); - vfs_umount(&_test_spiffs_mount); + vfs_umount(&_test_spiffs_mount, false); spiffs_desc.base_addr = 0; spiffs_desc.block_count = 0; @@ -151,7 +151,7 @@ static void test_spiffs_teardown(void) static void tests_spiffs_format(void) { int res; - vfs_umount(&_test_spiffs_mount); + vfs_umount(&_test_spiffs_mount, false); res = mtd_erase(_dev, 0, _dev->page_size * _dev->pages_per_sector * _dev->sector_count); TEST_ASSERT_EQUAL_INT(0, res); @@ -165,7 +165,7 @@ static void tests_spiffs_format(void) res = vfs_mount(&_test_spiffs_mount); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_spiffs_mount); + res = vfs_umount(&_test_spiffs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); /* 2. format a valid file system */ @@ -176,7 +176,7 @@ static void tests_spiffs_format(void) static void tests_spiffs_mount_umount(void) { int res; - res = vfs_umount(&_test_spiffs_mount); + res = vfs_umount(&_test_spiffs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); res = vfs_mount(&_test_spiffs_mount); @@ -387,7 +387,7 @@ static void tests_spiffs_statvfs(void) static void tests_spiffs_partition(void) { - vfs_umount(&_test_spiffs_mount); + vfs_umount(&_test_spiffs_mount, false); spiffs_desc.base_addr = _dev->page_size * _dev->pages_per_sector; spiffs_desc.block_count = 2; diff --git a/tests/pkg_tinyvcdiff/main.c b/tests/pkg_tinyvcdiff/main.c index 2285622dc7..6a0a8a7f91 100644 --- a/tests/pkg_tinyvcdiff/main.c +++ b/tests/pkg_tinyvcdiff/main.c @@ -116,7 +116,7 @@ static void test_tinyvcdiff_vfs(void) TEST_ASSERT_EQUAL_INT(0, memcmp(target_bin, target_buf, sizeof(target_buf))); vfs_close(target_fd); - vfs_umount(&mnt); + vfs_umount(&mnt, false); } Test *tests_tinyvcdiff(void) diff --git a/tests/unittests/tests-vfs/tests-vfs-dir-ops.c b/tests/unittests/tests-vfs/tests-vfs-dir-ops.c index fa56a04ff7..cfa38cefa9 100644 --- a/tests/unittests/tests-vfs/tests-vfs-dir-ops.c +++ b/tests/unittests/tests-vfs/tests-vfs-dir-ops.c @@ -81,7 +81,7 @@ static void teardown(void) vfs_closedir(&_test_dir); _test_vfs_dir_op_status = -1; } - vfs_umount(&_test_vfs_mount_null); + vfs_umount(&_test_vfs_mount_null, false); } static void test_vfs_null_dir_ops_opendir(void) diff --git a/tests/unittests/tests-vfs/tests-vfs-file-ops.c b/tests/unittests/tests-vfs/tests-vfs-file-ops.c index 4b5ae05ea5..1a588de6f5 100644 --- a/tests/unittests/tests-vfs/tests-vfs-file-ops.c +++ b/tests/unittests/tests-vfs/tests-vfs-file-ops.c @@ -80,7 +80,7 @@ static void teardown(void) vfs_close(_test_vfs_file_op_my_fd); _test_vfs_file_op_my_fd = -1; } - vfs_umount(&_test_vfs_mount_null); + vfs_umount(&_test_vfs_mount_null, false); } static void test_vfs_null_file_ops_close(void) diff --git a/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c b/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c index 05070109a4..3dfd152067 100644 --- a/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c +++ b/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c @@ -71,7 +71,7 @@ static void setup(void) static void teardown(void) { - vfs_umount(&_test_vfs_mount_null); + vfs_umount(&_test_vfs_mount_null, false); } static void test_vfs_null_fs_ops_mount(void) @@ -85,9 +85,9 @@ static void test_vfs_null_fs_ops_mount(void) static void test_vfs_null_fs_ops_umount(void) { TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res); - int res = vfs_umount(&_test_vfs_mount_null); + int res = vfs_umount(&_test_vfs_mount_null, false); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_vfs_mount_null); + res = vfs_umount(&_test_vfs_mount_null, false); /* Not mounted */ TEST_ASSERT_EQUAL_INT(-EINVAL, res); } diff --git a/tests/unittests/tests-vfs/tests-vfs-mount-constfs.c b/tests/unittests/tests-vfs/tests-vfs-mount-constfs.c index 601ae74fc8..08872f20fa 100644 --- a/tests/unittests/tests-vfs/tests-vfs-mount-constfs.c +++ b/tests/unittests/tests-vfs/tests-vfs-mount-constfs.c @@ -72,7 +72,7 @@ static void test_vfs_mount_umount(void) int res; res = vfs_mount(&_test_vfs_mount); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_vfs_mount); + res = vfs_umount(&_test_vfs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); } @@ -89,9 +89,9 @@ static void test_vfs_mount__invalid(void) static void test_vfs_umount__invalid_mount(void) { int res; - res = vfs_umount(NULL); + res = vfs_umount(NULL, false); TEST_ASSERT(res < 0); - res = vfs_umount(&_test_vfs_mount); + res = vfs_umount(&_test_vfs_mount, false); TEST_ASSERT(res < 0); } @@ -124,7 +124,7 @@ static void test_vfs_constfs_open(void) TEST_ASSERT_EQUAL_INT(0, res); } - res = vfs_umount(&_test_vfs_mount); + res = vfs_umount(&_test_vfs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); } @@ -169,7 +169,7 @@ static void test_vfs_constfs_read_lseek(void) res = vfs_close(fd); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_vfs_mount); + res = vfs_umount(&_test_vfs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); } @@ -199,7 +199,7 @@ static void test_vfs_constfs__posix(void) res = close(fd); TEST_ASSERT_EQUAL_INT(0, res); - res = vfs_umount(&_test_vfs_mount); + res = vfs_umount(&_test_vfs_mount, false); TEST_ASSERT_EQUAL_INT(0, res); } #endif diff --git a/tests/vfs_iterate_mount/main.c b/tests/vfs_iterate_mount/main.c index 6d415fa4bd..e5c47e51a1 100644 --- a/tests/vfs_iterate_mount/main.c +++ b/tests/vfs_iterate_mount/main.c @@ -96,7 +96,7 @@ int main(void) { /* N1N2, unmount 3, N4E */ iter_and_report(&iter); iter_and_report(&iter); - res |= vfs_umount(&mount3); + res |= vfs_umount(&mount3, false); iter_and_report(&iter); iter_and_report(&iter); @@ -104,19 +104,19 @@ int main(void) { /* 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); + res |= vfs_umount(&mount2, false); iter_and_report(&iter); res |= vfs_mount(&mount3); iter_and_report(&iter); - res |= vfs_umount(&mount1); + res |= vfs_umount(&mount1, false); 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); + res |= vfs_umount(&mount1, false); + res |= vfs_umount(&mount3, false); + res |= vfs_umount(&mount4, false); printf("All unmounted\n"); /* Only O */ From 2658eb151b0e33f406fd5272c0a96fad95a205fb Mon Sep 17 00:00:00 2001 From: Martine Lenders Date: Tue, 28 Feb 2023 17:36:29 +0100 Subject: [PATCH 3/3] tests: test the -EBUSY case for vfs_umount() --- .../unittests/tests-vfs/tests-vfs-file-system-ops.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c b/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c index 3dfd152067..8981d76bed 100644 --- a/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c +++ b/tests/unittests/tests-vfs/tests-vfs-file-system-ops.c @@ -72,6 +72,7 @@ static void setup(void) static void teardown(void) { vfs_umount(&_test_vfs_mount_null, false); + atomic_store(&_test_vfs_mount_null.open_files, 0); } static void test_vfs_null_fs_ops_mount(void) @@ -92,6 +93,17 @@ static void test_vfs_null_fs_ops_umount(void) TEST_ASSERT_EQUAL_INT(-EINVAL, res); } +static void test_vfs_null_fs_ops_umount__EBUSY(void) +{ + TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res); + atomic_fetch_add(&_test_vfs_mount_null.open_files, 1); + int res = vfs_umount(&_test_vfs_mount_null, false); + TEST_ASSERT_EQUAL_INT(-EBUSY, res); + /* force unmount */ + res = vfs_umount(&_test_vfs_mount_null, true); + TEST_ASSERT_EQUAL_INT(0, res); +} + static void test_vfs_null_fs_ops_rename(void) { TEST_ASSERT_EQUAL_INT(0, _test_vfs_fs_op_mount_res); @@ -151,6 +163,7 @@ Test *tests_vfs_null_file_system_ops_tests(void) EMB_UNIT_TESTFIXTURES(fixtures) { new_TestFixture(test_vfs_null_fs_ops_mount), new_TestFixture(test_vfs_null_fs_ops_umount), + new_TestFixture(test_vfs_null_fs_ops_umount__EBUSY), new_TestFixture(test_vfs_null_fs_ops_rename), new_TestFixture(test_vfs_null_fs_ops_unlink), new_TestFixture(test_vfs_null_fs_ops_mkdir),