From 1d92631ef0aa750461d31c1e16ae64f4f6ea2259 Mon Sep 17 00:00:00 2001 From: Emery Hemingway Date: Fri, 30 Oct 2015 12:33:26 +0100 Subject: [PATCH] VFS, File_system: Not_empty Unlink returns an error for non-empty directories when the backend does not support recursive unlinking. Fixes #1750 --- repos/libports/src/lib/libc/vfs_plugin.cc | 7 +- .../file_system_session/file_system_session.h | 10 ++- repos/os/include/vfs/directory_service.h | 3 +- repos/os/include/vfs/fs_file_system.h | 5 +- repos/os/src/test/vfs_stress/main.cc | 82 +++++++++++++------ repos/ports/src/lib/libc_noux/plugin.cc | 7 +- 6 files changed, 78 insertions(+), 36 deletions(-) diff --git a/repos/libports/src/lib/libc/vfs_plugin.cc b/repos/libports/src/lib/libc/vfs_plugin.cc index 531077fbd..4a1117f9a 100644 --- a/repos/libports/src/lib/libc/vfs_plugin.cc +++ b/repos/libports/src/lib/libc/vfs_plugin.cc @@ -762,9 +762,10 @@ int Libc::Vfs_plugin::unlink(char const *path) typedef Vfs::Directory_service::Unlink_result Result; switch (_root_dir.unlink(path)) { - case Result::UNLINK_ERR_NO_ENTRY: errno = ENOENT; return -1; - case Result::UNLINK_ERR_NO_PERM: errno = EPERM; return -1; - case Result::UNLINK_OK: break; + case Result::UNLINK_ERR_NO_ENTRY: errno = ENOENT; return -1; + case Result::UNLINK_ERR_NO_PERM: errno = EPERM; return -1; + case Result::UNLINK_ERR_NOT_EMPTY: errno = ENOTEMPTY; return -1; + case Result::UNLINK_OK: break; } return 0; } diff --git a/repos/os/include/file_system_session/file_system_session.h b/repos/os/include/file_system_session/file_system_session.h index 1b79ea56b..6ccbf9554 100644 --- a/repos/os/include/file_system_session/file_system_session.h +++ b/repos/os/include/file_system_session/file_system_session.h @@ -60,6 +60,7 @@ namespace File_system { class Invalid_handle : Exception { }; class Invalid_name : Exception { }; class Size_limit_reached : Exception { }; + class Not_empty : Exception { }; struct Session; } @@ -275,6 +276,12 @@ struct File_system::Session : public Genode::Session /** * Delete file or directory + * + * \throw Permission_denied + * \throw Invalid_name + * \throw Lookup_failed + * \throw Not_empty argument is a non-empty directory and + * the backend does not support recursion */ virtual void unlink(Dir_handle, Name const &) = 0; @@ -336,7 +343,8 @@ struct File_system::Session : public Genode::Session GENODE_RPC(Rpc_status, Status, status, Node_handle); GENODE_RPC(Rpc_control, void, control, Node_handle, Control); GENODE_RPC_THROW(Rpc_unlink, void, unlink, - GENODE_TYPE_LIST(Permission_denied, Invalid_name, Lookup_failed), + GENODE_TYPE_LIST(Permission_denied, Invalid_name, + Lookup_failed, Not_empty), Dir_handle, Name const &); GENODE_RPC_THROW(Rpc_truncate, void, truncate, GENODE_TYPE_LIST(Permission_denied, Invalid_handle, No_space), diff --git a/repos/os/include/vfs/directory_service.h b/repos/os/include/vfs/directory_service.h index cda7e11d5..90e30a572 100644 --- a/repos/os/include/vfs/directory_service.h +++ b/repos/os/include/vfs/directory_service.h @@ -121,7 +121,8 @@ struct Vfs::Directory_service ** Unlink ** ************/ - enum Unlink_result { UNLINK_ERR_NO_ENTRY, UNLINK_ERR_NO_PERM, UNLINK_OK }; + enum Unlink_result { UNLINK_ERR_NO_ENTRY, UNLINK_ERR_NO_PERM, + UNLINK_ERR_NOT_EMPTY, UNLINK_OK }; virtual Unlink_result unlink(char const *path) = 0; diff --git a/repos/os/include/vfs/fs_file_system.h b/repos/os/include/vfs/fs_file_system.h index 6cd5c89f2..b722507ba 100644 --- a/repos/os/include/vfs/fs_file_system.h +++ b/repos/os/include/vfs/fs_file_system.h @@ -349,8 +349,9 @@ class Vfs::Fs_file_system : public File_system _fs.unlink(dir, file_name.base() + 1); } - catch (::File_system::Permission_denied) { return UNLINK_ERR_NO_PERM; } - catch (...) { return UNLINK_ERR_NO_ENTRY; } + catch (::File_system::Permission_denied) { return UNLINK_ERR_NO_PERM; } + catch (::File_system::Not_empty) { return UNLINK_ERR_NOT_EMPTY; } + catch (...) { return UNLINK_ERR_NO_ENTRY; } return UNLINK_OK; } diff --git a/repos/os/src/test/vfs_stress/main.cc b/repos/os/src/test/vfs_stress/main.cc index f3ae06ee3..eac3e0fa4 100644 --- a/repos/os/src/test/vfs_stress/main.cc +++ b/repos/os/src/test/vfs_stress/main.cc @@ -130,6 +130,8 @@ inline void assert_unlink(Vfs::Directory_service::Unlink_result r) PERR("UNLINK_ERR_NO_ENTRY"); break; case Result::UNLINK_ERR_NO_PERM: PERR("UNLINK_ERR_NO_PERM"); break; + case Result::UNLINK_ERR_NOT_EMPTY: + PERR("UNLINK_ERR_NOT_EMPTY"); break; } throw Exception(); } @@ -145,15 +147,18 @@ struct Stress_thread : public Genode::Thread<4*1024*sizeof(Genode::addr_t)> Vfs::file_size count; Vfs::File_system &vfs; - Stress_thread(Vfs::File_system &vfs, char const *parent) - : Thread(parent), path(parent), count(0), vfs(vfs) { } + Stress_thread(Vfs::File_system &vfs, char const *parent, Affinity::Location affinity) + : Thread(parent), path(parent), count(0), vfs(vfs) + { + env()->cpu_session()->affinity(cap(), affinity); + } }; struct Mkdir_thread : public Stress_thread { - Mkdir_thread(Vfs::File_system &vfs, char const *parent) - : Stress_thread(vfs, parent) { start(); } + Mkdir_thread(Vfs::File_system &vfs, char const *parent, Affinity::Location affinity) + : Stress_thread(vfs, parent, affinity) { start(); } void mkdir_b(int depth) { @@ -201,8 +206,8 @@ struct Mkdir_thread : public Stress_thread struct Populate_thread : public Stress_thread { - Populate_thread(Vfs::File_system &vfs, char const *parent) - : Stress_thread(vfs, parent) { start(); } + Populate_thread(Vfs::File_system &vfs, char const *parent, Affinity::Location affinity) + : Stress_thread(vfs, parent, affinity) { start(); } void populate(int depth) @@ -266,8 +271,8 @@ struct Populate_thread : public Stress_thread struct Write_thread : public Stress_thread { - Write_thread(Vfs::File_system &vfs, char const *parent) - : Stress_thread(vfs, parent) { start(); } + Write_thread(Vfs::File_system &vfs, char const *parent, Affinity::Location affinity) + : Stress_thread(vfs, parent, affinity) { start(); } void write(int depth) { @@ -334,8 +339,8 @@ struct Write_thread : public Stress_thread struct Read_thread : public Stress_thread { - Read_thread(Vfs::File_system &vfs, char const *parent) - : Stress_thread(vfs, parent) { start(); } + Read_thread(Vfs::File_system &vfs, char const *parent, Affinity::Location affinity) + : Stress_thread(vfs, parent, affinity) { start(); } void read(int depth) { @@ -404,8 +409,8 @@ struct Read_thread : public Stress_thread struct Unlink_thread : public Stress_thread { - Unlink_thread(Vfs::File_system &vfs, char const *parent) - : Stress_thread(vfs, parent) { start(); } + Unlink_thread(Vfs::File_system &vfs, char const *parent, Affinity::Location affinity) + : Stress_thread(vfs, parent, affinity) { start(); } void empty_dir(char const *path) { @@ -427,6 +432,7 @@ struct Unlink_thread : public Stress_thread default: try { assert_unlink(vfs.unlink(subpath.base())); + ++count; } catch (...) { PERR("unlink %s failed", subpath.base()); throw; @@ -438,10 +444,32 @@ struct Unlink_thread : public Stress_thread void entry() { + typedef Vfs::Directory_service::Unlink_result Result; try { - empty_dir(path.base()); - vfs.unlink(path.base()); - } catch (...) { } + Result r = vfs.unlink(path.base()); + switch (r) { + case Result::UNLINK_ERR_NOT_EMPTY: + PLOG("recursive unlink not supported"); + empty_dir(path.base()); + r = vfs.unlink(path.base()); + + case Result::UNLINK_OK: + PLOG("recursive unlink supported"); + ++count; + return; + + default: break; + } + assert_unlink(r); + } catch (...) { + PERR("unlink %s failed", path.base()); + } + } + + int wait() + { + join(); + return count; } }; @@ -468,6 +496,8 @@ int main() /* populate the directory file system at / */ vfs_root.num_dirent("/"); + Affinity::Space space = env()->cpu_session()->affinity_space(); + size_t initial_consumption = env()->ram_session()->used(); /************************** @@ -483,7 +513,7 @@ int main() snprintf(path, 3, "/%lu", i); vfs_root.mkdir(path, 0); threads[i] = new (Genode::env()->heap()) - Mkdir_thread(vfs_root, path); + Mkdir_thread(vfs_root, path, space.location_of_index(i)); } for (size_t i = 0; i < thread_count; ++i) { @@ -510,9 +540,8 @@ int main() for (size_t i = 0; i < thread_count; ++i) { snprintf(path, 3, "/%lu", i); - vfs_root.mkdir(path, 0); threads[i] = new (Genode::env()->heap()) - Populate_thread(vfs_root, path); + Populate_thread(vfs_root, path, space.location_of_index(i)); } for (size_t i = 0; i < thread_count; ++i) { @@ -549,9 +578,8 @@ int main() for (size_t i = 0; i < thread_count; ++i) { snprintf(path, 3, "/%lu", i); - vfs_root.mkdir(path, 0); threads[i] = new (Genode::env()->heap()) - Write_thread(vfs_root, path); + Write_thread(vfs_root, path, space.location_of_index(i)); } for (size_t i = 0; i < thread_count; ++i) { @@ -588,9 +616,8 @@ int main() for (size_t i = 0; i < thread_count; ++i) { snprintf(path, 3, "/%lu", i); - vfs_root.mkdir(path, 0); threads[i] = new (Genode::env()->heap()) - Read_thread(vfs_root, path); + Read_thread(vfs_root, path, space.location_of_index(i)); } for (size_t i = 0; i < thread_count; ++i) { @@ -620,19 +647,20 @@ int main() return 0; } { + Vfs::file_size count = 0; + Unlink_thread *threads[thread_count]; PLOG("unlink files..."); elapsed_ms = timer.elapsed_ms(); for (size_t i = 0; i < thread_count; ++i) { snprintf(path, 3, "/%lu", i); - vfs_root.mkdir(path, 0); threads[i] = new (Genode::env()->heap()) - Unlink_thread(vfs_root, path); + Unlink_thread(vfs_root, path, space.location_of_index(i)); } for (size_t i = 0; i < thread_count; ++i) { - threads[i]->join(); + count += threads[i]->wait(); destroy(Genode::env()->heap(), threads[i]); } @@ -640,8 +668,8 @@ int main() vfs_root.sync("/"); - PINF("unlink in %lums, %luKB consumed", - elapsed_ms, env()->ram_session()->used()/1024); + PINF("unlinked %llu files in %lums, %luKB consumed", + count, elapsed_ms, env()->ram_session()->used()/1024); } PINF("total: %lums, %luKB consumed", diff --git a/repos/ports/src/lib/libc_noux/plugin.cc b/repos/ports/src/lib/libc_noux/plugin.cc index 07c0477fc..cec8b2562 100644 --- a/repos/ports/src/lib/libc_noux/plugin.cc +++ b/repos/ports/src/lib/libc_noux/plugin.cc @@ -1634,9 +1634,12 @@ namespace { if (!noux_syscall(Noux::Session::SYSCALL_UNLINK)) { PWRN("unlink syscall failed for path \"%s\"", path); + typedef Vfs::Directory_service::Unlink_result Result; switch (sysio()->error.unlink) { - case Vfs::Directory_service::UNLINK_ERR_NO_ENTRY: errno = ENOENT; break; - default: errno = EPERM; break; + case Result::UNLINK_ERR_NO_ENTRY: errno = ENOENT; break; + case Result::UNLINK_ERR_NOT_EMPTY: errno = ENOTEMPTY; break; + case Result::UNLINK_ERR_NO_PERM: errno = EPERM; break; + case Result::UNLINK_OK: break; /* only here to complete the enumeration */ } return -1; }