On 10/23/24 10:02, 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@xxxxxxxxxxxxxxxxxxx>
> Co-authored-by: Vineeth Pillai <viremana@xxxxxxxxxxxxxxxxxxx>
> ---
> 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..21a9ee273e 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;
The cleanup function for virCHMonitor type is virCHMonitorClose().
> 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);
And here, mon->eventmonitorpath is allocated.
> + if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) {
> virReportSystemError(errno,
> _("Cannot create socket directory '%1$s'"),
> - socketdir);
> + cfg->stateDir);
> return NULL;
So if this fails for example, and this return is executed, then
virCHMonitorClose() is called ....
> }
>
> @@ -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 (virFileExists(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);
.. and since the file does NOT exist, this g_free() is never called
resulting in memory leak.
> + }
> +
> virObjectUnref(mon);
Michal
