From 3523b9b05220639208648fd62e9ef1054409a788 Mon Sep 17 00:00:00 2001
From: Jake Correnti <jakecorrenti+github@proton.me>
Date: Thu, 27 Jul 2023 09:53:21 -0400
Subject: [PATCH] Break QEMU `config.go` code into its own functions

Breaks some of the code in QEMU's `VirtProvider` implementation located
at `pkg/machine/qemu/config.go` into its own functions. Aids in
improving the readability of the code.

[NO NEW TESTS NEEDED]

Signed-off-by: Jake Correnti <jakecorrenti+github@proton.me>
---
 pkg/machine/qemu/config.go | 91 +++++++++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 31 deletions(-)

diff --git a/pkg/machine/qemu/config.go b/pkg/machine/qemu/config.go
index b061d7635a..cc2cc0a0e7 100644
--- a/pkg/machine/qemu/config.go
+++ b/pkg/machine/qemu/config.go
@@ -21,6 +21,54 @@ type QEMUVirtualization struct {
 	machine.Virtualization
 }
 
+// findQEMUBinary locates and returns the QEMU binary
+func findQEMUBinary() (string, error) {
+	cfg, err := config.Default()
+	if err != nil {
+		return "", err
+	}
+	return cfg.FindHelperBinary(QemuCommand, true)
+}
+
+// setQMPMonitorSocket sets the virtual machine's QMP Monitor socket
+func (v *MachineVM) setQMPMonitorSocket() error {
+	monitor, err := NewQMPMonitor("unix", v.Name, defaultQMPTimeout)
+	if err != nil {
+		return err
+	}
+	v.QMPMonitor = monitor
+	return nil
+}
+
+// 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
+}
+
 // NewMachine initializes an instance of a virtual machine based on the qemu
 // virtualization.
 func (p *QEMUVirtualization) NewMachine(opts machine.InitOptions) (machine.VM, error) {
@@ -32,16 +80,21 @@ func (p *QEMUVirtualization) NewMachine(opts machine.InitOptions) (machine.VM, e
 	if len(opts.Name) > 0 {
 		vm.Name = opts.Name
 	}
+
+	// set VM ignition file
 	ignitionFile, err := machine.NewMachineFile(filepath.Join(vmConfigDir, vm.Name+".ign"), nil)
 	if err != nil {
 		return nil, err
 	}
 	vm.IgnitionFile = *ignitionFile
+
+	// set VM image file
 	imagePath, err := machine.NewMachineFile(opts.ImagePath, nil)
 	if err != nil {
 		return nil, err
 	}
 	vm.ImagePath = *imagePath
+
 	vm.RemoteUsername = opts.Username
 
 	// Add a random port for ssh
@@ -57,51 +110,27 @@ func (p *QEMUVirtualization) NewMachine(opts machine.InitOptions) (machine.VM, e
 
 	vm.Created = time.Now()
 
-	// Find the qemu executable
-	cfg, err := config.Default()
-	if err != nil {
-		return nil, err
-	}
-	execPath, err := cfg.FindHelperBinary(QemuCommand, true)
+	// find QEMU binary
+	execPath, err := findQEMUBinary()
 	if err != nil {
 		return nil, err
 	}
+
 	if err := vm.setPIDSocket(); err != nil {
 		return nil, err
 	}
-	cmd := []string{execPath}
-	// Add memory
-	cmd = append(cmd, []string{"-m", strconv.Itoa(int(vm.Memory))}...)
-	// Add cpus
-	cmd = append(cmd, []string{"-smp", strconv.Itoa(int(vm.CPUs))}...)
-	// Add ignition file
-	cmd = append(cmd, []string{"-fw_cfg", "name=opt/com.coreos/config,file=" + vm.IgnitionFile.GetPath()}...)
+
 	// Add qmp socket
-	monitor, err := NewQMPMonitor("unix", vm.Name, defaultQMPTimeout)
-	if err != nil {
+	if err := vm.setQMPMonitorSocket(); err != nil {
 		return nil, err
 	}
-	vm.QMPMonitor = monitor
-	cmd = append(cmd, []string{"-qmp", monitor.Network + ":" + monitor.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"}...)
 	if err := vm.setReadySocket(); err != nil {
 		return nil, err
 	}
 
-	// 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=" + vm.ReadySocket.Path + ",server=on,wait=off,id=a" + vm.Name + "_ready",
-		"-device", "virtserialport,chardev=a" + vm.Name + "_ready" + ",name=org.fedoraproject.port.0",
-		"-pidfile", vm.VMPidFilePath.GetPath()}...)
-	vm.CmdLine = cmd
+	// configure command to run
+	vm.setNewMachineCMD(execPath)
 	return vm, nil
 }