* [PATCH 0/2] qga: improve "syslog" domain logging
@ 2022-11-28 18:54 Andrey Drobyshev via
2022-11-28 18:54 ` [PATCH 1/2] qga-win: add logging to Windows event log Andrey Drobyshev via
2022-11-28 18:54 ` [PATCH 2/2] qga: map GLib log levels to system levels Andrey Drobyshev via
0 siblings, 2 replies; 8+ messages in thread
From: Andrey Drobyshev via @ 2022-11-28 18:54 UTC (permalink / raw)
To: qemu-devel; +Cc: andrey.drobyshev, den, yur
These patches extend QGA logging interface, primarily under Windows
guests. They enable QGA to write to Windows event log, much like
syslog() on *nix. In addition we get rid of hardcoded log level used by
ga_log().
Andrey Drobyshev (2):
qga-win: add logging to Windows event log
qga: map GLib log levels to system levels
configure | 3 +++
qga/installer/qemu-ga.wxs | 5 ++++
qga/main.c | 49 +++++++++++++++++++++++++++++++++++----
qga/meson.build | 19 ++++++++++++++-
qga/messages-win32.mc | 9 +++++++
5 files changed, 80 insertions(+), 5 deletions(-)
create mode 100644 qga/messages-win32.mc
--
2.38.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] qga-win: add logging to Windows event log
2022-11-28 18:54 [PATCH 0/2] qga: improve "syslog" domain logging Andrey Drobyshev via
@ 2022-11-28 18:54 ` Andrey Drobyshev via
2022-11-28 22:36 ` M M
2022-11-29 7:37 ` Marc-André Lureau
2022-11-28 18:54 ` [PATCH 2/2] qga: map GLib log levels to system levels Andrey Drobyshev via
1 sibling, 2 replies; 8+ messages in thread
From: Andrey Drobyshev via @ 2022-11-28 18:54 UTC (permalink / raw)
To: qemu-devel; +Cc: andrey.drobyshev, den, yur
This commit allows QGA to write to Windows event log using Win32 API's
ReportEvent() [1], much like syslog() under *nix guests.
In order to generate log message definitions we use a very basic message
text file [2], so that every QGA's message gets ID 1. The tools
"windmc" and "windres" respectively are used to generate ".rc" file and
COFF object file, and then the COFF file is linked into qemu-ga.exe.
[1] https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
[2] https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files
Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
configure | 3 +++
qga/installer/qemu-ga.wxs | 5 +++++
qga/main.c | 15 ++++++++++++---
qga/meson.build | 19 ++++++++++++++++++-
qga/messages-win32.mc | 9 +++++++++
5 files changed, 47 insertions(+), 4 deletions(-)
create mode 100644 qga/messages-win32.mc
diff --git a/configure b/configure
index 26c7bc5154..789a4f6cc9 100755
--- a/configure
+++ b/configure
@@ -372,6 +372,7 @@ smbd="$SMBD"
strip="${STRIP-${cross_prefix}strip}"
widl="${WIDL-${cross_prefix}widl}"
windres="${WINDRES-${cross_prefix}windres}"
+windmc="${WINDMC-${cross_prefix}windmc}"
pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
query_pkg_config() {
"${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
@@ -2561,6 +2562,7 @@ if test "$skip_meson" = no; then
echo "strip = [$(meson_quote $strip)]" >> $cross
echo "widl = [$(meson_quote $widl)]" >> $cross
echo "windres = [$(meson_quote $windres)]" >> $cross
+ echo "windmc = [$(meson_quote $windmc)]" >> $cross
if test "$cross_compile" = "yes"; then
cross_arg="--cross-file config-meson.cross"
echo "[host_machine]" >> $cross
@@ -2667,6 +2669,7 @@ preserve_env SMBD
preserve_env STRIP
preserve_env WIDL
preserve_env WINDRES
+preserve_env WINDMC
printf "exec" >>config.status
for i in "$0" "$@"; do
diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 73ce2c4965..d9567836f3 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -110,6 +110,11 @@
<RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
<RegistryValue Type="string" Name="Version" Value="$(var.QEMU_GA_VERSION)" />
</RegistryKey>
+ <RegistryKey Root="HKLM"
+ Key="System\CurrentControlSet\Services\EventLog\Application\qemu-ga">
+ <RegistryValue Type="integer" Name="TypesSupported" Value="7" />
+ <RegistryValue Type="string" Name="EventMessageFile" Value="[qemu_ga_directory]qemu-ga.exe" />
+ </RegistryKey>
</Component>
</Directory>
</Directory>
diff --git a/qga/main.c b/qga/main.c
index b3580508fa..10314dfe5d 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -82,6 +82,7 @@ struct GAState {
bool logging_enabled;
#ifdef _WIN32
GAService service;
+ HANDLE event_log;
HANDLE wakeup_event;
#endif
bool delimit_response;
@@ -324,13 +325,14 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
}
level &= G_LOG_LEVEL_MASK;
-#ifndef _WIN32
if (g_strcmp0(domain, "syslog") == 0) {
+#ifndef _WIN32
syslog(LOG_INFO, "%s: %s", level_str, msg);
- } else if (level & s->log_level) {
#else
- if (level & s->log_level) {
+ ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
+ 0, 1, NULL, 1, 0, &msg, NULL);
#endif
+ } else if (level & s->log_level) {
g_autoptr(GDateTime) now = g_date_time_new_now_utc();
g_autofree char *nowstr = g_date_time_format(now, "%s.%f");
fprintf(s->log_file, "%s: %s: %s\n", nowstr, level_str, msg);
@@ -1286,6 +1288,13 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
g_debug("Guest agent version %s started", QEMU_FULL_VERSION);
#ifdef _WIN32
+ s->event_log = RegisterEventSource(NULL, "qemu-ga");
+ if (!s->event_log) {
+ g_autofree gchar *errmsg = g_win32_error_message(GetLastError());
+ g_critical("unable to register event source: %s", errmsg);
+ return NULL;
+ }
+
/* On win32 the state directory is application specific (be it the default
* or a user override). We got past the command line parsing; let's create
* the directory (with any intermediate directories). If we run into an
diff --git a/qga/meson.build b/qga/meson.build
index 3cfb9166e5..1ff159edc1 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -98,7 +98,24 @@ if targetos == 'windows'
endif
endif
-qga = executable('qemu-ga', qga_ss.sources(),
+qga_objs = []
+if targetos == 'windows'
+ windmc = find_program('windmc', required: true)
+ windres = find_program('windres', required: true)
+
+ msgrc = custom_target('messages-win32.rc',
+ input: 'messages-win32.mc',
+ output: ['messages-win32.rc', 'MSG00409.bin', 'messages-win32.h'],
+ command: [windmc, '-h', '@OUTDIR@', '-r', '@OUTDIR@', '@INPUT@'])
+ msgobj = custom_target('messages-win32.o',
+ input: msgrc[0],
+ output: 'messages-win32.o',
+ command: [windres, '-I', '@OUTDIR@', '-o', '@OUTPUT@', '@INPUT@'])
+
+ qga_objs = [msgobj]
+endif
+
+qga = executable('qemu-ga', qga_ss.sources() + qga_objs,
link_args: qga_libs,
dependencies: [qemuutil, libudev],
install: true)
diff --git a/qga/messages-win32.mc b/qga/messages-win32.mc
new file mode 100644
index 0000000000..e21019cebe
--- /dev/null
+++ b/qga/messages-win32.mc
@@ -0,0 +1,9 @@
+LanguageNames=(
+ English=0x409:MSG00409
+)
+
+MessageId=1
+SymbolicName=QEMU_GA_EVENTLOG_GENERAL
+Language=English
+%1
+.
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] qga: map GLib log levels to system levels
2022-11-28 18:54 [PATCH 0/2] qga: improve "syslog" domain logging Andrey Drobyshev via
2022-11-28 18:54 ` [PATCH 1/2] qga-win: add logging to Windows event log Andrey Drobyshev via
@ 2022-11-28 18:54 ` Andrey Drobyshev via
2022-11-29 7:38 ` Marc-André Lureau
1 sibling, 1 reply; 8+ messages in thread
From: Andrey Drobyshev via @ 2022-11-28 18:54 UTC (permalink / raw)
To: qemu-devel; +Cc: andrey.drobyshev, den, yur
This patch translates GLib-specific log levels to system ones, so that
they may be used by both *nix syslog() (as a "priority" argument) and
Windows ReportEvent() (as a "wType" argument).
Currently the only codepath to write to "syslog" domain is slog()
function. However, this patch allows the interface to be extended.
Note that since slog() is using G_LOG_LEVEL_INFO level, its behaviour
doesn't change.
Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
qga/main.c | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 10314dfe5d..0467d5daf8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -314,6 +314,38 @@ void ga_enable_logging(GAState *s)
s->logging_enabled = true;
}
+static int glib_log_level_to_system(int level)
+{
+ switch (level) {
+#ifndef _WIN32
+ case G_LOG_LEVEL_ERROR:
+ return LOG_ERR;
+ case G_LOG_LEVEL_CRITICAL:
+ return LOG_CRIT;
+ case G_LOG_LEVEL_WARNING:
+ return LOG_WARNING;
+ case G_LOG_LEVEL_MESSAGE:
+ return LOG_NOTICE;
+ case G_LOG_LEVEL_DEBUG:
+ return LOG_DEBUG;
+ case G_LOG_LEVEL_INFO:
+ default:
+ return LOG_INFO;
+#else
+ case G_LOG_LEVEL_ERROR:
+ case G_LOG_LEVEL_CRITICAL:
+ return EVENTLOG_ERROR_TYPE;
+ case G_LOG_LEVEL_WARNING:
+ return EVENTLOG_WARNING_TYPE;
+ case G_LOG_LEVEL_MESSAGE:
+ case G_LOG_LEVEL_INFO:
+ case G_LOG_LEVEL_DEBUG:
+ default:
+ return EVENTLOG_INFORMATION_TYPE;
+#endif
+ }
+}
+
static void ga_log(const gchar *domain, GLogLevelFlags level,
const gchar *msg, gpointer opaque)
{
@@ -327,9 +359,9 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
level &= G_LOG_LEVEL_MASK;
if (g_strcmp0(domain, "syslog") == 0) {
#ifndef _WIN32
- syslog(LOG_INFO, "%s: %s", level_str, msg);
+ syslog(glib_log_level_to_system(level), "%s: %s", level_str, msg);
#else
- ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
+ ReportEvent(s->event_log, glib_log_level_to_system(level),
0, 1, NULL, 1, 0, &msg, NULL);
#endif
} else if (level & s->log_level) {
--
2.38.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] qga-win: add logging to Windows event log
2022-11-28 18:54 ` [PATCH 1/2] qga-win: add logging to Windows event log Andrey Drobyshev via
@ 2022-11-28 22:36 ` M M
2022-11-29 11:15 ` Andrey Drobyshev
2022-11-29 7:37 ` Marc-André Lureau
1 sibling, 1 reply; 8+ messages in thread
From: M M @ 2022-11-28 22:36 UTC (permalink / raw)
To: Andrey Drobyshev; +Cc: qemu-devel, den, yur
> On 28. 11. 2022., at 21:54, Andrey Drobyshev via <qemu-devel@nongnu.org> wrote:
>
> This commit allows QGA to write to Windows event log using Win32 API's
> ReportEvent() [1], much like syslog() under *nix guests.
>
> In order to generate log message definitions we use a very basic message
> text file [2], so that every QGA's message gets ID 1. The tools
> "windmc" and "windres" respectively are used to generate ".rc" file and
> COFF object file, and then the COFF file is linked into qemu-ga.exe.
>
> [1] https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
> [2] https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files
>
> Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> configure | 3 +++
> qga/installer/qemu-ga.wxs | 5 +++++
> qga/main.c | 15 ++++++++++++---
> qga/meson.build | 19 ++++++++++++++++++-
> qga/messages-win32.mc | 9 +++++++++
> 5 files changed, 47 insertions(+), 4 deletions(-)
> create mode 100644 qga/messages-win32.mc
>
> diff --git a/configure b/configure
> index 26c7bc5154..789a4f6cc9 100755
> --- a/configure
> +++ b/configure
> @@ -372,6 +372,7 @@ smbd="$SMBD"
> strip="${STRIP-${cross_prefix}strip}"
> widl="${WIDL-${cross_prefix}widl}"
> windres="${WINDRES-${cross_prefix}windres}"
> +windmc="${WINDMC-${cross_prefix}windmc}"
> pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
> query_pkg_config() {
> "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
> @@ -2561,6 +2562,7 @@ if test "$skip_meson" = no; then
> echo "strip = [$(meson_quote $strip)]" >> $cross
> echo "widl = [$(meson_quote $widl)]" >> $cross
> echo "windres = [$(meson_quote $windres)]" >> $cross
> + echo "windmc = [$(meson_quote $windmc)]" >> $cross
> if test "$cross_compile" = "yes"; then
> cross_arg="--cross-file config-meson.cross"
> echo "[host_machine]" >> $cross
> @@ -2667,6 +2669,7 @@ preserve_env SMBD
> preserve_env STRIP
> preserve_env WIDL
> preserve_env WINDRES
> +preserve_env WINDMC
>
> printf "exec" >>config.status
> for i in "$0" "$@"; do
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 73ce2c4965..d9567836f3 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -110,6 +110,11 @@
> <RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
> <RegistryValue Type="string" Name="Version" Value="$(var.QEMU_GA_VERSION)" />
> </RegistryKey>
> + <RegistryKey Root="HKLM"
> + Key="System\CurrentControlSet\Services\EventLog\Application\qemu-ga">
> + <RegistryValue Type="integer" Name="TypesSupported" Value="7" />
> + <RegistryValue Type="string" Name="EventMessageFile" Value="[qemu_ga_directory]qemu-ga.exe" />
> + </RegistryKey>
> </Component>
> </Directory>
> </Directory>
> diff --git a/qga/main.c b/qga/main.c
> index b3580508fa..10314dfe5d 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -82,6 +82,7 @@ struct GAState {
> bool logging_enabled;
> #ifdef _WIN32
> GAService service;
> + HANDLE event_log;
Just for the sake of uniformity
shouldn't we close this handle at cleanup_agent() similar to wakeup_event ?
> HANDLE wakeup_event;
> #endif
> bool delimit_response;
> @@ -324,13 +325,14 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
> }
>
> level &= G_LOG_LEVEL_MASK;
> -#ifndef _WIN32
> if (g_strcmp0(domain, "syslog") == 0) {
> +#ifndef _WIN32
> syslog(LOG_INFO, "%s: %s", level_str, msg);
> - } else if (level & s->log_level) {
> #else
> - if (level & s->log_level) {
> + ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
> + 0, 1, NULL, 1, 0, &msg, NULL);
> #endif
> + } else if (level & s->log_level) {
> g_autoptr(GDateTime) now = g_date_time_new_now_utc();
> g_autofree char *nowstr = g_date_time_format(now, "%s.%f");
> fprintf(s->log_file, "%s: %s: %s\n", nowstr, level_str, msg);
> @@ -1286,6 +1288,13 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
> g_debug("Guest agent version %s started", QEMU_FULL_VERSION);
>
> #ifdef _WIN32
> + s->event_log = RegisterEventSource(NULL, "qemu-ga");
> + if (!s->event_log) {
> + g_autofree gchar *errmsg = g_win32_error_message(GetLastError());
> + g_critical("unable to register event source: %s", errmsg);
> + return NULL;
> + }
> +
> /* On win32 the state directory is application specific (be it the default
> * or a user override). We got past the command line parsing; let's create
> * the directory (with any intermediate directories). If we run into an
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] qga-win: add logging to Windows event log
2022-11-28 18:54 ` [PATCH 1/2] qga-win: add logging to Windows event log Andrey Drobyshev via
2022-11-28 22:36 ` M M
@ 2022-11-29 7:37 ` Marc-André Lureau
2022-11-29 12:17 ` Andrey Drobyshev
1 sibling, 1 reply; 8+ messages in thread
From: Marc-André Lureau @ 2022-11-29 7:37 UTC (permalink / raw)
To: Andrey Drobyshev; +Cc: qemu-devel, den, yur
Hi
On Tue, Nov 29, 2022 at 12:59 AM Andrey Drobyshev via
<qemu-devel@nongnu.org> wrote:
>
> This commit allows QGA to write to Windows event log using Win32 API's
> ReportEvent() [1], much like syslog() under *nix guests.
>
> In order to generate log message definitions we use a very basic message
> text file [2], so that every QGA's message gets ID 1. The tools
> "windmc" and "windres" respectively are used to generate ".rc" file and
> COFF object file, and then the COFF file is linked into qemu-ga.exe.
>
> [1] https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
> [2] https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files
>
fyi, some years ago, I tried to add support to GLib:
https://gitlab.gnome.org/GNOME/glib/-/issues/665. Unfortunately it
didn't land. And in the meantime, GLib got structured logging. There
is a long-standing FIXME in GLog to support Event Log. Someone
probably needs to start from scratch.
> Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> configure | 3 +++
> qga/installer/qemu-ga.wxs | 5 +++++
> qga/main.c | 15 ++++++++++++---
> qga/meson.build | 19 ++++++++++++++++++-
> qga/messages-win32.mc | 9 +++++++++
> 5 files changed, 47 insertions(+), 4 deletions(-)
> create mode 100644 qga/messages-win32.mc
>
> diff --git a/configure b/configure
> index 26c7bc5154..789a4f6cc9 100755
> --- a/configure
> +++ b/configure
> @@ -372,6 +372,7 @@ smbd="$SMBD"
> strip="${STRIP-${cross_prefix}strip}"
> widl="${WIDL-${cross_prefix}widl}"
> windres="${WINDRES-${cross_prefix}windres}"
> +windmc="${WINDMC-${cross_prefix}windmc}"
> pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
> query_pkg_config() {
> "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
> @@ -2561,6 +2562,7 @@ if test "$skip_meson" = no; then
> echo "strip = [$(meson_quote $strip)]" >> $cross
> echo "widl = [$(meson_quote $widl)]" >> $cross
> echo "windres = [$(meson_quote $windres)]" >> $cross
> + echo "windmc = [$(meson_quote $windmc)]" >> $cross
> if test "$cross_compile" = "yes"; then
> cross_arg="--cross-file config-meson.cross"
> echo "[host_machine]" >> $cross
> @@ -2667,6 +2669,7 @@ preserve_env SMBD
> preserve_env STRIP
> preserve_env WIDL
> preserve_env WINDRES
> +preserve_env WINDMC
>
> printf "exec" >>config.status
> for i in "$0" "$@"; do
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index 73ce2c4965..d9567836f3 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -110,6 +110,11 @@
> <RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
> <RegistryValue Type="string" Name="Version" Value="$(var.QEMU_GA_VERSION)" />
> </RegistryKey>
> + <RegistryKey Root="HKLM"
> + Key="System\CurrentControlSet\Services\EventLog\Application\qemu-ga">
> + <RegistryValue Type="integer" Name="TypesSupported" Value="7" />
> + <RegistryValue Type="string" Name="EventMessageFile" Value="[qemu_ga_directory]qemu-ga.exe" />
> + </RegistryKey>
> </Component>
> </Directory>
> </Directory>
> diff --git a/qga/main.c b/qga/main.c
> index b3580508fa..10314dfe5d 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -82,6 +82,7 @@ struct GAState {
> bool logging_enabled;
> #ifdef _WIN32
> GAService service;
> + HANDLE event_log;
> HANDLE wakeup_event;
> #endif
> bool delimit_response;
> @@ -324,13 +325,14 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
> }
>
> level &= G_LOG_LEVEL_MASK;
> -#ifndef _WIN32
> if (g_strcmp0(domain, "syslog") == 0) {
> +#ifndef _WIN32
> syslog(LOG_INFO, "%s: %s", level_str, msg);
> - } else if (level & s->log_level) {
> #else
> - if (level & s->log_level) {
> + ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
> + 0, 1, NULL, 1, 0, &msg, NULL);
Interesting. imho we should remove the "syslog" domain, that's not how
the GLog "domain" is supposed to be used. That's another story..
> #endif
> + } else if (level & s->log_level) {
> g_autoptr(GDateTime) now = g_date_time_new_now_utc();
> g_autofree char *nowstr = g_date_time_format(now, "%s.%f");
> fprintf(s->log_file, "%s: %s: %s\n", nowstr, level_str, msg);
> @@ -1286,6 +1288,13 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
> g_debug("Guest agent version %s started", QEMU_FULL_VERSION);
>
> #ifdef _WIN32
> + s->event_log = RegisterEventSource(NULL, "qemu-ga");
> + if (!s->event_log) {
> + g_autofree gchar *errmsg = g_win32_error_message(GetLastError());
> + g_critical("unable to register event source: %s", errmsg);
> + return NULL;
> + }
> +
> /* On win32 the state directory is application specific (be it the default
> * or a user override). We got past the command line parsing; let's create
> * the directory (with any intermediate directories). If we run into an
> diff --git a/qga/meson.build b/qga/meson.build
> index 3cfb9166e5..1ff159edc1 100644
> --- a/qga/meson.build
> +++ b/qga/meson.build
> @@ -98,7 +98,24 @@ if targetos == 'windows'
> endif
> endif
>
> -qga = executable('qemu-ga', qga_ss.sources(),
> +qga_objs = []
> +if targetos == 'windows'
> + windmc = find_program('windmc', required: true)
> + windres = find_program('windres', required: true)
> +
> + msgrc = custom_target('messages-win32.rc',
> + input: 'messages-win32.mc',
> + output: ['messages-win32.rc', 'MSG00409.bin', 'messages-win32.h'],
> + command: [windmc, '-h', '@OUTDIR@', '-r', '@OUTDIR@', '@INPUT@'])
> + msgobj = custom_target('messages-win32.o',
> + input: msgrc[0],
> + output: 'messages-win32.o',
> + command: [windres, '-I', '@OUTDIR@', '-o', '@OUTPUT@', '@INPUT@'])
> +
> + qga_objs = [msgobj]
> +endif
> +
> +qga = executable('qemu-ga', qga_ss.sources() + qga_objs,
> link_args: qga_libs,
> dependencies: [qemuutil, libudev],
> install: true)
> diff --git a/qga/messages-win32.mc b/qga/messages-win32.mc
> new file mode 100644
> index 0000000000..e21019cebe
> --- /dev/null
> +++ b/qga/messages-win32.mc
> @@ -0,0 +1,9 @@
> +LanguageNames=(
> + English=0x409:MSG00409
> +)
> +
> +MessageId=1
> +SymbolicName=QEMU_GA_EVENTLOG_GENERAL
> +Language=English
> +%1
> +.
> --
> 2.38.1
>
>
lgtm, with the cleanup requested by Mike, I'll add my r-b
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] qga: map GLib log levels to system levels
2022-11-28 18:54 ` [PATCH 2/2] qga: map GLib log levels to system levels Andrey Drobyshev via
@ 2022-11-29 7:38 ` Marc-André Lureau
0 siblings, 0 replies; 8+ messages in thread
From: Marc-André Lureau @ 2022-11-29 7:38 UTC (permalink / raw)
To: Andrey Drobyshev; +Cc: qemu-devel, den, yur
On Tue, Nov 29, 2022 at 12:59 AM Andrey Drobyshev via
<qemu-devel@nongnu.org> wrote:
>
> This patch translates GLib-specific log levels to system ones, so that
> they may be used by both *nix syslog() (as a "priority" argument) and
> Windows ReportEvent() (as a "wType" argument).
>
> Currently the only codepath to write to "syslog" domain is slog()
> function. However, this patch allows the interface to be extended.
>
> Note that since slog() is using G_LOG_LEVEL_INFO level, its behaviour
> doesn't change.
>
> Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qga/main.c | 36 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 10314dfe5d..0467d5daf8 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -314,6 +314,38 @@ void ga_enable_logging(GAState *s)
> s->logging_enabled = true;
> }
>
> +static int glib_log_level_to_system(int level)
> +{
> + switch (level) {
> +#ifndef _WIN32
> + case G_LOG_LEVEL_ERROR:
> + return LOG_ERR;
> + case G_LOG_LEVEL_CRITICAL:
> + return LOG_CRIT;
> + case G_LOG_LEVEL_WARNING:
> + return LOG_WARNING;
> + case G_LOG_LEVEL_MESSAGE:
> + return LOG_NOTICE;
> + case G_LOG_LEVEL_DEBUG:
> + return LOG_DEBUG;
> + case G_LOG_LEVEL_INFO:
> + default:
> + return LOG_INFO;
> +#else
> + case G_LOG_LEVEL_ERROR:
> + case G_LOG_LEVEL_CRITICAL:
> + return EVENTLOG_ERROR_TYPE;
> + case G_LOG_LEVEL_WARNING:
> + return EVENTLOG_WARNING_TYPE;
> + case G_LOG_LEVEL_MESSAGE:
> + case G_LOG_LEVEL_INFO:
> + case G_LOG_LEVEL_DEBUG:
> + default:
> + return EVENTLOG_INFORMATION_TYPE;
> +#endif
> + }
> +}
> +
> static void ga_log(const gchar *domain, GLogLevelFlags level,
> const gchar *msg, gpointer opaque)
> {
> @@ -327,9 +359,9 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
> level &= G_LOG_LEVEL_MASK;
> if (g_strcmp0(domain, "syslog") == 0) {
> #ifndef _WIN32
> - syslog(LOG_INFO, "%s: %s", level_str, msg);
> + syslog(glib_log_level_to_system(level), "%s: %s", level_str, msg);
> #else
> - ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
> + ReportEvent(s->event_log, glib_log_level_to_system(level),
> 0, 1, NULL, 1, 0, &msg, NULL);
> #endif
> } else if (level & s->log_level) {
> --
> 2.38.1
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] qga-win: add logging to Windows event log
2022-11-28 22:36 ` M M
@ 2022-11-29 11:15 ` Andrey Drobyshev
0 siblings, 0 replies; 8+ messages in thread
From: Andrey Drobyshev @ 2022-11-29 11:15 UTC (permalink / raw)
To: M M; +Cc: qemu-devel, den, yur
On 11/29/22 00:36, M M wrote:
>
>
>> On 28. 11. 2022., at 21:54, Andrey Drobyshev via <qemu-devel@nongnu.org> wrote:
>>
>> This commit allows QGA to write to Windows event log using Win32 API's
>> ReportEvent() [1], much like syslog() under *nix guests.
>>
>> In order to generate log message definitions we use a very basic message
>> text file [2], so that every QGA's message gets ID 1. The tools
>> "windmc" and "windres" respectively are used to generate ".rc" file and
>> COFF object file, and then the COFF file is linked into qemu-ga.exe.
>>
>> [1] https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
>> [2] https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files
>>
>> Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> configure | 3 +++
>> qga/installer/qemu-ga.wxs | 5 +++++
>> qga/main.c | 15 ++++++++++++---
>> qga/meson.build | 19 ++++++++++++++++++-
>> qga/messages-win32.mc | 9 +++++++++
>> 5 files changed, 47 insertions(+), 4 deletions(-)
>> create mode 100644 qga/messages-win32.mc
>>
>> diff --git a/configure b/configure
>> index 26c7bc5154..789a4f6cc9 100755
>> --- a/configure
>> +++ b/configure
>> @@ -372,6 +372,7 @@ smbd="$SMBD"
>> strip="${STRIP-${cross_prefix}strip}"
>> widl="${WIDL-${cross_prefix}widl}"
>> windres="${WINDRES-${cross_prefix}windres}"
>> +windmc="${WINDMC-${cross_prefix}windmc}"
>> pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>> query_pkg_config() {
>> "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
>> @@ -2561,6 +2562,7 @@ if test "$skip_meson" = no; then
>> echo "strip = [$(meson_quote $strip)]" >> $cross
>> echo "widl = [$(meson_quote $widl)]" >> $cross
>> echo "windres = [$(meson_quote $windres)]" >> $cross
>> + echo "windmc = [$(meson_quote $windmc)]" >> $cross
>> if test "$cross_compile" = "yes"; then
>> cross_arg="--cross-file config-meson.cross"
>> echo "[host_machine]" >> $cross
>> @@ -2667,6 +2669,7 @@ preserve_env SMBD
>> preserve_env STRIP
>> preserve_env WIDL
>> preserve_env WINDRES
>> +preserve_env WINDMC
>>
>> printf "exec" >>config.status
>> for i in "$0" "$@"; do
>> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
>> index 73ce2c4965..d9567836f3 100644
>> --- a/qga/installer/qemu-ga.wxs
>> +++ b/qga/installer/qemu-ga.wxs
>> @@ -110,6 +110,11 @@
>> <RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
>> <RegistryValue Type="string" Name="Version" Value="$(var.QEMU_GA_VERSION)" />
>> </RegistryKey>
>> + <RegistryKey Root="HKLM"
>> + Key="System\CurrentControlSet\Services\EventLog\Application\qemu-ga">
>> + <RegistryValue Type="integer" Name="TypesSupported" Value="7" />
>> + <RegistryValue Type="string" Name="EventMessageFile" Value="[qemu_ga_directory]qemu-ga.exe" />
>> + </RegistryKey>
>> </Component>
>> </Directory>
>> </Directory>
>> diff --git a/qga/main.c b/qga/main.c
>> index b3580508fa..10314dfe5d 100644
>> --- a/qga/main.c
>> +++ b/qga/main.c
>> @@ -82,6 +82,7 @@ struct GAState {
>> bool logging_enabled;
>> #ifdef _WIN32
>> GAService service;
>> + HANDLE event_log;
> Just for the sake of uniformity
> shouldn't we close this handle at cleanup_agent() similar to wakeup_event ?
Sure thing! Thanks for your note, will include that in v2.
>
>> HANDLE wakeup_event;
>> #endif
>> bool delimit_response;
>> @@ -324,13 +325,14 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
>> }
>>
>> level &= G_LOG_LEVEL_MASK;
>> -#ifndef _WIN32
>> if (g_strcmp0(domain, "syslog") == 0) {
>> +#ifndef _WIN32
>> syslog(LOG_INFO, "%s: %s", level_str, msg);
>> - } else if (level & s->log_level) {
>> #else
>> - if (level & s->log_level) {
>> + ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
>> + 0, 1, NULL, 1, 0, &msg, NULL);
>> #endif
>> + } else if (level & s->log_level) {
>> g_autoptr(GDateTime) now = g_date_time_new_now_utc();
>> g_autofree char *nowstr = g_date_time_format(now, "%s.%f");
>> fprintf(s->log_file, "%s: %s: %s\n", nowstr, level_str, msg);
>> @@ -1286,6 +1288,13 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
>> g_debug("Guest agent version %s started", QEMU_FULL_VERSION);
>>
>> #ifdef _WIN32
>> + s->event_log = RegisterEventSource(NULL, "qemu-ga");
>> + if (!s->event_log) {
>> + g_autofree gchar *errmsg = g_win32_error_message(GetLastError());
>> + g_critical("unable to register event source: %s", errmsg);
>> + return NULL;
>> + }
>> +
>> /* On win32 the state directory is application specific (be it the default
>> * or a user override). We got past the command line parsing; let's create
>> * the directory (with any intermediate directories). If we run into an
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] qga-win: add logging to Windows event log
2022-11-29 7:37 ` Marc-André Lureau
@ 2022-11-29 12:17 ` Andrey Drobyshev
0 siblings, 0 replies; 8+ messages in thread
From: Andrey Drobyshev @ 2022-11-29 12:17 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, den, yur
On 11/29/22 09:37, Marc-André Lureau wrote:
> Hi
>
> On Tue, Nov 29, 2022 at 12:59 AM Andrey Drobyshev via
> <qemu-devel@nongnu.org> wrote:
>>
>> This commit allows QGA to write to Windows event log using Win32 API's
>> ReportEvent() [1], much like syslog() under *nix guests.
>>
>> In order to generate log message definitions we use a very basic message
>> text file [2], so that every QGA's message gets ID 1. The tools
>> "windmc" and "windres" respectively are used to generate ".rc" file and
>> COFF object file, and then the COFF file is linked into qemu-ga.exe.
>>
>> [1] https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
>> [2] https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files
>>
>
> fyi, some years ago, I tried to add support to GLib:
> https://gitlab.gnome.org/GNOME/glib/-/issues/665. Unfortunately it
> didn't land. And in the meantime, GLib got structured logging. There
> is a long-standing FIXME in GLog to support Event Log. Someone
> probably needs to start from scratch.
>
"Several applications and services running on Windows would really
benefit logging in production, without having to resort writting
similar handlers in each projects." -- very true. Until that bright day
comes -- we're doomed to perform tricks like this...
BTW the message text file (.mc) you suggested looks much more
sophisticated, maybe we can adopt it as well in the future before GLib
gets this feature.
>
>
>> Originally-by: Yuri Pudgorodskiy <yur@virtuozzo.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> configure | 3 +++
>> qga/installer/qemu-ga.wxs | 5 +++++
>> qga/main.c | 15 ++++++++++++---
>> qga/meson.build | 19 ++++++++++++++++++-
>> qga/messages-win32.mc | 9 +++++++++
>> 5 files changed, 47 insertions(+), 4 deletions(-)
>> create mode 100644 qga/messages-win32.mc
>>
>> diff --git a/configure b/configure
>> index 26c7bc5154..789a4f6cc9 100755
>> --- a/configure
>> +++ b/configure
>> @@ -372,6 +372,7 @@ smbd="$SMBD"
>> strip="${STRIP-${cross_prefix}strip}"
>> widl="${WIDL-${cross_prefix}widl}"
>> windres="${WINDRES-${cross_prefix}windres}"
>> +windmc="${WINDMC-${cross_prefix}windmc}"
>> pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>> query_pkg_config() {
>> "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
>> @@ -2561,6 +2562,7 @@ if test "$skip_meson" = no; then
>> echo "strip = [$(meson_quote $strip)]" >> $cross
>> echo "widl = [$(meson_quote $widl)]" >> $cross
>> echo "windres = [$(meson_quote $windres)]" >> $cross
>> + echo "windmc = [$(meson_quote $windmc)]" >> $cross
>> if test "$cross_compile" = "yes"; then
>> cross_arg="--cross-file config-meson.cross"
>> echo "[host_machine]" >> $cross
>> @@ -2667,6 +2669,7 @@ preserve_env SMBD
>> preserve_env STRIP
>> preserve_env WIDL
>> preserve_env WINDRES
>> +preserve_env WINDMC
>>
>> printf "exec" >>config.status
>> for i in "$0" "$@"; do
>> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
>> index 73ce2c4965..d9567836f3 100644
>> --- a/qga/installer/qemu-ga.wxs
>> +++ b/qga/installer/qemu-ga.wxs
>> @@ -110,6 +110,11 @@
>> <RegistryValue Type="string" Name="ProductID" Value="fb0a0d66-c7fb-4e2e-a16b-c4a3bfe8d13b" />
>> <RegistryValue Type="string" Name="Version" Value="$(var.QEMU_GA_VERSION)" />
>> </RegistryKey>
>> + <RegistryKey Root="HKLM"
>> + Key="System\CurrentControlSet\Services\EventLog\Application\qemu-ga">
>> + <RegistryValue Type="integer" Name="TypesSupported" Value="7" />
>> + <RegistryValue Type="string" Name="EventMessageFile" Value="[qemu_ga_directory]qemu-ga.exe" />
>> + </RegistryKey>
>> </Component>
>> </Directory>
>> </Directory>
>> diff --git a/qga/main.c b/qga/main.c
>> index b3580508fa..10314dfe5d 100644
>> --- a/qga/main.c
>> +++ b/qga/main.c
>> @@ -82,6 +82,7 @@ struct GAState {
>> bool logging_enabled;
>> #ifdef _WIN32
>> GAService service;
>> + HANDLE event_log;
>> HANDLE wakeup_event;
>> #endif
>> bool delimit_response;
>> @@ -324,13 +325,14 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
>> }
>>
>> level &= G_LOG_LEVEL_MASK;
>> -#ifndef _WIN32
>> if (g_strcmp0(domain, "syslog") == 0) {
>> +#ifndef _WIN32
>> syslog(LOG_INFO, "%s: %s", level_str, msg);
>> - } else if (level & s->log_level) {
>> #else
>> - if (level & s->log_level) {
>> + ReportEvent(s->event_log, EVENTLOG_INFORMATION_TYPE,
>> + 0, 1, NULL, 1, 0, &msg, NULL);
>
> Interesting. imho we should remove the "syslog" domain, that's not how
> the GLog "domain" is supposed to be used. That's another story..
>
>> #endif
>> + } else if (level & s->log_level) {
>> g_autoptr(GDateTime) now = g_date_time_new_now_utc();
>> g_autofree char *nowstr = g_date_time_format(now, "%s.%f");
>> fprintf(s->log_file, "%s: %s: %s\n", nowstr, level_str, msg);
>> @@ -1286,6 +1288,13 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
>> g_debug("Guest agent version %s started", QEMU_FULL_VERSION);
>>
>> #ifdef _WIN32
>> + s->event_log = RegisterEventSource(NULL, "qemu-ga");
>> + if (!s->event_log) {
>> + g_autofree gchar *errmsg = g_win32_error_message(GetLastError());
>> + g_critical("unable to register event source: %s", errmsg);
>> + return NULL;
>> + }
>> +
>> /* On win32 the state directory is application specific (be it the default
>> * or a user override). We got past the command line parsing; let's create
>> * the directory (with any intermediate directories). If we run into an
>> diff --git a/qga/meson.build b/qga/meson.build
>> index 3cfb9166e5..1ff159edc1 100644
>> --- a/qga/meson.build
>> +++ b/qga/meson.build
>> @@ -98,7 +98,24 @@ if targetos == 'windows'
>> endif
>> endif
>>
>> -qga = executable('qemu-ga', qga_ss.sources(),
>> +qga_objs = []
>> +if targetos == 'windows'
>> + windmc = find_program('windmc', required: true)
>> + windres = find_program('windres', required: true)
>> +
>> + msgrc = custom_target('messages-win32.rc',
>> + input: 'messages-win32.mc',
>> + output: ['messages-win32.rc', 'MSG00409.bin', 'messages-win32.h'],
>> + command: [windmc, '-h', '@OUTDIR@', '-r', '@OUTDIR@', '@INPUT@'])
>> + msgobj = custom_target('messages-win32.o',
>> + input: msgrc[0],
>> + output: 'messages-win32.o',
>> + command: [windres, '-I', '@OUTDIR@', '-o', '@OUTPUT@', '@INPUT@'])
>> +
>> + qga_objs = [msgobj]
>> +endif
>> +
>> +qga = executable('qemu-ga', qga_ss.sources() + qga_objs,
>> link_args: qga_libs,
>> dependencies: [qemuutil, libudev],
>> install: true)
>> diff --git a/qga/messages-win32.mc b/qga/messages-win32.mc
>> new file mode 100644
>> index 0000000000..e21019cebe
>> --- /dev/null
>> +++ b/qga/messages-win32.mc
>> @@ -0,0 +1,9 @@
>> +LanguageNames=(
>> + English=0x409:MSG00409
>> +)
>> +
>> +MessageId=1
>> +SymbolicName=QEMU_GA_EVENTLOG_GENERAL
>> +Language=English
>> +%1
>> +.
>> --
>> 2.38.1
>>
>>
>
> lgtm, with the cleanup requested by Mike, I'll add my r-b
>
Great, thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-29 12:17 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-28 18:54 [PATCH 0/2] qga: improve "syslog" domain logging Andrey Drobyshev via
2022-11-28 18:54 ` [PATCH 1/2] qga-win: add logging to Windows event log Andrey Drobyshev via
2022-11-28 22:36 ` M M
2022-11-29 11:15 ` Andrey Drobyshev
2022-11-29 7:37 ` Marc-André Lureau
2022-11-29 12:17 ` Andrey Drobyshev
2022-11-28 18:54 ` [PATCH 2/2] qga: map GLib log levels to system levels Andrey Drobyshev via
2022-11-29 7:38 ` Marc-André Lureau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.