code simplification in the VM creation + inline a function

diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h
index 4964376..cbd0318 100644 (file)
--- a/include/simgrid/simix.h
+++ b/include/simgrid/simix.h
@@ -259,7 +259,6 @@ XBT_PUBLIC(e_smx_state_t) simcall_execution_wait(smx_activity_t execution);
 
 /******************************* VM simcalls ********************************/
 // Create the vm_workstation at the SURF level
 
 /******************************* VM simcalls ********************************/
 // Create the vm_workstation at the SURF level

-XBT_PUBLIC(sg_host_t) simcall_vm_create(const char *name, sg_host_t host);

 XBT_PUBLIC(void) simcall_vm_start(sg_host_t vm);
 XBT_PUBLIC(void *) simcall_vm_get_pm(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_set_bound(sg_host_t vm, double bound);
 XBT_PUBLIC(void) simcall_vm_start(sg_host_t vm);
 XBT_PUBLIC(void *) simcall_vm_get_pm(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_set_bound(sg_host_t vm, double bound);


@@ -267,7 +266,6 @@ XBT_PUBLIC(void) simcall_vm_resume(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_save(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_restore(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_suspend(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_save(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_restore(sg_host_t vm);
 XBT_PUBLIC(void) simcall_vm_suspend(sg_host_t vm);

-XBT_PUBLIC(void) simcall_vm_destroy(sg_host_t vm);

 XBT_PUBLIC(void) simcall_vm_shutdown(sg_host_t vm);
 
 /**************************** Process simcalls ********************************/
 XBT_PUBLIC(void) simcall_vm_shutdown(sg_host_t vm);
 
 /**************************** Process simcalls ********************************/

diff --git a/src/msg/instr_msg_vm.cpp b/src/msg/instr_msg_vm.cpp
index a102d45..c4f2242 100644 (file)
--- a/src/msg/instr_msg_vm.cpp
+++ b/src/msg/instr_msg_vm.cpp
@@ -54,17 +54,6 @@ void TRACE_msg_vm_change_host(msg_vm_t vm, msg_host_t old_host, msg_host_t new_h
   }
 }
 
   }
 }
 

-void TRACE_msg_vm_create(const char *vm_name, msg_host_t host)
-{
-  if (TRACE_msg_vm_is_enabled()){
-    int len = INSTR_DEFAULT_STR_SIZE;
-    char str[INSTR_DEFAULT_STR_SIZE];
-
-    container_t host_container = PJ_container_get (sg_host_get_name(host));
-    PJ_container_new(instr_vm_id_2(vm_name, str, len), INSTR_MSG_VM, host_container);
-  }
-}
-

 void TRACE_msg_vm_start(msg_vm_t vm)
 {
   if (TRACE_msg_vm_is_enabled()){
 void TRACE_msg_vm_start(msg_vm_t vm)
 {
   if (TRACE_msg_vm_is_enabled()){

diff --git a/src/msg/msg_private.h b/src/msg/msg_private.h
index b07c9b6..d02abc7 100644 (file)
--- a/src/msg/msg_private.h
+++ b/src/msg/msg_private.h
@@ -175,7 +175,6 @@ XBT_PRIVATE char *instr_vm_id(msg_vm_t vm, char *str, int len);
 XBT_PRIVATE char *instr_vm_id_2(const char *vm_name, char *str, int len);
 XBT_PRIVATE void TRACE_msg_vm_change_host(msg_vm_t vm, msg_host_t old_host, msg_host_t new_host);
 XBT_PRIVATE void TRACE_msg_vm_start(msg_vm_t vm);
 XBT_PRIVATE char *instr_vm_id_2(const char *vm_name, char *str, int len);
 XBT_PRIVATE void TRACE_msg_vm_change_host(msg_vm_t vm, msg_host_t old_host, msg_host_t new_host);
 XBT_PRIVATE void TRACE_msg_vm_start(msg_vm_t vm);

-XBT_PRIVATE void TRACE_msg_vm_create(const char *vm_name, msg_host_t host);

 XBT_PRIVATE void TRACE_msg_vm_kill(msg_vm_t process);
 XBT_PRIVATE void TRACE_msg_vm_suspend(msg_vm_t vm);
 XBT_PRIVATE void TRACE_msg_vm_resume(msg_vm_t vm);
 XBT_PRIVATE void TRACE_msg_vm_kill(msg_vm_t process);
 XBT_PRIVATE void TRACE_msg_vm_suspend(msg_vm_t vm);
 XBT_PRIVATE void TRACE_msg_vm_resume(msg_vm_t vm);

diff --git a/src/msg/msg_vm.cpp b/src/msg/msg_vm.cpp
index c70ff31..c3c29d8 100644 (file)
--- a/src/msg/msg_vm.cpp
+++ b/src/msg/msg_vm.cpp
@@ -158,28 +158,12 @@ msg_vm_t MSG_vm_create(msg_host_t pm, const char *name, int ncpus, int ramsize,
  *
  * A VM is treated as a host. The name of the VM must be unique among all hosts.
  */
  *
  * A VM is treated as a host. The name of the VM must be unique among all hosts.
  */

-msg_vm_t MSG_vm_create_core(msg_host_t ind_pm, const char *name)
+msg_vm_t MSG_vm_create_core(msg_host_t pm, const char* name)

 {
 {

-  /* make sure the VM of the same name does not exit */
-  {
-    simgrid::s4u::Host* ind_host_tmp = sg_host_by_name(name);
-    if (ind_host_tmp != nullptr && sg_host_simix(ind_host_tmp) != nullptr) {
-      XBT_ERROR("host %s already exits", name);
-      return nullptr;
-    }
-  }
-
-  /* Note: ind_vm and vm_workstation point to the same elm object. */
-  /* Ask the SIMIX layer to create the surf vm resource */
-  sg_host_t ind_vm_workstation = simcall_vm_create(name, ind_pm);
-
-  msg_vm_t ind_vm = (msg_vm_t) __MSG_host_create(ind_vm_workstation);
+  xbt_assert(sg_host_by_name(name) == nullptr,
+             "Cannot create a VM named %s: this name is already used by an host or a VM", name);

 
 

-  XBT_DEBUG("A new VM (%s) has been created", name);
-
-  TRACE_msg_vm_create(name, ind_pm);
-
-  return ind_vm;
+  return new simgrid::s4u::VirtualMachine(name, pm);

 }
 
 /** @brief Destroy a VM. Destroy the VM object from the simulation.
 }
 
 /** @brief Destroy a VM. Destroy the VM object from the simulation.


@@ -188,19 +172,33 @@ msg_vm_t MSG_vm_create_core(msg_host_t ind_pm, const char *name)
 void MSG_vm_destroy(msg_vm_t vm)
 {
   if (MSG_vm_is_migrating(vm))
 void MSG_vm_destroy(msg_vm_t vm)
 {
   if (MSG_vm_is_migrating(vm))

-    THROWF(vm_error, 0, "VM(%s) is migrating", sg_host_get_name(vm));
+    THROWF(vm_error, 0, "VM(%s) is migrating", vm->name().c_str());

 
   /* First, terminate all processes on the VM if necessary */
   if (MSG_vm_is_running(vm))
 
   /* First, terminate all processes on the VM if necessary */
   if (MSG_vm_is_running(vm))

-      simcall_vm_shutdown(vm);
+    MSG_vm_shutdown(vm);

 
 

-  if (!MSG_vm_is_created(vm)) {
-    XBT_CRITICAL("shutdown the given VM before destroying it");
-    DIE_IMPOSSIBLE;
-  }
+  xbt_assert(MSG_vm_is_created(vm), "shutdown the given VM before destroying it");

 
   /* Then, destroy the VM object */
 
   /* Then, destroy the VM object */

-  simcall_vm_destroy(vm);
+  simgrid::simix::kernelImmediate([vm]() {
+    /* this code basically performs a similar thing like SIMIX_host_destroy() */
+    XBT_DEBUG("destroy %s", vm->name().c_str());
+
+    /* FIXME: this is really strange that everything fails if the next line is removed.
+     * This is as if we shared these data with the PM, which definitely should not be the case...
+     *
+     * We need to test that suspending a VM does not suspends the processes running on its PM, for example.
+     * Or we need to simplify this code enough to make it actually readable (but this sounds harder than testing)
+     */
+    vm->extension_set<simgrid::simix::Host>(nullptr);
+
+    /* Don't free these things twice: they are the ones of my physical host */
+    vm->pimpl_cpu     = nullptr;
+    vm->pimpl_netcard = nullptr;
+
+    vm->destroy();
+  });

 
   TRACE_msg_vm_end(vm);
 }
 
   TRACE_msg_vm_end(vm);
 }

diff --git a/src/plugins/vm/s4u_VirtualMachine.cpp b/src/plugins/vm/s4u_VirtualMachine.cpp
index 32df015..ec4ee2f 100644 (file)
--- a/src/plugins/vm/s4u_VirtualMachine.cpp
+++ b/src/plugins/vm/s4u_VirtualMachine.cpp
@@ -8,15 +8,27 @@
 #include "simgrid/s4u/host.hpp"
 #include "simgrid/simix.hpp"
 #include "src/plugins/vm/VirtualMachineImpl.hpp"
 #include "simgrid/s4u/host.hpp"
 #include "simgrid/simix.hpp"
 #include "src/plugins/vm/VirtualMachineImpl.hpp"

+#include "src/simix/smx_host_private.h"

 #include "src/surf/HostImpl.hpp"
 #include "xbt/asserts.h"
 #include "src/surf/HostImpl.hpp"
 #include "xbt/asserts.h"

+#include "src/instr/instr_private.h"
+
+XBT_LOG_NEW_DEFAULT_CATEGORY(s4u_vm, "S4U virtual machines");

 
 namespace simgrid {
 namespace s4u {
 
 
 namespace simgrid {
 namespace s4u {
 

-VirtualMachine::VirtualMachine(const char* name, s4u::Host* Pm) : Host(name)
+VirtualMachine::VirtualMachine(const char* name, s4u::Host* pm) : Host(name)

 {
 {

-  pimpl_vm_ = new surf::VirtualMachineImpl(this, Pm);
+  XBT_DEBUG("Create VM %s", name);
+
+  pimpl_vm_ = new surf::VirtualMachineImpl(this, pm);
+  extension_set<simgrid::simix::Host>(new simgrid::simix::Host());
+
+  if (TRACE_msg_vm_is_enabled()) {
+    container_t host_container = PJ_container_get(sg_host_get_name(pm));
+    PJ_container_new(name, INSTR_MSG_VM, host_container);
+  }

 }
 
 VirtualMachine::~VirtualMachine()
 }
 
 VirtualMachine::~VirtualMachine()

diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp
index c506bfe..606f99a 100644 (file)
--- a/src/simix/libsmx.cpp
+++ b/src/simix/libsmx.cpp
@@ -175,25 +175,6 @@ e_smx_state_t simcall_execution_wait(smx_activity_t execution)
   return (e_smx_state_t) simcall_BODY_execution_wait(execution);
 }
 
   return (e_smx_state_t) simcall_BODY_execution_wait(execution);
 }
 

-/**
- * \ingroup simix_vm_management
- * \brief Create a VM on the given physical host.
- *
- * \param name VM name
- * \param dest Physical host on which to create the VM
- *
- * \return The host object of the VM
- */
-sg_host_t simcall_vm_create(const char* name, sg_host_t dest)
-{
-  return simgrid::simix::kernelImmediate([&name, &dest] {
-    sg_host_t host = new simgrid::s4u::VirtualMachine(name, dest);
-    host->extension_set<simgrid::simix::Host>(new simgrid::simix::Host());
-
-    return host;
-  });
-}
-

 /**
  * \ingroup simix_vm_management
  * \brief Start the given VM to the given physical host
 /**
  * \ingroup simix_vm_management
  * \brief Start the given VM to the given physical host


@@ -301,34 +282,6 @@ void simcall_vm_shutdown(sg_host_t vm)
   simcall_BODY_vm_shutdown(vm);
 }
 
   simcall_BODY_vm_shutdown(vm);
 }
 

-/**
- * \ingroup simix_vm_management
- * \brief Destroy the given VM
- *
- * \param vm VM
- */
-void simcall_vm_destroy(sg_host_t vm)
-{
-  simgrid::simix::kernelImmediate([vm]() {
-    /* this code basically performs a similar thing like SIMIX_host_destroy() */
-    XBT_DEBUG("destroy %s", vm->name().c_str());
-
-    /* FIXME: this is really strange that everything fails if the next line is removed.
-     * This is as if we shared these data with the PM, which definitely should not be the case...
-     *
-     * We need to test that suspending a VM does not suspends the processes running on its PM, for example.
-     * Or we need to simplify this code enough to make it actually readable (but this sounds harder than testing)
-     */
-    vm->extension_set<simgrid::simix::Host>(nullptr);
-
-    /* Don't free these things twice: they are the ones of my physical host */
-    vm->pimpl_cpu     = nullptr;
-    vm->pimpl_netcard = nullptr;
-
-    vm->destroy();
-  });
-}
-

 /**
  * \ingroup simix_process_management
  * \brief Kills a SIMIX process.
 /**
  * \ingroup simix_process_management
  * \brief Kills a SIMIX process.