[PATCH v2 0/2] qga: improve "syslog" domain logging

[PATCH v2 0/2] qga: improve "syslog" domain logging

[Andrey Drobyshev via]

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().

v2:
* Close event_log handle when doing cleanup_agent()
* Fix switch cases indentation as reported by scripts/checkpatch.pl

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                | 50 +++++++++++++++++++++++++++++++++++----
 qga/meson.build           | 19 ++++++++++++++-
 qga/messages-win32.mc     |  9 +++++++
 5 files changed, 81 insertions(+), 5 deletions(-)
 create mode 100644 qga/messages-win32.mc

-- 
2.38.1

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

[Andrey Drobyshev via]

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                | 16 +++++++++++++---
 qga/meson.build           | 19 ++++++++++++++++++-
 qga/messages-win32.mc     |  9 +++++++++
 5 files changed, 48 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..e9f4f44cbb 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -83,6 +83,7 @@ struct GAState {
 #ifdef _WIN32
     GAService service;
     HANDLE wakeup_event;
+    HANDLE event_log;
 #endif
     bool delimit_response;
     bool frozen;
@@ -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
@@ -1377,6 +1386,7 @@ static void cleanup_agent(GAState *s)
 {
 #ifdef _WIN32
     CloseHandle(s->wakeup_event);
+    CloseHandle(s->event_log);
 #endif
     if (s->command_state) {
         ga_command_state_cleanup_all(s->command_state);
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

[PATCH v2 2/2] qga: map GLib log levels to system levels

[Andrey Drobyshev via]

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 9c3c35a423..cf784b279d 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

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

[Marc-André Lureau]

Hi

On Tue, Nov 29, 2022 at 9:37 PM Andrey Drobyshev
<andrey.drobyshev@virtuozzo.com> 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>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  configure                 |  3 +++
>  qga/installer/qemu-ga.wxs |  5 +++++
>  qga/main.c                | 16 +++++++++++++---
>  qga/meson.build           | 19 ++++++++++++++++++-
>  qga/messages-win32.mc     |  9 +++++++++
>  5 files changed, 48 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..e9f4f44cbb 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -83,6 +83,7 @@ struct GAState {
>  #ifdef _WIN32
>      GAService service;
>      HANDLE wakeup_event;
> +    HANDLE event_log;
>  #endif
>      bool delimit_response;
>      bool frozen;
> @@ -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
> @@ -1377,6 +1386,7 @@ static void cleanup_agent(GAState *s)
>  {
>  #ifdef _WIN32
>      CloseHandle(s->wakeup_event);
> +    CloseHandle(s->event_log);
>  #endif
>      if (s->command_state) {
>          ga_command_state_cleanup_all(s->command_state);
> 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
>


-- 
Marc-André Lureau

Re: [PATCH v2 0/2] qga: improve "syslog" domain logging

[Andrey Drobyshev]

On 11/29/22 19:38, Andrey Drobyshev wrote:
> 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().
> 
> v2:
> * Close event_log handle when doing cleanup_agent()
> * Fix switch cases indentation as reported by scripts/checkpatch.pl
> 
> 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                | 50 +++++++++++++++++++++++++++++++++++----
>  qga/meson.build           | 19 ++++++++++++++-
>  qga/messages-win32.mc     |  9 +++++++
>  5 files changed, 81 insertions(+), 5 deletions(-)
>  create mode 100644 qga/messages-win32.mc
> 

Could you please clarify the status of these patches?

Re: [PATCH v2 0/2] qga: improve "syslog" domain logging

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]

Currently, there is a code freeze in QEMU for release 7.2.
I will merge this after it https://wiki.qemu.org/Planning/7.2

Best Regards,
Konstantin Kostiuk.


On Mon, Dec 12, 2022 at 2:17 PM Andrey Drobyshev <
andrey.drobyshev@virtuozzo.com> wrote:

> On 11/29/22 19:38, Andrey Drobyshev wrote:
> > 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().
> >
> > v2:
> > * Close event_log handle when doing cleanup_agent()
> > * Fix switch cases indentation as reported by scripts/checkpatch.pl
> >
> > 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                | 50 +++++++++++++++++++++++++++++++++++----
> >  qga/meson.build           | 19 ++++++++++++++-
> >  qga/messages-win32.mc     |  9 +++++++
> >  5 files changed, 81 insertions(+), 5 deletions(-)
> >  create mode 100644 qga/messages-win32.mc
> >
>
> Could you please clarify the status of these patches?
>
>

[-- Attachment #2: Type: text/html, Size: 2135 bytes --]

Re: [PATCH v2 0/2] qga: improve "syslog" domain logging

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Tested-by: Konstantin Kostiuk <kkostiuk@redhat.com>


Hi Andrey,
Do you expect Event Viewer to be empty by default in the current
implementation?
Currently, all logs that we write in the guest agent do not have "syslog"
domain
so we will have logs only in file.

On Mon, Dec 12, 2022 at 2:29 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> Currently, there is a code freeze in QEMU for release 7.2.
> I will merge this after it https://wiki.qemu.org/Planning/7.2
>
> Best Regards,
> Konstantin Kostiuk.
>
>
> On Mon, Dec 12, 2022 at 2:17 PM Andrey Drobyshev <
> andrey.drobyshev@virtuozzo.com> wrote:
>
>> On 11/29/22 19:38, Andrey Drobyshev wrote:
>> > 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().
>> >
>> > v2:
>> > * Close event_log handle when doing cleanup_agent()
>> > * Fix switch cases indentation as reported by scripts/checkpatch.pl
>> >
>> > 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                | 50 +++++++++++++++++++++++++++++++++++----
>> >  qga/meson.build           | 19 ++++++++++++++-
>> >  qga/messages-win32.mc     |  9 +++++++
>> >  5 files changed, 81 insertions(+), 5 deletions(-)
>> >  create mode 100644 qga/messages-win32.mc
>> >
>>
>> Could you please clarify the status of these patches?
>>
>>

[-- Attachment #2: Type: text/html, Size: 3047 bytes --]

Re: [PATCH v2 0/2] qga: improve "syslog" domain logging

[Andrey Drobyshev]

On 12/16/22 12:01, Konstantin Kostiuk wrote:
> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com
> <mailto:kkostiuk@redhat.com>>
> Tested-by: Konstantin Kostiuk <kkostiuk@redhat.com
> <mailto:kkostiuk@redhat.com>>
> 
> 
> Hi Andrey,
> Do you expect Event Viewer to be empty by default in the current
> implementation?
> Currently, all logs that we write in the guest agent do not have
> "syslog" domain
> so we will have logs only in file.
> 
> [...]

Do they not really?  For instance, I see quite a few such calls in the
implementation of guest commands:

# grep 'slog(' qga/commands-win32.c | wc -l
21

They just need to be triggered.  For instance, if I build QGA with these
patches applied, run it in a VM make a fsfreeze call:

# virsh domfsfreeze win2k16-qga-win
Froze 2 filesystem(s)

then I see the message "guest-fsfreeze called" in the event log.

Re: [PATCH v2 0/2] qga: improve "syslog" domain logging

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

Thanks.

You are right. I tested another command and don't see logs in the event
viewer.

PR was sent.

On Fri, Dec 16, 2022 at 5:27 PM Andrey Drobyshev <
andrey.drobyshev@virtuozzo.com> wrote:

> On 12/16/22 12:01, Konstantin Kostiuk wrote:
> > Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com
> > <mailto:kkostiuk@redhat.com>>
> > Tested-by: Konstantin Kostiuk <kkostiuk@redhat.com
> > <mailto:kkostiuk@redhat.com>>
> >
> >
> > Hi Andrey,
> > Do you expect Event Viewer to be empty by default in the current
> > implementation?
> > Currently, all logs that we write in the guest agent do not have
> > "syslog" domain
> > so we will have logs only in file.
> >
> > [...]
>
> Do they not really?  For instance, I see quite a few such calls in the
> implementation of guest commands:
>
> # grep 'slog(' qga/commands-win32.c | wc -l
> 21
>
> They just need to be triggered.  For instance, if I build QGA with these
> patches applied, run it in a VM make a fsfreeze call:
>
> # virsh domfsfreeze win2k16-qga-win
> Froze 2 filesystem(s)
>
> then I see the message "guest-fsfreeze called" in the event log.
>
>

[-- Attachment #2: Type: text/html, Size: 1892 bytes --]

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

[Stefan Weil via]

Hi,

cross builds fail with this code. Please see details below.

Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:
> 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                | 16 +++++++++++++---
>   qga/meson.build           | 19 ++++++++++++++++++-
>   qga/messages-win32.mc     |  9 +++++++++
>   5 files changed, 48 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}"

Here the needed cross prefix is added ...

>   pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>   query_pkg_config() {
>       "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
[...]
> 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)

... but here the cross prefix is missing and the cross build aborts 
because windmc does not exist.

Regards
Stefan

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

[Andrey Drobyshev]

Hi Stefan,

On 1/23/23 19:28, Stefan Weil wrote:
> Hi,
> 
> cross builds fail with this code. Please see details below.
> 
> Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:
>> 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                | 16 +++++++++++++---
>>   qga/meson.build           | 19 ++++++++++++++++++-
>>   qga/messages-win32.mc     |  9 +++++++++
>>   5 files changed, 48 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}"
> 
> Here the needed cross prefix is added ...
> 
>>   pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>>   query_pkg_config() {
>>       "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
> [...]
>> 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)
> 
> ... but here the cross prefix is missing and the cross build aborts
> because windmc does not exist.

There's no need for the cross prefix here.  After you've run ./configure
with --cross-prefix, argument, you'll see the following in
build/config-meson.cross file:

[binaries]
....
widl = ['x86_64-w64-mingw32-widl']
windres = ['x86_64-w64-mingw32-windres']
windmc = ['x86_64-w64-mingw32-windmc']

And these are the actual values meson's find_program() is going to be
looking for.  So it doesn't seem like there's anything broken here, it's
a matter of build requirements.

> 
> Regards
> Stefan

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

[Denis V. Lunev]

Hi Stefan!

On 1/23/23 20:38, Andrey Drobyshev wrote:
> Hi Stefan,
>
> On 1/23/23 19:28, Stefan Weil wrote:
>> Hi,
>>
>> cross builds fail with this code. Please see details below.
>>
>> Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:
>>> 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                | 16 +++++++++++++---
>>>    qga/meson.build           | 19 ++++++++++++++++++-
>>>    qga/messages-win32.mc     |  9 +++++++++
>>>    5 files changed, 48 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}"
>> Here the needed cross prefix is added ...
>>
>>>    pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>>>    query_pkg_config() {
>>>        "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
>> [...]
>>> 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)
>> ... but here the cross prefix is missing and the cross build aborts
>> because windmc does not exist.
> There's no need for the cross prefix here.  After you've run ./configure
> with --cross-prefix, argument, you'll see the following in
> build/config-meson.cross file:
>
> [binaries]
> ....
> widl = ['x86_64-w64-mingw32-widl']
> windres = ['x86_64-w64-mingw32-windres']
> windmc = ['x86_64-w64-mingw32-windmc']
>
> And these are the actual values meson's find_program() is going to be
> looking for.  So it doesn't seem like there's anything broken here, it's
> a matter of build requirements.


may be if you will provide Andrey with a configure command line
we will be able to make a fix. It seems that something is
definitely untold :)

Regards,
     Den

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

[Stefan Weil via]

Am 23.01.23 um 20:38 schrieb Andrey Drobyshev:

> Hi Stefan,
>
> On 1/23/23 19:28, Stefan Weil wrote:
>> Hi,
>>
>> cross builds fail with this code. Please see details below.
>>
>> Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:
>>> 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                | 16 +++++++++++++---
>>>    qga/meson.build           | 19 ++++++++++++++++++-
>>>    qga/messages-win32.mc     |  9 +++++++++
>>>    5 files changed, 48 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}"
>> Here the needed cross prefix is added ...
>>
>>>    pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>>>    query_pkg_config() {
>>>        "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
>> [...]
>>> 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)
>> ... but here the cross prefix is missing and the cross build aborts
>> because windmc does not exist.
> There's no need for the cross prefix here.  After you've run ./configure
> with --cross-prefix, argument, you'll see the following in
> build/config-meson.cross file:
>
> [binaries]
> ....
> widl = ['x86_64-w64-mingw32-widl']
> windres = ['x86_64-w64-mingw32-windres']
> windmc = ['x86_64-w64-mingw32-windmc']
>
> And these are the actual values meson's find_program() is going to be
> looking for.  So it doesn't seem like there's anything broken here, it's
> a matter of build requirements.


My configure terminates with an error because of the missing windmc 
before it has written config-meson.cross. I have run an incremental build:

Program windmc found: NO

../../../qga/meson.build:103:2: ERROR: Program 'windmc' not found or not 
executable

A full log can be found at 
/qemu/bin/debug/x86_64-w64-mingw32/meson-logs/meson-log.txt
ninja: error: rebuilding 'build.ninja': subcommand failed
FAILED: build.ninja
/usr/bin/python3 /qemu/meson/meson.py --internal regenerate /qemu 
/home/stefan/src/gitlab/qemu-project/qemu/bin/debug/x86_64-w64-mingw32 
--backend ninja
make: *** [Makefile:165: run-ninja] Fehler 1
make: Verzeichnis „/qemu/bin/debug/x86_64-w64-mingw32“ wird verlassen

A clean fresh build works indeed fine.

Stefan

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

[Andrey Drobyshev]

On 1/23/23 22:52, Stefan Weil wrote:
> Am 23.01.23 um 20:38 schrieb Andrey Drobyshev:
> 
>> Hi Stefan,
>>
>> On 1/23/23 19:28, Stefan Weil wrote:
>>> Hi,
>>>
>>> cross builds fail with this code. Please see details below.
>>>
>>> Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:
>>>> 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                | 16 +++++++++++++---
>>>>    qga/meson.build           | 19 ++++++++++++++++++-
>>>>    qga/messages-win32.mc     |  9 +++++++++
>>>>    5 files changed, 48 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}"
>>> Here the needed cross prefix is added ...
>>>
>>>>    pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
>>>>    query_pkg_config() {
>>>>        "${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
>>> [...]
>>>> 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)
>>> ... but here the cross prefix is missing and the cross build aborts
>>> because windmc does not exist.
>> There's no need for the cross prefix here.  After you've run ./configure
>> with --cross-prefix, argument, you'll see the following in
>> build/config-meson.cross file:
>>
>> [binaries]
>> ....
>> widl = ['x86_64-w64-mingw32-widl']
>> windres = ['x86_64-w64-mingw32-windres']
>> windmc = ['x86_64-w64-mingw32-windmc']
>>
>> And these are the actual values meson's find_program() is going to be
>> looking for.  So it doesn't seem like there's anything broken here, it's
>> a matter of build requirements.
> 
> 
> My configure terminates with an error because of the missing windmc
> before it has written config-meson.cross. I have run an incremental build:
> 
> Program windmc found: NO
> 
> ../../../qga/meson.build:103:2: ERROR: Program 'windmc' not found or not
> executable
> 
> A full log can be found at
> /qemu/bin/debug/x86_64-w64-mingw32/meson-logs/meson-log.txt
> ninja: error: rebuilding 'build.ninja': subcommand failed
> FAILED: build.ninja
> /usr/bin/python3 /qemu/meson/meson.py --internal regenerate /qemu
> /home/stefan/src/gitlab/qemu-project/qemu/bin/debug/x86_64-w64-mingw32
> --backend ninja
> make: *** [Makefile:165: run-ninja] Fehler 1
> make: Verzeichnis „/qemu/bin/debug/x86_64-w64-mingw32“ wird verlassen
> 
> A clean fresh build works indeed fine.
> 

I don't think the issue is caused by this particular patch.
Here's what's happening:

Makefile:72
> # force a rerun of configure if config-host.mak is too old or corrupted         
> ifeq ($(MESON),)                                                                
> .PHONY: config-host.mak                                                         
> x := $(shell rm -rf meson-private meson-info meson-logs)                                                                                                       
> endif


Makefile:92
> # 1. ensure config-host.mak is up-to-date                                                                                                                      
> config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/scripts/meson-buildoptions.sh $(SRC_PATH)/VERSION
>     @echo config-host.mak is out-of-date, running configure                     
>     @if test -f meson-private/coredata.dat; then \                              
>       ./config.status --skip-meson; \                                           
>     else \                                                                      
>       ./config.status && touch build.ninja.stamp; \                             
>     fi

configure:2529
> if test "$skip_meson" = no; then
> ...
>   echo "widl = [$(meson_quote $widl)]" >> $cross                                
>   echo "windres = [$(meson_quote $windres)]" >> $cross                          
>   echo "windmc = [$(meson_quote $windmc)]" >> $cross

Now if you checkout a revision which doesn't have this patch, perform a
build, then checkout to a revision which has this patch and try to
perform a build again, ./configure will be called with --skip-meson and
config-meson.cross won't be regenerated.

I'm not sure how we could detect a necessity to regenerate
config-meson.cross in this case.  Your suggestions are welcome, if any.