bhyve: add UNIX domain socket support to rfb

[quentin.thebault_defenso.fr]

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 <[e-mail]>
Sponsored by: Defenso
MFC after: 1 week

[kevans]

usr.sbin/bhyve/bhyve.8

927

New sentence must start on a new line

usr.sbin/bhyve/rfb.c

1310

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.

1385–1386

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]

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

[kevans]

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

[markj]

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.

[quentin.thebault_defenso.fr]

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.

[kevans]

usr.sbin/bhyve/pci_fbuf.c

289

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.

[quentin.thebault_defenso.fr]

Replace asserts() with error messages

[kevans]

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

297

This could just be unindented by a level in the containing block, but I don't insist

[quentin.thebault_defenso.fr]

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

[markj]

Looks ok to me aside from the one question.

usr.sbin/bhyve/rfb.c

1338

Why unlink it? Shouldn't it be an error if we try to bind the socket and it is already bound?

[kevans]

usr.sbin/bhyve/rfb.c

1338

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]

usr.sbin/bhyve/rfb.c

1338

Ah, right, I was thinking they could be reused like named pipes.

[quentin.thebault_defenso.fr]

GitHub PR rebased on main.