From: Frederic Suter <frederic.suter@cc.in2p3.fr>
Date: Mon, 9 Sep 2019 11:53:37 +0000 (+0200)
Subject: simplify disk management
X-Git-Tag: v3.24~101
X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/2c35f563c1eb2624079c12f7ab27453e809ff599?ds=sidebyside

simplify disk management
* disks are related to a given host, there is no need to maintain a
global map.
* No more map means no by_name methods
* This allows for same name for disks on different hosts
---

diff --git a/examples/s4u/io-async/s4u-io-async.cpp b/examples/s4u/io-async/s4u-io-async.cpp
index 31ae6d7cc6..597284fcc9 100644
--- a/examples/s4u/io-async/s4u-io-async.cpp
+++ b/examples/s4u/io-async/s4u-io-async.cpp
@@ -9,7 +9,7 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(s4u_test, "Messages specific for this s4u example")
 
 static void test(sg_size_t size)
 {
-  simgrid::s4u::Disk* disk = simgrid::s4u::Disk::by_name("Disk1");
+  simgrid::s4u::Disk* disk = simgrid::s4u::Host::current()->get_disks().front();
   XBT_INFO("Hello! read %llu bytes from %s", size, disk->get_cname());
 
   simgrid::s4u::IoPtr activity = disk->io_init(size, simgrid::s4u::Io::OpType::READ);
@@ -21,7 +21,7 @@ static void test(sg_size_t size)
 
 static void test_cancel(sg_size_t size)
 {
-  simgrid::s4u::Disk* disk = simgrid::s4u::Disk::by_name("Disk2");
+  simgrid::s4u::Disk* disk = simgrid::s4u::Host::current()->get_disks().front();
   XBT_INFO("Hello! write %llu bytes from %s", size, disk->get_cname());
 
   simgrid::s4u::IoPtr activity = disk->write_async(size);
diff --git a/examples/s4u/io-disk-raw/s4u-io-disk-raw.cpp b/examples/s4u/io-disk-raw/s4u-io-disk-raw.cpp
index a490816774..2152a10b53 100644
--- a/examples/s4u/io-disk-raw/s4u-io-disk-raw.cpp
+++ b/examples/s4u/io-disk-raw/s4u-io-disk-raw.cpp
@@ -14,15 +14,15 @@ static void host()
   /* - Display information on the disks mounted by the current host */
   XBT_INFO("*** Storage info on %s ***", simgrid::s4u::Host::current()->get_cname());
 
-  /* - Retrieve all mount points of current host */
+  /* - Retrieve all disks from current host */
   std::vector<simgrid::s4u::Disk*> const& disk_list = simgrid::s4u::Host::current()->get_disks();
 
   /* - For each disk mounted on host, display disk name and mount point */
   for (auto disk : disk_list)
     XBT_INFO("Disk name: %s", disk->get_cname());
 
-  /* - Write 400,000 bytes on Disk4 */
-  simgrid::s4u::Disk* disk = simgrid::s4u::Disk::by_name("Disk1");
+  /* - Write 400,000 bytes on Disk1 */
+  simgrid::s4u::Disk* disk = disk_list.front();
   sg_size_t write          = disk->write(400000);
   XBT_INFO("Wrote %llu bytes on '%s'", write, disk->get_cname());
 
diff --git a/include/simgrid/s4u/Disk.hpp b/include/simgrid/s4u/Disk.hpp
index 29ea07db5a..70c5bf2366 100644
--- a/include/simgrid/s4u/Disk.hpp
+++ b/include/simgrid/s4u/Disk.hpp
@@ -31,7 +31,7 @@ class XBT_PUBLIC Disk : public xbt::Extendable<Disk> {
   friend kernel::resource::DiskImpl;
 
 public:
-  explicit Disk(const std::string& name, kernel::resource::DiskImpl* pimpl);
+  explicit Disk(const std::string& name, kernel::resource::DiskImpl* pimpl) : pimpl_(pimpl), name_(name) {}
 
 protected:
   virtual ~Disk() = default;
@@ -44,10 +44,6 @@ public:
   /** @brief Callback signal fired when a Storage's state changes */
   static xbt::signal<void(Disk const&)> on_state_change;
 
-  /** Retrieve a Storage by its name. It must exist in the platform file */
-  static Disk* by_name(const std::string& name);
-  static Disk* by_name_or_null(const std::string& name);
-
   /** @brief Retrieves the name of that storage as a C++ string */
   std::string const& get_name() const { return name_; }
   /** @brief Retrieves the name of that storage as a C string */
diff --git a/include/simgrid/s4u/Engine.hpp b/include/simgrid/s4u/Engine.hpp
index acca2fb055..37bc47157a 100644
--- a/include/simgrid/s4u/Engine.hpp
+++ b/include/simgrid/s4u/Engine.hpp
@@ -96,8 +96,6 @@ protected:
   void host_unregister(const std::string& name);
   void link_register(const std::string& name, Link* link);
   void link_unregister(const std::string& name);
-  void disk_register(const std::string& name, Disk* storage);
-  void disk_unregister(const std::string& name);
   void storage_register(const std::string& name, Storage* storage);
   void storage_unregister(const std::string& name);
   void netpoint_register(simgrid::kernel::routing::NetPoint* card);
@@ -122,9 +120,6 @@ public:
   std::vector<ActorPtr> get_all_actors();
   std::vector<ActorPtr> get_filtered_actors(const std::function<bool(ActorPtr)>& filter);
 
-  Disk* disk_by_name(const std::string& name);
-  Disk* disk_by_name_or_null(const std::string& name);
-
   size_t get_storage_count();
   std::vector<Storage*> get_all_storages();
   Storage* storage_by_name(const std::string& name);
diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp
index 16c5ffe862..e28a95bebf 100644
--- a/include/simgrid/s4u/Host.hpp
+++ b/include/simgrid/s4u/Host.hpp
@@ -116,6 +116,7 @@ public:
   int get_pstate() const;
 
   std::vector<Disk*> get_disks() const;
+
   std::vector<const char*> get_attached_storages() const;
 
   /** Get an associative list [mount point]->[Storage] of all local mount points.
diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index a4405ff0d0..4d98ab4ed8 100644
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -27,10 +27,6 @@ EngineImpl::~EngineImpl()
   for (auto const& kv : netpoints_)
     delete kv.second;
 
-  for (auto const& kv : disks_)
-    if (kv.second)
-      kv.second->get_impl()->destroy();
-
   for (auto const& kv : storages_)
     if (kv.second)
       kv.second->get_impl()->destroy();
diff --git a/src/kernel/EngineImpl.hpp b/src/kernel/EngineImpl.hpp
index 2a197f51ce..f82414901d 100644
--- a/src/kernel/EngineImpl.hpp
+++ b/src/kernel/EngineImpl.hpp
@@ -15,7 +15,6 @@ namespace kernel {
 class EngineImpl {
   std::map<std::string, s4u::Host*> hosts_;
   std::map<std::string, s4u::Link*> links_;
-  std::map<std::string, s4u::Disk*> disks_;
   std::map<std::string, s4u::Storage*> storages_;
   std::unordered_map<std::string, routing::NetPoint*> netpoints_;
   friend s4u::Engine;
diff --git a/src/s4u/s4u_Disk.cpp b/src/s4u/s4u_Disk.cpp
index b543f9499e..c9477e434d 100644
--- a/src/s4u/s4u_Disk.cpp
+++ b/src/s4u/s4u_Disk.cpp
@@ -20,21 +20,6 @@ xbt::signal<void(Disk&)> Disk::on_creation;
 xbt::signal<void(Disk const&)> Disk::on_destruction;
 xbt::signal<void(Disk const&)> Disk::on_state_change;
 
-Disk::Disk(const std::string& name, kernel::resource::DiskImpl* pimpl) : pimpl_(pimpl), name_(name)
-{
-  Engine::get_instance()->disk_register(name_, this);
-}
-
-Disk* Disk::by_name(const std::string& name)
-{
-  return Engine::get_instance()->disk_by_name(name);
-}
-
-Disk* Disk::by_name_or_null(const std::string& name)
-{
-  return Engine::get_instance()->disk_by_name_or_null(name);
-}
-
 const std::unordered_map<std::string, std::string>* Disk::get_properties() const
 {
   return pimpl_->get_properties();
diff --git a/src/s4u/s4u_Engine.cpp b/src/s4u/s4u_Engine.cpp
index c92bdaa769..db64283370 100644
--- a/src/s4u/s4u_Engine.cpp
+++ b/src/s4u/s4u_Engine.cpp
@@ -211,25 +211,6 @@ std::vector<Storage*> Engine::get_all_storages()
   return res;
 }
 
-/** @brief Find a disk from its name.
- *
- *  @throw std::invalid_argument if the searched disk does not exist.
- */
-Disk* Engine::disk_by_name(const std::string& name)
-{
-  if (pimpl->disks_.find(name) == pimpl->disks_.end())
-    throw std::invalid_argument(std::string("Disk not found: ") + name);
-
-  return pimpl->disks_.at(name);
-}
-
-/** @brief Find a disk from its name (or nullptr if that disk does not exist) */
-Disk* Engine::disk_by_name_or_null(const std::string& name)
-{
-  auto disk = pimpl->disks_.find(name);
-  return disk == pimpl->disks_.end() ? nullptr : disk->second;
-}
-
 /** @brief Find a storage from its name.
  *
  *  @throw std::invalid_argument if the searched storage does not exist.
@@ -259,16 +240,6 @@ void Engine::storage_unregister(const std::string& name)
   pimpl->storages_.erase(name);
 }
 
-void Engine::disk_register(const std::string& name, Disk* disk)
-{
-  pimpl->disks_[name] = disk;
-}
-
-void Engine::disk_unregister(const std::string& name)
-{
-  pimpl->disks_.erase(name);
-}
-
 /** @brief Returns the amount of links in the platform */
 size_t Engine::get_link_count()
 {
diff --git a/src/s4u/s4u_Host.cpp b/src/s4u/s4u_Host.cpp
index 02698af270..0bb59b510b 100644
--- a/src/s4u/s4u_Host.cpp
+++ b/src/s4u/s4u_Host.cpp
@@ -288,6 +288,7 @@ std::vector<Disk*> Host::get_disks() const
 {
   return kernel::actor::simcall([this] { return this->pimpl_->get_disks(); });
 }
+
 /**
  * @ingroup simix_storage_management
  * @brief Returns the list of storages attached to a host.
diff --git a/src/surf/HostImpl.cpp b/src/surf/HostImpl.cpp
index a749282b60..f1a1b1ed4d 100644
--- a/src/surf/HostImpl.cpp
+++ b/src/surf/HostImpl.cpp
@@ -92,6 +92,9 @@ HostImpl::~HostImpl()
   for (auto const& arg : actors_at_boot_)
     delete arg;
   actors_at_boot_.clear();
+
+  for (auto const& d : disks_)
+    d->destroy();
 }
 
 /** Re-starts all the actors that are marked as restartable.