mantle/platform: add BindMountHostRO Machine Option#4513
Merged
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to bind mount host directories into QEMU machines via MachineOptions and the --qemu-bind-ro flag. It unifies the parsing logic for bind options into the platform package and updates the test harness to support these mounts in external tests. Feedback includes correcting an inconsistency in the help text, addressing a breaking change in flag behavior where a comma-separated source/destination pair is now required, fixing a redundant slice expression, and resolving minor grammatical and typographical errors in error messages and comments.
The statement in the original commit d812406 says: ``` This one doesn't support configuring the path because we can't reliably change the Ignition config here... ``` but we can just modify the config here so let's add destination support and also relocate and reuse the parseBindOpt function from qemuexec.go that's used for --bind-ro over there.
The previous check only rejected AppendKernelArgs. Use reflect.DeepEqual against a zero-value MachineOptions to catch any machine option being set on a non-exclusive test. Written-by: <anthropic/claude-opus-4.6>
This extends what was already supported via the CLI --qemu-bind-ro switch, but to MachineOptions which can be set directly or mapped into from external test metadata.
For testing numad as part of [1]. The thinking is that we'll bind mount in COSA into the VM and run a container based on the COSA rootfs that spanws stress-ng. [1] coreos/fedora-coreos-config#4051
0a07211 to
38c8f7f
Compare
Member
Author
|
/retest |
Member
Author
|
will wait for FCOS releases today to finish up and then I'll merge this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
BindMountHostROwill allow someone to specify host (COSA) paths to mount into the started VM machine. It's very similar to the existing--qemu-bind-rooption today.Part of this work is to enable running the bind mounted COSA filesystem as a container inside our test VMs without having to pull new containers down. See the discussion at coreos/fedora-coreos-config#4051 (comment)
We also here add in
stress-ngsince coreos/fedora-coreos-config#4051 needs it to test numad.See individual commit messages for rationale about each change.