1. Patchew
  2. QEMU
  3. View series

[PATCH] util: win32: Write hex value when can't get error message

Kostiantyn Kostiuk posted 1 patch 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250717145948.77870-1-kkostiuk@redhat.com
Maintainers: Markus Armbruster <armbru@redhat.com>
util/error.c | 5 +++++
1 file changed, 5 insertions(+)
[PATCH] util: win32: Write hex value when can't get error message
Posted by Kostiantyn Kostiuk 4 months ago 
g_win32_error_message - translate a Win32 error code
(as returned by GetLastError()) into the corresponding message.

In the same time, we call error_setg_win32_internal with
error codes from different Windows componets like VSS or
Performance monitor that provides different codes and
can't be converted with g_win32_error_message. In this
case, the empty suffix will be returned so error will be
masked.

QGA error example:
 - before changes:
  {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: "}}
 - after changes:
  {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: unknown Windows error 0x8004230e"}}

Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
---
 util/error.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/util/error.c b/util/error.c
index daea2142f3..b1342558ae 100644
--- a/util/error.c
+++ b/util/error.c
@@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
 
     if (win32_err != 0) {
         suffix = g_win32_error_message(win32_err);
+        // g_win32_error_message() failed
+        if (!suffix[0]) {
+            g_free(suffix);
+            suffix = g_strdup_printf("unknown Windows error 0x%x", win32_err);
+        }
     }
 
     va_start(ap, fmt);
-- 
2.48.1
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Markus Armbruster 3 months, 4 weeks ago 
Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:

> g_win32_error_message - translate a Win32 error code
> (as returned by GetLastError()) into the corresponding message.
>
> In the same time, we call error_setg_win32_internal with
> error codes from different Windows componets like VSS or
> Performance monitor that provides different codes and
> can't be converted with g_win32_error_message.

Are these error codes from GetLastError()?

>                                                In this
> case, the empty suffix will be returned so error will be
> masked.
>
> QGA error example:
>  - before changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: "}}
>  - after changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: unknown Windows error 0x8004230e"}}

Exact reproducer?

> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> ---
>  util/error.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/util/error.c b/util/error.c
> index daea2142f3..b1342558ae 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
>  
>      if (win32_err != 0) {
>          suffix = g_win32_error_message(win32_err);
> +        // g_win32_error_message() failed
> +        if (!suffix[0]) {
> +            g_free(suffix);
> +            suffix = g_strdup_printf("unknown Windows error 0x%x", win32_err);
> +        }
>      }
>  
>      va_start(ap, fmt);
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Kostiantyn Kostiuk 3 months, 3 weeks ago 
On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com> wrote:

> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>
> > g_win32_error_message - translate a Win32 error code
> > (as returned by GetLastError()) into the corresponding message.
> >
> > In the same time, we call error_setg_win32_internal with
> > error codes from different Windows componets like VSS or
> > Performance monitor that provides different codes and
> > can't be converted with g_win32_error_message.
>
> Are these error codes from GetLastError()?
>

No.
VSS functions directly return an error code.
Section: Return value -
https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Performance Counters API can return a system error code or a PDH error code.
Section: Return value -
https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
System error code = GetLastError, PDH error code, something else.

https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.


> >                                                In this
> > case, the empty suffix will be returned so error will be
> > masked.
> >
> > QGA error example:
> >  - before changes:
> >   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to
> snapshot set: "}}
> >  - after changes:
> >   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to
> snapshot set: unknown Windows error 0x8004230e"}}
>
> Exact reproducer?
>

Yes, for example, with VSS.
Execute: {"execute": "guest-fsfreeze-freeze-list", "arguments":
{"mountpoints": ["C:"]}}
After this commit, you will get {"error": {"class": "GenericError", "desc":
"failed to add C: to snapshot set: unknown Windows error 0x80042308"}}


>
> > Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> > ---
> >  util/error.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/util/error.c b/util/error.c
> > index daea2142f3..b1342558ae 100644
> > --- a/util/error.c
> > +++ b/util/error.c
> > @@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
> >
> >      if (win32_err != 0) {
> >          suffix = g_win32_error_message(win32_err);
> > +        // g_win32_error_message() failed
> > +        if (!suffix[0]) {
> > +            g_free(suffix);
> > +            suffix = g_strdup_printf("unknown Windows error 0x%x",
> win32_err);
> > +        }
> >      }
> >
> >      va_start(ap, fmt);
>
>
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Markus Armbruster 3 months, 3 weeks ago 
Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:

> On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>>
>> > g_win32_error_message - translate a Win32 error code
>> > (as returned by GetLastError()) into the corresponding message.
>> >
>> > In the same time, we call error_setg_win32_internal with
>> > error codes from different Windows componets like VSS or
>> > Performance monitor that provides different codes and
>> > can't be converted with g_win32_error_message.
>>
>> Are these error codes from GetLastError()?
>>
>
> No.
> VSS functions directly return an error code.
> Section: Return value -
> https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
>
> Performance Counters API can return a system error code or a PDH error code.
> Section: Return value -
> https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
> System error code = GetLastError, PDH error code, something else.
>
> https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
> FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.

The error code error_setg_win32() takes is passed to
g_win32_error_message().  Contract:

    g_win32_error_message ()

    gchar *
    g_win32_error_message (gint error);

    Translate a Win32 error code (as returned by GetLastError() or
    WSAGetLastError()) into the corresponding message.  The message is
    either language neutral, or in the thread's language, or the user's
    language, the system's language, or US English (see docs for
    FormatMessage()).  The returned string is in UTF-8.  It should be
    deallocated with g_free().

    Parameters

        error error code.

    Returns

        newly-allocated error message

https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-Functions.php#g-win32-error-message

Passing error codes from sources other than GetLastError() or
WSAGetLastError() violates this contract.

Apparently, g_win32_error_message() returns NULL then.  This is not
documented behavior.

Your fix relies on this undocumented behavior.

I believe we should instead fix the misuses of error_setg_win32().

[...]
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Kostiantyn Kostiuk 3 months, 3 weeks ago 
On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com>
wrote:

> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>
> > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
> >>
> >> > g_win32_error_message - translate a Win32 error code
> >> > (as returned by GetLastError()) into the corresponding message.
> >> >
> >> > In the same time, we call error_setg_win32_internal with
> >> > error codes from different Windows componets like VSS or
> >> > Performance monitor that provides different codes and
> >> > can't be converted with g_win32_error_message.
> >>
> >> Are these error codes from GetLastError()?
> >>
> >
> > No.
> > VSS functions directly return an error code.
> > Section: Return value -
> >
> https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
> >
> > Performance Counters API can return a system error code or a PDH error
> code.
> > Section: Return value -
> >
> https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
> > System error code = GetLastError, PDH error code, something else.
> >
> > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
> > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
>
> The error code error_setg_win32() takes is passed to
> g_win32_error_message().  Contract:
>
>     g_win32_error_message ()
>
>     gchar *
>     g_win32_error_message (gint error);
>
>     Translate a Win32 error code (as returned by GetLastError() or
>     WSAGetLastError()) into the corresponding message.  The message is
>     either language neutral, or in the thread's language, or the user's
>     language, the system's language, or US English (see docs for
>     FormatMessage()).  The returned string is in UTF-8.  It should be
>     deallocated with g_free().
>
>     Parameters
>
>         error error code.
>
>     Returns
>
>         newly-allocated error message
>
>
> https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-Functions.php#g-win32-error-message
>
> Passing error codes from sources other than GetLastError() or
> WSAGetLastError() violates this contract.
>
> Apparently, g_win32_error_message() returns NULL then.  This is not
> documented behavior.
>

It returns an empty string, not NULL.
https://gitlab.gnome.org/GNOME/glib/-/blob/a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216


> Your fix relies on this undocumented behavior.
>

As for me, this behaviour is almost expected (I expected NULL instead of an
empty string) because
g_win32_error_message uses FormatMessage, and FormatMessage returns NULL if
an unknown error code
is provided. And I know this, because FormatMessage is the only way on
Windows to get a human-readable
string from the error code.


>
> I believe we should instead fix the misuses of error_setg_win32().
>

This will be more complicated.
1. I don't know what code was returned by the Performance Counters API
(system or PDH)
2. QGA call error_setg_win32_internal as part of error propagation with
different error codes,
 fix the misuses in this case means have a different error propagator for
different error codes (back to 1)
3. Also, this means that I need to reimplement error_setg_win32_internal
for non-system errors with only
one difference: "unknown Windows error 0x%x" instead of
g_win32_error_message, because anyway
I need the full error propagation part.


> [...]
>
>
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Philippe Mathieu-Daudé 3 months, 3 weeks ago 
On 21/7/25 12:12, Kostiantyn Kostiuk wrote:
> 
> 
> On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com 
> <mailto:armbru@redhat.com>> wrote:
> 
>     Kostiantyn Kostiuk <kkostiuk@redhat.com
>     <mailto:kkostiuk@redhat.com>> writes:
> 
>      > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster
>     <armbru@redhat.com <mailto:armbru@redhat.com>> wrote:
>      >
>      >> Kostiantyn Kostiuk <kkostiuk@redhat.com
>     <mailto:kkostiuk@redhat.com>> writes:
>      >>
>      >> > g_win32_error_message - translate a Win32 error code
>      >> > (as returned by GetLastError()) into the corresponding message.
>      >> >
>      >> > In the same time, we call error_setg_win32_internal with
>      >> > error codes from different Windows componets like VSS or
>      >> > Performance monitor that provides different codes and
>      >> > can't be converted with g_win32_error_message.
>      >>
>      >> Are these error codes from GetLastError()?
>      >>
>      >
>      > No.
>      > VSS functions directly return an error code.
>      > Section: Return value -
>      > https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-
>     vsbackup-ivssbackupcomponents-addtosnapshotset <https://
>     learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-
>     ivssbackupcomponents-addtosnapshotset>
>      >
>      > Performance Counters API can return a system error code or a PDH
>     error code.
>      > Section: Return value -
>      > https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-
>     pdhopenqueryw <https://learn.microsoft.com/en-us/windows/win32/api/
>     pdh/nf-pdh-pdhopenqueryw>
>      > System error code = GetLastError, PDH error code, something else.
>      >
>      > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-
>     error-codes <https://learn.microsoft.com/en-us/windows/win32/
>     perfctrs/pdh-error-codes>
>      > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
> 
>     The error code error_setg_win32() takes is passed to
>     g_win32_error_message().  Contract:
> 
>          g_win32_error_message ()
> 
>          gchar *
>          g_win32_error_message (gint error);
> 
>          Translate a Win32 error code (as returned by GetLastError() or
>          WSAGetLastError()) into the corresponding message.  The message is
>          either language neutral, or in the thread's language, or the user's
>          language, the system's language, or US English (see docs for
>          FormatMessage()).  The returned string is in UTF-8.  It should be
>          deallocated with g_free().
> 
>          Parameters
> 
>              error error code.
> 
>          Returns
> 
>              newly-allocated error message
> 
>     https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-
>     Compatibility-Functions.php#g-win32-error-message <https://
>     www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-
>     Functions.php#g-win32-error-message>
> 
>     Passing error codes from sources other than GetLastError() or
>     WSAGetLastError() violates this contract.
> 
>     Apparently, g_win32_error_message() returns NULL then.  This is not
>     documented behavior.
> 
> 
> It returns an empty string, not NULL.
> https://gitlab.gnome.org/GNOME/glib/-/blob/ 
> a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216 <https:// 
> gitlab.gnome.org/GNOME/glib/-/blob/ 
> a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216>

Worth reporting to GLib IMHO.
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Kostiantyn Kostiuk 3 months, 3 weeks ago 
Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740
PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716

@Markus Armbruster <armbru@redhat.com>
Based on the documentation from this PR, do you have any other objections
to this patch?

Best Regards,
Konstantin Kostiuk.


On Mon, Jul 21, 2025 at 2:53 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 21/7/25 12:12, Kostiantyn Kostiuk wrote:
> >
> >
> > On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com
> > <mailto:armbru@redhat.com>> wrote:
> >
> >     Kostiantyn Kostiuk <kkostiuk@redhat.com
> >     <mailto:kkostiuk@redhat.com>> writes:
> >
> >      > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster
> >     <armbru@redhat.com <mailto:armbru@redhat.com>> wrote:
> >      >
> >      >> Kostiantyn Kostiuk <kkostiuk@redhat.com
> >     <mailto:kkostiuk@redhat.com>> writes:
> >      >>
> >      >> > g_win32_error_message - translate a Win32 error code
> >      >> > (as returned by GetLastError()) into the corresponding message.
> >      >> >
> >      >> > In the same time, we call error_setg_win32_internal with
> >      >> > error codes from different Windows componets like VSS or
> >      >> > Performance monitor that provides different codes and
> >      >> > can't be converted with g_win32_error_message.
> >      >>
> >      >> Are these error codes from GetLastError()?
> >      >>
> >      >
> >      > No.
> >      > VSS functions directly return an error code.
> >      > Section: Return value -
> >      > https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-
> >     vsbackup-ivssbackupcomponents-addtosnapshotset <https://
> >     learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-
> >     ivssbackupcomponents-addtosnapshotset>
> >      >
> >      > Performance Counters API can return a system error code or a PDH
> >     error code.
> >      > Section: Return value -
> >      > https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-
> >     pdhopenqueryw <https://learn.microsoft.com/en-us/windows/win32/api/
> >     pdh/nf-pdh-pdhopenqueryw>
> >      > System error code = GetLastError, PDH error code, something else.
> >      >
> >      > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-
> >     error-codes <https://learn.microsoft.com/en-us/windows/win32/
> >     perfctrs/pdh-error-codes>
> >      > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
> >
> >     The error code error_setg_win32() takes is passed to
> >     g_win32_error_message().  Contract:
> >
> >          g_win32_error_message ()
> >
> >          gchar *
> >          g_win32_error_message (gint error);
> >
> >          Translate a Win32 error code (as returned by GetLastError() or
> >          WSAGetLastError()) into the corresponding message.  The message
> is
> >          either language neutral, or in the thread's language, or the
> user's
> >          language, the system's language, or US English (see docs for
> >          FormatMessage()).  The returned string is in UTF-8.  It should
> be
> >          deallocated with g_free().
> >
> >          Parameters
> >
> >              error error code.
> >
> >          Returns
> >
> >              newly-allocated error message
> >
> >     https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-
> >     Compatibility-Functions.php#g-win32-error-message <https://
> >     www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-
> >     Functions.php#g-win32-error-message>
> >
> >     Passing error codes from sources other than GetLastError() or
> >     WSAGetLastError() violates this contract.
> >
> >     Apparently, g_win32_error_message() returns NULL then.  This is not
> >     documented behavior.
> >
> >
> > It returns an empty string, not NULL.
> > https://gitlab.gnome.org/GNOME/glib/-/blob/
> > a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216 <https://
> > gitlab.gnome.org/GNOME/glib/-/blob/
> > a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216>
>
> Worth reporting to GLib IMHO.
>
>
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Markus Armbruster 3 months, 2 weeks ago 
Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:

> Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740
> PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716
>
> @Markus Armbruster <armbru@redhat.com>
> Based on the documentation from this PR, do you have any other objections
> to this patch?

I can't see that the commit invalidates my objection.

The revised contract still specifies the error code comes from
GetLastError() or WSAGetLastError().  Passing anything else still
violates it.

What can go wrong when we pass some other integer?

Say we pass EINVAL.  It's 22 on Linux.  Interpreted as Windows system
error code (the thing GetLastError() returns), that's ERROR_BAD_COMMAND,
documented as "The device does not recognize the command."[*].
g_win32_error_message() and thus error_setg_win32() will report
confusing nonsense.

Another common integer error code is -1.  This isn't a valid Windows
system error code, so we can expect to hit the "unknown Windows error"
branch.

Perhaps the code is so confused about error codes that passing them to
an appropriate function is impractical, and taking our chances with
g_win32_error_message() is the best we can do.  Wouldn't exactly inspire
confidence in the soundness of QEMU's Windows code.




[*] https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499-
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Philippe Mathieu-Daudé 3 months, 2 weeks ago 
On 25/7/25 11:59, Kostiantyn Kostiuk wrote:
> Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740 
> <https://gitlab.gnome.org/GNOME/glib/-/issues/3740>
> PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716 
> <https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716>

Even already merged, thank you!
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Kostiantyn Kostiuk 3 months, 2 weeks ago 
Based on the following part from the new docs
* If a human readable message cannot be found for the given @error, an empty
* string is returned.
Do you have any objections to this patch?

Best Regards,
Konstantin Kostiuk.


On Mon, Jul 28, 2025 at 12:48 PM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 25/7/25 11:59, Kostiantyn Kostiuk wrote:
> > Issue reported to GLib https://gitlab.gnome.org/GNOME/glib/-/issues/3740
> > <https://gitlab.gnome.org/GNOME/glib/-/issues/3740>
> > PR with fix https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716
> > <https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4716>
>
> Even already merged, thank you!
>
>
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Yan Vugenfirer 3 months, 3 weeks ago 
On Mon, Jul 21, 2025 at 1:12 PM Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
>
>
> On Mon, Jul 21, 2025 at 12:22 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>>
>> > On Sat, Jul 19, 2025 at 9:27 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> Kostiantyn Kostiuk <kkostiuk@redhat.com> writes:
>> >>
>> >> > g_win32_error_message - translate a Win32 error code
>> >> > (as returned by GetLastError()) into the corresponding message.
>> >> >
>> >> > In the same time, we call error_setg_win32_internal with
>> >> > error codes from different Windows componets like VSS or
>> >> > Performance monitor that provides different codes and
>> >> > can't be converted with g_win32_error_message.
>> >>
>> >> Are these error codes from GetLastError()?
>> >>
>> >
>> > No.
>> > VSS functions directly return an error code.
>> > Section: Return value -
>> > https://learn.microsoft.com/en-us/windows/win32/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
>> >
>> > Performance Counters API can return a system error code or a PDH error code.
>> > Section: Return value -
>> > https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhopenqueryw
>> > System error code = GetLastError, PDH error code, something else.
>> >
>> > https://learn.microsoft.com/en-us/windows/win32/perfctrs/pdh-error-codes
>> > FormatMessage requires LoadLibrary(L"pdh.dll") to work properly.
>>
>> The error code error_setg_win32() takes is passed to
>> g_win32_error_message().  Contract:
>>
>>     g_win32_error_message ()
>>
>>     gchar *
>>     g_win32_error_message (gint error);
>>
>>     Translate a Win32 error code (as returned by GetLastError() or
>>     WSAGetLastError()) into the corresponding message.  The message is
>>     either language neutral, or in the thread's language, or the user's
>>     language, the system's language, or US English (see docs for
>>     FormatMessage()).  The returned string is in UTF-8.  It should be
>>     deallocated with g_free().
>>
>>     Parameters
>>
>>         error error code.
>>
>>     Returns
>>
>>         newly-allocated error message
>>
>> https://www.manpagez.com/html/glib/glib-2.46.0/glib-Windows-Compatibility-Functions.php#g-win32-error-message
>>
>> Passing error codes from sources other than GetLastError() or
>> WSAGetLastError() violates this contract.
>>
>> Apparently, g_win32_error_message() returns NULL then.  This is not
>> documented behavior.

If glib wrapper behaviour is undocumented, let's not use it in Windows
related code and use Win32 API directly.
We can also fix the documentation.

Also, if we take a look at
https://learn.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror
then it looks like GetLastError might return error codes that don't
have strings in the system, again this is exactly the case were we
want the numeric value to be propagated and not silently dropped.

Best regards,
Yan.

>
>
> It returns an empty string, not NULL.
> https://gitlab.gnome.org/GNOME/glib/-/blob/a205d01adc3fbc4f243e0b0fb52a3a0a8a0d9ae7/glib/gwin32.c#L216
>
>>
>> Your fix relies on this undocumented behavior.
>
>
> As for me, this behaviour is almost expected (I expected NULL instead of an empty string) because
> g_win32_error_message uses FormatMessage, and FormatMessage returns NULL if an unknown error code
> is provided. And I know this, because FormatMessage is the only way on Windows to get a human-readable
> string from the error code.
>
>>
>>
>> I believe we should instead fix the misuses of error_setg_win32().
>
>
> This will be more complicated.
> 1. I don't know what code was returned by the Performance Counters API (system or PDH)
> 2. QGA call error_setg_win32_internal as part of error propagation with different error codes,
>  fix the misuses in this case means have a different error propagator for different error codes (back to 1)
> 3. Also, this means that I need to reimplement error_setg_win32_internal for non-system errors with only
> one difference: "unknown Windows error 0x%x" instead of g_win32_error_message, because anyway
> I need the full error propagation part.
>
>>
>> [...]
>>
Re: [PATCH] util: win32: Write hex value when can't get error message
Posted by Yan Vugenfirer 4 months ago 
On Thu, Jul 17, 2025 at 5:59 PM Kostiantyn Kostiuk <kkostiuk@redhat.com> wrote:
>
> g_win32_error_message - translate a Win32 error code
> (as returned by GetLastError()) into the corresponding message.
>
> In the same time, we call error_setg_win32_internal with
> error codes from different Windows componets like VSS or
> Performance monitor that provides different codes and
> can't be converted with g_win32_error_message. In this
> case, the empty suffix will be returned so error will be
> masked.
>
> QGA error example:
>  - before changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: "}}
>  - after changes:
>   {"error": {"class": "GenericError", "desc": "failed to add D:\\ to snapshot set: unknown Windows error 0x8004230e"}}
>
> Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
> ---
>  util/error.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/util/error.c b/util/error.c
> index daea2142f3..b1342558ae 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -188,6 +188,11 @@ void error_setg_win32_internal(Error **errp,
>
>      if (win32_err != 0) {
>          suffix = g_win32_error_message(win32_err);
> +        // g_win32_error_message() failed
> +        if (!suffix[0]) {
> +            g_free(suffix);
> +            suffix = g_strdup_printf("unknown Windows error 0x%x", win32_err);
> +        }
>      }
>
>      va_start(ap, fmt);
> --
> 2.48.1
>

Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.