Currently, there are two (at least) issues in virBhyveProcessStop().

Before going into details, a quick overview of the bhyve shutdown
process. It is a two stage process*: first, the main bhyve
process gets destroyed (either via an external command or within the
guest), then the resources need to be cleaned up using the bhyvectl(8)
tool.

The first issue is that if virCommandRun() for bhyvectl(8) fails,
virBhyveProcessStop() jumps to the 'cleanup' label and misses cleaning
of some resources.

The second issue is more serious. Currently, monitor is closed only
after running of the bhyvectl(8) command. That means that the monitor
could catch the domain destroy event and try to run
virBhyveProcessStop() on the same domain again, resulting in trying
to release already released resources, such as the monitor itself.

Address by:

 * Making virCommandRun() on bhyvectl(8) non-critical. Even if it
   fails, we try to clean up all resources. We consider the function
   failed (return value 1) though.

 * Close monitor before running bhyvectl(8)

Additionally, do not verify that virBhyveProcessBuildDestroyCmd()
returns non-NULL, there could be only allocation errors.

Reported-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
---
 src/bhyve/bhyve_process.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 26320200c5..842ff0d6fc 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -515,7 +515,7 @@ virBhyveProcessStop(struct _bhyveConn *driver,
                     virDomainObj *vm,
                     virDomainShutoffReason reason)
 {
-    int ret = -1;
+    int ret = 0;
     g_autoptr(virCommand) cmd = NULL;
     bhyveDomainObjPrivate *priv = vm->privateData;
 
@@ -531,15 +531,19 @@ virBhyveProcessStop(struct _bhyveConn *driver,
         return -1;
     }
 
-    if (!(cmd = virBhyveProcessBuildDestroyCmd(driver, vm->def)))
-        return -1;
-
-    if (virCommandRun(cmd, NULL) < 0)
-        goto cleanup;
-
+    /* Destroy monitor before running the actual destroy command to prevent
+     * it from detecting VM shutdown and entering this cleanup routine again */
     if ((priv != NULL) && (priv->mon != NULL))
          bhyveMonitorClose(priv->mon);
 
+    cmd = virBhyveProcessBuildDestroyCmd(driver, vm->def);
+    if (virCommandRun(cmd, NULL) < 0) {
+        /* Only failure of the actual destroy command warrants unsuccessful return code,
+         * other failures are not considered critical */
+        ret = -1;
+        VIR_WARN("Failed to run the domain destroy command");
+    }
+
     bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_STOPPED);
 
     /* Cleanup network interfaces */
@@ -554,8 +558,6 @@ virBhyveProcessStop(struct _bhyveConn *driver,
         }
     }
 
-    ret = 0;
-
     virCloseCallbacksDomainRemove(vm, NULL, bhyveProcessAutoDestroy);
 
     virDomainObjSetState(vm, VIR_DOMAIN_SHUTOFF, reason);
@@ -563,8 +565,6 @@ virBhyveProcessStop(struct _bhyveConn *driver,
     vm->def->id = -1;
 
     bhyveProcessStopHook(driver, vm, VIR_HOOK_BHYVE_OP_RELEASE);
-
- cleanup:
     virPidFileDelete(BHYVE_STATE_DIR, vm->def->name);
     bhyveProcessRemoveDomainStatus(BHYVE_STATE_DIR, vm->def->name);
 
-- 
2.52.0

On Thu, Apr 23, 2026 at 20:15:16 +0200, Roman Bogorodskiy wrote:
> Currently, there are two (at least) issues in virBhyveProcessStop().
> 
> Before going into details, a quick overview of the bhyve shutdown
> process. It is a two stage process*: first, the main bhyve
> process gets destroyed (either via an external command or within the
> guest), then the resources need to be cleaned up using the bhyvectl(8)
> tool.
> 
> The first issue is that if virCommandRun() for bhyvectl(8) fails,
> virBhyveProcessStop() jumps to the 'cleanup' label and misses cleaning
> of some resources.
> 
> The second issue is more serious. Currently, monitor is closed only
> after running of the bhyvectl(8) command. That means that the monitor
> could catch the domain destroy event and try to run
> virBhyveProcessStop() on the same domain again, resulting in trying
> to release already released resources, such as the monitor itself.
> 
> Address by:
> 
>  * Making virCommandRun() on bhyvectl(8) non-critical. Even if it
>    fails, we try to clean up all resources. We consider the function
>    failed (return value 1) though.
> 
>  * Close monitor before running bhyvectl(8)
> 
> Additionally, do not verify that virBhyveProcessBuildDestroyCmd()
> returns non-NULL, there could be only allocation errors.

And with 'glib' they result in an abort() so no need to worry about
those.

> 
> Reported-by: Peter Krempa <pkrempa@redhat.com>
> Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com>
> ---
>  src/bhyve/bhyve_process.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

  Peter Krempa wrote:

> On Thu, Apr 23, 2026 at 20:15:16 +0200, Roman Bogorodskiy wrote:
> > Currently, there are two (at least) issues in virBhyveProcessStop().
> > 
> > Before going into details, a quick overview of the bhyve shutdown
> > process. It is a two stage process*: first, the main bhyve

I noticed I put a mark (*) and forgot to explain it.

In FreeBSD -CURRENT bhyve has a monitor mode:

     -M          Run the VM in ‘monitor’ mode.  In this mode, a guest reboot
                 does not cause the bhyve process to exit.  Instead, bhyve
                 will restart the VM.  Once the bhyve process exits or is
                 killed, the VM will be destroyed automatically.

So I think at some point the two stage shutdown will go away. But as
this feature is not available in any released FreeBSD version, and as I
didn't test it, it is quite unlikely that it will happen soon.

[...]
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Thanks for reviews!

As it is a freeze now, and also considering that I'll be mostly AFK next
week, I'll push this and the virtio-console series after the release.

Thanks,
Roman