From: Martin Quinson Date: Mon, 21 Nov 2016 00:20:06 +0000 (+0100) Subject: code simplification in the VM creation + inline a function X-Git-Tag: v3_14~162 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/c080feddf0dd4e58c906e4b7b423b5676a8517e0?ds=sidebyside code simplification in the VM creation + inline a function --- diff --git a/include/simgrid/simix.h b/include/simgrid/simix.h index 4964376be9..cbd0318e7c 100644 --- 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 -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); @@ -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_destroy(sg_host_t vm); 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 a102d457b0..c4f224233c 100644 --- 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()){ diff --git a/src/msg/msg_private.h b/src/msg/msg_private.h index b07c9b6894..d02abc789f 100644 --- 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 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); diff --git a/src/msg/msg_vm.cpp b/src/msg/msg_vm.cpp index c70ff31bd4..c3c29d87f9 100644 --- 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. */ -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. @@ -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)) - 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)) - 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 */ - 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(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); } diff --git a/src/plugins/vm/s4u_VirtualMachine.cpp b/src/plugins/vm/s4u_VirtualMachine.cpp index 32df01519e..ec4ee2fcb1 100644 --- 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 "src/simix/smx_host_private.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 { -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(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() diff --git a/src/simix/libsmx.cpp b/src/simix/libsmx.cpp index c506bfeae8..606f99a616 100644 --- 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); } -/** - * \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(new simgrid::simix::Host()); - - return 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); } -/** - * \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(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.