From 83f6dceb8f5c0a1efe806a9a4905cbc743a8a378 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 30 Jul 2025 09:27:09 +0200 Subject: qga: Fix ubsan warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When compiling QEMU with --enable-ubsan there is a undefined behavior warning when running "make check": .../qga/commands-linux.c:452:15: runtime error: applying non-zero offset 5 to null pointer #0 0x55ea7b89450c in build_guest_fsinfo_for_pci_dev ..../qga/commands-linux.c:452:15 Fix it by avoiding the additional pointer variable here and use an "offset" integer variable instead. Signed-off-by: Thomas Huth Reviewed-by: Daniel P. Berrangé Reviewed-by: Kostiantyn Kostiuk Link: https://lore.kernel.org/qemu-devel/20250730072709.27077-1-thuth@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/commands-linux.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/qga/commands-linux.c b/qga/commands-linux.c index 9dc0c82..4a09ddc 100644 --- a/qga/commands-linux.c +++ b/qga/commands-linux.c @@ -400,10 +400,10 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath, Error **errp) { unsigned int pci[4], host, hosts[8], tgt[3]; - int i, nhosts = 0, pcilen; + int i, offset, nhosts = 0, pcilen; GuestPCIAddress *pciaddr = disk->pci_controller; bool has_ata = false, has_host = false, has_tgt = false; - char *p, *q, *driver = NULL; + char *p, *driver = NULL; bool ret = false; p = strstr(syspath, "/devices/pci"); @@ -445,13 +445,13 @@ static bool build_guest_fsinfo_for_pci_dev(char const *syspath, p = strstr(syspath, "/ata"); if (p) { - q = p + 4; + offset = 4; has_ata = true; } else { p = strstr(syspath, "/host"); - q = p + 5; + offset = 5; } - if (p && sscanf(q, "%u", &host) == 1) { + if (p && sscanf(p + offset, "%u", &host) == 1) { has_host = true; nhosts = build_hosts(syspath, p, has_ata, hosts, ARRAY_SIZE(hosts), errp); -- cgit v1.1 From 42bdb911c22f9449f7a310efc73b70548ca42b24 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Thu, 7 Aug 2025 15:32:21 +0200 Subject: qga: fix potentially not initialized nr_volumes in qga_vss_fsfreeze() In this function we could have this variable not initialized. If this could be acceptable on error, the variable could be left not initialized f.e. as follows: void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset) { ... if (mountpoints) { ... if (num_mount_points == 0) { /* If there is no valid mount points, just exit. */ goto out; } } ... if (!mountpoints) { ... if (num_fixed_drives == 0) { goto out; /* If there is no fixed drive, just exit. */ } } ... } Stay on safe side, initialize the variable at the beginning. Signed-off-by: Denis V. Lunev CC: Kostiantyn Kostiuk CC: Michael Roth Reviewed-by: Kostiantyn Kostiuk Link: https://lore.kernel.org/qemu-devel/20250807133221.1135453-1-den@openvz.org Signed-off-by: Kostiantyn Kostiuk --- qga/vss-win32.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qga/vss-win32.c b/qga/vss-win32.c index f444a25..b272bfc 100644 --- a/qga/vss-win32.c +++ b/qga/vss-win32.c @@ -157,6 +157,8 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, .errp = errp, }; + *nr_volume = 0; + g_assert(errp); /* requester.cpp requires it */ func = (QGAVSSRequesterFunc)GetProcAddress(provider_lib, func_name); if (!func) { -- cgit v1.1 From 9646155bb01c076f982601b1ebdead73efcb58a1 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 25 Aug 2025 17:52:40 +0300 Subject: qga-vss: Replace asserts with condition and report error Reviewed-by: Yan Vugenfirer Link: https://lore.kernel.org/qemu-devel/20250825145241.170717-2-kkostiuk@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/vss-win32/requester.cpp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp index 4401d55..bc260ab 100644 --- a/qga/vss-win32/requester.cpp +++ b/qga/vss-win32/requester.cpp @@ -347,7 +347,12 @@ void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset) goto out; } - assert(pCreateVssBackupComponents != NULL); + if (!pCreateVssBackupComponents) { + err_set(errset, (HRESULT)ERROR_PROC_NOT_FOUND, + "CreateVssBackupComponents proc address absent. Did you call requester_init()?"); + goto out; + } + hr = pCreateVssBackupComponents(&vss_ctx.pVssbc); if (FAILED(hr)) { err_set(errset, hr, "failed to create VSS backup components"); @@ -579,8 +584,16 @@ void requester_thaw(int *num_vols, void *mountpints, ErrorSet *errset) /* Tell the provider that the snapshot is finished. */ SetEvent(vss_ctx.hEventThaw); - assert(vss_ctx.pVssbc); - assert(vss_ctx.pAsyncSnapshot); + if (!vss_ctx.pVssbc) { + err_set(errset, (HRESULT)VSS_E_BAD_STATE, + "CreateVssBackupComponents is missing. Did you freeze the volumes?"); + return; + } + if (!vss_ctx.pAsyncSnapshot) { + err_set(errset, (HRESULT)VSS_E_BAD_STATE, + "AsyncSnapshot set is missing. Did you freeze the volumes?"); + return; + } HRESULT hr = WaitForAsync(vss_ctx.pAsyncSnapshot); switch (hr) { -- cgit v1.1 From ed42682a66ef9f5e892abb7bcc3f211e1180f075 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 25 Aug 2025 17:52:41 +0300 Subject: qga-vss: Remove unused dependencies Reviewed-by: Yan Vugenfirer Link: https://lore.kernel.org/qemu-devel/20250825145241.170717-3-kkostiuk@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/vss-win32/meson.build | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build index 0ac9189..a6b810f 100644 --- a/qga/vss-win32/meson.build +++ b/qga/vss-win32/meson.build @@ -13,13 +13,11 @@ qga_vss = shared_module( link_args: link_args, vs_module_defs: 'qga-vss.def', dependencies: [ - glib, socket, cc.find_library('ole32'), cc.find_library('oleaut32'), cc.find_library('shlwapi'), - cc.find_library('uuid'), - cc.find_library('intl') + cc.find_library('uuid') ] ) -- cgit v1.1 From 3b0ba59762380fff9c91a031301f6f661abaac96 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 25 Aug 2025 17:05:48 +0300 Subject: qga: Fix channel initialization check in run_agent_once Reviewed-by: Yan Vugenfirer Reviewed-by: Michal Privoznik Link: https://lore.kernel.org/qemu-devel/20250825140549.146617-2-kkostiuk@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qga/main.c b/qga/main.c index 6c02f3e..a1bf8f5 100644 --- a/qga/main.c +++ b/qga/main.c @@ -1563,7 +1563,7 @@ static void cleanup_agent(GAState *s) static int run_agent_once(GAState *s) { if (!s->channel && - channel_init(s, s->config->method, s->config->channel_path, + !channel_init(s, s->config->method, s->config->channel_path, s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) { g_critical("failed to initialize guest agent channel"); return EXIT_FAILURE; -- cgit v1.1 From b44c8a6d837ed4e082dd03d79095a4e9141eff5b Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 25 Aug 2025 17:05:49 +0300 Subject: qga: ignore channel_init() fail if 'retry_path' is set On Windows, we run QGA with `-d --retry-path` options by default, and expect that QGA will start even without the vioserial driver and will wait for communication forever. Reviewed-by: Yan Vugenfirer Reviewed-by: Michal Privoznik Link: https://lore.kernel.org/qemu-devel/20250825140549.146617-3-kkostiuk@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/qga/main.c b/qga/main.c index a1bf8f5..dd1c216 100644 --- a/qga/main.c +++ b/qga/main.c @@ -1512,8 +1512,12 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation) if (!channel_init(s, s->config->method, s->config->channel_path, s->socket_activation ? FIRST_SOCKET_ACTIVATION_FD : -1)) { - g_critical("failed to initialize guest agent channel"); - return NULL; + if (s->config->retry_path) { + g_info("failed to initialize guest agent channel, will retry"); + } else { + g_critical("failed to initialize guest agent channel"); + return NULL; + } } if (config->daemonize) { -- cgit v1.1 From edf3780a7dad4658ab7b72ea37e310a2be9b16d3 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 25 Aug 2025 16:53:11 +0300 Subject: qga-vss: Write hex value of error in log QGA-VSS writes error using error_setg_win32_internal, which call g_win32_error_message. 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. This commit directly add hex value of error code. Reproduce: - Run QGA command: {"execute": "guest-fsfreeze-freeze-list", "arguments": {"mountpoints": ["D:"]}} 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: Windows error 0x8004230e: "}} Reviewed-by: Yan Vugenfirer Link: https://lore.kernel.org/qemu-devel/20250825135311.138330-1-kkostiuk@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/vss-win32/requester.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp index bc260ab..5615955 100644 --- a/qga/vss-win32/requester.cpp +++ b/qga/vss-win32/requester.cpp @@ -28,8 +28,9 @@ #define err_set(e, err, fmt, ...) { \ (e)->error_setg_win32_wrapper((e)->errp, __FILE__, __LINE__, __func__, \ - err, fmt, ## __VA_ARGS__); \ - qga_debug(fmt, ## __VA_ARGS__); \ + err, fmt ": Windows error 0x%lx", \ + ## __VA_ARGS__, err); \ + qga_debug(fmt ": Windows error 0x%lx", ## __VA_ARGS__, err); \ } /* Bad idea, works only when (e)->errp != NULL: */ #define err_is_set(e) ((e)->errp && *(e)->errp) -- cgit v1.1 From 85ff0e956bf26a93c92e4dca8f6257613269a0cf Mon Sep 17 00:00:00 2001 From: Kostiantyn Kostiuk Date: Mon, 25 Aug 2025 17:31:55 +0300 Subject: qga/installer: Remove QGA VSS if QGA installation failed When QGA Installer failed to install QGA service but install QGA VSS provider, provider should be removed before installer exits. Otherwise QGA VSS will has broken infomation and prevent QGA installation in next run. Reviewed-by: Yan Vugenfirer Link: https://lore.kernel.org/qemu-devel/20250825143155.160913-1-kkostiuk@redhat.com Signed-off-by: Kostiantyn Kostiuk --- qga/installer/qemu-ga.wxs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index df572ad..32b8308 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -151,6 +151,14 @@ Return="check" > + + @@ -174,8 +182,19 @@ - Installed - NOT REMOVE + + + + + + + NOT REMOVE + + + NOT REMOVE + + + Installed -- cgit v1.1 From 28c5d27dd4dc4100a96ff4c9e5871dd23c6b02ec Mon Sep 17 00:00:00 2001 From: "minglei.liu" Date: Fri, 11 Jul 2025 10:17:14 +0800 Subject: qga: Fix truncated output handling in guest-exec status reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: minglei.liu Fixes: a1853dca743 Reviewed-by: Daniel P. Berrangé Reviewed-by: Kostiantyn Kostiuk Link: https://lore.kernel.org/qemu-devel/20250711021714.91258-1-minglei.liu@smartx.com Signed-off-by: Kostiantyn Kostiuk --- qga/commands.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index 5a5fad3..5f20af2 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -205,13 +205,15 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **errp) #endif if (gei->out.length > 0) { ges->out_data = g_base64_encode(gei->out.data, gei->out.length); - ges->has_out_truncated = gei->out.truncated; + ges->has_out_truncated = true; + ges->out_truncated = gei->out.truncated; } g_free(gei->out.data); if (gei->err.length > 0) { ges->err_data = g_base64_encode(gei->err.data, gei->err.length); - ges->has_err_truncated = gei->err.truncated; + ges->has_err_truncated = true; + ges->err_truncated = gei->err.truncated; } g_free(gei->err.data); -- cgit v1.1