More consistency in resource creation/destruction

diff --git a/include/simgrid/s4u/Host.hpp b/include/simgrid/s4u/Host.hpp
index d50d8f9..ae737ff 100644 (file)
--- a/include/simgrid/s4u/Host.hpp
+++ b/include/simgrid/s4u/Host.hpp
@@ -59,13 +59,13 @@ protected:
 public:
   /** Called on each newly created host */
   static xbt::signal<void(Host&)> on_creation;
-  /** Called just before destructing a host */
-  static xbt::signal<void(Host const&)> on_destruction;
   /** Called when the machine is turned on or off (called AFTER the change) */
   static xbt::signal<void(Host const&)> on_state_change;
   /** Called when the speed of the machine is changed (called AFTER the change)
    * (either because of a pstate switch or because of an external load event coming from the profile) */
   static xbt::signal<void(Host const&)> on_speed_change;
+  /** Called just before destructing a host */
+  static xbt::signal<void(Host const&)> on_destruction;
 
   virtual void destroy();
 #ifndef DOXYGEN

diff --git a/include/simgrid/s4u/Link.hpp b/include/simgrid/s4u/Link.hpp
index 4eb8cf6..8e144d9 100644 (file)
--- a/include/simgrid/s4u/Link.hpp
+++ b/include/simgrid/s4u/Link.hpp
@@ -141,9 +141,6 @@ public:
   /** @brief Callback signal fired when a new Link is created */
   static xbt::signal<void(Link&)> on_creation;
 
-  /** @brief Callback signal fired when a Link is destroyed */
-  static xbt::signal<void(Link const&)> on_destruction;
-
   /** @brief Callback signal fired when the state of a Link changes (when it is turned on or off) */
   static xbt::signal<void(Link const&)> on_state_change;
 
@@ -156,6 +153,9 @@ public:
   /** @brief Callback signal fired when a communication changes it state (ready/done/cancel) */
   static xbt::signal<void(kernel::resource::NetworkAction&, kernel::resource::Action::State)>
       on_communication_state_change;
+
+  /** @brief Callback signal fired when a Link is destroyed */
+  static xbt::signal<void(Link const&)> on_destruction;
 };
 
 /**

diff --git a/include/simgrid/s4u/VirtualMachine.hpp b/include/simgrid/s4u/VirtualMachine.hpp
index 76e8380..73f03e5 100644 (file)
--- a/include/simgrid/s4u/VirtualMachine.hpp
+++ b/include/simgrid/s4u/VirtualMachine.hpp
@@ -23,7 +23,6 @@ namespace s4u {
  */
 class XBT_PUBLIC VirtualMachine : public s4u::Host {
   vm::VirtualMachineImpl* const pimpl_vm_;
-  ~VirtualMachine() override;
 
 public:
   explicit VirtualMachine(const std::string& name, Host* physical_host, int core_amount);
@@ -60,6 +59,7 @@ public:
   VirtualMachine* set_bound(double bound);
 
   State get_state() const;
+  static xbt::signal<void(VirtualMachine&)> on_creation;
   static xbt::signal<void(VirtualMachine const&)> on_start;
   static xbt::signal<void(VirtualMachine const&)> on_started;
   static xbt::signal<void(VirtualMachine const&)> on_shutdown;
@@ -67,6 +67,7 @@ public:
   static xbt::signal<void(VirtualMachine const&)> on_resume;
   static xbt::signal<void(VirtualMachine const&)> on_migration_start;
   static xbt::signal<void(VirtualMachine const&)> on_migration_end;
+  static xbt::signal<void(VirtualMachine const&)> on_destruction;
 };
 } // namespace s4u
 } // namespace simgrid

diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp
index 4d5cf7e..4f74809 100644 (file)
--- a/src/kernel/EngineImpl.cpp
+++ b/src/kernel/EngineImpl.cpp
@@ -161,9 +161,8 @@ EngineImpl::~EngineImpl()
   for (auto const& kv : netpoints_)
     delete kv.second;
 
-  for (auto const& kv : links_)
-    if (kv.second)
-      kv.second->destroy();
+  while (not links_.empty())
+    links_.begin()->second->destroy();
 
   for (auto const& kv : mailboxes_)
     delete kv.second;

diff --git a/src/plugins/dirty_page_tracking.cpp b/src/plugins/dirty_page_tracking.cpp
index 98cf2a9..6b643cf 100644 (file)
--- a/src/plugins/dirty_page_tracking.cpp
+++ b/src/plugins/dirty_page_tracking.cpp
@@ -67,14 +67,14 @@ double DirtyPageTrackingExt::computed_flops_lookup()
 } // namespace vm
 } // namespace simgrid
 
-static void on_virtual_machine_creation(simgrid::vm::VirtualMachineImpl& vm)
+static void on_virtual_machine_creation(simgrid::s4u::VirtualMachine& vm)
 {
-  vm.extension_set<simgrid::vm::DirtyPageTrackingExt>(new simgrid::vm::DirtyPageTrackingExt());
+  vm.get_vm_impl()->extension_set<simgrid::vm::DirtyPageTrackingExt>(new simgrid::vm::DirtyPageTrackingExt());
 }
 
 static void on_exec_creation(simgrid::s4u::Exec const& e)
 {
-  auto exec                        = static_cast<simgrid::kernel::activity::ExecImpl*>(e.get_impl());
+  auto exec                              = static_cast<simgrid::kernel::activity::ExecImpl*>(e.get_impl());
   const simgrid::s4u::VirtualMachine* vm = dynamic_cast<simgrid::s4u::VirtualMachine*>(exec->get_host());
   if (vm == nullptr)
     return;
@@ -107,7 +107,7 @@ void sg_vm_dirty_page_tracking_init()
   if (not simgrid::vm::DirtyPageTrackingExt::EXTENSION_ID.valid()) {
     simgrid::vm::DirtyPageTrackingExt::EXTENSION_ID =
         simgrid::vm::VirtualMachineImpl::extension_create<simgrid::vm::DirtyPageTrackingExt>();
-    simgrid::vm::VirtualMachineImpl::on_creation.connect(&on_virtual_machine_creation);
+    simgrid::s4u::VirtualMachine::on_creation.connect(&on_virtual_machine_creation);
     simgrid::s4u::Exec::on_start.connect(&on_exec_creation);
     simgrid::s4u::Exec::on_completion.connect(&on_exec_completion);
   }

diff --git a/src/plugins/vm/VirtualMachineImpl.cpp b/src/plugins/vm/VirtualMachineImpl.cpp
index 0f389ca..6fe26b7 100644 (file)
--- a/src/plugins/vm/VirtualMachineImpl.cpp
+++ b/src/plugins/vm/VirtualMachineImpl.cpp
@@ -40,11 +40,6 @@ namespace simgrid {
 template class xbt::Extendable<vm::VirtualMachineImpl>;
 
 namespace vm {
-/*************
- * Callbacks *
- *************/
-xbt::signal<void(VirtualMachineImpl&)> VirtualMachineImpl::on_creation;
-xbt::signal<void(VirtualMachineImpl const&)> VirtualMachineImpl::on_destruction;
 
 /*********
  * Model *
@@ -191,13 +186,12 @@ VirtualMachineImpl::VirtualMachineImpl(const std::string& name, s4u::VirtualMach
   update_action_weight();
 
   XBT_VERB("Create VM(%s)@PM(%s)", name.c_str(), physical_host_->get_cname());
-  on_creation(*this);
 }
 
 /** @brief A physical host does not disappear in the current SimGrid code, but a VM may disappear during a simulation */
-VirtualMachineImpl::~VirtualMachineImpl()
+void VirtualMachineImpl::destroy()
 {
-  on_destruction(*this);
+  s4u::VirtualMachine::on_destruction(*piface_);
   /* I was already removed from the allVms set if the VM was destroyed cleanly */
   auto iter = find(allVms_.begin(), allVms_.end(), piface_);
   if (iter != allVms_.end())
@@ -210,7 +204,7 @@ VirtualMachineImpl::~VirtualMachineImpl()
 
 void VirtualMachineImpl::suspend(smx_actor_t issuer)
 {
-  if (get_state() != s4u::VirtualMachine::State::RUNNING)
+  if (vm_state_ != s4u::VirtualMachine::State::RUNNING)
     throw VmFailureException(XBT_THROW_POINT,
                              xbt::string_printf("Cannot suspend VM %s: it is not running.", piface_->get_cname()));
   if (issuer->get_host() == piface_)
@@ -233,7 +227,7 @@ void VirtualMachineImpl::suspend(smx_actor_t issuer)
 
 void VirtualMachineImpl::resume()
 {
-  if (get_state() != s4u::VirtualMachine::State::SUSPENDED)
+  if (vm_state_ != s4u::VirtualMachine::State::SUSPENDED)
     throw VmFailureException(XBT_THROW_POINT,
                              xbt::string_printf("Cannot resume VM %s: it was not suspended", piface_->get_cname()));
 
@@ -258,7 +252,7 @@ void VirtualMachineImpl::resume()
  */
 void VirtualMachineImpl::shutdown(smx_actor_t issuer)
 {
-  if (get_state() != s4u::VirtualMachine::State::RUNNING)
+  if (vm_state_ != s4u::VirtualMachine::State::RUNNING)
     XBT_VERB("Shutting down the VM %s even if it's not running but in state %s", piface_->get_cname(),
              s4u::VirtualMachine::to_c_str(get_state()));
 

diff --git a/src/plugins/vm/VirtualMachineImpl.hpp b/src/plugins/vm/VirtualMachineImpl.hpp
index 68bd7c3..c775761 100644 (file)
--- a/src/plugins/vm/VirtualMachineImpl.hpp
+++ b/src/plugins/vm/VirtualMachineImpl.hpp
@@ -33,20 +33,15 @@ class XBT_PUBLIC VirtualMachineImpl : public surf::HostImpl, public simgrid::xbt
 #endif
 
 public:
-  /** @brief Callbacks fired after VM creation. Signature: `void(VirtualMachineImpl&)` */
-  static xbt::signal<void(simgrid::vm::VirtualMachineImpl&)> on_creation;
-  /** @brief Callbacks fired after VM destruction. Signature: `void(VirtualMachineImpl const&)` */
-  static xbt::signal<void(simgrid::vm::VirtualMachineImpl const&)> on_destruction;
-
   static std::deque<s4u::VirtualMachine*> allVms_;
 
   explicit VirtualMachineImpl(const std::string& name, s4u::VirtualMachine* piface, s4u::Host* host, int core_amount,
                               size_t ramsize);
-  ~VirtualMachineImpl() override;
 
   void suspend(kernel::actor::ActorImpl* issuer);
   void resume();
   void shutdown(kernel::actor::ActorImpl* issuer);
+  void destroy();
 
   /** @brief Change the physical host on which the given VM is running */
   void set_physical_host(s4u::Host* dest);

diff --git a/src/plugins/vm/s4u_VirtualMachine.cpp b/src/plugins/vm/s4u_VirtualMachine.cpp
index 486a75b..ce07172 100644 (file)
--- a/src/plugins/vm/s4u_VirtualMachine.cpp
+++ b/src/plugins/vm/s4u_VirtualMachine.cpp
@@ -16,6 +16,7 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(s4u_vm, s4u, "S4U virtual machines");
 
 namespace simgrid {
 namespace s4u {
+simgrid::xbt::signal<void(VirtualMachine&)> VirtualMachine::on_creation;
 simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_start;
 simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_started;
 simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_shutdown;
@@ -23,6 +24,7 @@ simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_suspend;
 simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_resume;
 simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_migration_start;
 simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_migration_end;
+simgrid::xbt::signal<void(VirtualMachine const&)> VirtualMachine::on_destruction;
 
 VirtualMachine::VirtualMachine(const std::string& name, s4u::Host* physical_host, int core_amount)
     : VirtualMachine(name, physical_host, core_amount, 1024)
@@ -54,14 +56,7 @@ VirtualMachine::VirtualMachine(const std::string& name, s4u::Host* physical_host
     set_pstate(physical_host->get_pstate());
 
   seal(); // seal this host
-}
-
-VirtualMachine::~VirtualMachine()
-{
-  on_destruction(*this);
-
-  /* Don't free these things twice: they are the ones of my physical host */
-  set_netpoint(nullptr);
+  s4u::VirtualMachine::on_creation(*this);
 }
 
 void VirtualMachine::start()
@@ -127,7 +122,11 @@ void VirtualMachine::destroy()
 
     /* Then, destroy the VM object */
     kernel::actor::simcall([this]() {
+      get_vm_impl()->destroy();
       get_impl()->destroy();
+
+      /* Don't free these things twice: they are the ones of my physical host */
+      set_netpoint(nullptr);
       delete this;
     });
   };

diff --git a/src/s4u/s4u_Disk.cpp b/src/s4u/s4u_Disk.cpp
index 7ad08c3..e9d7222 100644 (file)
--- a/src/s4u/s4u_Disk.cpp
+++ b/src/s4u/s4u_Disk.cpp
@@ -157,7 +157,7 @@ Disk* Disk::set_factor_cb(const std::function<IoFactorCb>& cb)
 Disk* Disk::seal()
 {
   kernel::actor::simcall([this]{ pimpl_->seal(); });
-  Disk::on_creation(*this);
+  Disk::on_creation(*this); // notify the signal
   return this;
 }
 } // namespace s4u

diff --git a/src/s4u/s4u_Link.cpp b/src/s4u/s4u_Link.cpp
index 9c6d41a..a9d9753 100644 (file)
--- a/src/s4u/s4u_Link.cpp
+++ b/src/s4u/s4u_Link.cpp
@@ -145,6 +145,7 @@ void Link::turn_off()
 Link* Link::seal()
 {
   kernel::actor::simcall([this]() { this->pimpl_->seal(); });
+  s4u::Link::on_creation(*this); // notify the signal
   return this;
 }
 

diff --git a/src/surf/HostImpl.cpp b/src/surf/HostImpl.cpp
index e81edd4..aeac40d 100644 (file)
--- a/src/surf/HostImpl.cpp
+++ b/src/surf/HostImpl.cpp
@@ -54,7 +54,7 @@ HostImpl::~HostImpl()
   actors_at_boot_.clear();
 
   for (auto const& d : disks_)
-    d->destroy();
+    d.second->destroy();
 }
 
 /** @brief Fire the required callbacks and destroy the object
@@ -125,7 +125,7 @@ std::vector<s4u::Disk*> HostImpl::get_disks() const
 {
   std::vector<s4u::Disk*> disks;
   for (auto const& d : disks_)
-    disks.push_back(d->get_iface());
+    disks.push_back(d.second->get_iface());
   return disks;
 }
 
@@ -138,19 +138,12 @@ s4u::Disk* HostImpl::create_disk(const std::string& name, double read_bandwidth,
 
 void HostImpl::add_disk(const s4u::Disk* disk)
 {
-  disks_.push_back(disk->get_impl());
+  disks_[disk->get_name()] = disk->get_impl();
 }
 
-void HostImpl::remove_disk(const std::string& disk_name)
+void HostImpl::remove_disk(const std::string& name)
 {
-  auto position = disks_.begin();
-  for (auto const& d : disks_) {
-    if (d->get_name() == disk_name) {
-      disks_.erase(position);
-      break;
-    }
-    position++;
-  }
+  disks_.erase(name);
 }
 
 void HostImpl::seal()
@@ -163,8 +156,8 @@ void HostImpl::seal()
   sealed_ = true;
 
   /* seal its disks */
-  for (auto* disk : disks_)
-    disk->seal();
+  for (auto const& disk : disks_)
+    disk.second->seal();
 }
 } // namespace surf
 } // namespace simgrid

diff --git a/src/surf/HostImpl.hpp b/src/surf/HostImpl.hpp
index 6ac9e5a..8b16e7c 100644 (file)
--- a/src/surf/HostImpl.hpp
+++ b/src/surf/HostImpl.hpp
@@ -49,7 +49,7 @@ class XBT_PRIVATE HostImpl : public xbt::PropertyHolder {
   ActorList actor_list_;
   std::vector<kernel::actor::ProcessArg*> actors_at_boot_;
   s4u::Host piface_;
-  std::vector<kernel::resource::DiskImpl*> disks_;
+  std::map<std::string, kernel::resource::DiskImpl*, std::less<>> disks_;
   xbt::string name_{"noname"};
   bool sealed_ = false;
 
@@ -66,7 +66,7 @@ public:
   std::vector<s4u::Disk*> get_disks() const;
   s4u::Disk* create_disk(const std::string& name, double read_bandwidth, double write_bandwidth);
   void add_disk(const s4u::Disk* disk);
-  void remove_disk(const std::string& disk_name);
+  void remove_disk(const std::string& name);
 
   virtual const s4u::Host* get_iface() const { return &piface_; }
   virtual s4u::Host* get_iface() { return &piface_; }

diff --git a/src/surf/LinkImpl.cpp b/src/surf/LinkImpl.cpp
index ecbf1ac..d97bc34 100644 (file)
--- a/src/surf/LinkImpl.cpp
+++ b/src/surf/LinkImpl.cpp
@@ -34,7 +34,8 @@ LinkImpl::LinkImpl(const std::string& name) : LinkImplIntf(name), piface_(this)
  */
 void LinkImpl::destroy()
 {
-  s4u::Link::on_destruction(this->piface_);
+  s4u::Link::on_destruction(piface_);
+  s4u::Engine::get_instance()->link_unregister(get_name());
   delete this;
 }
 
@@ -110,7 +111,6 @@ void LinkImpl::seal()
 
   xbt_assert(this->get_model(), "Cannot seal Link(%s) without setting the Network model first", this->get_cname());
   Resource::seal();
-  s4u::Link::on_creation(piface_);
 }
 
 void LinkImpl::on_bandwidth_change() const
@@ -144,4 +144,4 @@ void LinkImpl::set_concurrency_limit(int limit) const
 
 } // namespace resource
 } // namespace kernel
-} // namespace simgrid
\ No newline at end of file
+} // namespace simgrid