Merge pull request #14771 from klaases/qemu-tidy1

Improve qemu driver code for readability

Author: spowelljr

pkg/drivers/qemu/qemu.go

@@ -100,15 +100,13 @@ func (d *Driver) GetSSHPort() (int, error) {
 	if d.SSHPort == 0 {
 		d.SSHPort = 22
 	}
-
 	return d.SSHPort, nil
 }
 
 func (d *Driver) GetSSHUsername() string {
 	if d.SSHUser == "" {
 		d.SSHUser = "docker"
 	}
-
 	return d.SSHUser
 }
 
@@ -117,13 +115,12 @@ func (d *Driver) DriverName() string {
 }
 
 func (d *Driver) GetURL() (string, error) {
-	log.Debugf("GetURL called")
 	if _, err := os.Stat(d.pidfilePath()); err != nil {
 		return "", nil
 	}
 	ip, err := d.GetIP()
 	if err != nil {
-		log.Warnf("Failed to get IP: %s", err)
+		log.Warnf("Failed to get IP: %v", err)
 		return "", err
 	}
 	if ip == "" {
@@ -170,7 +167,6 @@ func checkPid(pid int) error {
 }
 
 func (d *Driver) GetState() (state.State, error) {
-
 	if _, err := os.Stat(d.pidfilePath()); err != nil {
 		return state.Stopped, nil
 	}
@@ -191,6 +187,7 @@ func (d *Driver) GetState() (state.State, error) {
 	if err != nil {
 		return state.Error, err
 	}
+
 	// RunState is one of:
 	// 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
 	// 'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
@@ -241,24 +238,24 @@ func (d *Driver) Create() error {
 		return err
 	}
 
-	log.Infof("Creating SSH key...")
+	log.Info("Creating SSH key...")
 	if err := ssh.GenerateSSHKey(d.sshKeyPath()); err != nil {
 		return err
 	}
 
-	log.Infof("Creating Disk image...")
+	log.Info("Creating Disk image...")
 	if err := d.generateDiskImage(d.DiskSize); err != nil {
 		return err
 	}
 
 	if d.UserDataFile != "" {
-		log.Infof("Creating Userdata Disk...")
+		log.Info("Creating Userdata Disk...")
 		if d.CloudConfigRoot, err = d.generateUserdataDisk(d.UserDataFile); err != nil {
 			return err
 		}
 	}
 
-	log.Infof("Starting QEMU VM...")
+	log.Info("Starting QEMU VM...")
 	return d.Start()
 }
 
@@ -270,34 +267,34 @@ func parsePortRange(rawPortRange string) (int, int, error) {
 	portRange := strings.Split(rawPortRange, "-")
 
 	if len(portRange) < 2 {
-		return 0, 0, errors.New("Invalid port range, must be at least of length 2")
+		return 0, 0, errors.New("invalid port range, requires at least 2 ports")
 	}
 
 	minPort, err := strconv.Atoi(portRange[0])
 	if err != nil {
-		return 0, 0, errors.Wrap(err, "Invalid port range")
+		return 0, 0, errors.Wrap(err, "invalid min port range")
 	}
+
 	maxPort, err := strconv.Atoi(portRange[1])
 	if err != nil {
-		return 0, 0, errors.Wrap(err, "Invalid port range")
+		return 0, 0, errors.Wrap(err, "invalid max port range")
 	}
 
 	if maxPort < minPort {
-		return 0, 0, errors.New("Invalid port range")
+		return 0, 0, errors.New("invalid port range, max less than min")
 	}
 
 	if maxPort-minPort < 2 {
-		return 0, 0, errors.New("Port range must be minimum 2 ports")
+		return 0, 0, errors.New("invalid port range, requires at least 2 ports")
 	}
-
 	return minPort, maxPort, nil
 }
 
-func getRandomPortNumberInRange(min int, max int) int {
+func getRandomPortNumberInRange(min, max int) int {
 	return rand.Intn(max-min) + min
 }
 
-func getAvailableTCPPortFromRange(minPort int, maxPort int) (int, error) {
+func getAvailableTCPPortFromRange(minPort, maxPort int) (int, error) {
 	port := 0
 	for i := 0; i <= 10; i++ {
 		var ln net.Listener
@@ -327,13 +324,12 @@ func getAvailableTCPPortFromRange(minPort int, maxPort int) (int, error) {
 			port = p
 			return port, nil
 		}
-		time.Sleep(1 * time.Second)
+		time.Sleep(time.Second)
 	}
 	return 0, fmt.Errorf("unable to allocate tcp port")
 }
 
 func (d *Driver) Start() error {
-	// fmt.Printf("Init qemu %s\n", i.VM)
 	machineDir := filepath.Join(d.StorePath, "machines", d.GetMachineName())
 
 	var startCmd []string
@@ -344,6 +340,7 @@ func (d *Driver) Start() error {
 			"-M", machineType,
 		)
 	}
+
 	if d.CPUType != "" {
 		startCmd = append(startCmd,
 			"-cpu", d.CPUType,
@@ -380,9 +377,13 @@ func (d *Driver) Start() error {
 
 	// hardware acceleration is important, it increases performance by 10x
 	if runtime.GOOS == "darwin" {
-		startCmd = append(startCmd, "-accel", "hvf")
+		// On macOS, enable the Hypervisor framework accelerator.
+		startCmd = append(startCmd,
+			"-accel", "hvf")
 	} else if _, err := os.Stat("/dev/kvm"); err == nil && runtime.GOOS == "linux" {
-		startCmd = append(startCmd, "-accel", "kvm")
+		// On Linux, enable the Kernel Virtual Machine accelerator.
+		startCmd = append(startCmd,
+			"-accel", "kvm")
 	}
 
 	startCmd = append(startCmd,
@@ -402,41 +403,46 @@ func (d *Driver) Start() error {
 		"-pidfile", d.pidfilePath(),
 	)
 
-	if d.Network == "user" {
+	switch d.Network {
+	case "user":
 		startCmd = append(startCmd,
 			"-nic", fmt.Sprintf("user,model=virtio,hostfwd=tcp::%d-:22,hostfwd=tcp::%d-:2376,hostname=%s", d.SSHPort, d.EnginePort, d.GetMachineName()),
 		)
-	} else if d.Network == "tap" {
+	case "tap":
 		startCmd = append(startCmd,
 			"-nic", fmt.Sprintf("tap,model=virtio,ifname=%s,script=no,downscript=no", d.NetworkInterface),
 		)
-	} else if d.Network == "vde" {
+	case "vde":
 		startCmd = append(startCmd,
 			"-nic", fmt.Sprintf("vde,model=virtio,sock=%s", d.NetworkSocket),
 		)
-	} else if d.Network == "bridge" {
+	case "bridge":
 		startCmd = append(startCmd,
 			"-nic", fmt.Sprintf("bridge,model=virtio,br=%s", d.NetworkBridge),
 		)
-	} else {
-		log.Errorf("Unknown network: %s", d.Network)
+	default:
+		log.Errorf("unknown network: %s", d.Network)
 	}
 
-	startCmd = append(startCmd, "-daemonize")
+	startCmd = append(startCmd,
+		"-daemonize")
 
 	if d.CloudConfigRoot != "" {
 		startCmd = append(startCmd,
 			"-fsdev",
 			fmt.Sprintf("local,security_model=passthrough,readonly,id=fsdev0,path=%s", d.CloudConfigRoot))
-		startCmd = append(startCmd, "-device", "virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=config-2")
+		startCmd = append(startCmd,
+			"-device",
+			"virtio-9p-pci,id=fs0,fsdev=fsdev0,mount_tag=config-2")
 	}
 
 	if d.VirtioDrives {
 		startCmd = append(startCmd,
 			"-drive", fmt.Sprintf("file=%s,index=0,media=disk,if=virtio", d.diskPath()))
 	} else {
 		// last argument is always the name of the disk image
-		startCmd = append(startCmd, d.diskPath())
+		startCmd = append(startCmd,
+			d.diskPath())
 	}
 
 	if stdout, stderr, err := cmdOutErr(d.Program, startCmd...); err != nil {
@@ -457,27 +463,25 @@ func cmdOutErr(cmdStr string, args ...string) (string, string, error) {
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
 	err := cmd.Run()
+	stdoutStr := stdout.String()
 	stderrStr := stderr.String()
-	log.Debugf("STDOUT: %v", stdout.String())
+	log.Debugf("STDOUT: %v", stdoutStr)
 	log.Debugf("STDERR: %v", stderrStr)
 	if err != nil {
 		if ee, ok := err.(*exec.Error); ok && ee == exec.ErrNotFound {
-			err = fmt.Errorf("mystery error: %s", ee)
+			err = fmt.Errorf("mystery error: %v", ee)
 		}
 	} else {
-		// also catch error messages in stderr, even if the return code
-		// looks OK
+		// also catch error messages in stderr, even if the return code looks OK
 		if strings.Contains(stderrStr, "error:") {
-			err = fmt.Errorf("%v %v failed: %v", cmdStr, strings.Join(args, " "), stderrStr)
+			err = fmt.Errorf("%s %s failed: %s", cmdStr, strings.Join(args, " "), stderrStr)
 		}
 	}
-	return stdout.String(), stderrStr, err
+	return stdoutStr, stderrStr, err
 }
 
 func (d *Driver) Stop() error {
-	// _, err := d.RunQMPCommand("stop")
-	_, err := d.RunQMPCommand("system_powerdown")
-	if err != nil {
+	if _, err := d.RunQMPCommand("system_powerdown"); err != nil {
 		return err
 	}
 	return nil
@@ -494,8 +498,7 @@ func (d *Driver) Remove() error {
 		}
 	}
 	if s != state.Stopped {
-		_, err = d.RunQMPCommand("quit")
-		if err != nil {
+		if _, err := d.RunQMPCommand("quit"); err != nil {
 			return errors.Wrap(err, "quit")
 		}
 	}
@@ -517,9 +520,7 @@ func (d *Driver) Restart() error {
 }
 
 func (d *Driver) Kill() error {
-	// _, err := d.RunQMPCommand("quit")
-	_, err := d.RunQMPCommand("system_powerdown")
-	if err != nil {
+	if _, err := d.RunQMPCommand("system_powerdown"); err != nil {
 		return err
 	}
 	return nil
@@ -623,7 +624,6 @@ func (d *Driver) generateDiskImage(size int) error {
 		return err
 	}
 	log.Debugf("DONE writing to %s and %s", rawFile, d.diskPath())
-
 	return nil
 }
 
@@ -652,13 +652,10 @@ func (d *Driver) generateUserdataDisk(userdataFile string) (string, error) {
 	if err := os.WriteFile(writeFile, userdata, 0644); err != nil {
 		return "", err
 	}
-
 	return ccRoot, nil
-
 }
 
 func (d *Driver) RunQMPCommand(command string) (map[string]interface{}, error) {
-
 	// connect to monitor
 	conn, err := net.Dial("unix", d.monitorPath())
 	if err != nil {
@@ -701,8 +698,7 @@ func (d *Driver) RunQMPCommand(command string) (map[string]interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
-	_, err = conn.Write(jsonCommand)
-	if err != nil {
+	if _, err := conn.Write(jsonCommand); err != nil {
 		return nil, err
 	}
 	nr, err = conn.Read(buf[:])
@@ -727,8 +723,7 @@ func (d *Driver) RunQMPCommand(command string) (map[string]interface{}, error) {
 	if err != nil {
 		return nil, err
 	}
-	_, err = conn.Write(jsonCommand)
-	if err != nil {
+	if _, err := conn.Write(jsonCommand); err != nil {
 		return nil, err
 	}
 	nr, err = conn.Read(buf[:])
@@ -756,7 +751,7 @@ func WaitForTCPWithDelay(addr string, duration time.Duration) error {
 			continue
 		}
 		defer conn.Close()
-		if _, err = conn.Read(make([]byte, 1)); err != nil {
+		if _, err := conn.Read(make([]byte, 1)); err != nil {
 			time.Sleep(duration)
 			continue
 		}