From 783b33766eccfc78e3b205d896323169ec42e7c1 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Sun, 28 Aug 2016 18:08:11 +0200 Subject: [PATCH 1/1] cosmetics in simix VM (much more to do) --- include/simgrid/host.h | 1 - src/simgrid/host.cpp | 3 - src/simix/smx_vm.cpp | 126 ++++++++++++++++++++--------------------- 3 files changed, 60 insertions(+), 70 deletions(-) diff --git a/include/simgrid/host.h b/include/simgrid/host.h index 83c66d7648..4321c24a66 100644 --- a/include/simgrid/host.h +++ b/include/simgrid/host.h @@ -38,7 +38,6 @@ XBT_PUBLIC(void) sg_host_msg_set(sg_host_t host, msg_host_priv_t priv); // ========== Simix layer ============= XBT_PUBLIC(smx_host_priv_t) sg_host_simix(sg_host_t host); XBT_PUBLIC(void) sg_host_simix_set(sg_host_t host, smx_host_priv_t priv); -XBT_PUBLIC(void) sg_host_simix_destroy(sg_host_t host); // ========= storage related functions ============ XBT_PUBLIC(xbt_dict_t) sg_host_get_mounted_storage_list(sg_host_t host); diff --git a/src/simgrid/host.cpp b/src/simgrid/host.cpp index 1d32fc2d42..8c12c9c90c 100644 --- a/src/simgrid/host.cpp +++ b/src/simgrid/host.cpp @@ -98,9 +98,6 @@ smx_host_priv_t sg_host_simix(sg_host_t host){ void sg_host_simix_set(sg_host_t host, smx_host_priv_t smx_host) { host->extension_set(smx_host); } -void sg_host_simix_destroy(sg_host_t host) { - host->extension_set(nullptr); -} // ========= storage related functions ============ xbt_dict_t sg_host_get_mounted_storage_list(sg_host_t host){ diff --git a/src/simix/smx_vm.cpp b/src/simix/smx_vm.cpp index b560598d53..ba05d17e6a 100644 --- a/src/simix/smx_vm.cpp +++ b/src/simix/smx_vm.cpp @@ -154,9 +154,9 @@ void *SIMIX_vm_get_pm(sg_host_t host) * @param host the vm host (a sg_host_t) * @param bound bound (a double) */ -void SIMIX_vm_set_bound(sg_host_t host, double bound) +void SIMIX_vm_set_bound(sg_host_t vm, double bound) { - surf_vm_set_bound(host, bound); + surf_vm_set_bound(vm, bound); } /** @@ -164,22 +164,20 @@ void SIMIX_vm_set_bound(sg_host_t host, double bound) * VM. All the processes on this VM will pause. The state of the VM is * preserved on memory. We can later resume it again. * - * @param host the vm host to suspend (a sg_host_t) + * @param vm the vm host to suspend (a sg_host_t) */ -void SIMIX_vm_suspend(sg_host_t ind_vm, smx_actor_t issuer) +void SIMIX_vm_suspend(sg_host_t vm, smx_actor_t issuer) { - const char *name = sg_host_get_name(ind_vm); + if (SIMIX_vm_get_state(vm) != SURF_VM_STATE_RUNNING) + THROWF(vm_error, 0, "VM(%s) is not running", vm->name().c_str()); - if (SIMIX_vm_get_state(ind_vm) != SURF_VM_STATE_RUNNING) - THROWF(vm_error, 0, "VM(%s) is not running", name); - - XBT_DEBUG("suspend VM(%s), where %d processes exist", name, xbt_swag_size(sg_host_simix(ind_vm)->process_list)); + XBT_DEBUG("suspend VM(%s), where %d processes exist", vm->name().c_str(), xbt_swag_size(sg_host_simix(vm)->process_list)); /* jump to vm_ws_suspend. The state will be set. */ - surf_vm_suspend(ind_vm); + surf_vm_suspend(vm); smx_actor_t smx_process, smx_process_safe; - xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(ind_vm)->process_list) { + xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(vm)->process_list) { XBT_DEBUG("suspend %s", smx_process->name.c_str()); SIMIX_process_suspend(smx_process, issuer); } @@ -187,14 +185,11 @@ void SIMIX_vm_suspend(sg_host_t ind_vm, smx_actor_t issuer) XBT_DEBUG("suspend all processes on the VM done done"); } -void simcall_HANDLER_vm_suspend(smx_simcall_t simcall, sg_host_t ind_vm) +void simcall_HANDLER_vm_suspend(smx_simcall_t simcall, sg_host_t vm) { - if (simcall->issuer->host == ind_vm) { - XBT_ERROR("cannot suspend the VM where I run"); - DIE_IMPOSSIBLE; - } + xbt_assert(simcall->issuer->host != vm, "cannot suspend the VM where I run"); - SIMIX_vm_suspend(ind_vm, simcall->issuer); + SIMIX_vm_suspend(vm, simcall->issuer); XBT_DEBUG("simcall_HANDLER_vm_suspend done"); } @@ -204,30 +199,29 @@ void simcall_HANDLER_vm_suspend(smx_simcall_t simcall, sg_host_t ind_vm) * @brief Function to resume a SIMIX VM host. This function restart the execution of the * VM. All the processes on this VM will run again. * - * @param host the vm host to resume (a sg_host_t) + * @param vm the vm host to resume (a sg_host_t) */ -void SIMIX_vm_resume(sg_host_t ind_vm, smx_actor_t issuer) +void SIMIX_vm_resume(sg_host_t vm, smx_actor_t issuer) { - const char *name = sg_host_get_name(ind_vm); + if (SIMIX_vm_get_state(vm) != SURF_VM_STATE_SUSPENDED) + THROWF(vm_error, 0, "VM(%s) was not suspended", vm->name().c_str()); - if (SIMIX_vm_get_state(ind_vm) != SURF_VM_STATE_SUSPENDED) - THROWF(vm_error, 0, "VM(%s) was not suspended", name); - - XBT_DEBUG("resume VM(%s), where %d processes exist", name, xbt_swag_size(sg_host_simix(ind_vm)->process_list)); + XBT_DEBUG("resume VM(%s), where %d processes exist", + vm->name().c_str(), xbt_swag_size(sg_host_simix(vm)->process_list)); /* jump to vm_ws_resume() */ - surf_vm_resume(ind_vm); + surf_vm_resume(vm); smx_actor_t smx_process, smx_process_safe; - xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(ind_vm)->process_list) { + xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(vm)->process_list) { XBT_DEBUG("resume %s", smx_process->name.c_str()); SIMIX_process_resume(smx_process, issuer); } } -void simcall_HANDLER_vm_resume(smx_simcall_t simcall, sg_host_t ind_vm) +void simcall_HANDLER_vm_resume(smx_simcall_t simcall, sg_host_t vm) { - SIMIX_vm_resume(ind_vm, simcall->issuer); + SIMIX_vm_resume(vm, simcall->issuer); } @@ -236,31 +230,30 @@ void simcall_HANDLER_vm_resume(smx_simcall_t simcall, sg_host_t ind_vm) * This function is the same as vm_suspend, but the state of the VM is saved to the disk, and not preserved on memory. * We can later restore it again. * - * @param host the vm host to save (a sg_host_t) + * @param vm the vm host to save (a sg_host_t) */ -void SIMIX_vm_save(sg_host_t ind_vm, smx_actor_t issuer) +void SIMIX_vm_save(sg_host_t vm, smx_actor_t issuer) { - const char *name = sg_host_get_name(ind_vm); + const char *name = sg_host_get_name(vm); - if (SIMIX_vm_get_state(ind_vm) != SURF_VM_STATE_RUNNING) + if (SIMIX_vm_get_state(vm) != SURF_VM_STATE_RUNNING) THROWF(vm_error, 0, "VM(%s) is not running", name); - - XBT_DEBUG("save VM(%s), where %d processes exist", name, xbt_swag_size(sg_host_simix(ind_vm)->process_list)); + XBT_DEBUG("save VM(%s), where %d processes exist", name, xbt_swag_size(sg_host_simix(vm)->process_list)); /* jump to vm_ws_save() */ - surf_vm_save(ind_vm); + surf_vm_save(vm); smx_actor_t smx_process, smx_process_safe; - xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(ind_vm)->process_list) { + xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(vm)->process_list) { XBT_DEBUG("suspend %s", smx_process->name.c_str()); SIMIX_process_suspend(smx_process, issuer); } } -void simcall_HANDLER_vm_save(smx_simcall_t simcall, sg_host_t ind_vm) +void simcall_HANDLER_vm_save(smx_simcall_t simcall, sg_host_t vm) { - SIMIX_vm_save(ind_vm, simcall->issuer); + SIMIX_vm_save(vm, simcall->issuer); } @@ -268,30 +261,29 @@ void simcall_HANDLER_vm_save(smx_simcall_t simcall, sg_host_t ind_vm) * @brief Function to restore a SIMIX VM host. This function restart the execution of the * VM. All the processes on this VM will run again. * - * @param host the vm host to restore (a sg_host_t) + * @param vm the vm host to restore (a sg_host_t) */ -void SIMIX_vm_restore(sg_host_t ind_vm, smx_actor_t issuer) +void SIMIX_vm_restore(sg_host_t vm, smx_actor_t issuer) { - const char *name = sg_host_get_name(ind_vm); - - if (SIMIX_vm_get_state(ind_vm) != SURF_VM_STATE_SAVED) - THROWF(vm_error, 0, "VM(%s) was not saved", name); + if (SIMIX_vm_get_state(vm) != SURF_VM_STATE_SAVED) + THROWF(vm_error, 0, "VM(%s) was not saved", vm->name().c_str()); - XBT_DEBUG("restore VM(%s), where %d processes exist", name, xbt_swag_size(sg_host_simix(ind_vm)->process_list)); + XBT_DEBUG("restore VM(%s), where %d processes exist", + vm->name().c_str(), xbt_swag_size(sg_host_simix(vm)->process_list)); /* jump to vm_ws_restore() */ - surf_vm_resume(ind_vm); + surf_vm_resume(vm); smx_actor_t smx_process, smx_process_safe; - xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(ind_vm)->process_list) { + xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(vm)->process_list) { XBT_DEBUG("resume %s", smx_process->name.c_str()); SIMIX_process_resume(smx_process, issuer); } } -void simcall_HANDLER_vm_restore(smx_simcall_t simcall, sg_host_t ind_vm) +void simcall_HANDLER_vm_restore(smx_simcall_t simcall, sg_host_t vm) { - SIMIX_vm_restore(ind_vm, simcall->issuer); + SIMIX_vm_restore(vm, simcall->issuer); } @@ -300,36 +292,33 @@ void simcall_HANDLER_vm_restore(smx_simcall_t simcall, sg_host_t ind_vm) * VM. All the processes on this VM will be killed. But, the state of the VM is * preserved on memory. We can later start it again. * - * @param host the vm host to shutdown (a sg_host_t) + * @param vm the VM to shutdown (a sg_host_t) */ -void SIMIX_vm_shutdown(sg_host_t ind_vm, smx_actor_t issuer) +void SIMIX_vm_shutdown(sg_host_t vm, smx_actor_t issuer) { - const char *name = sg_host_get_name(ind_vm); + if (SIMIX_vm_get_state(vm) != SURF_VM_STATE_RUNNING) + THROWF(vm_error, 0, "VM(%s) is not running", vm->name().c_str()); - if (SIMIX_vm_get_state(ind_vm) != SURF_VM_STATE_RUNNING) - THROWF(vm_error, 0, "VM(%s) is not running", name); - - XBT_DEBUG("shutdown %s", name); - XBT_DEBUG("%d processes in the VM", xbt_swag_size(sg_host_simix(ind_vm)->process_list)); + XBT_DEBUG("shutdown VM %s, that contains %d processes", + vm->name().c_str(),xbt_swag_size(sg_host_simix(vm)->process_list)); smx_actor_t smx_process, smx_process_safe; - xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(ind_vm)->process_list) { + xbt_swag_foreach_safe(smx_process, smx_process_safe, sg_host_simix(vm)->process_list) { XBT_DEBUG("kill %s", smx_process->name.c_str()); SIMIX_process_kill(smx_process, issuer); } /* FIXME: we may have to do something at the surf layer, e.g., vcpu action */ static_cast( - ind_vm->extension() + vm->extension() )->setState(SURF_VM_STATE_CREATED); } -void simcall_HANDLER_vm_shutdown(smx_simcall_t simcall, sg_host_t ind_vm) +void simcall_HANDLER_vm_shutdown(smx_simcall_t simcall, sg_host_t vm) { - SIMIX_vm_shutdown(ind_vm, simcall->issuer); + SIMIX_vm_shutdown(vm, simcall->issuer); } - /** * @brief Function to destroy a SIMIX VM host. * @@ -338,10 +327,15 @@ void simcall_HANDLER_vm_shutdown(smx_simcall_t simcall, sg_host_t ind_vm) void SIMIX_vm_destroy(sg_host_t vm) { /* this code basically performs a similar thing like SIMIX_host_destroy() */ - XBT_DEBUG("destroy %s", sg_host_get_name(vm)); - - /* this will call the registered callback function, i.e., SIMIX_host_destroy(). */ - sg_host_simix_destroy(vm); + 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; -- 2.20.1