public inbox for qemu-devel@nongnu.org
 help / color / mirror / Atom feed
* [PATCH v1] qga: rework slog to support multiple severity levels
@ 2026-03-12 14:16 Elizabeth Ashurov
       [not found] ` <abLVj2eLZmKbPs9J@redhat.com>
  2026-03-18 15:47 ` [PATCH v2 1/2] qga: replace slog() with standard GLib logging Elizabeth Ashurov
  0 siblings, 2 replies; 11+ messages in thread
From: Elizabeth Ashurov @ 2026-03-12 14:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: kkostiuk, berrange, Elizabeth Ashurov

ga_log() forwarded all "syslog" domain messages unconditionally,
bypassing the log level filter. Furthermore, slog() hardcoded
all messages to G_LOG_LEVEL_INFO, causing log spam from frequent
guest-ping calls.

- Split slog() into slog_error() (WARNING), slog() (MESSAGE), and slog_trace() (INFO)
- Apply s->log_level filtering to "syslog" in ga_log()
- Move guest-ping to slog_trace()
- Move error messages to slog_error()

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3214

Signed-off-by: Elizabeth Ashurov <eashurov@redhat.com>
---
 qga/commands-linux.c   |  6 ++++--
 qga/commands-posix.c   | 13 +++++++------
 qga/commands-win32.c   | 34 +++++++++++++++++-----------------
 qga/commands.c         | 30 ++++++++++++++++++++++++------
 qga/guest-agent-core.h |  2 ++
 qga/main.c             |  6 +++++-
 6 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/qga/commands-linux.c b/qga/commands-linux.c
index 378f4d080c..b924e44517 100644
--- a/qga/commands-linux.c
+++ b/qga/commands-linux.c
@@ -44,7 +44,8 @@ static int dev_major_minor(const char *devpath,
     *devminor = 0;
 
     if (stat(devpath, &st) < 0) {
-        slog("failed to stat device file '%s': %s", devpath, strerror(errno));
+        slog_error("failed to stat device file '%s': %s",
+                   devpath, strerror(errno));
         return -1;
     }
     if (S_ISDIR(st.st_mode)) {
@@ -2072,7 +2073,8 @@ GuestCpuStatsList *qmp_guest_get_cpustats(Error **errp)
         }
 
         if (i < 5) {
-            slog("Parsing cpu stat from %s failed, see \"man proc\"", cpustats);
+            slog_error("Parsing cpu stat from %s failed, see \"man proc\"",
+                       cpustats);
             break;
         }
 
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 837be51c40..e7dc92fed0 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -645,7 +645,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
     write_count = fwrite(buf, 1, count, fh);
     if (ferror(fh)) {
         error_setg_errno(errp, errno, "failed to write to file");
-        slog("guest-file-write failed, handle: %" PRId64, handle);
+        slog_error("guest-file-write failed, handle: %" PRId64, handle);
     } else {
         write_data = g_new0(GuestFileWrite, 1);
         write_data->count = write_count;
@@ -849,8 +849,8 @@ static void guest_fsfreeze_cleanup(void)
     if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) {
         qmp_guest_fsfreeze_thaw(&err);
         if (err) {
-            slog("failed to clean up frozen filesystems: %s",
-                 error_get_pretty(err));
+            slog_error("failed to clean up frozen filesystems: %s",
+                       error_get_pretty(err));
             error_free(err);
         }
     }
@@ -1282,19 +1282,20 @@ static GKeyFile *ga_parse_osrelease(const char *fname)
     const char *group = "[os-release]\n";
 
     if (!g_file_get_contents(fname, &content, NULL, &err)) {
-        slog("failed to read '%s', error: %s", fname, err->message);
+        slog_error("failed to read '%s', error: %s", fname, err->message);
         goto fail;
     }
 
     if (!g_utf8_validate(content, -1, NULL)) {
-        slog("file is not utf-8 encoded: %s", fname);
+        slog_error("file is not utf-8 encoded: %s", fname);
         goto fail;
     }
     content2 = g_strdup_printf("%s%s", group, content);
 
     if (!g_key_file_load_from_data(keys, content2, -1, G_KEY_FILE_NONE,
                                    &err)) {
-        slog("failed to parse file '%s', error: %s", fname, err->message);
+        slog_error("failed to parse file '%s', error: %s", fname,
+                   err->message);
         goto fail;
     }
 
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index c0bf3467bd..cada3135fc 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -361,7 +361,7 @@ void qmp_guest_shutdown(const char *mode, Error **errp)
 
     if (!ExitWindowsEx(shutdown_flag, SHTDN_REASON_FLAG_PLANNED)) {
         g_autofree gchar *emsg = g_win32_error_message(GetLastError());
-        slog("guest-shutdown failed: %s", emsg);
+        slog_error("guest-shutdown failed: %s", emsg);
         error_setg_win32(errp, GetLastError(), "guest-shutdown failed");
     }
 }
@@ -426,7 +426,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
     is_ok = WriteFile(fh, buf, count, &write_count, NULL);
     if (!is_ok) {
         error_setg_win32(errp, GetLastError(), "failed to write to file");
-        slog("guest-file-write-failed, handle: %" PRId64, handle);
+        slog_error("guest-file-write-failed, handle: %" PRId64, handle);
     } else {
         write_data = g_new0(GuestFileWrite, 1);
         write_data->count = (size_t) write_count;
@@ -1310,8 +1310,8 @@ static void guest_fsfreeze_cleanup(void)
     if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) {
         qmp_guest_fsfreeze_thaw(&err);
         if (err) {
-            slog("failed to clean up frozen filesystems: %s",
-                 error_get_pretty(err));
+            slog_error("failed to clean up frozen filesystems: %s",
+                   error_get_pretty(err));
             error_free(err);
         }
     }
@@ -1464,7 +1464,7 @@ static DWORD WINAPI do_suspend(LPVOID opaque)
 
     if (!SetSuspendState(*mode == GUEST_SUSPEND_MODE_DISK, TRUE, TRUE)) {
         g_autofree gchar *emsg = g_win32_error_message(GetLastError());
-        slog("failed to suspend guest: %s", emsg);
+        slog_error("failed to suspend guest: %s", emsg);
         ret = -1;
     }
     g_free(mode);
@@ -2174,8 +2174,8 @@ static char *ga_get_win_name(const OSVERSIONINFOEXW *os_version, bool id)
         }
         ++table;
     }
-    slog("failed to lookup Windows version: major=%lu, minor=%lu",
-        major, minor);
+    slog_error("failed to lookup Windows version: major=%lu, minor=%lu",
+               major, minor);
     return g_strdup("N/A");
 }
 
@@ -2244,8 +2244,8 @@ static char *ga_get_current_arch(void)
         break;
     case PROCESSOR_ARCHITECTURE_UNKNOWN:
     default:
-        slog("unknown processor architecture 0x%0x",
-            info.wProcessorArchitecture);
+        slog_error("unknown processor architecture 0x%0x",
+                   info.wProcessorArchitecture);
         result = g_strdup("unknown");
         break;
     }
@@ -2308,14 +2308,14 @@ static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
         buffer, &buffer_len, 0);
     if (cr != CR_SUCCESS && cr != CR_BUFFER_SMALL) {
 
-        slog("failed to get property size, error=0x%lx", cr);
+        slog_error("failed to get property size, error=0x%lx", cr);
         return NULL;
     }
     buffer = g_new0(BYTE, buffer_len + 1);
     cr = CM_Get_DevNode_PropertyW(devInst, propName, propType,
         buffer, &buffer_len, 0);
     if (cr != CR_SUCCESS) {
-        slog("failed to get device property, error=0x%lx", cr);
+        slog_error("failed to get device property, error=0x%lx", cr);
         return NULL;
     }
     return g_steal_pointer(&buffer);
@@ -2329,7 +2329,7 @@ static GStrv ga_get_hardware_ids(DEVINST devInstance)
     g_autofree LPWSTR property = (LPWSTR)cm_get_property(devInstance,
         &qga_DEVPKEY_Device_HardwareIds, &cm_type);
     if (property == NULL) {
-        slog("failed to get hardware IDs");
+        slog_error("failed to get hardware IDs");
         return NULL;
     }
     if (*property == '\0') {
@@ -2385,7 +2385,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
         name = (LPWSTR)cm_get_property(dev_info_data.DevInst,
             &qga_DEVPKEY_NAME, &cm_type);
         if (name == NULL) {
-            slog("failed to get device description");
+            slog_error("failed to get device description");
             continue;
         }
         device->driver_name = g_utf16_to_utf8(name, -1, NULL, NULL, NULL);
@@ -2424,7 +2424,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
         version = (LPWSTR)cm_get_property(dev_info_data.DevInst,
             &qga_DEVPKEY_Device_DriverVersion, &cm_type);
         if (version == NULL) {
-            slog("failed to get driver version");
+            slog_error("failed to get driver version");
             continue;
         }
         device->driver_version = g_utf16_to_utf8(version, -1, NULL,
@@ -2437,7 +2437,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
         date = (LPFILETIME)cm_get_property(dev_info_data.DevInst,
             &qga_DEVPKEY_Device_DriverDate, &cm_type);
         if (date == NULL) {
-            slog("failed to get driver date");
+            slog_error("failed to get driver date");
             continue;
         }
         device->driver_date = filetime_to_ns(date);
@@ -2479,8 +2479,8 @@ static VOID CALLBACK load_avg_callback(PVOID hCounter, BOOLEAN timedOut)
         (PDH_HCOUNTER)hCounter, PDH_FMT_DOUBLE, 0, &displayValue);
     /* Skip updating the load if we can't get the value successfully */
     if (err != ERROR_SUCCESS) {
-        slog("PdhGetFormattedCounterValue failed to get load value with 0x%lx",
-             err);
+        slog_error("PdhGetFormattedCounterValue failed to get load value"
+                   " with 0x%lx", err);
         return;
     }
     currentLoad = displayValue.doubleValue;
diff --git a/qga/commands.c b/qga/commands.c
index 5f20af25d3..ed52a7d3c2 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -38,6 +38,24 @@ void slog(const gchar *fmt, ...)
 {
     va_list ap;
 
+    va_start(ap, fmt);
+    g_logv("syslog", G_LOG_LEVEL_MESSAGE, fmt, ap);
+    va_end(ap);
+}
+
+void slog_error(const gchar *fmt, ...)
+{
+    va_list ap;
+
+    va_start(ap, fmt);
+    g_logv("syslog", G_LOG_LEVEL_WARNING, fmt, ap);
+    va_end(ap);
+}
+
+void slog_trace(const gchar *fmt, ...)
+{
+    va_list ap;
+
     va_start(ap, fmt);
     g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap);
     va_end(ap);
@@ -56,7 +74,7 @@ int64_t qmp_guest_sync(int64_t id, Error **errp)
 
 void qmp_guest_ping(Error **errp)
 {
-    slog("guest-ping called");
+    slog_trace("guest-ping called");
 }
 
 static void qmp_command_info(const QmpCommand *cmd, void *opaque)
@@ -285,8 +303,8 @@ static void guest_exec_task_setup(gpointer data)
          * inside the parent, not the child.
          */
         if (dup2(STDOUT_FILENO, STDERR_FILENO) != 0) {
-            slog("dup2() failed to merge stderr into stdout: %s",
-                 strerror(errno));
+            slog_error("dup2() failed to merge stderr into stdout: %s",
+                       strerror(errno));
         }
     }
 
@@ -295,8 +313,8 @@ static void guest_exec_task_setup(gpointer data)
     sigact.sa_handler = SIG_DFL;
 
     if (sigaction(SIGPIPE, &sigact, NULL) != 0) {
-        slog("sigaction() failed to reset child process's SIGPIPE: %s",
-             strerror(errno));
+        slog_error("sigaction() failed to reset child process's SIGPIPE: %s",
+                   strerror(errno));
     }
 #endif
 }
@@ -626,7 +644,7 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
 
     read_data = guest_file_read_unsafe(gfh, count, errp);
     if (!read_data) {
-        slog("guest-file-write failed, handle: %" PRId64, handle);
+        slog_error("guest-file-write failed, handle: %" PRId64, handle);
     }
 
     return read_data;
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index d9f3922adf..7c50e43dfc 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -41,6 +41,8 @@ bool ga_logging_enabled(GAState *s);
 void ga_disable_logging(GAState *s);
 void ga_enable_logging(GAState *s);
 void G_GNUC_PRINTF(1, 2) slog(const gchar *fmt, ...);
+void G_GNUC_PRINTF(1, 2) slog_error(const gchar *fmt, ...);
+void G_GNUC_PRINTF(1, 2) slog_trace(const gchar *fmt, ...);
 void ga_set_response_delimited(GAState *s);
 bool ga_is_frozen(GAState *s);
 void ga_set_frozen(GAState *s);
diff --git a/qga/main.c b/qga/main.c
index fd19c7037d..bc786593fd 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -392,6 +392,9 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
 
     level &= G_LOG_LEVEL_MASK;
     if (g_strcmp0(domain, "syslog") == 0) {
+        if (!(level & s->log_level)) {
+            return;
+        }
 #ifndef _WIN32
         syslog(glib_log_level_to_system(level), "%s: %s", level_str, msg);
 #else
@@ -1673,7 +1676,8 @@ int main(int argc, char **argv)
     GAConfig *config = g_new0(GAConfig, 1);
     int socket_activation;
 
-    config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
+    config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL
+                      | G_LOG_LEVEL_WARNING | G_LOG_LEVEL_MESSAGE;
 
     qemu_init_exec_dir(argv[0]);
     qga_qmp_init_marshal(&ga_commands);
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-03-24 18:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-12 14:16 [PATCH v1] qga: rework slog to support multiple severity levels Elizabeth Ashurov
     [not found] ` <abLVj2eLZmKbPs9J@redhat.com>
     [not found]   ` <CAPMcbCq4i8DhwRqPkJxywt6H==NRW=3_7NA0ZQhe8cMFtVGRCA@mail.gmail.com>
2026-03-12 15:36     ` Daniel P. Berrangé
2026-03-18 16:13       ` Elizabeth Ashurov
2026-03-18 15:47 ` [PATCH v2 1/2] qga: replace slog() with standard GLib logging Elizabeth Ashurov
2026-03-18 15:47   ` [PATCH v2 2/2] qga: add --audit option for command logging control Elizabeth Ashurov
2026-03-23 13:46     ` Kostiantyn Kostiuk
2026-03-24 14:08       ` Elizabeth Ashurov
2026-03-24 14:18         ` Kostiantyn Kostiuk
2026-03-20 12:58   ` [PATCH v2 1/2] qga: replace slog() with standard GLib logging Daniel P. Berrangé
2026-03-24 14:15     ` Elizabeth Ashurov
2026-03-24 18:02   ` Daniel P. Berrangé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox