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 <[email protected]> --- 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
