[PULL for-6.1 0/6] qemu-ga patch queue for hard-freeze

[PULL for-6.1 0/6] qemu-ga patch queue for hard-freeze

[Michael Roth]

Hi Peter,

Sorry for the late submission. These patches affect only the w32 build of
qemu-ga. A number of these patches I've had queued for some time, but a bug
in the MSI installer that was just fixed was blocking testing. Now that that
is working again I am hoping to get these in along with a couple of other
fixes that have come in since then.

The following changes since commit 7f1cab9c628a798ae2607940993771e6300e9e00:

  Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-08-02 17:21:50 +0100)

are available in the Git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2021-08-03-pull-tag

for you to fetch changes up to e300858ed4a6d0cbd52b7fb5082d3c69cc371965:

  qga-win/msi: fix missing libstdc++-6 DLL in MSI installer (2021-08-03 07:01:36 -0500)

----------------------------------------------------------------
qemu-ga patch queue for hard-freeze

* w32: Fix missing/incorrect DLLs in MSI installer
* w32: Fix memory leaks in guest-get-osinfo/guest-get-fsinfo
* w32: Increase timeout for guest-fsfreeze-freeze

----------------------------------------------------------------
Basil Salman (3):
      qga-win: Increase VSS freeze timeout to 60 secs instead of 10
      qga-win: Fix build_guest_fsinfo() close of nonexistent
      qga-win: Fix handle leak in ga_get_win_product_name()

Gerd Hoffmann (1):
      qemu-ga/msi: fix w32 libgcc name

Kostiantyn Kostiuk (1):
      qga-win: Free GMatchInfo properly

Michael Roth (1):
      qga-win/msi: fix missing libstdc++-6 DLL in MSI installer

 qga/commands-win32.c        | 18 ++++++++++++------
 qga/installer/qemu-ga.wxs   |  6 +++++-
 qga/vss-win32/requester.cpp |  2 +-
 3 files changed, 18 insertions(+), 8 deletions(-)

[PULL for-6.1 1/6] qga-win: Increase VSS freeze timeout to 60 secs instead of 10

[Michael Roth]

From: Basil Salman <basil@daynix.com>

Currently Requester freeze times out after 10 seconds, while
the default timeout for Writer Freeze is 60 seconds. according to
VSS Documentation [1].
[1]: https://docs.microsoft.com/en-us/windows/win32/vss/overview-of-processing-a-backup-under-vss

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1909073

Signed-off-by: Basil Salman <bsalman@daynix.com>
Signed-off-by: Basil Salman <basil@daynix.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qga/vss-win32/requester.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 5378c55d23..940a2c8f55 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -18,7 +18,7 @@
 #include <inc/win2003/vsbackup.h>
 
 /* Max wait time for frozen event (VSS can only hold writes for 10 seconds) */
-#define VSS_TIMEOUT_FREEZE_MSEC 10000
+#define VSS_TIMEOUT_FREEZE_MSEC 60000
 
 /* Call QueryStatus every 10 ms while waiting for frozen event */
 #define VSS_TIMEOUT_EVENT_MSEC 10
-- 
2.25.1

[PULL for-6.1 2/6] qga-win: Fix build_guest_fsinfo() close of nonexistent

[Michael Roth]

From: Basil Salman <basil@daynix.com>

On the current error path of build_guest_fsinfo(), a non existent handle
is passed to CloseHandle().

This patch adds initialization of hLocalDiskHandle to
INVALID_HANDLE_VALUE, and checks for handle validity before the handle
is closed.

Signed-off-by: Basil Salman <basil@daynix.com>
Signed-off-by: Basil Salman <basil@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qga/commands-win32.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index a099acb34d..763186efd4 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1091,7 +1091,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
     size_t len;
     uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes;
     GuestFilesystemInfo *fs = NULL;
-    HANDLE hLocalDiskHandle = NULL;
+    HANDLE hLocalDiskHandle = INVALID_HANDLE_VALUE;
 
     GetVolumePathNamesForVolumeName(guid, (LPCH)&mnt, 0, &info_size);
     if (GetLastError() != ERROR_MORE_DATA) {
@@ -1149,7 +1149,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp)
     fs->type = g_strdup(fs_name);
     fs->disk = build_guest_disk_info(guid, errp);
 free:
-    CloseHandle(hLocalDiskHandle);
+    if (hLocalDiskHandle != INVALID_HANDLE_VALUE) {
+        CloseHandle(hLocalDiskHandle);
+    }
     g_free(mnt_point);
     return fs;
 }
-- 
2.25.1

[PULL for-6.1 3/6] qga-win: Fix handle leak in ga_get_win_product_name()

[Michael Roth]

From: Basil Salman <basil@daynix.com>

In ga_get_win_product_name() a handle to Registry key was open but not
closed.

In this patch the handle is closed as part of the free routine.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1929144

Signed-off-by: Basil Salman <basil@daynix.com>
Signed-off-by: Basil Salman <bsalman@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qga/commands-win32.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 763186efd4..098211e724 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2231,7 +2231,7 @@ static char *ga_get_win_name(OSVERSIONINFOEXW const *os_version, bool id)
 
 static char *ga_get_win_product_name(Error **errp)
 {
-    HKEY key = NULL;
+    HKEY key = INVALID_HANDLE_VALUE;
     DWORD size = 128;
     char *result = g_malloc0(size);
     LONG err = ERROR_SUCCESS;
@@ -2241,7 +2241,8 @@ static char *ga_get_win_product_name(Error **errp)
                       &key);
     if (err != ERROR_SUCCESS) {
         error_setg_win32(errp, err, "failed to open registry key");
-        goto fail;
+        g_free(result);
+        return NULL;
     }
 
     err = RegQueryValueExA(key, "ProductName", NULL, NULL,
@@ -2262,9 +2263,13 @@ static char *ga_get_win_product_name(Error **errp)
         goto fail;
     }
 
+    RegCloseKey(key);
     return result;
 
 fail:
+    if (key != INVALID_HANDLE_VALUE) {
+        RegCloseKey(key);
+    }
     g_free(result);
     return NULL;
 }
-- 
2.25.1

[PULL for-6.1 4/6] qga-win: Free GMatchInfo properly

[Michael Roth]

From: Kostiantyn Kostiuk <konstantin@daynix.com>

The g_regex_match function creates match_info even if it
returns FALSE. So we should always call g_match_info_free.
A better solution is using g_autoptr for match_info variable.

Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qga/commands-win32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 098211e724..7bac0c5d42 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2459,7 +2459,7 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
             continue;
         }
         for (j = 0; hw_ids[j] != NULL; j++) {
-            GMatchInfo *match_info;
+            g_autoptr(GMatchInfo) match_info;
             GuestDeviceIdPCI *id;
             if (!g_regex_match(device_pci_re, hw_ids[j], 0, &match_info)) {
                 continue;
@@ -2476,7 +2476,6 @@ GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
             id->vendor_id = g_ascii_strtoull(vendor_id, NULL, 16);
             id->device_id = g_ascii_strtoull(device_id, NULL, 16);
 
-            g_match_info_free(match_info);
             break;
         }
         if (skip) {
-- 
2.25.1

[PULL for-6.1 5/6] qemu-ga/msi: fix w32 libgcc name

[Michael Roth]

From: Gerd Hoffmann <kraxel@redhat.com>

This is what I find on my Fedora 34 mingw install.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qga/installer/qemu-ga.wxs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index 9cb4c3d733..ce7b25b5e1 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -31,7 +31,7 @@
   <?endif?>
 
   <?if $(var.Arch) = "32"?>
-    <?define ArchLib=libgcc_s_sjlj-1.dll?>
+    <?define ArchLib=libgcc_s_dw2-1.dll?>
     <?define GaProgramFilesFolder="ProgramFilesFolder" ?>
   <?endif?>
 
-- 
2.25.1

[PULL for-6.1 6/6] qga-win/msi: fix missing libstdc++-6 DLL in MSI installer

[Michael Roth]

libstdc++ is required for the qga-vss.dll that provides fsfreeze
functionality. Currently it is not provided by the MSI installer,
resulting in fsfreeze being disabled in guest environments where it has
not been installed by other means.

In the future this would be better handled via gcc-cpp ComponentGroup
provided by msitools, but that would be better handled with a general
rework of DLL dependency handling in the installer build. Keep it
simple for now to fix this regression.

Tested with Fedora 34 mingw build environment.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Kostiantyn Kostiuk <konstantin@daynix.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 qga/installer/qemu-ga.wxs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index ce7b25b5e1..0950e8c6be 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -84,6 +84,9 @@
             <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="yes" />
           </Component>
           <?ifdef var.InstallVss?>
+          <Component Id="libstdc++_6_lib" Guid="{55E737B5-9127-4A11-9FC3-A29367714574}">
+            <File Id="libstdc++-6.lib" Name="libstdc++-6.dll" Source="$(var.Mingw_bin)/libstdc++-6.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
           <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}">
             <File Id="qga_vss.dll" Name="qga-vss.dll" Source="$(env.BUILD_DIR)/qga/vss-win32/qga-vss.dll" KeyPath="yes" DiskId="1"/>
           </Component>
@@ -164,6 +167,7 @@
     <Feature Id="QEMUFeature" Title="QEMU Guest Agent" Level="1">
       <ComponentRef Id="qemu_ga" />
       <?ifdef var.InstallVss?>
+      <ComponentRef Id="libstdc++_6_lib" />
       <ComponentRef Id="qga_vss_dll" />
       <ComponentRef Id="qga_vss_tlb" />
       <?endif?>
-- 
2.25.1

Re: [PULL for-6.1 0/6] qemu-ga patch queue for hard-freeze

[Peter Maydell]

On Tue, 3 Aug 2021 at 14:26, Michael Roth <michael.roth@amd.com> wrote:
>
> Hi Peter,
>
> Sorry for the late submission. These patches affect only the w32 build of
> qemu-ga. A number of these patches I've had queued for some time, but a bug
> in the MSI installer that was just fixed was blocking testing. Now that that
> is working again I am hoping to get these in along with a couple of other
> fixes that have come in since then.
>
> The following changes since commit 7f1cab9c628a798ae2607940993771e6300e9e00:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2021-08-02 17:21:50 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/mdroth/qemu.git tags/qga-pull-2021-08-03-pull-tag
>
> for you to fetch changes up to e300858ed4a6d0cbd52b7fb5082d3c69cc371965:
>
>   qga-win/msi: fix missing libstdc++-6 DLL in MSI installer (2021-08-03 07:01:36 -0500)
>
> ----------------------------------------------------------------
> qemu-ga patch queue for hard-freeze
>
> * w32: Fix missing/incorrect DLLs in MSI installer
> * w32: Fix memory leaks in guest-get-osinfo/guest-get-fsinfo
> * w32: Increase timeout for guest-fsfreeze-freeze
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.

-- PMM