-
Notifications
You must be signed in to change notification settings - Fork 712
[WIP]: Support for libkrun
using krunkit
#4137
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// SPDX-FileCopyrightText: Copyright The Lima Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package main | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/lima-vm/lima/v2/pkg/driver/external/server" | ||
"github.com/lima-vm/lima/v2/pkg/driver/krunkit" | ||
) | ||
|
||
// To be used as an external driver for Lima. | ||
func main() { | ||
server.Serve(context.Background(), krunkit.New()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
// SPDX-FileCopyrightText: Copyright The Lima Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package krunkit | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"fmt" | ||
"os" | ||
"os/exec" | ||
"path/filepath" | ||
"strconv" | ||
|
||
"github.com/docker/go-units" | ||
"github.com/lima-vm/lima/v2/pkg/imgutil/proxyimgutil" | ||
"github.com/lima-vm/lima/v2/pkg/iso9660util" | ||
"github.com/lima-vm/lima/v2/pkg/limatype" | ||
"github.com/lima-vm/lima/v2/pkg/limatype/filenames" | ||
"github.com/lima-vm/lima/v2/pkg/limayaml" | ||
"github.com/lima-vm/lima/v2/pkg/networks" | ||
) | ||
|
||
const ( | ||
logLevelInfo = "3" | ||
KrunEfi = "krun-efi" // efi variable store | ||
) | ||
|
||
// Cmdline constructs the command line arguments for krunkit based on the instance configuration. | ||
func Cmdline(inst *limatype.Instance) (*exec.Cmd, error) { | ||
var args = []string{ | ||
"--memory", strconv.Itoa(2048), | ||
"--cpus", fmt.Sprintf("%d", *inst.Config.CPUs), | ||
"--device", fmt.Sprintf("virtio-serial,logFilePath=%s", filepath.Join(inst.Dir, filenames.SerialLog)), | ||
"--krun-log-level", logLevelInfo, | ||
"--restful-uri", "none://", | ||
|
||
// First virtio-blk device is the boot disk | ||
"--device", fmt.Sprintf("virtio-blk,path=%s,format=raw", filepath.Join(inst.Dir, filenames.DiffDisk)), | ||
"--device", fmt.Sprintf("virtio-blk,path=%s", filepath.Join(inst.Dir, filenames.CIDataISO)), | ||
} | ||
|
||
// TODO: socket_vmnet and ssh not working | ||
networkArgs, err := buildNetworkArgs(inst) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to build network arguments: %w", err) | ||
} | ||
args = append(args, networkArgs...) | ||
|
||
return exec.CommandContext(context.Background(), "krunkit", args...), nil | ||
} | ||
|
||
func buildNetworkArgs(inst *limatype.Instance) ([]string, error) { | ||
var args []string | ||
nwCfg, err := networks.LoadConfig() | ||
if err != nil { | ||
return nil, err | ||
} | ||
socketVMNetOk, err := nwCfg.IsDaemonInstalled(networks.SocketVMNet) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's rename the argument to make the code more clear:
|
||
if err != nil { | ||
return nil, err | ||
} | ||
if socketVMNetOk { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets change this to: if ! socketVMNetInstalled { |
||
sock, err := networks.Sock(networks.ModeShared) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why hard code shared mode? this driver should be able to use both shared and bridged modes. It is fined to start with shared mode initially, but the final driver should support both shared and bridged. Since we really require socket_vment, we can fail if the instance is not using vment. |
||
if err != nil { | ||
return nil, err | ||
} | ||
networkArg := fmt.Sprintf("virtio-net,type=unixstream,path=%s,mac=%s", | ||
sock, | ||
limayaml.MACAddress(inst.Dir), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks correct now, so krunkit should be connected via socket_vment with this change and works similar to qemu. |
||
) | ||
args = append(args, "--device", networkArg) | ||
|
||
return args, nil | ||
} | ||
|
||
return args, errors.New("socket_vmnet is not installed") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returning the error at the end is not good. The common pattern we should always follow is the happy path:
This makes error handling much easier to understand since the error message is close to the check triggering it. It also help to be sure we handled all errors, and it keeps indentation simple and flat, more important in go, forcing tabs (8 spaces) for every indentation level. |
||
} | ||
|
||
func EnsureDisk(ctx context.Context, inst *limatype.Instance) error { | ||
diffDisk := filepath.Join(inst.Dir, filenames.DiffDisk) | ||
if _, err := os.Stat(diffDisk); err == nil || !errors.Is(err, os.ErrNotExist) { | ||
// disk is already ensured | ||
return err | ||
} | ||
|
||
diskUtil := proxyimgutil.NewDiskUtil(ctx) | ||
|
||
baseDisk := filepath.Join(inst.Dir, filenames.BaseDisk) | ||
|
||
diskSize, _ := units.RAMInBytes(*inst.Config.Disk) | ||
if diskSize == 0 { | ||
return nil | ||
} | ||
isBaseDiskISO, err := iso9660util.IsISO9660(baseDisk) | ||
if err != nil { | ||
return err | ||
} | ||
if isBaseDiskISO { | ||
// Create an empty data volume (sparse) | ||
diffDiskF, err := os.Create(diffDisk) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = diskUtil.MakeSparse(ctx, diffDiskF, 0) | ||
if err != nil { | ||
diffDiskF.Close() | ||
return fmt.Errorf("failed to create sparse diff disk %q: %w", diffDisk, err) | ||
} | ||
return diffDiskF.Close() | ||
} | ||
|
||
// Krunkit also supports qcow2 disks but raw is faster to create and use. | ||
if err = diskUtil.ConvertToRaw(ctx, baseDisk, diffDisk, &diskSize, false); err != nil { | ||
return fmt.Errorf("failed to convert %q to a raw disk %q: %w", baseDisk, diffDisk, err) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. krunkit support qcow2 disks but using raw disk is likely to perform better. We can add a comment here. |
||
return err | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,209 @@ | ||
// SPDX-FileCopyrightText: Copyright The Lima Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package krunkit | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net" | ||
"os" | ||
"os/exec" | ||
"path/filepath" | ||
"syscall" | ||
"time" | ||
|
||
"github.com/sirupsen/logrus" | ||
|
||
"github.com/lima-vm/lima/v2/pkg/driver" | ||
"github.com/lima-vm/lima/v2/pkg/executil" | ||
"github.com/lima-vm/lima/v2/pkg/limatype" | ||
"github.com/lima-vm/lima/v2/pkg/limatype/filenames" | ||
"github.com/lima-vm/lima/v2/pkg/ptr" | ||
) | ||
|
||
type LimaKrunDriver struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need the Lima prefix? Do we use LimaQemuDriver for the qemu driver? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes |
||
Instance *limatype.Instance | ||
SSHLocalPort int | ||
|
||
krunkitCmd *exec.Cmd | ||
krunkitWaitCh chan error | ||
} | ||
|
||
var _ driver.Driver = (*LimaKrunDriver)(nil) | ||
|
||
func New() *LimaKrunDriver { | ||
return &LimaKrunDriver{} | ||
} | ||
|
||
func (l *LimaKrunDriver) Configure(inst *limatype.Instance) *driver.ConfiguredDriver { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we use If we use this in other drivers we should really change to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes it can be another issue but can be done for krunkit! |
||
l.Instance = inst | ||
l.SSHLocalPort = inst.SSHLocalPort | ||
|
||
return &driver.ConfiguredDriver{ | ||
Driver: l, | ||
} | ||
} | ||
|
||
func (l *LimaKrunDriver) CreateDisk(ctx context.Context) error { | ||
return EnsureDisk(ctx, l.Instance) | ||
} | ||
|
||
func (l *LimaKrunDriver) Start(ctx context.Context) (chan error, error) { | ||
krunCmd, err := Cmdline(l.Instance) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to construct krunkit command line: %w", err) | ||
} | ||
krunCmd.SysProcAttr = executil.BackgroundSysProcAttr | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commenting why we need this will be helpful, you can use the same comment used for other drivers. |
||
|
||
logPath := filepath.Join(l.Instance.Dir, "krun.log") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "krunkit.log" to match the driver name. |
||
logfile, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0644) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to open krunkit logfile: %w", err) | ||
} | ||
krunCmd.Stderr = logfile | ||
|
||
logrus.Infof("Starting krun VM (hint: to watch the progress, see %q)", logPath) | ||
logrus.Debugf("krunCmd.Args: %v", krunCmd.Args) | ||
|
||
if err := krunCmd.Start(); err != nil { | ||
logfile.Close() | ||
return nil, err | ||
} | ||
|
||
pidPath := filepath.Join(l.Instance.Dir, filenames.PIDFile(*l.Instance.Config.VMType)) | ||
if err := os.WriteFile(pidPath, []byte(fmt.Sprintf("%d\n", krunCmd.Process.Pid)), 0644); err != nil { | ||
logrus.WithError(err).Warn("Failed to write PID file") | ||
} | ||
|
||
l.krunkitCmd = krunCmd | ||
l.krunkitWaitCh = make(chan error, 1) | ||
go func() { | ||
defer func() { | ||
logfile.Close() | ||
os.RemoveAll(pidPath) | ||
close(l.krunkitWaitCh) | ||
}() | ||
l.krunkitWaitCh <- krunCmd.Wait() | ||
}() | ||
|
||
return l.krunkitWaitCh, nil | ||
} | ||
|
||
func (l *LimaKrunDriver) Stop(ctx context.Context) error { | ||
if l.krunkitCmd == nil { | ||
return nil | ||
} | ||
|
||
if err := l.krunkitCmd.Process.Signal(syscall.SIGTERM); err != nil { | ||
logrus.WithError(err).Warn("Failed to send interrupt signal") | ||
} | ||
|
||
timeout := time.After(30 * time.Second) | ||
select { | ||
case <-l.krunkitWaitCh: | ||
return nil | ||
case <-timeout: | ||
if err := l.krunkitCmd.Process.Kill(); err != nil { | ||
return err | ||
} | ||
|
||
<-l.krunkitWaitCh | ||
return nil | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to wait for the process after killing it. Otherwise it will remain a zombie until this process terminates. |
||
} | ||
|
||
func (l *LimaKrunDriver) Delete(ctx context.Context) error { | ||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) InspectStatus(ctx context.Context, inst *limatype.Instance) string { | ||
return "" | ||
} | ||
|
||
func (l *LimaKrunDriver) RunGUI() error { | ||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) ChangeDisplayPassword(ctx context.Context, password string) error { | ||
return fmt.Errorf("display password change not supported by krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) DisplayConnection(ctx context.Context) (string, error) { | ||
return "", fmt.Errorf("display connection not supported by krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) CreateSnapshot(ctx context.Context, tag string) error { | ||
return fmt.Errorf("snapshots not supported by krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) ApplySnapshot(ctx context.Context, tag string) error { | ||
return fmt.Errorf("snapshots not supported by krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) DeleteSnapshot(ctx context.Context, tag string) error { | ||
return fmt.Errorf("snapshots not supported by krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) ListSnapshots(ctx context.Context) (string, error) { | ||
return "", fmt.Errorf("snapshots not supported by krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) Register(ctx context.Context) error { | ||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) Unregister(ctx context.Context) error { | ||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) ForwardGuestAgent() bool { | ||
return true | ||
} | ||
|
||
func (l *LimaKrunDriver) GuestAgentConn(ctx context.Context) (net.Conn, string, error) { | ||
return nil, "", fmt.Errorf("guest agent connection not implemented for krun driver") | ||
} | ||
|
||
func (l *LimaKrunDriver) Validate(ctx context.Context) error { | ||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) FillConfig(ctx context.Context, cfg *limatype.LimaYAML, filePath string) error { | ||
if cfg.MountType == nil { | ||
cfg.MountType = ptr.Of(limatype.VIRTIOFS) | ||
} else { | ||
*cfg.MountType = limatype.VIRTIOFS | ||
} | ||
|
||
cfg.VMType = ptr.Of("krunkit") | ||
|
||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) BootScripts() (map[string][]byte, error) { | ||
return nil, nil | ||
} | ||
|
||
func (l *LimaKrunDriver) Create(ctx context.Context) error { | ||
return nil | ||
} | ||
|
||
func (l *LimaKrunDriver) Info() driver.Info { | ||
var info driver.Info | ||
info.Name = "krunkit" | ||
if l.Instance != nil && l.Instance.Dir != "" { | ||
info.InstanceDir = l.Instance.Dir | ||
} | ||
|
||
info.Features = driver.DriverFeatures{ | ||
DynamicSSHAddress: false, | ||
SkipSocketForwarding: false, | ||
CanRunGUI: false, | ||
} | ||
return info | ||
} | ||
|
||
func (l *LimaKrunDriver) SSHAddress(ctx context.Context) (string, error) { | ||
return "127.0.0.1", nil | ||
} |
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.
Worth a comment that this is the boot disk. The order of the device matters.