-
Notifications
You must be signed in to change notification settings - Fork 646
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.
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
Implement VM plugin subsystem #3384
Conversation
DefaultTimeout = 30 * time.Second | ||
|
||
// DefaultPluginDir is the default directory where plugins are stored | ||
DefaultPluginDir = "/usr/local/lib/lima/plugins" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be probably like PREFIX/libexec/lima/plugins/vm
.
The PREFIX must be detected via the os.Executable
path
lima/pkg/usrlocalsharelima/usrlocalsharelima.go
Lines 41 to 43 in 0625d0b
// self: /usr/local/bin/limactl | |
selfDir := filepath.Dir(self) | |
selfDirDir := filepath.Dir(selfDir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively we can look for lima-plugin-vm-<PLUGIN>
in PATH
.
This model is similar to containerd runtime plugins. (containerd-shim-runc-v2
)
} | ||
|
||
message StartVMRequest { | ||
string config = 1; // Serialized config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the serialization format?
string name = 1; | ||
string version = 2; | ||
string description = 3; | ||
repeated string supported_vm_types = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an interesting topic.
- Is there any use case to support multiple VM types in a single plugin? (e.g., "qemu" plugin may support both "qemu" and "qemu-utm" VM types?)
- Also, is there any use case to support the same VM type in multiple plugins?
message StartVMResponse { | ||
bool success = 1; | ||
string message = 2; | ||
bool can_run_gui = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this available in Metadata?
} | ||
|
||
message StopVMRequest { | ||
string instance_id = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "id" different from "name"?
|
||
// Response is a common response type used by helper functions | ||
message Response { | ||
bool success = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably Response
should not be returned on an error?
gRPC has its own error reporting system
https://github.com/grpc/grpc-go/blob/6819ed796fcd0232a46dab21c1b7826aa7f1d561/examples/features/error_details/client/main.go#L55-L65
|
||
option go_package = "github.com/lima-vm/lima/pkg/plugins"; | ||
|
||
service VMDriver { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually this service should also have a method "Fulfill the empty fields such as mountType"
|
||
// GetInstanceDiskPath returns the instance disk file path | ||
func GetInstanceDiskPath(instanceID string) string { | ||
return filepath.Join(GetInstanceDir(instanceID), "disk.img") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path isn't correct https://lima-vm.io/docs/dev/internals/#instance-directory-lima_homeinstance
} | ||
|
||
// GetInstanceDir returns the instance directory path | ||
func GetInstanceDir(instanceID string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
Line 95 in 3a45903
func InstanceDir(name string) (string, error) { |
|
||
// CreateDiskImage creates a disk image with the given size | ||
func CreateDiskImage(path string, sizeGB int) error { | ||
cmd := exec.Command("qemu-img", "create", "-f", "qcow2", path, fmt.Sprintf("%dG", sizeGB)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qemu-img
isn't always available
} | ||
|
||
// GetSSHConfig returns the SSH configuration for the VM | ||
func GetSSHConfig(instanceID string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should utilize https://github.com/lima-vm/lima/tree/master/pkg/sshutil
} | ||
|
||
// WaitForSocket waits for a Unix socket to become available | ||
func WaitForSocket(path string, timeout time.Duration) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just use stdio instead of sockets
https://github.com/moby/buildkit/blob/de56a3c5056341667b5bad71f414ece70b50724f/frontend/gateway/grpcclient/client.go#L1240
|
||
// GetPluginSocketPath returns the plugin socket file path | ||
func GetPluginSocketPath(pluginName string) string { | ||
return filepath.Join("/tmp", fmt.Sprintf("lima-plugin-%s.sock", pluginName)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't allow running multiple Lima instances especially with multiple user accounts on the host.
This is also vulnerable to a race condition.
You can just use stdio to eliminate the sockets
https://github.com/moby/buildkit/blob/de56a3c5056341667b5bad71f414ece70b50724f/frontend/gateway/grpcclient/client.go#L1240
} | ||
|
||
// Create Unix domain socket listener | ||
lis, err := net.Listen("unix", socketPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just use stdio to eliminate the sockets
https://github.com/moby/buildkit/blob/de56a3c5056341667b5bad71f414ece70b50724f/frontend/gateway/grpcclient/client.go#L1240
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably this should be in a place like plugins/vm/qemu/qemu.go
.
} | ||
|
||
// NewQemuPlugin creates a new QEMU plugin | ||
func NewQemuPlugin() *QemuPlugin { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func NewQemuPlugin() *QemuPlugin { | |
func New() *Plugin { |
"qemu" is already implied in the package name
return &QemuPlugin{ | ||
BasePluginServer: framework.NewBasePluginServer( | ||
"qemu", | ||
"1.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably built-in plugins should just use https://github.com/lima-vm/lima/tree/master/pkg/version
"github.com/lima-vm/lima/pkg/limayaml" | ||
"github.com/lima-vm/lima/pkg/plugins" | ||
"github.com/lima-vm/lima/pkg/plugins/framework" | ||
"github.com/lima-vm/lima/pkg/qemu" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually the two "qemu" packages should be unified.
} | ||
|
||
// VZ driver does not support display connections | ||
return &plugins.GetDisplayConnectionResponse{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably gRPC's "UNIMPLEMENTED" error can be used
https://grpc.io/docs/guides/status-codes/
Thanks, let me close this PR until the acceptance of the GSoC, but feel free to continue the conversation. Also, please register yourself in the GSoC's website if you haven't done it yet |
Hi @AkihiroSuda , Thanks for the detailed feedback. I’ll start working on the issues you highlighted, including the plugin subsystem, paths, and QEMU plugin changes. I’ll keep you updated and reach out if I need clarification. I highly appreciate your guidance on the topic as I am fairly new to open source. I’ll also register for GSoC 2025 as suggested. Thank you. |
Decouple Lima’s built‐in VM drivers by implementing a plugin subsystem. VM drivers (qemu, vz, wsl2) will run as separate binaries and communicate with the main Lima binary via a gRPC-based RPC interface.