Re: [PATCH 1/2] qga-win: add logging to Windows event log

[M M]

> 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