8:02 a.m.
cloud-hypervisor raises various events, including VM lifecylce operations
such as boot, shutdown, pause, resume, etc. Libvirt will now read these
events and take the necessary actions, such as correctly updating the
domain state. A FIFO file is passed to `--event-monitor` option of
cloud-hypervisor. Libvirt creates a new thread that acts as the reader
of the fifo file and continuously monitors for new events. Currently,
shutdown events are handled by updating the domain state appropriately.
Purna Pavan Chandra Aekkaladevi (6):
utils: Implement virFileIsNamedPipe
ch: pass --event-monitor option to cloud-hypervisor
ch: start a new thread for handling ch events
ch: events: Read and parse cloud-hypervisor events
ch: events: facilitate lifecycle events handling
NEWS: Mention event handling support in ch driver
NEWS.rst | 7 +
po/POTFILES | 1 +
src/ch/ch_events.c | 337 +++++++++++++++++++++++++++++++++++++++
src/ch/ch_events.h | 54 +++++++
src/ch/ch_monitor.c | 48 +++++-
src/ch/ch_monitor.h | 11 ++
src/ch/meson.build | 2 +
src/libvirt_private.syms | 1 +
src/util/virfile.c | 8 +
src/util/virfile.h | 1 +
10 files changed, 466 insertions(+), 4 deletions(-)
create mode 100644 src/ch/ch_events.c
create mode 100644 src/ch/ch_events.h
--
2.34.1
8:02 a.m.
New subject: [PATCH 1/6] utils: Implement virFileIsNamedPipe
virFileIsNamedPipe checks whether passed path is a FIFO file or not.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
src/libvirt_private.syms | 1 +
src/util/virfile.c | 8 ++++++++
src/util/virfile.h | 1 +
3 files changed, 10 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f15d16c292..4dac9c17f6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2357,6 +2357,7 @@ virFileIsDir;
virFileIsExecutable;
virFileIsLink;
virFileIsMountPoint;
+virFileIsNamedPipe;
virFileIsRegular;
virFileIsSharedFS;
virFileIsSharedFSType;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index d820172405..19b074ed8a 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2023,6 +2023,14 @@ virFileIsDir(const char *path)
}
+bool
+virFileIsNamedPipe(const char *path)
+{
+ struct stat s;
+ return (stat(path, &s) == 0) && S_ISFIFO(s.st_mode);
+}
+
+
bool
virFileIsRegular(const char *path)
{
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 7df3fcb840..4f7ff72483 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -215,6 +215,7 @@ void virFileActivateDirOverrideForLib(void);
off_t virFileLength(const char *path, int fd) ATTRIBUTE_NONNULL(1);
bool virFileIsDir (const char *file) ATTRIBUTE_NONNULL(1);
+bool virFileIsNamedPipe (const char *file) ATTRIBUTE_NONNULL(1);
bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1) G_NO_INLINE;
bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
bool virFileIsRegular(const char *file) ATTRIBUTE_NONNULL(1);
--
2.34.1
8:02 a.m.
New subject: [PATCH 2/6] ch: pass --event-monitor option to cloud-hypervisor
The `--event-monitor` option in cloud-hypervisor outputs events to a
specified file. This file can then be used to monitor VM lifecycle,
other vmm events and trigger appropriate actions.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
src/ch/ch_monitor.c | 20 ++++++++++++++++----
src/ch/ch_monitor.h | 2 ++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 3e49902791..8c99fe1019 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -540,7 +540,6 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
{
g_autoptr(virCHMonitor) mon = NULL;
g_autoptr(virCommand) cmd = NULL;
- const char *socketdir = cfg->stateDir;
int socket_fd = 0;
if (virCHMonitorInitialize() < 0)
@@ -556,11 +555,13 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
}
/* prepare to launch Cloud-Hypervisor socket */
- mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
vm->def->name);
- if (g_mkdir_with_parents(socketdir, 0777) < 0) {
+ mon->socketpath = g_strdup_printf("%s/%s-socket", cfg->stateDir,
vm->def->name);
+ mon->eventmonitorpath = g_strdup_printf("%s/%s-event-monitor",
+ cfg->stateDir, vm->def->name);
+ if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) {
virReportSystemError(errno,
_("Cannot create socket directory
'%1$s'"),
- socketdir);
+ cfg->stateDir);
return NULL;
}
@@ -585,6 +586,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
virCommandAddArgFormat(cmd, "fd=%d", socket_fd);
virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ virCommandAddArg(cmd, "--event-monitor");
+ virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath);
+
/* launch Cloud-Hypervisor socket */
if (virCommandRunAsync(cmd, &mon->pid) < 0)
return NULL;
@@ -629,6 +633,14 @@ void virCHMonitorClose(virCHMonitor *mon)
g_free(mon->socketpath);
}
+ if (mon->eventmonitorpath) {
+ if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
+ VIR_WARN("Unable to remove CH event monitor file '%s'",
+ mon->eventmonitorpath);
+ }
+ g_free(mon->eventmonitorpath);
+ }
+
virObjectUnref(mon);
}
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index b35f5ea027..2ef8706b99 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -95,6 +95,8 @@ struct _virCHMonitor {
char *socketpath;
+ char *eventmonitorpath;
+
pid_t pid;
virDomainObj *vm;
--
2.34.1
4:25 p.m.
New subject: [PATCH 2/6] ch: pass --event-monitor option to cloud-hypervisor
On 9/19/2024 8:02 AM, Purna Pavan Chandra Aekkaladevi wrote:
The `--event-monitor` option in cloud-hypervisor outputs events to a
specified file. This file can then be used to monitor VM lifecycle,
other vmm events and trigger appropriate actions.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
src/ch/ch_monitor.c | 20 ++++++++++++++++----
src/ch/ch_monitor.h | 2 ++
2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 3e49902791..8c99fe1019 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -540,7 +540,6 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
{
g_autoptr(virCHMonitor) mon = NULL;
g_autoptr(virCommand) cmd = NULL;
- const char *socketdir = cfg->stateDir;
int socket_fd = 0;
if (virCHMonitorInitialize() < 0)
@@ -556,11 +555,13 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
}
/* prepare to launch Cloud-Hypervisor socket */
- mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
vm->def->name);
- if (g_mkdir_with_parents(socketdir, 0777) < 0) {
+ mon->socketpath = g_strdup_printf("%s/%s-socket", cfg->stateDir,
vm->def->name);
+ mon->eventmonitorpath = g_strdup_printf("%s/%s-event-monitor",
+ cfg->stateDir, vm->def->name);
+ if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) {
virReportSystemError(errno,
_("Cannot create socket directory
'%1$s'"),
- socketdir);
+ cfg->stateDir);
return NULL;
}
@@ -585,6 +586,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
virCommandAddArgFormat(cmd, "fd=%d", socket_fd);
virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+ virCommandAddArg(cmd, "--event-monitor");
+ virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath);
+
/* launch Cloud-Hypervisor socket */
if (virCommandRunAsync(cmd, &mon->pid) < 0)
return NULL;
@@ -629,6 +633,14 @@ void virCHMonitorClose(virCHMonitor *mon)
g_free(mon->socketpath);
}
+ if (mon->eventmonitorpath) {
+ if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
+ VIR_WARN("Unable to remove CH event monitor file '%s'",
+ mon->eventmonitorpath);
+ }
+ g_free(mon->eventmonitorpath);
+ }
+
Do you really need to remove mon->eventmonitorpath here?
In patch3, you introduce `virCHStopEventHandler`, which causes the
thread to exit its while loop and run
`VIR_FORCE_CLOSE(event_monitor_fd)`, which also deletes the
event_monitor file.
virObjectUnref(mon);
}
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index b35f5ea027..2ef8706b99 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -95,6 +95,8 @@ struct _virCHMonitor {
char *socketpath;
+ char *eventmonitorpath;
+
pid_t pid;
virDomainObj *vm;
--
Regards,
Praveen K Paladugu
8:07 a.m.
New subject: [PATCH 2/6] ch: pass --event-monitor option to cloud-hypervisor
On Mon, Sep 23, 2024 at 04:25:54PM -0500, Praveen K Paladugu wrote:
On 9/19/2024 8:02 AM, Purna Pavan Chandra Aekkaladevi wrote:
>The `--event-monitor` option in cloud-hypervisor outputs events to a
>specified file. This file can then be used to monitor VM lifecycle,
>other vmm events and trigger appropriate actions.
>
>Signed-off-by: Purna Pavan Chandra Aekkaladevi
<paekkaladevi(a)linux.microsoft.com>
>Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
>---
> src/ch/ch_monitor.c | 20 ++++++++++++++++----
> src/ch/ch_monitor.h | 2 ++
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
>diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
>index 3e49902791..8c99fe1019 100644
>--- a/src/ch/ch_monitor.c
>+++ b/src/ch/ch_monitor.c
>@@ -540,7 +540,6 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
> {
> g_autoptr(virCHMonitor) mon = NULL;
> g_autoptr(virCommand) cmd = NULL;
>- const char *socketdir = cfg->stateDir;
> int socket_fd = 0;
> if (virCHMonitorInitialize() < 0)
>@@ -556,11 +555,13 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
> }
> /* prepare to launch Cloud-Hypervisor socket */
>- mon->socketpath = g_strdup_printf("%s/%s-socket", socketdir,
vm->def->name);
>- if (g_mkdir_with_parents(socketdir, 0777) < 0) {
>+ mon->socketpath = g_strdup_printf("%s/%s-socket", cfg->stateDir,
vm->def->name);
>+ mon->eventmonitorpath = g_strdup_printf("%s/%s-event-monitor",
>+ cfg->stateDir, vm->def->name);
>+ if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) {
> virReportSystemError(errno,
> _("Cannot create socket directory
'%1$s'"),
>- socketdir);
>+ cfg->stateDir);
> return NULL;
> }
>@@ -585,6 +586,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
> virCommandAddArgFormat(cmd, "fd=%d", socket_fd);
> virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
>+ virCommandAddArg(cmd, "--event-monitor");
>+ virCommandAddArgFormat(cmd, "path=%s", mon->eventmonitorpath);
>+
> /* launch Cloud-Hypervisor socket */
> if (virCommandRunAsync(cmd, &mon->pid) < 0)
> return NULL;
>@@ -629,6 +633,14 @@ void virCHMonitorClose(virCHMonitor *mon)
> g_free(mon->socketpath);
> }
>+ if (mon->eventmonitorpath) {
>+ if (virFileRemove(mon->eventmonitorpath, -1, -1) < 0) {
>+ VIR_WARN("Unable to remove CH event monitor file
'%s'",
>+ mon->eventmonitorpath);
>+ }
>+ g_free(mon->eventmonitorpath);
>+ }
>+
Do you really need to remove mon->eventmonitorpath here?
In patch3, you introduce `virCHStopEventHandler`, which causes the
thread to exit its while loop and run
`VIR_FORCE_CLOSE(event_monitor_fd)`, which also deletes the
event_monitor file.
`VIR_FORCE_CLOSE` just closes the fd. The file needs to be explicitly
deleted.
> virObjectUnref(mon);
> }
>diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
>index b35f5ea027..2ef8706b99 100644
>--- a/src/ch/ch_monitor.h
>+++ b/src/ch/ch_monitor.h
>@@ -95,6 +95,8 @@ struct _virCHMonitor {
> char *socketpath;
>+ char *eventmonitorpath;
>+
> pid_t pid;
> virDomainObj *vm;
Regards,
Purna Pavan Chandra
8:02 a.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
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(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
po/POTFILES | 1 +
src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
src/ch/ch_events.h | 26 ++++++++++++
src/ch/ch_monitor.c | 28 +++++++++++++
src/ch/ch_monitor.h | 3 ++
src/ch/meson.build | 2 +
6 files changed, 156 insertions(+)
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..851fbc9f26
--- /dev/null
+++ b/src/ch/ch_events.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright Microsoft Corp. 2024
+ *
+ * ch_events.h: 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) {
+ g_usleep(100000); // 100 milli seconds
+ 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);
+
+ /*
+ * We would need to wait until VM is initialized.
+ */
+ while (!(vm = virObjectRef(mon->vm)))
+ g_usleep(100000); // 100 milli seconds
+
+ 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("event-handler-%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..5e43bf9ef8 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,28 @@ 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) &&
+ !virFileIsNamedPipe(mon->eventmonitorpath)) {
+ VIR_WARN("Monitor file (%s) is not a FIFO, 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,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
if (virCommandRunAsync(cmd, &mon->pid) < 0)
return NULL;
+ if (virCHStartEventHandler(mon) < 0)
+ return NULL;
+
/* get a curl handle */
mon->handle = curl_easy_init();
@@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
g_free(mon->eventmonitorpath);
}
+ virCHStopEventHandler(mon);
+
virObjectUnref(mon);
}
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
4:29 p.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
On 9/19/2024 8:02 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(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
po/POTFILES | 1 +
src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
src/ch/ch_events.h | 26 ++++++++++++
src/ch/ch_monitor.c | 28 +++++++++++++
src/ch/ch_monitor.h | 3 ++
src/ch/meson.build | 2 +
6 files changed, 156 insertions(+)
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..851fbc9f26
--- /dev/null
+++ b/src/ch/ch_events.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright Microsoft Corp. 2024
+ *
+ * ch_events.h: Handle Cloud-Hypervisor events
please fix the filename here.
+ *
+ * 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) {
+ g_usleep(100000); // 100 milli seconds
+ 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);
+
+ /*
+ * We would need to wait until VM is initialized.
+ */
Does this comment apply to next while loop where the events are
processed?
+ while (!(vm = virObjectRef(mon->vm)))
+ g_usleep(100000); // 100 milli seconds
In this while loop you are only getting a reference to vm object.
+
+ 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("event-handler-%d", mon->pid);
It would be better to name the thread something similar to
"event-handler_ch_${pid}_${vmname}". A similar format is
used for cgroup naming.
+
+ 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..5e43bf9ef8 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,28 @@ 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) &&
+ !virFileIsNamedPipe(mon->eventmonitorpath)) {
+ VIR_WARN("Monitor file (%s) is not a FIFO, 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;
+ }
+ }
+
This does not cover the case when eventmonitorpath exists, and is a
named pipe, may be, not cleaned up from a previous run.
You are better off just deleting the file and open a new one with
mkfifo below.
+ 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,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
if (virCommandRunAsync(cmd, &mon->pid) < 0)
return NULL;
+ if (virCHStartEventHandler(mon) < 0)
+ return NULL;
+
/* get a curl handle */
mon->handle = curl_easy_init();
@@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
g_free(mon->eventmonitorpath);
}
+ virCHStopEventHandler(mon);
+
Please move virCHStopEventHandler above `if (mon->eventmonitorpath) {`
section. Doing so, will allow the event monitor thread to cleanly exit,
before you invoke `virFileRemove(mon->eventmonitorpath...`
virObjectUnref(mon);
}
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',
--
Regards,
Praveen K Paladugu
8:22 a.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
On Mon, Sep 23, 2024 at 04:29:35PM -0500, Praveen K Paladugu wrote:
On 9/19/2024 8:02 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(a)linux.microsoft.com>
>Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
>---
> po/POTFILES | 1 +
> src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
> src/ch/ch_events.h | 26 ++++++++++++
> src/ch/ch_monitor.c | 28 +++++++++++++
> src/ch/ch_monitor.h | 3 ++
> src/ch/meson.build | 2 +
> 6 files changed, 156 insertions(+)
> 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..851fbc9f26
>--- /dev/null
>+++ b/src/ch/ch_events.c
>@@ -0,0 +1,96 @@
>+/*
>+ * Copyright Microsoft Corp. 2024
>+ *
>+ * ch_events.h: Handle Cloud-Hypervisor events
please fix the filename here.
>+ *
>+ * 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) {
>+ g_usleep(100000); // 100 milli seconds
>+ 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);
>+
>+ /*
>+ * We would need to wait until VM is initialized.
>+ */
Does this comment apply to next while loop where the events are
processed?
No, this applies to the immediate while loop where vm ref is obtained.
>+ while (!(vm = virObjectRef(mon->vm)))
>+ g_usleep(100000); // 100 milli seconds
In this while loop you are only getting a reference to vm object.
Yes. At this point, we are not sure if mon->vm is initialized. The
parent thread initializes mon->vm only after starting this event handler
thread. Hence, wait until we have a vm ref.
>+
>+ 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("event-handler-%d", mon->pid);
It would be better to name the thread something similar to
"event-handler_ch_${pid}_${vmname}". A similar format is
used for cgroup naming.
>+
>+ 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..5e43bf9ef8 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,28 @@ 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) &&
>+ !virFileIsNamedPipe(mon->eventmonitorpath)) {
>+ VIR_WARN("Monitor file (%s) is not a FIFO, 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;
>+ }
>+ }
>+
This does not cover the case when eventmonitorpath exists, and is a
named pipe, may be, not cleaned up from a previous run.
You are better off just deleting the file and open a new one with
mkfifo below.
>+ 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,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
> if (virCommandRunAsync(cmd, &mon->pid) < 0)
> return NULL;
>+ if (virCHStartEventHandler(mon) < 0)
>+ return NULL;
>+
> /* get a curl handle */
> mon->handle = curl_easy_init();
>@@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
> g_free(mon->eventmonitorpath);
> }
>+ virCHStopEventHandler(mon);
>+
Please move virCHStopEventHandler above `if (mon->eventmonitorpath) {`
section. Doing so, will allow the event monitor thread to cleanly
exit, before you invoke `virFileRemove(mon->eventmonitorpath...`
Thanks for this catch. I will move it.
And will also apply other suggestions.
> virObjectUnref(mon);
> }
>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',
--
Regards,
Praveen K Paladugu
Regards,
Purna Pavan Chandra
11:11 a.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
On 9/24/2024 8:22 AM, Purna Pavan Chandra Aekkaladevi wrote:
On Mon, Sep 23, 2024 at 04:29:35PM -0500, Praveen K Paladugu wrote:
>
>
> On 9/19/2024 8:02 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(a)linux.microsoft.com>
>> Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
>> ---
>> po/POTFILES | 1 +
>> src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
>> src/ch/ch_events.h | 26 ++++++++++++
>> src/ch/ch_monitor.c | 28 +++++++++++++
>> src/ch/ch_monitor.h | 3 ++
>> src/ch/meson.build | 2 +
>> 6 files changed, 156 insertions(+)
>> 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..851fbc9f26
>> --- /dev/null
>> +++ b/src/ch/ch_events.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright Microsoft Corp. 2024
>> + *
>> + * ch_events.h: Handle Cloud-Hypervisor events
>
> please fix the filename here.
>
>> + *
>> + * 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) {
>> + g_usleep(100000); // 100 milli seconds
>> + 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);
>> +
>> + /*
>> + * We would need to wait until VM is initialized.
>> + */
> Does this comment apply to next while loop where the events are
> processed?
>
No, this applies to the immediate while loop where vm ref is obtained.
>> + while (!(vm = virObjectRef(mon->vm)))
>> + g_usleep(100000); // 100 milli seconds
>
> In this while loop you are only getting a reference to vm object.
>
Yes. At this point, we are not sure if mon->vm is initialized. The
parent thread initializes mon->vm only after starting this event handler
thread. Hence, wait until we have a vm ref.
Why do you need to wait here? Is there a race condition here?
If yes, will just waiting for a reference for vm object enough?
>> +
>> + 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("event-handler-%d", mon->pid);
>
> It would be better to name the thread something similar to
> "event-handler_ch_${pid}_${vmname}". A similar format is
> used for cgroup naming.
>
>> +
>> + 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..5e43bf9ef8 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,28 @@ 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) &&
>> + !virFileIsNamedPipe(mon->eventmonitorpath)) {
>> + VIR_WARN("Monitor file (%s) is not a FIFO, 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;
>> + }
>> + }
>> +
> This does not cover the case when eventmonitorpath exists, and is a
> named pipe, may be, not cleaned up from a previous run.
>
> You are better off just deleting the file and open a new one with
> mkfifo below.
>
>> + 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,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
>> if (virCommandRunAsync(cmd, &mon->pid) < 0)
>> return NULL;
>> + if (virCHStartEventHandler(mon) < 0)
>> + return NULL;
>> +
>> /* get a curl handle */
>> mon->handle = curl_easy_init();
>> @@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
>> g_free(mon->eventmonitorpath);
>> }
>> + virCHStopEventHandler(mon);
>> +
> Please move virCHStopEventHandler above `if (mon->eventmonitorpath) {`
> section. Doing so, will allow the event monitor thread to cleanly
> exit, before you invoke `virFileRemove(mon->eventmonitorpath...`
Thanks for this catch. I will move it.
And will also apply other suggestions.
>
>> virObjectUnref(mon);
>> }
>> 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',
>
>
> --
> Regards,
> Praveen K Paladugu
Regards,
Purna Pavan Chandra
--
Regards,
Praveen K Paladugu
6:11 a.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
On Thu, Sep 26, 2024 at 11:11:54AM -0500, Praveen K Paladugu wrote:
On 9/24/2024 8:22 AM, Purna Pavan Chandra Aekkaladevi wrote:
> On Mon, Sep 23, 2024 at 04:29:35PM -0500, Praveen K Paladugu wrote:
> >
> >
> > On 9/19/2024 8:02 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(a)linux.microsoft.com>
> > > Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
> > > ---
> > > po/POTFILES | 1 +
> > > src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
> > > src/ch/ch_events.h | 26 ++++++++++++
> > > src/ch/ch_monitor.c | 28 +++++++++++++
> > > src/ch/ch_monitor.h | 3 ++
> > > src/ch/meson.build | 2 +
> > > 6 files changed, 156 insertions(+)
> > > 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..851fbc9f26
> > > --- /dev/null
> > > +++ b/src/ch/ch_events.c
> > > @@ -0,0 +1,96 @@
> > > +/*
> > > + * Copyright Microsoft Corp. 2024
> > > + *
> > > + * ch_events.h: Handle Cloud-Hypervisor events
> >
> > please fix the filename here.
Will fix it in V2.
> >
> > > + *
> > > + * 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) {
> > > + g_usleep(100000); // 100 milli seconds
> > > + 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);
> > > +
> > > + /*
> > > + * We would need to wait until VM is initialized.
> > > + */
> > Does this comment apply to next while loop where the events are
> > processed?
> >
>
> No, this applies to the immediate while loop where vm ref is obtained.
>
> > > + while (!(vm = virObjectRef(mon->vm)))
> > > + g_usleep(100000); // 100 milli seconds
> >
> > In this while loop you are only getting a reference to vm object.
> >
>
> Yes. At this point, we are not sure if mon->vm is initialized. The
> parent thread initializes mon->vm only after starting this event handler
> thread. Hence, wait until we have a vm ref.
Why do you need to wait here? Is there a race condition here?
If yes, will just waiting for a reference for vm object enough?
This is actually not needed if mon->vm is initialized before started
this thread. So will do that and get rid of this loop in V2.
>
> > > +
> > > + 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("event-handler-%d", mon->pid);
> >
> > It would be better to name the thread something similar to
> > "event-handler_ch_${pid}_${vmname}". A similar format is
> > used for cgroup naming.
> >
Since there is a contraint on the length of thread name, I will follow
the convention of `ch-evt-${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..5e43bf9ef8 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,28 @@ 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) &&
> > > + !virFileIsNamedPipe(mon->eventmonitorpath)) {
> > > + VIR_WARN("Monitor file (%s) is not a FIFO, 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;
> > > + }
> > > + }
> > > +
> > This does not cover the case when eventmonitorpath exists, and is a
> > named pipe, may be, not cleaned up from a previous run.
> >
> > You are better off just deleting the file and open a new one with
> > mkfifo below.
> >
Will take care in V2.
> > > + 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,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig
*cfg)
> > > if (virCommandRunAsync(cmd, &mon->pid) < 0)
> > > return NULL;
> > > + if (virCHStartEventHandler(mon) < 0)
> > > + return NULL;
> > > +
> > > /* get a curl handle */
> > > mon->handle = curl_easy_init();
> > > @@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
> > > g_free(mon->eventmonitorpath);
> > > }
> > > + virCHStopEventHandler(mon);
> > > +
> > Please move virCHStopEventHandler above `if (mon->eventmonitorpath) {`
> > section. Doing so, will allow the event monitor thread to cleanly
> > exit, before you invoke `virFileRemove(mon->eventmonitorpath...`
>
> Thanks for this catch. I will move it.
> And will also apply other suggestions.
>
> >
> > > virObjectUnref(mon);
> > > }
> > > 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',
> >
> >
> > --
> > Regards,
> > Praveen K Paladugu
>
> Regards,
> Purna Pavan Chandra
>
--
Regards,
Praveen K Paladugu
Regards,
Purna Pavan Chandra
11 a.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
On 9/19/2024 8:02 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(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
po/POTFILES | 1 +
src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
src/ch/ch_events.h | 26 ++++++++++++
src/ch/ch_monitor.c | 28 +++++++++++++
src/ch/ch_monitor.h | 3 ++
src/ch/meson.build | 2 +
6 files changed, 156 insertions(+)
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..851fbc9f26
--- /dev/null
+++ b/src/ch/ch_events.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright Microsoft Corp. 2024
+ *
+ * ch_events.h: 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) {
+ g_usleep(100000); // 100 milli seconds
+ 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);
+
+ /*
+ * We would need to wait until VM is initialized.
+ */
+ while (!(vm = virObjectRef(mon->vm)))
+ g_usleep(100000); // 100 milli seconds
+
+ 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("event-handler-%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..5e43bf9ef8 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,28 @@ 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) &&
+ !virFileIsNamedPipe(mon->eventmonitorpath)) {
+ VIR_WARN("Monitor file (%s) is not a FIFO, 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;
+ }
Why do you need to create a pipe here as instead of opening the file at
eventmonitorpath, after cloud-hypervisor starts?
+
cmd = virCommandNew(vm->def->emulator);
virCommandSetUmask(cmd, 0x002);
socket_fd = chMonitorCreateSocket(mon->socketpath);
@@ -593,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
if (virCommandRunAsync(cmd, &mon->pid) < 0)
return NULL;
+ if (virCHStartEventHandler(mon) < 0)
+ return NULL;
+
/* get a curl handle */
mon->handle = curl_easy_init();
@@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
g_free(mon->eventmonitorpath);
}
+ virCHStopEventHandler(mon);
+
virObjectUnref(mon);
}
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',
--
Regards,
Praveen K Paladugu
6:16 a.m.
New subject: [PATCH 3/6] ch: start a new thread for handling ch events
On Thu, Sep 26, 2024 at 11:00:26AM -0500, Praveen K Paladugu wrote:
On 9/19/2024 8:02 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(a)linux.microsoft.com>
> Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
> ---
> po/POTFILES | 1 +
> src/ch/ch_events.c | 96 +++++++++++++++++++++++++++++++++++++++++++++
> src/ch/ch_events.h | 26 ++++++++++++
> src/ch/ch_monitor.c | 28 +++++++++++++
> src/ch/ch_monitor.h | 3 ++
> src/ch/meson.build | 2 +
> 6 files changed, 156 insertions(+)
> 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..851fbc9f26
> --- /dev/null
> +++ b/src/ch/ch_events.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright Microsoft Corp. 2024
> + *
> + * ch_events.h: 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) {
> + g_usleep(100000); // 100 milli seconds
> + 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);
> +
> + /*
> + * We would need to wait until VM is initialized.
> + */
> + while (!(vm = virObjectRef(mon->vm)))
> + g_usleep(100000); // 100 milli seconds
> +
> + 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("event-handler-%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..5e43bf9ef8 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,28 @@ 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) &&
> + !virFileIsNamedPipe(mon->eventmonitorpath)) {
> + VIR_WARN("Monitor file (%s) is not a FIFO, 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;
> + }
Why do you need to create a pipe here as instead of opening the file at
eventmonitorpath, after cloud-hypervisor starts?
Since the scenario here is of reader and writer form, named pipe(fifo) suits best.
Also, fifo is faster as it doesn't involve writing into the actual
storage.
"
When processes are exchanging data via the FIFO, the kernel passes all data
internally without writing it to the filesystem.
"
Ref: https://www.man7.org/linux/man-pages/man7/fifo.7.html
> +
> cmd = virCommandNew(vm->def->emulator);
> virCommandSetUmask(cmd, 0x002);
> socket_fd = chMonitorCreateSocket(mon->socketpath);
> @@ -593,6 +616,9 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg)
> if (virCommandRunAsync(cmd, &mon->pid) < 0)
> return NULL;
> + if (virCHStartEventHandler(mon) < 0)
> + return NULL;
> +
> /* get a curl handle */
> mon->handle = curl_easy_init();
> @@ -641,6 +667,8 @@ void virCHMonitorClose(virCHMonitor *mon)
> g_free(mon->eventmonitorpath);
> }
> + virCHStopEventHandler(mon);
> +
> virObjectUnref(mon);
> }
> 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',
--
Regards,
Praveen K Paladugu
Regards,
Purna Pavan Chandra
8:02 a.m.
New subject: [PATCH 4/6] ch: events: Read and parse cloud-hypervisor events
Implement `chReadProcessEvents` and `chProcessEvents` to read events from
event monitor FIFO file and parse them accordingly.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
src/ch/ch_events.c | 142 +++++++++++++++++++++++++++++++++++++++++++-
src/ch/ch_events.h | 2 +
src/ch/ch_monitor.h | 6 ++
3 files changed, 149 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index 851fbc9f26..a028f9813e 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -28,6 +28,142 @@
VIR_LOG_INIT("ch.ch_events");
+static int virCHProcessEvents(virCHMonitor *mon)
+{
+ char *buf = mon->event_buffer.buffer;
+ ssize_t sz = mon->event_buffer.buf_fill_sz;
+ virJSONValue *obj = NULL;
+ int blocks = 0;
+ size_t i = 0;
+ char *json_start;
+ ssize_t start_index = -1;
+ ssize_t end_index = -1;
+ char tmp;
+ int ret = 0;
+
+ while (i < sz) {
+ if (buf[i] == '{') {
+ blocks++;
+ if (blocks == 1)
+ start_index = i;
+ } else if (buf[i] == '}' && blocks > 0) {
+ blocks--;
+ if (blocks == 0) {
+ // valid json document
+ end_index = i;
+
+ /*
+ * We may hit a corner case where a valid JSON
+ * doc happens to end right at the end of the buffer.
+ * virJSONValueFromString needs '\0' end the JSON doc.
+ * So we need to adjust the buffer accordingly.
+ */
+ if (end_index == CH_EVENT_BUFFER_SZ - 1) {
+ if (start_index == 0) {
+ /*
+ * We have a valid JSON doc same as the buffer
+ * size. As per protocol, max JSON doc should be
+ * less than the buffer size. So this is an error.
+ * Ignore this JSON doc.
+ */
+ VIR_WARN("Invalid JSON doc size. Expected <= %d",
+ CH_EVENT_BUFFER_SZ);
+ start_index = -1;
+ ret = -1;
+ break;
+ }
+
+ /*
+ * Move the valid JSON doc to the start of the buffer so
+ * that we can safely fit a '\0' at the end.
+ */
+ memmove(buf, buf+start_index, end_index-start_index+1);
+ end_index -= start_index;
+ start_index = 0;
+ }
+
+ // temporarily null terminate the JSON doc
+ tmp = buf[end_index + 1];
+ buf[end_index + 1] = '\0';
+ json_start = buf + start_index;
+
+ if ((obj = virJSONValueFromString(json_start))) {
+ /* Process the event string (obj) here */
+ virJSONValueFree(obj);
+ } else {
+ VIR_WARN("Invalid JSON event doc: %s", json_start);
+ ret = -1;
+ }
+
+ // replace the original character
+ buf[end_index + 1] = tmp;
+ start_index = -1;
+ }
+ }
+
+ i++;
+ }
+
+ if (start_index == -1) {
+ // We have processed all the JSON docs in the buffer.
+ mon->event_buffer.buf_fill_sz = 0;
+ } else if (start_index > 0) {
+ // We have an incomplete JSON doc at the end of the buffer.
+ // Move it to the start of the buffer.
+ mon->event_buffer.buf_fill_sz = sz - start_index;
+ memmove(buf, buf+start_index, mon->event_buffer.buf_fill_sz);
+ }
+
+ return ret;
+}
+
+static void virCHReadProcessEvents(virCHMonitor *mon,
+ int event_monitor_fd)
+{
+ size_t max_sz = CH_EVENT_BUFFER_SZ;
+ char *buf = mon->event_buffer.buffer;
+ virDomainObj *vm = mon->vm;
+ bool incomplete = false;
+ size_t sz = 0;
+
+ memset(buf, 0, max_sz);
+ do {
+ ssize_t ret;
+
+ ret = read(event_monitor_fd, buf + sz, max_sz - sz);
+ if (ret == 0 || (ret < 0 && errno == EINTR)) {
+ g_usleep(G_USEC_PER_SEC);
+ continue;
+ } else if (ret < 0) {
+ /*
+ * We should never reach here. read(2) says possible errors
+ * are EINTR, EAGAIN, EBADF, EFAULT, EINVAL, EIO, EISDIR
+ * We handle EINTR gracefully. There is some serious issue
+ * if we encounter any of the other errors(either in our code
+ * or in the system). Better to bail out.
+ */
+ VIR_ERROR(_("Failed to read ch events!: %1$s"),
g_strerror(errno));
+ VIR_FORCE_CLOSE(event_monitor_fd);
+ abort();
+ }
+
+ sz += ret;
+ mon->event_buffer.buf_fill_sz = sz;
+
+ if (virCHProcessEvents(mon) < 0)
+ VIR_WARN("Failed to parse and process events");
+
+ if (mon->event_buffer.buf_fill_sz != 0)
+ incomplete = true;
+ else
+ incomplete = false;
+ sz = mon->event_buffer.buf_fill_sz;
+
+ } while (virDomainObjIsActive(vm) && (sz < max_sz) &&
incomplete);
+
+ return;
+}
+
static void virCHEventHandlerLoop(void *data)
{
virCHMonitor *mon = data;
@@ -51,6 +187,9 @@ static void virCHEventHandlerLoop(void *data)
}
VIR_DEBUG("Opened the event monitor FIFO(%s)", mon->eventmonitorpath);
+ mon->event_buffer.buffer = g_malloc_n(sizeof(char), CH_EVENT_BUFFER_SZ);
+ mon->event_buffer.buf_fill_sz = 0;
+
/*
* We would need to wait until VM is initialized.
*/
@@ -59,9 +198,10 @@ static void virCHEventHandlerLoop(void *data)
while (g_atomic_int_get(&mon->event_handler_stop) == 0) {
VIR_DEBUG("Reading events from event monitor file...");
- /* Read and process events here */
+ virCHReadProcessEvents(mon, event_monitor_fd);
}
+ g_free(mon->event_buffer.buffer);
VIR_FORCE_CLOSE(event_monitor_fd);
virObjectUnref(vm);
diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
index 4c8a48231d..2e9cdf03bb 100644
--- a/src/ch/ch_events.h
+++ b/src/ch/ch_events.h
@@ -22,5 +22,7 @@
#include "ch_monitor.h"
+#define CH_EVENT_BUFFER_SZ PIPE_BUF
+
int virCHStartEventHandler(virCHMonitor *mon);
void virCHStopEventHandler(virCHMonitor *mon);
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index 878a185f29..6b4045d300 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -99,6 +99,12 @@ struct _virCHMonitor {
virThread event_handler_thread;
int event_handler_stop;
+ struct {
+ // Buffer to hold the data read from pipe
+ char *buffer;
+ // Size of the data read from pipe in buffer
+ size_t buf_fill_sz;
+ } event_buffer;
pid_t pid;
--
2.34.1
11:16 a.m.
New subject: [PATCH 4/6] ch: events: Read and parse cloud-hypervisor events
On 9/19/2024 8:02 AM, Purna Pavan Chandra Aekkaladevi wrote:
Implement `chReadProcessEvents` and `chProcessEvents` to read events
from
event monitor FIFO file and parse them accordingly.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
---
src/ch/ch_events.c | 142 +++++++++++++++++++++++++++++++++++++++++++-
src/ch/ch_events.h | 2 +
src/ch/ch_monitor.h | 6 ++
3 files changed, 149 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index 851fbc9f26..a028f9813e 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -28,6 +28,142 @@
VIR_LOG_INIT("ch.ch_events");
Please add a comment here with a sample event here for reference.
+static int virCHProcessEvents(virCHMonitor *mon)
+{
+ char *buf = mon->event_buffer.buffer;
+ ssize_t sz = mon->event_buffer.buf_fill_sz;
+ virJSONValue *obj = NULL;
+ int blocks = 0;
+ size_t i = 0;
+ char *json_start;
+ ssize_t start_index = -1;
+ ssize_t end_index = -1;
+ char tmp;
+ int ret = 0;
+
+ while (i < sz) {
+ if (buf[i] == '{') {
+ blocks++;
+ if (blocks == 1)
+ start_index = i;
+ } else if (buf[i] == '}' && blocks > 0) {
+ blocks--;
+ if (blocks == 0) {
+ // valid json document
+ end_index = i;
+
+ /*
+ * We may hit a corner case where a valid JSON
+ * doc happens to end right at the end of the buffer.
+ * virJSONValueFromString needs '\0' end the JSON doc.
+ * So we need to adjust the buffer accordingly.
+ */
+ if (end_index == CH_EVENT_BUFFER_SZ - 1) {
+ if (start_index == 0) {
+ /*
+ * We have a valid JSON doc same as the buffer
+ * size. As per protocol, max JSON doc should be
+ * less than the buffer size. So this is an error.
+ * Ignore this JSON doc.
+ */
+ VIR_WARN("Invalid JSON doc size. Expected <= %d",
+ CH_EVENT_BUFFER_SZ);
+ start_index = -1;
+ ret = -1;
+ break;
+ }
+
+ /*
+ * Move the valid JSON doc to the start of the buffer so
+ * that we can safely fit a '\0' at the end.
+ */
+ memmove(buf, buf+start_index, end_index-start_index+1);
+ end_index -= start_index;
+ start_index = 0;
+ }
+
+ // temporarily null terminate the JSON doc
+ tmp = buf[end_index + 1];
+ buf[end_index + 1] = '\0';
+ json_start = buf + start_index;
+
+ if ((obj = virJSONValueFromString(json_start))) {
+ /* Process the event string (obj) here */
+ virJSONValueFree(obj);
+ } else {
+ VIR_WARN("Invalid JSON event doc: %s", json_start);
+ ret = -1;
+ }
+
+ // replace the original character
+ buf[end_index + 1] = tmp;
+ start_index = -1;
+ }
+ }
+
+ i++;
+ }
+
+ if (start_index == -1) {
+ // We have processed all the JSON docs in the buffer.
+ mon->event_buffer.buf_fill_sz = 0;
+ } else if (start_index > 0) {
+ // We have an incomplete JSON doc at the end of the buffer.
+ // Move it to the start of the buffer.
+ mon->event_buffer.buf_fill_sz = sz - start_index;
+ memmove(buf, buf+start_index, mon->event_buffer.buf_fill_sz);
+ }
+
+ return ret;
+}
+
+static void virCHReadProcessEvents(virCHMonitor *mon,
+ int event_monitor_fd)
+{
+ size_t max_sz = CH_EVENT_BUFFER_SZ;
To avoid the corner case in
virCHProcessEvents where the event ends
at end of buffer, you can set
max_sz = CH_EVENT_BUFFER_SZ -1;
This ensures there is always space for a Null char at the end.
+ char *buf = mon->event_buffer.buffer;
+ virDomainObj *vm = mon->vm;
+ bool incomplete = false;
+ size_t sz = 0;
+
+ memset(buf, 0, max_sz);
+ do {
+ ssize_t ret;
+
+ ret = read(event_monitor_fd, buf + sz, max_sz - sz);
+ if (ret == 0 || (ret < 0 && errno == EINTR)) {
+ g_usleep(G_USEC_PER_SEC);
+ continue;
+ } else if (ret < 0) {
+ /*
+ * We should never reach here. read(2) says possible errors
+ * are EINTR, EAGAIN, EBADF, EFAULT, EINVAL, EIO, EISDIR
+ * We handle EINTR gracefully. There is some serious issue
+ * if we encounter any of the other errors(either in our code
+ * or in the system). Better to bail out.
+ */
+ VIR_ERROR(_("Failed to read ch events!: %1$s"),
g_strerror(errno));
+ VIR_FORCE_CLOSE(event_monitor_fd);
+ abort();
+ }
+
+ sz += ret;
+ mon->event_buffer.buf_fill_sz = sz;
+
+ if (virCHProcessEvents(mon) < 0)
+ VIR_WARN("Failed to parse and process events");
+
+ if (mon->event_buffer.buf_fill_sz != 0)
+ incomplete = true;
+ else
+ incomplete = false;
+ sz = mon->event_buffer.buf_fill_sz;
+
+ } while (virDomainObjIsActive(vm) && (sz < max_sz) &&
incomplete);
+
+ return;
+}
+
static void virCHEventHandlerLoop(void *data)
{
virCHMonitor *mon = data;
@@ -51,6 +187,9 @@ static void virCHEventHandlerLoop(void *data)
}
VIR_DEBUG("Opened the event monitor FIFO(%s)",
mon->eventmonitorpath);
+ mon->event_buffer.buffer = g_malloc_n(sizeof(char), CH_EVENT_BUFFER_SZ);
+ mon->event_buffer.buf_fill_sz = 0;
+
/*
* We would need to wait until VM is initialized.
*/
@@ -59,9 +198,10 @@ static void virCHEventHandlerLoop(void *data)
while (g_atomic_int_get(&mon->event_handler_stop) == 0) {
VIR_DEBUG("Reading events from event monitor file...");
- /* Read and process events here */
+ virCHReadProcessEvents(mon, event_monitor_fd);
}
+ g_free(mon->event_buffer.buffer);
VIR_FORCE_CLOSE(event_monitor_fd);
virObjectUnref(vm);
diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
index 4c8a48231d..2e9cdf03bb 100644
--- a/src/ch/ch_events.h
+++ b/src/ch/ch_events.h
@@ -22,5 +22,7 @@
#include "ch_monitor.h"
+#define CH_EVENT_BUFFER_SZ PIPE_BUF
+
int virCHStartEventHandler(virCHMonitor *mon);
void virCHStopEventHandler(virCHMonitor *mon);
diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
index 878a185f29..6b4045d300 100644
--- a/src/ch/ch_monitor.h
+++ b/src/ch/ch_monitor.h
@@ -99,6 +99,12 @@ struct _virCHMonitor {
virThread event_handler_thread;
int event_handler_stop;
+ struct {
+ // Buffer to hold the data read from pipe
Please replace all `//` comments with `/**/`. This is the preferred
comment style in Libvirt.
+ char *buffer;
+ // Size of the data read from pipe in buffer
nit: "Size of the data
read from pipe into buffer"
+ size_t buf_fill_sz;
+ } event_buffer;
pid_t pid;
--
Regards,
Praveen K Paladugu
6:25 a.m.
New subject: [PATCH 4/6] ch: events: Read and parse cloud-hypervisor events
On Thu, Sep 26, 2024 at 11:16:37AM -0500, Praveen K Paladugu wrote:
On 9/19/2024 8:02 AM, Purna Pavan Chandra Aekkaladevi wrote:
> Implement `chReadProcessEvents` and `chProcessEvents` to read events from
> event monitor FIFO file and parse them accordingly.
>
> Signed-off-by: Purna Pavan Chandra Aekkaladevi
<paekkaladevi(a)linux.microsoft.com>
> Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com>
> ---
> src/ch/ch_events.c | 142 +++++++++++++++++++++++++++++++++++++++++++-
> src/ch/ch_events.h | 2 +
> src/ch/ch_monitor.h | 6 ++
> 3 files changed, 149 insertions(+), 1 deletion(-)
>
> diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
> index 851fbc9f26..a028f9813e 100644
> --- a/src/ch/ch_events.c
> +++ b/src/ch/ch_events.c
> @@ -28,6 +28,142 @@
> VIR_LOG_INIT("ch.ch_events");
>
Please add a comment here with a sample event here for reference.
Sure, will do.
> +static int virCHProcessEvents(virCHMonitor *mon)
> +{
> + char *buf = mon->event_buffer.buffer;
> + ssize_t sz = mon->event_buffer.buf_fill_sz;
> + virJSONValue *obj = NULL;
> + int blocks = 0;
> + size_t i = 0;
> + char *json_start;
> + ssize_t start_index = -1;
> + ssize_t end_index = -1;
> + char tmp;
> + int ret = 0;
> +
> + while (i < sz) {
> + if (buf[i] == '{') {
> + blocks++;
> + if (blocks == 1)
> + start_index = i;
> + } else if (buf[i] == '}' && blocks > 0) {
> + blocks--;
> + if (blocks == 0) {
> + // valid json document
> + end_index = i;
> +
> + /*
> + * We may hit a corner case where a valid JSON
> + * doc happens to end right at the end of the buffer.
> + * virJSONValueFromString needs '\0' end the JSON doc.
> + * So we need to adjust the buffer accordingly.
> + */
> + if (end_index == CH_EVENT_BUFFER_SZ - 1) {
> + if (start_index == 0) {
> + /*
> + * We have a valid JSON doc same as the buffer
> + * size. As per protocol, max JSON doc should be
> + * less than the buffer size. So this is an error.
> + * Ignore this JSON doc.
> + */
> + VIR_WARN("Invalid JSON doc size. Expected <=
%d",
> + CH_EVENT_BUFFER_SZ);
> + start_index = -1;
> + ret = -1;
> + break;
> + }
> +
> + /*
> + * Move the valid JSON doc to the start of the buffer so
> + * that we can safely fit a '\0' at the end.
> + */
> + memmove(buf, buf+start_index, end_index-start_index+1);
> + end_index -= start_index;
> + start_index = 0;
> + }
> +
> + // temporarily null terminate the JSON doc
> + tmp = buf[end_index + 1];
> + buf[end_index + 1] = '\0';
> + json_start = buf + start_index;
> +
> + if ((obj = virJSONValueFromString(json_start))) {
> + /* Process the event string (obj) here */
> + virJSONValueFree(obj);
> + } else {
> + VIR_WARN("Invalid JSON event doc: %s", json_start);
> + ret = -1;
> + }
> +
> + // replace the original character
> + buf[end_index + 1] = tmp;
> + start_index = -1;
> + }
> + }
> +
> + i++;
> + }
> +
> + if (start_index == -1) {
> + // We have processed all the JSON docs in the buffer.
> + mon->event_buffer.buf_fill_sz = 0;
> + } else if (start_index > 0) {
> + // We have an incomplete JSON doc at the end of the buffer.
> + // Move it to the start of the buffer.
> + mon->event_buffer.buf_fill_sz = sz - start_index;
> + memmove(buf, buf+start_index, mon->event_buffer.buf_fill_sz);
> + }
> +
> + return ret;
> +}
> +
> +static void virCHReadProcessEvents(virCHMonitor *mon,
> + int event_monitor_fd)
> +{
> + size_t max_sz = CH_EVENT_BUFFER_SZ;
To avoid the corner case in virCHProcessEvents where the event ends
at end of buffer, you can set
max_sz = CH_EVENT_BUFFER_SZ -1;
This ensures there is always space for a Null char at the end.
This would be same as what current logic is doing i.e., supporting
events of length less than CH_EVENT_BUFFER_SZ.
Currently, when event length is CH_EVENT_BUFFER_SZ, we warn saying
expected length is <CH_EVENT_BUFFER_SZ. Otherwise, we already have
enough space to additionally terminate with null char.
> + char *buf = mon->event_buffer.buffer;
> + virDomainObj *vm = mon->vm;
> + bool incomplete = false;
> + size_t sz = 0;
> +
> + memset(buf, 0, max_sz);
> + do {
> + ssize_t ret;
> +
> + ret = read(event_monitor_fd, buf + sz, max_sz - sz);
> + if (ret == 0 || (ret < 0 && errno == EINTR)) {
> + g_usleep(G_USEC_PER_SEC);
> + continue;
> + } else if (ret < 0) {
> + /*
> + * We should never reach here. read(2) says possible errors
> + * are EINTR, EAGAIN, EBADF, EFAULT, EINVAL, EIO, EISDIR
> + * We handle EINTR gracefully. There is some serious issue
> + * if we encounter any of the other errors(either in our code
> + * or in the system). Better to bail out.
> + */
> + VIR_ERROR(_("Failed to read ch events!: %1$s"),
g_strerror(errno));
> + VIR_FORCE_CLOSE(event_monitor_fd);
> + abort();
> + }
> +
> + sz += ret;
> + mon->event_buffer.buf_fill_sz = sz;
> +
> + if (virCHProcessEvents(mon) < 0)
> + VIR_WARN("Failed to parse and process events");
> +
> + if (mon->event_buffer.buf_fill_sz != 0)
> + incomplete = true;
> + else
> + incomplete = false;
> + sz = mon->event_buffer.buf_fill_sz;
> +
> + } while (virDomainObjIsActive(vm) && (sz < max_sz) &&
incomplete);
> +
> + return;
> +}
> +
> static void virCHEventHandlerLoop(void *data)
> {
> virCHMonitor *mon = data;
> @@ -51,6 +187,9 @@ static void virCHEventHandlerLoop(void *data)
> }
> VIR_DEBUG("Opened the event monitor FIFO(%s)",
mon->eventmonitorpath);
> + mon->event_buffer.buffer = g_malloc_n(sizeof(char), CH_EVENT_BUFFER_SZ);
> + mon->event_buffer.buf_fill_sz = 0;
> +
> /*
> * We would need to wait until VM is initialized.
> */
> @@ -59,9 +198,10 @@ static void virCHEventHandlerLoop(void *data)
> while (g_atomic_int_get(&mon->event_handler_stop) == 0) {
> VIR_DEBUG("Reading events from event monitor file...");
> - /* Read and process events here */
> + virCHReadProcessEvents(mon, event_monitor_fd);
> }
> + g_free(mon->event_buffer.buffer);
> VIR_FORCE_CLOSE(event_monitor_fd);
> virObjectUnref(vm);
> diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
> index 4c8a48231d..2e9cdf03bb 100644
> --- a/src/ch/ch_events.h
> +++ b/src/ch/ch_events.h
> @@ -22,5 +22,7 @@
> #include "ch_monitor.h"
> +#define CH_EVENT_BUFFER_SZ PIPE_BUF
> +
> int virCHStartEventHandler(virCHMonitor *mon);
> void virCHStopEventHandler(virCHMonitor *mon);
> diff --git a/src/ch/ch_monitor.h b/src/ch/ch_monitor.h
> index 878a185f29..6b4045d300 100644
> --- a/src/ch/ch_monitor.h
> +++ b/src/ch/ch_monitor.h
> @@ -99,6 +99,12 @@ struct _virCHMonitor {
> virThread event_handler_thread;
> int event_handler_stop;
> + struct {
> + // Buffer to hold the data read from pipe
Please replace all `//` comments with `/**/`. This is the preferred
comment style in Libvirt.
Will take care of it in V2.
> + char *buffer;
> + // Size of the data read from pipe in buffer
nit: "Size of the data read from pipe into buffer"
Sure
> + size_t buf_fill_sz;
> + } event_buffer;
> pid_t pid;
--
Regards,
Praveen K Paladugu
Regards,
Purna Pavan Chandra
8:02 a.m.
New subject: [PATCH 5/6] ch: events: facilitate lifecycle events handling
Implement `virCHProcessEvent` that maps event string to corresponding
event type and take appropriate actions. As part of this, handle the
shutdown event by correctly updating the domain state. this change also
facilitates the handling of other VM lifecycle events, such as booting,
rebooting, pause, resume, etc.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
---
src/ch/ch_events.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-
src/ch/ch_events.h | 26 ++++++++++++
2 files changed, 128 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index a028f9813e..6ed03d1c90 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -22,12 +22,110 @@
#include <fcntl.h>
+#include "ch_domain.h"
#include "ch_events.h"
+#include "ch_process.h"
#include "virfile.h"
#include "virlog.h"
VIR_LOG_INIT("ch.ch_events");
+VIR_ENUM_IMPL(virCHEvent,
+ VIR_CH_EVENT_LAST,
+ "vmm:starting",
+ "vmm:shutdown",
+ "vm:booting",
+ "vm:booted",
+ "vm:rebooting",
+ "vm:rebooted",
+ "vm:shutdown",
+ "vm:deleted",
+ "vm:pausing",
+ "vm:paused",
+ "vm:resuming",
+ "vm:resumed",
+ "vm:snapshotting",
+ "vm:snapshotted",
+ "vm:restoring",
+ "vm:restored",
+);
+
+static int virCHEventStopProcess(virDomainObj *vm,
+ virDomainShutoffReason reason)
+{
+ virCHDriver *driver = ((virCHDomainObjPrivate *)vm->privateData)->driver;
+
+ if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY))
+ return -1;
+ virCHProcessStop(driver, vm, reason);
+ virDomainObjEndJob(vm);
+
+ return 0;
+}
+
+static int virCHProcessEvent(virCHMonitor *mon,
+ virJSONValue *eventJSON)
+{
+ const char *event;
+ const char *source;
+ virCHEvent ev;
+ g_autofree char *timestamp = NULL;
+ g_autofree char *full_event = NULL;
+ virDomainObj *vm = mon->vm;
+ int ret = 0;
+
+ if (virJSONValueObjectHasKey(eventJSON, "source") == 0) {
+ VIR_WARN("Invalid JSON from monitor, no source key");
+ return -1;
+ }
+ if (virJSONValueObjectHasKey(eventJSON, "event") == 0) {
+ VIR_WARN("Invalid JSON from monitor, no event key");
+ return -1;
+ }
+ source = virJSONValueObjectGetString(eventJSON, "source");
+ event = virJSONValueObjectGetString(eventJSON, "event");
+ full_event = g_strdup_printf("%s:%s", source, event);
+ ev = virCHEventTypeFromString(full_event);
+ VIR_DEBUG("Source: %s Event: %s, ev: %d", source, event, ev);
+
+ switch (ev) {
+ case VIR_CH_EVENT_VMM_STARTING:
+ case VIR_CH_EVENT_VM_BOOTING:
+ case VIR_CH_EVENT_VM_BOOTED:
+ case VIR_CH_EVENT_VM_REBOOTING:
+ case VIR_CH_EVENT_VM_REBOOTED:
+ case VIR_CH_EVENT_VM_PAUSING:
+ case VIR_CH_EVENT_VM_PAUSED:
+ case VIR_CH_EVENT_VM_RESUMING:
+ case VIR_CH_EVENT_VM_RESUMED:
+ case VIR_CH_EVENT_VM_SNAPSHOTTING:
+ case VIR_CH_EVENT_VM_SNAPSHOTTED:
+ case VIR_CH_EVENT_VM_RESTORING:
+ case VIR_CH_EVENT_VM_RESTORED:
+ case VIR_CH_EVENT_VM_DELETED:
+ break;
+ case VIR_CH_EVENT_VMM_SHUTDOWN:
+ virObjectLock(vm);
+ if (virCHEventStopProcess(vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN)) {
+ VIR_WARN("Failed to mark the VM(%s) as SHUTDOWN!",
+ vm->def->name);
+ ret = -1;
+ }
+ virObjectUnlock(vm);
+ break;
+ case VIR_CH_EVENT_VM_SHUTDOWN:
+ virObjectLock(vm);
+ virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
+ virObjectUnlock(vm);
+ break;
+ case VIR_CH_EVENT_LAST:
+ default:
+ VIR_WARN("Unknown event: %s", full_event);
+ }
+
+ return ret;
+}
+
static int virCHProcessEvents(virCHMonitor *mon)
{
char *buf = mon->event_buffer.buffer;
@@ -88,7 +186,10 @@ static int virCHProcessEvents(virCHMonitor *mon)
json_start = buf + start_index;
if ((obj = virJSONValueFromString(json_start))) {
- /* Process the event string (obj) here */
+ if (virCHProcessEvent(mon, obj) < 0) {
+ VIR_WARN("Failed to process JSON event doc: %s",
json_start);
+ ret = -1;
+ }
virJSONValueFree(obj);
} else {
VIR_WARN("Invalid JSON event doc: %s", json_start);
diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
index 2e9cdf03bb..3b360628f7 100644
--- a/src/ch/ch_events.h
+++ b/src/ch/ch_events.h
@@ -24,5 +24,31 @@
#define CH_EVENT_BUFFER_SZ PIPE_BUF
+typedef enum {
+ /* source: vmm */
+ VIR_CH_EVENT_VMM_STARTING = 0,
+ VIR_CH_EVENT_VMM_SHUTDOWN,
+
+ /* source: vm */
+ VIR_CH_EVENT_VM_BOOTING,
+ VIR_CH_EVENT_VM_BOOTED,
+ VIR_CH_EVENT_VM_REBOOTING,
+ VIR_CH_EVENT_VM_REBOOTED,
+ VIR_CH_EVENT_VM_SHUTDOWN,
+ VIR_CH_EVENT_VM_DELETED,
+ VIR_CH_EVENT_VM_PAUSING,
+ VIR_CH_EVENT_VM_PAUSED,
+ VIR_CH_EVENT_VM_RESUMING,
+ VIR_CH_EVENT_VM_RESUMED,
+ VIR_CH_EVENT_VM_SNAPSHOTTING,
+ VIR_CH_EVENT_VM_SNAPSHOTTED,
+ VIR_CH_EVENT_VM_RESTORING,
+ VIR_CH_EVENT_VM_RESTORED,
+
+ VIR_CH_EVENT_LAST
+} virCHEvent;
+
+VIR_ENUM_DECL(virCHEvent);
+
int virCHStartEventHandler(virCHMonitor *mon);
void virCHStopEventHandler(virCHMonitor *mon);
--
2.34.1
11:27 a.m.
New subject: [PATCH 5/6] ch: events: facilitate lifecycle events handling
On 9/19/2024 8:02 AM, Purna Pavan Chandra Aekkaladevi wrote:
Implement `virCHProcessEvent` that maps event string to
corresponding
event type and take appropriate actions. As part of this, handle the
shutdown event by correctly updating the domain state. this change also
facilitates the handling of other VM lifecycle events, such as booting,
rebooting, pause, resume, etc.
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
---
src/ch/ch_events.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-
src/ch/ch_events.h | 26 ++++++++++++
2 files changed, 128 insertions(+), 1 deletion(-)
diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
index a028f9813e..6ed03d1c90 100644
--- a/src/ch/ch_events.c
+++ b/src/ch/ch_events.c
@@ -22,12 +22,110 @@
#include <fcntl.h>
+#include "ch_domain.h"
#include "ch_events.h"
+#include "ch_process.h"
#include "virfile.h"
#include "virlog.h"
VIR_LOG_INIT("ch.ch_events");
+VIR_ENUM_IMPL(virCHEvent,
+ VIR_CH_EVENT_LAST,
+ "vmm:starting",
+ "vmm:shutdown",
+ "vm:booting",
+ "vm:booted",
+ "vm:rebooting",
+ "vm:rebooted",
+ "vm:shutdown",
+ "vm:deleted",
+ "vm:pausing",
+ "vm:paused",
+ "vm:resuming",
+ "vm:resumed",
+ "vm:snapshotting",
+ "vm:snapshotted",
+ "vm:restoring",
+ "vm:restored",
+);
+
+static int virCHEventStopProcess(virDomainObj *vm,
+ virDomainShutoffReason reason)
+{
+ virCHDriver *driver = ((virCHDomainObjPrivate *)vm->privateData)->driver;
+
+ if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY))
+ return -1;
+ virCHProcessStop(driver, vm, reason);
+ virDomainObjEndJob(vm);
+
+ return 0;
+}
+
+static int virCHProcessEvent(virCHMonitor *mon,
+ virJSONValue *eventJSON)
+{
+ const char *event;
+ const char *source;
+ virCHEvent ev;
+ g_autofree char *timestamp = NULL;
+ g_autofree char *full_event = NULL;
+ virDomainObj *vm = mon->vm;
+ int ret = 0;
+
+ if (virJSONValueObjectHasKey(eventJSON, "source") == 0) {
+ VIR_WARN("Invalid JSON from monitor, no source key");
+ return -1;
+ }
+ if (virJSONValueObjectHasKey(eventJSON, "event") == 0) {
+ VIR_WARN("Invalid JSON from monitor, no event key");
+ return -1;
+ }
+ source = virJSONValueObjectGetString(eventJSON, "source");
+ event = virJSONValueObjectGetString(eventJSON, "event");
+ full_event = g_strdup_printf("%s:%s", source, event);
+ ev = virCHEventTypeFromString(full_event);
+ VIR_DEBUG("Source: %s Event: %s, ev: %d", source, event, ev);
+
+ switch (ev) {
+ case VIR_CH_EVENT_VMM_STARTING:
+ case VIR_CH_EVENT_VM_BOOTING:
+ case VIR_CH_EVENT_VM_BOOTED:
+ case VIR_CH_EVENT_VM_REBOOTING:
+ case VIR_CH_EVENT_VM_REBOOTED:
+ case VIR_CH_EVENT_VM_PAUSING:
+ case VIR_CH_EVENT_VM_PAUSED:
+ case VIR_CH_EVENT_VM_RESUMING:
+ case VIR_CH_EVENT_VM_RESUMED:
+ case VIR_CH_EVENT_VM_SNAPSHOTTING:
+ case VIR_CH_EVENT_VM_SNAPSHOTTED:
+ case VIR_CH_EVENT_VM_RESTORING:
+ case VIR_CH_EVENT_VM_RESTORED:
+ case VIR_CH_EVENT_VM_DELETED:
+ break;
+ case VIR_CH_EVENT_VMM_SHUTDOWN:
+ virObjectLock(vm);
+ if (virCHEventStopProcess(vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN)) {
+ VIR_WARN("Failed to mark the VM(%s) as SHUTDOWN!",
+ vm->def->name);
+ ret = -1;
+ }
+ virObjectUnlock(vm);
+ break;
+ case VIR_CH_EVENT_VM_SHUTDOWN:
+ virObjectLock(vm);
+ virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
Is the ch process already killed in this scenario?
+ virObjectUnlock(vm);
+ break;
+ case VIR_CH_EVENT_LAST:
+ default:
+ VIR_WARN("Unknown event: %s", full_event);
+ }
+
+ return ret;
+}
+
static int virCHProcessEvents(virCHMonitor *mon)
{
char *buf = mon->event_buffer.buffer;
@@ -88,7 +186,10 @@ static int virCHProcessEvents(virCHMonitor *mon)
json_start = buf + start_index;
if ((obj = virJSONValueFromString(json_start))) {
- /* Process the event string (obj) here */
+ if (virCHProcessEvent(mon, obj) < 0) {
+ VIR_WARN("Failed to process JSON event doc: %s",
json_start);
+ ret = -1;
+ }
virJSONValueFree(obj);
} else {
VIR_WARN("Invalid JSON event doc: %s", json_start);
diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
index 2e9cdf03bb..3b360628f7 100644
--- a/src/ch/ch_events.h
+++ b/src/ch/ch_events.h
@@ -24,5 +24,31 @@
#define CH_EVENT_BUFFER_SZ PIPE_BUF
+typedef enum {
+ /* source: vmm */
+ VIR_CH_EVENT_VMM_STARTING = 0,
+ VIR_CH_EVENT_VMM_SHUTDOWN,
+
+ /* source: vm */
+ VIR_CH_EVENT_VM_BOOTING,
+ VIR_CH_EVENT_VM_BOOTED,
+ VIR_CH_EVENT_VM_REBOOTING,
+ VIR_CH_EVENT_VM_REBOOTED,
+ VIR_CH_EVENT_VM_SHUTDOWN,
+ VIR_CH_EVENT_VM_DELETED,
+ VIR_CH_EVENT_VM_PAUSING,
+ VIR_CH_EVENT_VM_PAUSED,
+ VIR_CH_EVENT_VM_RESUMING,
+ VIR_CH_EVENT_VM_RESUMED,
+ VIR_CH_EVENT_VM_SNAPSHOTTING,
+ VIR_CH_EVENT_VM_SNAPSHOTTED,
+ VIR_CH_EVENT_VM_RESTORING,
+ VIR_CH_EVENT_VM_RESTORED,
+
+ VIR_CH_EVENT_LAST
+} virCHEvent;
+
+VIR_ENUM_DECL(virCHEvent);
+
int virCHStartEventHandler(virCHMonitor *mon);
void virCHStopEventHandler(virCHMonitor *mon);
--
Regards,
Praveen K Paladugu
7:07 a.m.
New subject: [PATCH 5/6] ch: events: facilitate lifecycle events handling
On Thu, Sep 26, 2024 at 11:27:01AM -0500, Praveen K Paladugu wrote:
On 9/19/2024 8:02 AM, Purna Pavan Chandra Aekkaladevi wrote:
> Implement `virCHProcessEvent` that maps event string to corresponding
> event type and take appropriate actions. As part of this, handle the
> shutdown event by correctly updating the domain state. this change also
> facilitates the handling of other VM lifecycle events, such as booting,
> rebooting, pause, resume, etc.
>
> Signed-off-by: Purna Pavan Chandra Aekkaladevi
<paekkaladevi(a)linux.microsoft.com>
> ---
> src/ch/ch_events.c | 103 ++++++++++++++++++++++++++++++++++++++++++++-
> src/ch/ch_events.h | 26 ++++++++++++
> 2 files changed, 128 insertions(+), 1 deletion(-)
>
> diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c
> index a028f9813e..6ed03d1c90 100644
> --- a/src/ch/ch_events.c
> +++ b/src/ch/ch_events.c
> @@ -22,12 +22,110 @@
> #include <fcntl.h>
> +#include "ch_domain.h"
> #include "ch_events.h"
> +#include "ch_process.h"
> #include "virfile.h"
> #include "virlog.h"
> VIR_LOG_INIT("ch.ch_events");
> +VIR_ENUM_IMPL(virCHEvent,
> + VIR_CH_EVENT_LAST,
> + "vmm:starting",
> + "vmm:shutdown",
> + "vm:booting",
> + "vm:booted",
> + "vm:rebooting",
> + "vm:rebooted",
> + "vm:shutdown",
> + "vm:deleted",
> + "vm:pausing",
> + "vm:paused",
> + "vm:resuming",
> + "vm:resumed",
> + "vm:snapshotting",
> + "vm:snapshotted",
> + "vm:restoring",
> + "vm:restored",
> +);
> +
> +static int virCHEventStopProcess(virDomainObj *vm,
> + virDomainShutoffReason reason)
> +{
> + virCHDriver *driver = ((virCHDomainObjPrivate
*)vm->privateData)->driver;
> +
> + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY))
> + return -1;
> + virCHProcessStop(driver, vm, reason);
> + virDomainObjEndJob(vm);
> +
> + return 0;
> +}
> +
> +static int virCHProcessEvent(virCHMonitor *mon,
> + virJSONValue *eventJSON)
> +{
> + const char *event;
> + const char *source;
> + virCHEvent ev;
> + g_autofree char *timestamp = NULL;
> + g_autofree char *full_event = NULL;
> + virDomainObj *vm = mon->vm;
> + int ret = 0;
> +
> + if (virJSONValueObjectHasKey(eventJSON, "source") == 0) {
> + VIR_WARN("Invalid JSON from monitor, no source key");
> + return -1;
> + }
> + if (virJSONValueObjectHasKey(eventJSON, "event") == 0) {
> + VIR_WARN("Invalid JSON from monitor, no event key");
> + return -1;
> + }
> + source = virJSONValueObjectGetString(eventJSON, "source");
> + event = virJSONValueObjectGetString(eventJSON, "event");
> + full_event = g_strdup_printf("%s:%s", source, event);
> + ev = virCHEventTypeFromString(full_event);
> + VIR_DEBUG("Source: %s Event: %s, ev: %d", source, event, ev);
> +
> + switch (ev) {
> + case VIR_CH_EVENT_VMM_STARTING:
> + case VIR_CH_EVENT_VM_BOOTING:
> + case VIR_CH_EVENT_VM_BOOTED:
> + case VIR_CH_EVENT_VM_REBOOTING:
> + case VIR_CH_EVENT_VM_REBOOTED:
> + case VIR_CH_EVENT_VM_PAUSING:
> + case VIR_CH_EVENT_VM_PAUSED:
> + case VIR_CH_EVENT_VM_RESUMING:
> + case VIR_CH_EVENT_VM_RESUMED:
> + case VIR_CH_EVENT_VM_SNAPSHOTTING:
> + case VIR_CH_EVENT_VM_SNAPSHOTTED:
> + case VIR_CH_EVENT_VM_RESTORING:
> + case VIR_CH_EVENT_VM_RESTORED:
> + case VIR_CH_EVENT_VM_DELETED:
> + break;
> + case VIR_CH_EVENT_VMM_SHUTDOWN:
> + virObjectLock(vm);
> + if (virCHEventStopProcess(vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN)) {
> + VIR_WARN("Failed to mark the VM(%s) as SHUTDOWN!",
> + vm->def->name);
> + ret = -1;
> + }
> + virObjectUnlock(vm);
> + break;
> + case VIR_CH_EVENT_VM_SHUTDOWN:
> + virObjectLock(vm);
> + virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF,
VIR_DOMAIN_SHUTOFF_SHUTDOWN);
Is the ch process already killed in this scenario?
Yes, vmm shutdown event is raised when the process is terminated.
With master branch, I dont the CH process running anymore after issue
`shutdown` command from within the domain.
> > + virObjectUnlock(vm);
> > + break;
> > + case VIR_CH_EVENT_LAST:
> > + default:
> > + VIR_WARN("Unknown event: %s", full_event);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static int virCHProcessEvents(virCHMonitor *mon)
> > {
> > char *buf = mon->event_buffer.buffer;
> > @@ -88,7 +186,10 @@ static int virCHProcessEvents(virCHMonitor *mon)
> > json_start = buf + start_index;
> > if ((obj = virJSONValueFromString(json_start))) {
> > - /* Process the event string (obj) here */
> > + if (virCHProcessEvent(mon, obj) < 0) {
> > + VIR_WARN("Failed to process JSON event doc:
%s", json_start);
> > + ret = -1;
> > + }
> > virJSONValueFree(obj);
> > } else {
> > VIR_WARN("Invalid JSON event doc: %s",
json_start);
> > diff --git a/src/ch/ch_events.h b/src/ch/ch_events.h
> > index 2e9cdf03bb..3b360628f7 100644
> > --- a/src/ch/ch_events.h
> > +++ b/src/ch/ch_events.h
> > @@ -24,5 +24,31 @@
> > #define CH_EVENT_BUFFER_SZ PIPE_BUF
> > +typedef enum {
> > + /* source: vmm */
> > + VIR_CH_EVENT_VMM_STARTING = 0,
> > + VIR_CH_EVENT_VMM_SHUTDOWN,
> > +
> > + /* source: vm */
> > + VIR_CH_EVENT_VM_BOOTING,
> > + VIR_CH_EVENT_VM_BOOTED,
> > + VIR_CH_EVENT_VM_REBOOTING,
> > + VIR_CH_EVENT_VM_REBOOTED,
> > + VIR_CH_EVENT_VM_SHUTDOWN,
> > + VIR_CH_EVENT_VM_DELETED,
> > + VIR_CH_EVENT_VM_PAUSING,
> > + VIR_CH_EVENT_VM_PAUSED,
> > + VIR_CH_EVENT_VM_RESUMING,
> > + VIR_CH_EVENT_VM_RESUMED,
> > + VIR_CH_EVENT_VM_SNAPSHOTTING,
> > + VIR_CH_EVENT_VM_SNAPSHOTTED,
> > + VIR_CH_EVENT_VM_RESTORING,
> > + VIR_CH_EVENT_VM_RESTORED,
> > +
> > + VIR_CH_EVENT_LAST
> > +} virCHEvent;
> > +
> > +VIR_ENUM_DECL(virCHEvent);
> > +
> > int virCHStartEventHandler(virCHMonitor *mon);
> > void virCHStopEventHandler(virCHMonitor *mon);
>
> --
> Regards,
> Praveen K Paladugu
8:02 a.m.
New subject: [PATCH 6/6] NEWS: Mention event handling support in ch driver
Signed-off-by: Purna Pavan Chandra <paekkaladevi(a)linux.microsoft.com>
Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi(a)linux.microsoft.com>
---
NEWS.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst
index beea8221e1..43bddedc3d 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -17,6 +17,13 @@ v10.8.0 (unreleased)
* **New features**
+ * ch: Support handling events from cloud-hypervisor
+
+ The ch driver now supports handling events from the cloud-hypervisor.
+ Events include VM lifecyle operations such as shutdown, pause, resume,
+ etc. Libvirt will now read these events and take actions such as
+ updating domain state, etc.
+
* **Improvements**
* **Bug fixes**
--
2.34.1
52
days inactive
64
days old
18 comments
2 participants
participants (2)
-
Praveen K Paladugu -
Purna Pavan Chandra Aekkaladevi