Skip to content

tests/ci: use bhyve for arm64 host and guest by default #1815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

tests/ci: use bhyve for arm64 host and guest by default #1815

wants to merge 6 commits into from
+58 −40

Conversation

svmhdvn
Copy link
Contributor

@svmhdvn svmhdvn commented Aug 19, 2025

See full commit stack for incremental details and explanations.

@svmhdvn
Copy link
Contributor Author

svmhdvn commented Aug 19, 2025 •
edited
Loading

@lwhsu @5u623l20 switching to github for the time being since it's a lot easier to reorganize commits and give full context automatically.

@lwhsu
Copy link
Member

lwhsu commented Aug 21, 2025

I'm still reading and thinking. Just a quick response: I think we can drop i386 (Makefile.i386 file) as it's also disabled in main branch. I don't think we would MFC those test/ci stuff as older branches can still use the old mechanism.

lwhsu
lwhsu reviewed Aug 21, 2025
View reviewed changes
tests/ci/Makefile.aarch64 Outdated
QEMU_MAX_MEM_SIZE=64

portinstall-aarch64: portinstall-pkg .PHONY
.if !exists(/usr/local/share/u-boot/u-boot-qemu-arm64/u-boot.bin)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this part also needs to be guarded by USE_QEMU ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied the feedback in v2.

@lwhsu
Copy link
Member

lwhsu commented Aug 21, 2025

Moving bhyve bits into Makefile.{aarch64,amd64} seems introducing some code duplication, perhaps the difference part like -o bootrom can be set as a variable in those files like BHYVE_ADDITIONAL_PARAMETERS and the common parts can still be Makefile?

@svmhdvn
Copy link
Contributor Author

svmhdvn commented Aug 22, 2025

Moving bhyve bits into Makefile.{aarch64,amd64} seems introducing some code duplication, perhaps the difference part like -o bootrom can be set as a variable in those files like BHYVE_ADDITIONAL_PARAMETERS and the common parts can still be Makefile?

The difference is quite large:

  • bhyveload is not supported yet on arm64 (i.e. difference in bootrom loading)
  • lpc is not supported on arm64 (i.e. difference in devices)
  • -o console is used on arm64 (i.e. difference in output configuration)

There is not too much code duplication here in the grand scheme of things, and it keeps things simpler by relying on less variable indirection.

@svmhdvn svmhdvn force-pushed the push-rstnsvuzltmw branch 2 times, most recently from affba5b to 01f28ec Compare August 27, 2025 15:35
@svmhdvn
Copy link
Contributor Author

svmhdvn commented Aug 27, 2025

Fixed merge conflict in v3.

@svmhdvn
tests/ci: set chmod 0755 on META_DIROUT
ba7d49b 
Currently, since META_DIROUT is created by mktemp(1), it is 0600 by default. 
This makes it slightly annoying to copy out test-report.* files from a CI run.

Signed-off-by: Siva Mahadevan <[email protected]>
Sponsored by: The FreeBSD Foundation
@svmhdvn
tests/ci: also use CIDISK for image output
ec96ce8 
This sets up future work to allow multiple concurrent
CI runs, each possibly with their own image file.

Signed-off-by: Siva Mahadevan <[email protected]>
Sponsored by: The FreeBSD Foundation
@svmhdvn
tests/ci: only install qemu-user-static if QEMUSTATIC is set
31f0d88 
In an upcoming patch, arm64 hosts will default to using bhyve to run CI. 
However, emulators/qemu-user-static does not exist for arm64, and is not needed 
when the target is also arm64.

This patch ensures that qemu-user-static will only be requested for install if QEMUSTATIC is set.

Signed-off-by: Siva Mahadevan <[email protected]>
Sponsored by: The FreeBSD Foundation
@svmhdvn
tests/ci: simplify top-level 'ci' target
f28d1c0 
This patch removes the ci-set-*-var targets and conditionally runs ci-extract-meta
only if CITYPE=full.

The intended external interface into the tests/ci Makefile infrastructure already
requires CITYPE to be set (either through its default CITYPE?=full line, or
through the Make command line). Thus, the ci-set-*-var targets don't
functionally do anything.

Signed-off-by: Siva Mahadevan <[email protected]>
Sponsored by: The FreeBSD Foundation
@svmhdvn
tests/ci: separate ci-runtest into qemu and bhyve variants
77fa26d 
Bhyve is only supported on amd64/amd64, amd64/i386, and
arm64/arm64. Furthermore, amd64 and arm64 have different
command-line arguments and requirements for running bhyve.

This patch separates the bhyve version of ci-runtest into
architecture-specific Makefiles.

Signed-off-by: Siva Mahadevan <[email protected]>
Sponsored by: The FreeBSD Foundation
@svmhdvn
tests/ci: use bhyve for arm64 host and guest
844cb87 
Signed-off-by: Siva Mahadevan <[email protected]>
Sponsored by: The FreeBSD Foundation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lwhsu lwhsu lwhsu left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@svmhdvn @lwhsu