From 917ebcb170273913bca33d44263bc5fd14f72fd7 Mon Sep 17 00:00:00 2001 From: Basil Salman Date: Mon, 6 Jul 2020 18:50:39 +0300 Subject: [PATCH 1/4] qga-win: Fix QGA VSS Provider service stop failure 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 Signed-off-by: Basil Salman Signed-off-by: Michael Roth --- 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 #include #include +#include #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 pUnknown; - COMPointer 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; } From 844bd70b5652f30bbace89499f513e3fbbb6457a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Thu, 4 Jun 2020 11:44:25 +0200 Subject: [PATCH 2/4] qga: fix assert regression on guest-shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reported-by: Christian Ehrhardt Signed-off-by: Marc-André Lureau Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Christian Ehrhardt Tested-by: Christian Ehrhardt Cc: qemu-stable@nongnu.org Signed-off-by: Michael Roth --- 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) { From e47f4765afcab2b78dfa5b0115abf64d1d49a5d3 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 22 Jun 2020 20:19:35 +0200 Subject: [PATCH 3/4] util: Introduce qemu_get_host_name() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Daniel P. Berrangé Cc: qemu-stable@nongnu.org Signed-off-by: Michael Roth --- 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); +} From 0d3a8f32b1e0eca279da1b0cc793efc7250c3daf Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 22 Jun 2020 20:19:36 +0200 Subject: [PATCH 4/4] qga: Use qemu_get_host_name() instead of g_get_host_name() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel P. Berrangé Cc: qemu-stable@nongnu.org Signed-off-by: Michael Roth --- 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; }