refactor: support external drivers for the recent changes #3910
+693
−326
Conversation
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
AkihiroSuda
added
gsoc/2025
|
Please rebase |
AkihiroSuda
reviewed
Aug 26, 2025
cmd/lima-driver-vz/main_darwin.go
Outdated
| // To be used as an external driver for Lima. | ||
| func main() { | ||
| server.Serve(context.Background(), vz.New()) | ||
| server.Serve(vz.New()) |
AkihiroSuda
reviewed
Aug 26, 2025
pkg/driver/external/server/server.go
Outdated
|
|
||
| var payload []byte | ||
| if err := decoder.Decode(&payload); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error encoding response: %v\n", err) |
There was a problem hiding this comment.
If we change this to logrus, it will not be clean. For e.g:
➜ lima2 git:(refactor/support-external-drivers) ✗ ./_output/bin/limactl edit default2 --mount-type virtiofs
FATA[0000] failed to resolve vm for "/Users/ansumansahoo/.lima/default2/lima.yaml": external driver stderr:
time="2025-08-27T01:51:33+05:30" level=error msg="Failed to accept config: config not supported by the QEMU driver:
field `mountType` must be \"reverse-sshfs\" or \"9p\" for QEMU driver on non-Linux, got \"virtiofs\""
Using fmt.Fprintf results in a clean error:
➜ lima2 git:(refactor/support-external-drivers) ✗ ./_output/bin/limactl edit default2 --mount-type virtiofs
FATA[0000] failed to resolve vm for "/Users/ansumansahoo/.lima/default2/lima.yaml":
external driver stderr: Failed to accept config: config not supported by the QEMU driver:
field `mountType` must be "reverse-sshfs" or "9p" for QEMU driver on non-Linux, got "virtiofs"
And since it will be never be stored in the driver.stderr.log
AkihiroSuda
reviewed
Aug 26, 2025
pkg/driverutil/vm.go
Outdated
| } | ||
|
|
||
| func handlePreConfiguredDriverAction(y *limatype.LimaYAML, extDriverPath, filePath string) error { | ||
| cmd := exec.CommandContext(context.Background(), extDriverPath, "--pre-driver-action") |
There was a problem hiding this comment.
ctx should be passed from the caller
AkihiroSuda
reviewed
Aug 26, 2025
pkg/driverutil/vm.go
Outdated
| } | ||
|
|
||
| func handleInspectStatusAction(inst *limatype.Instance, extDriverPath string) (string, error) { | ||
| cmd := exec.CommandContext(context.Background(), extDriverPath, "--inspect-status") |
AkihiroSuda
reviewed
Aug 26, 2025
pkg/driver/driver.go
Outdated
| VirtioPort string `json:"virtioPort"` | ||
| InstanceDir string `json:"instanceDir,omitempty"` | ||
| DriverName string `json:"driverName"` | ||
| CanRunGUI bool `json:"canRunGui,omitempty"` |
There was a problem hiding this comment.
This can be moved to DriverFeatures
unsuman
force-pushed
the
refactor/support-external-drivers
branch
from
7e99f45 to
3cf0d16
Compare
|
I will open the PR after some time, working on making the pre-validation action error handling better. |
unsuman
force-pushed
the
refactor/support-external-drivers
branch
4 times, most recently
from
8c27373 to
3990779
Compare
Signed-off-by: Ansuman Sahoo <[email protected]>
unsuman
force-pushed
the
refactor/support-external-drivers
branch
from
3990779 to
d4dc0a5
Compare
AkihiroSuda
reviewed
Aug 27, 2025
There was a problem hiding this comment.
Next time please split a commit for this
AkihiroSuda
reviewed
Aug 27, 2025
| type DriverFeatures struct { | ||
| DynamicSSHAddress bool `json:"dynamicSSHAddress"` | ||
| SkipSocketForwarding bool `json:"skipSocketForwarding"` | ||
| DriverName string `json:"driverName"` |
There was a problem hiding this comment.
Name does not seem to be a feature
There was a problem hiding this comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Note
This is the last PR of the three PRs, which depends on #3901. Merging all will close #3769.
Key Changes
.protofiles to incorporate with the driver interface.FillConfig()andAcceptConfig()are both pre-configured driver operations, I have implemented stdio piping transport using json betweenlimactland the driver binary. The driver binary can know thatlimactlis calling for a pre-grpc action by a boolean flag--pre-driver-action.One more thing, we tend to inspect instances a lot throughout the codebase(even for simply listing Lima instances through
limactl ls) so I have also implemented the same stdio transport mechanism forInspectStatus()so that it saves us time and resource but it is an add-on, let me know if we don't need this then I can revert the changes!