[PATCH] bhyve: hooks: improve process start error handling

Roman Bogorodskiy posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20250925173023.39599-1-bogorodskiy@gmail.com
src/bhyve/bhyve_process.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
[PATCH] bhyve: hooks: improve process start error handling
Posted by Roman Bogorodskiy 1 week, 2 days ago
When the virBhyveProcessStart() fails early, make sure to execute
the "stopped" and "release" hooks.

Spotted while running TCK hooks tests against the bhyve driver.

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

diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 4bfd511bae..4fb7e642e1 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -413,18 +413,24 @@ virBhyveProcessStart(bhyveConn *driver,
 {
     /* Run an early hook to setup missing devices. */
     if (bhyveProcessStartHook(driver, vm, VIR_HOOK_BHYVE_OP_PREPARE) < 0)
-        return -1;
+        goto cleanup;
 
     if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY)
         virCloseCallbacksDomainAdd(vm, conn, bhyveProcessAutoDestroy);
 
     if (bhyveProcessPrepareDomain(driver, vm, flags) < 0)
-        return -1;
+        goto cleanup;
 
     if (bhyveProcessPrepareHost(driver, vm->def, flags) < 0)
-        return -1;
+        goto cleanup;
 
     return virBhyveProcessStartImpl(driver, vm, reason);
+
+ cleanup:
+    bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_STOPPED);
+    bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_RELEASE);
+
+    return -1;
 }
 
 static void
-- 
2.51.0
Re: [PATCH] bhyve: hooks: improve process start error handling
Posted by Martin Kletzander via Devel 1 week, 1 day ago
On Thu, Sep 25, 2025 at 07:30:23PM +0200, Roman Bogorodskiy wrote:
>When the virBhyveProcessStart() fails early, make sure to execute
>the "stopped" and "release" hooks.
>
>Spotted while running TCK hooks tests against the bhyve driver.
>
>Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
>---
> src/bhyve/bhyve_process.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
>index 4bfd511bae..4fb7e642e1 100644
>--- a/src/bhyve/bhyve_process.c
>+++ b/src/bhyve/bhyve_process.c
>@@ -413,18 +413,24 @@ virBhyveProcessStart(bhyveConn *driver,
> {
>     /* Run an early hook to setup missing devices. */
>     if (bhyveProcessStartHook(driver, vm, VIR_HOOK_BHYVE_OP_PREPARE) < 0)
>-        return -1;
>+        goto cleanup;
>
>     if (flags & VIR_BHYVE_PROCESS_START_AUTODESTROY)
>         virCloseCallbacksDomainAdd(vm, conn, bhyveProcessAutoDestroy);
>
>     if (bhyveProcessPrepareDomain(driver, vm, flags) < 0)
>-        return -1;
>+        goto cleanup;
>
>     if (bhyveProcessPrepareHost(driver, vm->def, flags) < 0)
>-        return -1;
>+        goto cleanup;
>

I wanted to suggest that if you want to avoid the goto's, then just
stick the above in a "StartPrep" function and call it here and then the
stop hooks if it fails.  In the end the diff was larger than what you
suggested =)

So whichever you like more,

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

and safe for freeze.

>     return virBhyveProcessStartImpl(driver, vm, reason);
>+
>+ cleanup:
>+    bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_STOPPED);
>+    bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_RELEASE);
>+
>+    return -1;
> }
>
> static void
>-- 
>2.51.0
>