This makes lifecycle management a bit easier thanks to ref counting, and
it is closer to what the qemu driver does.

Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
---
 src/bhyve/bhyve_monitor.c | 116 +++++++++++++++++++++++++-------------
 1 file changed, 76 insertions(+), 40 deletions(-)

diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
index 0e55e19772..58d40e1f70 100644
--- a/src/bhyve/bhyve_monitor.c
+++ b/src/bhyve/bhyve_monitor.c
@@ -32,24 +32,51 @@
 #include "virerror.h"
 #include "virfile.h"
 #include "virlog.h"
+#include "virobject.h"
 
 #define VIR_FROM_THIS   VIR_FROM_BHYVE
 
 VIR_LOG_INIT("bhyve.bhyve_monitor");
 
 struct _bhyveMonitor {
+    virObject parent;
+
     int kq;
     int watch;
     bhyveConnPtr driver;
+    virDomainObjPtr vm;
 };
 
+static virClassPtr bhyveMonitorClass;
+
+static void
+bhyveMonitorDispose(void *obj)
+{
+    bhyveMonitorPtr mon = obj;
+
+    VIR_FORCE_CLOSE(mon->kq);
+    virObjectUnref(mon->vm);
+}
+
+static int
+bhyveMonitorOnceInit(void)
+{
+    if (!VIR_CLASS_NEW(bhyveMonitor, virClassForObject()))
+        return -1;
+
+    return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(bhyveMonitor);
+
 static void
 bhyveMonitorIO(int watch, int kq, int events G_GNUC_UNUSED, void *opaque)
 {
     const struct timespec zerowait = { 0, 0 };
-    virDomainObjPtr vm = opaque;
-    bhyveDomainObjPrivatePtr priv = vm->privateData;
-    bhyveMonitorPtr mon = priv->mon;
+    bhyveMonitorPtr mon = opaque;
+    virDomainObjPtr vm = mon->vm;
+    bhyveConnPtr driver = mon->driver;
+    const char *name;
     struct kevent kev;
     int rc, status;
 
@@ -82,60 +109,49 @@ bhyveMonitorIO(int watch, int kq, int events G_GNUC_UNUSED, void *opaque)
             return;
         }
 
+        name = vm->def->name;
         status = kev.data;
         if (WIFSIGNALED(status) && WCOREDUMP(status)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Guest %s got signal %d and crashed"),
-                           vm->def->name,
-                           WTERMSIG(status));
-            virBhyveProcessStop(mon->driver, vm,
-                                VIR_DOMAIN_SHUTOFF_CRASHED);
+                           name, WTERMSIG(status));
+            virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED);
         } else if (WIFEXITED(status)) {
             if (WEXITSTATUS(status) == 0) {
                 /* 0 - reboot */
                 /* TODO: Implementing reboot is a little more complicated. */
-                VIR_INFO("Guest %s rebooted; destroying domain.",
-                         vm->def->name);
-                virBhyveProcessStop(mon->driver, vm,
-                                    VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+                VIR_INFO("Guest %s rebooted; destroying domain.", name);
+                virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
             } else if (WEXITSTATUS(status) < 3) {
                 /* 1 - shutdown, 2 - halt, 3 - triple fault. others - error */
-                VIR_INFO("Guest %s shut itself down; destroying domain.",
-                         vm->def->name);
-                virBhyveProcessStop(mon->driver, vm,
-                                    VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+                VIR_INFO("Guest %s shut itself down; destroying domain.", name);
+                virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
             } else {
                 VIR_INFO("Guest %s had an error and exited with status %d; destroying domain.",
-                         vm->def->name, WEXITSTATUS(status));
-                virBhyveProcessStop(mon->driver, vm,
-                                    VIR_DOMAIN_SHUTOFF_UNKNOWN);
+                         name, WEXITSTATUS(status));
+                virBhyveProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN);
             }
         }
     }
 }
 
-static void
-bhyveMonitorRelease(void *opaque)
-{
-    virDomainObjPtr vm = opaque;
-    bhyveDomainObjPrivatePtr priv = vm->privateData;
-    bhyveMonitorPtr mon = priv->mon;
-
-    VIR_FORCE_CLOSE(mon->kq);
-    VIR_FREE(mon);
-}
-
-bhyveMonitorPtr
-bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
+static bhyveMonitorPtr
+bhyveMonitorOpenImpl(virDomainObjPtr vm, bhyveConnPtr driver)
 {
-    bhyveMonitorPtr mon = NULL;
+    bhyveMonitorPtr mon;
     struct kevent kev;
 
-    if (VIR_ALLOC(mon) < 0)
+    if (bhyveMonitorInitialize() < 0)
+        return NULL;
+
+    if (!(mon = virObjectNew(bhyveMonitorClass)))
         return NULL;
 
     mon->driver = driver;
 
+    virObjectRef(vm);
+    mon->vm = vm;
+
     mon->kq = kqueue();
     if (mon->kq < 0) {
         virReportError(VIR_ERR_SYSTEM_ERROR, "%s",
@@ -150,14 +166,17 @@ bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
         goto cleanup;
     }
 
+    virObjectRef(mon);
     mon->watch = virEventAddHandle(mon->kq,
                                    VIR_EVENT_HANDLE_READABLE |
                                    VIR_EVENT_HANDLE_ERROR |
                                    VIR_EVENT_HANDLE_HANGUP,
                                    bhyveMonitorIO,
-                                   vm,
-                                   bhyveMonitorRelease);
+                                   mon,
+                                   virObjectFreeCallback);
     if (mon->watch < 0) {
+        VIR_DEBUG("failed to add event handle for mon %p", mon);
+        virObjectUnref(mon);
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("unable to register monitor events"));
         goto cleanup;
@@ -166,18 +185,35 @@ bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
     return mon;
 
  cleanup:
-    bhyveMonitorRelease(mon);
+    bhyveMonitorClose(mon);
     return NULL;
 }
 
+bhyveMonitorPtr
+bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
+{
+    bhyveMonitorPtr mon;
+
+    virObjectRef(vm);
+    mon = bhyveMonitorOpenImpl(vm, driver);
+    virObjectUnref(vm);
+
+    return mon;
+}
+
 void
 bhyveMonitorClose(bhyveMonitorPtr mon)
 {
     if (mon == NULL)
         return;
 
-    if (mon->watch > 0)
-        virEventRemoveHandle(mon->watch);
-    else
-        bhyveMonitorRelease(mon);
+    VIR_DEBUG("cleaning up bhyveMonitor %p", mon);
+
+    if (mon->watch < 0)
+        return;
+
+    virEventRemoveHandle(mon->watch);
+    mon->watch = -1;
+
+    virObjectUnref(mon);
 }
-- 
2.24.1

On Mon, Feb 24, 2020 at 01:46:16AM -0500, Ryan Moeller wrote:
> This makes lifecycle management a bit easier thanks to ref counting, and
> it is closer to what the qemu driver does.

FWIW, if you want todo any more conversions in the future, note that
we're intending to replace virObject with GObject.  eg as illustrated
in:

  commit 16121a88a7ef933220bcf9eff6575367278a06f7
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Sep 19 15:38:03 2019 +0100

    util: convert virIdentity class to use GObject

> 
> Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
> ---
>  src/bhyve/bhyve_monitor.c | 116 +++++++++++++++++++++++++-------------
>  1 file changed, 76 insertions(+), 40 deletions(-)

None the less, I'm fine taking this conversion to virObject
as is, since it is better than what currently exists.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|