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 <mahe.tardy@gmail.com>
cilium · Apr 16, 2024 · bfa9167 · bfa9167
1 parent f246390
commit bfa9167
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 91 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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