Use a FIFO(named pipe) for --event-monitor option in CH. Introduce a new
thread, `virCHEventHandlerLoop`, to continuously monitor and handle
events from cloud-hypervisor.

Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana@linux.microsoft.com>
---
 po/POTFILES         |  1 +
 src/ch/ch_events.c  | 93 +++++++++++++++++++++++++++++++++++++++++++++
 src/ch/ch_events.h  | 26 +++++++++++++
 src/ch/ch_monitor.c | 32 ++++++++++++++--
 src/ch/ch_monitor.h |  3 ++
 src/ch/meson.build  |  2 +
 6 files changed, 154 insertions(+), 3 deletions(-)
 create mode 100644 src/ch/ch_events.c
 create mode 100644 src/ch/ch_events.h

diff --git a/po/POTFILES b/po/POTFILES
index 1ed4086d2c..d53307cec4 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -21,6 +21,7 @@ src/bhyve/bhyve_process.c
 src/ch/ch_conf.c
 src/ch/ch_domain.c
 src/ch/ch_driver.c
+src/ch/ch_events.c
 src/ch/ch_interface.c
 src/ch/ch_monitor.c
 src/ch/ch_process.c
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
new file mode 100644
index 0000000000..899d4f48c1
--- /dev/null
+++ b/src/ch/ch_events.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright Microsoft Corp. 2024
+ *
+ * ch_events.c: Handle Cloud-Hypervisor events
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include <fcntl.h>
+
+#include "ch_events.h"
+#include "virfile.h"
+#include "virlog.h"
+
+VIR_LOG_INIT("ch.ch_events");
+
+static void virCHEventHandlerLoop(void *data)
+{
+    virCHMonitor *mon = data;
+    virDomainObj *vm = NULL;
+    int event_monitor_fd;
+
+    VIR_DEBUG("Event handler loop thread starting");
+
+    while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) {
+        if (errno == EINTR) {
+            /* 100 milli seconds */
+            g_usleep(100000);
+            continue;
+        }
+        /* Any other error should be a BUG(kernel/libc/libvirtd)
+         * (ENOMEM can happen on exceeding per-user limits)
+         */
+        VIR_ERROR(_("Failed to open the event monitor FIFO(%1$s) read end!"),
+                  mon->eventmonitorpath);
+        abort();
+    }
+    VIR_DEBUG("Opened the event monitor FIFO(%s)", mon->eventmonitorpath);
+
+    /* Obtain a vm reference */
+    vm = virObjectRef(mon->vm);
+
+    while (g_atomic_int_get(&mon->event_handler_stop) == 0) {
+        VIR_DEBUG("Reading events from event monitor file...");
+        /* Read and process events here */
+    }
+
+    VIR_FORCE_CLOSE(event_monitor_fd);
+    virObjectUnref(vm);
+
+    VIR_DEBUG("Event handler loop thread exiting");
+    return;
+}
+
+int virCHStartEventHandler(virCHMonitor *mon)
+{
+    g_autofree char *name = NULL;
+    name = g_strdup_printf("ch-evt-%d", mon->pid);
+
+    virObjectRef(mon);
+    if (virThreadCreateFull(&mon->event_handler_thread,
+                            false,
+                            virCHEventHandlerLoop,
+                            name,
+                            false,
+                            mon) < 0) {
+        virObjectUnref(mon);
+        return -1;
+    }
+    virObjectUnref(mon);
+
+    g_atomic_int_set(&mon->event_handler_stop, 0);
+    return 0;
+}
+
+void virCHStopEventHandler(virCHMonitor *mon)
+{
+    g_atomic_int_set(&mon->event_handler_stop, 1);
+}
diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
new file mode 100644
index 0000000000..4c8a48231d
--- /dev/null
+++ b/src/ch/ch_events.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright Microsoft Corp. 2024
+ *
+ * ch_events.h: header file for handling Cloud-Hypervisor events
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "ch_monitor.h"
+
+int virCHStartEventHandler(virCHMonitor *mon);
+void virCHStopEventHandler(virCHMonitor *mon);
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 8c99fe1019..696d4e4675 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -26,6 +26,7 @@
 
 #include "datatypes.h"
 #include "ch_conf.h"
+#include "ch_events.h"
 #include "ch_interface.h"
 #include "ch_monitor.h"
 #include "domain_interface.h"
@@ -572,6 +573,27 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
         return NULL;
     }
 
+    /* Event monitor file to listen for VM state changes */
+    mon->eventmonitorpath = g_strdup_printf("%s/%s-event-monitor-fifo",
+                                            cfg->stateDir, vm->def->name);
+    if (virFileExists(mon->eventmonitorpath)) {
+        VIR_WARN("Monitor file (%s) already exists, trying to delete!",
+                  mon->eventmonitorpath);
+        if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Failed to remove the file: %1$s"),
+                           mon->eventmonitorpath);
+            return NULL;
+        }
+    }
+
+    if (mkfifo(mon->eventmonitorpath, S_IWUSR | S_IRUSR) < 0 &&
+            errno != EEXIST) {
+        virReportSystemError(errno, "%s",
+                             _("Cannot create monitor FIFO"));
+        return NULL;
+    }
+
     cmd = virCommandNew(vm->def->emulator);
     virCommandSetUmask(cmd, 0x002);
     socket_fd = chMonitorCreateSocket(mon->socketpath);
@@ -593,12 +615,15 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
     if (virCommandRunAsync(cmd, &mon->pid) < 0)
         return NULL;
 
-    /* get a curl handle */
-    mon->handle = curl_easy_init();
-
     /* now has its own reference */
     mon->vm = virObjectRef(vm);
 
+    if (virCHStartEventHandler(mon) < 0)
+        return NULL;
+
+    /* get a curl handle */
+    mon->handle = curl_easy_init();
+
     return g_steal_pointer(&mon);
 }
 
@@ -633,6 +658,7 @@ void virCHMonitorClose(virCHMonitor *mon)
         g_free(mon->socketpath);
     }
 
+    virCHStopEventHandler(mon);
     if (mon->eventmonitorpath) {
         if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
             VIR_WARN("Unable to remove CH event monitor file '%s'",
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index 2ef8706b99..878a185f29 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -97,6 +97,9 @@ struct _virCHMonitor {
 
     char *eventmonitorpath;
 
+    virThread event_handler_thread;
+    int event_handler_stop;
+
     pid_t pid;
 
     virDomainObj *vm;
diff --git a/src/ch/meson.build b/src/ch/meson.build
index 633966aac7..684beac1f2 100644
--- a/src/ch/meson.build
+++ b/src/ch/meson.build
@@ -7,6 +7,8 @@ ch_driver_sources = [
   'ch_domain.h',
   'ch_driver.c',
   'ch_driver.h',
+  'ch_events.c',
+  'ch_driver.h',
   'ch_interface.c',
   'ch_interface.h',
   'ch_monitor.c',
-- 
2.34.1

On 10/1/2024 7:20 AM, Purna Pavan Chandra Aekkaladevi wrote:
> Use a FIFO(named pipe) for --event-monitor option in CH. Introduce a new
> thread, `virCHEventHandlerLoop`, to continuously monitor and handle
> events from cloud-hypervisor.
> 
> Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
> Co-authored-by: Vineeth Pillai <viremana@linux.microsoft.com>
> ---
>   po/POTFILES         |  1 +
>   src/ch/ch_events.c  | 93 +++++++++++++++++++++++++++++++++++++++++++++
>   src/ch/ch_events.h  | 26 +++++++++++++
>   src/ch/ch_monitor.c | 32 ++++++++++++++--
>   src/ch/ch_monitor.h |  3 ++
>   src/ch/meson.build  |  2 +
>   6 files changed, 154 insertions(+), 3 deletions(-)
>   create mode 100644 src/ch/ch_events.c
>   create mode 100644 src/ch/ch_events.h
> 
> diff --git a/po/POTFILES b/po/POTFILES
> index 1ed4086d2c..d53307cec4 100644
> --- a/po/POTFILES
> +++ b/po/POTFILES
> @@ -21,6 +21,7 @@ src/bhyve/bhyve_process.c
>   src/ch/ch_conf.c
>   src/ch/ch_domain.c
>   src/ch/ch_driver.c
> +src/ch/ch_events.c
>   src/ch/ch_interface.c
>   src/ch/ch_monitor.c
>   src/ch/ch_process.c
> diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
> new file mode 100644
> index 0000000000..899d4f48c1
> --- /dev/null
> +++ b/src/ch/ch_events.c
> @@ -0,0 +1,93 @@
> +/*
> + * Copyright Microsoft Corp. 2024
> + *
> + * ch_events.c: Handle Cloud-Hypervisor events
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <config.h>
> +
> +#include <fcntl.h>
> +
> +#include "ch_events.h"
> +#include "virfile.h"
> +#include "virlog.h"
> +
> +VIR_LOG_INIT("ch.ch_events");
> +
> +static void virCHEventHandlerLoop(void *data)
> +{
> +    virCHMonitor *mon = data;
> +    virDomainObj *vm = NULL;
> +    int event_monitor_fd;
> +
> +    VIR_DEBUG("Event handler loop thread starting");

consider adding thread name to the debug msg.

> +
> +    while ((event_monitor_fd = open(mon->eventmonitorpath, O_RDONLY)) < 0) {
> +        if (errno == EINTR) {
> +            /* 100 milli seconds */
> +            g_usleep(100000);
> +            continue;
> +        }
> +        /* Any other error should be a BUG(kernel/libc/libvirtd)
> +         * (ENOMEM can happen on exceeding per-user limits)
> +         */
> +        VIR_ERROR(_("Failed to open the event monitor FIFO(%1$s) read end!"),
> +                  mon->eventmonitorpath);
> +        abort();
> +    }
> +    VIR_DEBUG("Opened the event monitor FIFO(%s)", mon->eventmonitorpath);
> +
> +    /* Obtain a vm reference */
> +    vm = virObjectRef(mon->vm);
> +
> +    while (g_atomic_int_get(&mon->event_handler_stop) == 0) {
> +        VIR_DEBUG("Reading events from event monitor file...");
> +        /* Read and process events here */
> +    }
> +
> +    VIR_FORCE_CLOSE(event_monitor_fd);
> +    virObjectUnref(vm);
> +
> +    VIR_DEBUG("Event handler loop thread exiting");
same as above
> +    return;
> +}
> +
> +int virCHStartEventHandler(virCHMonitor *mon)
> +{
> +    g_autofree char *name = NULL;
> +    name = g_strdup_printf("ch-evt-%d", mon->pid);
> +
> +    virObjectRef(mon);
> +    if (virThreadCreateFull(&mon->event_handler_thread,
> +                            false,
> +                            virCHEventHandlerLoop,
> +                            name,
> +                            false,
> +                            mon) < 0) {
> +        virObjectUnref(mon);
> +        return -1;
> +    }
> +    virObjectUnref(mon);
> +
> +    g_atomic_int_set(&mon->event_handler_stop, 0);
> +    return 0;
> +}
> +
> +void virCHStopEventHandler(virCHMonitor *mon)
> +{
> +    g_atomic_int_set(&mon->event_handler_stop, 1);
> +}
> diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
> new file mode 100644
> index 0000000000..4c8a48231d
> --- /dev/null
> +++ b/src/ch/ch_events.h
> @@ -0,0 +1,26 @@
> +/*
> + * Copyright Microsoft Corp. 2024
> + *
> + * ch_events.h: header file for handling Cloud-Hypervisor events
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library.  If not, see
> + * <http://www.gnu.org/licenses/>.
> + */
> +
> +#pragma once
> +
> +#include "ch_monitor.h"
> +
> +int virCHStartEventHandler(virCHMonitor *mon);
> +void virCHStopEventHandler(virCHMonitor *mon);
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index 8c99fe1019..696d4e4675 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -26,6 +26,7 @@
>   
>   #include "datatypes.h"
>   #include "ch_conf.h"
> +#include "ch_events.h"
>   #include "ch_interface.h"
>   #include "ch_monitor.h"
>   #include "domain_interface.h"
> @@ -572,6 +573,27 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
>           return NULL;
>       }
>   
> +    /* Event monitor file to listen for VM state changes */
> +    mon->eventmonitorpath = g_strdup_printf("%s/%s-event-monitor-fifo",
> +                                            cfg->stateDir, vm->def->name);
> +    if (virFileExists(mon->eventmonitorpath)) {
> +        VIR_WARN("Monitor file (%s) already exists, trying to delete!",
> +                  mon->eventmonitorpath);
> +        if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Failed to remove the file: %1$s"),
> +                           mon->eventmonitorpath);
> +            return NULL;
> +        }
> +    }
> +
> +    if (mkfifo(mon->eventmonitorpath, S_IWUSR | S_IRUSR) < 0 &&
> +            errno != EEXIST) {
> +        virReportSystemError(errno, "%s",
> +                             _("Cannot create monitor FIFO"));
> +        return NULL;
> +    }
> +
>       cmd = virCommandNew(vm->def->emulator);
>       virCommandSetUmask(cmd, 0x002);
>       socket_fd = chMonitorCreateSocket(mon->socketpath);
> @@ -593,12 +615,15 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
>       if (virCommandRunAsync(cmd, &mon->pid) < 0)
>           return NULL;
>   
> -    /* get a curl handle */
> -    mon->handle = curl_easy_init();
> -
>       /* now has its own reference */
>       mon->vm = virObjectRef(vm);
>   
> +    if (virCHStartEventHandler(mon) < 0)
> +        return NULL;
> +
> +    /* get a curl handle */
> +    mon->handle = curl_easy_init();
> +
>       return g_steal_pointer(&mon);
>   }
>   
> @@ -633,6 +658,7 @@ void virCHMonitorClose(virCHMonitor *mon)
>           g_free(mon->socketpath);
>       }
>   
> +    virCHStopEventHandler(mon);
>       if (mon->eventmonitorpath) {
>           if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
>               VIR_WARN("Unable to remove CH event monitor file '%s'",
> diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
> index 2ef8706b99..878a185f29 100644
> --- a/src/ch/ch_monitor.h
> +++ b/src/ch/ch_monitor.h
> @@ -97,6 +97,9 @@ struct _virCHMonitor {
>   
>       char *eventmonitorpath;
>   
> +    virThread event_handler_thread;
> +    int event_handler_stop;
> +
>       pid_t pid;
>   
>       virDomainObj *vm;
> diff --git a/src/ch/meson.build b/src/ch/meson.build
> index 633966aac7..684beac1f2 100644
> --- a/src/ch/meson.build
> +++ b/src/ch/meson.build
> @@ -7,6 +7,8 @@ ch_driver_sources = [
>     'ch_domain.h',
>     'ch_driver.c',
>     'ch_driver.h',
> +  'ch_events.c',
> +  'ch_driver.h',
should this be ch_events.h?
>     'ch_interface.c',
>     'ch_interface.h',
>     'ch_monitor.c',

-- 
Regards,
Praveen K Paladugu