Merge pull request #20245 from jakecorrenti/qemu-wrapper

Create Qemu command wrapper

pkg/machine/qemu/command.go

@@ -0,0 +1,87 @@
+package qemu
+
+import (
+	"fmt"
+	"strconv"
+
+	"github.com/containers/podman/v4/pkg/machine"
+)
+
+// QemuCmd is an alias around a string slice to prevent the need to migrate the
+// MachineVM struct due to changes
+type QemuCmd []string
+
+// NewQemuBuilder creates a new QemuCmd object that we will build on top of,
+// starting with the qemu binary, architecture specific options, and propogated
+// proxy and SSL settings
+func NewQemuBuilder(binary string, options []string) QemuCmd {
+	q := QemuCmd{binary}
+	return append(q, options...)
+}
+
+// SetMemory adds the specified amount of memory for the machine
+func (q *QemuCmd) SetMemory(m uint64) {
+	*q = append(*q, "-m", strconv.FormatUint(m, 10))
+}
+
+// SetCPUs adds the number of CPUs the machine will have
+func (q *QemuCmd) SetCPUs(c uint64) {
+	*q = append(*q, "-smp", strconv.FormatUint(c, 10))
+}
+
+// SetIgnitionFile specifies the machine's ignition file
+func (q *QemuCmd) SetIgnitionFile(file machine.VMFile) {
+	*q = append(*q, "-fw_cfg", "name=opt/com.coreos/config,file="+file.GetPath())
+}
+
+// SetQmpMonitor specifies the machine's qmp socket
+func (q *QemuCmd) SetQmpMonitor(monitor Monitor) {
+	*q = append(*q, "-qmp", monitor.Network+":"+monitor.Address.GetPath()+",server=on,wait=off")
+}
+
+// SetNetwork adds a network device to the machine
+func (q *QemuCmd) SetNetwork() {
+	// Right now the mac address is hardcoded so that the host networking gives it a specific IP address.  This is
+	// why we can only run one vm at a time right now
+	*q = append(*q, "-netdev", "socket,id=vlan,fd=3", "-device", "virtio-net-pci,netdev=vlan,mac=5a:94:ef:e4:0c:ee")
+}
+
+// SetSerialPort adds a serial port to the machine for readiness
+func (q *QemuCmd) SetSerialPort(readySocket, vmPidFile machine.VMFile, name string) {
+	*q = append(*q,
+		"-device", "virtio-serial",
+		// qemu needs to establish the long name; other connections can use the symlink'd
+		// Note both id and chardev start with an extra "a" because qemu requires that it
+		// starts with a letter but users can also use numbers
+		"-chardev", "socket,path="+readySocket.GetPath()+",server=on,wait=off,id=a"+name+"_ready",
+		"-device", "virtserialport,chardev=a"+name+"_ready"+",name=org.fedoraproject.port.0",
+		"-pidfile", vmPidFile.GetPath())
+}
+
+// SetVirtfsMount adds a virtfs mount to the machine
+func (q *QemuCmd) SetVirtfsMount(source, tag, securityModel string, readonly bool) {
+	virtfsOptions := fmt.Sprintf("local,path=%s,mount_tag=%s,security_model=%s", source, tag, securityModel)
+	if readonly {
+		virtfsOptions += ",readonly"
+	}
+	*q = append(*q, "-virtfs", virtfsOptions)
+}
+
+// SetBootableImage specifies the image the machine will use to boot
+func (q *QemuCmd) SetBootableImage(image string) {
+	*q = append(*q, "-drive", "if=virtio,file="+image)
+}
+
+// SetDisplay specifies whether the machine will have a display
+func (q *QemuCmd) SetDisplay(display string) {
+	*q = append(*q, "-display", display)
+}
+
+// SetPropagatedHostEnvs adds options that propagate SSL and proxy settings
+func (q *QemuCmd) SetPropagatedHostEnvs() {
+	*q = propagateHostEnv(*q)
+}
+
+func (q *QemuCmd) Build() []string {
+	return *q
+}

pkg/machine/qemu/config.go

@@ -6,7 +6,6 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"time"
 
@@ -43,30 +42,13 @@ func (v *MachineVM) setQMPMonitorSocket() error {
 // setNewMachineCMD configure the CLI command that will be run to create the new
 // machine
 func (v *MachineVM) setNewMachineCMD(qemuBinary string) {
-	cmd := []string{qemuBinary}
-	// Add memory
-	cmd = append(cmd, []string{"-m", strconv.Itoa(int(v.Memory))}...)
-	// Add cpus
-	cmd = append(cmd, []string{"-smp", strconv.Itoa(int(v.CPUs))}...)
-	// Add ignition file
-	cmd = append(cmd, []string{"-fw_cfg", "name=opt/com.coreos/config,file=" + v.IgnitionFile.GetPath()}...)
-	cmd = append(cmd, []string{"-qmp", v.QMPMonitor.Network + ":" + v.QMPMonitor.Address.GetPath() + ",server=on,wait=off"}...)
-
-	// Add network
-	// Right now the mac address is hardcoded so that the host networking gives it a specific IP address.  This is
-	// why we can only run one vm at a time right now
-	cmd = append(cmd, []string{"-netdev", "socket,id=vlan,fd=3", "-device", "virtio-net-pci,netdev=vlan,mac=5a:94:ef:e4:0c:ee"}...)
-
-	// Add serial port for readiness
-	cmd = append(cmd, []string{
-		"-device", "virtio-serial",
-		// qemu needs to establish the long name; other connections can use the symlink'd
-		// Note both id and chardev start with an extra "a" because qemu requires that it
-		// starts with a letter but users can also use numbers
-		"-chardev", "socket,path=" + v.ReadySocket.Path + ",server=on,wait=off,id=a" + v.Name + "_ready",
-		"-device", "virtserialport,chardev=a" + v.Name + "_ready" + ",name=org.fedoraproject.port.0",
-		"-pidfile", v.VMPidFilePath.GetPath()}...)
-	v.CmdLine = cmd
+	v.CmdLine = NewQemuBuilder(qemuBinary, v.addArchOptions())
+	v.CmdLine.SetMemory(v.Memory)
+	v.CmdLine.SetCPUs(v.CPUs)
+	v.CmdLine.SetIgnitionFile(v.IgnitionFile)
+	v.CmdLine.SetQmpMonitor(v.QMPMonitor)
+	v.CmdLine.SetNetwork()
+	v.CmdLine.SetSerialPort(v.ReadySocket, v.VMPidFilePath, v.Name)
 }
 
 // NewMachine initializes an instance of a virtual machine based on the qemu

pkg/machine/qemu/machine.go

@@ -48,7 +48,7 @@ type MachineVM struct {
 	// ConfigPath is the path to the configuration file
 	ConfigPath machine.VMFile
 	// The command line representation of the qemu command
-	CmdLine []string
+	CmdLine QemuCmd
 	// HostUser contains info about host user
 	machine.HostUser
 	// ImageConfig describes the bootable image
@@ -215,11 +215,7 @@ func (v *MachineVM) addMountsToVM(opts machine.InitOptions) error {
 		target := extractTargetPath(paths)
 		readonly, securityModel := extractMountOptions(paths)
 		if volumeType == VolumeTypeVirtfs {
-			virtfsOptions := fmt.Sprintf("local,path=%s,mount_tag=%s,security_model=%s", source, tag, securityModel)
-			if readonly {
-				virtfsOptions += ",readonly"
-			}
-			v.CmdLine = append(v.CmdLine, []string{"-virtfs", virtfsOptions}...)
+			v.CmdLine.SetVirtfsMount(source, tag, securityModel, readonly)
 			mounts = append(mounts, machine.Mount{Type: MountType9p, Tag: tag, Source: source, Target: target, ReadOnly: readonly})
 		}
 	}
@@ -294,9 +290,6 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	v.ImagePath = *imagePath
 	v.ImageStream = strm.String()
 
-	// Add arch specific options including image location
-	v.CmdLine = append(v.CmdLine, v.addArchOptions()...)
-
 	if err := v.addMountsToVM(opts); err != nil {
 		return false, err
 	}
@@ -304,7 +297,7 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	v.UID = os.Getuid()
 
 	// Add location of bootable image
-	v.CmdLine = append(v.CmdLine, "-drive", "if=virtio,file="+v.getImageFile())
+	v.CmdLine.SetBootableImage(v.getImageFile())
 
 	if err := machine.AddSSHConnectionsToPodmanSocket(
 		v.UID,
@@ -706,12 +699,12 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
 	attr.Files = files
 	cmdLine := v.CmdLine
 
-	cmdLine = propagateHostEnv(cmdLine)
+	cmdLine.SetPropagatedHostEnvs()
 
 	// Disable graphic window when not in debug mode
 	// Done in start, so we're not suck with the debug level we used on init
 	if !logrus.IsLevelEnabled(logrus.DebugLevel) {
-		cmdLine = append(cmdLine, "-display", "none")
+		cmdLine.SetDisplay("none")
 	}
 
 	logrus.Debugf("qemu cmd: %v", cmdLine)
@@ -804,7 +797,7 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error {
 // propagateHostEnv is here for providing the ability to propagate
 // proxy and SSL settings (e.g. HTTP_PROXY and others) on a start
 // and avoid a need of re-creating/re-initiating a VM
-func propagateHostEnv(cmdLine []string) []string {
+func propagateHostEnv(cmdLine QemuCmd) QemuCmd {
 	varsToPropagate := make([]string, 0)
 
 	for k, v := range machine.GetProxyVariables() {

pkg/machine/qemu/machine_test.go

@@ -17,12 +17,12 @@ import (
 
 func TestEditCmd(t *testing.T) {
 	vm := new(MachineVM)
-	vm.CmdLine = []string{"command", "-flag", "value"}
+	vm.CmdLine = QemuCmd{"command", "-flag", "value"}
 
 	vm.editCmdLine("-flag", "newvalue")
 	vm.editCmdLine("-anotherflag", "anothervalue")
 
-	require.Equal(t, vm.CmdLine, []string{"command", "-flag", "newvalue", "-anotherflag", "anothervalue"})
+	require.Equal(t, vm.CmdLine.Build(), []string{"command", "-flag", "newvalue", "-anotherflag", "anothervalue"})
 }
 
 func TestPropagateHostEnv(t *testing.T) {

pkg/machine/qemu/qemu_command_test.go

@@ -0,0 +1,67 @@
+//go:build (amd64 && !windows) || (arm64 && !windows)
+// +build amd64,!windows arm64,!windows
+
+package qemu
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/containers/podman/v4/pkg/machine"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestQemuCmd(t *testing.T) {
+	ignFile, err := machine.NewMachineFile(t.TempDir()+"demo-ignition-file.ign", nil)
+	assert.NoError(t, err)
+
+	machineAddrFile, err := machine.NewMachineFile(t.TempDir()+"tmp.sock", nil)
+	assert.NoError(t, err)
+
+	readySocket, err := machine.NewMachineFile(t.TempDir()+"readySocket.sock", nil)
+	assert.NoError(t, err)
+
+	vmPidFile, err := machine.NewMachineFile(t.TempDir()+"vmpidfile.pid", nil)
+	assert.NoError(t, err)
+
+	monitor := Monitor{
+		Address: *machineAddrFile,
+		Network: "unix",
+		Timeout: 3,
+	}
+	ignPath := ignFile.GetPath()
+	addrFilePath := machineAddrFile.GetPath()
+	readySocketPath := readySocket.GetPath()
+	vmPidFilePath := vmPidFile.GetPath()
+	bootableImagePath := t.TempDir() + "test-machine_fedora-coreos-38.20230918.2.0-qemu.x86_64.qcow2"
+
+	cmd := NewQemuBuilder("/usr/bin/qemu-system-x86_64", []string{})
+	cmd.SetMemory(2048)
+	cmd.SetCPUs(4)
+	cmd.SetIgnitionFile(*ignFile)
+	cmd.SetQmpMonitor(monitor)
+	cmd.SetNetwork()
+	cmd.SetSerialPort(*readySocket, *vmPidFile, "test-machine")
+	cmd.SetVirtfsMount("/tmp/path", "vol10", "none", true)
+	cmd.SetBootableImage(bootableImagePath)
+	cmd.SetDisplay("none")
+
+	expected := []string{
+		"/usr/bin/qemu-system-x86_64",
+		"-m", "2048",
+		"-smp", "4",
+		"-fw_cfg", fmt.Sprintf("name=opt/com.coreos/config,file=%s", ignPath),
+		"-qmp", fmt.Sprintf("unix:%s,server=on,wait=off", addrFilePath),
+		"-netdev", "socket,id=vlan,fd=3",
+		"-device", "virtio-net-pci,netdev=vlan,mac=5a:94:ef:e4:0c:ee",
+		"-device", "virtio-serial",
+		"-chardev", fmt.Sprintf("socket,path=%s,server=on,wait=off,id=atest-machine_ready", readySocketPath),
+		"-device", "virtserialport,chardev=atest-machine_ready,name=org.fedoraproject.port.0",
+		"-pidfile", vmPidFilePath,
+		"-virtfs", "local,path=/tmp/path,mount_tag=vol10,security_model=none,readonly",
+		"-drive", fmt.Sprintf("if=virtio,file=%s", bootableImagePath),
+		"-display", "none"}
+
+	require.Equal(t, cmd.Build(), expected)
+}