Skip to content

Be able to start and monitor BHyve virtual machines#250

Open
hannesm wants to merge 25 commits intomainfrom
other-vm-types
Open

Be able to start and monitor BHyve virtual machines#250
hannesm wants to merge 25 commits intomainfrom
other-vm-types

Conversation

@hannesm
Copy link
Collaborator

@hannesm hannesm commented Jan 16, 2026

This is not very well tested yet. EDIT: but a basic FreeBSD VM is starting :) [and console output is visible]

  • Test with a Linux VM (using the grub-bhyve path)
  • Test with multiple network interfaces
  • What is not done is pinning to specific CPU(s). This requires some further change(s), since there's only a single "cpu", but a BHyve VM may use more CPUs.
  • Also, the resources/resource usage needs to be adjusted for these BHyve VMs. At the moment, we have a short exit for /dev/zvol -- not albatross_daemon has a command-line argument --allow-zvol-for - as a temporary workaround (until feature request: separate the block handling (esp. create / dump / set) to a separate daemon #219 is a thing)
  • Only some common and standard things are supported, namely there's no support for -d for grub-bhyve.
  • The restart-on-failure as well needs to be rethought / revised. <- this is fine, if a reboot is issued, the exit code of bhyve is 0, which matches the restart-on-failure. Exit code 1 is for "halt"

Later a path on how to install a virtual machine given you have nothing -- usually done via a boot CD or memstick... this is not very well thought out yet (but if you have an image, just block_set it, and you're done :)

@reynir reynir self-requested a review January 16, 2026 13:47
@hannesm
Copy link
Collaborator Author

hannesm commented Jan 16, 2026

I pushed some more commits, this works now to the extend that a FreeBSD VM with one network and one block device starts fine. More testing is needed, also I updated the initial comment with other things that need to be taken care of (plus the /dev/zvol/ escape in the vmm_resources.ml).

@hannesm
Copy link
Collaborator Author

hannesm commented Jan 22, 2026

This is fine to review. I'd leave the CPU pinning out of this PR. It for sure requires some more testing, esp. with Linux guests.

Copy link
Contributor

@reynir reynir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I wonder if we should guard this behind a --experimental flag? So we can sort of warn people that it might break or change in the near future?

About CPUs and CPU pinning. There doesn't have to be a one-to-one correspondence between cpu sets and numcpu - the OS can schedule the virtual CPUs on the allowed CPUs I suspect. So we could allow multiple --cpu arguments and for unikernels the numcpu is inherently fixed to 1.

About the digest: maybe we need to rethink the data structure so it can vary depending on the type (solo5/bhyve/...) with type appropriate fields. But I'm fine with this especially with an --experimental flag or so.

@hannesm
Copy link
Collaborator Author

hannesm commented Jan 26, 2026

(rebased & force-pushed)

@reynir
Copy link
Contributor

reynir commented Jan 26, 2026

The last two commits were meant to be one. It changes the printing so that the digest field is printed as "digest " for `Solo5 and "bhyve-vmname " for `BHyve. This should hopefully hide to the user that we abuse the same field for both.

@hannesm
Copy link
Collaborator Author

hannesm commented Jan 26, 2026

That's excellent @reynir. I'm working on a patchset for having cpuids in the unikernel/vm context. this is a breaking change though... but it is fine to include here. :)

@hannesm
Copy link
Collaborator Author

hannesm commented Jan 26, 2026

52f59c2 revises the unikernel_info/config/arguments to carry a set of cpuids -- since this modifies the wire format in a non-backwards compatible way, I made the versioning dance...

This is as well supported for unikernels. since even there for a single-cpu unikernel it makes sense that this may be on cpu 4 and 8, so the hypervisor can schedule it depending on load of CPU 4 and 8...

@hannesm
Copy link
Collaborator Author

hannesm commented Jan 26, 2026

CI error on ppc64 (I plan to ignore this):

(cd _build/default && /home/opam/.opam/5.4/bin/ocamlopt.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -I src/.albatross.objs/byte -I src/.albatross.objs/native -I /home/opam/.opam/5.4/lib/asn1-combinators -I /home/opam/.opam/5.4/lib/astring -I /home/opam/.opam/5.4/lib/bigstringaf -I /home/opam/.opam/5.4/lib/checkseum -I /home/opam/.opam/5.4/lib/decompress/de -I /home/opam/.opam/5.4/lib/decompress/zl -I /home/opam/.opam/5.4/lib/domain-name -I /home/opam/.opam/5.4/lib/duration -I /home/opam/.opam/5.4/lib/fmt -I /home/opam/.opam/5.4/lib/fpath -I /home/opam/.opam/5.4/lib/ipaddr -I /home/opam/.opam/5.4/lib/logs -I /home/opam/.opam/5.4/lib/macaddr -I /home/opam/.opam/5.4/lib/metrics -I /home/opam/.opam/5.4/lib/ohex -I /home/opam/.opam/5.4/lib/optint -I /home/opam/.opam/5.4/lib/ptime -cmi-file src/.albatross.objs/byte/vmm_asn.cmi -no-alias-deps -opaque -o src/.albatross.objs/native/vmm_asn.cmx -c -impl src/vmm_asn.ml)
/tmp/build_221c95_dune/camlasm270cda.s: Assembler messages:
/tmp/build_221c95_dune/camlasm270cda.s:8051: Error: operand out of range (0xfffffffffffead49 is not between 0xffffffffffff8000 and 0xffff)

So, somehow vmm_asn.ml isn't in good shape anymore for the assembler / ppc64 backend.

@hannesm
Copy link
Collaborator Author

hannesm commented Jan 26, 2026

Thanks! I wonder if we should guard this behind a --experimental flag? So we can sort of warn people that it might break or change in the near future?

I would like to not have such flags. The issue is that we need to care about backwards compatibility anyways in terms of what is stored on disk (or in certificates). So, an --experimental flag would in my opinion only lead to more conditionals... Also taking into account the number of deployed albatross instances on FreeBSD, it is likely only us needing to care about that. :)

Copy link
Contributor

@reynir reynir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ASN.1 changes are quite large, but looks okay. I didn't review that part as thoroughly.

@ (optional ~label:"startup" int)
@ (required ~label:"fail-behaviour" fail_behaviour)
@ (required ~label:"cpuid" int)
@ (required ~label:"cpuids" (set_of int))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the ASN.1 is it possible to make this

required ~label:"cpuids" (choice2 int (set_of int))

and have backwards compatibility?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sure. What I'm aiming for is "old clients should be able to talk to new servers". So, an old client data would then be accepted by the new server.

But, when the old client requests a unikernel_info, it would as well receive this new ASN with the set_of, and then fail to parse it.

Thus (and to keep unikernel_config and unikernel_info in sync), I prefer to change it as done here. The old client now sends unikernel_info, and the server decodes it as old_unikernel_info5 and replies with Old_unikernel_info5 -- in a form that the old client understands the layout and is able to deal with it.

Certainly, the other compatibility -- a new client talking to an old server -- is not given. Somehow I assume that servers are updated to the latest version ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants