[PATCH v2] bhyve: implement domainShutdown

[Roman Bogorodskiy <bogorodskiy@xxxxxxxxx>]

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.html

 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list