[PATCH v3 1/5] ch: pass --event-monitor option to cloud-hypervisor

Purna Pavan Chandra Aekkaladevi posted 5 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v3 1/5] ch: pass --event-monitor option to cloud-hypervisor
Posted by Purna Pavan Chandra Aekkaladevi 1 year, 3 months ago
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@linux.microsoft.com>
Co-authored-by: Vineeth Pillai <viremana@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..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;
     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 (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);
+    }
+
     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
Re: [PATCH v3 1/5] ch: pass --event-monitor option to cloud-hypervisor
Posted by Michal Prívozník 1 year, 2 months ago
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@linux.microsoft.com>
> Co-authored-by: Vineeth Pillai <viremana@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..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