[PATCH] bhyve: fix bhyveConnectAgent()

Roman Bogorodskiy posted 1 patch 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260614065123.4121-1-bogorodskiy@gmail.com
src/bhyve/bhyve_process.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
[PATCH] bhyve: fix bhyveConnectAgent()
Posted by Roman Bogorodskiy 1 week, 3 days ago
The bhyveConnectAgent() function calls qemuAgentOpen() to open an agent
connection. If it fails, e.g. because of insufficient permissions to
open the socket, it returns NULL. Currently, if that happens,
bhyveConnectAgent() just sets agentError to true and exits with 0.
This does not match the contract of bhyveDomainEnsureAgent(), which
should either provide an agent connection or fail.

Fix that by returning -1 when qemuAgentOpen() fails. To make intent
clearer, add a documentation for bhyveConnectAgent().

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
 src/bhyve/bhyve_process.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index e92bdeb416..ed7e4e7725 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -238,6 +238,18 @@ static qemuAgentCallbacks agentCallbacks = {
     .errorNotify = bhyveProcessHandleAgentError,
 };
 
+/**
+ * bhyveConnectAgent:
+ * @driver: driver object
+ * @vm: domain object
+ *
+ * Connect to the guest agent via the UNIX socket specified in the domain
+ * definition. On success, bhyveDomainObjPrivate's agent field should
+ * point to the agent instance.
+ *
+ * Returns: 0 on success,
+ *          -1 on error
+ */
 int
 bhyveConnectAgent(struct _bhyveConn *driver G_GNUC_UNUSED, virDomainObj *vm)
 {
@@ -270,8 +282,7 @@ bhyveConnectAgent(struct _bhyveConn *driver G_GNUC_UNUSED, virDomainObj *vm)
     priv->agent = agent;
     if (!priv->agent) {
         VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name);
-        priv->agentError = true;
-        virResetLastError();
+        return -1;
     }
 
     return 0;
-- 
2.52.0
Re: [PATCH] bhyve: fix bhyveConnectAgent()
Posted by Ján Tomko via Devel 1 week, 2 days ago
On a Sunday in 2026, Roman Bogorodskiy wrote:
>The bhyveConnectAgent() function calls qemuAgentOpen() to open an agent
>connection. If it fails, e.g. because of insufficient permissions to
>open the socket, it returns NULL. Currently, if that happens,
>bhyveConnectAgent() just sets agentError to true and exits with 0.
>This does not match the contract of bhyveDomainEnsureAgent(), which
>should either provide an agent connection or fail.
>
>Fix that by returning -1 when qemuAgentOpen() fails. To make intent
>clearer, add a documentation for bhyveConnectAgent().
>
>Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
>---
> src/bhyve/bhyve_process.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
>index e92bdeb416..ed7e4e7725 100644
>--- a/src/bhyve/bhyve_process.c
>+++ b/src/bhyve/bhyve_process.c
>@@ -238,6 +238,18 @@ static qemuAgentCallbacks agentCallbacks = {
>     .errorNotify = bhyveProcessHandleAgentError,
> };
>
>+/**
>+ * bhyveConnectAgent:
>+ * @driver: driver object
>+ * @vm: domain object
>+ *
>+ * Connect to the guest agent via the UNIX socket specified in the domain
>+ * definition. On success, bhyveDomainObjPrivate's agent field should
>+ * point to the agent instance.
>+ *
>+ * Returns: 0 on success,
>+ *          -1 on error
>+ */
> int
> bhyveConnectAgent(struct _bhyveConn *driver G_GNUC_UNUSED, virDomainObj *vm)
> {
>@@ -270,8 +282,7 @@ bhyveConnectAgent(struct _bhyveConn *driver G_GNUC_UNUSED, virDomainObj *vm)
>     priv->agent = agent;
>     if (!priv->agent) {
>         VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name);
>-        priv->agentError = true;
>-        virResetLastError();

There should be a virReportError() call as well, otherwise the user will
get the cryptic "an error occurred but the cause is unknown" message

>+        return -1;
>     }

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano