On Mon, Apr 7, 2025 at 8:22 PM Peter Krempa via Devel
<devel(a)lists.libvirt.org> wrote:
From: Peter Krempa <pkrempa(a)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(a)redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau(a)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