Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/unikontainers/hypervisors/qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ func (q *Qemu) Path() string {
func (q *Qemu) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
qemuMem := BytesToStringMB(args.MemSizeB)
cmdString := q.binaryPath + " -m " + qemuMem + "M"
cmdString += " -L /usr/share/qemu" // Set the path for qemu bios/data
cmdString += " -cpu host" // Choose CPU
cmdString += " -enable-kvm" // Enable KVM to use CPU virt extensions
cmdString += " -L /usr/share/qemu" // Set the path for qemu bios/data
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes seem unrelated. Maybe we can drop them to keep the PR focused on the feature?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for these changes was because of lint errors, when I run make lint it produces these output.

.....
INFO [runner] linters took 1.360433664s with stages: goanalysis_metalinter: 1.345219838s 
pkg/unikontainers/hypervisors/qemu.go:61:1: File is not properly formatted (gofmt)
        cmdString += " -L /usr/share/qemu" // Set the path for qemu bios/data
^
pkg/unikontainers/unikernels/hermit_rs.go:83:3: ineffectual assignment to deviceArgs (ineffassign)
                deviceArgs := " -device " + device + ",netdev=net0"
                ^
2 issues:
* gofmt: 1
* ineffassign: 1
INFO File cache stats: 2 entries of total size 7.6KiB 
INFO Memory: 194 samples, avg is 44.2MB, max is 204.4MB 

So inorder to fix that, I did run

gofmt -w pkg/unikontainers/hypervisors/qemu.go

Thats what led to these changes. However, considering they are not part of the feature, I am going to revert them.

cmdString += " -cpu host" // Choose CPU
cmdString += " -enable-kvm" // Enable KVM to use CPU virt extensions
cmdString += " -display none -vga none -serial stdio -monitor null" // Disable graphic output

if args.VCPUs > 0 {
Expand Down
148 changes: 148 additions & 0 deletions pkg/unikontainers/unikernels/hermit_rs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// Copyright (c) 2023-2026, Nubificus LTD
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package unikernels

import (
"fmt"
"runtime"
"strings"

"github.com/urunc-dev/urunc/pkg/unikontainers/types"
)

const HermitUnikernel string = "hermit"

type Hermit struct {
Command string
Monitor string
Net HermitNet
}

type HermitNet struct {
Address string
Mask int
Gateway string
}

func (h *Hermit) CommandString() (string, error) {
kernelArgs := make([]string, 0, 2)

if h.Net.Address != "" {
kernelArgs = append(kernelArgs, fmt.Sprintf("ip=%s/%d", h.Net.Address, h.Net.Mask))
}
if h.Net.Gateway != "" {
kernelArgs = append(kernelArgs, fmt.Sprintf("gateway=%s", h.Net.Gateway))
}

args := strings.Join(kernelArgs, " ")
appArgs := strings.TrimSpace(h.Command)

switch {
case args != "" && appArgs != "":
return args + " -- " + appArgs, nil
case args != "":
return args, nil
default:
return appArgs, nil
}
}

func (h *Hermit) SupportsBlock() bool {
return false
}

func (h *Hermit) SupportsFS(fsType string) bool {
switch fsType {
case "initrd":
return true
case "virtiofs":
return true
default:
return false
}
Comment on lines +67 to +74
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
switch fsType {
case "initrd":
return true
case "virtiofs":
return true
default:
return false
}
return fsType == "initrd" || fsType == "virtiofs"

nit: I think we could do this instead.

}

func (h *Hermit) MonitorNetCli(ifName string, mac string) string {
switch h.Monitor {
case "qemu":
netdev := fmt.Sprintf(" -netdev tap,id=net0,ifname=%s,script=no,downscript=no", ifName)

device := "virtio-net-pci"
var deviceArgs string

// QEMU on x86_64 typically uses virtio-net-pci.
// On arm64 virtio-net-device is the safer default.
if runtime.GOARCH == "arm64" {
device = "virtio-net-device"
deviceArgs = " -device " + device + ",netdev=net0"
} else {
deviceArgs = " -device " + device + ",netdev=net0,disable-legacy=on"
}

if mac != "" {
deviceArgs += ",mac=" + mac
}

return netdev + deviceArgs
default:
return ""
}
}

func (h *Hermit) MonitorBlockCli() []types.MonitorBlockArgs {
return nil
}

func (h *Hermit) MonitorCli() types.MonitorCliArgs {
switch h.Monitor {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Looks like we only check h.Monitor == "qemu". Do we need a switch here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I am using a switch statement here is because, within the urunc codebase responsible for adding a new unikernel type, I didnt find any specific area a unikernel can define the specific VM Monitors is supports. For that reason, I think h.Monitor can vary and that means for we need to filter out the Monitors we specifically support(in this case qemu) so that we can apply the appropriate MonitorCliArgs.

I could replace this with another filtering method, but I think switch is appropriate for now and it will be simple to add another Monitor incase its tested to be supported by Hermit-rs in the future.

However, that was my initial reason for implementing it that way. Incase am mistaken I am happily willing to change it.

I also took inspiration from how it was implemented in the Mewz unikernel support file here:

switch m.Monitor {

case "qemu":
// isa-debug-exit is x86/QEMU friendly.
// For arm64, keep it minimal.
if runtime.GOARCH == "arm64" {
return types.MonitorCliArgs{
OtherArgs: " -no-reboot",
}
}

return types.MonitorCliArgs{
OtherArgs: " -no-reboot -device isa-debug-exit,iobase=0xf4,iosize=0x04",
}
default:
return types.MonitorCliArgs{}
}
}

func (h *Hermit) Init(data types.UnikernelParams) error {
mask := 24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: What if we make this a constant, like DefaultCIDRMask?

if data.Net.Mask != "" {
cidr, err := subnetMaskToCIDR(data.Net.Mask)
if err != nil {
return err
}
mask = cidr
}

h.Command = strings.Join(data.CmdLine, " ")
h.Monitor = data.Monitor
h.Net.Address = data.Net.IP
h.Net.Gateway = data.Net.Gateway
h.Net.Mask = mask

return nil
}

func newHermit() *Hermit {
return new(Hermit)
}
3 changes: 3 additions & 0 deletions pkg/unikontainers/unikernels/unikernel.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ func New(unikernelType string) (types.Unikernel, error) {
case LinuxUnikernel:
unikernel := newLinux()
return unikernel, nil
case HermitUnikernel:
unikernel := newHermit()
return unikernel, nil
default:
return nil, ErrNotSupportedUnikernel
}
Expand Down