feat(shim): forward ro bind-mount option to virtio-fs share#205
Open
ndeloof wants to merge 1 commit into
Open
Conversation
2 tasks
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for read-only virtio-fs shares by preferring libkrun's krun_add_virtiofs3 symbol (with a fallback to the legacy krun_add_virtiofs) and threading the ro mount option from the OCI spec through to the sandbox.
Changes:
- Introduce an optional-symbol resolution mechanism (
,optionalC tag +registerOptionalLibFunc) so missing libkrun exports do not panic. - Update
AddVirtiofsto take areadonlyflag and route tokrun_add_virtiofs3when available, logging a fallback notice otherwise. - Parse
ro/rwmount options inbindMounterand propagate the resultingReadonlyflag throughSandboxOpts.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/vm/libkrun/krun.go | Adds AddVirtiofs3 binding, optional-tag parsing, and readonly-aware AddVirtiofs. |
| internal/vm/libkrun/krun_test.go | New tests for tag parsing and v3/legacy dispatch behavior. |
| internal/vm/libkrun/dlfcn_unix.go | Adds registerOptionalLibFunc for Unix using purego.Dlsym. |
| internal/vm/libkrun/dlfcn_windows.go | Adds registerOptionalLibFunc for Windows using GetProcAddress. |
| internal/vm/libkrun/instance.go | Plumbs MountConfig.Readonly into AddVirtiofs. |
| internal/shim/task/mount.go | Parses ro/rw options and passes readonly to sandbox.WithFS. |
| internal/shim/task/mount_test.go | Adds readonly bind-mount cases and verifies SandboxOpts filesystems. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ndeloof
force-pushed
the
virtiofs-ro-host-edge
branch
from
2f60004 to
2bb7246
Compare
Member
I don't think we need the fallback, we should just build libkrun at a specific version and support that as the minimum. The prepackaged builds have not been reliable. |
akerouanton
reviewed
Jun 1, 2026
ndeloof
force-pushed
the
virtiofs-ro-host-edge
branch
from
2bb7246 to
13014fa
Compare
akerouanton
reviewed
Jun 2, 2026
Member
akerouanton
left a comment
akerouanton
left a comment
There was a problem hiding this comment.
Left a nit, otherwise LGTM.
The ext4 mount path in mount.go already parses `ro` from the OCI mount options; the bind-mount path silently dropped it. Plumb the flag through bindMounter and vmInstance.AddFS so the virtio-fs share itself is marked read-only at the host edge via krun_add_virtiofs3 (libkrun >= 1.18), giving us defense-in-depth on top of the guest-side `ro` bind mount and opening the door to VMM-side optimizations on immutable shares. When the loaded libkrun is older and does not export krun_add_virtiofs3, the binding falls back to krun_add_virtiofs with an info log and enforcement stays on the guest mount options exactly as today. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
ndeloof
force-pushed
the
virtiofs-ro-host-edge
branch
from
13014fa to
6db5a00
Compare
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.
Summary
roOCI mount option inbindMounter.FromBundleand carry it onbindMount, mirroring what the ext4 branch in the same file already does.sandbox.WithFStovmInstance.AddFS, which now appliesvm.MountOptsbefore calling into libkrun.krun_add_virtiofs3symbol (libkrun >= 1.18) via a smallregisterOptionalLibFunchelper and a,optionalstruct-tag flag; routeAddVirtiofsthrough it when present so the share is marked read-only at the host edge.krun_add_virtiofswith a single warn log when the v3 symbol is not exported; guest-sideroenforcement via OCI mount options is unchanged.parseCTagand theAddVirtiofsv3-vs-legacy matrix (prefers v3, falls back when v3 is nil, surfaces non-zero return codes on both paths, errors when neither symbol is bound).Behavior
< 1.18(nokrun_add_virtiofs3exported, e.g. the v1.17.4 currently pinned)rothrough the OCI mount options exactly as today.>= 1.18(krun_add_virtiofs3exported)Test plan
go build ./...go test ./internal/shim/task/... ./internal/vm/libkrun/...