Details

Reviewers

kevans
markj

Group Reviewers

bhyve

Commits

rGcbc6f7e941e4: bhyve: add UNIX domain socket support to rfb

Summary

This commit adds support for a UNIX domain socket to bhyve's remote
framebuffer. It enables the use of the graphical console when the bhyve instance
is running in a jail with no networking, for instance. A VNC client running on
the host can then connect to the UNIX domain socket through the filesystem.

Signed-off-by: Quentin Thébault <quentin.thebault@defenso.fr>
Sponsored by: Defenso
MFC after: 1 week

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 68817
Build 65700: arc lint + arc unit

Event Timeline

quentin.thebault_defenso.fr created this revision.Nov 19 2025, 8:24 AM

Herald added subscribers: ziaee, bcran, rgrimes, imp. · View Herald TranscriptNov 19 2025, 8:24 AM

quentin.thebault_defenso.fr requested review of this revision.Nov 19 2025, 8:24 AM

Harbormaster completed remote builds in B68722: Diff 166735.Nov 19 2025, 8:24 AM

jlduran added a subscriber: jlduran.Nov 19 2025, 6:12 PM

kevans added a subscriber: kevans.Nov 23 2025, 1:28 AM

kevans added inline comments.

usr.sbin/bhyve/bhyve.8
909–910	New sentence must start on a new line
usr.sbin/bhyve/rfb.c
1309	`strncpy` won't terminate if it fills the buffer, which is fine since you zeroed it all out but imo it's better to be defensive and clear here.
1384–1385	I would have maybe instead asserted that hostname[0] != '/' if AI is non-NULL, since that could be indicative of funkiness.

quentin.thebault_defenso.fr retitled this revision from bhyve: add UNIX domain socket support for rfb to bhyve: add UNIX domain socket support to rfb.Nov 24 2025, 3:47 AM

quentin.thebault_defenso.fr edited the summary of this revision. (Show Details)

Replace strncpy with strlcpy, add assert before freeaddrinfo and newline to bhyve.8

Harbormaster completed remote builds in B68817: Diff 167007.Nov 24 2025, 3:49 AM

quentin.thebault_defenso.fr marked 3 inline comments as done.Nov 24 2025, 3:51 AM

Implementation looks fine to me, but I defer to bhyve-y folks for final review... in particular, I do wonder if they might prefer some more explicit prefix like unix:/path/to/sock

This revision is now accepted and ready to land.Nov 24 2025, 11:28 PM

I think this is ok, but yes I'd somewhat prefer to have an explicit unix:<path>. That way there's no need to require the path to be absolute.

Added a unix: prefix for UNIX domain sockets

This enables the use of relative paths.
To avoid parsing the configuration value twice, I decided to pass a sa_family_t parameter to the rfb_init function. I hope that is an acceptable way to implement it.

This revision now requires review to proceed.Dec 1 2025, 2:21 PM

Harbormaster completed remote builds in B68955: Diff 167358.Dec 1 2025, 2:21 PM

kevans added inline comments.Dec 1 2025, 2:50 PM

usr.sbin/bhyve/pci_fbuf.c
285	I probably wouldn't assert on these, since `value` is presumably user-supplied -- we can do an EPRINTLN / returrn -1, as in the above error cases.

Replace asserts() with error messages

Harbormaster completed remote builds in B68987: Diff 167410.Dec 2 2025, 3:58 AM

quentin.thebault_defenso.fr marked an inline comment as done.Dec 2 2025, 3:58 AM

In D53814#1233992, @quentin.thebault_defenso.fr wrote:

Added a unix: prefix for UNIX domain sockets

This enables the use of relative paths.
To avoid parsing the configuration value twice, I decided to pass a sa_family_t parameter to the rfb_init function. I hope that is an acceptable way to implement it.

IMO passing the family hint is a perfectly fine option - I can't really picture a better alternative offhand.

usr.sbin/bhyve/pci_fbuf.c
293	This could just be unindented by a level in the containing block, but I don't insist

kevans accepted this revision.Dec 2 2025, 5:05 AM

This revision is now accepted and ready to land.Dec 2 2025, 5:05 AM

Github pull request created: https://github.com/freebsd/freebsd-src/pull/1912

Looks ok to me aside from the one question.

usr.sbin/bhyve/rfb.c
1337	Why unlink it? Shouldn't it be an error if we try to bind the socket and it is already bound?

kevans added inline comments.Dec 2 2025, 2:53 PM

usr.sbin/bhyve/rfb.c
1337	I think the theory is that we don't know if it's bound or not; iirc, bind(2) will fail if the file exists at all (and not just if something's currerntly bound to it).

markj accepted this revision as: markj.Dec 2 2025, 2:55 PM

markj added inline comments.

usr.sbin/bhyve/rfb.c
1337	Ah, right, I was thinking they could be reused like named pipes.

GitHub PR rebased on main.

Closed by commit rGcbc6f7e941e4: bhyve: add UNIX domain socket support to rfb (authored by quentin.thebault_defenso.fr, committed by markj). · Explain WhyJan 8 2026, 4:06 PM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rGcbc6f7e941e4: bhyve: add UNIX domain socket support to rfb.

bhyve: add UNIX domain socket support to rfb
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 167007

usr.sbin/bhyve/bhyve.8

usr.sbin/bhyve/bhyve_config.5

usr.sbin/bhyve/pci_fbuf.c

usr.sbin/bhyve/rfb.c

bhyve: add UNIX domain socket support to rfbClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 167007

usr.sbin/bhyve/bhyve.8

usr.sbin/bhyve/bhyve_config.5

usr.sbin/bhyve/pci_fbuf.c

usr.sbin/bhyve/rfb.c

bhyve: add UNIX domain socket support to rfb
ClosedPublic
Actions

Revision Contents
Changeset List