Re: [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&gt; >Co-authored-by: Vineeth Pillai <viremana(a)linux.microsoft.com&gt; >--- > 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;