Merge pull request #14182 from sharifelgamal/firmware-path

fix qemu firmware path locations

pkg/drivers/qemu/qemu.go

@@ -38,6 +38,7 @@ import (
 	"github.com/docker/machine/libmachine/mcnutils"
 	"github.com/docker/machine/libmachine/ssh"
 	"github.com/docker/machine/libmachine/state"
+	"github.com/pkg/errors"
 
 	pkgdrivers "k8s.io/minikube/pkg/drivers"
 )
@@ -79,8 +80,6 @@ type Driver struct {
 	DiskPath         string
 	CacheMode        string
 	IOMode           string
-	//	conn             *libvirt.Connect
-	//	VM               *libvirt.Domain
 	UserDataFile     string
 	CloudConfigRoot  string
 	LocalPorts       string
@@ -271,21 +270,25 @@ 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")
+	}
+
 	minPort, err := strconv.Atoi(portRange[0])
 	if err != nil {
-		return 0, 0, fmt.Errorf("invalid port range")
+		return 0, 0, errors.Wrap(err, "Invalid port range")
 	}
 	maxPort, err := strconv.Atoi(portRange[1])
 	if err != nil {
-		return 0, 0, fmt.Errorf("invalid port range")
+		return 0, 0, errors.Wrap(err, "Invalid port range")
 	}
 
 	if maxPort < minPort {
-		return 0, 0, fmt.Errorf("invalid port range")
+		return 0, 0, errors.New("Invalid port range")
 	}
 
 	if maxPort-minPort < 2 {
-		return 0, 0, fmt.Errorf("port range must be minimum 2 ports")
+		return 0, 0, errors.New("Port range must be minimum 2 ports")
 	}
 
 	return minPort, maxPort, nil
@@ -338,10 +341,6 @@ func (d *Driver) Start() error {
 
 	if d.MachineType != "" {
 		machineType := d.MachineType
-		if runtime.GOOS == "darwin" {
-			// highmem=off needed, see https://patchwork.kernel.org/project/qemu-devel/patch/20201126215017.41156-9-agraf@csgraf.de/#23800615 for details
-			machineType += ",accel=hvf,highmem=off"
-		}
 		startCmd = append(startCmd,
 			"-M", machineType,
 		)
@@ -380,6 +379,13 @@ func (d *Driver) Start() error {
 		}
 	}
 
+	// hardware acceleration is important, it increases performance by 10x
+	if runtime.GOOS == "darwin" {
+		startCmd = append(startCmd, "-accel", "hvf")
+	} else if _, err := os.Stat("/dev/kvm"); err == nil && runtime.GOOS == "linux" {
+		startCmd = append(startCmd, "-accel", "kvm")
+	}
+
 	startCmd = append(startCmd,
 		"-m", fmt.Sprintf("%d", d.Memory),
 		"-smp", fmt.Sprintf("%d", d.CPU),
@@ -419,12 +425,6 @@ func (d *Driver) Start() error {
 
 	startCmd = append(startCmd, "-daemonize")
 
-	// other options
-	// "-enable-kvm" if its available
-	if _, err := os.Stat("/dev/kvm"); err == nil {
-		startCmd = append(startCmd, "-enable-kvm")
-	}
-
 	if d.CloudConfigRoot != "" {
 		startCmd = append(startCmd,
 			"-fsdev",

pkg/minikube/registry/drvs/qemu2/qemu2.go

@@ -18,8 +18,10 @@ package qemu2
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"runtime"
 
@@ -64,17 +66,35 @@ func qemuSystemProgram() (string, error) {
 
 func qemuFirmwarePath() (string, error) {
 	arch := runtime.GOARCH
+	// For macOS, find the correct brew installation path for qemu firmware
+	if runtime.GOOS == "darwin" {
+		var p, fw string
 		switch arch {
 		case "amd64":
-		// on macOS, we assume qemu is installed via homebrew for simplicity
-		if runtime.GOOS == "darwin" {
-			return "/usr/local/Cellar/qemu/6.2.0_1/share/qemu/edk2-x86_64-code.fd", nil
+			p = "/usr/local/Cellar/qemu"
+			fw = "share/qemu/edk2-x86_64-code.fd"
+		case "arm64":
+			p = "/opt/homebrew/Cellar/qemu"
+			fw = "share/qemu/edk2-aarch64-code.fd"
+		default:
+			return "", fmt.Errorf("unknown arch: %s", arch)
 		}
+
+		v, err := ioutil.ReadDir(p)
+		if err != nil {
+			return "", fmt.Errorf("lookup qemu: %v", err)
+		}
+		for _, version := range v {
+			if version.IsDir() {
+				return path.Join(p, version.Name(), fw), nil
+			}
+		}
+	}
+
+	switch arch {
+	case "amd64":
 		return "/usr/share/OVMF/OVMF_CODE.fd", nil
 	case "arm64":
-		if runtime.GOOS == "darwin" {
-			return "/opt/homebrew/Cellar/qemu/6.2.0_1/share/qemu/edk2-aarch64-code.fd", nil
-		}
 		return "/usr/share/AAVMF/AAVMF_CODE.fd", nil
 	default:
 		return "", fmt.Errorf("unknown arch: %s", arch)
@@ -96,6 +116,13 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) {
 	case "arm64":
 		qemuMachine = "virt"
 		qemuCPU = "cortex-a72"
+		// highmem=off needed, see https://patchwork.kernel.org/project/qemu-devel/patch/20201126215017.41156-9-agraf@csgraf.de/#23800615 for details
+		if runtime.GOOS == "darwin" {
+			qemuMachine = "virt,highmem=off"
+		} else if _, err := os.Stat("/dev/kvm"); err == nil {
+			qemuMachine = "virt,gic-version=3"
+			qemuCPU = "host"
+		}
 	default:
 		return nil, fmt.Errorf("unknown arch: %s", runtime.GOARCH)
 	}