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

diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
index 0e55e19772..566c672ba0 100644
--- a/src/bhyve/bhyve_monitor.c
+++ b/src/bhyve/bhyve_monitor.c
@@ -32,24 +32,82 @@
 #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, int, int, void *);
+
+static bool
+bhyveMonitorRegister(bhyveMonitorPtr mon)
+{
+    virObjectRef(mon);
+    mon->watch = virEventAddHandle(mon->kq,
+                                   VIR_EVENT_HANDLE_READABLE |
+                                   VIR_EVENT_HANDLE_ERROR |
+                                   VIR_EVENT_HANDLE_HANGUP,
+                                   bhyveMonitorIO,
+                                   mon,
+                                   virObjectFreeCallback);
+    if (mon->watch < 0) {
+        VIR_DEBUG("failed to add event handle for mon %p", mon);
+        virObjectUnref(mon);
+        return false;
+    }
+    return true;
+}
+
+static void
+bhyveMonitorUnregister(bhyveMonitorPtr mon)
+{
+    if (mon->watch < 0)
+        return;
+
+    virEventRemoveHandle(mon->watch);
+    mon->watch = -1;
+}
+
 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 +140,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; restarting 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 +197,7 @@ bhyveMonitorOpen(virDomainObjPtr vm, bhyveConnPtr driver)
         goto cleanup;
     }
 
-    mon->watch = virEventAddHandle(mon->kq,
-                                   VIR_EVENT_HANDLE_READABLE |
-                                   VIR_EVENT_HANDLE_ERROR |
-                                   VIR_EVENT_HANDLE_HANGUP,
-                                   bhyveMonitorIO,
-                                   vm,
-                                   bhyveMonitorRelease);
-    if (mon->watch < 0) {
+    if (!bhyveMonitorRegister(mon)) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("unable to register monitor events"));
         goto cleanup;
@@ -166,18 +206,30 @@ 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);
+
+    bhyveMonitorUnregister(mon);
+    virObjectUnref(mon);
 }
-- 
2.23.0


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

On Thu, Jan 02, 2020 at 05:46:31PM +0000, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
> ---
>  src/bhyve/bhyve_monitor.c | 144 ++++++++++++++++++++++++++------------
>  1 file changed, 98 insertions(+), 46 deletions(-)

FWIW, we're aiming to replace  virClass with GObject, but since
you've already done the work here I'm not going to reject it
for that reason.



> 
> diff --git a/src/bhyve/bhyve_monitor.c b/src/bhyve/bhyve_monitor.c
> index 0e55e19772..566c672ba0 100644
> --- a/src/bhyve/bhyve_monitor.c
> +++ b/src/bhyve/bhyve_monitor.c
> @@ -32,24 +32,82 @@
>  #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, int, int, void *);
> +
> +static bool
> +bhyveMonitorRegister(bhyveMonitorPtr mon)
> +{
> +    virObjectRef(mon);
> +    mon->watch = virEventAddHandle(mon->kq,
> +                                   VIR_EVENT_HANDLE_READABLE |
> +                                   VIR_EVENT_HANDLE_ERROR |
> +                                   VIR_EVENT_HANDLE_HANGUP,
> +                                   bhyveMonitorIO,
> +                                   mon,
> +                                   virObjectFreeCallback);
> +    if (mon->watch < 0) {
> +        VIR_DEBUG("failed to add event handle for mon %p", mon);
> +        virObjectUnref(mon);
> +        return false;
> +    }
> +    return true;
> +}
> +
> +static void
> +bhyveMonitorUnregister(bhyveMonitorPtr mon)
> +{
> +    if (mon->watch < 0)
> +        return;
> +
> +    virEventRemoveHandle(mon->watch);
> +    mon->watch = -1;
> +}

These two new methods are really separate refactoring from
the introduction of virClass. So as a future note, we'd generally
prefer if this had been two patches, but I'm fine merging this
one now.

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 :|

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