feat: add Hermit unikernel backend support#540
feat: add Hermit unikernel backend support#540jim-junior wants to merge 1 commit intourunc-dev:mainfrom
Conversation
53fbfde - QEMU-based execution only - x86_64 architecture support - initrd-based boot model - virtio-net support via tap (no vhost) - no block device support - basic boot argument mapping for networking Enables running Hermit unikernels via urunc with initial networking support. Fixes: urunc-dev#97 Signed-off-by: jim-junior <jimjunior854@gmail.com>
✅ Deploy Preview for urunc canceled.
|
IrvingMg
left a comment
IrvingMg
left a comment
There was a problem hiding this comment.
Thanks! Would it be possible to add at least some basic test cases for this?
| 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 |
There was a problem hiding this comment.
These changes seem unrelated. Maybe we can drop them to keep the PR focused on the feature?
There was a problem hiding this comment.
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.
| switch fsType { | ||
| case "initrd": | ||
| return true | ||
| case "virtiofs": | ||
| return true | ||
| default: | ||
| return false | ||
| } |
There was a problem hiding this comment.
| 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) MonitorCli() types.MonitorCliArgs { | ||
| switch h.Monitor { |
There was a problem hiding this comment.
nit: Looks like we only check h.Monitor == "qemu". Do we need a switch here?
There was a problem hiding this comment.
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:
| } | ||
|
|
||
| func (h *Hermit) Init(data types.UnikernelParams) error { | ||
| mask := 24 |
There was a problem hiding this comment.
nit: What if we make this a constant, like DefaultCIDRMask?
|
Alright @IrvingMg , let me update the PR basing on your feedback |
2 participants
Description
This PR adds initial support for the Hermit unikernel backend in urunc.
The implementation introduces a new unikernel type (Hermit) that integrates Hermit-based applications with the urunc runtime and QEMU as the underlying VM monitor. It also uses
Initrdfor storage and currently supports and is only tested onx86arch.Related issues
How was this tested?
LLM usage
N/A
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).