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.

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 check hardcodes filename OVMF_CODE.fd #373

Closed
gibmat opened this issue Jan 7, 2024 · 1 comment · Fixed by #375
Closed

QEMU check hardcodes filename OVMF_CODE.fd #373

gibmat opened this issue Jan 7, 2024 · 1 comment · Fixed by #375
Assignees
Labels
Bug Confirmed to be a bug Easy Good for new contributors
Milestone

Comments

@gibmat
Copy link
Collaborator

gibmat commented Jan 7, 2024

On a Debian sid system, Incus 0.4 fails to properly detect that QEMU is available as a backend. This is caused by a hardcoded check for the file OVMF_CODE.fd (driver_qemu.go line 8309).

Beginning with trixie, Debian's ovmf package only ships the 4M images, which include "_4M" in their file names.

I've come up with the following patch that mimics other bits of code in driver_qemu.go to dynamically determine the correct firmware file. This would essentially be the third copy of the same logic in that file, so I didn't submit a PR because it feels like there should be some refactoring as well.

diff --git a/internal/server/instance/drivers/driver_qemu.go b/internal/server/instance/drivers/driver_qemu.go
index 242364a8c..55eb4ffdc 100644
--- a/internal/server/instance/drivers/driver_qemu.go
+++ b/internal/server/instance/drivers/driver_qemu.go
@@ -8306,7 +8306,27 @@ func (d *qemu) checkFeatures(hostArch int, qemuPath string) (map[string]any, err
 	}
 
 	if d.architectureSupportsUEFI(hostArch) {
-		qemuArgs = append(qemuArgs, "-drive", fmt.Sprintf("if=pflash,format=raw,readonly=on,file=%s", filepath.Join(d.ovmfPath(), "OVMF_CODE.fd")))
+		// Determine expected firmware.
+		firmwares := ovmfGenericFirmwares
+		if util.IsTrue(d.expandedConfig["security.csm"]) {
+			firmwares = ovmfCSMFirmwares
+		} else if util.IsTrueOrEmpty(d.expandedConfig["security.secureboot"]) {
+			firmwares = ovmfSecurebootFirmwares
+		}
+
+		var ovmfCode string
+		for _, firmware := range firmwares {
+			if util.PathExists(filepath.Join(d.ovmfPath(), firmware.code)) {
+				ovmfCode = firmware.code
+				break
+			}
+		}
+
+		if ovmfCode == "" {
+			return nil, fmt.Errorf("Unable to locate matching firmware: %+v", firmwares)
+		}
+
+		qemuArgs = append(qemuArgs, "-drive", fmt.Sprintf("if=pflash,format=raw,readonly=on,file=%s", filepath.Join(d.ovmfPath(), ovmfCode)))
 	}
 
 	var stderr bytes.Buffer
@stgraber stgraber self-assigned this Jan 7, 2024
@stgraber stgraber added Bug Confirmed to be a bug Easy Good for new contributors labels Jan 7, 2024
@stgraber stgraber added this to the incus-0.5 milestone Jan 7, 2024
@stgraber
Copy link
Member

stgraber commented Jan 8, 2024

checkFeatures is a function which is called once on daemon startup to validate that QEMU is functional and detect some QEMU capabilities. It's not tied to any particular instance's config so we don't need that much logic in there.

I think I'll just put in a simple loop to try to find "any" firmware it can use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed to be a bug Easy Good for new contributors
Development

Successfully merging a pull request may close this issue.

2 participants