Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QEMU Apple Silicon: Find BIOS FD wherever #11453

Merged
merged 1 commit into from
Sep 8, 2021
Merged

QEMU Apple Silicon: Find BIOS FD wherever #11453

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion pkg/machine/qemu/options_darwin_arm64.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package qemu

import (
"os"
"os/exec"
"path/filepath"
)
Expand All @@ -16,7 +17,7 @@ func (v *MachineVM) addArchOptions() []string {
"-accel", "tcg",
"-cpu", "cortex-a57",
"-M", "virt,highmem=off",
"-drive", "file=/usr/local/share/qemu/edk2-aarch64-code.fd,if=pflash,format=raw,readonly=on",
"-drive", "file=" + getEdk2CodeFd("edk2-aarch64-code.fd") + ",if=pflash,format=raw,readonly=on",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the Leading " + " matter here?
Wouldn't this be different then the existing code?

Copy link
Contributor Author

@jonpspri jonpspri Sep 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean by "The leading plus". getEdk2CodeFd will return either /usr/local/share/qemu/edk2-aarch64-code.fd or /opt/homebrew/share/qemu/edk2-aarch64-code.fd. Those get concatenated in where the full filename used to be. If it makes things clearer, I can switch to using a formatted string.

Unless you mean the diff plus... in which case it very definitely matters :).

"-drive", "file=" + ovmfDir + ",if=pflash,format=raw"}
return opts
}
Expand All @@ -35,3 +36,23 @@ func (v *MachineVM) archRemovalFiles() []string {
func getOvmfDir(imagePath, vmName string) string {
return filepath.Join(filepath.Dir(imagePath), vmName+"_ovmf_vars.fd")
}

/*
* QEmu can be installed in multiple locations on MacOS, especially on
* Apple Silicon systems. A build from source will likely install it in
* /usr/local/bin, whereas Homebrew package management standard is to
* install in /opt/homebrew
*/
func getEdk2CodeFd(name string) string {
jonpspri marked this conversation as resolved.
Show resolved Hide resolved
dirs := []string{
"/usr/local/share/qemu",
"/opt/homebrew/share/qemu",
}
for _, dir := range dirs {
fullpath := filepath.Join(dir, name)
if _, err := os.Stat(fullpath); err == nil {
return fullpath
}
}
return name
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case if nothing is found inside loop or stat call fails, it should default to return filepath.Join(dirs[0], name) instead of return name at line no 56. Otherwise default return would be edk2-aarch64-code.fd which is not a valid path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the convention established in options_linux_arm64.go. I believe the rationale is to return a naked filename to pass to qemu and let it deal with trying to find it or erroring out. I considered adding go-style error returns across the package but decided instead to follow already-established patterns.

}