[libvirt] [PATCH v2] bhyve: implement domainShutdown
Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve process. Add the bhyveDomainShutdown() function and virBhyveProcessShutdown() helper function that just sends SIGTERM to VM's bhyve process. If a guest supports ACPI shutdown then process will be terminated and this event will be noticed by the bhyve monitor code that will handle setting proper status and clean up VM's resources. Also, remove usage of virProcessKillPainfully() from domainDestroy. First, it sends SIGTERM to the process that actually triggers ACPI reset and that's not we want to do. Second, we're doing bhyvectl --destroy later and it kills bhyve process, so there's no need to manually kill it. --- Changes from v1: - Sending SIGKILL dropped completely for destroy due to reasons described in the commit message. This is mainly based on the results of this (ongoing) discussion: https://lists.freebsd.org/pipermail/freebsd-virtualization/2016-April/004348... src/bhyve/bhyve_driver.c | 27 +++++++++++++++++++++++++++ src/bhyve/bhyve_process.c | 39 +++++++++++++++++++++++++++------------ src/bhyve/bhyve_process.h | 2 ++ 3 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 5526bb0..4fc504e 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1018,6 +1018,32 @@ bhyveDomainDestroy(virDomainPtr dom) } static int +bhyveDomainShutdown(virDomainPtr dom) +{ + virDomainObjPtr vm; + int ret = -1; + + if (!(vm = bhyveDomObjFromDomain(dom))) + goto cleanup; + + if (virDomainShutdownEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("Domain is not running")); + goto cleanup; + } + + ret = virBhyveProcessShutdown(vm); + + cleanup: + if (vm) + virObjectUnlock(vm); + return ret; +} + +static int bhyveDomainOpenConsole(virDomainPtr dom, const char *dev_name ATTRIBUTE_UNUSED, virStreamPtr st, @@ -1502,6 +1528,7 @@ static virHypervisorDriver bhyveHypervisorDriver = { .domainCreateWithFlags = bhyveDomainCreateWithFlags, /* 1.2.3 */ .domainCreateXML = bhyveDomainCreateXML, /* 1.2.4 */ .domainDestroy = bhyveDomainDestroy, /* 1.2.2 */ + .domainShutdown = bhyveDomainShutdown, /* 1.3.3 */ .domainLookupByUUID = bhyveDomainLookupByUUID, /* 1.2.2 */ .domainLookupByName = bhyveDomainLookupByName, /* 1.2.2 */ .domainLookupByID = bhyveDomainLookupByID, /* 1.2.3 */ diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c index 14588a9..e42ed74 100644 --- a/src/bhyve/bhyve_process.c +++ b/src/bhyve/bhyve_process.c @@ -281,23 +281,15 @@ virBhyveProcessStop(bhyveConnPtr driver, if ((priv != NULL) && (priv->mon != NULL)) bhyveMonitorClose(priv->mon); - /* First, try to kill 'bhyve' process */ - if (virProcessKillPainfully(vm->pid, true) != 0) - VIR_WARN("Failed to gracefully stop bhyve VM '%s' (pid: %d)", - vm->def->name, - (int)vm->pid); - - /* Cleanup network interfaces */ - bhyveNetCleanup(vm); - - /* No matter if shutdown was successful or not, we - * need to unload the VM */ if (!(cmd = virBhyveProcessBuildDestroyCmd(driver, vm->def))) - goto cleanup; + return -1; if (virCommandRun(cmd, NULL) < 0) goto cleanup; + /* Cleanup network interfaces */ + bhyveNetCleanup(vm); + ret = 0; virCloseCallbacksUnset(driver->closeCallbacks, vm, @@ -317,6 +309,29 @@ virBhyveProcessStop(bhyveConnPtr driver, } int +virBhyveProcessShutdown(virDomainObjPtr vm) +{ + if (vm->pid <= 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid PID %d for VM"), + (int)vm->pid); + return -1; + } + + /* Bhyve tries to perform ACPI shutdown when it receives + * SIGTERM signal. So we just issue SIGTERM here and rely + * on the bhyve monitor to clean things up if process disappears. + */ + if (virProcessKill(vm->pid, SIGTERM) != 0) { + VIR_WARN("Failed to terminate bhyve process for VM '%s': %s", + vm->def->name, virGetLastErrorMessage()); + return -1; + } + + return 0; +} + +int virBhyveGetDomainTotalCpuStats(virDomainObjPtr vm, unsigned long long *cpustats) { diff --git a/src/bhyve/bhyve_process.h b/src/bhyve/bhyve_process.h index cfa80af..ebabe17 100644 --- a/src/bhyve/bhyve_process.h +++ b/src/bhyve/bhyve_process.h @@ -34,6 +34,8 @@ int virBhyveProcessStop(bhyveConnPtr driver, virDomainObjPtr vm, virDomainShutoffReason reason); +int virBhyveProcessShutdown(virDomainObjPtr vm); + int virBhyveGetDomainTotalCpuStats(virDomainObjPtr vm, unsigned long long *cpustats); -- 2.7.4
Roman Bogorodskiy wrote:
Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve process. Add the bhyveDomainShutdown() function and virBhyveProcessShutdown() helper function that just sends SIGTERM to VM's bhyve process. If a guest supports ACPI shutdown then process will be terminated and this event will be noticed by the bhyve monitor code that will handle setting proper status and clean up VM's resources.
Also, remove usage of virProcessKillPainfully() from domainDestroy. First, it sends SIGTERM to the process that actually triggers ACPI reset and that's not we want to do. Second, we're doing bhyvectl --destroy later and it kills bhyve process, so there's no need to manually kill it. --- Changes from v1:
- Sending SIGKILL dropped completely for destroy due to reasons described in the commit message. This is mainly based on the results of this (ongoing) discussion:
https://lists.freebsd.org/pipermail/freebsd-virtualization/2016-April/004348...
src/bhyve/bhyve_driver.c | 27 +++++++++++++++++++++++++++ src/bhyve/bhyve_process.c | 39 +++++++++++++++++++++++++++------------ src/bhyve/bhyve_process.h | 2 ++ 3 files changed, 56 insertions(+), 12 deletions(-)
ping? Roman Bogorodskiy
On 04/24/2016 02:11 PM, Roman Bogorodskiy wrote:
Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve process. Add the bhyveDomainShutdown() function and virBhyveProcessShutdown() helper function that just sends SIGTERM to VM's bhyve process. If a guest supports ACPI shutdown then process will be terminated and this event will be noticed by the bhyve monitor code that will handle setting proper status and clean up VM's resources.
Also, remove usage of virProcessKillPainfully() from domainDestroy. First, it sends SIGTERM to the process that actually triggers ACPI reset
do you mean 'ACPI shutdown' here? That's what it says in the below comments
and that's not we want to do. Second, we're doing bhyvectl --destroy later and it kills bhyve process, so there's no need to manually kill it.
This seems like two distinct changes, please send as two patches and I'll review One general comment: what handles the equivalent of bhyveNetCleanup for graceful VM shutdown? The bhyve process itself? Thanks, Cole
Cole Robinson wrote:
On 04/24/2016 02:11 PM, Roman Bogorodskiy wrote:
Bhyve supports ACPI shutdown by issuing SIGTERM signal to the bhyve process. Add the bhyveDomainShutdown() function and virBhyveProcessShutdown() helper function that just sends SIGTERM to VM's bhyve process. If a guest supports ACPI shutdown then process will be terminated and this event will be noticed by the bhyve monitor code that will handle setting proper status and clean up VM's resources.
Also, remove usage of virProcessKillPainfully() from domainDestroy. First, it sends SIGTERM to the process that actually triggers ACPI reset
do you mean 'ACPI shutdown' here? That's what it says in the below comments
Oops, yes, I was talking about "ACPI shutdown".
and that's not we want to do. Second, we're doing bhyvectl --destroy later and it kills bhyve process, so there's no need to manually kill it.
This seems like two distinct changes, please send as two patches and I'll review
Will do.
One general comment: what handles the equivalent of bhyveNetCleanup for graceful VM shutdown? The bhyve process itself?
No, bhyve process does not do cleanup. It works this way: * We send SIGTERM to the bhyve process - If the guest does not support ACPI shutdown, nothing happens (VM remains running like nothing has happened). - If the guest does support ACPI shutdown, it takes some time for it to shut itself down, then the corresponding bhyve process exits, however, the vmm node stays. Then code in bhyve_monitor.c notices that the bhyve process gone way and calls virBhyveProcessStop() which runs bhyvectl --destroy to cleanup the vmm node and calls bhyveNetCleanup() to cleanup networking. Roman Bogorodskiy
participants (2)
-
Cole Robinson -
Roman Bogorodskiy