From 3c9db84d1a8170d383e8fa96438bc2318309e973 Mon Sep 17 00:00:00 2001 From: Henri Casanova Date: Thu, 10 Oct 2024 15:44:05 -1000 Subject: [PATCH] Minor code cleanup Added more tests --- include/fsmod/FileSystem.hpp | 6 +++--- include/fsmod/Partition.hpp | 4 ++-- src/FileMetadata.cpp | 1 - src/FileSystem.cpp | 6 +++--- src/JBODStorage.cpp | 6 +++--- test/caching_test.cpp | 33 +++++++++++++++++++++++++++++++++ test/register_test.cpp | 1 + 7 files changed, 45 insertions(+), 12 deletions(-) diff --git a/include/fsmod/FileSystem.hpp b/include/fsmod/FileSystem.hpp index e8398b7..68b6397 100644 --- a/include/fsmod/FileSystem.hpp +++ b/include/fsmod/FileSystem.hpp @@ -28,7 +28,7 @@ namespace simgrid::fsmod { static xbt::Extension EXTENSION_ID; void register_file_system(const std::shared_ptr& fs); - const std::map, std::less<>>& get_all_file_systems() const { return file_systems_;} + [[nodiscard]] const std::map, std::less<>>& get_all_file_systems() const { return file_systems_;} }; /** @@ -44,10 +44,10 @@ namespace simgrid::fsmod { static std::shared_ptr create(const std::string &name, int max_num_open_files = 1024); static const std::map, std::less<>>& - get_file_systems_by_actor(s4u::ActorPtr actor); + get_file_systems_by_actor(const s4u::ActorPtr& actor); static const std::map, std::less<>>& get_file_systems_by_netzone(const s4u::NetZone* netzone); - static void register_file_system(const s4u::NetZone* netzone, std::shared_ptr fs); + static void register_file_system(const s4u::NetZone* netzone, const std::shared_ptr& fs); /** * @brief Retrieves the file system's name diff --git a/include/fsmod/Partition.hpp b/include/fsmod/Partition.hpp index 0f4b2b0..445b1a8 100644 --- a/include/fsmod/Partition.hpp +++ b/include/fsmod/Partition.hpp @@ -97,7 +97,7 @@ namespace simgrid::fsmod { std::string name_; - FileSystem *file_system_;FileSystem *file_system; + FileSystem *file_system_; std::shared_ptr storage_; sg_size_t size_ = 0; sg_size_t free_space_ = 0; @@ -110,7 +110,7 @@ namespace simgrid::fsmod { void create_new_directory(const std::string& dir_path); [[nodiscard]] bool directory_exists(const std::string& dir_path) const { return content_.find(dir_path) != content_.end(); } - std::set> list_files_in_directory(const std::string &dir_path) const; + [[nodiscard]] std::set> list_files_in_directory(const std::string &dir_path) const; void delete_directory(const std::string &dir_path); [[nodiscard]] FileMetadata* get_file_metadata(const std::string& dir_path, const std::string& file_name) const; diff --git a/src/FileMetadata.cpp b/src/FileMetadata.cpp index 991712e..dbbee5f 100644 --- a/src/FileMetadata.cpp +++ b/src/FileMetadata.cpp @@ -8,7 +8,6 @@ #include "fsmod/Partition.hpp" #include "fsmod/FileMetadata.hpp" -#include "fsmod/FileSystemException.hpp" namespace simgrid::fsmod { diff --git a/src/FileSystem.cpp b/src/FileSystem.cpp index d183f28..27d5fbe 100644 --- a/src/FileSystem.cpp +++ b/src/FileSystem.cpp @@ -122,7 +122,7 @@ namespace simgrid::fsmod { * @param netzone The SimGrid NetZone * @param fs The FSMOD file system */ - void FileSystem::register_file_system(const s4u::NetZone* netzone, std::shared_ptr fs) { + void FileSystem::register_file_system(const s4u::NetZone* netzone, const std::shared_ptr& fs) { if (not FileSystemNetZoneImplExtension::EXTENSION_ID.valid()) { // This is the first time we register a FileSystem, register the NetZoneImpls extension properly FileSystemNetZoneImplExtension::EXTENSION_ID = @@ -145,7 +145,7 @@ namespace simgrid::fsmod { * @return A file system map, using names as keys */ const std::map, std::less<>>& - FileSystem::get_file_systems_by_actor(s4u::ActorPtr actor) { + FileSystem::get_file_systems_by_actor(const s4u::ActorPtr& actor) { const kernel::routing::NetZoneImpl* netzone_impl; if (not actor || simgrid::s4u::Actor::is_maestro()) { netzone_impl = s4u::Engine::get_instance()->get_netzone_root()->get_impl(); @@ -271,7 +271,7 @@ namespace simgrid::fsmod { /** - * @brief Set the evictability of a file so that it can or cannot be evicted + * @brief Set the evictable status of a file so that it can or cannot be evicted * if stored on a partition that implements caching * @param full_path: an absolute file path * @param evictable: true if the file should be evictable, false if not diff --git a/src/JBODStorage.cpp b/src/JBODStorage.cpp index 587ed3a..44463b9 100644 --- a/src/JBODStorage.cpp +++ b/src/JBODStorage.cpp @@ -50,7 +50,7 @@ namespace simgrid::fsmod { s4u::IoPtr JBODStorage::read_async(sg_size_t size) { // Determine what to read from each disk - sg_size_t read_size = 0; + sg_size_t read_size; std::vector targets; std::stringstream debug_msg; @@ -138,7 +138,7 @@ namespace simgrid::fsmod { // Determine what to write on each individual disk according to RAID level and which disk will store the // parity block - sg_size_t write_size = 0; + sg_size_t write_size; switch(raid_level_) { case RAID::RAID0: write_size = size / num_disks_; @@ -162,7 +162,7 @@ namespace simgrid::fsmod { } // Compute the parity block (if any) - s4u::ExecPtr parity_block_comp = nullptr; + s4u::ExecPtr parity_block_comp; if (raid_level_ == RAID::RAID4 || raid_level_ == RAID::RAID5 || raid_level_ == RAID::RAID6) { // Assume 1 flop per byte to write per parity block and two for RAID6. diff --git a/test/caching_test.cpp b/test/caching_test.cpp index f78be5c..e3886ce 100644 --- a/test/caching_test.cpp +++ b/test/caching_test.cpp @@ -88,6 +88,39 @@ TEST_F(CachingTest, FIFOBasics) { }); } +TEST_F(CachingTest, FIFOBasicsWithUnevictableFiles) { + DO_TEST_WITH_FORK([this]() { + this->setup_platform(); + // Create one actor (for this test we could likely do it all in the maestro but what the hell) + sg4::Actor::create("TestActor", host_, [this]() { + std::shared_ptr file; + XBT_INFO("Create a 20MB file at /dev/fifo/20mb.txt"); + ASSERT_NO_THROW(fs_->create_file("/dev/fifo/20mb.txt", "20MB")); + XBT_INFO("Making the 20MB file at /dev/fifo/20mb.txt unevictable"); + ASSERT_THROW(fs_->make_file_evictable("/dev/fifo/bogus.txt", false), sgfs::FileNotFoundException); + ASSERT_NO_THROW(fs_->make_file_evictable("/dev/fifo/20mb.txt", false)); + XBT_INFO("Create a 60MB file at /dev/a/60mb.txt"); + ASSERT_NO_THROW(fs_->create_file("/dev/fifo/60mb.txt", "60MB")); + XBT_INFO("Create a 30MB file at /dev/fifo/60mb.txt"); + ASSERT_NO_THROW(fs_->create_file("/dev/fifo/30mb.txt", "30MB")); + XBT_INFO("Check that files are as they should be"); + ASSERT_TRUE(fs_->file_exists("/dev/fifo/20mb.txt")); + ASSERT_FALSE(fs_->file_exists("/dev/fifo/60mb.txt")); + ASSERT_TRUE(fs_->file_exists("/dev/fifo/30mb.txt")); + ASSERT_NO_THROW(fs_->make_file_evictable("/dev/fifo/20mb.txt", true)); + ASSERT_NO_THROW(fs_->create_file("/dev/fifo/60mb.txt", "60MB")); + ASSERT_FALSE(fs_->file_exists("/dev/fifo/20mb.txt")); + ASSERT_TRUE(fs_->file_exists("/dev/fifo/60mb.txt")); + ASSERT_TRUE(fs_->file_exists("/dev/fifo/30mb.txt")); + + + }); + + // Run the simulation + ASSERT_NO_THROW(sg4::Engine::get_instance()->run()); + }); +} + TEST_F(CachingTest, FIFODontEvictOpenFiles) { DO_TEST_WITH_FORK([this]() { this->setup_platform(); diff --git a/test/register_test.cpp b/test/register_test.cpp index 859a450..598de44 100644 --- a/test/register_test.cpp +++ b/test/register_test.cpp @@ -73,6 +73,7 @@ TEST_F(RegisterTest, RetrieveByActor) { ASSERT_EQ(accessible_file_systems.size(), 2); ASSERT_NO_THROW(fs = accessible_file_systems["my_fs_0"]); ASSERT_EQ(fs->get_name(), "my_fs_0"); + ASSERT_EQ(strcmp(fs->get_cname(), "my_fs_0"), 0); ASSERT_NO_THROW(fs = accessible_file_systems["my_extra_fs"]); ASSERT_EQ(fs->get_name(), "my_extra_fs"); } else {