From 0414f88b3af26b6027d135896e91fb4c924d7ee0 Mon Sep 17 00:00:00 2001 From: Jake Correnti Date: Tue, 3 Oct 2023 00:22:32 -0400 Subject: [PATCH] Create Qemu command wrapper Creates a wrapper around the Qemu command line implementation to prevent the need to hard-code the different command line options in Init and Start. Signed-off-by: Jake Correnti --- pkg/machine/qemu/command.go | 87 +++++++++++++++++++++++++++ pkg/machine/qemu/config.go | 32 +++------- pkg/machine/qemu/machine.go | 19 ++---- pkg/machine/qemu/machine_test.go | 4 +- pkg/machine/qemu/qemu_command_test.go | 67 +++++++++++++++++++++ 5 files changed, 169 insertions(+), 40 deletions(-) create mode 100644 pkg/machine/qemu/command.go create mode 100644 pkg/machine/qemu/qemu_command_test.go diff --git a/pkg/machine/qemu/command.go b/pkg/machine/qemu/command.go new file mode 100644 index 0000000000..080d7c16be --- /dev/null +++ b/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 +} diff --git a/pkg/machine/qemu/config.go b/pkg/machine/qemu/config.go index b1a84964f0..965daa9b24 100644 --- a/pkg/machine/qemu/config.go +++ b/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 diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 9134847522..7d25892521 100644 --- a/pkg/machine/qemu/machine.go +++ b/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() { diff --git a/pkg/machine/qemu/machine_test.go b/pkg/machine/qemu/machine_test.go index 18cb21f921..dbf2ebaa68 100644 --- a/pkg/machine/qemu/machine_test.go +++ b/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) { diff --git a/pkg/machine/qemu/qemu_command_test.go b/pkg/machine/qemu/qemu_command_test.go new file mode 100644 index 0000000000..3a8848a07d --- /dev/null +++ b/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) +}