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

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

[Michael Roth]

The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:

  Merge remote-tracking branch 'remotes/kraxel/tags/fixes-20200713-pull-request' into staging (2020-07-13 16:58:44 +0100)

are available in the Git repository at:

  git://github.com/mdroth/qemu.git tags/qga-pull-2020-07-13-tag

for you to fetch changes up to 0d3a8f32b1e0eca279da1b0cc793efc7250c3daf:

  qga: Use qemu_get_host_name() instead of g_get_host_name() (2020-07-13 17:44:58 -0500)

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

* fix erroneously reporting stale hostname in guest-get-host-name
* fix regression where guest-shutdown asserts when called
* fix race condition with guest-fs-freeze/thaw on w32

----------------------------------------------------------------
Basil Salman (1):
      qga-win: Fix QGA VSS Provider service stop failure

Marc-André Lureau (1):
      qga: fix assert regression on guest-shutdown

Michal Privoznik (2):
      util: Introduce qemu_get_host_name()
      qga: Use qemu_get_host_name() instead of g_get_host_name()

 include/qemu/osdep.h      | 10 ++++++++++
 qga/commands.c            | 17 +++++++++++++----
 qga/main.c                |  6 +++++-
 qga/vss-win32/install.cpp | 33 ++++++++++++++++++++-------------
 util/oslib-posix.c        | 35 +++++++++++++++++++++++++++++++++++
 util/oslib-win32.c        | 13 +++++++++++++
 6 files changed, 96 insertions(+), 18 deletions(-)

[PULL 1/4] qga-win: Fix QGA VSS Provider service stop failure

[Michael Roth]

From: Basil Salman <bsalman@redhat.com>

On one hand "guest-fsfreeze-freeze" command, "COM+ System Application service" is
stopped, on the other hand "guest-fsfreeze-thaw" stops QGA VSS Provider service from
"COM+ Application Admin Catalog".
Invoking a series of freeze and thaw commands may result in QGA failing to stop
VSS Provider service as "COM+ System Application service" is stopped, which can
cause some delay in qga response.
In this commit StopService function was changed and VSS Provider service is now
stopped using Winsvc library API.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1549425

Signed-off-by: Basil Salman <bsalman@redhat.com>
Signed-off-by: Basil Salman <basil@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/vss-win32/install.cpp | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index a456841360..40de133774 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -19,6 +19,7 @@
 #include <comdef.h>
 #include <comutil.h>
 #include <sddl.h>
+#include <winsvc.h>
 
 #define BUFFER_SIZE 1024
 
@@ -509,26 +510,32 @@ namespace _com_util
     }
 }
 
-/* Stop QGA VSS provider service from COM+ Application Admin Catalog */
-
+/* Stop QGA VSS provider service using Winsvc API  */
 STDAPI StopService(void)
 {
     HRESULT hr;
-    COMInitializer initializer;
-    COMPointer<IUnknown> pUnknown;
-    COMPointer<ICOMAdminCatalog2> pCatalog;
+    SC_HANDLE manager = OpenSCManager(NULL, NULL, SC_MANAGER_ALL_ACCESS);
+    SC_HANDLE service = NULL;
 
-    int count = 0;
+    if (!manager) {
+        errmsg(E_FAIL, "Failed to open service manager");
+        hr = E_FAIL;
+        goto out;
+    }
+    service = OpenService(manager, QGA_PROVIDER_NAME, SC_MANAGER_ALL_ACCESS);
 
-    chk(QGAProviderFind(QGAProviderCount, (void *)&count));
-    if (count) {
-        chk(CoCreateInstance(CLSID_COMAdminCatalog, NULL, CLSCTX_INPROC_SERVER,
-            IID_IUnknown, (void **)pUnknown.replace()));
-        chk(pUnknown->QueryInterface(IID_ICOMAdminCatalog2,
-            (void **)pCatalog.replace()));
-        chk(pCatalog->ShutdownApplication(_bstr_t(QGA_PROVIDER_LNAME)));
+    if (!service) {
+        errmsg(E_FAIL, "Failed to open service");
+        hr =  E_FAIL;
+        goto out;
+    }
+    if (!(ControlService(service, SERVICE_CONTROL_STOP, NULL))) {
+        errmsg(E_FAIL, "Failed to stop service");
+        hr = E_FAIL;
     }
 
 out:
+    CloseServiceHandle(service);
+    CloseServiceHandle(manager);
     return hr;
 }
-- 
2.17.1

[PULL 2/4] qga: fix assert regression on guest-shutdown

[Michael Roth]

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

Since commit 781f2b3d1e ("qga: process_event() simplification"),
send_response() is called unconditionally, but will assert when "rsp" is
NULL. This may happen with QCO_NO_SUCCESS_RESP commands, such as
"guest-shutdown".

Fixes: 781f2b3d1e5ef389b44016a897fd55e7a780bf35
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index f0e454f28d..3febf3b0fd 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -531,7 +531,11 @@ static int send_response(GAState *s, const QDict *rsp)
     QString *payload_qstr, *response_qstr;
     GIOStatus status;
 
-    g_assert(rsp && s->channel);
+    g_assert(s->channel);
+
+    if (!rsp) {
+        return 0;
+    }
 
     payload_qstr = qobject_to_json(QOBJECT(rsp));
     if (!payload_qstr) {
-- 
2.17.1

[PULL 3/4] util: Introduce qemu_get_host_name()

[Michael Roth]

From: Michal Privoznik <mprivozn@redhat.com>

This function offers operating system agnostic way to fetch host
name. It is implemented for both POSIX-like and Windows systems.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 include/qemu/osdep.h | 10 ++++++++++
 util/oslib-posix.c   | 35 +++++++++++++++++++++++++++++++++++
 util/oslib-win32.c   | 13 +++++++++++++
 3 files changed, 58 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 979a403984..4841b5c6b5 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -655,4 +655,14 @@ static inline void qemu_reset_optind(void)
 #endif
 }
 
+/**
+ * qemu_get_host_name:
+ * @errp: Error object
+ *
+ * Operating system agnostic way of querying host name.
+ *
+ * Returns allocated hostname (caller should free), NULL on failure.
+ */
+char *qemu_get_host_name(Error **errp);
+
 #endif
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 72907d4d7f..e60aea85b6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -794,3 +794,38 @@ void sigaction_invoke(struct sigaction *action,
     }
     action->sa_sigaction(info->ssi_signo, &si, NULL);
 }
+
+#ifndef HOST_NAME_MAX
+# ifdef _POSIX_HOST_NAME_MAX
+#  define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
+# else
+#  define HOST_NAME_MAX 255
+# endif
+#endif
+
+char *qemu_get_host_name(Error **errp)
+{
+    long len = -1;
+    g_autofree char *hostname = NULL;
+
+#ifdef _SC_HOST_NAME_MAX
+    len = sysconf(_SC_HOST_NAME_MAX);
+#endif /* _SC_HOST_NAME_MAX */
+
+    if (len < 0) {
+        len = HOST_NAME_MAX;
+    }
+
+    /* Unfortunately, gethostname() below does not guarantee a
+     * NULL terminated string. Therefore, allocate one byte more
+     * to be sure. */
+    hostname = g_new0(char, len + 1);
+
+    if (gethostname(hostname, len) < 0) {
+        error_setg_errno(errp, errno,
+                         "cannot get hostname");
+        return NULL;
+    }
+
+    return g_steal_pointer(&hostname);
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e9b14ab178..3b49d27297 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -808,3 +808,16 @@ bool qemu_write_pidfile(const char *filename, Error **errp)
     }
     return true;
 }
+
+char *qemu_get_host_name(Error **errp)
+{
+    wchar_t tmp[MAX_COMPUTERNAME_LENGTH + 1];
+    DWORD size = G_N_ELEMENTS(tmp);
+
+    if (GetComputerNameW(tmp, &size) == 0) {
+        error_setg_win32(errp, GetLastError(), "failed close handle");
+        return NULL;
+    }
+
+    return g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
+}
-- 
2.17.1

[PULL 4/4] qga: Use qemu_get_host_name() instead of g_get_host_name()

[Michael Roth]

From: Michal Privoznik <mprivozn@redhat.com>

Problem with g_get_host_name() is that on the first call it saves
the hostname into a global variable and from then on, every
subsequent call returns the saved hostname. Even if the hostname
changes. This doesn't play nicely with guest agent, because if
the hostname is acquired before the guest is set up (e.g. on the
first boot, or before DHCP) we will report old, invalid hostname.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1845127

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/commands.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/qga/commands.c b/qga/commands.c
index efc8b90281..d3fec807c1 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -515,11 +515,20 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
 GuestHostName *qmp_guest_get_host_name(Error **errp)
 {
     GuestHostName *result = NULL;
-    gchar const *hostname = g_get_host_name();
-    if (hostname != NULL) {
-        result = g_new0(GuestHostName, 1);
-        result->host_name = g_strdup(hostname);
+    g_autofree char *hostname = qemu_get_host_name(errp);
+
+    /*
+     * We want to avoid using g_get_host_name() because that
+     * caches the result and we wouldn't reflect changes in the
+     * host name.
+     */
+
+    if (!hostname) {
+        hostname = g_strdup("localhost");
     }
+
+    result = g_new0(GuestHostName, 1);
+    result->host_name = g_steal_pointer(&hostname);
     return result;
 }
 
-- 
2.17.1

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

[Peter Maydell]

On Tue, 14 Jul 2020 at 05:51, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
>
> The following changes since commit 20c1df5476e1e9b5d3f5b94f9f3ce01d21f14c46:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/fixes-20200713-pull-request' into staging (2020-07-13 16:58:44 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/mdroth/qemu.git tags/qga-pull-2020-07-13-tag
>
> for you to fetch changes up to 0d3a8f32b1e0eca279da1b0cc793efc7250c3daf:
>
>   qga: Use qemu_get_host_name() instead of g_get_host_name() (2020-07-13 17:44:58 -0500)
>
> ----------------------------------------------------------------
> qemu-ga patch queue for hard-freeze
>
> * fix erroneously reporting stale hostname in guest-get-host-name
> * fix regression where guest-shutdown asserts when called
> * fix race condition with guest-fs-freeze/thaw on w32
>
> ----------------------------------------------------------------



Applied, thanks.

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

-- PMM