Skip to content

refactor: replace string concatenation in monitor CLI option construc… #858

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.

Sign up for GitHub

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

Open
singhshresth26 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: replace string concatenation in monitor CLI option construc… #858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 19 additions & 15 deletions pkg/unikontainers/hypervisors/cloud_hypervisor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ package hypervisors

import (
"fmt"
"strings"
"strconv"

"github.com/urunc-dev/urunc/pkg/unikontainers/types"
"golang.org/x/sys/unix"
Expand Down Expand Up @@ -72,14 +72,17 @@ func (ch *CloudHypervisor) BuildExecCmd(args types.ExecArgs, ukernel types.Unike

// Memory configuration
if args.Sharedfs.Type == "virtiofs" {
exArgs = append(exArgs, "--memory", fmt.Sprintf("size=%sM,shared=on", chMem))
memArg := "size=" + chMem + "M,shared=on"
exArgs = append(exArgs, "--memory", memArg)
} else {
exArgs = append(exArgs, "--memory", fmt.Sprintf("size=%sM", chMem))
memArg := "size=" + chMem + "M"
exArgs = append(exArgs, "--memory", memArg)
}

// CPU configuration
if args.VCPUs > 0 {
exArgs = append(exArgs, "--cpus", fmt.Sprintf("boot=%d", args.VCPUs))
cpuArg := "boot=" + strconv.Itoa(int(args.VCPUs))
exArgs = append(exArgs, "--cpus", cpuArg)
}

// Kernel path
Expand All @@ -98,23 +101,24 @@ func (ch *CloudHypervisor) BuildExecCmd(args types.ExecArgs, ukernel types.Unike
// Network configuration
if args.Net.TapDev != "" {
netCli := ukernel.MonitorNetCli(args.Net.TapDev, args.Net.MAC)
if netCli == "" {
if len(netCli) == 0 {
// Default network configuration for Cloud Hypervisor
exArgs = append(exArgs, "--net", fmt.Sprintf("tap=%s,mac=%s,mtu=%d", args.Net.TapDev, args.Net.MAC, args.Net.MTU))
netArg := fmt.Sprintf("tap=%s,mac=%s,mtu=%d", args.Net.TapDev, args.Net.MAC, args.Net.MTU)
exArgs = append(exArgs, "--net", netArg)
} else {
exArgs = append(exArgs, strings.Split(strings.TrimSpace(netCli), " ")...)
exArgs = append(exArgs, netCli...)
}
}

// Block device configuration
blockArgs := ukernel.MonitorBlockCli()
for _, blockArg := range blockArgs {
if blockArg.ExactArgs != "" {
exArgs = append(exArgs, strings.Split(strings.TrimSpace(blockArg.ExactArgs), " ")...)
if len(blockArg.ExactArgs) > 0 {
exArgs = append(exArgs, blockArg.ExactArgs...)
} else if blockArg.Path != "" {
diskArg := fmt.Sprintf("path=%s", blockArg.Path)
diskArg := "path=" + blockArg.Path
if blockArg.ID != "" {
diskArg += fmt.Sprintf(",id=%s", blockArg.ID)
diskArg += ",id=" + blockArg.ID
}
exArgs = append(exArgs, "--disk", diskArg)
}
Expand All @@ -139,12 +143,12 @@ func (ch *CloudHypervisor) BuildExecCmd(args types.ExecArgs, ukernel types.Unike
}

if args.VAccelType == "vsock" {
exArgs = append(exArgs, "--vsock", fmt.Sprintf("cid=%d,socket=%s/vaccel.sock",
args.VSockDevID, args.VSockDevPath))
vsockArg := fmt.Sprintf("cid=%d,socket=%s/vaccel.sock", args.VSockDevID, args.VSockDevPath)
exArgs = append(exArgs, "--vsock", vsockArg)
}

if extraMonArgs.OtherArgs != "" {
exArgs = append(exArgs, strings.Split(strings.TrimSpace(extraMonArgs.OtherArgs), " ")...)
if len(extraMonArgs.OtherArgs) > 0 {
exArgs = append(exArgs, extraMonArgs.OtherArgs...)
}

// Add the command line arguments for the kernel
Expand Down
7 changes: 2 additions & 5 deletions pkg/unikontainers/hypervisors/firecracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/urunc-dev/urunc/pkg/unikontainers/types"
"golang.org/x/sys/unix"
Expand Down Expand Up @@ -108,11 +107,10 @@ func (fc *Firecracker) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel
// options in FC, since the string return value of the Monitor related
// functions in the unikernel interface do not integrate well with FC's
// json configuration.
cmdString := fc.Path() + " --no-api --config-file "
JSONConfigFile := filepath.Join("/tmp/", FCJsonFilename)
cmdString += JSONConfigFile
exArgs := []string{fc.Path(), "--no-api", "--config-file", JSONConfigFile}
if !args.Seccomp {
cmdString += " --no-seccomp"
exArgs = append(exArgs, "--no-seccomp")
}

// VM config for Firecracker
Expand Down Expand Up @@ -200,7 +198,6 @@ func (fc *Firecracker) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel
}
vmmLog.WithField("Json", string(FCConfigJSON)).Debug("Firecracker json config")

exArgs := strings.Split(cmdString, " ")
return exArgs, nil
}

Expand Down
24 changes: 14 additions & 10 deletions pkg/unikontainers/hypervisors/hvt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,21 +154,25 @@ func (h *HVT) Ok() error {

func (h *HVT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
hvtMem := BytesToStringMB(args.MemSizeB)
cmdString := h.binaryPath + " --mem=" + hvtMem
exArgs := []string{h.binaryPath, "--mem=" + hvtMem}
if args.Net.TapDev != "" {
cmdString += " "
cmdString += ukernel.MonitorNetCli(args.Net.TapDev, args.Net.MAC)
netCli := ukernel.MonitorNetCli(args.Net.TapDev, args.Net.MAC)
exArgs = append(exArgs, netCli...)
}
extraMonArgs := ukernel.MonitorCli()
bArgs := ukernel.MonitorBlockCli()
for _, blockArg := range bArgs {
cmdString = appendNonEmpty(cmdString, " --block:"+blockArg.ID+"=",
blockArg.Path)
if blockArg.Path != "" {
exArgs = append(exArgs, "--block:"+blockArg.ID+"="+blockArg.Path)
}
}
extraMonArgs := ukernel.MonitorCli()
exArgs = append(exArgs, extraMonArgs.OtherArgs...)
exArgs = append(exArgs, args.UnikernelPath)

@cmainas cmainas Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let;s add the -- which is the Solo5 separator for Solo5 cli options and unikernel cl options.

if args.Command != "" {
exArgs = append(exArgs, "--")
exArgs = append(exArgs, strings.Fields(args.Command)...)
}
cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs)
cmdString += " " + args.UnikernelPath + " " + args.Command
cmdArgs := strings.Split(cmdString, " ")
return cmdArgs, nil
return exArgs, nil
}

// PreExec performs pre-execution setup for HVT.
Expand Down
5 changes: 3 additions & 2 deletions pkg/unikontainers/hypervisors/hyperlight.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package hypervisors

import (
"fmt"
"strconv"

"github.com/urunc-dev/urunc/pkg/unikontainers/types"
"golang.org/x/sys/unix"
Expand Down Expand Up @@ -68,7 +68,8 @@ func (h *Hyperlight) BuildExecCmd(args types.ExecArgs, _ types.Unikernel) ([]str
cmdArgs = append(cmdArgs, "--initrd", args.InitrdPath)
}
if args.MemSizeB > 0 {
cmdArgs = append(cmdArgs, "--memory", fmt.Sprintf("%d", args.MemSizeB))
memArg := strconv.FormatUint(args.MemSizeB, 10)
cmdArgs = append(cmdArgs, "--memory", memArg)
}
return cmdArgs, nil
}
Expand Down
109 changes: 56 additions & 53 deletions pkg/unikontainers/hypervisors/qemu.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package hypervisors
import (
"fmt"
"runtime"
"strings"
"strconv"

"github.com/urunc-dev/urunc/pkg/unikontainers/types"
"golang.org/x/sys/unix"
Expand Down Expand Up @@ -62,87 +62,98 @@ 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 += " -display none -vga none -serial stdio -monitor null" // Disable graphic output
exArgs := []string{
q.binaryPath,
"-m", qemuMem + "M",
"-L", "/usr/share/qemu",
"-cpu", "host",
"-enable-kvm",
"-display", "none",
"-vga", "none",
"-serial", "stdio",
"-monitor", "null",
}

if args.VCPUs > 0 {
cmdString += fmt.Sprintf(" -smp %d", args.VCPUs)
exArgs = append(exArgs, "-smp", strconv.Itoa(int(args.VCPUs)))
}

if args.Seccomp {
// Enable Seccomp in QEMU
cmdString += " --sandbox on"
// Allow or Deny Obsolete system calls
cmdString += ",obsolete=deny"
// Allow or Deny set*uid|gid system calls
cmdString += ",elevateprivileges=deny"
// Allow or Deny *fork and execve
cmdString += ",spawn=deny"
// Allow or Deny process affinity and schedular priority
cmdString += ",resourcecontrol=deny"
exArgs = append(exArgs,
"--sandbox", "on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny",
)
}

// TODO: Check if this check causes any performance drop
// or explore alternative implementations
if runtime.GOARCH == "arm64" {
machineType := " -M virt"
cmdString += machineType
exArgs = append(exArgs, "-M", "virt")
}

cmdString += " -kernel " + args.UnikernelPath
exArgs = append(exArgs, "-kernel", args.UnikernelPath)
if args.Net.TapDev != "" {
netcli := ukernel.MonitorNetCli(args.Net.TapDev, args.Net.MAC)
if netcli == "" {
netcli += " -netdev tap,id=net0,script=no,downscript=no,ifname="
netcli += args.Net.TapDev
if len(netcli) == 0 {
netdevArg := fmt.Sprintf("tap,id=net0,script=no,downscript=no,ifname=%s", args.Net.TapDev)
if q.vhost {
netcli += ",vhost=on"
netdevArg += ",vhost=on"
}
exArgs = append(exArgs, "-netdev", netdevArg)

devType := "virtio-net-pci"
if runtime.GOARCH == "arm64" {
devType = "virtio-net-device"
}
netcli += fmt.Sprintf(" %s,host_mtu=%d,mac=%s", getVirtioNetArg(), args.Net.MTU, args.Net.MAC)
netCliDev := fmt.Sprintf("%s,netdev=net0,host_mtu=%d,mac=%s", devType, args.Net.MTU, args.Net.MAC)
exArgs = append(exArgs, "-device", netCliDev)
} else {
exArgs = append(exArgs, netcli...)
}
cmdString += netcli
} else {
cmdString += " -nic none"
exArgs = append(exArgs, "-nic", "none")
}

blockArgs := ukernel.MonitorBlockCli()
for _, blockArg := range blockArgs {
blockCli := blockArg.ExactArgs
if blockCli == "" && blockArg.ID != "" && blockArg.Path != "" {
blockCli1 := fmt.Sprintf(" -device virtio-blk-pci,serial=%s,drive=%s,scsi=off", blockArg.ID, blockArg.ID)
blockCli2 := fmt.Sprintf(" -drive format=raw,if=none,id=%s,file=%s", blockArg.ID, blockArg.Path)
blockCli = blockCli1 + blockCli2
if blockArg.ID != "" && blockArg.Path != "" {
devArg := fmt.Sprintf("virtio-blk-pci,serial=%s,drive=%s,scsi=off", blockArg.ID, blockArg.ID)
drvArg := fmt.Sprintf("format=raw,if=none,id=%s,file=%s", blockArg.ID, blockArg.Path)
exArgs = append(exArgs, "-device", devArg, "-drive", drvArg)
} else if len(blockArg.ExactArgs) > 0 {
exArgs = append(exArgs, blockArg.ExactArgs...)
}
cmdString += blockCli
}

if args.InitrdPath != "" {
cmdString += " -initrd " + args.InitrdPath
exArgs = append(exArgs, "-initrd", args.InitrdPath)
}

switch args.Sharedfs.Type {
case "9pfs":
cmdString += " -fsdev local,id=rootfs9p,security_model=none,path=" + args.Sharedfs.Path
cmdString += " -device virtio-9p-pci,fsdev=rootfs9p,mount_tag=fs0"
fsdevArg := fmt.Sprintf("local,id=rootfs9p,security_model=none,path=%s", args.Sharedfs.Path)
exArgs = append(exArgs, "-fsdev", fsdevArg, "-device", "virtio-9p-pci,fsdev=rootfs9p,mount_tag=fs0")
case "virtiofs":
cmdString += " -object memory-backend-file,id=mem,size=" + qemuMem + "M,mem-path=/tmp,share=on"
cmdString += " -numa node,memdev=mem"
cmdString += " -chardev socket,id=char0,path=/tmp/vhostqemu"
cmdString += " -device vhost-user-fs-pci,queue-size=1024,chardev=char0,tag=fs0"
objArg := fmt.Sprintf("memory-backend-file,id=mem,size=%sM,mem-path=/tmp,share=on", qemuMem)
exArgs = append(exArgs,
"-object", objArg,
"-numa", "node,memdev=mem",
"-chardev", "socket,id=char0,path=/tmp/vhostqemu",
"-device", "vhost-user-fs-pci,queue-size=1024,chardev=char0,tag=fs0",
)
default:
// Nothing to add
}

extraMonArgs := ukernel.MonitorCli()
if extraMonArgs.ExtraInitrd != "" {
cmdString += " -initrd " + extraMonArgs.ExtraInitrd
exArgs = append(exArgs, "-initrd", extraMonArgs.ExtraInitrd)
}
cmdString += extraMonArgs.OtherArgs
exArgs = append(exArgs, extraMonArgs.OtherArgs...)

if args.VAccelType == "vsock" {
cmdString += " -device vhost-vsock-pci,id=vhost-vsock-pci0,guest-cid=" + fmt.Sprintf("%d", args.VSockDevID)
vsockArg := fmt.Sprintf("vhost-vsock-pci,id=vhost-vsock-pci0,guest-cid=%d", args.VSockDevID)
exArgs = append(exArgs, "-device", vsockArg)
}

exArgs := strings.Split(cmdString, " ")
exArgs = append(exArgs, "-append", args.Command)
return exArgs, nil
}
Expand All @@ -151,11 +162,3 @@ func (q *Qemu) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]str
func (q *Qemu) PreExec(_ types.ExecArgs) error {
return nil
}

func getVirtioNetArg() string {
devType := "virtio-net-pci"
if runtime.GOARCH == "arm64" {
devType = "virtio-net-device"
}
return "-device " + devType + ",netdev=net0"
}
10 changes: 5 additions & 5 deletions pkg/unikontainers/hypervisors/qemu_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
// Qemu.BuildExecCmd. The three Monitor* methods are the ones the function
// consults; the rest return zero values.
type fakeUnikernel struct {
netCli string
netCli []string
blockCli []types.MonitorBlockArgs
monitorCli types.MonitorCliArgs
}
Expand All @@ -36,7 +36,7 @@ func (f *fakeUnikernel) Init(types.UnikernelParams) error { return nil
func (f *fakeUnikernel) CommandString() (string, error) { return "", nil }
func (f *fakeUnikernel) SupportsBlock() bool { return true }
func (f *fakeUnikernel) SupportsFS(string) bool { return true }
func (f *fakeUnikernel) MonitorNetCli(string, string) string { return f.netCli }
func (f *fakeUnikernel) MonitorNetCli(string, string) []string { return f.netCli }
func (f *fakeUnikernel) MonitorBlockCli() []types.MonitorBlockArgs { return f.blockCli }
func (f *fakeUnikernel) MonitorCli() types.MonitorCliArgs { return f.monitorCli }

Expand Down Expand Up @@ -164,7 +164,7 @@ func TestQemuBuildExecCmd(t *testing.T) {
Command: testCommand,
Net: types.NetDevParams{TapDev: "tap0"},
},
unikernel: &fakeUnikernel{netCli: " -netdev user,id=net0 -device e1000,netdev=net0"},
unikernel: &fakeUnikernel{netCli: []string{"-netdev", "user,id=net0", "-device", "e1000,netdev=net0"}},
mustContain: []string{"-netdev user,id=net0", "-device e1000,netdev=net0"},
mustNotContain: []string{"-netdev tap"},
},
Expand Down Expand Up @@ -219,7 +219,7 @@ func TestQemuBuildExecCmd(t *testing.T) {
UnikernelPath: testKernelPath,
Command: testCommand,
},
unikernel: &fakeUnikernel{blockCli: []types.MonitorBlockArgs{{ExactArgs: " -hda /custom/disk.img"}}},
unikernel: &fakeUnikernel{blockCli: []types.MonitorBlockArgs{{ExactArgs: []string{"-hda", "/custom/disk.img"}}}},
mustContain: []string{"-hda /custom/disk.img"},
mustNotContain: []string{"virtio-blk-pci"},
},
Expand All @@ -238,7 +238,7 @@ func TestQemuBuildExecCmd(t *testing.T) {
UnikernelPath: testKernelPath,
Command: testCommand,
},
unikernel: &fakeUnikernel{monitorCli: types.MonitorCliArgs{OtherArgs: " -nographic -no-reboot"}},
unikernel: &fakeUnikernel{monitorCli: types.MonitorCliArgs{OtherArgs: []string{"-nographic", "-no-reboot"}}},
mustContain: []string{"-nographic", "-no-reboot"},
},
{
Expand Down
Loading