-
Notifications
You must be signed in to change notification settings - Fork 667
[WIP]: Create a Plugin System for Lima #3573
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
base: master
Are you sure you want to change the base?
Conversation
77877e0
to
7a082d2
Compare
cmd/limactl/start.go
Outdated
@@ -54,6 +56,9 @@ $ limactl create --set='.cpus = 2 | .memory = "2GiB"' | |||
To see the template list: | |||
$ limactl create --list-templates | |||
|
|||
To see the drivers list: | |||
$ limactl create --list-drivers |
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.
The example currently just focuses on templates, not on drivers.
@@ -16,7 +16,7 @@ import ( | |||
"github.com/docker/go-units" | |||
"github.com/lima-vm/go-qcow2reader" | |||
"github.com/lima-vm/lima/pkg/nativeimgutil" | |||
"github.com/lima-vm/lima/pkg/qemu/imgutil" | |||
"github.com/lima-vm/lima/pkg/qemuimgutil" |
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 should follow:
Not an urgent topic though
pkg/builtins/drivers.go
Outdated
// SPDX-FileCopyrightText: Copyright The Lima Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package builtins |
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.
builitins should be in cmd/limactl/main*.go
pkg/builtins/drivers.go
Outdated
import ( | ||
// Import all built-in drivers to register them in the registry. | ||
_ "github.com/lima-vm/lima/pkg/driver/qemu" | ||
_ "github.com/lima-vm/lima/pkg/driver/vz" |
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.
should be in main_darwin.go
pkg/builtins/drivers.go
Outdated
// Import all built-in drivers to register them in the registry. | ||
_ "github.com/lima-vm/lima/pkg/driver/qemu" | ||
_ "github.com/lima-vm/lima/pkg/driver/vz" | ||
_ "github.com/lima-vm/lima/pkg/driver/wsl2" |
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.
should be in main_windows.go
pkg/driver/driver.go
Outdated
|
||
DeleteSnapshot(_ context.Context, tag string) error | ||
// Snapshot defines operations for managing snapshots. | ||
type Snapshot interface { |
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 represents a manager of snapshots, not a snapshot itself.
So it should be named like SnapshotManager
or Snapshotter
?
The latter one might be confusing, as it conflicts with the terminology of containerd, though.
pkg/driver/vz/vz_driver_darwin.go
Outdated
} | ||
|
||
func (l *LimaVzDriver) ListSnapshots(_ context.Context) (string, error) { | ||
return "", errors.New("unimplemented") |
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.
The common ErrImplemented
should be defined so that it can be compared with errors.Is
pkg/driver/vz/vz_driver_others.go
Outdated
@@ -0,0 +1,118 @@ | |||
//go:build !darwin || no_vz |
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.
The package shouldn't be compiled at all on non-Darwin
3f1de89
to
e53a368
Compare
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
…plugin related functions Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
…rnal drivers Signed-off-by: Ansuman Sahoo <[email protected]>
…internal plugins Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
…rface Signed-off-by: Ansuman Sahoo <[email protected]>
…ement common error Signed-off-by: Ansuman Sahoo <[email protected]>
e53a368
to
4630452
Compare
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
|
||
message Empty {} | ||
|
||
message InstanceConfig { |
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.
Can we just pass the byte array of YAML (or JSON), so that we do not need to sync the YAML struct with this proto definition?
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.
Sure we can do that!
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
|
||
rpc GetVSockPort(google.protobuf.Empty) returns (GetVSockPortResponse); | ||
rpc GetVirtioPort(google.protobuf.Empty) returns (GetVirtioPortResponse); | ||
} |
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.
CanRunGUI
, Name
, GetVSockPort
, and GetVirtioPort
could be consolidated into a single Info()
rpc call.
Not urgent.
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.
@AkihiroSuda Should I also consolidate these functions in the driver.Driver
interface, in order to maintain consistency?
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.
Yes
…tion Signed-off-by: Ansuman Sahoo <[email protected]>
…call Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
…function Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
func (d *DriverClient) Initialize(ctx context.Context) error { | ||
d.logger.Debug("Initializing driver instance") | ||
|
||
connCtx, cancel := context.WithTimeout(ctx, 5*time.Second) |
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.
timeout should be specified in the caller
for { | ||
n, err := conn.Read(buf) | ||
if err != nil { | ||
if err == io.EOF { |
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.
errors.Is
is more robust
…ocess Signed-off-by: Ansuman Sahoo <[email protected]>
Signed-off-by: Ansuman Sahoo <[email protected]>
…in SetConfig() Signed-off-by: Ansuman Sahoo <[email protected]>
605e45e
to
2a82bf3
Compare
Important
This PR is only for preview purposes and not meant to be merged. Later, the commits can be cherry-picked and raised as separate small PRs.
This PR creates a Plugin System for drivers which can be embedded in the main binary or built as separate binaries, which then communicate with the main binary using gRPC. This work is done as part of my GSoC '25 task.