Details

Reviewers

andrew
manu
bnovkov

Group Reviewers

bhyve
manpages

Commits

rGed60e5710e74: bhyvectl: support querying VM pid

Summary

Extend bhyvectl(8) to support querying VM pid using the
--get-vm-pid flag.

This is useful in monitor mode when the VM pid differs
from the main bhyve(8) process run by the user. Knowing the VM pid
is necessary, for example, to query process resource usage or
trigger ACPI shutdown with SIGTERM.

Of course, it could be obtained by matching the monitor process
children by the process title, but it's a little more complex
and fragile than it could be.

Implement that by adding the "get_vm_pid" IPC command,
and using it to implement bhyvectl --get-vm-pid, which
prints the VM PID.

When the VM PID is not known, ESRCH is returned.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 75963
Build 72846: arc lint + arc unit

Event Timeline

novel created this revision.Aug 22 2026, 10:37 AM

Herald added a reviewer: andrew. · View Herald TranscriptAug 22 2026, 10:37 AM

Herald added a reviewer: andrew. · View Herald Transcript

Herald added a reviewer: manu. · View Herald Transcript

Herald added a reviewer: bhyve. · View Herald Transcript

Herald added subscribers: riscv, bcran, rgrimes and 2 others. · View Herald Transcript

novel requested review of this revision.Aug 22 2026, 10:37 AM

Harbormaster completed remote builds in B75963: Diff 184734.Aug 22 2026, 10:37 AM

This is a good feature, but I don't think that this is the correct approach design-wise. Adding an ioctl handler to just to fetch a VM's PID is overkill IMO, new ioctl commands should only be added when there's no other way to obtain the needed information (e.g., fetching memsegs and other metadata that is stored in the vmm module).

I strongly suggest that you implement this functionality using bhyve's IPC facilities. These were only reserved for the snapshot subsystem up until very recently, but I had to split these out into a separate subsystem as a part of my hotplugging work.
This should make the implementation way simpler since you just need to register an IPC handler instead of adding a new ioctl.
I just committed the refactor so you can build on top of it.

In D59108#1355951, @bnovkov wrote:

This is a good feature, but I don't think that this is the correct approach design-wise. Adding an ioctl handler to just to fetch a VM's PID is overkill IMO, new ioctl commands should only be added when there's no other way to obtain the needed information (e.g., fetching memsegs and other metadata that is stored in the vmm module).

I strongly suggest that you implement this functionality using bhyve's IPC facilities. These were only reserved for the snapshot subsystem up until very recently, but I had to split these out into a separate subsystem as a part of my hotplugging work.
This should make the implementation way simpler since you just need to register an IPC handler instead of adding a new ioctl.
I just committed the refactor so you can build on top of it.

Thanks, I'll take a look at the new IPC code.

Re-implement using IPC.

Harbormaster completed remote builds in B76158: Diff 185089.Aug 25 2026, 4:49 PM

bnovkov added inline comments.Aug 31 2026, 7:12 PM

usr.sbin/bhyvectl/bhyvectl.c
267	Are we allowing the error to be suppressed so we can handle `get_all` gracefully?
297	If we pass a non-NULL `replyp` we'll end up passing NULL to this `nvlist_destroy` call. We should only be destroying it if the caller isn't interested in the value.
531–535	Why not just handle the error in `vm_pid_request`?

Do not call nvlist_destroy() on NULL.
Update vm_pid_request() to properly set report_error depending on whether it's --get-all or --get-vm-pid option.
bhyvectl.8: rundir: s/checkpoint socket/bhyve IPC socket/.

Herald added a subscriber: ziaee. · View Herald TranscriptSep 2 2026, 4:37 PM

Harbormaster completed remote builds in B76407: Diff 185635.Sep 2 2026, 4:37 PM

novel marked 2 inline comments as not done.Sep 2 2026, 4:41 PM

novel added inline comments.

usr.sbin/bhyvectl/bhyvectl.c
267	Correct. As described in summary, my idea is that `--get-all` should print the PID if it knows it, but do not fail if it does not (and should not print errors coming from attempting to get it). And when explicitly requested with `--get-vm-pid`, it should fail if it cannot get it, and report errors related to that.
297	`nvlist_destroy()` allows passing NULL and does nothing in this case, so it should be safe. I'll update the code to be more explicit though.
531–535	As mentioned in the other comment, I'd like treat errors differently based on the mode we're currently in (`--get-all` or `--get-vm-pid`). I noticed though that I don't set `report_error` to `true` for the `--get-vm-pid`, so I'll update `vm_pid_request()`. Anyway, it feels like this error handling logic fits here better than in the `vm_pid_request()`.

bnovkov added inline comments.Sep 9 2026, 1:35 PM

usr.sbin/bhyvectl/bhyvectl.c
267	hm, I am a bit conflicted about having `--get-vm-pid` included in `--get-all` since it is implicitly dependent on the `rundir` option. I think that we shouldn't mix ioctl and IPC operations in `--get-all`, so I'd personally leave this out of `--get-all` . I'm not sure what the best solution here would be, so I won't insist.

novel added inline comments.Sep 9 2026, 4:11 PM

usr.sbin/bhyvectl/bhyvectl.c
267	I don't have practical reasons to keep it in `--get-all`. The only motivation was consistency in sense that "vm pid" falls under "all". On the other hand, the `--get-all` seems to be a sort of a service feature that we do not document in the manual page and do not promise anything about it . I personally do not use `--get-all`, at least not in the libvirt driver. I think I'll remove this part from the patch for now, it seems to create too many complications for a feature that probably nobody needs.