From bfa9167c854d9150ec32824530185c4efb75836e Mon Sep 17 00:00:00 2001 From: Mahe Tardy Date: Mon, 15 Apr 2024 13:34:34 +0000 Subject: [PATCH] vmtests: use LVH as a library to build qemu args Deduplicate some codes shared between vmtests and LVH: - inject LVH runner.RunConf into vmtests RunConf and reuse existing LVH configuration fields. - use LVH runner.BuildQemuArgs along with custom vmtests logic. - removes the support for HVF, we would need to put it in LVH to get this again but I don't think this is used nowadays. This is mainly so that we benefit from the arm64 support as LVH was recently updated to support it. Signed-off-by: Mahe Tardy --- cmd/tetragon-vmtests-run/conf.go | 20 ++++---- cmd/tetragon-vmtests-run/image.go | 8 ++-- cmd/tetragon-vmtests-run/main.go | 28 +++++------ cmd/tetragon-vmtests-run/qemu.go | 78 +++++++------------------------ 4 files changed, 43 insertions(+), 91 deletions(-) diff --git a/cmd/tetragon-vmtests-run/conf.go b/cmd/tetragon-vmtests-run/conf.go index e5b8ec05828..280b4101d73 100644 --- a/cmd/tetragon-vmtests-run/conf.go +++ b/cmd/tetragon-vmtests-run/conf.go @@ -4,37 +4,35 @@ package main import ( + "fmt" "path/filepath" "github.com/cilium/little-vm-helper/pkg/runner" "github.com/cilium/tetragon/pkg/vmtests" ) -// NB: we should use lvh's RunConf to avoid duplicating code type RunConf struct { - testImage string - baseFname string - kernelFname string + runner.RunConf + vmName string + baseImageFilename string dontRebuildImage bool useTetragonTesterInit bool testerOut string qemuPrint bool justBoot bool justBuildImage bool - disableKVM bool - enableHVF bool btfFile string disableUnifiedCgroups bool - portForwards runner.PortForwards testerConf vmtests.Conf detailedResults bool keepAllLogs bool - rootDev string filesystems []QemuFS } -func (rc *RunConf) testImageFname() string { - imagesDir := filepath.Dir(rc.baseFname) - return filepath.Join(imagesDir, rc.testImage) +func (rc RunConf) testImageFilename() string { + if ext := filepath.Ext(rc.vmName); ext == "" { + return fmt.Sprintf("%s.qcow2", rc.vmName) + } + return rc.vmName } diff --git a/cmd/tetragon-vmtests-run/image.go b/cmd/tetragon-vmtests-run/image.go index 2e14679994f..5b25f375367 100644 --- a/cmd/tetragon-vmtests-run/image.go +++ b/cmd/tetragon-vmtests-run/image.go @@ -9,7 +9,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/cilium/little-vm-helper/pkg/images" "github.com/cilium/tetragon/pkg/vmtests" @@ -201,8 +200,7 @@ func buildNetActions(tmpDir string) ([]images.Action, error) { func buildTestImage(log *logrus.Logger, rcnf *RunConf) error { - imagesDir, baseImage := filepath.Split(rcnf.baseFname) - hostname := strings.TrimSuffix(rcnf.testImage, filepath.Ext(rcnf.testImage)) + imagesDir, baseImage := filepath.Split(rcnf.baseImageFilename) tmpDir, err := os.MkdirTemp("", "tetragon-vmtests-") if err != nil { @@ -226,7 +224,7 @@ func buildTestImage(log *logrus.Logger, rcnf *RunConf) error { } actions := []images.Action{ - {Op: &images.SetHostnameCommand{Hostname: hostname}}, + {Op: &images.SetHostnameCommand{Hostname: rcnf.vmName}}, {Op: &images.AppendLineCommand{ File: "/etc/sysctl.d/local.conf", Line: "kernel.panic_on_rcu_stall=1", @@ -241,7 +239,7 @@ func buildTestImage(log *logrus.Logger, rcnf *RunConf) error { // TODO: might be useful to modify the images builder so that // we can build this image using qemu-img -b Images: []images.ImgConf{{ - Name: rcnf.testImage, + Name: rcnf.testImageFilename(), Parent: baseImage, Actions: actions, }}, diff --git a/cmd/tetragon-vmtests-run/main.go b/cmd/tetragon-vmtests-run/main.go index b835ebc1330..9bccd425e50 100644 --- a/cmd/tetragon-vmtests-run/main.go +++ b/cmd/tetragon-vmtests-run/main.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/cilium/little-vm-helper/pkg/arch" "github.com/cilium/little-vm-helper/pkg/runner" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -30,7 +31,7 @@ func main() { t0 := time.Now() var err error - rcnf.portForwards, err = runner.ParsePortForward(ports) + rcnf.ForwardedPorts, err = runner.ParsePortForward(ports) if err != nil { return fmt.Errorf("error parseing ports: %w", err) } @@ -75,19 +76,19 @@ func main() { return fmt.Errorf("failed to get cwd: %w", err) } - if ext := filepath.Ext(rcnf.testImage); ext == "" { - rcnf.testImage = fmt.Sprintf("%s.qcow2", rcnf.testImage) - } - err = buildTestImage(log, &rcnf) if err != nil || rcnf.justBuildImage { return err } - qemuBin := "qemu-system-x86_64" - qemuArgs, err := buildQemuArgs(log, &rcnf) + qemuBin, err := arch.QemuBinary() if err != nil { - return err + return fmt.Errorf("failed to get qemu binary: %w", err) + } + + qemuArgs, err := buildQemuArgs(log, rcnf) + if err != nil { + return fmt.Errorf("failed to build qemu args: %w", err) } if rcnf.qemuPrint { @@ -141,15 +142,14 @@ func main() { }, } - cmd.Flags().StringVar(&rcnf.baseFname, "base", "", "base image filename") + cmd.Flags().StringVar(&rcnf.baseImageFilename, "base", "", "base image filename") cmd.MarkFlagRequired("base") - cmd.Flags().StringVar(&rcnf.testImage, "name", "tetragon", "new vm (and basis for the image name). New vm image will be in the directory of the base image") - cmd.Flags().StringVar(&rcnf.kernelFname, "kernel", "", "kernel filename to boot with. (if empty no -kernel option will be passed to qemu)") + cmd.Flags().StringVar(&rcnf.vmName, "name", "tetragon", "new vm (and basis for the image name). New vm image will be in the directory of the base image") + cmd.Flags().StringVar(&rcnf.KernelFname, "kernel", "", "kernel filename to boot with. (if empty no -kernel option will be passed to qemu)") cmd.Flags().BoolVar(&rcnf.dontRebuildImage, "dont-rebuild-image", false, "dont rebuild image") cmd.Flags().BoolVar(&rcnf.useTetragonTesterInit, "use-tetragon-init", false, "use tetragon-vmtests-init as init process in the VM") cmd.Flags().BoolVar(&rcnf.qemuPrint, "qemu-cmd-print", false, "Do not run the qemu command, just print it") - cmd.Flags().BoolVar(&rcnf.disableKVM, "qemu-disable-kvm", false, "Do not use KVM acceleration, even if /dev/kvm exists") - cmd.Flags().BoolVar(&rcnf.enableHVF, "qemu-enable-hvf", false, "Use hvf acceleration") + cmd.Flags().BoolVar(&rcnf.DisableKVM, "qemu-disable-kvm", false, "Do not use KVM acceleration, even if /dev/kvm exists") cmd.Flags().BoolVar(&rcnf.justBoot, "just-boot", false, "Do not actually run any tests. Just setup everything and start the VM. User will be able to login to the VM.") cmd.Flags().BoolVar(&rcnf.justBuildImage, "just-build-image", false, "Just build an image. Do not actually run any tests or boot the VM.") cmd.Flags().BoolVar(&rcnf.testerConf.NoPowerOff, "no-poweroff", false, "Do not poweroff the VM at the end of the run") @@ -161,7 +161,7 @@ func main() { cmd.Flags().StringArrayVarP(&ports, "port", "p", nil, "Forward a port (hostport[:vmport[:tcp|udp]])") cmd.Flags().StringVar(&rcnf.testerConf.KernelVer, "kernel-ver", "", "kenel version") cmd.Flags().BoolVar(&rcnf.detailedResults, "enable-detailed-results", false, "produce detailed results") - cmd.Flags().StringVar(&rcnf.rootDev, "root-dev", "vda", "type of root device (hda or vda)") + cmd.Flags().StringVar(&rcnf.RootDev, "root-dev", "vda", "type of root device (hda or vda)") if err := cmd.Execute(); err != nil { os.Exit(1) diff --git a/cmd/tetragon-vmtests-run/qemu.go b/cmd/tetragon-vmtests-run/qemu.go index ebfb0a566d8..e8919477eb6 100644 --- a/cmd/tetragon-vmtests-run/qemu.go +++ b/cmd/tetragon-vmtests-run/qemu.go @@ -5,80 +5,36 @@ package main import ( "fmt" - "os" - "strings" + "path/filepath" + "github.com/cilium/little-vm-helper/pkg/runner" "github.com/sirupsen/logrus" ) -func buildQemuArgs(log *logrus.Logger, rcnf *RunConf) ([]string, error) { - qemuArgs := []string{ - // no need for all the default devices - "-nodefaults", - // no need display (-nographics seems a bit slower) - "-display", "none", - // don't reboot, just exit - "-no-reboot", - // cpus, memory - "-smp", "2", "-m", "4G", - } - - if rcnf.enableHVF { - log.Info("HVF enabled") - qemuArgs = append(qemuArgs, "-accel", "hvf") - } else if !rcnf.disableKVM { - // quick-and-dirty kvm detection - if f, err := os.OpenFile("/dev/kvm", os.O_RDWR, 0755); err == nil { - qemuArgs = append(qemuArgs, "-enable-kvm", "-cpu", "kvm64") - f.Close() - } else { - log.Infof("KVM disabled (%v)", err) - } - } - - var kernelRoot string - switch rcnf.rootDev { - case "hda": - qemuArgs = append(qemuArgs, "-hda", rcnf.testImageFname()) - kernelRoot = "/dev/sda" - case "vda": - qemuArgs = append(qemuArgs, "-drive", fmt.Sprintf("file=%s,if=virtio,index=0,media=disk", rcnf.testImageFname())) - kernelRoot = "/dev/vda" - default: - return nil, fmt.Errorf("invalid root device: %s", rcnf.rootDev) - } - - if rcnf.kernelFname != "" { - appendArgs := []string{ - fmt.Sprintf("root=%s", kernelRoot), - "console=ttyS0", - "earlyprintk=ttyS0", - "panic=-1", - } +// buildQemuArgs is a wrapper around LVH's runner.BuildQemuArgs and also handles +// custom configuration from vmtests +func buildQemuArgs(log *logrus.Logger, rcnf RunConf) ([]string, error) { + if rcnf.KernelFname != "" { if rcnf.disableUnifiedCgroups { - appendArgs = append(appendArgs, "systemd.unified_cgroup_hierarchy=0") + rcnf.KernelAppendArgs = append(rcnf.KernelAppendArgs, "systemd.unified_cgroup_hierarchy=0") } if rcnf.useTetragonTesterInit { - appendArgs = append(appendArgs, fmt.Sprintf("init=%s", TetragonTesterBin)) + rcnf.KernelAppendArgs = append(rcnf.KernelAppendArgs, fmt.Sprintf("init=%s", TetragonTesterBin)) } - qemuArgs = append(qemuArgs, - "-kernel", rcnf.kernelFname, - "-append", strings.Join(appendArgs, " "), - ) } - // NB: not sure what the best option is here, this is from trial-and-error - qemuArgs = append(qemuArgs, - "-serial", "mon:stdio", - "-device", "virtio-serial-pci", - ) + rcnf.CPU = 2 + rcnf.Mem = "4G" + // the new image is in the base image folder + rcnf.Image = filepath.Join(filepath.Dir(rcnf.baseImageFilename), rcnf.testImageFilename()) - for _, fs := range rcnf.filesystems { - qemuArgs = append(qemuArgs, fs.qemuArgs()...) + qemuArgs, err := runner.BuildQemuArgs(log, &rcnf.RunConf) + if err != nil { + return nil, err } - if len(rcnf.portForwards) > 0 { - qemuArgs = append(qemuArgs, rcnf.portForwards.QemuArgs()...) + for _, fs := range rcnf.filesystems { + qemuArgs = append(qemuArgs, fs.qemuArgs()...) } return qemuArgs, nil