-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
[macos: podman-machine] look for firmware (edk2-code-fd) based on the path of qemu binary #14324
Conversation
this allows users to use a qemu installation that is not in the default /usr/local/bin location a user can configure engine.helper_binaries_dir key or update PATH to include the installation location to find the qemu binary [NO NEW TESTS NEEDED] Signed-off-by: Anjan Nath <[email protected]>
@baude @ashley-cui PTAL |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: anjannath, mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@baude PTAL
@@ -45,6 +63,7 @@ func getOvmfDir(imagePath, vmName string) string { | |||
*/ | |||
func getEdk2CodeFd(name string) string { | |||
dirs := []string{ | |||
getEdk2CodeFdPathFromQemuBinaryPath(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this only be done if the output is NOT ""?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be OK without checking if it's "" empty string, as in the code below at L72 its forming the full path and checking if the file exists at the path before returning.
/lgtm |
this allows users to use a qemu installation that is
not in the default /usr/local/bin location
a user can configure engine.helper_binaries_dir key
or update PATH to include the installation location
to find the qemu binary
Signed-off-by: Anjan Nath [email protected]
Does this PR introduce a user-facing change?