From 9a393b1df8f908830becc6c69ee32d3588f9875b Mon Sep 17 00:00:00 2001 From: kacerr Date: Thu, 10 Sep 2026 09:42:33 +0000 Subject: [PATCH] Boot KVM guests via UEFI by default; per-image firmware setting Run 5 on the real host bisected the guest hang: the Debian 13 image hangs after GRUB under SeaBIOS with or without our seed and with either CPU model, and boots under OVMF. The image's legacy boot chain is the problem, not the provider. images..firmware is "uefi" (default) or "bios". The domain XML emits with secure boot off, so libvirt selects the OVMF binary and manages the NVRAM file, which the existing nvram undefine flag removes. Direct-URL images default to UEFI too. OVMF joins the hypervisor prerequisites; sample, config table, known gaps, decision log and the execution log record the finding. Co-Authored-By: Claude Fable 5.1 --- config/samples/providers-config.yaml | 2 + docs/architecture.md | 7 +++ docs/kvm-provider.md | 13 ++++- .../2026-09-04-1404-kvm-provider.md | 37 +++++++++++++++ internal/provider/config_kvm.go | 8 ++++ internal/provider/config_kvm_test.go | 5 ++ internal/provider/kvm/domainxml.go | 47 ++++++++++++++----- internal/provider/kvm/domainxml_test.go | 22 +++++++++ internal/provider/kvm/image.go | 13 ++--- internal/provider/kvm/image_test.go | 6 +-- internal/provider/kvm/kvm.go | 12 +++-- internal/provider/kvm/kvm_test.go | 5 +- 12 files changed, 151 insertions(+), 26 deletions(-) diff --git a/config/samples/providers-config.yaml b/config/samples/providers-config.yaml index af9e575..871b9d0 100644 --- a/config/samples/providers-config.yaml +++ b/config/samples/providers-config.yaml @@ -40,6 +40,8 @@ providers: debian-13: url: https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2 # sha256: "<64 hex>" # optional; verified after download + # firmware: uefi # default; "bios" only for images with no EFI partition + # # (Debian 13 does not boot via BIOS, see docs/kvm-provider.md) ubuntu-24.04: url: https://cloud-images.ubuntu.com/noble/current/noble-server-cloudimg-amd64.img machineTypes: # placement.machineType names a key here diff --git a/docs/architecture.md b/docs/architecture.md index 328c069..969017a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -454,6 +454,13 @@ they read as choices, not accidents. Chronological by build step. unknown statuses, `nostate` means libvirt could not determine the state at all; answering that with delete-and-recreate is too drastic, so the next poll decides. +- **KVM guests boot UEFI by default, secure boot off.** Found on the + first real host: Debian 13's generic image hangs after GRUB under + SeaBIOS regardless of CPU model or seed, and boots under OVMF. Every + current distro cloud image supports UEFI, so it is the default and + `firmware: bios` is the per-image escape; libvirt's firmware + autoselection (``) keeps the OVMF path out of the + operator's config. Secure boot is off so unsigned images boot too. - **A KVM domain GC cannot read is skipped, a host it cannot reach fails the sweep.** GC never deletes what it does not see, so leaving one domain out is safe; one corrupt domain blocking GC for every other one diff --git a/docs/kvm-provider.md b/docs/kvm-provider.md index d861acb..4c45b08 100644 --- a/docs/kvm-provider.md +++ b/docs/kvm-provider.md @@ -23,7 +23,11 @@ Per host, once: 4. **A storage pool** (default `default`, usually `/var/lib/libvirt/images`) with room for one base image per distro (a few hundred MB) plus the per-VM overlays (`diskSizeGb` each, thin-provisioned). -5. Optionally, **outbound internet from the VMs**: the default +5. **UEFI firmware for guests**: the `ovmf` package on Debian/Ubuntu, + `edk2-ovmf` on Fedora/RHEL. VMs boot UEFI by default (libvirt picks + the OVMF binary itself); a domain start fails with "no firmware + found" without it. Only `firmware: bios` images skip this. +6. Optionally, **outbound internet from the VMs**: the default vendor-data installs `qemu-guest-agent` from the distro mirrors on first boot. Without it the VM still works — the operator falls back to the host's ARP table to find the IP, which fills in once the guest @@ -107,6 +111,7 @@ ssh virt@nas1.home.lan ip link show br0 | `images..url` | yes per entry | | http(s) URL of a cloud image (qcow2 or raw) | | `images..sha256` | | | expected digest; verified after download, a change triggers a re-download | | `images..format` | | `qcow2` | `qcow2` or `raw` | +| `images..firmware` | | `uefi` | `uefi` (OVMF, secure boot off) or `bios` (SeaBIOS) for images without an EFI system partition; Debian 13's BIOS path does not boot, see *Known gaps* | | `machineTypes.` | ≥ 1 | | `{vcpus, memoryMiB}` size profile | | `hosts[].name` | yes | | the `placement.zone` value that selects this host; unique | | `hosts[].address` | yes | | SSH endpoint, `host:port` | @@ -228,6 +233,12 @@ The provider logs through controller-runtime's logger, named `kvm`, with - **IP discovery needs either guest traffic or the agent.** A VM on a bridge with no DHCP never gets an address and stays `Provisioning` forever; check `virsh console`. +- **Debian 13 cloud images do not boot via BIOS on the tested host** + (Ryzen 8845HS, libvirt 11.6, QEMU 10.1): GRUB prints "Booting" and + the kernel never reaches the console, with or without a seed, with + host-passthrough and host-model CPUs alike. The same image boots + under UEFI, which is why `firmware` defaults to `uefi`. Images that + need `firmware: bios` are the exception, not the rule. ## Testing diff --git a/docs/plans-executions/2026-09-04-1404-kvm-provider.md b/docs/plans-executions/2026-09-04-1404-kvm-provider.md index ff9d7b1..00991aa 100644 --- a/docs/plans-executions/2026-09-04-1404-kvm-provider.md +++ b/docs/plans-executions/2026-09-04-1404-kvm-provider.md @@ -477,3 +477,40 @@ outside the operator). Because the provider's own logs were invisible in the test output, this run also produced a side plan: [2026-09-10-0805-kvm-libvirt-tracing](2026-09-10-0805-kvm-libvirt-tracing.md), which traces every libvirt RPC at V(1) and routes it into `-v` output. + +### Run 5: the guest hang is Debian 13's BIOS boot path; UEFI fixes it + +The cached image's sha512 matched Debian's published sum, and +`qemu-img info` showed a sound 3 GiB qcow2, which also confirms that +the raw upload over a libvirt-created volume plus pool refresh yields a +byte-identical file (the Step 3b note is answered). Bisected by hand on +beelink with `virt-install --import` on a fresh overlay of the same +image: + +| variant | seed | CPU | firmware | result | +|---|---|---|---|---| +| operator's domain | ours | host-passthrough | SeaBIOS | hangs after GRUB "Booting", 100 % CPU | +| bare | none | host-passthrough | SeaBIOS | same hang | +| bare | none | host-model | SeaBIOS | same hang | +| bare | none | host-passthrough | **OVMF (`--boot uefi`)** | **boots to login** | + +So neither the seed ISO, the domain XML nor the CPU model was the +problem: the image's legacy GRUB → kernel handoff does not work on this +host, while its EFI stub path does. Not diagnosed further; it is +Debian's chain, not ours, and every current cloud image boots UEFI. + +Change: `images..firmware` (`uefi` default, `bios` escape), +threaded through `withDefaults` → `imageSource` → `domainParams`, and +`buildDomainXML` emits `` so libvirt selects the +OVMF binary and creates the NVRAM file; the `nvram` undefine flag Delete +already passes removes it (seen on the host: `virsh undefine` without +`--nvram` refuses such a domain). Direct-URL images default to UEFI +too. Docs: OVMF is a new hypervisor prerequisite, config table row, +sample comment, known-gaps entry, decision-log entry. + +Worth noting: the first image download went through with no checksum +configured, and the cache reuses a base image by key, so switching +`KVM_TEST_IMAGE_URL` requires deleting `base-test-image.qcow2` and its +marker by hand first. That is the reviewer's Step 4 finding showing up +in practice; still documented rather than fixed. diff --git a/internal/provider/config_kvm.go b/internal/provider/config_kvm.go index 606d643..61521c7 100644 --- a/internal/provider/config_kvm.go +++ b/internal/provider/config_kvm.go @@ -47,6 +47,11 @@ type KVMImage struct { SHA256 string `json:"sha256,omitempty"` // Format is the image's disk format, "qcow2" (default) or "raw". Format string `json:"format,omitempty"` + // Firmware is how VMs from this image boot: "uefi" (default; OVMF with + // secure boot off) or "bios" (SeaBIOS) for images with no EFI system + // partition. Current Debian, Ubuntu, Fedora and Rocky cloud images all + // boot UEFI, and Debian 13's BIOS path was found not to boot at all. + Firmware string `json:"firmware,omitempty"` } // KVMMachineType is a VM size profile. @@ -102,6 +107,9 @@ func validateKVM(prefix string, c *KVMConfig) error { if img.Format != "" && img.Format != "qcow2" && img.Format != "raw" { return fmt.Errorf("%s.format must be qcow2 or raw (got %q)", p, img.Format) } + if img.Firmware != "" && img.Firmware != "uefi" && img.Firmware != "bios" { + return fmt.Errorf("%s.firmware must be uefi or bios (got %q)", p, img.Firmware) + } } if len(c.MachineTypes) == 0 { return fmt.Errorf("%s: kvm.machineTypes must define at least one profile", prefix) diff --git a/internal/provider/config_kvm_test.go b/internal/provider/config_kvm_test.go index 6a00afb..7438e38 100644 --- a/internal/provider/config_kvm_test.go +++ b/internal/provider/config_kvm_test.go @@ -154,6 +154,11 @@ hosts: yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n d: {url: https://x/y.qcow2, sha256: abc}\n" + hostsAndTypes), wantErrSub: `images["d"].sha256 must be 64 hex characters`, }, + { + name: "image with unknown firmware", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n d: {url: https://x/y.qcow2, firmware: coreboot}\n" + hostsAndTypes), + wantErrSub: `images["d"].firmware must be uefi or bios`, + }, { name: "image with unknown format", yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n d: {url: https://x/y.vmdk, format: vmdk}\n" + hostsAndTypes), diff --git a/internal/provider/kvm/domainxml.go b/internal/provider/kvm/domainxml.go index bfdcfe4..4951fac 100644 --- a/internal/provider/kvm/domainxml.go +++ b/internal/provider/kvm/domainxml.go @@ -10,13 +10,15 @@ const guestAgentChannel = "org.qemu.guest_agent.0" // domainParams is everything buildDomainXML needs, already resolved from the // config and placement so the builder stays a pure function. DiskPath and -// SeedPath are the volumes' host paths as reported by libvirt. +// SeedPath are the volumes' host paths as reported by libvirt; Firmware is +// firmwareUEFI or firmwareBIOS. type domainParams struct { Name string UID string CreatedAt time.Time VCPUs int32 MemoryMiB int64 + Firmware string Bridge string DiskPath string SeedPath string @@ -32,17 +34,27 @@ type domainParams struct { // to sources that carry a path, so a volume-referenced disk fails to start // with "Permission denied" on any AppArmor-confined host (stock Debian and // Ubuntu). The paths cost one StorageVolGetPath round-trip each per Create. +// +// UEFI is expressed as with secure boot off, and libvirt +// picks the OVMF binary and creates the NVRAM file itself (removed again by +// the nvram undefine flag). Debian 13's BIOS boot path was found to hang +// after GRUB on a real host, while the same image boots under UEFI. func buildDomainXML(p domainParams) (string, error) { + os := osXML{ + Type: osTypeXML{Arch: "x86_64", Machine: "q35", Value: "hvm"}, + Boot: bootXML{Dev: "hd"}, + } + if p.Firmware == firmwareUEFI { + os.Firmware = "efi" + os.FirmwareFeatures = &firmwareXML{Features: []firmwareFeatureXML{{Enabled: "no", Name: "secure-boot"}}} + } d := domainXML{ - Type: "kvm", - Name: p.Name, - Metadata: domainMetadataXML{Instance: newInstanceMetadata(p.UID, p.CreatedAt)}, - Memory: sizeXML{Unit: "MiB", Value: p.MemoryMiB}, - VCPU: p.VCPUs, - OS: osXML{ - Type: osTypeXML{Arch: "x86_64", Machine: "q35", Value: "hvm"}, - Boot: bootXML{Dev: "hd"}, - }, + Type: "kvm", + Name: p.Name, + Metadata: domainMetadataXML{Instance: newInstanceMetadata(p.UID, p.CreatedAt)}, + Memory: sizeXML{Unit: "MiB", Value: p.MemoryMiB}, + VCPU: p.VCPUs, + OS: os, Features: featuresXML{ACPI: &struct{}{}, APIC: &struct{}{}}, CPU: cpuXML{Mode: "host-passthrough"}, Clock: clockXML{Offset: "utc"}, @@ -115,8 +127,19 @@ type sizeXML struct { } type osXML struct { - Type osTypeXML `xml:"type"` - Boot bootXML `xml:"boot"` + Firmware string `xml:"firmware,attr,omitempty"` + Type osTypeXML `xml:"type"` + FirmwareFeatures *firmwareXML `xml:"firmware"` + Boot bootXML `xml:"boot"` +} + +type firmwareXML struct { + Features []firmwareFeatureXML `xml:"feature"` +} + +type firmwareFeatureXML struct { + Enabled string `xml:"enabled,attr"` + Name string `xml:"name,attr"` } type osTypeXML struct { diff --git a/internal/provider/kvm/domainxml_test.go b/internal/provider/kvm/domainxml_test.go index b401670..8ba3803 100644 --- a/internal/provider/kvm/domainxml_test.go +++ b/internal/provider/kvm/domainxml_test.go @@ -14,6 +14,7 @@ func testDomainParams() domainParams { CreatedAt: time.Date(2026, 9, 4, 12, 0, 0, 0, time.UTC), VCPUs: 2, MemoryMiB: 2048, + Firmware: firmwareUEFI, Bridge: "br0", DiskPath: "/var/lib/libvirt/images/proxy-abc123def456ghij.qcow2", SeedPath: "/var/lib/libvirt/images/proxy-abc123def456ghij-seed.iso", @@ -40,6 +41,10 @@ func TestBuildDomainXML_fieldByField(t *testing.T) { if d.OS.Type.Value != "hvm" || d.OS.Type.Arch != "x86_64" || d.OS.Type.Machine != "q35" || d.OS.Boot.Dev != "hd" { t.Errorf("os = %+v", d.OS) } + if d.OS.Firmware != "efi" || d.OS.FirmwareFeatures == nil || len(d.OS.FirmwareFeatures.Features) != 1 || + d.OS.FirmwareFeatures.Features[0] != (firmwareFeatureXML{Enabled: "no", Name: "secure-boot"}) { + t.Errorf("os firmware = %q %+v, want efi with secure-boot disabled", d.OS.Firmware, d.OS.FirmwareFeatures) + } if d.Features.ACPI == nil || d.Features.APIC == nil { t.Errorf("features = %+v, want acpi and apic", d.Features) } @@ -113,3 +118,20 @@ func TestBuildDomainXML_escapesUserValues(t *testing.T) { t.Errorf("bridge = %q, want the raw value back", d.Devices.Interfaces[0].Source.Bridge) } } + +func TestBuildDomainXML_biosFirmware(t *testing.T) { + t.Parallel() + p := testDomainParams() + p.Firmware = firmwareBIOS + raw, err := buildDomainXML(p) + if err != nil { + t.Fatalf("buildDomainXML() error = %v", err) + } + var d domainXML + if err := xml.Unmarshal([]byte(raw), &d); err != nil { + t.Fatalf("output is not well-formed XML: %v", err) + } + if d.OS.Firmware != "" || d.OS.FirmwareFeatures != nil || strings.Contains(raw, "firmware") { + t.Errorf("bios domain still carries firmware settings:\n%s", raw) + } +} diff --git a/internal/provider/kvm/image.go b/internal/provider/kvm/image.go index a13645c..5fcbda6 100644 --- a/internal/provider/kvm/image.go +++ b/internal/provider/kvm/image.go @@ -22,10 +22,11 @@ import ( // imageSource is a resolved placement.image: either an entry from the // provider's images map (Key set) or a URL given directly. type imageSource struct { - Key string - URL string - SHA256 string - Format string + Key string + URL string + SHA256 string + Format string + Firmware string } // resolveImage maps placement.image to a source. Anything that is neither a @@ -33,11 +34,11 @@ type imageSource struct { // surface, not retry. func resolveImage(cfg provider.KVMConfig, image string) (imageSource, error) { if img, ok := cfg.Images[image]; ok { - return imageSource{Key: image, URL: img.URL, SHA256: img.SHA256, Format: img.Format}, nil + return imageSource{Key: image, URL: img.URL, SHA256: img.SHA256, Format: img.Format, Firmware: img.Firmware}, nil } u, err := url.Parse(image) if err == nil && (u.Scheme == "http" || u.Scheme == "https") && u.Host != "" { - return imageSource{URL: image, Format: formatQcow2}, nil + return imageSource{URL: image, Format: formatQcow2, Firmware: firmwareUEFI}, nil } return imageSource{}, fmt.Errorf("%w: placement.image %q is neither a key in kvm.images nor an http(s) URL", provider.ErrPermanent, image) diff --git a/internal/provider/kvm/image_test.go b/internal/provider/kvm/image_test.go index 2493c44..93ffdc6 100644 --- a/internal/provider/kvm/image_test.go +++ b/internal/provider/kvm/image_test.go @@ -88,9 +88,9 @@ func TestResolveImage(t *testing.T) { want imageSource wantErr bool }{ - {"configured key", "debian-13", imageSource{Key: "debian-13", URL: "https://cloud.debian.org/d.qcow2", SHA256: "abc", Format: "qcow2"}, false}, - {"direct https url", "https://x.example/img.qcow2", imageSource{URL: "https://x.example/img.qcow2", Format: "qcow2"}, false}, - {"direct http url", "http://mirror/img.qcow2", imageSource{URL: "http://mirror/img.qcow2", Format: "qcow2"}, false}, + {"configured key", "debian-13", imageSource{Key: "debian-13", URL: "https://cloud.debian.org/d.qcow2", SHA256: "abc", Format: "qcow2", Firmware: "uefi"}, false}, + {"direct https url", "https://x.example/img.qcow2", imageSource{URL: "https://x.example/img.qcow2", Format: "qcow2", Firmware: "uefi"}, false}, + {"direct http url", "http://mirror/img.qcow2", imageSource{URL: "http://mirror/img.qcow2", Format: "qcow2", Firmware: "uefi"}, false}, {"unknown key", "ubuntu", imageSource{}, true}, {"file url", "file:///tmp/x.qcow2", imageSource{}, true}, {"empty", "", imageSource{}, true}, diff --git a/internal/provider/kvm/kvm.go b/internal/provider/kvm/kvm.go index ed4613e..a42e5eb 100644 --- a/internal/provider/kvm/kvm.go +++ b/internal/provider/kvm/kvm.go @@ -28,6 +28,8 @@ const ( defaultPool = "default" formatQcow2 = "qcow2" formatRaw = "raw" + firmwareUEFI = "uefi" + firmwareBIOS = "bios" // undefineFlags removes everything libvirt may keep alongside a domain // definition, so an undefine never fails on leftover state. @@ -48,6 +50,9 @@ func withDefaults(cfg provider.KVMConfig) provider.KVMConfig { if img.Format == "" { img.Format = formatQcow2 } + if img.Firmware == "" { + img.Firmware = firmwareUEFI + } images[k] = img } cfg.Images = images @@ -199,7 +204,7 @@ func (p *Provider) Create(ctx context.Context, req provider.CreateRequest) (stri if err != nil { return fmt.Errorf("resolving seed path: %w", err) } - dom, err = p.defineDomain(log, api, host, req, mt, diskPath, seedPath) + dom, err = p.defineDomain(log, api, host, req, mt, src.Firmware, diskPath, seedPath) if err != nil { return err } @@ -263,13 +268,14 @@ func (p *Provider) writeSeed(log logr.Logger, api libvirtAPI, pool libvirt.Stora return vol, nil } -func (p *Provider) defineDomain(log logr.Logger, api libvirtAPI, host provider.KVMHost, req provider.CreateRequest, mt provider.KVMMachineType, diskPath, seedPath string) (libvirt.Domain, error) { +func (p *Provider) defineDomain(log logr.Logger, api libvirtAPI, host provider.KVMHost, req provider.CreateRequest, mt provider.KVMMachineType, firmware, diskPath, seedPath string) (libvirt.Domain, error) { xml, err := buildDomainXML(domainParams{ Name: req.Name, UID: req.UID, CreatedAt: p.now(), VCPUs: mt.VCPUs, MemoryMiB: mt.MemoryMiB, + Firmware: firmware, Bridge: host.Bridge, DiskPath: diskPath, SeedPath: seedPath, @@ -281,7 +287,7 @@ func (p *Provider) defineDomain(log logr.Logger, api libvirtAPI, host provider.K if err != nil { return libvirt.Domain{}, fmt.Errorf("defining domain: %w", err) } - log.V(1).Info("domain defined", "uid", req.UID) + log.V(1).Info("domain defined", "uid", req.UID, "firmware", firmware) return dom, nil } diff --git a/internal/provider/kvm/kvm_test.go b/internal/provider/kvm/kvm_test.go index c1f3603..17c7190 100644 --- a/internal/provider/kvm/kvm_test.go +++ b/internal/provider/kvm/kvm_test.go @@ -20,7 +20,7 @@ func TestWithDefaults(t *testing.T) { cfg := withDefaults(provider.KVMConfig{ Images: map[string]provider.KVMImage{ "debian": {URL: "https://x/d.qcow2"}, - "raw": {URL: "https://x/r.img", Format: "raw"}, + "raw": {URL: "https://x/r.img", Format: "raw", Firmware: "bios"}, }, Hosts: []provider.KVMHost{ {Name: "a", Address: "a:22", User: "virt", Bridge: "br0"}, @@ -36,6 +36,9 @@ func TestWithDefaults(t *testing.T) { if cfg.Images["debian"].Format != "qcow2" || cfg.Images["raw"].Format != "raw" { t.Errorf("image formats = %q/%q, want qcow2 default and raw kept", cfg.Images["debian"].Format, cfg.Images["raw"].Format) } + if cfg.Images["debian"].Firmware != firmwareUEFI || cfg.Images["raw"].Firmware != firmwareBIOS { + t.Errorf("image firmware = %q/%q, want uefi default and bios kept", cfg.Images["debian"].Firmware, cfg.Images["raw"].Firmware) + } if a := cfg.Hosts[0]; a.Socket != defaultSocket || a.Pool != defaultPool { t.Errorf("host a = %+v, want default socket and pool", a) }