Merge pull request #80 from AkihiroSuda/dev-override-qemu

Author: AkihiroSuda

docs/internal.md

@@ -59,6 +59,12 @@ The directory contains the following files:
 - `$LIMA_INSTANCE`: `lima ...` is expanded to `limactl shell ${LIMA_INSTANCE} ...`.
   - Default : `default`
 
+- `$QEMU_SYSTEM_X86_64`: path of `qemu-system-x86_64`
+  - Default: `qemu-system-x86_64` in `$PATH`
+
+- `$QEMU_SYSTEM_AARCH64`: path of `qemu-system-aarch64`
+  - Default: `qemu-system-aarch64` in `$PATH`
+
 ## `cidata.iso`
 `cidata.iso` contains the following files:
 

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/gorilla/mux v1.8.0
 	github.com/hashicorp/go-multierror v1.1.1
 	github.com/mattn/go-isatty v0.0.13
+	github.com/mattn/go-shellwords v1.0.12
 	github.com/norouter/norouter v0.6.3
 	github.com/nxadm/tail v1.4.8
 	github.com/pkg/errors v0.9.1

go.sum

@@ -496,6 +496,8 @@ github.com/mattn/go-isatty v0.0.13 h1:qdl+GuBjcsKKDco5BsxPJlId98mSWNKqYA+Co0SC1y
 github.com/mattn/go-isatty v0.0.13/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
 github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
 github.com/mattn/go-shellwords v1.0.3/go.mod h1:3xCvwCdWdlDJUrvuMn7Wuy9eWs4pE8vqg+NOMyg4B2o=
+github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk=
+github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b h1:j7+1HpAFS1zy5+Q4qx1fWh90gTKwiN4QCGoY9TWyyO4=

pkg/hostagent/hostagent.go

@@ -133,12 +133,12 @@ func (a *HostAgent) Run(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	defer logPipeRoutine(a.l, qStdout, "qemu[stdout]")
+	go logPipeRoutine(a.l, qStdout, "qemu[stdout]")
 	qStderr, err := qCmd.StderrPipe()
 	if err != nil {
 		return err
 	}
-	defer logPipeRoutine(a.l, qStderr, "qemu[stderr]")
+	go logPipeRoutine(a.l, qStderr, "qemu[stderr]")
 
 	a.l.Infof("Starting QEMU (hint: to watch the boot progress, see %q)", filepath.Join(a.instDir, filenames.SerialLog))
 	a.l.Debugf("qCmd.Args: %v", qCmd.Args)

pkg/qemu/qemu.go

@@ -7,12 +7,14 @@ import (
 	"path/filepath"
 	"runtime"
 	"strconv"
+	"strings"
 
 	"github.com/AkihiroSuda/lima/pkg/downloader"
 	"github.com/AkihiroSuda/lima/pkg/iso9660util"
 	"github.com/AkihiroSuda/lima/pkg/limayaml"
 	"github.com/AkihiroSuda/lima/pkg/store/filenames"
 	"github.com/docker/go-units"
+	"github.com/mattn/go-shellwords"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -81,38 +83,80 @@ func EnsureDisk(cfg Config) error {
 	return nil
 }
 
+func argValue(args []string, key string) (string, bool) {
+	if !strings.HasPrefix(key, "-") {
+		panic(errors.Errorf("got unexpected key %q", key))
+	}
+	for i, s := range args {
+		if s == key {
+			if i == len(args)-1 {
+				return "", true
+			}
+			value := args[i+1]
+			if strings.HasPrefix(value, "-") {
+				return "", true
+			}
+			return value, true
+		}
+	}
+	return "", false
+}
+
+// appendArgsIfNoConflict can be used for: -cpu, -machine, -m, -boot ...
+// appendArgsIfNoConflict cannot be used for: -drive, -cdrom, ...
+func appendArgsIfNoConflict(args []string, k, v string) []string {
+	if !strings.HasPrefix(k, "-") {
+		panic(errors.Errorf("got unexpected key %q", k))
+	}
+	switch k {
+	case "-drive", "-cdrom", "-chardev", "-blockdev", "-netdev", "-device":
+		panic(errors.Errorf("appendArgsIfNoConflict() must not be called with k=%q", k))
+	}
+
+	if v == "" {
+		if _, ok := argValue(args, k); ok {
+			return args
+		}
+		return append(args, k)
+	}
+
+	if origV, ok := argValue(args, k); ok {
+		logrus.Warnf("Not adding QEMU argument %q %q, as it conflicts with %q %q", k, v, k, origV)
+		return args
+	}
+	return append(args, k, v)
+}
+
 func Cmdline(cfg Config) (string, []string, error) {
 	y := cfg.LimaYAML
-	exeBase := "qemu-system-" + y.Arch
-	exe, err := exec.LookPath(exeBase)
+	exe, args, err := getExe(y.Arch)
 	if err != nil {
 		return "", nil, err
 	}
-	var args []string
 
 	// Architecture
 	accel := getAccel(y.Arch)
 	switch y.Arch {
 	case limayaml.X8664:
 		// NOTE: "-cpu host" seems to cause kernel panic
 		// (MacBookPro 2020, Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz, macOS 11.3, Ubuntu 21.04)
-		args = append(args, "-cpu", "Haswell-v4")
-		args = append(args, "-machine", "q35,accel="+accel)
+		args = appendArgsIfNoConflict(args, "-cpu", "Haswell-v4")
+		args = appendArgsIfNoConflict(args, "-machine", "q35,accel="+accel)
 	case limayaml.AARCH64:
-		args = append(args, "-cpu", "cortex-a72")
-		args = append(args, "-machine", "virt,accel="+accel+",highmem=off")
+		args = appendArgsIfNoConflict(args, "-cpu", "cortex-a72")
+		args = appendArgsIfNoConflict(args, "-machine", "virt,accel="+accel+",highmem=off")
 	}
 
 	// SMP
-	args = append(args, "-smp",
+	args = appendArgsIfNoConflict(args, "-smp",
 		fmt.Sprintf("%d,sockets=1,cores=%d,threads=1", y.CPUs, y.CPUs))
 
 	// Memory
 	memBytes, err := units.RAMInBytes(y.Memory)
 	if err != nil {
 		return "", nil, err
 	}
-	args = append(args, "-m", strconv.Itoa(int(memBytes>>20)))
+	args = appendArgsIfNoConflict(args, "-m", strconv.Itoa(int(memBytes>>20)))
 
 	// Firmware
 	if !y.Firmware.LegacyBIOS {
@@ -132,10 +176,10 @@ func Cmdline(cfg Config) (string, []string, error) {
 		return "", nil, err
 	}
 	if isBaseDiskCDROM {
-		args = append(args, "-boot", "order=d,splash-time=0,menu=on")
+		args = appendArgsIfNoConflict(args, "-boot", "order=d,splash-time=0,menu=on")
 		args = append(args, "-drive", fmt.Sprintf("file=%s,media=cdrom,readonly=on", baseDisk))
 	} else {
-		args = append(args, "-boot", "order=c,splash-time=0,menu=on")
+		args = appendArgsIfNoConflict(args, "-boot", "order=c,splash-time=0,menu=on")
 	}
 	if diskSize, _ := units.RAMInBytes(cfg.LimaYAML.Disk); diskSize > 0 {
 		args = append(args, "-drive", fmt.Sprintf("file=%s,if=virtio", diffDisk))
@@ -156,7 +200,7 @@ func Cmdline(cfg Config) (string, []string, error) {
 
 	// Graphics
 	if y.Video.Display != "" {
-		args = append(args, "-display", y.Video.Display)
+		args = appendArgsIfNoConflict(args, "-display", y.Video.Display)
 	}
 	switch y.Arch {
 	case limayaml.X8664:
@@ -205,6 +249,27 @@ func Cmdline(cfg Config) (string, []string, error) {
 	return exe, args, nil
 }
 
+func getExe(arch limayaml.Arch) (string, []string, error) {
+	exeBase := "qemu-system-" + arch
+	var args []string
+	envK := "QEMU_SYSTEM_" + strings.ToUpper(arch)
+	if envV := os.Getenv(envK); envV != "" {
+		ss, err := shellwords.Parse(envV)
+		if err != nil {
+			return "", nil, errors.Wrapf(err, "failed to parse %s value %q", envK, envV)
+		}
+		exeBase, args = ss[0], ss[1:]
+		if len(args) != 0 {
+			logrus.Warnf("Specifying args (%v) via $%s is supported only for debugging!", args, envK)
+		}
+	}
+	exe, err := exec.LookPath(exeBase)
+	if err != nil {
+		return "", nil, err
+	}
+	return exe, args, nil
+}
+
 func getAccel(arch limayaml.Arch) string {
 	nativeX8664 := arch == limayaml.X8664 && runtime.GOARCH == "amd64"
 	nativeAARCH64 := arch == limayaml.AARCH64 && runtime.GOARCH == "arm64"

pkg/qemu/qemu_test.go

@@ -0,0 +1,49 @@
+package qemu
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestArgValue(t *testing.T) {
+	type testCase struct {
+		key           string
+		expectedValue string
+		expectedOK    bool
+	}
+	args := []string{"-cpu", "foo", "-no-reboot", "-m", "2G", "-s"}
+	testCases := []testCase{
+		{
+			key:           "-cpu",
+			expectedValue: "foo",
+			expectedOK:    true,
+		},
+		{
+			key:           "-no-reboot",
+			expectedValue: "",
+			expectedOK:    true,
+		},
+		{
+			key:           "-m",
+			expectedValue: "2G",
+			expectedOK:    true,
+		},
+		{
+			key:           "-machine",
+			expectedValue: "",
+			expectedOK:    false,
+		},
+		{
+			key:           "-s",
+			expectedValue: "",
+			expectedOK:    true,
+		},
+	}
+
+	for _, tc := range testCases {
+		v, ok := argValue(args, tc.key)
+		assert.Equal(t, tc.expectedValue, v)
+		assert.Equal(t, tc.expectedOK, ok)
+	}
+}