[PATCH] qemu: rdp: Fix 'qemuRdpAvailable()'
From: Peter Krempa <pkrempa@redhat.com> qemuRdpAvailable() is called from the capability filing code, thus: - it must not report spurious errors - it should not call any extra processes We can solve the above by just checking existance of 'qemu-rdp' in the path as: - at the time of adding of qemuRdpAvailable() there was only one 'qemu-rdp' release - it supported all the features - the check can't change as we'd drop the capability Add comments and gut the check to only check existance of the file. Fixes: f5e5a9bec9ec3e6c762f5000e3b8a0ba6a3a8c8d Closes: https://gitlab.com/libvirt/libvirt/-/issues/763 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_rdp.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_rdp.c b/src/qemu/qemu_rdp.c index 984795d599..0c48b87e7b 100644 --- a/src/qemu/qemu_rdp.c +++ b/src/qemu/qemu_rdp.c @@ -413,12 +413,25 @@ qemuRdpSetCredentials(virDomainObj *vm, } +/** + * qemuRdpAvailable: + * @helper: name (or path to) 'qemu-rdp' binary + * + * Returns whether 'qemu-rdp' is available. + * + * Important: + * This function is called from 'virQEMUDriverGetDomainCapabilities'. It must + * not report any errors and must not add any additional checks. + * + * This function is mocked from 'tests/testutilsqemu.c' + * + */ bool qemuRdpAvailable(const char *helper) { - g_autoptr(qemuRdp) rdp = NULL; - - rdp = qemuRdpNewForHelper(helper); + g_autofree char *helperPath = NULL; - return rdp && qemuRdpHasFeature(rdp, QEMU_RDP_FEATURE_DBUS_ADDRESS); + /* This function was added corresponding to the first release of 'qemu-rdp' + * thus checking existance of the helper binary is sufficient. */ + return !!(helperPath = virFindFileInPath(helper)); } -- 2.49.0
On Mon, Apr 7, 2025 at 8:22 PM Peter Krempa via Devel <devel@lists.libvirt.org> wrote:
From: Peter Krempa <pkrempa@redhat.com>
qemuRdpAvailable() is called from the capability filing code, thus: - it must not report spurious errors - it should not call any extra processes
We can solve the above by just checking existance of 'qemu-rdp' in the path as: - at the time of adding of qemuRdpAvailable() there was only one 'qemu-rdp' release - it supported all the features - the check can't change as we'd drop the capability
Add comments and gut the check to only check existance of the file.
Fixes: f5e5a9bec9ec3e6c762f5000e3b8a0ba6a3a8c8d Closes: https://gitlab.com/libvirt/libvirt/-/issues/763 Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--- src/qemu/qemu_rdp.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_rdp.c b/src/qemu/qemu_rdp.c index 984795d599..0c48b87e7b 100644 --- a/src/qemu/qemu_rdp.c +++ b/src/qemu/qemu_rdp.c @@ -413,12 +413,25 @@ qemuRdpSetCredentials(virDomainObj *vm, }
+/** + * qemuRdpAvailable: + * @helper: name (or path to) 'qemu-rdp' binary + * + * Returns whether 'qemu-rdp' is available. + * + * Important: + * This function is called from 'virQEMUDriverGetDomainCapabilities'. It must + * not report any errors and must not add any additional checks. + * + * This function is mocked from 'tests/testutilsqemu.c' + * + */ bool qemuRdpAvailable(const char *helper) { - g_autoptr(qemuRdp) rdp = NULL; - - rdp = qemuRdpNewForHelper(helper); + g_autofree char *helperPath = NULL;
- return rdp && qemuRdpHasFeature(rdp, QEMU_RDP_FEATURE_DBUS_ADDRESS); + /* This function was added corresponding to the first release of 'qemu-rdp' + * thus checking existance of the helper binary is sufficient. */ + return !!(helperPath = virFindFileInPath(helper)); } -- 2.49.0
-- Marc-André Lureau
On 4/7/25 18:22, Peter Krempa via Devel wrote:
From: Peter Krempa<pkrempa@redhat.com>
qemuRdpAvailable() is called from the capability filing code, thus: - it must not report spurious errors - it should not call any extra processes
We can solve the above by just checking existance of 'qemu-rdp' in the path as: - at the time of adding of qemuRdpAvailable() there was only one 'qemu-rdp' release - it supported all the features - the check can't change as we'd drop the capability
Add comments and gut the check to only check existance of the file.
Fixes: f5e5a9bec9ec3e6c762f5000e3b8a0ba6a3a8c8d Closes:https://gitlab.com/libvirt/libvirt/-/issues/763 Signed-off-by: Peter Krempa<pkrempa@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (3)
-
Boris Fiuczynski -
Marc-André Lureau -
Peter Krempa