[PATCH 2/2] ch: add log level configuration option

Stefan Kober posted 2 patches 3 months, 2 weeks ago
There is a newer version of this series
[PATCH 2/2] ch: add log level configuration option
Posted by Stefan Kober 3 months, 2 weeks ago
Allow a user to set the verbosity of the cloud hypervisor instances by
specifying it in the ch.conf configuration file.
---
 src/ch/ch_conf.c    | 10 ++++++++++
 src/ch/ch_conf.h    | 15 +++++++++++++++
 src/ch/ch_monitor.c |  6 ++++++
 3 files changed, 31 insertions(+)

diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
index 7d3f600707..3e700586ca 100644
--- a/src/ch/ch_conf.c
+++ b/src/ch/ch_conf.c
@@ -98,6 +98,16 @@ int virCHDriverConfigLoadFile(virCHDriverConfig *cfg,
     if (!(conf = virConfReadFile(filename, 0)))
         return -1;
 
+    if (virConfGetValueUInt(conf, "log_level", &cfg->logLevel) < 0)
+        return -1;
+
+    if (!(cfg->logLevel < VIR_CH_LOGLEVEL_LAST)) {
+        VIR_WARN("Invalid log level %u. Using default %u instead.",
+                 cfg->logLevel,
+                 VIR_CH_LOGLEVEL_DEFAULT);
+        cfg->logLevel = VIR_CH_LOGLEVEL_DEFAULT;
+    }
+
     return 0;
 }
 
diff --git a/src/ch/ch_conf.h b/src/ch/ch_conf.h
index 2f0d090d35..1660762f2b 100644
--- a/src/ch/ch_conf.h
+++ b/src/ch/ch_conf.h
@@ -34,6 +34,19 @@ typedef struct _virCHDriver virCHDriver;
 
 typedef struct _virCHDriverConfig virCHDriverConfig;
 
+typedef enum {
+    /* Standard log level only showing warning and error messages. */
+    VIR_CH_LOGLEVEL_DEFAULT = 0,
+
+    /* Additional info messages are shown. Will not spam the log. */
+    VIR_CH_LOGLEVEL_INFO,
+
+    /* Additional debug messages are shown. Will be very verbose. */
+    VIR_CH_LOGLEVEL_DEBUG,
+
+    VIR_CH_LOGLEVEL_LAST
+} virCHLogLevel;
+
 struct _virCHDriverConfig {
     GObject parent;
 
@@ -48,6 +61,8 @@ struct _virCHDriverConfig {
     gid_t group;
 
     bool stdioLogD;
+
+    virCHLogLevel logLevel;
 };
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCHDriverConfig, virObjectUnref);
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index 3d3b4cb87d..6bf877fef3 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -698,6 +698,12 @@ virCHMonitorNew(virDomainObj *vm, virCHDriverConfig *cfg, int logfile)
         return NULL;
     }
 
+    if (cfg->logLevel == VIR_CH_LOGLEVEL_INFO) {
+        virCommandAddArg(cmd, "-v");
+    } else if (cfg->logLevel == VIR_CH_LOGLEVEL_DEBUG) {
+        virCommandAddArg(cmd, "-vv");
+    }
+
     virCommandAddArg(cmd, "--api-socket");
     virCommandAddArgFormat(cmd, "fd=%d", socket_fd);
     virCommandPassFD(cmd, socket_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
-- 
2.49.0
Re: [PATCH 2/2] ch: add log level configuration option
Posted by Michal Prívozník via Devel 3 months, 2 weeks ago
On 5/22/25 08:21, Stefan Kober wrote:
> Allow a user to set the verbosity of the cloud hypervisor instances by
> specifying it in the ch.conf configuration file.
> ---
>  src/ch/ch_conf.c    | 10 ++++++++++
>  src/ch/ch_conf.h    | 15 +++++++++++++++
>  src/ch/ch_monitor.c |  6 ++++++
>  3 files changed, 31 insertions(+)

Here you'd add some rules to the augeas file, example with comment into
ch.conf and maybe a test case.

> 
> diff --git a/src/ch/ch_conf.c b/src/ch/ch_conf.c
> index 7d3f600707..3e700586ca 100644
> --- a/src/ch/ch_conf.c
> +++ b/src/ch/ch_conf.c
> @@ -98,6 +98,16 @@ int virCHDriverConfigLoadFile(virCHDriverConfig *cfg,
>      if (!(conf = virConfReadFile(filename, 0)))
>          return -1;
>  
> +    if (virConfGetValueUInt(conf, "log_level", &cfg->logLevel) < 0)
> +        return -1;
> +
> +    if (!(cfg->logLevel < VIR_CH_LOGLEVEL_LAST)) {
> +        VIR_WARN("Invalid log level %u. Using default %u instead.",
> +                 cfg->logLevel,
> +                 VIR_CH_LOGLEVEL_DEFAULT);
> +        cfg->logLevel = VIR_CH_LOGLEVEL_DEFAULT;

I think we should just reject invalid values from the beginning. If user
requests invalid value they should fix their config file instead of us
falling back onto some default, albeit sensible. For instance, if I'd
provide "log_level" as a float the virConfGetValueUInt() above would
reject parsing anyways.

> +    }
> +
>      return 0;
>  }
>  

Michal