Point the manager config and sample at the beelink KVM host

The default manager config gains a kvm-home provider reaching beelink
over Tailscale, the provider sample names the same host, and the
manager kustomization uses the locally built kvm-dev image.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -1,5 +1,91 @@
 # Changelog
 
+## 2026-09-10 22:58 UTC — Code-review fixes on the KVM branch: deletion, validation, lazy SSH, guest agent
+
+Five findings from a review of `feat/kvm-provider`, all fixed on the branch.
+
+- **A Proxy can always finish deleting.** The delete path used to drop the finalizer
+  only when `Get` answered not-found, so a permanent error there retried forever —
+  a KVM host removed from the config was enough. It now calls `Delete` anyway, since
+  that is idempotent and can finish where `Get` cannot, and
+  `crawl.example.com/skip-cleanup: "true"` abandons the instance for whatever stays
+  stuck. In the KVM provider, deleting over an unconfigured host is a no-op success;
+  `Get` still fails permanently there so a typo cannot look like a missing VM.
+- **The guest agent ships as a vendor-data script**, superseding the cloud-config
+  merge in the entry below. Merging could never be made safe: cloud-init drops
+  vendor-data keys the user-data defines, and the strategy cannot be overridden from
+  either document. The Proxy's cloud-init now reaches the seed byte for byte in any
+  format — Jinja, MIME, gzip, scripts — where the round-trip previously re-typed
+  unquoted scalars (`version: 1.10` arrived as `1.1`), dropped comments and reordered
+  keys. New `installGuestAgent` setting, default on.
+- **A missing `kvm-ssh` Secret no longer crash-loops the manager.** The SSH material
+  is read on first use and cached, so only KVM Proxies fail and they recover when the
+  Secret appears. A missing file is transient, an unparseable key permanent, and
+  startup still logs a warning.
+- **Host names and image keys are validated.** Either can end up in a providerID or a
+  volume file name, where a slash made the VM unaddressable for good.
+- Verified end to end: unit suite, the integration test against a real hypervisor
+  (22 s, agent reporting the IP), both in-cluster Proxies Ready in 2 min with 204
+  through squid, the guest showing `qemu-guest-agent` active with its cloud-config
+  comments intact, and the manager staying up with the Secret pointed away.
+- Key files: `internal/provider/kvm/{seed.go,conn.go,kvm.go}`,
+  `internal/controller/proxy_controller.go`, `internal/provider/config_kvm.go`,
+  `api/v1alpha1/proxy_types.go`.
+
+## 2026-09-10 21:04 UTC — KVM provider passes in-cluster; guest agent merged into cloud-config
+
+- Two KVM Proxies with real cloud-configs reach `Ready` from inside the kind cluster
+  (2 min from apply, IP via the guest agent, 204 through squid from the LAN).
+- The `qemu-guest-agent` bootstrap is now merged into each Proxy's cloud-config
+  (`packages` + `runcmd`) instead of shipped as vendor-data: cloud-init's default
+  merger drops vendor-data keys the user-data also defines, so any Proxy with its own
+  `packages:` never got the agent and stayed `Provisioning` forever. `vendorData` is
+  now plain extra vendor-data with no default; non-cloud-config user-data (scripts,
+  `#include`) still gets the bootstrap as vendor-data. A cloud-config that is not a
+  YAML mapping fails the Proxy permanently instead of booting without its config.
+- The manager pod mounts a 64 MiB `emptyDir` on `/tmp`: the seed ISO writer stages
+  files on disk and the root filesystem is read-only.
+- Errors already classified with a provider sentinel no longer drop the cached
+  libvirt SSH connection; only libvirt and transport errors do.
+- Key files: `internal/provider/kvm/{seed.go,conn.go,errors.go}`,
+  `config/manager/manager.yaml`, `docs/kvm-provider.md`.
+
+## 2026-09-10 10:07 UTC — KVM provider validated on a real hypervisor; UEFI default; libvirt call tracing
+
+- `make test-integration-kvm` passes end to end against a libvirt 11.6 / QEMU 10.1
+  host (Debian 13 image, UEFI, guest agent reporting the IP after ~20 s). Four
+  defects found only on the real host, all fixed on the branch:
+  - domain disks are now `type="file"` with resolved paths: pool-volume references
+    get no AppArmor rule from `virt-aa-helper`, so QEMU was denied the disks;
+  - domain metadata is parsed by local name: libvirt returns the element without
+    its namespace declaration;
+  - guests boot **UEFI by default** (`images.<key>.firmware: uefi|bios`, OVMF now a
+    host prerequisite): Debian 13's image hangs in GRUB under SeaBIOS on a headless
+    VM with a serial console (Debian bug #1111240, open); bookworm is unaffected;
+  - the image cache's raw-upload path is confirmed to produce a byte-identical file.
+- Every libvirt RPC is logged at `--zap-log-level=debug` as one `libvirt call` line
+  (`rpc=virDomainCreate`, identifiers, duration, `libvirtError` on failure); level 2
+  adds XML, metadata and addresses; cloud-init is never logged. The integration test
+  shows the trace in `-v` output (`KVM_TEST_LOG_LEVEL`, default 1).
+- Key files: `internal/provider/kvm/{domainxml.go,metadata.go,trace.go,conn.go}`,
+  `internal/provider/config_kvm.go`, `docs/kvm-provider.md`.
+
+## 2026-09-08 11:00 UTC — KVM/libvirt provider
+
+- New `kvm` provider type: Managed proxies as VMs on your own libvirt/KVM hosts. Each
+  host is reached over SSH with the libvirt unix socket forwarded through the
+  connection (pure Go, nothing to expose on the hypervisors beyond sshd); one provider
+  instance can span several hosts, selected by `placement.zone`.
+- Base images (`images:` map or a direct URL in `placement.image`) are downloaded by the
+  operator into each host's storage pool on first use, verified by sha256 when configured,
+  and cached there. Per-VM disks are qcow2 overlays; cloud-init arrives on a NoCloud seed
+  ISO built in-process. GC identity lives in the domain's `<metadata>` element.
+- Wiring: `"kvm": kvm.New` in `cmd/main.go`; optional `kvm-ssh` Secret mounted at
+  `/etc/proxy-operator/kvm`; samples `config/samples/proxy_kvm.yaml` and a `kvm-home`
+  block in `providers-config.yaml`; `docs/kvm-provider.md`; opt-in integration test
+  (`make test-integration-kvm`) against a real hypervisor.
+- Not validated on a real host at the time of this entry; see 2026-09-10 above.
+
 ## 2026-08-10 09:34 CEST — kind e2e verified; fix Squid OOM in containers; quickstart goes in-cluster
 
 - Full end-to-end pass on a throwaway kind cluster: Squid pod Ready with a real CONNECT

Makefile

@@ -178,6 +178,20 @@ uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified
 	@out="$$( "$(KUSTOMIZE)" build config/crd 2>/dev/null || true )"; \
 	if [ -n "$$out" ]; then echo "$$out" | "$(KUBECTL)" delete --ignore-not-found=$(ignore-not-found) -f -; else echo "No CRDs to delete; skipping."; fi
 
+KVM_SSH_KEY ?= $(HOME)/.ssh/proxy-operator_ed25519
+KVM_KNOWN_HOSTS ?= $(HOME)/.ssh/kvm_known_hosts
+KVM_SSH_NAMESPACE ?= egress-proxies-operator-system
+
+.PHONY: kvm-ssh-secret
+kvm-ssh-secret: ## Create or update the kvm-ssh Secret from KVM_SSH_KEY (private key) and KVM_KNOWN_HOSTS.
+	@test -f "$(KVM_SSH_KEY)" || { echo "KVM_SSH_KEY=$(KVM_SSH_KEY) not found"; exit 1; }
+	@test -f "$(KVM_KNOWN_HOSTS)" || { echo "KVM_KNOWN_HOSTS=$(KVM_KNOWN_HOSTS) not found"; exit 1; }
+	@grep -q 'PRIVATE KEY' "$(KVM_SSH_KEY)" || { echo "KVM_SSH_KEY=$(KVM_SSH_KEY) is not a private key (did you pass the .pub?)"; exit 1; }
+	"$(KUBECTL)" -n $(KVM_SSH_NAMESPACE) create secret generic kvm-ssh \
+		--from-file=id_ed25519="$(KVM_SSH_KEY)" \
+		--from-file=known_hosts="$(KVM_KNOWN_HOSTS)" \
+		--dry-run=client -o yaml | "$(KUBECTL)" apply -f -
+
 .PHONY: deploy
 deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config.
 	cd config/manager && "$(KUSTOMIZE)" edit set image controller=${IMG}

README.md

@@ -15,9 +15,10 @@ itself**, and hands out to crawler clients via an HTTP list/lease API.
   cattle**: any meaningful spec change (placement, cloud-init, port)
   deletes and recreates the VM — never in-place mutation.
 - **Providers** behind one minimal interface: `kubernetes` (a real Squid
-  pod in this cluster — local dev/CI) and `gcp` (Compute Engine VMs with
+  pod in this cluster — local dev/CI), `gcp` (Compute Engine VMs with
-  ephemeral external IPs — the real egress fleet). Config is a YAML file
+  ephemeral external IPs — the real egress fleet) and `kvm` (VMs on your
-  (`--providers-config`) with named instances (`gcp-eu`, `gcp-us`, ...).
+  own libvirt hypervisors, reached over SSH). Config is a YAML file
+  (`--providers-config`) with named instances (`gcp-eu`, `kvm-home`, ...).
 - **Health engine** probes every proxy by fetching a URL *through* it (a
   real CONNECT tunnel — a proxy that accepts TCP but can't egress goes
   Unhealthy), with threshold logic and transition-only status writes.
@@ -131,11 +132,44 @@ make deploy IMG=<registry>/egress-proxies-operator:dev
    which also installs Squid via cloud-init). A missing field fails the
    Proxy with a message naming it.
 
+## KVM setup
+
+Managed proxies as VMs on libvirt/KVM hosts you own. Full guide:
+[docs/kvm-provider.md](docs/kvm-provider.md).
+
+1. Each host needs `libvirtd`, an SSH user in the `libvirt` group, a
+   bridge to a network with DHCP, and a storage pool.
+2. Create the `kvm-ssh` Secret from the operator's **private** key and a
+   `known_hosts` with each host's key; the manager mounts it at
+   `/etc/proxy-operator/kvm`:
+
+   ```sh
+   ssh-keyscan -H <host> >> ~/.ssh/kvm_known_hosts
+   kubectl -n egress-proxies-operator-system create secret generic kvm-ssh \
+     --from-file=id_ed25519=$HOME/.ssh/proxy-operator_ed25519 \
+     --from-file=known_hosts=$HOME/.ssh/kvm_known_hosts
+   # or: make kvm-ssh-secret   (same paths by default; KVM_SSH_KEY / KVM_KNOWN_HOSTS to override)
+   ```
+
+3. Add a `kvm` entry to the providers config with `images`,
+   `machineTypes` and `hosts` (see
+   [config/samples/providers-config.yaml](config/samples/providers-config.yaml)).
+   Base images are downloaded into each host's pool on first use.
+4. A Proxy names a host (`placement.zone`, optional with one host), a
+   size (`placement.machineType`) and an image (`placement.image`) — see
+   [config/samples/proxy_kvm.yaml](config/samples/proxy_kvm.yaml).
+
 Cloud-init from a Secret: the Secret **must** carry the label
 `crawl.example.com/cloud-init: "true"` — the operator's cache only holds
 labelled Secrets, so an unlabelled one is invisible (the Proxy reports
 `CloudInitError`). Rotating the Secret's content triggers VM replacement.
 
+Stuck deletion: a Proxy stays `Terminating` while the operator keeps failing
+to clean its instance up — credentials revoked, a provider or a KVM host
+dropped from the config. Annotate it with
+`crawl.example.com/skip-cleanup: "true"` to abandon the instance and release
+the object. Whatever is still running then has to be removed by hand.
+
 ## Providers
 
 `Managed` proxies are provisioned by a provider — a small compute backend

api/v1alpha1/proxy_types.go

@@ -62,6 +62,15 @@ const (
 	// must be replaced.
 	AnnotationSpecHash = "crawl.example.com/spec-hash"
 
+	// AnnotationSkipCleanup abandons the provider-side instance: set it to
+	// "true" and deletion removes the finalizer without asking the provider
+	// to clean anything up. It is the escape hatch for a cleanup that cannot
+	// succeed — a provider dropped from the config, credentials revoked, a
+	// backend that answers permanently — where the object would otherwise
+	// stay in Terminating forever. The instance is left running and becomes
+	// the administrator's to remove.
+	AnnotationSkipCleanup = "crawl.example.com/skip-cleanup"
+
 	// LabelCloudInit must be set (to "true") on every Secret referenced by
 	// spec.cloudInit.secretRef: the manager's cache only holds Secrets
 	// carrying this label, so an unlabelled Secret is invisible to the

cmd/main.go

@@ -58,6 +58,7 @@ import (
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider"
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/gcp"
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/kubernetes"
+	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/kvm"
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/registry"
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/tracing"
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/version"
@@ -187,6 +188,7 @@ func main() {
 		"gcp": func(ctx context.Context, pc provider.ProviderConfig) (provider.Provider, error) {
 			return gcp.NewWithWireOptions(ctx, pc, gcp.WireLogOptions{FullPayloads: gcpWireFullPayloads})
 		},
+		"kvm": kvm.New,
 	})
 	if err != nil {
 		setupLog.Error(err, "Failed to build providers")

config/manager/kustomization.yaml

@@ -5,5 +5,5 @@ apiVersion: kustomize.config.k8s.io/v1beta1
 kind: Kustomization
 images:
 - name: controller
-  newName: example.com/egress-proxies-operator
+  newName: egress-proxies-operator
-  newTag: v0.0.1
+  newTag: kvm-dev

config/manager/manager.yaml

@@ -145,9 +145,30 @@ spec:
         - name: providers-config
           mountPath: /etc/proxy-operator
           readOnly: true
+        # SSH material for kvm providers (sshKeyFile / knownHostsFile in the
+        # providers config point here). Optional: the mount is empty when the
+        # Secret does not exist, and only kvm entries need it. Create it with:
+        #   kubectl -n egress-proxies-operator-system create secret generic kvm-ssh \
+        #     --from-file=id_ed25519=<private key> \
+        #     --from-file=known_hosts=<known_hosts with every host's key>
+        - name: kvm-ssh
+          mountPath: /etc/proxy-operator/kvm
+          readOnly: true
+        # Scratch space: the root filesystem is read-only, and the kvm
+        # provider builds each VM's cloud-init seed ISO through a temp
+        # directory (a few hundred KB, deleted right after upload).
+        - name: tmp
+          mountPath: /tmp
       volumes:
       - name: providers-config
         configMap:
           name: providers-config
+      - name: kvm-ssh
+        secret:
+          secretName: kvm-ssh
+          optional: true
+      - name: tmp
+        emptyDir:
+          sizeLimit: 64Mi
       serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10

config/manager/providers_config.yaml

@@ -9,9 +9,46 @@ metadata:
 data:
   # Mounted at /etc/proxy-operator/providers.yaml (--providers-config).
   # The default ships only the kubernetes-pod provider so the operator runs
-  # out of the box; add gcp entries (type: gcp, gcp.project: ...) for real
+  # out of the box; add gcp entries (type: gcp, gcp.project: ...) or kvm
-  # egress fleets — see config/samples/providers-config.yaml.
+  # entries (type: kvm, hosts reached over SSH) for real egress fleets —
+  # see config/samples/providers-config.yaml.
   providers.yaml: |
     providers:
       - name: kubernetes
         type: kubernetes
+      - name: kvm-home
+        type: kvm
+        kvm:
+          # SSH to each host with the libvirt unix socket forwarded through the
+          # connection; nothing to open on the hypervisors beyond sshd. Both
+          # files come from the optional kvm-ssh Secret mounted by
+          # config/manager/manager.yaml — see docs/kvm-provider.md.
+          sshKeyFile: /etc/proxy-operator/kvm/id_ed25519
+          knownHostsFile: /etc/proxy-operator/kvm/known_hosts
+          # insecureIgnoreHostKey: true   # skip host-key checks (not recommended)
+          # diskSizeGb: 10                # per-VM overlay; cloud-init grows the root fs into it
+          # vendorData: |                 # extra vendor-data; the qemu-guest-agent bootstrap is merged
+          #   #cloud-config               # into each Proxy's cloud-config, not configured here
+          #   ...
+          images:
+            # placement.image names a key here (or is an https:// URL directly).
+            # Fetched into each host's pool on first use and cached there as
+            # base-<key>.qcow2; do not change an entry while Proxies use it.
+            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 hangs in GRUB under BIOS headless: Debian bug #1111240)
+            ubuntu-24.04:
+              url: https://cloud-images.ubuntu.com/noble/current/noble-server-cloudimg-amd64.img
+          machineTypes:                   # placement.machineType names a key here
+            small:  {vcpus: 1, memoryMiB: 1024}
+            medium: {vcpus: 2, memoryMiB: 2048}
+          hosts:                          # placement.zone names a host; optional with one host
+            - name: beelink
+              address: beelink.tail8c69c.ts.net:22
+              user: novakj                  # must be in the libvirt group
+              bridge: br0                 # to a LAN with DHCP; the VM's IP comes from there
+              # socket: /var/run/libvirt/libvirt-sock
+              # pool: default
+

config/samples/kustomization.yaml

@@ -4,5 +4,6 @@
 resources:
 - proxy_kubernetes.yaml
 - proxy_gcp.yaml
+- proxy_kvm.yaml
 - proxy_external.yaml
 # +kubebuilder:scaffold:manifestskustomizesamples

config/samples/providers-config.yaml

@@ -19,3 +19,39 @@ providers:
       # diskSizeGb: 10
       # auth: Application Default Credentials (workload identity
       # in-cluster, gcloud ADC locally). No key-file plumbing.
+  - name: kvm-home
+    type: kvm
+    kvm:
+      # SSH to each host with the libvirt unix socket forwarded through the
+      # connection; nothing to open on the hypervisors beyond sshd. Both
+      # files come from the optional kvm-ssh Secret mounted by
+      # config/manager/manager.yaml — see docs/kvm-provider.md.
+      sshKeyFile: /etc/proxy-operator/kvm/id_ed25519
+      knownHostsFile: /etc/proxy-operator/kvm/known_hosts
+      # insecureIgnoreHostKey: true   # skip host-key checks (not recommended)
+      # diskSizeGb: 10                # per-VM overlay; cloud-init grows the root fs into it
+      # installGuestAgent: false      # skip the vendor-data script that installs qemu-guest-agent
+      # vendorData: |                 # extra vendor-data, carried beside the agent script as a
+      #   #cloud-config               # MIME multipart. The Proxy's own cloud-init is never rewritten.
+      #   ...
+      images:
+        # placement.image names a key here (or is an https:// URL directly).
+        # Fetched into each host's pool on first use and cached there as
+        # base-<key>.qcow2; do not change an entry while Proxies use it.
+        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 hangs in GRUB under BIOS headless: Debian bug #1111240)
+        ubuntu-24.04:
+          url: https://cloud-images.ubuntu.com/noble/current/noble-server-cloudimg-amd64.img
+      machineTypes:                   # placement.machineType names a key here
+        small:  {vcpus: 1, memoryMiB: 1024}
+        medium: {vcpus: 2, memoryMiB: 2048}
+      hosts:                          # placement.zone names a host; optional with one host
+        - name: beelink
+          address: beelink:22
+          user: novakj                  # must be in the libvirt group
+          bridge: br0                 # to a LAN with DHCP; the VM's IP comes from there
+          # socket: /var/run/libvirt/libvirt-sock
+          # pool: default

config/samples/proxy_kvm.yaml

@@ -0,0 +1,43 @@
+# A Managed proxy backed by a libvirt/KVM hypervisor on the home network:
+# the operator boots a stock Debian cloud image as a VM on the named host,
+# with a NIC on the host's bridge (IP from the LAN's DHCP), and installs
+# Squid via cloud-init. Requires a providers-config entry named "kvm-home"
+# (see providers-config.yaml) and the kvm-ssh Secret (docs/kvm-provider.md).
+#
+# placement.zone is the host name from kvm.hosts (optional when only one
+# host is configured); machineType and image are keys in the provider's
+# machineTypes and images maps. The first Proxy for an image on a host
+# waits for the image download; later ones reuse the cached copy.
+apiVersion: crawl.example.com/v1alpha1
+kind: Proxy
+metadata:
+  name: proxy-kvm-sample
+spec:
+  mode: Managed
+  provider: kvm-home
+  placement:
+    zone: nas1
+    machineType: small
+    image: debian-13
+  port: 3128
+  cloudInit:
+    inline: |
+      #cloud-config
+      packages:
+        - squid
+      write_files:
+        - path: /etc/squid/conf.d/proxy-operator.conf
+          content: |
+            http_port 3128
+            http_access allow all
+            via off
+            forwarded_for off
+      runcmd:
+        - systemctl restart squid
+      # Optional: log in with `ssh debian@<ip>` (the Debian image's default
+      # user) to look around; `virsh console <name>` on the host works too.
+      # ssh_authorized_keys:
+      #   - ssh-ed25519 AAAA... you@laptop
+  attributes:
+    geo: home
+    purpose: crawl

docs/architecture.md

@@ -9,6 +9,7 @@
 | Provider contract | `internal/provider` | library | — | `Provider` interface, error taxonomy, deterministic naming, config, metrics decorator |
 | kubernetes provider | `internal/provider/kubernetes` | library | — | real Squid pods in this cluster (local dev/CI) |
 | gcp provider | `internal/provider/gcp` | library | — | Compute Engine VMs, four API calls, fire-and-forget ops |
+| kvm provider | `internal/provider/kvm` | library | — | VMs on libvirt hosts over SSH-forwarded sockets; image cache, overlay disks, NoCloud seeds |
 | Health engine | `internal/health` | Runnable | yes | through-the-proxy probes, thresholds, transition events |
 | Lease store | `internal/lease` | Runnable (expiry sweep) | no | in-memory leases + cooldowns, single mutex |
 | Discovery API | `internal/discovery` | Runnable | no | HTTP list/lease/release/report on `:8090` |
@@ -147,6 +148,25 @@ prov.ListByTag ─► AggregatedList(label filter,                   ─┘ prov
                     ReturnPartialSuccess: true)
 providerID = zones/<zone>/instances/<name> — zone-qualified, so Get/Delete
 stay correct even mid-replacement after a zone edit
+
+kvm provider (internal/provider/kvm/) — libvirt RPC over an SSH-forwarded unix socket, per host
+prov.Create ──► DomainLookupByName: exists → start if needed, done      ─┐ every step tolerates
+                else: ensure base image in pool (download on first use) │ the previous attempt
+                      overlay volume (exists = reuse)                    │ having got that far;
+                      seed ISO (always rebuilt — a cut-off upload must   │ the seed is the one
+                        never be trusted)                                │ thing redone
+                      DomainDefineXML + DomainCreate                    ─┘
+prov.Get    ──► DomainGetState + DomainGetMetadata + DomainInterfaceAddresses
+                  (ARP, then guest agent; first routable IPv4)
+                  running without IP = Provisioning; nostate = Provisioning
+prov.Delete ──► destroy (not running = fine) → undefine → delete overlay + seed by name
+prov.ListByTag ─► ConnectListAllDomains on every host, keep domains whose
+                    <metadata> carries managed=true; one host down fails the sweep,
+                    one unreadable domain is skipped
+providerID = hosts/<host>/domains/<name>; GC identity lives in the domain's
+<metadata> element (libvirt has no labels). All RPCs run under a context-
+bounded call: go-libvirt never times out on its own, so a cancelled reconcile
+closes the SSH connection to unblock it.
 ```
 
 The reconciler never watches provider-side resources (Pods or GCP VMs).
@@ -415,6 +435,41 @@ they read as choices, not accidents. Chronological by build step.
 - **Unknown GCP instance statuses map to `Stopped`:** the reconciler's
   answer to Stopped is delete-and-recreate, the always-safe move for
   cattle when the API grows a new state.
+- **KVM hosts are reached over SSH with the libvirt socket forwarded,**
+  not libvirtd's TCP/TLS listener: nothing to configure or expose on the
+  hypervisors beyond sshd, pure Go on the operator side (no cgo, no
+  libvirt libraries in the image), and the hosts list can grow by a
+  config edit because the providerID carries the host name.
+- **KVM base images are fetched by the operator into the host's pool**
+  through libvirt's upload stream, rather than expected to be pre-placed:
+  the operator then needs no filesystem access on the host and a new
+  host is usable with zero manual staging. The cost is a synchronous
+  first-time download inside Create.
+- **The KVM seed ISO is rebuilt on every Create attempt** before the
+  domain exists, instead of "exists → skip": a reconcile cancelled mid
+  upload leaves a truncated seed, and the compensating delete cannot run
+  because the connection was already closed to unblock the RPC. Skipping
+  it would boot a VM with no cloud-init and no retry would ever repair it.
+- **KVM `nostate` maps to `Provisioning`, not `Stopped`:** unlike GCP's
+  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 in GRUB under SeaBIOS
+  on a headless VM (Debian bug #1111240: `terminal_output gfxterm
+  serial` with a serial console and no video device) regardless of CPU
+  model or seed, and boots under OVMF with the serial console working.
+  The alternatives were a virtio console (loses the kernel's serial
+  output) or a video device the VMs have no use for. Every
+  current distro cloud image supports UEFI, so it is the default and
+  `firmware: bios` is the per-image escape; libvirt's firmware
+  autoselection (`<os firmware="efi">`) 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
+  is not. A whole host missing from the view would hide many, so that
+  still aborts.
 - **Kubernetes 403s classify as `ErrPermanent`** even though quota
   exhaustion also surfaces as 403 (indistinguishable from RBAC denial in
   `apierrors`): not hammering an API server that may never allow the
@@ -434,6 +489,15 @@ they read as choices, not accidents. Chronological by build step.
   cluster-wide Secret read RBAC — an unrestricted cache would hold every
   Secret in scope). Unlabelled referenced Secrets are invisible by
   construction, surfacing as `CloudInitError`.
+- **Deletion never gives up on its own, but can be released by hand:** a
+  permanent cleanup failure keeps retrying with the finalizer on, because
+  dropping it automatically would leak a running instance on any provider
+  that answers "permission denied". Where `Get` fails permanently the
+  reconciler still calls `Delete`, since it is idempotent and may finish
+  where `Get` cannot converge. `crawl.example.com/skip-cleanup: "true"`
+  abandons the instance and releases the object; it is checked before the
+  provider lookup, so it also frees a Proxy whose provider is gone from the
+  config.
 - **Events RBAC from the plan is omitted:** nothing wires an
   EventRecorder in the prototype, and granting verbs nothing uses would
   be RBAC lint noise. Add the marker together with the recorder if events

docs/kvm-provider.md

@@ -0,0 +1,318 @@
+# The KVM provider
+
+Runs Managed proxies as VMs on your own libvirt/KVM hypervisors. The
+operator reaches each host over SSH, forwards the libvirt unix socket
+through that connection, and talks libvirt's own RPC over it — so the
+hosts need nothing beyond `sshd` and `libvirtd`, and the operator image
+carries no libvirt libraries. VMs boot a stock distro cloud image that the
+operator downloads into the host's storage pool on first use, with the
+Proxy's cloud-init delivered on a NoCloud seed ISO built in-process.
+
+## Hypervisor prerequisites
+
+Per host, once:
+
+1. **libvirt + KVM** (`libvirtd` running, `/dev/kvm` present).
+2. **An SSH user in the `libvirt` group**, so it may open
+   `/var/run/libvirt/libvirt-sock`. The operator authenticates with a
+   private key; nothing else on the host is touched.
+3. **A bridge to a network with DHCP** (e.g. `br0` bridged onto the LAN).
+   Each VM gets one virtio NIC on it and takes its IP from that network's
+   DHCP; the cluster must be able to reach those IPs for health probes
+   and for crawlers to use the proxy.
+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. **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**: a vendor-data script
+   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 sends traffic —
+   but the ARP table only sees traffic to the host itself, so on a plain
+   bridge the agent is in practice required.
+
+Check from the operator's point of view before wiring it in:
+
+```sh
+ssh virt@nas1.home.lan virsh -c qemu:///system list --all   # user can drive libvirt
+ssh virt@nas1.home.lan virsh -c qemu:///system pool-info default
+ssh virt@nas1.home.lan ip link show br0
+```
+
+## Cluster-side setup
+
+1. Create the SSH Secret from a key pair dedicated to the operator and a
+   `known_hosts` file holding every host's key. The worked example below
+   uses `~/.ssh/proxy-operator_ed25519` and `~/.ssh/kvm_known_hosts`,
+   the same paths the integration test and `make kvm-ssh-secret` default
+   to.
+
+   ```sh
+   # once: a key pair for the operator; the .pub goes on every host
+   ssh-keygen -t ed25519 -f ~/.ssh/proxy-operator_ed25519 -N '' -C proxy-operator
+   ssh-copy-id -i ~/.ssh/proxy-operator_ed25519.pub virt@nas1.home.lan
+
+   # once per host: its host key, under the address the *operator* will
+   # dial (hosts[].address), which may differ from what your laptop uses
+   ssh-keyscan -H nas1.home.lan >> ~/.ssh/kvm_known_hosts
+
+   # the Secret: the PRIVATE key (not the .pub) plus known_hosts
+   kubectl -n egress-proxies-operator-system create secret generic kvm-ssh \
+     --from-file=id_ed25519=$HOME/.ssh/proxy-operator_ed25519 \
+     --from-file=known_hosts=$HOME/.ssh/kvm_known_hosts
+   ```
+
+   `make kvm-ssh-secret` runs that last command as an apply, so it also
+   updates an existing Secret; override the paths with
+   `KVM_SSH_KEY=... KVM_KNOWN_HOSTS=...`. Putting the `.pub` in the
+   Secret is the classic mistake: the operator then fails at startup
+   with a "not a private key" error. Without `known_hosts`, set
+   `insecureIgnoreHostKey: true` and accept that a MITM could hand you a
+   fake hypervisor.
+
+   `config/manager/manager.yaml` mounts the Secret (optional) at
+   `/etc/proxy-operator/kvm` as two files, `id_ed25519` and
+   `known_hosts`, which is what `sshKeyFile` and `knownHostsFile` in the
+   provider config point at. The deployment starts fine without the
+   Secret, even with a `kvm` provider configured: only that provider's
+   proxies fail, and they recover once the Secret exists. Rotating the
+   key: re-run the command (or the make target) and restart the manager;
+   both files are read once, on the first connection, and cached.
+
+2. Add a provider entry to the providers config
+   (`config/manager/providers_config.yaml` in-cluster; full sample in
+   `config/samples/providers-config.yaml`):
+
+   ```yaml
+   - name: kvm-home
+     type: kvm
+     kvm:
+       sshKeyFile: /etc/proxy-operator/kvm/id_ed25519
+       knownHostsFile: /etc/proxy-operator/kvm/known_hosts
+       images:
+         debian-13:
+           url: https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2
+       machineTypes:
+         small: {vcpus: 1, memoryMiB: 1024}
+       hosts:
+         - name: nas1
+           address: nas1.home.lan:22
+           user: virt
+           bridge: br0
+   ```
+
+   The operator checks both files at startup and logs a `WARNING:` line
+   when either is unreadable, but it does not refuse to start: the
+   provider set is built all-or-nothing, so exiting here would crash-loop
+   the manager and take the `kubernetes` and `gcp` providers with it.
+   The files are read for real on the first connection. A missing file is
+   a transient failure, so proxies heal by themselves once the Secret
+   lands; a key that does not parse is permanent and needs a fix.
+
+3. Apply a Proxy (`config/samples/proxy_kvm.yaml`) and watch it:
+
+   ```sh
+   kubectl get px -w
+   # NAME               MODE      PROVIDER   PHASE          IP             HEALTHY
+   # proxy-kvm-sample   Managed   kvm-home   Provisioning
+   # proxy-kvm-sample   Managed   kvm-home   Running        192.168.1.73
+   # proxy-kvm-sample   Managed   kvm-home   Ready          192.168.1.73   True
+   ```
+
+   The first Proxy for a given image on a given host sits in
+   `Provisioning` for the length of the image download; the operator log
+   shows `fetching base image into host pool` and `base image cached`
+   with the size and duration.
+
+## Config reference
+
+| Field | Required | Default | Meaning |
+|---|---|---|---|
+| `sshKeyFile` | yes | | private key used for every host |
+| `knownHostsFile` | unless `insecureIgnoreHostKey` | | OpenSSH `known_hosts` to verify host keys against |
+| `insecureIgnoreHostKey` | | `false` | skip host-key verification entirely |
+| `diskSizeGb` | | `10` | capacity of each VM's overlay disk; cloud-init's growpart expands the root fs into it |
+| `vendorData` | | | extra cloud-init vendor-data placed on every seed next to the Proxy's user-data. Beware that cloud-init drops any top-level key of a vendor-data **cloud-config** that the user-data also defines. Shipped as one part of a MIME multipart alongside the agent script when `installGuestAgent` is on |
+| `installGuestAgent` | | `true` | add the vendor-data script that installs and starts `qemu-guest-agent`. Turn off for images that ship it already or cannot reach a package mirror; IP discovery then depends on the host's ARP table |
+| `images.<key>` | | | the key names the image in `placement.image`. Letters, digits, dots, dashes and underscores only — it becomes the cached volume name `base-<key>.<format>` |
+| `images.<key>.url` | yes per entry | | http(s) URL of a cloud image (qcow2 or raw) |
+| `images.<key>.sha256` | | | expected digest; verified after download, a change triggers a re-download |
+| `images.<key>.format` | | `qcow2` | `qcow2` or `raw` |
+| `images.<key>.firmware` | | `uefi` | `uefi` (OVMF, secure boot off) or `bios` (SeaBIOS) for images without an EFI system partition; Debian 13 hangs in GRUB under BIOS on a headless VM, see *Known gaps* |
+| `machineTypes.<key>` | ≥ 1 | | `{vcpus, memoryMiB}` size profile |
+| `hosts[].name` | yes | | the `placement.zone` value that selects this host; unique. Letters, digits, dots, dashes and underscores only — it becomes part of the providerID `hosts/<name>/domains/<vm>` |
+| `hosts[].address` | yes | | SSH endpoint, `host:port` |
+| `hosts[].user` | yes | | SSH user, member of the `libvirt` group |
+| `hosts[].socket` | | `/var/run/libvirt/libvirt-sock` | libvirt socket path on the host |
+| `hosts[].pool` | | `default` | storage pool for base images, overlays and seeds |
+| `hosts[].bridge` | yes | | host bridge for the VM's NIC |
+
+## Placement
+
+| Proxy field | Meaning for kvm |
+|---|---|
+| `placement.zone` | host name from `hosts`. May be omitted when exactly one host is configured; otherwise required. Unknown → `Failed` |
+| `placement.machineType` | key in `machineTypes`. Unknown → `Failed` |
+| `placement.image` | key in `images`, or an `http(s)://` URL used directly. Anything else → `Failed` |
+| `placement.region` | ignored |
+
+`Failed` here means the reconciler stops retrying and the message names
+the offending value; fix the spec (which replaces the VM) or the config.
+
+## What lands on the host
+
+For a Proxy whose deterministic name is `proxy-abc…`, on the chosen
+host's pool:
+
+| Object | Name | Lifetime |
+|---|---|---|
+| base image | `base-<key>.qcow2` (or `base-<url-hash>.qcow2` for a URL) | cached; survives Proxy deletion |
+| image marker | `base-<key>.qcow2.sha256` | with the base image |
+| overlay disk | `proxy-abc….qcow2`, qcow2 backed by the base | with the Proxy |
+| seed ISO | `proxy-abc…-seed.iso` | with the Proxy |
+| domain | `proxy-abc…` | with the Proxy |
+
+The domain is a q35 machine with host-passthrough CPU, the overlay on
+virtio, the seed on a SATA cdrom, one virtio NIC on the bridge, a serial
+console, the guest-agent channel, a virtio RNG and no graphics. Its
+`<metadata>` carries the operator's element (managed flag, owning UID,
+creation time) — that is how orphan GC recognises its own VMs, since
+libvirt has no labels.
+
+The seed's `user-data` is the Proxy's `cloudInit`, **byte for byte**. Any
+format cloud-init accepts works, including a Jinja template, a MIME
+archive, a shell script or a gzipped payload; the operator never parses or
+re-renders it.
+
+The guest agent rides along in `vendor-data`, as a **shell script** rather
+than a cloud-config. That is the only shape cloud-init cannot drop:
+vendor-data cloud-config is merged under the user's own document with
+mergers that keep the user-data's value for every key both define and never
+append lists, so a Proxy carrying its own `packages:` silently lost the
+agent and sat in `Provisioning` forever. The strategy cannot be overridden
+from either document — `merge_how` in user-data governs only the first
+source, and `merge_how` in vendor-data is consumed and discarded before the
+document is stored. A vendor-data script skips all of it: cloud-init gives
+vendor-data its own handler chain, writes scripts to a separate directory
+and runs them from `cc_scripts_vendor`, after the user's packages are
+installed and before the user's `runcmd`. The script is idempotent, picks
+whichever of `apt-get`, `dnf`, `yum`, `zypper` or `apk` the image has, and
+logs into `/var/log/cloud-init-output.log`. Set `installGuestAgent: false`
+to leave it out. When `vendorData` is also configured, the two travel as
+separate parts of a MIME multipart vendor-data.
+
+## The image cache
+
+The base image is downloaded **by the operator, through libvirt**, into
+the host's pool the first time a Proxy asks for it on that host — nothing
+is pre-placed on the hypervisor, and the operator needs no filesystem
+access there. The bytes stream from the URL through the operator into
+the pool with a running sha256; nothing is buffered. The marker volume is
+written last and holds the digest, so a crash mid-download leaves a base
+volume without a marker, which the next attempt deletes and redoes.
+
+A re-download happens when: the marker is missing, or `sha256` is
+configured and differs from the marker's content. Otherwise the cached
+copy is used for as long as it exists, even if the URL's content changed
+upstream (a `latest` URL is pinned at the moment of first fetch, per
+host).
+
+To evict manually:
+
+```sh
+ssh virt@nas1 virsh -c qemu:///system vol-delete --pool default base-debian-13.qcow2.sha256
+ssh virt@nas1 virsh -c qemu:///system vol-delete --pool default base-debian-13.qcow2
+```
+
+⚠️ **Do not change an image entry's `url` or `sha256`, and do not evict
+its base volume, while Proxies using that image exist on the host.** The
+overlays back onto the base file; deleting or replacing it corrupts every
+VM that uses it. Delete those Proxies first, or add the new image under a
+new key and move Proxies over by changing their `placement.image` (which
+replaces the VM).
+
+## Logging into a VM
+
+- Put your key in the Proxy's cloud-init (`ssh_authorized_keys`) and
+  `ssh debian@<ip>` (Debian) / `ssh ubuntu@<ip>` (Ubuntu).
+- Or on the host: `virsh console proxy-abc…` attaches to the serial
+  console; cloud-init's output scrolls by there on first boot.
+- `virsh domifaddr proxy-abc… --source arp` / `--source agent` shows what
+  the operator sees when it looks for the IP.
+
+## Logging
+
+The provider logs through controller-runtime's logger, named `kvm`, with
+`provider`, `host` and `name` (the domain) on every line.
+
+- `--zap-log-level=debug` (logr V(1)): one line per provider operation
+  (`domain defined`, `KVM domain ensured running`, ...) **and one
+  `libvirt call` line per libvirt RPC**, named `kvm.libvirt`, with
+  `rpc` (the libvirt API function, e.g. `virDomainLookupByName`), the
+  identifiers it was given (`domain`, `pool`, `volume`), curated results
+  (`state`, `path`, `interfaces`), `duration`, and on failure `error`
+  plus `libvirtError` (the code name, e.g. `ErrNoDomain`) and
+  `libvirtCode`. Connection events (`libvirt host connected`, dial
+  failures, `libvirt connection dropped` with a reason) log at the same
+  level. A Create is about ten RPCs; a Get is four.
+- `--zap-log-level=2`: the same lines also carry the domain and volume
+  `xml`, the `metadata` element, the interface `addresses` and the
+  domain `names` from a listing.
+- Cloud-init is never logged at any level: the seed ISO upload reports
+  only `length` and `duration`. There is no raw wire tier like the GCP
+  provider's V(5); go-libvirt exposes no hook for it.
+
+## Known gaps
+
+- **No quota or capacity detection.** libvirt reports an out-of-space
+  pool or an over-committed host as an ordinary error; the reconciler
+  treats it as transient and retries with backoff rather than the slower
+  quota backoff.
+- **Orphan volumes without a domain are invisible to GC.** A crash
+  between creating the volumes and defining the domain leaves an overlay
+  and seed that `ListByTag` cannot see (it lists domains). They are
+  cleaned when the same Proxy reconciles again (Delete removes volumes by
+  name even with no domain), but a Proxy CR that is gone for good leaves
+  them for `virsh vol-list` to find.
+- **A host removed from the config takes its VMs with it.** `Delete` for a
+  providerID naming an unconfigured host is a no-op success, so the Proxy
+  finishes deleting instead of pinning its finalizer, but the domain and its
+  volumes stay on the hypervisor and orphan GC cannot see them either (it
+  only sweeps configured hosts). Delete the Proxies before retiring a host,
+  or clean up afterwards with `virsh undefine --nvram` plus `virsh vol-delete`.
+  `Get` still fails permanently for such a providerID on purpose: a typo in
+  `hosts` must not look like a missing VM and trigger a rebuild elsewhere.
+- **The first image fetch is synchronous.** It holds a reconcile worker
+  for the download (LAN speed, a few hundred MB, once per image per
+  host). Other Proxies keep reconciling on the remaining workers.
+- **Replacing an image in use breaks running VMs** — see the warning
+  under *The image cache*.
+- **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`. The same symptom with a booted, addressed
+  VM means the guest agent never came up: check `systemctl status
+  qemu-guest-agent`, `cloud-init status --long` and the vendor script's
+  output in `/var/log/cloud-init-output.log` (a VM without outbound
+  internet cannot install it). A user-data that disables vendor-data
+  (`vendor_data: {enabled: false}`) also removes the agent, by design.
+- **Debian 13 cloud images hang in GRUB under BIOS on a headless VM**
+  ([Debian bug #1111240](https://bugs.debian.org/1111240), open, in
+  `grub2`): trixie's image ships `terminal_output gfxterm serial` in its
+  GRUB config, and with a serial console but no video device (exactly
+  the domain this provider defines) GRUB prints "Booting" and never
+  hands over to the kernel. Not host-specific; reproduced by the Debian
+  cloud team. The same image boots under UEFI with the serial console
+  intact, which is why `firmware` defaults to `uefi`. Debian 12
+  (bookworm) images are unaffected and boot either way. Images that
+  need `firmware: bios` are the exception, not the rule.
+
+## Testing
+
+Unit tests run over an in-memory fake of the libvirt API. The optional
+integration test against a real host is described in
+[testing.md](testing.md#the-kvm-integration-test-real-hypervisor); its
+`-v` output includes the `libvirt call` trace above, at the verbosity
+`KVM_TEST_LOG_LEVEL` selects (default 1).

docs/plans-executions/2026-09-04-1404-kvm-provider.md

@@ -8,9 +8,9 @@ Plan: [2026-09-04-1404-kvm-provider.md](../plans/2026-09-04-1404-kvm-provider.md
 - [x] Step 3 — Transport + errors (`conn.go`, `errors.go`)
 - [x] Step 3b — Image cache (`image.go`)
 - [x] Step 4 — Provider methods (`kvm.go`) over a fake API
-- [ ] Step 5 — Wiring, manifests, samples, docs, CHANGELOG
+- [x] Step 5 — Wiring, manifests, samples, docs, CHANGELOG
 - [x] Step 6 — Integration test against a real hypervisor (written and compiled; first real run is part of Step 7)
-- [ ] Step 7 — Manual validation + MR
+- [x] Step 7 — Manual validation + MR
 
 ## Step 0 — Save plan + branch
 
@@ -69,6 +69,9 @@ Two design choices differ from the plan text:
   `<disk type="volume"><source pool=... volume=.../>`, so only the *base*
   image path is ever looked up (the overlay's `<backingStore>` needs a real
   path). Fewer RPCs and the builder's inputs are all names.
+  **Reverted in Step 7**: volume-referenced disks cannot start on an
+  AppArmor host, see there. The domain XML is back to `type="file"` with
+  the two `StorageVolGetPath` calls the plan had.
 - **Capacities are always `unit="bytes"`.** libvirt's `G` means GiB but
   `GB` means 10^9; spelling the byte count out removes the ambiguity from
   both the overlay and the seed/base volumes.
@@ -78,7 +81,9 @@ The metadata element is written with a default namespace
 marshal. libvirt may hand it back prefixed; `parseInstanceMetadata` is
 tested against both spellings since `encoding/xml` matches on the namespace
 URI. Whether libvirt accepts the default-namespace form on `DomainDefineXML`
-is confirmed in Step 6 against a real host, not here.
+is confirmed in Step 6 against a real host, not here. (Step 7: it does,
+but `DomainGetMetadata` returns the element with no namespace at all; the
+parser now matches on local name only.)
 
 Checked before relying on `github.com/kdomanski/iso9660` for the seed: its
 filename mangling lowercases and keeps `-`, so `user-data`/`meta-data`/
@@ -338,3 +343,331 @@ Step 3b "confirm on a real host" notes get answered. The test's config
 is built directly rather than through the provider package's validation
 (unexported), so a typo in an env var surfaces from `New` or from
 libvirt, not from a validation message.
+
+## Step 5 — Wiring, manifests, samples, docs, CHANGELOG
+
+Done after Step 6 (see that entry for why the order flipped). Everything
+the plan listed, plus a few things it did not:
+
+- `cmd/main.go`: `"kvm": kvm.New` in the builtins map. No RBAC change —
+  the provider talks to hypervisors, not the API server.
+- `config/manager/manager.yaml`: `kvm-ssh` Secret mounted read-only at
+  `/etc/proxy-operator/kvm` with `optional: true`, so deployments without
+  a kvm provider are unaffected. It nests under the providers-config
+  mount at `/etc/proxy-operator`; kustomize renders it (`kubectl
+  kustomize config/default` checked). The container runs as uid 65532
+  and Secret files default to 0644, so the key is readable without an
+  `fsGroup`; the `kubectl create secret` recipe is in a comment next to
+  the mount, in `docs/kvm-provider.md` and in the README.
+- `config/samples/providers-config.yaml`: `kvm-home` block with Debian 13
+  and Ubuntu 24.04 official image URLs, two machine types, one host, and
+  every optional field shown commented out. Parsed through
+  `provider.LoadConfigFile` in a throwaway test to make sure the sample
+  validates. `config/samples/proxy_kvm.yaml` uses the GCP sample's squid
+  cloud-init and shows `ssh_authorized_keys` commented out rather than
+  set — a sample with a placeholder key would either fail cloud-init or
+  invite pasting a real key into a committed file. Added to the samples
+  kustomization; `config/manager/providers_config.yaml`'s comment now
+  mentions kvm.
+- `docs/kvm-provider.md`: prerequisites with a three-line preflight
+  (`virsh list`, `pool-info`, `ip link` over ssh), cluster setup, the full
+  config reference table, placement table, a table of what lands on the
+  host and its lifetime, how the image cache works and how to evict, the
+  **warning not to change or evict an image while Proxies use it** (the
+  reviewer's finding from Step 4, carried as a documented rule since the
+  content-addressed-naming alternative was not chosen), how to log in,
+  and the known gaps.
+- README: providers bullet and a "KVM setup" section mirroring the GCP
+  one. `docs/architecture.md`: components row, a §5 block for the kvm
+  call flow in the same style as the other two, and five decision-log
+  entries (SSH transport, operator-fetched images, seed always rebuilt,
+  `nostate` → Provisioning, skip-vs-fail in the GC sweep).
+- `CHANGELOG.md` entry at the top, stating explicitly that the provider
+  is not yet validated on a real host.
+
+Verification: `go build ./...`, `go vet ./...`, `go test -short` on
+`cmd` and `internal/provider/...`, and both kustomizations rendered.
+`hack/providers-dev.yaml` untouched, as planned.
+
+Worth noting: the CRD has no rule requiring all three placement fields
+(that is GCP's own check), so the single-host "zone may be omitted"
+behaviour works without an API change. And with this step the branch is
+feature-complete but **unproven**: Step 7 runs the integration test and
+the end-to-end checklist against a real hypervisor before the MR.
+
+## Step 7 — Manual validation + MR (in progress)
+
+Host: `beelink` (Ubuntu, libvirt 11.6.0, QEMU 10.1.0, pool `default` at
+`/srv/vms/images` on local ext4, bridge `br0`), reached over Tailscale as
+`beelink.tail8c69c.ts.net`, SSH user `novakj`. Not the `nas1.home.lan`
+the plan and samples name; that host does not resolve from the dev box.
+
+```bash
+export KVM_TEST_HOST=beelink.tail8c69c.ts.net:22
+export KVM_TEST_USER=novakj
+export KVM_TEST_SSH_KEY=$HOME/.ssh/proxy-operator_ed25519
+export KVM_TEST_KNOWN_HOSTS=$HOME/.ssh/kvm_known_hosts
+export KVM_TEST_BRIDGE=br0
+export KVM_TEST_IMAGE_URL=https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2
+make test-integration-kvm
+```
+
+### Run 1 and 2: `Could not open '/srv/vms/images/base-test-image.qcow2': Permission denied`
+
+Everything up to `DomainCreate` worked on the first try against a real
+host, which answers the open notes from Steps 2 and 3b: the SSH-forwarded
+libvirt handshake, the raw qcow2 upload plus pool refresh as a backing
+image, the default-namespace metadata element, and `DomainDefineXML` for
+the generated XML were all accepted by libvirt 11.6. QEMU then exited at
+start with `Permission denied` on the base image.
+
+Ownership was ruled out the slow way: the file was `root:root 0600` (the
+dir-pool default; libvirt's DAC driver chowns the chain at start and
+restores it after a failed start, so that observation meant nothing),
+`qemu.conf` was all defaults (`libvirt-qemu`, `dynamic_ownership=1`), and
+after a manual `chown libvirt-qemu:kvm` the second run failed identically.
+
+The actual cause is the Step 2 choice of `<disk type="volume">`.
+libvirt's AppArmor helper builds each domain's profile from the domain XML
+without consulting the storage driver, and its `add_file_path` only emits a
+rule when the source has a `path`; a volume-referenced disk has pool and
+volume names instead, so neither the overlay nor its backing image gets a
+rule and QEMU is denied on the first one it opens. Every stock Debian and
+Ubuntu hypervisor confines QEMU this way. The unit tests could not have
+caught it: the fake accepts either form.
+
+Fix, in this step: `buildDomainXML` takes `DiskPath`/`SeedPath` and emits
+`type="file"` disks; `ensureOverlay` and `writeSeed` return the volume
+handles (the exists-already branch now does a `StorageVolLookupByName`),
+and Create resolves both paths with `StorageVolGetPath` before defining
+the domain. Two extra cheap RPCs per Create. `TestBuildDomainXML_fieldByField`
+now fails on any `type="volume"` or `pool=` attribute so the design cannot
+drift back, and `TestCreate_freshHost` asserts the disk sources equal the
+paths the fake reports.
+
+Worth noting: the first two runs left the base image and its marker in
+the pool as designed, so later runs skip the 435 MB download.
+
+### Run 3: `parsing domain metadata: expected element <instance> in name space … but have no name space`
+
+With file-backed disks, Create returned in 1 s (image cached) and the
+domain started. Get then failed to parse the metadata: libvirt 11.6 finds
+the element by our URI but returns it as a bare `<instance>` with no
+namespace declaration at all. Step 2 had tested the default-namespace and
+prefixed spellings and left "does libvirt accept the default-namespace
+form" for the real host; it does, it just does not give it back that way.
+
+Fix: `parseInstanceMetadata` matches on the local name only, since the
+lookup already selected the element by URI. The fake now strips the
+declaration when storing metadata, so the provider's Get and ListByTag
+tests parse the shape libvirt really returns, and a new
+`TestParseInstanceMetadata_libvirtBareForm` pins it.
+
+### Run 4: guest hangs at GRUB's "Booting" line
+
+Create and start passed; Get polled `Provisioning` with no IP while the
+VM sat at 100 % CPU after GRUB printed "Booting `Debian GNU/Linux`" and
+before the kernel wrote anything to the serial console. The domain XML
+matches a working VM on the same host (q35, BIOS, host-passthrough,
+virtio disk, SATA cdrom, br0), so the difference is the disk content:
+the cached image was downloaded without a checksum and never verified.
+Diagnosis was handed to the user (sha512 against Debian's published
+sums, `qemu-img check`, and a `virt-install --import` of the same image
+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.<key>.firmware` (`uefi` default, `bios` escape),
+threaded through `withDefaults` → `imageSource` → `domainParams`, and
+`buildDomainXML` emits `<os firmware="efi"><firmware><feature
+enabled="no" name="secure-boot"/></firmware>` 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.
+
+Follow-up, same day: the cause is known upstream. [Debian bug
+#1111240](https://bugs.debian.org/1111240) (filed 2025-08-16 against
+the cloud image, reassigned to `grub2`, still open as of 2026-09-10;
+trixie ships grub2 2.12-9+deb13u2) reports exactly this with a domain
+XML almost identical to ours: BIOS, virtio disk, SATA cdrom seed, bridge
+NIC, `<console><target type='serial'/>`, no `<graphics>`/`<video>`.
+The cloud team reproduced it: trixie's grub.cfg has `terminal_output
+gfxterm serial`, and with a serial port present but no video adapter
+GRUB never hands over to the kernel. Their fix in the image
+(`terminal_output serial`) is not something an operator can apply;
+the reporter's workaround (console `target type='virtio'`) boots but
+takes the kernel's `console=ttyS0` output with it. UEFI sidesteps the
+gfxterm path and keeps the serial console, so the decision stands, but
+the docs now name the bug rather than blaming the host, and note that
+bookworm images are unaffected. `latest` on cloud.debian.org was the
+2026-08-31 build when this was hit, so every trixie point release to
+date carries it.
+
+### Run 6: pass
+
+With UEFI the full lifecycle passed in 23 s on a warm cache: Create 2 s
+(domain start 0.9 s), guest agent answering with `192.168.0.89` after
+20 s (ARP never did; the host had not talked to the guest), TCP 22
+reachable from the dev machine, ListByTag seeing it, Delete leaving only
+the base image and marker in the pool, second Delete a no-op. Trace
+excerpt worth keeping for the record:
+
+```
+09:44:28.603 libvirt virDomainDefineXML xmlBytes=1509 domain=proxy-hk5oavoqk45n3twq duration=13.51ms
+09:44:28.603 kvm domain defined uid=9bbceb41-… firmware=uefi
+09:44:29.542 libvirt virDomainCreate domain=proxy-hk5oavoqk45n3twq duration=938.705ms
+09:44:49.573 libvirt virDomainInterfaceAddresses domain=proxy-hk5oavoqk45n3twq source=DomainInterfaceAddressesSrcAgent interfaces=2 duration=1.742ms
+09:44:49.573 kvm KVM domain fetched state=Running ip=192.168.0.89 uid=9bbceb41-…
+```
+
+Worth noting: the sweep listed a second managed domain,
+`proxy-qxge3dbm2mnpc3ih`, shut off, left behind by an earlier failed
+run whose cleanup never ran. That is exactly the orphan GC exists for;
+on the host it needs `virsh undefine --nvram` plus its two volumes by
+hand. The `ListByTag` sweep costs one `virDomainGetMetadata` per
+domain on the host (21 here, ~0.4 ms each over the tunnel), which is
+fine at this scale but is the call to watch on a busy hypervisor.
+
+### Run 7: in-cluster, two defects the integration test could not see
+
+The e2e pass ran the operator inside the kind cluster
+(`egress-proxies-operator:kvm-dev`, built with `make docker-build`,
+loaded with `kind load docker-image`, deployed via the kustomize
+`config/manager` with the `kvm-home` provider pointing at beelink) and
+two Proxies with real cloud-configs: one plain squid, one with a debug
+login user. Both defects come from the difference between the test
+harness and the real thing, not from libvirt.
+
+**Read-only root filesystem.** The manager runs with
+`readOnlyRootFilesystem: true`, and `iso9660.NewWriter` stages the seed
+in a temp directory, so the first Create failed with `creating iso
+writer: read-only file system`. Fix: a 64 MiB `emptyDir` on `/tmp` in
+`config/manager/manager.yaml`. Side finding: the error is classified
+`ErrPermanent` by `writeSeed`, yet `hostPool.call` treated any
+non-libvirt error as a transport failure and dropped the SSH connection,
+so every retry redialled. `call` now leaves the connection alone for
+errors already carrying a provider sentinel (`isClassified`), with a
+test.
+
+**The guest agent never got installed.** Both VMs booted (confirmed on
+the console by hand) but sat in `Provisioning` for 25 minutes: the
+trace showed `virDomainInterfaceAddresses` from the agent source failing
+with `ErrAgentUnresponsive` on every poll and the ARP source empty. The
+integration test had passed with `CloudInit: "#cloud-config\n"`; the
+real Proxies have `packages: [squid]` and `runcmd:`. cloud-init merges
+vendor-data under user-data with its default mergers, which keep the
+user-data's value for any key both define and never append lists, so the
+vendor-data's `packages`/`runcmd` were dropped wholesale. Confirmed
+locally with the dev box's cloud-init 26.1 before changing anything:
+
+```sh
+python3 - <<'EOF'
+from cloudinit import mergers
+user = {"packages": ["squid"], "runcmd": ["systemctl restart squid"]}
+vendor = {"packages": ["qemu-guest-agent"], "runcmd": [["systemctl", "enable", "--now", "qemu-guest-agent"]]}
+print(mergers.construct(mergers.default_mergers()).merge(dict(user), dict(vendor)))
+EOF
+# {'packages': ['squid'], 'runcmd': ['systemctl restart squid']}
+```
+
+Fix: `withGuestAgent` in `seed.go` merges the bootstrap into the Proxy's
+cloud-config itself (append to `packages` and `runcmd`, skip if present,
+re-render with `sigs.k8s.io/yaml`), so vendor-data is no longer defaulted
+and `vendorData` is plain extra data. User-data that is not a cloud-config
+document (script, `#include`, MIME) still gets the bootstrap as
+vendor-data since there is nothing to collide with; a cloud-config that
+does not parse as a mapping fails the Proxy as `ErrPermanent`. The
+rendered document for the squid Proxy passed `cloud-init schema
+--config-file`. The logging test's fixture (`#cloud-config` + a bare
+sentinel line) is not a mapping and had to become `hostname: <sentinel>`.
+
+Rebuilt, reloaded, restarted, deleted and recreated both Proxies: both
+`Ready`/`HEALTHY True` in 2 min 1 s from `kubectl apply`, IP via the
+agent 110 s after `virDomainCreate` (the guest installs squid and the
+agent from the mirrors first), and `curl -x http://<ip>:3128
+https://www.gstatic.com/generate_204` from the dev box returns 204
+through both.
+
+Worth noting: the `#cloud-config` round-trip drops comments and reorders
+keys alphabetically; semantics are preserved (numbers, booleans and
+block scalars survive, checked in tests) but a user diffing the seed
+against their CR will see a different document. Vendor-data supplied via
+`vendorData` is subject to the same cloud-init merge rule and is now
+documented as such. The kind pod also restarted twice during the session
+from "Pod sandbox changed" (the node briefly lost the API server), which
+is unrelated to the operator. Still uncommitted on purpose: the
+`kustomization.yaml` image override (`egress-proxies-operator:kvm-dev`)
+and the `beelink` host entries in `config/manager/providers_config.yaml`
+and `config/samples/providers-config.yaml`, which are this machine's dev
+setup rather than the repo's defaults.
+
+### Run 8: review fixes
+
+A `/code-review high` pass over the branch found five defects, fixed in
+[plan 2026-09-10-2222](../plans/2026-09-10-2222-kvm-review-fixes.md) with
+its own execution log. Two of them land on Run 7's conclusions above and
+supersede them:
+
+- The guest-agent **merge described in Run 7 is gone**. It could not be made
+  safe for user-data that is not a plain cloud-config, and its YAML
+  round-trip re-typed unquoted scalars. The bootstrap now ships as a
+  vendor-data shell script, which cloud-init never merges, and the Proxy's
+  cloud-init reaches the seed byte for byte.
+- The read-only-filesystem fix from Run 7 still stands, but the eager SSH
+  key load it sat next to does not: a missing `kvm-ssh` Secret aborted the
+  whole provider set and crash-looped the manager. The material is read on
+  first use now.
+
+Re-validated the same way: integration test against beelink, both Proxies
+recreated in kind, 204 through squid.
+
+### MR opened
+
+[PR #5](https://gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/pulls/5),
+`feat/kvm-provider` → `main`, 19 commits. `tea` is not installed on the
+dev box and the HTTPS remote has no credential helper, so the push used a
+one-off helper reading a Gitea access token from `~/.config/gitea-token`,
+and the MR was created with the API instead of `tea pr create`:
+
+```sh
+git -c credential.helper='!f() { echo username=kacerr; echo "password=$(tr -d "[:space:]" < ~/.config/gitea-token)"; }; f' \
+  push -u origin feat/kvm-provider
+curl -s -H "Authorization: token $(tr -d '[:space:]' < ~/.config/gitea-token)" \
+  -H 'Content-Type: application/json' -d @pr.json \
+  https://gitea.home.hrajfrisbee.cz/api/v1/repos/kacerr/egress-proxies-operator/pulls
+# pr.json: {"title": ..., "head": "feat/kvm-provider", "base": "main", "body": ...}
+```
+
+The push printed a libsecret "store failed" warning from the system
+credential store; harmless, the token was not persisted anywhere but the
+file.

docs/plans-executions/2026-09-10-0805-kvm-libvirt-tracing.md

@@ -0,0 +1,109 @@
+# Execution: Trace every libvirt call in the KVM provider
+
+Plan: [2026-09-10-0805-kvm-libvirt-tracing.md](../plans/2026-09-10-0805-kvm-libvirt-tracing.md)
+
+- [x] Step 0 — Save plan
+- [x] Step 1 — `errors.go`: shared error fields
+- [x] Step 2 — `trace.go`: the `tracingAPI` decorator
+- [x] Step 3 — `conn.go`: pool logging and wrapping
+- [x] Step 4 — Call sites
+- [x] Step 5 — Integration test output
+- [x] Step 6 — Unit tests
+- [x] Step 7 — Docs, execution log, CHANGELOG
+
+## Steps 1–6 — Tracer, pool logging, tests, integration wiring
+
+Done as one commit on `feat/kvm-provider`; the plan's assumption that
+this belongs on the KVM branch rather than its own feature branch was
+accepted at plan approval.
+
+The code follows the plan closely. Things that came out differently
+or are worth knowing:
+
+- **Logger names join with `/` in funcr, `.` in zap.** The trace lines
+  are `kvm/libvirt` in the unit-test capture and in `testr` output, and
+  `kvm.libvirt` under the operator's zap logger. The docs use the zap
+  spelling; the unit test asserts the funcr one. First run of the tier
+  test failed on exactly this.
+- **Cancel is not reported as a transport error.** The original `call`
+  dropped the connection twice on cancellation (once from the abort
+  closure, once from the "not a libvirt error" branch, harmlessly, since
+  drop is idempotent). With a log line on each drop that would have
+  printed both `context cancelled` and `transport error` for one event,
+  so the transport branch now skips `ctx.Err()`. Covered by
+  `TestHostPool_call_logsDropReason`.
+- **`ConnectListAllDomains` in the fake includes every defined domain**,
+  so the trace test's fixture (one running managed domain plus one
+  stopped domain for `DomainCreate` to start) lists two; the V(2)
+  assertion checks for the `names` key rather than an exact list.
+- **`libvirtErrorFields` also feeds `logAPIError`**, which therefore
+  gains `libvirtCodeName` (e.g. `ErrNoDomain`) next to the numeric code.
+- The seam-coverage guard (`reflect` count of `libvirtAPI` methods vs
+  the test table) is the only thing standing between a future 18th RPC
+  and a silently untraced call; keep it.
+
+Verification run before committing:
+
+```bash
+go build ./... && go vet ./... && go vet -tags=integration ./internal/provider/kvm/
+go test -short ./...
+go test -race -count=1 -cover ./internal/provider/kvm/   # 90.2 %, up from 88.2
+go test -tags=integration -count=1 -run TestKVM ./internal/provider/kvm/   # self-skips without env
+```
+
+Worth noting: the live run (`make test-integration-kvm` against beelink)
+had not happened when this was written; it is what Step 7 of the KVM
+provider plan is waiting on, and its output is the acceptance test for
+this feature. Expected shape is in the plan's Verification section.
+
+## Revision after the first live run — less noise, libvirt names
+
+The first live run on beelink showed the trace working but hard to read:
+`testr` printed `"ts"`, `"level"`, `"msg"` and the three per-run
+constants on every line, the error text appeared twice (`error` and
+`libvirtMessage`), `host` appeared twice on image-cache lines (the
+logger already carried it), and `method=DomainLookupByName` was
+go-libvirt's name rather than something to look up in libvirt's docs.
+The user asked for "less verbose but more informative". Changes:
+
+- The tracer's headline field is now `rpc=virDomainLookupByName`: the
+  public libvirt API function, which go-libvirt's method names are
+  exactly minus the `vir` prefix (they come from the same
+  `remote_protocol.x`). Not gRPC, for the record: libvirt's own
+  XDR-based RPC over the forwarded unix socket.
+- Errors are one `error` field plus `libvirtError=ErrNoDomain` and
+  `libvirtCode=42`; `libvirtMessage` is gone from both the tracer and
+  `logAPIError`, since it is the error text.
+- `image.go` no longer adds `host` to lines whose logger has it.
+- The integration test replaces `testr` with a small `compactSink`
+  (`logr.LogSink` plus `GetCallStackHelper`, so `t.Log` still
+  attributes lines to `trace.go`): `HH:MM:SS.mmm libvirt
+  virDomainCreate domain=… duration=… error=… libvirtError=…`. The
+  `rpc` value becomes the headline, `provider`/`host`/`name` are
+  dropped (the run's first `t.Log` line states them), and values with
+  spaces are quoted. Production zap output is unchanged apart from the
+  field renames; structured logs are meant to repeat context.
+
+Previewed by driving the sink through the fake provider with an
+injected `DomainCreate` failure (a throwaway integration-tagged test,
+not committed):
+
+```
+trace.go:55: 09:04:33.165 libvirt virDomainLookupByName domain=proxy-abc123def456ghij duration=4µs error="Domain not found: …" libvirtError=ErrNoDomain libvirtCode=42
+trace.go:55: 09:04:33.165 libvirt virStorageVolUpload pool=default volume=proxy-abc123def456ghij-seed.iso offset=0 length=0 duration=71µs
+kvm.go:262:  09:04:33.165 kvm seed volume uploaded volume=proxy-abc123def456ghij-seed.iso bytes=45056
+trace.go:55: 09:04:33.165 libvirt virDomainCreate domain=proxy-abc123def456ghij duration=0s error="internal error: process exited…" libvirtError=ErrResourceBusy libvirtCode=87
+```
+
+Worth noting: `length=0` on `virStorageVolUpload` is what the provider
+passes (libvirt reads to EOF); the following `seed volume uploaded`
+line has the byte count.
+
+## Step 7 — Live run confirmed; CHANGELOG
+
+The compact trace was accepted on the first passing live run
+(2026-09-10, beelink): one line per RPC with the libvirt function name,
+the provider's own lines between them, `Provisioning` polls readable at
+a glance (`SrcArp interfaces=0`, `SrcAgent … ErrAgentUnresponsive`
+until the agent came up at +20 s), and a full `ListByTag` sweep over 21
+domains in 15 ms. CHANGELOG entry added under the KVM validation entry.

docs/plans-executions/2026-09-10-2222-kvm-review-fixes.md

@@ -0,0 +1,229 @@
+# Execution: fix the five code-review findings on `feat/kvm-provider`
+Plan: [docs/plans/2026-09-10-2222-kvm-review-fixes.md](../plans/2026-09-10-2222-kvm-review-fixes.md)
+
+- [x] Step 0 — Save plan into the repo
+- [x] Step 1 — Unwedge deletion (finding 1)
+- [x] Step 2 — Validate names that become identifiers (finding 2)
+- [x] Step 3 — Read the SSH material lazily (finding 3)
+- [x] Step 4 — Guest agent via vendor-data script (findings 4 and 5)
+- [x] Step 5 — Validate, record, push
+
+## Step 0 — plan committed
+
+Copied out of `~/.claude/plans/` and committed alone before touching code, as
+the repo's plan rules require.
+
+## Step 1 — Unwedge deletion
+
+Three changes, in the order the bug unfolds.
+
+`reconcileDelete` calls `Get` first and only drops the finalizer when it
+answers not-found, so any permanent error there retried forever. It now
+switches on the class: a permanent `Get` still calls `Delete`, which is
+idempotent, and releases the object when that succeeds. Polling a `Get`
+that cannot converge was the wedge; `Delete` is the only call that can
+still make progress.
+
+`crawl.example.com/skip-cleanup: "true"` abandons the instance and removes
+the finalizer. It is checked **before** the provider lookup, which also
+frees a Proxy whose provider was deleted from the config — a second wedge
+sitting three lines above the reported one, with the same shape. No
+`EventRecorder` exists on the reconciler and none was added, so the
+abandonment is an Info log; `docs/architecture.md` already records that
+decision for the whole controller.
+
+In the KVM provider, `Delete` for a providerID naming an unconfigured host
+now logs and returns success. The precedent is two functions further down:
+`removeVolumes` treats a missing pool the same way, with a comment saying
+outright that refusing "would pin the Proxy's finalizer".
+
+**`Get` deliberately still fails permanently for that case.** Making it
+answer not-found would have been a one-line fix for the wedge, but the live
+reconcile path reads not-found as "the VM is gone" and rebuilds it
+elsewhere. A typo in `hosts` would then quietly duplicate every Proxy on
+that host, with the originals still running where GC cannot see them, since
+`ListByTag` only sweeps configured hosts. The asymmetry is now pinned by a
+test that asserts both halves at once.
+
+## Step 2 — Validate names that become identifiers
+
+One regexp, `^[A-Za-z0-9][A-Za-z0-9_.-]*$`, applied to `hosts[].name` and to
+the `images` map keys. Dots stay legal because a host name like
+`nas1.home.lan` is ordinary and harmless; the character that actually
+breaks things is `/`.
+
+The review only reported the host name. The image key has the same defect
+and is arguably worse: it is not merely embedded in an ID but pasted into a
+file name, `base-<key>.<format>`, so `../etc/x` would be a path traversal
+into the storage pool rather than a parse failure. Both are validated in the
+same pass. Machine-type keys are left alone deliberately — they are only
+ever compared against `placement.machineType`, never embedded anywhere.
+
+Worth noting: this is validation of the operator's own config file, which
+only a cluster admin can write, so the traversal was never a privilege
+boundary. It is still cheaper to reject at load than to explain a stuck
+Proxy later. Both new table cases assert the message text, so the wording
+that names the consequence is now pinned by tests.
+
+## Step 3 — Read the SSH material lazily
+
+`kvm.New` no longer loads the key; `lazyAuth` does, on the first connection,
+caching the result once it works. `registry.Build` is all-or-nothing by
+design, so the old constructor error did not merely fail the KVM provider,
+it exited the manager before any controller started.
+
+The plan said "lazy dialer", and the first attempt was exactly that — one
+closure wrapping both the loading and the dial. It could not be tested: the
+fixture's SSH server is real, but the socket behind it is an echo server, so
+every successful dial dies in the libvirt handshake and the assertion could
+not tell "the key worked" from "the key did not". Splitting the seam in two,
+`lazyAuth` for the material and `lazyDialer` for the wiring, made both
+testable: the healing path against `lazyAuth`, and a separate test that the
+dialer really authenticates, asserted by the error being past SSH and by the
+server having seen the forward request.
+
+Error classes carry the intent. A missing file stays unclassified, so
+`provider.Class` defaults it to transient and a Proxy heals on its own when
+the Secret appears — the common case, since the Secret is usually created
+after the manager. A key that does not parse wraps `ErrPermanent`, because
+retrying cannot fix it. `newSSHAuth` distinguishes the two with
+`errors.Is(err, fs.ErrNotExist)`.
+
+Worth noting: the startup check is kept, just non-fatally, logged with the
+`WARNING:` prefix the repo already uses where logr has no warn level. That
+preserves the diagnostic the eager version was written for. The caching test
+deliberately deletes the key file after a successful load and asserts the
+cached value is reused, which pins the rotation behaviour the docs promise:
+a rotated Secret needs a manager restart. `TestNew_badKeyPathFailsFast` was
+inverted rather than deleted, so the old contract stays visible in the diff
+next to the new one.
+
+## Step 4 — Guest agent via vendor-data script
+
+The review filed two findings against `withGuestAgent` — detection too
+narrow, and YAML re-typing on the round-trip. Reading cloud-init 26.1's
+sources while planning turned both into one finding: the operator should
+not be editing the user's document at all.
+
+Vendor-data gets its own handler chain (`stages.py:_default_vendordata_handlers`),
+with separate script and cloud-config paths. A vendor-data **script** is
+therefore written to the vendor script directory and run by
+`cc_scripts_vendor`, never touching the config merger that drops colliding
+keys. On the final-stage module list it lands after
+`package_update_upgrade_install` and before the user's `runcmd`, so the
+agent now comes up *earlier* than the old `runcmd` append did.
+
+That also closed the door the previous approach had left ajar. The merge
+cannot be rescued from inside either document: `merge_how` in user-data
+governs only the first source, and `merge_how` in vendor-data is popped by
+`dict_extract_mergers` and discarded before the document is written. There
+was no fix that kept the merge.
+
+`withGuestAgent`, `isCloudConfig` and `appendUnique` are gone, along with
+seed.go's `sigs.k8s.io/yaml` dependency. `installGuestAgent` (default true)
+turns the script off. When `vendorData` is also configured the two ship as
+a MIME multipart, with the user's document declared `text/plain` — a type
+cloud-init re-sniffs from the content's own header line, so this code never
+classifies what the user wrote.
+
+Before deleting the old path, its damage was captured rather than taken on
+trust. Round-tripping a plausible cloud-config through it:
+
+```sh
+# temporary test in the package, deleted afterwards
+go test -run TestZZProbeRetyping -v ./internal/provider/kvm/
+# in:  version: 1.10
+# out: version: 1.1
+```
+
+`1.10` arriving as `1.1` is worse than the reviewer's `passwd` example,
+since nothing about it looks wrong in the seed. Comments were dropped and
+keys reordered on the same pass.
+
+The generated vendor-data was then checked against the cloud-init actually
+installed on this machine, rather than against the docs:
+
+```sh
+python3 -c 'from cloudinit import user_data, handlers; ...'
+# vendor-script:    multipart=False  sniffed=text/x-shellscript
+# vendor-multipart: multipart=True   part 1 text/plain -> text/cloud-config
+#                                    part 2 text/x-shellscript
+```
+
+and the script itself through `sh -n`.
+
+Worth noting: the script has to choose a package manager where cloud-init's
+`packages:` key was distro-agnostic. It covers apt, dnf, yum, zypper and
+apk, and both systemd and OpenRC for the enable step, but an image outside
+that set now needs `installGuestAgent: false` and the agent baked in. The
+seed's user-data is byte-identical to the Proxy's `cloudInit` from here on,
+which is what the new `TestBuildSeedISO_userDataIsNeverRewritten` pins
+across seven formats. One consequence worth remembering: a user-data that
+sets `vendor_data: {enabled: false}` disables the agent, and cloud-init
+documents that as intended behaviour.
+
+## Step 5 — Validation
+
+Unit suite, `go vet` and `gofmt` clean across the repo. Then the two real
+runs.
+
+**Integration against beelink**, unchanged environment from the KVM plan's
+log, passed in 22 s on a warm cache, with
+`virDomainInterfaceAddresses source=DomainInterfaceAddressesSrcAgent
+interfaces=2` — the agent installed by the new vendor script answering, on
+a stock Debian 13 image.
+
+**In-cluster**, both Proxies deleted and recreated against the rebuilt
+image: Ready with an address 2 min 3 s after `kubectl apply`, 204 through
+squid from the dev box for both. Inside the debug VM:
+
+```sh
+ssh novakj@192.168.0.94 'systemctl is-active qemu-guest-agent; cloud-init status; \
+  sudo head -4 /var/lib/cloud/instance/user-data.txt; sudo ls /var/lib/cloud/instance/scripts/vendor/'
+# active
+# status: done
+# #cloud-config
+# # Login for debugging: user "novakj", password "kvm-dev", passwordless sudo.
+# ...
+# part-001
+```
+
+Two things worth having seen with your own eyes there: the user's **comment
+lines survived** into the guest, which the old merge stripped, and the
+script really did land in the vendor directory rather than being merged
+into the user's config.
+
+**Negative check for the Secret**, done by pointing the deployment's volume
+at a name that does not exist rather than deleting the real Secret:
+
+```sh
+kubectl -n egress-proxies-operator-system patch deploy egress-proxies-operator-controller-manager \
+  --type=json -p '[{"op":"replace","path":"/spec/template/spec/volumes/1/secret/secretName","value":"kvm-ssh-absent"}]'
+```
+
+The manager came up and stayed up, 0 restarts, logging
+`WARNING: kvm ssh material is not usable ... open /etc/proxy-operator/kvm/id_ed25519:
+no such file or directory`. The Kubernetes Proxies kept reconciling; the
+KVM ones failed per reconcile and retried. Before this branch the same
+config exited the process before any controller started. Patched back
+afterwards, everything Ready.
+
+Worth noting: the KVM Proxies kept reporting `Ready` during that window
+rather than degrading visibly. That is existing behaviour, not a
+regression — a transient provider error rides the workqueue and does not
+clear a healthy status — but it does mean the only signal of a missing
+Secret is the log and the reconcile errors. Also, the ListByTag sweep on
+beelink still reports `managed=2` where one domain is an orphan from an
+older failed run; unrelated to this work, and the same leftover noted in
+the KVM plan's Run 6.
+
+Worth noting: the review filed this as a KVM finding, but the fix is mostly
+in the controller, because KVM is the only provider that resolves a
+providerID segment against operator config at all. GCP and Kubernetes hand
+the lookup to the backend, which answers 404 for a retired zone or a deleted
+namespace, so they reach not-found and release on their own. Anything else
+that classifies permanently on the delete path — revoked RBAC on the
+Kubernetes provider is the realistic one — was equally stuck before this
+step and is what the annotation is really for. The KVM table case
+`"host not configured"` moved out of `TestDelete_errors` into its own named
+test, since it is no longer an error.

docs/plans/2026-09-10-0805-kvm-libvirt-tracing.md

@@ -0,0 +1,186 @@
+# Plan: Trace every libvirt call in the KVM provider
+**Created:** 2026-09-10 08:05
+
+## Context
+
+Step 7 of the KVM provider plan (live validation on `beelink`) has gone through
+three fixes found only on the real host, and the current failure (guest hangs at
+GRUB's "Booting" line) is being diagnosed blind: the provider logs at V(1)/V(2)
+but nothing shows them. The integration test never installs a logger, so
+controller-runtime prints "log.SetLogger(...) was never called" and swallows
+everything; and even with a logger, the provider only logs per *operation*
+(Create, Get…), not per libvirt RPC, so the sequence and timing of what was
+actually sent to the hypervisor is invisible.
+
+Goal: one log line per libvirt call at debug level (`--zap-log-level=debug`, logr
+V(1) in this repo's scheme: V(1) = outcome per operation, V(2) = curated detail,
+V(5) = raw wire, per the GCP verbose-logging plans), with method, host, domain or
+volume, duration and the libvirt error code on failure; and those lines visible in
+`make test-integration-kvm` output.
+
+Assumption: this lands on the current `feat/kvm-provider` branch, since it exists
+to finish that branch's validation, and the plan file plus execution log follow
+CLAUDE.md conventions (plan committed alone first, execution summary alongside
+each step). Say so if you want a separate branch instead.
+
+## Design
+
+- **Decorator, not call-site logging.** New `tracingAPI` implements the 17-method
+  `libvirtAPI` seam (`internal/provider/kvm/conn.go:26-44`), wrapping the real
+  client. Every method logs one V(1) line after the inner call returns. Applied in
+  `hostPool.call`, so every provider method, `image.go`'s cache and the
+  integration test's own `p.hosts.call` are traced with no per-site changes.
+- **Logger passed explicitly.** `call(ctx, log, host, fn)` and `get(ctx, log, host)`
+  gain a `logr.Logger` parameter, matching how this package already threads `log`
+  into every helper. The five call sites (`kvm.go:169, 322, 458, 530`,
+  `kvm_integration_test.go:220`) already hold the right logger with
+  provider/host/name values. `call` does `log = log.WithName("libvirt")`, so trace
+  lines are named `kvm.libvirt`.
+- **Wrap only when `log.V(1).Enabled()`.** Zero cost at info level; the
+  controller-runtime unfulfilled root sink reports disabled, so unit tests and
+  `conn_test.go`'s `hostClient{api: nil}` cases are unaffected.
+- **One line per RPC at every level.** At V(2) the same line gains extra keys
+  (`xml`, `metadata`, `addresses`, `names`) instead of a second line. Upload and
+  download log offset/length/duration only, never bytes (the seed ISO carries
+  cloud-init). Domain and volume XML contain no secrets, so V(2) is fine for them.
+- **Connection events traced too** in the pool: dial with duration, dial failure,
+  dead cached client redial, and both drop reasons (context cancelled, transport
+  error). `dialSSH`/`connectLibvirt`/`keepalive` stay untouched: changing
+  `hostDialer`'s signature would ripple into every test dialer, and the pool
+  already observes every event.
+- **Rendering.** Handles by name only (`domain`, `pool`, `volume`), never UUID/Key.
+  `libvirt.DomainState` and `DomainInterfaceAddressesSource` have stringers, pass
+  them typed. Bitmask flags don't stringify, log as integers only when non-zero.
+  `libvirt.OptString` → first element or `""` under `uri`.
+- **awaitOrCancel.** `done` runs inside fn's goroutine; on the normal path it logs
+  before `call` returns. On cancel, the abandoned RPC logs one late line when the
+  closed connection makes it fail: desirable in production, and in the integration
+  test only reachable in an already-failing timeout run (testr panics on a log after
+  the test ends; note that in the header, don't engineer around it).
+
+## Steps
+
+### Step 0 — Save plan
+Copy this file to `docs/plans/2026-09-10-0805-kvm-libvirt-tracing.md`, commit alone.
+
+### Step 1 — `errors.go`: shared error fields
+Factor `libvirtErrorFields(err error) []any` out of `logAPIError` (`errors.go:62-72`):
+returns `libvirtCode` (uint32), `libvirtCodeName` (`libvirt.ErrorNumber(code)`,
+e.g. `ErrNoDomain`), `libvirtMessage`; nil for non-libvirt errors; works through
+`%w` wrapping via `errors.As`. `logAPIError` keeps its message and gains
+`libvirtCodeName`. Test `TestLibvirtErrorFields` (table: libvirt.Error, wrapped,
+plain error) in `errors_test.go`.
+
+### Step 2 — new `trace.go`
+```go
+type tracingAPI struct { inner libvirtAPI; log logr.Logger }
+type tracedCall struct { t *tracingAPI; method string; start time.Time; kv []any }
+func (t *tracingAPI) begin(method string, kv ...any) *tracedCall
+func (c *tracedCall) detail(kv ...any) *tracedCall // appended only if V(2) enabled
+func (c *tracedCall) done(err error, kv ...any)    // one V(1) line
+```
+`done` emits `log.V(1).Info("libvirt call", ...)` with keys in order: `method`,
+begin args, result kv, `duration` (rounded to µs), then on error `error` plus
+`libvirtErrorFields`. Per-method keys:
+
+| method | V(1) args | V(1) results | V(2) extra |
+|---|---|---|---|
+| DomainLookupByName | domain | | |
+| DomainDefineXML | xmlBytes | domain | xml |
+| DomainCreate, DomainDestroy | domain | | |
+| DomainUndefineFlags | domain, flags | | |
+| DomainGetState | domain | state, reason | |
+| DomainGetMetadata | domain, type, uri | metadataBytes | metadata |
+| DomainInterfaceAddresses | domain, source | interfaces (count) | addresses |
+| ConnectListAllDomains | | count | names |
+| StoragePoolLookupByName, StoragePoolRefresh | pool | | |
+| StorageVolLookupByName | pool, volume | | |
+| StorageVolGetPath | pool, volume | path | |
+| StorageVolCreateXML | pool, xmlBytes | volume | xml |
+| StorageVolUpload, StorageVolDownload | pool, volume, offset, length | | (never content) |
+| StorageVolDelete | pool, volume | | |
+
+Return values pass through untouched.
+
+### Step 3 — `conn.go`: pool logging and wrapping
+- `get(ctx, log, host)`: dead cached client → `"libvirt connection lost, redialing"`;
+  time the dial; `"libvirt host connected"` with `address`, `duration`; failure →
+  `"libvirt host dial failed"` with `address`, `duration`, `error`.
+- `call(ctx, log, host, fn)`: `log = log.WithName("libvirt")`; wrap when
+  `log.V(1).Enabled()`; abort closure logs `"libvirt connection dropped"`
+  `reason="context cancelled"` before `drop`; transport-error branch logs the same
+  message with `reason="transport error"`. Update `call`'s doc comment.
+
+### Step 4 — call sites
+`kvm.go` Create/Get/Delete pass `log`, ListByTag passes `hlog`;
+`kvm_integration_test.go:220` passes `p.logger(ctx).WithValues("host", host.Name)`;
+`conn_test.go` (12 `p.call(` sites) gets `var discard = logr.Discard()`.
+
+### Step 5 — integration test output
+- `integrationLogger(t)`: `KVM_TEST_LOG_LEVEL` (default `1`, `strconv.Atoi`,
+  `t.Fatalf` on garbage) → `testr.NewWithOptions(t, testr.Options{LogTimestamp:
+  true, Verbosity: n})` (`github.com/go-logr/logr/testr`, present in logr v1.4.3,
+  no go.mod change); `logf.SetLogger(log)` once so nothing falls back to the
+  unfulfilled root.
+- `TestKVM_lifecycle`: `ctx = logr.NewContext(ctx, log)` after `WithTimeout`; the
+  cleanup ctx gets the same logger.
+- Header comment: document `KVM_TEST_LOG_LEVEL` (1 = one line per RPC, 2 adds
+  XML/metadata/addresses, 0 = only `t.Log`) and the late-line-after-timeout caveat.
+
+### Step 6 — unit tests
+- `kvm_test.go`: `captureContext(verbosity)` like `gcp_test.go:277-286` but with a
+  mutex around the slice; `TestLogging_verbosityTiers` (mirror `gcp_test.go:322-399`):
+  V0 → no lines; V1 → `"msg"="libvirt call"`, `"method"="DomainDefineXML"`,
+  `"method"="StorageVolUpload"`, `"msg"="libvirt host connected"`, provider/host/name
+  values, `"libvirtCodeName"="ErrNoDomain"` from the first lookup, no `"xml"=`;
+  V2 → `"xml"="<domain` and `"metadata"=`; a cloud-init sentinel absent at every tier.
+- `trace_test.go`: `TestTracingAPI_logsEveryMethod` (table of 17 calls over a
+  `readyHost()` fake, exactly one line each, plus
+  `reflect.TypeOf((*libvirtAPI)(nil)).Elem().NumMethod() == len(cases)` so a new
+  interface method without tracing fails); `TestTracingAPI_passesResultsThrough`;
+  `TestTracingAPI_errorFields` (libvirt error → code/name/message; `io.EOF` →
+  `"error"="EOF"`, no code); `TestTracingAPI_detailOnlyAtV2`;
+  `TestTracingAPI_uploadLogsSizeNotContent`.
+- `conn_test.go`: `fakeDialer` gains an `api` field; `TestHostPool_call_tracesAtV1`
+  (V1 → connected + method lines; V0 → no lines and fn receives the bare
+  `*fakeAPI`); `TestHostPool_call_logsDropReason` (transport error, cancel).
+
+### Step 7 — docs, execution log, CHANGELOG
+- `docs/kvm-provider.md`: `## Logging` section before "Known gaps":
+  `--zap-log-level=debug` = one `libvirt call` line per RPC plus connect/drop lines;
+  `--zap-log-level=2` adds XML, metadata, addresses; cloud-init never logged; no
+  V(5) wire tier because go-libvirt exposes no hook. Mention `KVM_TEST_LOG_LEVEL`
+  in its Testing paragraph.
+- `docs/testing.md`: `KVM_TEST_LOG_LEVEL` in the optional vars; `-v` now shows every
+  RPC; extend the `kvm_test.go` table row.
+- `docs/plans-executions/2026-09-10-0805-kvm-libvirt-tracing.md` with the checklist
+  and per-step entries; also a short cross-reference line in the KVM provider
+  execution log's Step 7 section.
+- `CHANGELOG.md` entry once the user confirms the live run shows the trace.
+
+## Verification
+
+```bash
+go build ./... && go vet ./... && go vet -tags=integration ./internal/provider/kvm/
+go test -race -count=1 ./internal/provider/kvm/
+go test -short ./...
+```
+Live, with the usual env: `make test-integration-kvm`. Expect, interleaved with
+`t.Log` lines and attributed to `trace.go:<line>`, lines like
+
+```
+kvm.libvirt: "level"=1 "msg"="libvirt call" "provider"="kvm-integration" "host"="kvm-test" "name"="proxy-…" "method"="DomainLookupByName" "domain"="proxy-…" "duration"="2.1ms" "error"="…" "libvirtCode"=42 "libvirtCodeName"="ErrNoDomain" "libvirtMessage"="…"
+```
+
+plus one `libvirt host connected` line with `duration`, a `StorageVolUpload` line
+whose duration is the seed upload, and no "log.SetLogger(...) was never called"
+text. `KVM_TEST_LOG_LEVEL=2` shows `"xml"="<domain` and `"addresses"=`;
+`KVM_TEST_LOG_LEVEL=0` shows only `t.Log` output. No line at any level contains
+`#cloud-config`.
+
+## Critical files
+- `internal/provider/kvm/conn.go`, `trace.go` (new), `trace_test.go` (new)
+- `internal/provider/kvm/errors.go`, `errors_test.go`
+- `internal/provider/kvm/kvm.go`, `kvm_test.go`, `conn_test.go`
+- `internal/provider/kvm/kvm_integration_test.go`
+- `docs/kvm-provider.md`, `docs/testing.md`

docs/plans/2026-09-10-2222-kvm-review-fixes.md

@@ -0,0 +1,195 @@
+# Plan: Fix the five code-review findings on `feat/kvm-provider`
+**Created:** 2026-09-10 22:22 UTC
+
+## Context
+
+PR #5 (`feat/kvm-provider` → `main`, 19 commits) is open and awaiting merge. A
+`/code-review high` pass over the branch returned five confirmed findings: two that can
+pin a Proxy's finalizer forever, one startup crash-loop, and two in the guest-agent merge
+committed earlier today. All five are fixed on the same branch so the PR updates in place,
+then re-validated against the real hypervisor before merging.
+
+Four decisions, made by the user during planning:
+
+1. **Wedged deletes** — attempt `Delete` even when `Get` fails permanently, make an
+   unconfigured KVM host a no-op delete, and add an annotation that abandons cleanup for
+   whatever still cannot finish.
+2. **Missing `kvm-ssh` Secret** — read the SSH material lazily, so only KVM Proxies fail.
+3. **Guest agent** — stop rewriting the user's cloud-config. Ship the bootstrap as a
+   vendor-data shell script instead.
+4. Validation is unit tests, the integration test against beelink, and the kind e2e. The
+   uncommitted dev-only files stay exactly as they are and out of the PR.
+
+## Why the guest-agent fix is a rewrite, not a patch
+
+Both cloud-init findings come from the same root cause: this operator edits the user's
+document. Reading cloud-init 26.1's sources settles that it never has to.
+
+Vendor-data is consumed by its **own** handler chain with separate script and config paths
+(`stages.py:_default_vendordata_handlers`). A vendor-data **shell script** is therefore
+written to the vendor script directory and run by `cc_scripts_vendor`, never passing
+through the config merger that drops colliding keys. On the final-stage module list it
+runs after `package_update_upgrade_install` and before the user's own `runcmd`, so the
+agent comes up early regardless of what the user's config does.
+
+That also closes the door the previous approach left open. The merge cannot be rescued
+from inside the documents: `merge_how` in the user-data governs only the first source, and
+`merge_how` in vendor-data is popped and discarded by `dict_extract_mergers` before the
+document is ever written. There is no supported way to make vendor-data cloud-config keys
+survive a user-data that defines the same key.
+
+Consequences: user-data reaches the seed byte for byte in any format, so Jinja templates,
+MIME archives, gzip payloads and shell scripts stop being special cases (finding 4), and
+nothing re-serialises YAML, so unquoted scalars keep their type (finding 5). The cost is
+that the script picks the package manager itself where cloud-init's `packages` key was
+distro-agnostic.
+
+## Why the delete fix is shaped this way
+
+KVM is the only provider that resolves a providerID segment against operator config. GCP
+and Kubernetes let the backend answer "not found", so retiring a location yields
+`ErrNotFound` and the finalizer drops. `Provider.host()`
+([kvm.go:118-139](internal/provider/kvm/kvm.go#L118-L139)) returns `ErrPermanent`, and
+`reconcileDelete` ([proxy_controller.go:281-291](internal/controller/proxy_controller.go#L281-L291))
+drops the finalizer only on `ErrNotFound`. `deletionFailure`
+([:327-336](internal/controller/proxy_controller.go#L327-L336)) says in its own comment
+that it retries "until an operator intervenes", and no intervention mechanism exists. The
+annotation is that mechanism, and it also rescues the unconfigured-provider wedge at
+[:276-279](internal/controller/proxy_controller.go#L276-L279).
+
+`Get` keeps returning `ErrPermanent` for an unknown host deliberately. Turning it into
+`ErrNotFound` would make a typo in the host list look like a missing VM on the live path,
+and the reconciler would recreate every affected Proxy elsewhere while the originals keep
+running where garbage collection cannot see them.
+
+## Steps
+
+One commit per step on `feat/kvm-provider`, each ending with
+`Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>`.
+
+### Step 0 — Save this plan into the repo
+Copy to `docs/plans/2026-09-10-2222-kvm-review-fixes.md` with the same header timestamp and
+commit it alone, before any code change. Start
+`docs/plans-executions/2026-09-10-2222-kvm-review-fixes.md` with the status checklist when
+Step 1 lands.
+
+### Step 1 — Unwedge deletion (finding 1)
+- `api/v1alpha1/proxy_types.go`: add `AnnotationSkipCleanup = "crawl.example.com/skip-cleanup"`
+  beside `AnnotationSpecHash` ([:59-63](api/v1alpha1/proxy_types.go#L59-L63)), documented as
+  abandoning the backend instance.
+- `internal/controller/proxy_controller.go`, `reconcileDelete`: honour the annotation after
+  the empty-providerID branch and **before** the provider lookup, so it also covers a
+  provider that is no longer configured. Log the abandoned providerID at Info.
+  `ProxyReconciler` has no `EventRecorder`; do not add one for this.
+- Same function: replace the two `if`s after `Get` with a switch. On
+  `provider.Class(err) == provider.ErrPermanent`, call `Delete` anyway, since it is
+  idempotent, and remove the finalizer when it succeeds rather than requeueing into a `Get`
+  that can never converge. On failure, `deletionFailure(errors.Join(getErr, delErr))`.
+  Guard on `err != nil`, since `Class(nil)` is nil.
+- `internal/provider/kvm/kvm.go`, `Delete`: an unconfigured host segment means there is
+  nothing this operator can clean, so log it and return success, mirroring `removeVolumes`
+  ([kvm.go:506-514](internal/provider/kvm/kvm.go#L506-L514)) for a missing pool. `Get` is
+  unchanged.
+- Tests: extend the deletion table in `internal/controller/reconcile_test.go`
+  ([:357-393](internal/controller/reconcile_test.go#L357-L393)) with permanent-Get plus
+  succeeding Delete, permanent-Get plus failing Delete, the annotation with the provider
+  never called, and the annotation on an unconfigured provider. In `kvm_test.go`, the
+  `"host not configured"` case at [:737](internal/provider/kvm/kvm_test.go#L737) flips from
+  `ErrPermanent` to success; add a named test beside `TestDelete_missingPoolIsNotAnError`
+  ([:715](internal/provider/kvm/kvm_test.go#L715)).
+- Docs: the annotation in `README.md` (~line 163) and `docs/architecture.md` (~line 487);
+  a *Known gaps* entry in `docs/kvm-provider.md` saying a domain on a host removed from
+  config can no longer be cleaned up by the operator, the Proxy still finishes deleting,
+  and the VM must be removed by hand because garbage collection only sweeps configured
+  hosts.
+
+### Step 2 — Validate names that become identifiers (finding 2)
+- `internal/provider/config_kvm.go`: add a name regexp beside `sha256Hex`
+  ([:6](internal/provider/config_kvm.go#L6)) allowing letters, digits, dot, dash and
+  underscore. Apply it to `hosts[].name` after the existing empty check
+  ([:129-131](internal/provider/config_kvm.go#L129-L131)) and to each `images` map key,
+  which becomes the cached volume name `base-<key>.<format>`
+  ([image.go:49-55](internal/provider/kvm/image.go#L49-L55)) and half of the image-cache
+  lock key ([image.go:96](internal/provider/kvm/image.go#L96)). `machineTypes` keys are only
+  compared, never embedded, so leave them.
+- Keep the file's first-error-wins style and `fmt.Sprintf` field paths. The host message
+  should name the consequence: the value becomes part of `hosts/<name>/domains/<vm>`, which
+  `parseProviderID` splits on `/` ([id.go:11-21](internal/provider/kvm/id.go#L11-L21)).
+- Tests: two entries in the `TestLoadConfig_kvmInvalid` table
+  ([config_kvm_test.go:76](internal/provider/config_kvm_test.go#L76)), existing
+  name/yaml/wantErrSub shape.
+
+### Step 3 — Read the SSH material lazily (finding 3)
+- `internal/provider/kvm/conn.go`: keep `newSSHAuth`, and add a lazy dialer that builds it
+  on first dial and caches it once it succeeds, so a `kvm-ssh` Secret created after the
+  manager starts heals on the next reconcile with no restart.
+- `internal/provider/kvm/kvm.go`, `New`: stop calling `newSSHAuth`; pass the lazy dialer.
+  Use the context parameter it currently ignores to log a warning when either file is
+  unreadable at startup, keeping the diagnostic without the exit.
+- Error classes: a missing or unreadable file stays unclassified, so `Class` defaults it to
+  transient and the Proxy retries until the Secret appears; an unparseable key or malformed
+  known_hosts wraps `ErrPermanent`. Distinguish with `errors.Is(err, fs.ErrNotExist)`.
+- Fix the two doc comments promising startup failure
+  ([kvm.go:79-81](internal/provider/kvm/kvm.go#L79-L81),
+  [conn.go:182-185](internal/provider/kvm/conn.go#L182-L185)) and the matching claim in
+  `docs/kvm-provider.md` (~lines 107-109). `DISCOVERY_TOKEN` already sets the house
+  convention for an optional mount: degrade with a loud warning.
+- Tests in `conn_test.go`, which already writes key files: `New` succeeds with no key
+  present, the first dial fails transiently, a dial after the file appears succeeds, and a
+  corrupt key is permanent.
+
+### Step 4 — Guest agent via vendor-data script (findings 4 and 5)
+- `internal/provider/kvm/seed.go`: delete `withGuestAgent`, `isCloudConfig`, `appendUnique`
+  and the `sigs.k8s.io/yaml` import. User-data goes onto the seed exactly as the Proxy
+  supplied it.
+- Add the bootstrap as a `#!/bin/sh` vendor-data script: skip when the agent is already
+  present, otherwise install `qemu-guest-agent` through whichever of `apt-get`, `dnf`,
+  `yum`, `zypper` or `apk` exists, then `systemctl enable --now`. Run `apt-get update`
+  before installing, since nothing else guarantees a fresh index when the user's config has
+  no `packages` key. Keep it idempotent and let it log to stdout, which lands in
+  `cloud-init-output.log`.
+- When the provider config also sets `vendorData`, wrap both into a MIME multipart
+  vendor-data: the user's document as one part, the script as another. Cloud-init detects
+  multipart by finding `mime-version:` in the first 4096 bytes, so the header must lead.
+- `internal/provider/config_kvm.go`: add `installGuestAgent`, defaulting to on. Off means
+  the seed carries only the user's own vendor-data and IP discovery falls back to the host
+  ARP table. Nothing fails permanently any more, so the error path in `writeSeed`
+  ([kvm.go:252-255](internal/provider/kvm/kvm.go#L252-L255)) narrows to MIME assembly.
+- Tests in `seed_test.go`: user-data byte-identical for cloud-config, Jinja, MIME, gzip and
+  script inputs; the script present in vendor-data; the opt-out; the multipart case
+  including the `mime-version:` position; keep the existing ISO tests.
+- Docs: rewrite the *What lands on the host* passage added earlier today, the `vendorData`
+  row and a new `installGuestAgent` row in the config table, and the ARP note in
+  *Known gaps*. Cite the cloud-init behaviour that forced the change.
+
+### Step 5 — Validate, record, push
+- `gofmt -l internal/`, `go vet ./...`, `go test -short ./...`.
+- `make test-integration-kvm` against beelink, environment as recorded in the KVM execution
+  log.
+- Rebuild the image, `kind load`, restart the manager, delete and recreate both Proxies,
+  curl a test URL through each.
+- `CHANGELOG.md` entry at the top with a real `date` timestamp, noting it supersedes the
+  merge approach described in today's earlier entry. Add a Run 8 section to
+  `docs/plans-executions/2026-09-04-1404-kvm-provider.md` plus this plan's own execution
+  file. Push with the token helper; PR #5 updates in place.
+
+## Verification
+- Unit: the controller cases prove a permanent `Get` no longer pins the finalizer and that
+  the annotation short-circuits before the provider is touched. `TestDelete_*` proves an
+  unconfigured host is a clean delete while `TestGet_*` stays permanent, which is what keeps
+  the live path from recreating VMs.
+- Seed: assert the uploaded user-data equals the Proxy's `cloudInit` byte for byte, which is
+  the property the old merge could not offer.
+- Integration on beelink: Create, IP via the guest agent, ListByTag, Delete. In the guest,
+  `cloud-init status --long` clean and `systemctl is-active qemu-guest-agent` active.
+- In-cluster: both Proxies reach Ready with an IP and return 204 through squid. Their
+  cloud-config defines its own `packages`, which is the exact case that failed in Run 7.
+- Negative check in kind: rename the `kvm-ssh` Secret away and restart. The pod must stay
+  up, the Kubernetes Proxies must stay Ready, and only the KVM Proxies degrade. Restore the
+  Secret and confirm they recover without a restart.
+
+## Out of scope
+- The two items the review noted but did not report: deleting a cached base image while
+  running VMs back onto it, and an ad-hoc image URL always being treated as qcow2. Both are
+  documented known gaps and neither is a regression from this branch.
+- The uncommitted dev-only files, by the user's decision.

docs/testing.md

@@ -56,7 +56,8 @@ manual runs too.
 | `internal/provider/gcp` | `insert_test.go`, `errors_test.go`, `gcp_test.go` | field-by-field `buildInsertRequest`; HTTP-code classification table; zone-qualified IDs, 409-is-success, 9-row state mapping, ListByTag filter + `ReturnPartialSuccess` | pure builder needs **no fake**; the rest uses the flattened `instancesAPI` seam |
 | `internal/provider/kvm` | `domainxml_test.go`, `volume_test.go`, `seed_test.go`, `id_test.go`, `metadata_test.go` | pure builders: domain/volume XML unmarshalled back and checked field by field, seed ISO reopened with the same library, providerID round-trip, metadata in both namespace spellings | pure |
 | | `errors_test.go`, `conn_test.go` | libvirt-code classification table; connection pool (reuse, redial, drop on transport error, cancel releases a blocked RPC, one dial per host under concurrency); SSH dial, host-key policy, keepalive | fake dialer; a **real in-process `x/crypto/ssh` server** forwarding a unix socket |
-| | `image_test.go`, `kvm_test.go` | image cache (fresh fetch, ready, marker missing, checksum change/mismatch, 4xx/5xx, concurrent callers); Create resume after every partial step, leftover truncated seed, state/IP mapping, every Delete already-gone case, ListByTag across hosts / one down / unreadable domain | `httptest` image server + in-memory `fakeAPI` behind the `libvirtAPI` seam |
+| | `image_test.go`, `kvm_test.go` | image cache (fresh fetch, ready, marker missing, checksum change/mismatch, 4xx/5xx, concurrent callers); Create resume after every partial step, leftover truncated seed, state/IP mapping, every Delete already-gone case, ListByTag across hosts / one down / unreadable domain; logging tiers (V0 silent, V1 one line per RPC, V2 XML) with cloud-init never logged | `httptest` image server + in-memory `fakeAPI` behind the `libvirtAPI` seam |
+| | `trace_test.go`, `conn_test.go`, `errors_test.go` | every `libvirtAPI` method traced exactly once (guarded by reflection on the interface), results and errors pass through, libvirt error fields, detail keys only at V2, upload content never logged; pool connect/drop lines, no wrapping below V1 | `fakeAPI` + `funcr` capture |
 | | `kvm_integration_test.go` (`-tags=integration`) | one real VM: image fetch, Create, idempotent re-Create, Running with IP, ListByTag, Delete, pool state afterwards | a **real hypervisor** — see below |
 | `internal/health` | `probe_test.go` | probes through a **real CONNECT-capable proxy stub** to a real TLS server: tunnel success, refused CONNECT, wrong status, dead proxy, plain-http forward | `httptest` + hijacked bidirectional tunnel |
 | | `engine_test.go` | thresholds, first-verdict, latency suppression matrix, dropped-event retry, stale-probe UID guard, tick scheduling/seeding/pruning, `Start` end-to-end | fake `client.Reader` + injected `probeFn` |
@@ -116,8 +117,16 @@ make test-integration-kvm
 Optional: `KVM_TEST_KNOWN_HOSTS` (without it host keys are **not**
 verified — the log says so), `KVM_TEST_IMAGE_SHA256`, `KVM_TEST_POOL`
 (default `default`), `KVM_TEST_SOCKET`, `KVM_TEST_TIMEOUT` (default
-`20m`, must cover the first-time image download), and `KVM_TEST_KEEP`
+`20m`, must cover the first-time image download), `KVM_TEST_KEEP`
-to leave the VM behind for `virsh console` when something goes wrong.
+to leave the VM behind for `virsh console` when something goes wrong,
+and `KVM_TEST_LOG_LEVEL` (default `1`) for how much of the provider's
+logging the `-v` output shows: at `1` every libvirt RPC is one compact
+line (`HH:MM:SS.mmm libvirt virDomainLookupByName domain=... duration=...
+error=... libvirtError=ErrNoDomain`), interleaved with the test's own
+`t.Log` lines; `2` adds the domain and volume XML, metadata and
+addresses; `0` leaves only `t.Log`. The per-run constants (provider,
+host, domain name) are left off each line. See the Logging section of
+[kvm-provider.md](kvm-provider.md#logging).
 
 What one run asserts, in order: `New` dials the host; `Create` returns a
 `hosts/<host>/domains/<name>` ID (downloading the image on a cold host —

internal/controller/proxy_controller.go

@@ -270,8 +270,14 @@ func (r *ProxyReconciler) reconcileDelete(ctx context.Context, p *crawlv1alpha1.
 	if p.Status.ProviderID == "" {
 		// Nothing was ever recorded as created; orphan GC reaps any stray
 		// instance a crashed create might have left behind.
-		controllerutil.RemoveFinalizer(p, crawlv1alpha1.FinalizerName)
+		return r.releaseFinalizer(ctx, p)
-		return ctrl.Result{}, r.Update(ctx, p)
+	}
+	if p.Annotations[crawlv1alpha1.AnnotationSkipCleanup] == "true" {
+		// Checked before the provider lookup so it also releases a Proxy whose
+		// provider is gone from the config. The instance is left running.
+		logf.FromContext(ctx).Info("skip-cleanup annotation set; abandoning the instance",
+			"providerID", p.Status.ProviderID, "provider", p.Spec.Provider)
+		return r.releaseFinalizer(ctx, p)
 	}
 	prov, ok := r.Providers[p.Spec.Provider]
 	if !ok {
@@ -279,11 +285,22 @@ func (r *ProxyReconciler) reconcileDelete(ctx context.Context, p *crawlv1alpha1.
 			"provider %q is not configured; cannot clean up instance %s", p.Spec.Provider, p.Status.ProviderID)
 	}
 	_, err := prov.Get(ctx, p.Status.ProviderID)
-	if provider.Class(err) == provider.ErrNotFound {
+	switch {
-		controllerutil.RemoveFinalizer(p, crawlv1alpha1.FinalizerName)
+	case err == nil:
-		return ctrl.Result{}, r.Update(ctx, p)
+	case provider.Class(err) == provider.ErrNotFound:
+		return r.releaseFinalizer(ctx, p)
+	case provider.Class(err) == provider.ErrPermanent:
+		// Get will never answer for this instance — a host retired from the
+		// config, a providerID the provider can no longer resolve. Delete is
+		// idempotent and may still finish, so try it instead of polling a Get
+		// that cannot converge, and release the object when it does.
+		if delErr := prov.Delete(ctx, p.Status.ProviderID); delErr != nil {
+			return r.deletionFailure(errors.Join(err, delErr))
 		}
-	if err != nil {
+		logf.FromContext(ctx).Info("instance unreachable but cleanup succeeded; removing the finalizer",
+			"providerID", p.Status.ProviderID, "error", err.Error())
+		return r.releaseFinalizer(ctx, p)
+	default:
 		return r.deletionFailure(err)
 	}
 	if err := prov.Delete(ctx, p.Status.ProviderID); err != nil {
@@ -294,6 +311,11 @@ func (r *ProxyReconciler) reconcileDelete(ctx context.Context, p *crawlv1alpha1.
 	return ctrl.Result{RequeueAfter: r.DeletionPoll}, nil
 }
 
+func (r *ProxyReconciler) releaseFinalizer(ctx context.Context, p *crawlv1alpha1.Proxy) (ctrl.Result, error) {
+	controllerutil.RemoveFinalizer(p, crawlv1alpha1.FinalizerName)
+	return ctrl.Result{}, r.Update(ctx, p)
+}
+
 func (r *ProxyReconciler) reconcileExternal(_ context.Context, p *crawlv1alpha1.Proxy) (ctrl.Result, error) {
 	if p.Spec.Endpoint == nil {
 		// CEL guarantees an endpoint on any object that went through the API

internal/controller/reconcile_test.go

@@ -2,6 +2,7 @@ package controller
 
 import (
 	"context"
+	"slices"
 	"testing"
 	"time"
 
@@ -78,6 +79,10 @@ func notFoundErr() error {
 	return provider.Wrap(provider.ErrNotFound, "get", "stub", "some-id", nil)
 }
 
+func permanentErr() error {
+	return provider.Wrap(provider.ErrPermanent, "get", "stub", "some-id", nil)
+}
+
 func testScheme(t *testing.T) *runtime.Scheme {
 	t.Helper()
 	s := runtime.NewScheme()
@@ -125,6 +130,15 @@ func withSpecHashAnnotation(hash string) func(*crawlv1alpha1.Proxy) {
 	}
 }
 
+func withSkipCleanup() func(*crawlv1alpha1.Proxy) {
+	return func(p *crawlv1alpha1.Proxy) {
+		if p.Annotations == nil {
+			p.Annotations = map[string]string{}
+		}
+		p.Annotations[crawlv1alpha1.AnnotationSkipCleanup] = "true"
+	}
+}
+
 func deleting() func(*crawlv1alpha1.Proxy) {
 	return func(p *crawlv1alpha1.Proxy) {
 		now := metav1.Now()
@@ -375,6 +389,59 @@ func TestReconcile_actionTable(t *testing.T) {
 				assertProxyGone(t, r)
 			},
 		},
+		{
+			// A permanent Get (a retired KVM host, a providerID the provider
+			// cannot resolve) used to retry forever with the finalizer on.
+			name:       "deletion with an unreachable instance still deletes and releases",
+			proxy:      managedProxy(deleting(), withProviderID("stub-id-1")),
+			stub:       &stubProvider{getErr: permanentErr()},
+			wantResult: ctrl.Result{},
+			verify: func(t *testing.T, r *ProxyReconciler, stub *stubProvider) {
+				assertProxyGone(t, r)
+				if stub.deleteCalls != 1 {
+					t.Errorf("deleteCalls = %d, want 1 — Delete must be tried when Get cannot answer", stub.deleteCalls)
+				}
+			},
+		},
+		{
+			name:  "deletion keeps the finalizer when cleanup itself fails",
+			proxy: managedProxy(deleting(), withProviderID("stub-id-1")),
+			stub: &stubProvider{
+				getErr:    permanentErr(),
+				deleteErr: provider.Wrap(provider.ErrPermanent, "delete", "stub", "stub-id-1", nil),
+			},
+			wantResult: ctrl.Result{},
+			wantErr:    true,
+			verify: func(t *testing.T, r *ProxyReconciler, _ *stubProvider) {
+				// getProxy fails the test if the object is gone, which is what
+				// finalizer removal would have caused.
+				if p := getProxy(t, r); !slices.Contains(p.Finalizers, crawlv1alpha1.FinalizerName) {
+					t.Error("finalizer removed although cleanup failed; the instance would leak silently")
+				}
+			},
+		},
+		{
+			name:       "skip-cleanup annotation abandons the instance",
+			proxy:      managedProxy(deleting(), withProviderID("stub-id-1"), withSkipCleanup()),
+			stub:       &stubProvider{getInst: &provider.Instance{ID: "stub-id-1", State: provider.StateRunning}},
+			wantResult: ctrl.Result{},
+			verify: func(t *testing.T, r *ProxyReconciler, stub *stubProvider) {
+				assertProxyGone(t, r)
+				if stub.getCalls != 0 || stub.deleteCalls != 0 {
+					t.Errorf("getCalls/deleteCalls = %d/%d, want 0/0 — the provider must not be asked", stub.getCalls, stub.deleteCalls)
+				}
+			},
+		},
+		{
+			name: "skip-cleanup annotation releases a proxy whose provider is gone",
+			proxy: managedProxy(deleting(), withProviderID("stub-id-1"), withSkipCleanup(),
+				func(p *crawlv1alpha1.Proxy) { p.Spec.Provider = "retired" }),
+			stub:       &stubProvider{},
+			wantResult: ctrl.Result{},
+			verify: func(t *testing.T, r *ProxyReconciler, _ *stubProvider) {
+				assertProxyGone(t, r)
+			},
+		},
 		{
 			name:  "deletion deletes the instance and polls",
 			proxy: managedProxy(deleting(), withProviderID("stub-id-1")),

internal/provider/config_kvm.go

@@ -22,10 +22,18 @@ type KVMConfig struct {
 	// DiskSizeGB is the capacity of each VM's overlay disk. Default 10. The
 	// guest's cloud-init growpart expands the root filesystem into it.
 	DiskSizeGB int64 `json:"diskSizeGb,omitempty"`
-	// VendorData is cloud-init vendor-data placed on every seed next to the
+	// VendorData is optional cloud-init vendor-data placed on every seed next
-	// Proxy's own user-data. Default: installs and starts qemu-guest-agent so
+	// to the Proxy's own user-data. Note that cloud-init drops any top-level
-	// the provider can read the VM's IP even before it shows up in ARP.
+	// key of a vendor-data cloud-config that the user-data also defines.
+	// When InstallGuestAgent is on, this document and the agent script are
+	// carried as separate parts of a MIME multipart vendor-data.
 	VendorData string `json:"vendorData,omitempty"`
+	// InstallGuestAgent adds a vendor-data script that installs and starts
+	// qemu-guest-agent, which is how the provider learns a VM's address
+	// before the host's ARP table has seen traffic from it. Default true.
+	// Turn it off for images that already ship the agent, or that cannot
+	// reach a package mirror; discovery then depends on ARP alone.
+	InstallGuestAgent *bool `json:"installGuestAgent,omitempty"`
 	// Images are named base images; placement.image names a key here (or is
 	// an http(s) URL directly). The provider downloads each image into a
 	// host's storage pool on first use and caches it there.
@@ -47,6 +55,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.
@@ -75,8 +88,31 @@ type KVMHost struct {
 	Bridge string `json:"bridge"`
 }
 
+// Accepted KVMImage.Format and KVMImage.Firmware values.
+const (
+	KVMFormatQcow2  = "qcow2"
+	KVMFormatRaw    = "raw"
+	KVMFirmwareUEFI = "uefi"
+	KVMFirmwareBIOS = "bios"
+)
+
+// IsHTTPURL reports whether s is an absolute http(s) URL with a host, the
+// only kind of image source the kvm provider can fetch.
+func IsHTTPURL(s string) bool {
+	u, err := url.Parse(s)
+	return err == nil && (u.Scheme == "http" || u.Scheme == "https") && u.Host != ""
+}
+
 var sha256Hex = regexp.MustCompile(`^[0-9a-fA-F]{64}$`)
 
+// kvmIdentifier constrains the config names that are pasted into
+// identifiers elsewhere: a host name goes into the providerID
+// hosts/<name>/domains/<vm>, which is split on "/" to parse, and an image
+// key becomes the file name of the cached base volume. A slash in either
+// produces an ID or a path that never resolves again, so the VM leaks with
+// nothing able to address it.
+var kvmIdentifier = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]*$`)
+
 func validateKVM(prefix string, c *KVMConfig) error {
 	if c == nil {
 		return fmt.Errorf("%s: kvm block is required", prefix)
@@ -92,15 +128,21 @@ func validateKVM(prefix string, c *KVMConfig) error {
 	}
 	for name, img := range c.Images {
 		p := fmt.Sprintf("%s: kvm.images[%q]", prefix, name)
-		u, err := url.Parse(img.URL)
+		if !kvmIdentifier.MatchString(name) {
-		if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
+			return fmt.Errorf("%s: key must be letters, digits, dots, dashes or underscores"+
+				" — it becomes the cached volume name base-<key>.<format>", p)
+		}
+		if !IsHTTPURL(img.URL) {
 			return fmt.Errorf("%s.url must be an http(s) URL (got %q)", p, img.URL)
 		}
 		if img.SHA256 != "" && !sha256Hex.MatchString(img.SHA256) {
 			return fmt.Errorf("%s.sha256 must be 64 hex characters", p)
 		}
-		if img.Format != "" && img.Format != "qcow2" && img.Format != "raw" {
+		if img.Format != "" && img.Format != KVMFormatQcow2 && img.Format != KVMFormatRaw {
-			return fmt.Errorf("%s.format must be qcow2 or raw (got %q)", p, img.Format)
+			return fmt.Errorf("%s.format must be %s or %s (got %q)", p, KVMFormatQcow2, KVMFormatRaw, img.Format)
+		}
+		if img.Firmware != "" && img.Firmware != KVMFirmwareUEFI && img.Firmware != KVMFirmwareBIOS {
+			return fmt.Errorf("%s.firmware must be %s or %s (got %q)", p, KVMFirmwareUEFI, KVMFirmwareBIOS, img.Firmware)
 		}
 	}
 	if len(c.MachineTypes) == 0 {
@@ -120,6 +162,10 @@ func validateKVM(prefix string, c *KVMConfig) error {
 		if h.Name == "" {
 			return fmt.Errorf("%s: name is required", p)
 		}
+		if !kvmIdentifier.MatchString(h.Name) {
+			return fmt.Errorf("%s: name %q must be letters, digits, dots, dashes or underscores"+
+				" — it becomes part of the providerID hosts/<name>/domains/<vm>", p, h.Name)
+		}
 		if seen[h.Name] {
 			return fmt.Errorf("%s: duplicate host name %q", p, h.Name)
 		}

internal/provider/config_kvm_test.go

@@ -129,6 +129,19 @@ hosts:
 			yaml:       minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n  small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n  - {name: nas1, address: a:22, user: virt, bridge: br0}\n  - {name: nas1, address: b:22, user: virt, bridge: br0}\n"),
 			wantErrSub: `duplicate host name "nas1"`,
 		},
+		{
+			// A slash would produce hosts/dc1/rack2/domains/<vm>, which
+			// parseProviderID can never split back, so the VM would be
+			// unreachable for Get, Delete and GC alike.
+			name:       "host name with a slash",
+			yaml:       minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n  small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n  - {name: dc1/rack2, address: a:22, user: virt, bridge: br0}\n"),
+			wantErrSub: `name "dc1/rack2" must be letters, digits, dots, dashes or underscores`,
+		},
+		{
+			name:       "image key with a slash",
+			yaml:       minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n  ../etc/d: {url: https://x/y.qcow2}\n" + hostsAndTypes),
+			wantErrSub: `images["../etc/d"]: key must be letters, digits, dots, dashes or underscores`,
+		},
 		{
 			name:       "host missing address",
 			yaml:       minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n  small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n  - {name: nas1, user: virt, bridge: br0}\n"),
@@ -154,6 +167,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),

internal/provider/kvm/conn.go

@@ -2,14 +2,17 @@ package kvm
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net"
 	"os"
 	"sync"
 	"time"
 
 	libvirt "github.com/digitalocean/go-libvirt"
+	"github.com/go-logr/logr"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/knownhosts"
 
@@ -95,7 +98,7 @@ func (p *hostPool) entry(name string) *hostEntry {
 	return e
 }
 
-func (p *hostPool) get(ctx context.Context, host provider.KVMHost) (*hostClient, error) {
+func (p *hostPool) get(ctx context.Context, log logr.Logger, host provider.KVMHost) (*hostClient, error) {
 	e := p.entry(host.Name)
 	e.mu.Lock()
 	defer e.mu.Unlock()
@@ -103,13 +106,17 @@ func (p *hostPool) get(ctx context.Context, host provider.KVMHost) (*hostClient,
 		if e.client.alive() {
 			return e.client, nil
 		}
+		log.V(1).Info("libvirt connection lost, redialing", "address", host.Address)
 		e.client.close()
 		e.client = nil
 	}
+	start := time.Now()
 	c, err := p.dial(ctx, host)
 	if err != nil {
+		log.V(1).Info("libvirt host dial failed", "address", host.Address, "duration", time.Since(start).Round(time.Millisecond), "error", err.Error())
 		return nil, fmt.Errorf("connecting to host %s (%s): %w", host.Name, host.Address, err)
 	}
+	log.V(1).Info("libvirt host connected", "address", host.Address, "duration", time.Since(start).Round(time.Millisecond))
 	e.client = c
 	return c, nil
 }
@@ -129,16 +136,29 @@ func (p *hostPool) drop(host string, c *hostClient) {
 // call runs fn against host's connection. go-libvirt RPCs take no context
 // and block until the daemon answers, so when ctx ends first the connection
 // is closed — which unblocks fn with an error — and ctx.Err() is returned.
-// A transport error (anything that is not a libvirt.Error) also drops the
+// A transport error also drops the cached connection so the next call
-// cached connection so the next call redials instead of reusing a dead
+// redials instead of reusing a dead link. "Transport error" means anything
-// link.
+// that is neither a libvirt.Error nor an error fn already classified with a
-func (p *hostPool) call(ctx context.Context, host provider.KVMHost, fn func(api libvirtAPI) error) error {
+// provider sentinel (a failed seed build, a bad placement): those are local
-	c, err := p.get(ctx, host)
+// verdicts, not evidence about the link. When log has V(1) enabled, every
+// RPC fn makes is traced through it (see tracingAPI), as are connection
+// events.
+func (p *hostPool) call(ctx context.Context, log logr.Logger, host provider.KVMHost, fn func(api libvirtAPI) error) error {
+	log = log.WithName("libvirt")
+	c, err := p.get(ctx, log, host)
 	if err != nil {
 		return err
 	}
-	err = awaitOrCancel(ctx, func() error { return fn(c.api) }, func() { p.drop(host.Name, c) })
+	api := c.api
-	if err != nil && !isLibvirtError(err) {
+	if log.V(1).Enabled() {
+		api = &tracingAPI{inner: c.api, log: log}
+	}
+	err = awaitOrCancel(ctx, func() error { return fn(api) }, func() {
+		log.V(1).Info("libvirt connection dropped", "reason", "context cancelled", "error", ctx.Err().Error())
+		p.drop(host.Name, c)
+	})
+	if err != nil && !isLibvirtError(err) && !isClassified(err) && !errors.Is(err, ctx.Err()) {
+		log.V(1).Info("libvirt connection dropped", "reason", "transport error", "error", err.Error())
 		p.drop(host.Name, c)
 	}
 	return err
@@ -161,14 +181,17 @@ func awaitOrCancel(ctx context.Context, fn func() error, abort func()) error {
 }
 
 // sshAuth holds the pieces of the SSH client config shared by every host:
-// the private key and the host-key policy, both loaded once at startup so a
+// the private key and the host-key policy. Both are loaded on the first
-// missing or unreadable file fails the operator fast rather than on the
+// connection rather than at startup — see lazySSH.
-// first Create.
 type sshAuth struct {
 	signer  ssh.Signer
 	hostKey ssh.HostKeyCallback
 }
 
+// newSSHAuth loads the key material. A file that is not there yet stays
+// unclassified, so the provider treats it as transient and heals when the
+// Secret appears; content that will never parse is permanent, since
+// retrying cannot fix it.
 func newSSHAuth(cfg provider.KVMConfig) (*sshAuth, error) {
 	key, err := os.ReadFile(cfg.SSHKeyFile)
 	if err != nil {
@@ -176,23 +199,69 @@ func newSSHAuth(cfg provider.KVMConfig) (*sshAuth, error) {
 	}
 	signer, err := ssh.ParsePrivateKey(key)
 	if err != nil {
-		return nil, fmt.Errorf("parsing kvm.sshKeyFile: %w", err)
+		return nil, fmt.Errorf("%w: parsing kvm.sshKeyFile: %v", provider.ErrPermanent, err)
 	}
 	hostKey := ssh.InsecureIgnoreHostKey() //nolint:gosec // opt-in via config, documented as insecure
 	if !cfg.InsecureIgnoreHostKey {
 		hostKey, err = knownhosts.New(cfg.KnownHostsFile)
-		if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
 			return nil, fmt.Errorf("loading kvm.knownHostsFile: %w", err)
 		}
+		if err != nil {
+			return nil, fmt.Errorf("%w: loading kvm.knownHostsFile: %v", provider.ErrPermanent, err)
+		}
 	}
 	return &sshAuth{signer: signer, hostKey: hostKey}, nil
 }
 
-// dialer returns the production hostDialer: SSH to the host, forward its
+// lazySSH defers loading the SSH material until it is first needed and
-// libvirt unix socket through the connection, and open a libvirt session
+// caches it once it works. The kvm-ssh Secret is an optional mount: the
-// over that.
+// manager has to start without it and keep every other provider running,
-func (a *sshAuth) dialer() hostDialer {
+// and a Secret created afterwards has to heal the KVM proxies on the next
-	return func(ctx context.Context, host provider.KVMHost) (*hostClient, error) {
+// reconcile rather than needing a restart.
+type lazySSH struct {
+	cfg provider.KVMConfig
+
+	mu   sync.Mutex
+	auth *sshAuth
+}
+
+// newLazySSH keeps only the SSH fields of cfg, since the loader lives as
+// long as the Provider.
+func newLazySSH(cfg provider.KVMConfig) *lazySSH {
+	return &lazySSH{cfg: provider.KVMConfig{
+		SSHKeyFile:            cfg.SSHKeyFile,
+		KnownHostsFile:        cfg.KnownHostsFile,
+		InsecureIgnoreHostKey: cfg.InsecureIgnoreHostKey,
+	}}
+}
+
+func (l *lazySSH) load() (*sshAuth, error) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	if l.auth != nil {
+		return l.auth, nil
+	}
+	auth, err := newSSHAuth(l.cfg)
+	if err != nil {
+		return nil, err
+	}
+	l.auth = auth
+	return auth, nil
+}
+
+// dial is the production hostDialer.
+func (l *lazySSH) dial(ctx context.Context, host provider.KVMHost) (*hostClient, error) {
+	a, err := l.load()
+	if err != nil {
+		return nil, err
+	}
+	return a.dial(ctx, host)
+}
+
+// dial SSHes to the host, forwards its libvirt unix socket through the
+// connection, and opens a libvirt session over that.
+func (a *sshAuth) dial(ctx context.Context, host provider.KVMHost) (*hostClient, error) {
 	client, err := a.dialSSH(ctx, host)
 	if err != nil {
 		return nil, err
@@ -203,7 +272,6 @@ func (a *sshAuth) dialer() hostDialer {
 		return nil, err
 	}
 	return hc, nil
-	}
 }
 
 // dialSSH establishes the SSH connection. The handshake is bounded by ctx's

internal/provider/kvm/conn_test.go

@@ -7,6 +7,7 @@ import (
 	"crypto/rand"
 	"encoding/pem"
 	"errors"
+	"fmt"
 	"io"
 	"net"
 	"os"
@@ -18,6 +19,7 @@ import (
 	"time"
 
 	libvirt "github.com/digitalocean/go-libvirt"
+	"github.com/go-logr/logr"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/knownhosts"
 
@@ -43,6 +45,8 @@ type fakeDialer struct {
 	err     error
 	delay   time.Duration
 	clients []*fakeClient
+	// api is what the dialled client exposes; nil for pool-only tests.
+	api libvirtAPI
 }
 
 func (d *fakeDialer) dial(ctx context.Context, _ provider.KVMHost) (*hostClient, error) {
@@ -61,7 +65,7 @@ func (d *fakeDialer) dial(ctx context.Context, _ provider.KVMHost) (*hostClient,
 	}
 	fc := &fakeClient{done: make(chan struct{})}
 	d.clients = append(d.clients, fc)
-	return &hostClient{api: nil, done: fc.done, close: fc.close}, nil
+	return &hostClient{api: d.api, done: fc.done, close: fc.close}, nil
 }
 
 func (d *fakeDialer) dialCount() int {
@@ -72,6 +76,8 @@ func (d *fakeDialer) dialCount() int {
 
 var testHost = provider.KVMHost{Name: "nas1", Address: "nas1:22", User: "virt", Socket: defaultSocket, Pool: "default", Bridge: "br0"}
 
+var discard = logr.Discard()
+
 func noop(libvirtAPI) error { return nil }
 
 func TestHostPool_call_reusesConnection(t *testing.T) {
@@ -79,7 +85,7 @@ func TestHostPool_call_reusesConnection(t *testing.T) {
 	d := &fakeDialer{}
 	p := newHostPool(d.dial)
 	for range 3 {
-		if err := p.call(context.Background(), testHost, noop); err != nil {
+		if err := p.call(context.Background(), discard, testHost, noop); err != nil {
 			t.Fatalf("call() error = %v", err)
 		}
 	}
@@ -92,11 +98,11 @@ func TestHostPool_call_redialsAfterDisconnect(t *testing.T) {
 	t.Parallel()
 	d := &fakeDialer{}
 	p := newHostPool(d.dial)
-	if err := p.call(context.Background(), testHost, noop); err != nil {
+	if err := p.call(context.Background(), discard, testHost, noop); err != nil {
 		t.Fatalf("call() error = %v", err)
 	}
 	d.clients[0].close()
-	if err := p.call(context.Background(), testHost, noop); err != nil {
+	if err := p.call(context.Background(), discard, testHost, noop); err != nil {
 		t.Fatalf("second call() error = %v", err)
 	}
 	if d.dialCount() != 2 {
@@ -104,41 +110,48 @@ func TestHostPool_call_redialsAfterDisconnect(t *testing.T) {
 	}
 }
 
-func TestHostPool_call_transportErrorDropsConnection(t *testing.T) {
+// Only a transport error is evidence about the link. A daemon-reported
+// libvirt error, or a verdict fn reached locally, must not cost the
+// connection.
+func TestHostPool_call_dropsConnectionOnlyOnTransportError(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name        string
+		err         error
+		wantDropped bool
+	}{
+		{"transport error", io.ErrUnexpectedEOF, true},
+		{"libvirt error", lvErr(libvirt.ErrNoDomain), false},
+		{"locally classified error", fmt.Errorf("%w: creating iso writer: read-only file system", provider.ErrPermanent), false},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			d := &fakeDialer{}
 			p := newHostPool(d.dial)
-	err := p.call(context.Background(), testHost, func(libvirtAPI) error { return io.ErrUnexpectedEOF })
+			log, snapshot := captureLogger(1)
-	if !errors.Is(err, io.ErrUnexpectedEOF) {
+			err := p.call(context.Background(), log, testHost, func(libvirtAPI) error { return tc.err })
-		t.Fatalf("call() error = %v, want the transport error passed through", err)
+			if !errors.Is(err, tc.err) {
+				t.Fatalf("call() error = %v, want %v passed through", err, tc.err)
 			}
-	if !d.clients[0].closed.Load() {
+			if got := d.clients[0].closed.Load(); got != tc.wantDropped {
-		t.Error("connection not closed after a transport error")
+				t.Errorf("connection closed = %v, want %v", got, tc.wantDropped)
 			}
-	if err := p.call(context.Background(), testHost, noop); err != nil {
+			joined := strings.Join(snapshot(), "\n")
-		t.Fatalf("call() after transport error = %v", err)
+			if logged := strings.Contains(joined, `"msg"="libvirt connection dropped"`) && strings.Contains(joined, `"reason"="transport error"`); logged != tc.wantDropped {
+				t.Errorf("transport drop logged = %v, want %v:\n%s", logged, tc.wantDropped, joined)
 			}
-	if d.dialCount() != 2 {
+			if err := p.call(context.Background(), discard, testHost, noop); err != nil {
-		t.Errorf("dials = %d, want 2", d.dialCount())
+				t.Fatalf("next call() error = %v", err)
 			}
-}
+			wantDials := 1
-
+			if tc.wantDropped {
-func TestHostPool_call_libvirtErrorKeepsConnection(t *testing.T) {
+				wantDials = 2
-	t.Parallel()
-	d := &fakeDialer{}
-	p := newHostPool(d.dial)
-	err := p.call(context.Background(), testHost, func(libvirtAPI) error { return lvErr(libvirt.ErrNoDomain) })
-	if !hasCode(err, libvirt.ErrNoDomain) {
-		t.Fatalf("call() error = %v, want the libvirt error passed through", err)
 			}
-	if err := p.call(context.Background(), testHost, noop); err != nil {
+			if d.dialCount() != wantDials {
-		t.Fatalf("call() error = %v", err)
+				t.Errorf("dials = %d, want %d", d.dialCount(), wantDials)
 			}
-	if d.dialCount() != 1 {
+		})
-		t.Errorf("dials = %d, want 1: a daemon-reported error must not cost the connection", d.dialCount())
-	}
-	if d.clients[0].closed.Load() {
-		t.Error("connection closed after a libvirt error")
 	}
 }
 
@@ -146,7 +159,7 @@ func TestHostPool_call_dialErrorPropagates(t *testing.T) {
 	t.Parallel()
 	d := &fakeDialer{err: errors.New("ssh: handshake failed")}
 	p := newHostPool(d.dial)
-	err := p.call(context.Background(), testHost, noop)
+	err := p.call(context.Background(), discard, testHost, noop)
 	if err == nil || !strings.Contains(err.Error(), "nas1") || !strings.Contains(err.Error(), "handshake failed") {
 		t.Errorf("call() error = %v, want the dial error naming the host", err)
 	}
@@ -162,7 +175,7 @@ func TestHostPool_call_cancelAbandonsBlockedCall(t *testing.T) {
 		time.Sleep(20 * time.Millisecond)
 		cancel()
 	}()
-	err := p.call(ctx, testHost, func(libvirtAPI) error {
+	err := p.call(ctx, discard, testHost, func(libvirtAPI) error {
 		// Simulates an RPC stuck on a hung hypervisor: it only returns once
 		// the pool closes the connection out from under it.
 		<-d.clients[0].done
@@ -177,7 +190,7 @@ func TestHostPool_call_cancelAbandonsBlockedCall(t *testing.T) {
 	case <-time.After(time.Second):
 		t.Fatal("blocked call was not released by closing the connection")
 	}
-	if err := p.call(context.Background(), testHost, noop); err != nil {
+	if err := p.call(context.Background(), discard, testHost, noop); err != nil {
 		t.Fatalf("call() after cancel = %v", err)
 	}
 	if d.dialCount() != 2 {
@@ -192,7 +205,7 @@ func TestHostPool_call_concurrentCallersShareOneDial(t *testing.T) {
 	var wg sync.WaitGroup
 	for range 8 {
 		wg.Go(func() {
-			if err := p.call(context.Background(), testHost, noop); err != nil {
+			if err := p.call(context.Background(), discard, testHost, noop); err != nil {
 				t.Errorf("call() error = %v", err)
 			}
 		})
@@ -210,7 +223,7 @@ func TestHostPool_call_hostsAreIndependent(t *testing.T) {
 	other := testHost
 	other.Name = "lab2"
 	for _, h := range []provider.KVMHost{testHost, other, testHost, other} {
-		if err := p.call(context.Background(), h, noop); err != nil {
+		if err := p.call(context.Background(), discard, h, noop); err != nil {
 			t.Fatalf("call(%s) error = %v", h.Name, err)
 		}
 	}
@@ -382,13 +395,19 @@ func newSSHFixture(t *testing.T, answerGlobalRequests bool) *sshFixture {
 	}
 }
 
-func TestSSHAuth_dialSSH_forwardsUnixSocket(t *testing.T) {
+func mustSSHAuth(t *testing.T, cfg provider.KVMConfig) *sshAuth {
-	t.Parallel()
+	t.Helper()
-	f := newSSHFixture(t, true)
+	auth, err := newSSHAuth(cfg)
-	auth, err := newSSHAuth(f.cfg)
 	if err != nil {
 		t.Fatalf("newSSHAuth() error = %v", err)
 	}
+	return auth
+}
+
+func TestSSHAuth_dialSSH_forwardsUnixSocket(t *testing.T) {
+	t.Parallel()
+	f := newSSHFixture(t, true)
+	auth := mustSSHAuth(t, f.cfg)
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 	client, err := auth.dialSSH(ctx, f.host)
@@ -422,10 +441,7 @@ func TestSSHAuth_dialSSH_rejectsUnknownHostKey(t *testing.T) {
 	if err := os.WriteFile(f.cfg.KnownHostsFile, []byte(line+"\n"), 0o600); err != nil {
 		t.Fatal(err)
 	}
-	auth, err := newSSHAuth(f.cfg)
+	auth := mustSSHAuth(t, f.cfg)
-	if err != nil {
-		t.Fatalf("newSSHAuth() error = %v", err)
-	}
 	client, err := auth.dialSSH(context.Background(), f.host)
 	if err == nil {
 		client.Close()
@@ -442,11 +458,7 @@ func TestSSHAuth_dialSSH_insecureIgnoreHostKey(t *testing.T) {
 	f := newSSHFixture(t, true)
 	f.cfg.KnownHostsFile = ""
 	f.cfg.InsecureIgnoreHostKey = true
-	auth, err := newSSHAuth(f.cfg)
+	client, err := mustSSHAuth(t, f.cfg).dialSSH(context.Background(), f.host)
-	if err != nil {
-		t.Fatalf("newSSHAuth() error = %v", err)
-	}
-	client, err := auth.dialSSH(context.Background(), f.host)
 	if err != nil {
 		t.Fatalf("dialSSH() error = %v", err)
 	}
@@ -460,11 +472,7 @@ func TestSSHAuth_dialSSH_rejectsWrongClientKey(t *testing.T) {
 	if err := os.WriteFile(f.keyFile, otherPEM, 0o600); err != nil {
 		t.Fatal(err)
 	}
-	auth, err := newSSHAuth(f.cfg)
+	client, err := mustSSHAuth(t, f.cfg).dialSSH(context.Background(), f.host)
-	if err != nil {
-		t.Fatalf("newSSHAuth() error = %v", err)
-	}
-	client, err := auth.dialSSH(context.Background(), f.host)
 	if err == nil {
 		client.Close()
 		t.Fatal("dialSSH() succeeded with a key the server does not know")
@@ -477,13 +485,9 @@ func TestSSHAuth_dialSSH_rejectsWrongClientKey(t *testing.T) {
 func TestSSHAuth_dialSSH_cancelledContext(t *testing.T) {
 	t.Parallel()
 	f := newSSHFixture(t, true)
-	auth, err := newSSHAuth(f.cfg)
-	if err != nil {
-		t.Fatalf("newSSHAuth() error = %v", err)
-	}
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	if client, err := auth.dialSSH(ctx, f.host); err == nil {
+	if client, err := mustSSHAuth(t, f.cfg).dialSSH(ctx, f.host); err == nil {
 		client.Close()
 		t.Fatal("dialSSH() succeeded with an already-cancelled context")
 	}
@@ -519,14 +523,101 @@ func TestNewSSHAuth_missingFiles(t *testing.T) {
 	}
 }
 
+// The kvm-ssh Secret is an optional mount, so the provider has to be
+// constructible without it and has to start working once it appears — the
+// alternative was a constructor error that crash-looped the whole manager.
+func TestLazySSH_load_healsWhenTheKeyAppears(t *testing.T) {
+	t.Parallel()
+	f := newSSHFixture(t, true)
+	keyPEM, err := os.ReadFile(f.keyFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Remove(f.keyFile); err != nil {
+		t.Fatal(err)
+	}
+	auth := newLazySSH(f.cfg).load
+
+	if _, err := auth(); err == nil {
+		t.Fatal("loading with no key file succeeded, want an error")
+	} else if class := classify(err); class != provider.ErrTransient {
+		t.Errorf("class = %v, want ErrTransient so the proxy retries once the Secret lands", class)
+	}
+	if err := os.WriteFile(f.keyFile, keyPEM, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	loaded, err := auth()
+	if err != nil {
+		t.Fatalf("loading after the key appeared: %v", err)
+	}
+	// Cached from here on: the file may vanish again (a Secret rotated out)
+	// without breaking connections that already work.
+	if err := os.Remove(f.keyFile); err != nil {
+		t.Fatal(err)
+	}
+	if again, err := auth(); err != nil || again != loaded {
+		t.Errorf("auth() = %v, %v; want the cached value reused", again, err)
+	}
+}
+
+func TestLazySSH_load_unparseableKeyIsPermanent(t *testing.T) {
+	t.Parallel()
+	f := newSSHFixture(t, true)
+	if err := os.WriteFile(f.keyFile, []byte("not a key"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	_, err := newLazySSH(f.cfg).load()
+	if classify(err) != provider.ErrPermanent {
+		t.Errorf("class = %v, want ErrPermanent: retrying cannot fix a key that does not parse", classify(err))
+	}
+}
+
+// The dialer must authenticate with the material the lazy loader produced,
+// not a stale or empty one. The echo socket is not a libvirt server, so the
+// call gets as far as the libvirt handshake and no further; what matters is
+// that it is past SSH by then.
+func TestLazySSH_dial_authenticatesOverSSH(t *testing.T) {
+	t.Parallel()
+	f := newSSHFixture(t, true)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	_, err := newLazySSH(f.cfg).dial(ctx, f.host)
+	if err == nil {
+		t.Fatal("dial to an echo socket succeeded, want a libvirt handshake error")
+	}
+	if strings.Contains(err.Error(), "sshKeyFile") || strings.Contains(err.Error(), "ssh handshake") {
+		t.Errorf("dial failed before libvirt: %v", err)
+	}
+	if paths := f.server.requestedPaths(); len(paths) != 1 || paths[0] != f.host.Socket {
+		t.Errorf("server saw forward requests %v, want [%s]", paths, f.host.Socket)
+	}
+}
+
+// New must not fail on missing key material, or one misconfigured provider
+// aborts registry.Build and the manager exits before any controller starts.
+func TestNew_missingSSHMaterialStillBuilds(t *testing.T) {
+	t.Parallel()
+	p, err := New(context.Background(), provider.ProviderConfig{
+		Name: "kvm-home", Type: "kvm",
+		KVM: &provider.KVMConfig{
+			SSHKeyFile:     filepath.Join(t.TempDir(), "absent"),
+			KnownHostsFile: filepath.Join(t.TempDir(), "absent"),
+			Hosts:          []provider.KVMHost{{Name: "nas1", Address: "nas1:22", User: "virt", Bridge: "br0"}},
+		},
+	})
+	if err != nil || p == nil {
+		t.Fatalf("New() = %v, %v; want a usable provider", p, err)
+	}
+	if _, err := p.Get(context.Background(), "hosts/nas1/domains/proxy-x"); provider.Class(err) != provider.ErrTransient {
+		t.Errorf("Get class = %v, want ErrTransient", provider.Class(err))
+	}
+}
+
 func TestKeepalive_closesClientWhenServerStalls(t *testing.T) {
 	t.Parallel()
 	f := newSSHFixture(t, false)
-	auth, err := newSSHAuth(f.cfg)
+	client, err := mustSSHAuth(t, f.cfg).dialSSH(context.Background(), f.host)
-	if err != nil {
-		t.Fatalf("newSSHAuth() error = %v", err)
-	}
-	client, err := auth.dialSSH(context.Background(), f.host)
 	if err != nil {
 		t.Fatalf("dialSSH() error = %v", err)
 	}
@@ -547,11 +638,7 @@ func TestKeepalive_closesClientWhenServerStalls(t *testing.T) {
 func TestKeepalive_keepsHealthyClientOpen(t *testing.T) {
 	t.Parallel()
 	f := newSSHFixture(t, true)
-	auth, err := newSSHAuth(f.cfg)
+	client, err := mustSSHAuth(t, f.cfg).dialSSH(context.Background(), f.host)
-	if err != nil {
-		t.Fatalf("newSSHAuth() error = %v", err)
-	}
-	client, err := auth.dialSSH(context.Background(), f.host)
 	if err != nil {
 		t.Fatalf("dialSSH() error = %v", err)
 	}
@@ -567,3 +654,69 @@ func TestKeepalive_keepsHealthyClientOpen(t *testing.T) {
 	}
 	conn.Close()
 }
+
+func TestHostPool_call_tracesAtV1(t *testing.T) {
+	t.Parallel()
+	d := &fakeDialer{api: readyHost()}
+	p := newHostPool(d.dial)
+	log, snapshot := captureLogger(1)
+	err := p.call(context.Background(), log, testHost, func(api libvirtAPI) error {
+		_, err := api.StoragePoolLookupByName("default")
+		return err
+	})
+	if err != nil {
+		t.Fatalf("call() error = %v", err)
+	}
+	joined := strings.Join(snapshot(), "\n")
+	for _, want := range []string{`"msg"="libvirt host connected"`, `"address"="nas1:22"`, `"rpc"="virStoragePoolLookupByName"`, `"pool"="default"`, "libvirt "} {
+		if !strings.Contains(joined, want) {
+			t.Errorf("output missing %s:\n%s", want, joined)
+		}
+	}
+}
+
+func TestHostPool_call_noTracingBelowV1(t *testing.T) {
+	t.Parallel()
+	inner := readyHost()
+	d := &fakeDialer{api: inner}
+	p := newHostPool(d.dial)
+	log, snapshot := captureLogger(0)
+	err := p.call(context.Background(), log, testHost, func(api libvirtAPI) error {
+		if api != libvirtAPI(inner) {
+			t.Errorf("fn received %T, want the bare fake when V(1) is off", api)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("call() error = %v", err)
+	}
+	if lines := snapshot(); len(lines) != 0 {
+		t.Errorf("verbosity 0 logged %d lines:\n%s", len(lines), strings.Join(lines, "\n"))
+	}
+}
+
+func TestHostPool_call_logsCancelDropReason(t *testing.T) {
+	t.Parallel()
+	d := &fakeDialer{}
+	p := newHostPool(d.dial)
+	log, snapshot := captureLogger(1)
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	released := make(chan struct{})
+	err := p.call(ctx, log, testHost, func(libvirtAPI) error {
+		<-d.clients[0].done
+		close(released)
+		return io.EOF
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("call() error = %v, want context.Canceled", err)
+	}
+	<-released
+	joined := strings.Join(snapshot(), "\n")
+	if !strings.Contains(joined, `"reason"="context cancelled"`) {
+		t.Errorf("output missing the cancel drop line:\n%s", joined)
+	}
+	if strings.Contains(joined, `"reason"="transport error"`) {
+		t.Errorf("cancel was also reported as a transport error:\n%s", joined)
+	}
+}

internal/provider/kvm/domainxml.go

@@ -9,35 +9,54 @@ import (
 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.
+// config and placement so the builder stays a pure function. DiskPath and
+// 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
-	Pool      string
+	Firmware  string
 	Bridge    string
-	DiskVol   string
+	DiskPath  string
-	SeedVol   string
+	SeedPath  string
 }
 
 // buildDomainXML renders a q35 KVM guest: host-passthrough CPU, a virtio
-// overlay disk and a SATA cdrom for the seed (both referenced as pool
+// overlay disk and a SATA cdrom for the seed, one virtio NIC on the host
-// volumes so no host paths are needed), one virtio NIC on the host bridge,
+// bridge, a serial console, the guest-agent channel, a virtio RNG, no
-// a serial console, the guest-agent channel, a virtio RNG, no graphics, and
+// graphics, and the operator's metadata element.
-// the operator's metadata element.
+//
+// Disks are type="file" with host paths, not type="volume" with pool/volume
+// names: libvirt's AppArmor helper (virt-aa-helper) only grants QEMU access
+// 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 <os firmware="efi"> with secure boot off, and libvirt
+// picks the OVMF binary and creates the NVRAM file itself (removed again by
+// the nvram undefine flag). Under SeaBIOS, Debian 13's GRUB hangs on a
+// headless domain with a serial console (Debian bug #1111240:
+// "terminal_output gfxterm serial" with no video device); the same image
+// boots under UEFI with the serial console intact.
 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{
+		OS:         os,
-			Type: osTypeXML{Arch: "x86_64", Machine: "q35", Value: "hvm"},
-			Boot: bootXML{Dev: "hd"},
-		},
 		Features:   featuresXML{ACPI: &struct{}{}, APIC: &struct{}{}},
 		CPU:        cpuXML{Mode: "host-passthrough"},
 		Clock:      clockXML{Offset: "utc"},
@@ -47,17 +66,17 @@ func buildDomainXML(p domainParams) (string, error) {
 		Devices: devicesXML{
 			Disks: []diskXML{
 				{
-					Type:   "volume",
+					Type:   "file",
 					Device: "disk",
 					Driver: driverXML{Name: "qemu", Type: formatQcow2},
-					Source: diskSourceXML{Pool: p.Pool, Volume: p.DiskVol},
+					Source: diskSourceXML{File: p.DiskPath},
 					Target: targetXML{Dev: "vda", Bus: "virtio"},
 				},
 				{
-					Type:     "volume",
+					Type:     "file",
 					Device:   "cdrom",
 					Driver:   driverXML{Name: "qemu", Type: formatRaw},
-					Source:   diskSourceXML{Pool: p.Pool, Volume: p.SeedVol},
+					Source:   diskSourceXML{File: p.SeedPath},
 					Target:   targetXML{Dev: "sda", Bus: "sata"},
 					ReadOnly: &struct{}{},
 				},
@@ -110,10 +129,21 @@ type sizeXML struct {
 }
 
 type osXML struct {
+	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 {
 	Arch    string `xml:"arch,attr"`
 	Machine string `xml:"machine,attr"`
@@ -161,8 +191,7 @@ type driverXML struct {
 }
 
 type diskSourceXML struct {
-	Pool   string `xml:"pool,attr"`
+	File string `xml:"file,attr"`
-	Volume string `xml:"volume,attr"`
 }
 
 type targetXML struct {

internal/provider/kvm/domainxml_test.go

@@ -14,10 +14,10 @@ func testDomainParams() domainParams {
 		CreatedAt: time.Date(2026, 9, 4, 12, 0, 0, 0, time.UTC),
 		VCPUs:     2,
 		MemoryMiB: 2048,
-		Pool:      "default",
+		Firmware:  firmwareUEFI,
 		Bridge:    "br0",
-		DiskVol:   "proxy-abc123def456ghij.qcow2",
+		DiskPath:  "/var/lib/libvirt/images/proxy-abc123def456ghij.qcow2",
-		SeedVol:   "proxy-abc123def456ghij-seed.iso",
+		SeedPath:  "/var/lib/libvirt/images/proxy-abc123def456ghij-seed.iso",
 	}
 }
 
@@ -41,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)
 	}
@@ -63,16 +67,19 @@ func TestBuildDomainXML_fieldByField(t *testing.T) {
 		t.Fatalf("disks = %d, want 2", len(d.Devices.Disks))
 	}
 	disk, seed := d.Devices.Disks[0], d.Devices.Disks[1]
-	if disk.Type != "volume" || disk.Device != "disk" || disk.Driver.Type != "qcow2" ||
+	if disk.Type != "file" || disk.Device != "disk" || disk.Driver.Type != "qcow2" ||
-		disk.Source.Pool != "default" || disk.Source.Volume != "proxy-abc123def456ghij.qcow2" ||
+		disk.Source.File != "/var/lib/libvirt/images/proxy-abc123def456ghij.qcow2" ||
 		disk.Target.Dev != "vda" || disk.Target.Bus != "virtio" || disk.ReadOnly != nil {
 		t.Errorf("overlay disk = %+v", disk)
 	}
-	if seed.Type != "volume" || seed.Device != "cdrom" || seed.Driver.Type != "raw" ||
+	if seed.Type != "file" || seed.Device != "cdrom" || seed.Driver.Type != "raw" ||
-		seed.Source.Pool != "default" || seed.Source.Volume != "proxy-abc123def456ghij-seed.iso" ||
+		seed.Source.File != "/var/lib/libvirt/images/proxy-abc123def456ghij-seed.iso" ||
 		seed.Target.Bus != "sata" || seed.ReadOnly == nil {
 		t.Errorf("seed cdrom = %+v", seed)
 	}
+	if strings.Contains(raw, `type="volume"`) || strings.Contains(raw, " pool=") {
+		t.Errorf("disks reference pool volumes; virt-aa-helper cannot grant QEMU access to those:\n%s", raw)
+	}
 
 	if len(d.Devices.Interfaces) != 1 {
 		t.Fatalf("interfaces = %d, want 1", len(d.Devices.Interfaces))
@@ -111,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)
+	}
+}

internal/provider/kvm/errors.go

@@ -19,10 +19,8 @@ import (
 // function to recognise. libvirt exposes no quota signal, so nothing maps
 // to QuotaExceeded.
 func classify(err error) error {
-	for _, class := range []error{provider.ErrNotFound, provider.ErrQuotaExceeded, provider.ErrPermanent, provider.ErrTransient} {
+	if isClassified(err) {
-		if errors.Is(err, class) {
+		return provider.Class(err)
-			return class
-		}
 	}
 	var lerr libvirt.Error
 	if !errors.As(err, &lerr) {
@@ -51,6 +49,18 @@ func isLibvirtError(err error) bool {
 	return errors.As(err, &lerr)
 }
 
+// isClassified reports whether err already carries one of the provider's
+// sentinels, i.e. the package reached a verdict itself rather than hitting
+// a transport failure.
+func isClassified(err error) bool {
+	for _, class := range []error{provider.ErrNotFound, provider.ErrQuotaExceeded, provider.ErrPermanent, provider.ErrTransient} {
+		if errors.Is(err, class) {
+			return true
+		}
+	}
+	return false
+}
+
 func hasCode(err error, code libvirt.ErrorNumber) bool {
 	var lerr libvirt.Error
 	return errors.As(err, &lerr) && libvirt.ErrorNumber(lerr.Code) == code
@@ -63,10 +73,18 @@ func logAPIError(log logr.Logger, op string, err error) {
 	if !log.V(1).Enabled() {
 		return
 	}
-	kv := []any{"op", op, "error", err.Error()}
+	kv := append([]any{"op", op, "error", err.Error()}, libvirtErrorFields(err)...)
-	var lerr libvirt.Error
-	if errors.As(err, &lerr) {
-		kv = append(kv, "libvirtCode", lerr.Code, "libvirtMessage", lerr.Message)
-	}
 	log.V(1).Info("libvirt call failed", kv...)
 }
+
+// libvirtErrorFields renders a daemon-reported error as log key/values:
+// its constant name (ErrNoDomain, ...) and numeric code. The daemon's
+// message is already the error text. Nil for transport and other
+// non-libvirt errors.
+func libvirtErrorFields(err error) []any {
+	var lerr libvirt.Error
+	if !errors.As(err, &lerr) {
+		return nil
+	}
+	return []any{"libvirtError", libvirt.ErrorNumber(lerr.Code).String(), "libvirtCode", lerr.Code}
+}

internal/provider/kvm/errors_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"reflect"
 	"testing"
 
 	libvirt "github.com/digitalocean/go-libvirt"
@@ -67,3 +68,26 @@ func TestHasCode(t *testing.T) {
 		t.Error("io.EOF classified as a libvirt error")
 	}
 }
+
+func TestLibvirtErrorFields(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name string
+		err  error
+		want []any
+	}{
+		{"libvirt error", lvErr(libvirt.ErrNoDomain), []any{"libvirtError", "ErrNoDomain", "libvirtCode", uint32(42)}},
+		{"wrapped libvirt error", fmt.Errorf("looking up: %w", lvErr(libvirt.ErrNoDomain)), []any{"libvirtError", "ErrNoDomain", "libvirtCode", uint32(42)}},
+		{"transport error", io.EOF, nil},
+		{"cancelled", context.Canceled, nil},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got := libvirtErrorFields(tc.err)
+			if !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("libvirtErrorFields() = %#v, want %#v", got, tc.want)
+			}
+		})
+	}
+}

internal/provider/kvm/fake_test.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"slices"
+	"strings"
 	"sync"
 
 	libvirt "github.com/digitalocean/go-libvirt"
@@ -30,7 +31,6 @@ type fakeAPI struct {
 }
 
 type fakeVol struct {
-	name     string
 	format   string
 	capacity int64
 	backing  string
@@ -93,7 +93,7 @@ func (f *fakeAPI) vol(pool, name string) *fakeVol {
 func (f *fakeAPI) putVol(pool, name, format string, data []byte) {
 	f.mu.Lock()
 	defer f.mu.Unlock()
-	f.pools[pool][name] = &fakeVol{name: name, format: format, capacity: int64(len(data)), data: data}
+	f.pools[pool][name] = &fakeVol{format: format, capacity: int64(len(data)), data: data}
 }
 
 func (f *fakeAPI) volNames(pool string) []string {
@@ -151,8 +151,11 @@ func (f *fakeAPI) DomainDefineXML(raw string) (libvirt.Domain, error) {
 	if err != nil {
 		return libvirt.Domain{}, lvError(libvirt.ErrXMLError, "%v", err)
 	}
+	// libvirt hands the element back without its namespace declaration
+	// (seen on 11.6); mimic that so Get/ListByTag parse the real shape.
+	stored := strings.Replace(string(meta), ` xmlns="`+metadataURI+`"`, "", 1)
 	existing := f.domains[d.Name]
-	nd := &fakeDomain{name: d.Name, xml: raw, metadata: string(meta)}
+	nd := &fakeDomain{name: d.Name, xml: raw, metadata: stored}
 	if existing != nil {
 		nd.running = existing.running
 		nd.addrs = existing.addrs
@@ -344,7 +347,7 @@ func (f *fakeAPI) StorageVolCreateXML(pool libvirt.StoragePool, raw string, _ li
 	if _, exists := vols[v.Name]; exists {
 		return libvirt.StorageVol{}, lvError(libvirt.ErrStorageVolExist, "storage volume '%s' exists already", v.Name)
 	}
-	fv := &fakeVol{name: v.Name, format: v.Target.Format.Type, capacity: v.Capacity.Value}
+	fv := &fakeVol{format: v.Target.Format.Type, capacity: v.Capacity.Value}
 	if v.BackingStore != nil {
 		fv.backing = v.BackingStore.Path
 	}

internal/provider/kvm/image.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"strings"
 	"sync"
 	"time"
@@ -23,9 +22,7 @@ import (
 // provider's images map (Key set) or a URL given directly.
 type imageSource struct {
 	Key string
-	URL    string
+	provider.KVMImage
-	SHA256 string
-	Format string
 }
 
 // resolveImage maps placement.image to a source. Anything that is neither a
@@ -33,11 +30,10 @@ 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, KVMImage: img}, nil
 	}
-	u, err := url.Parse(image)
+	if provider.IsHTTPURL(image) {
-	if err == nil && (u.Scheme == "http" || u.Scheme == "https") && u.Host != "" {
+		return imageSource{KVMImage: imageDefaults(provider.KVMImage{URL: image})}, nil
-		return imageSource{URL: image, Format: formatQcow2}, nil
 	}
 	return imageSource{}, fmt.Errorf("%w: placement.image %q is neither a key in kvm.images nor an http(s) URL",
 		provider.ErrPermanent, image)
@@ -101,7 +97,7 @@ func (c *imageCache) ensure(ctx context.Context, log logr.Logger, api libvirtAPI
 		return "", err
 	}
 	if ready {
-		log.V(1).Info("base image already cached", "host", hostName, "volume", src.volumeName())
+		log.V(1).Info("base image already cached", "volume", src.volumeName())
 		return api.StorageVolGetPath(base)
 	}
 	if err := deleteIfExists(api, pool, src.markerName()); err != nil {
@@ -170,7 +166,7 @@ func (c *imageCache) fetch(ctx context.Context, log logr.Logger, api libvirtAPI,
 	}
 
 	log.Info("fetching base image into host pool",
-		"host", hostName, "pool", pool.Name, "volume", src.volumeName(),
+		"pool", pool.Name, "volume", src.volumeName(),
 		"url", src.URL, "bytes", resp.ContentLength)
 	start := time.Now()
 
@@ -212,7 +208,7 @@ func (c *imageCache) fetch(ctx context.Context, log logr.Logger, api libvirtAPI,
 		return libvirt.StorageVol{}, fmt.Errorf("writing image marker: %w", err)
 	}
 	log.Info("base image cached",
-		"host", hostName, "volume", src.volumeName(), "sha256", digest,
+		"volume", src.volumeName(), "sha256", digest,
 		"bytes", resp.ContentLength, "duration", time.Since(start).Round(time.Second))
 	return base, nil
 }

internal/provider/kvm/image_test.go

@@ -37,12 +37,11 @@ type imageServer struct {
 	status   int
 	noLength bool
 	delay    time.Duration
-	body     []byte
 }
 
 func newImageServer(t *testing.T) *imageServer {
 	t.Helper()
-	s := &imageServer{status: http.StatusOK, body: testImage}
+	s := &imageServer{status: http.StatusOK}
 	s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
 		s.hits.Add(1)
 		time.Sleep(s.delay)
@@ -55,16 +54,16 @@ func newImageServer(t *testing.T) *imageServer {
 		if s.noLength {
 			w.(http.Flusher).Flush()
 		} else {
-			w.Header().Set("Content-Length", strconv.Itoa(len(s.body)))
+			w.Header().Set("Content-Length", strconv.Itoa(len(testImage)))
 		}
-		_, _ = w.Write(s.body)
+		_, _ = w.Write(testImage)
 	}))
 	t.Cleanup(s.Close)
 	return s
 }
 
 func testSource(srv *imageServer, sha string) imageSource {
-	return imageSource{Key: "debian-13", URL: srv.URL + "/debian.qcow2", SHA256: sha, Format: formatQcow2}
+	return imageSource{Key: "debian-13", KVMImage: provider.KVMImage{URL: srv.URL + "/debian.qcow2", SHA256: sha, Format: formatQcow2}}
 }
 
 func ensureTest(t *testing.T, api *fakeAPI, src imageSource) (string, error) {
@@ -88,9 +87,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},
+		{"configured key", "debian-13", imageSource{Key: "debian-13", KVMImage: provider.KVMImage{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"}, false},
+		{"direct https url", "https://x.example/img.qcow2", imageSource{KVMImage: provider.KVMImage{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"}, false},
+		{"direct http url", "http://mirror/img.qcow2", imageSource{KVMImage: provider.KVMImage{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},
@@ -100,9 +99,7 @@ func TestResolveImage(t *testing.T) {
 			t.Parallel()
 			got, err := resolveImage(cfg, tc.image)
 			if tc.wantErr {
-				if !errors.Is(err, provider.ErrPermanent) {
+				wantClass(t, err, provider.ErrPermanent)
-					t.Fatalf("resolveImage(%q) error = %v, want ErrPermanent", tc.image, err)
-				}
 				return
 			}
 			if err != nil {
@@ -117,16 +114,16 @@ func TestResolveImage(t *testing.T) {
 
 func TestImageSource_volumeNames(t *testing.T) {
 	t.Parallel()
-	keyed := imageSource{Key: "debian-13", URL: "https://a/b.qcow2", Format: "qcow2"}
+	keyed := imageSource{Key: "debian-13", KVMImage: provider.KVMImage{URL: "https://a/b.qcow2", Format: "qcow2"}}
 	if keyed.volumeName() != "base-debian-13.qcow2" || keyed.markerName() != "base-debian-13.qcow2.sha256" {
 		t.Errorf("keyed names = %s / %s", keyed.volumeName(), keyed.markerName())
 	}
-	direct := imageSource{URL: "https://a/b.qcow2", Format: "qcow2"}
+	direct := imageSource{KVMImage: provider.KVMImage{URL: "https://a/b.qcow2", Format: "qcow2"}}
 	name := direct.volumeName()
 	if !strings.HasPrefix(name, "base-") || !strings.HasSuffix(name, ".qcow2") || len(name) != len("base-")+16+len(".qcow2") {
 		t.Errorf("direct-url name = %q, want base-<16 hex>.qcow2", name)
 	}
-	other := imageSource{URL: "https://a/c.qcow2", Format: "qcow2"}
+	other := imageSource{KVMImage: provider.KVMImage{URL: "https://a/c.qcow2", Format: "qcow2"}}
 	if other.volumeName() == name {
 		t.Error("different URLs produced the same volume name")
 	}
@@ -181,12 +178,27 @@ func TestImageCache_ensure_noChecksumConfigured(t *testing.T) {
 	}
 }
 
-func TestImageCache_ensure_alreadyCached(t *testing.T) {
+func TestImageCache_ensure_existingVolumes(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		base     []byte
+		marker   []byte
+		wantHits int32
+	}{
+		{"already cached", testImage, []byte(testImageSHA()), 0},
+		{"marker missing is a partial upload", []byte("half an image"), nil, 1},
+		{"configured checksum changed", []byte("an older release"), []byte(strings.Repeat("0", 64)), 1},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			srv := newImageServer(t)
 			api := newFakeAPI("default")
-	api.putVol("default", "base-debian-13.qcow2", "qcow2", testImage)
+			api.putVol("default", "base-debian-13.qcow2", "qcow2", tc.base)
-	api.putVol("default", "base-debian-13.qcow2.sha256", "raw", []byte(testImageSHA()))
+			if tc.marker != nil {
+				api.putVol("default", "base-debian-13.qcow2.sha256", "raw", tc.marker)
+			}
 			path, err := ensureTest(t, api, testSource(srv, testImageSHA()))
 			if err != nil {
 				t.Fatalf("ensure() error = %v", err)
@@ -194,44 +206,19 @@ func TestImageCache_ensure_alreadyCached(t *testing.T) {
 			if path != "/var/lib/libvirt/images/base-debian-13.qcow2" {
 				t.Errorf("path = %q", path)
 			}
-	if srv.hits.Load() != 0 {
+			if srv.hits.Load() != tc.wantHits {
-		t.Errorf("http hits = %d, want 0 for a cached image", srv.hits.Load())
+				t.Errorf("http hits = %d, want %d", srv.hits.Load(), tc.wantHits)
 			}
-	if api.hasCall("StorageVolCreateXML") || api.hasCall("StorageVolDelete") {
+			if touched := api.hasCall("StorageVolCreateXML") || api.hasCall("StorageVolDelete"); touched != (tc.wantHits > 0) {
-		t.Errorf("cached image was touched: calls = %v", api.calls)
+				t.Errorf("volumes touched = %v, want %v: calls = %v", touched, tc.wantHits > 0, api.calls)
-	}
-}
-
-func TestImageCache_ensure_markerMissingRedownloads(t *testing.T) {
-	t.Parallel()
-	srv := newImageServer(t)
-	api := newFakeAPI("default")
-	api.putVol("default", "base-debian-13.qcow2", "qcow2", []byte("half an image"))
-	if _, err := ensureTest(t, api, testSource(srv, testImageSHA())); err != nil {
-		t.Fatalf("ensure() error = %v", err)
-	}
-	if srv.hits.Load() != 1 {
-		t.Errorf("http hits = %d, want 1: a base without its marker is a partial upload", srv.hits.Load())
 			}
 			if base := api.vol("default", "base-debian-13.qcow2"); base == nil || !bytes.Equal(base.data, testImage) {
-		t.Error("partial base volume was not replaced")
+				t.Error("base volume does not hold the verified image")
-	}
-}
-
-func TestImageCache_ensure_checksumChangedRedownloads(t *testing.T) {
-	t.Parallel()
-	srv := newImageServer(t)
-	api := newFakeAPI("default")
-	api.putVol("default", "base-debian-13.qcow2", "qcow2", []byte("an older release"))
-	api.putVol("default", "base-debian-13.qcow2.sha256", "raw", []byte(strings.Repeat("0", 64)))
-	if _, err := ensureTest(t, api, testSource(srv, testImageSHA())); err != nil {
-		t.Fatalf("ensure() error = %v", err)
-	}
-	if srv.hits.Load() != 1 {
-		t.Errorf("http hits = %d, want 1 after the configured checksum changed", srv.hits.Load())
 			}
 			if marker := api.vol("default", "base-debian-13.qcow2.sha256"); marker == nil || string(marker.data) != testImageSHA() {
-		t.Errorf("marker not updated: %+v", marker)
+				t.Errorf("marker = %+v, want the verified digest", marker)
+			}
+		})
 	}
 }
 
@@ -240,9 +227,7 @@ func TestImageCache_ensure_checksumMismatchIsPermanent(t *testing.T) {
 	srv := newImageServer(t)
 	api := newFakeAPI("default")
 	_, err := ensureTest(t, api, testSource(srv, strings.Repeat("a", 64)))
-	if !errors.Is(err, provider.ErrPermanent) {
+	wantClass(t, err, provider.ErrPermanent)
-		t.Fatalf("ensure() error = %v, want ErrPermanent", err)
-	}
 	if names := api.volNames("default"); len(names) != 0 {
 		t.Errorf("volumes left behind after a checksum mismatch: %v", names)
 	}
@@ -283,8 +268,9 @@ func TestImageCache_ensure_noContentLengthIsPermanent(t *testing.T) {
 	srv.noLength = true
 	api := newFakeAPI("default")
 	_, err := ensureTest(t, api, testSource(srv, ""))
-	if !errors.Is(err, provider.ErrPermanent) || !strings.Contains(err.Error(), "Content-Length") {
+	wantClass(t, err, provider.ErrPermanent)
-		t.Fatalf("ensure() error = %v, want ErrPermanent about Content-Length", err)
+	if !strings.Contains(err.Error(), "Content-Length") {
+		t.Errorf("ensure() error = %v, want one about Content-Length", err)
 	}
 }
 

internal/provider/kvm/kvm.go

@@ -26,8 +26,10 @@ const (
 	defaultDiskSizeGB = 10
 	defaultSocket     = "/var/run/libvirt/libvirt-sock"
 	defaultPool       = "default"
-	formatQcow2       = "qcow2"
+	formatQcow2       = provider.KVMFormatQcow2
-	formatRaw         = "raw"
+	formatRaw         = provider.KVMFormatRaw
+	firmwareUEFI      = provider.KVMFirmwareUEFI
+	firmwareBIOS      = provider.KVMFirmwareBIOS
 
 	// undefineFlags removes everything libvirt may keep alongside a domain
 	// definition, so an undefine never fails on leftover state.
@@ -40,15 +42,9 @@ func withDefaults(cfg provider.KVMConfig) provider.KVMConfig {
 	if cfg.DiskSizeGB == 0 {
 		cfg.DiskSizeGB = defaultDiskSizeGB
 	}
-	if cfg.VendorData == "" {
-		cfg.VendorData = defaultVendorData
-	}
 	images := make(map[string]provider.KVMImage, len(cfg.Images))
 	for k, img := range cfg.Images {
-		if img.Format == "" {
+		images[k] = imageDefaults(img)
-			img.Format = formatQcow2
-		}
-		images[k] = img
 	}
 	cfg.Images = images
 	hosts := make([]provider.KVMHost, len(cfg.Hosts))
@@ -74,30 +70,41 @@ type Provider struct {
 	now    func() time.Time
 }
 
+func imageDefaults(img provider.KVMImage) provider.KVMImage {
+	if img.Format == "" {
+		img.Format = formatQcow2
+	}
+	if img.Firmware == "" {
+		img.Firmware = firmwareUEFI
+	}
+	return img
+}
+
 // New builds a Provider that reaches its hosts over SSH. The key and
-// known_hosts files are read here so a bad path fails at startup rather
+// known_hosts files are tried once here and kept when usable, but a failure
-// than on the first Create. Satisfies registry.Constructor.
+// is only a warning, retried on the first connection: they come from an
-func New(_ context.Context, pc provider.ProviderConfig) (provider.Provider, error) {
+// optional Secret mount, and a constructor that failed without it would
+// abort the whole provider set and crash-loop the manager, taking the
+// kubernetes and gcp providers down with it. Satisfies registry.Constructor.
+func New(ctx context.Context, pc provider.ProviderConfig) (provider.Provider, error) {
 	cfg := provider.KVMConfig{}
 	if pc.KVM != nil {
 		cfg = *pc.KVM
 	}
-	auth, err := newSSHAuth(cfg)
+	loader := newLazySSH(cfg)
-	if err != nil {
+	if _, err := loader.load(); err != nil {
-		return nil, fmt.Errorf("kvm provider %q: %w", pc.Name, err)
+		logf.FromContext(ctx).WithName("kvm").WithValues("provider", pc.Name).Info(
+			"WARNING: kvm ssh material is not usable; proxies on this provider will fail until it is",
+			"error", err.Error())
 	}
-	return newWithDialer(pc, auth.dialer(), nil), nil
+	return newWithDialer(pc.Name, cfg, loader.dial, nil), nil
 }
 
 // newWithDialer is the test seam: the dialer decides what libvirtAPI each
 // host resolves to, and the HTTP client is what image downloads go through.
-func newWithDialer(pc provider.ProviderConfig, dial hostDialer, httpClient *http.Client) *Provider {
+func newWithDialer(name string, cfg provider.KVMConfig, dial hostDialer, httpClient *http.Client) *Provider {
-	cfg := provider.KVMConfig{}
-	if pc.KVM != nil {
-		cfg = *pc.KVM
-	}
 	return &Provider{
-		name:   pc.Name,
+		name:   name,
 		cfg:    withDefaults(cfg),
 		hosts:  newHostPool(dial),
 		images: newImageCache(httpClient),
@@ -166,7 +173,7 @@ func (p *Provider) Create(ctx context.Context, req provider.CreateRequest) (stri
 		"vcpus", mt.VCPUs, "memoryMiB", mt.MemoryMiB, "diskSizeGb", p.cfg.DiskSizeGB,
 		"image", src.volumeName(), "cloudInitBytes", len(req.CloudInit))
 
-	err = p.hosts.call(ctx, host, func(api libvirtAPI) error {
+	err = p.hosts.call(ctx, log, host, func(api libvirtAPI) error {
 		dom, err := api.DomainLookupByName(req.Name)
 		if err == nil {
 			log.V(1).Info("domain already defined")
@@ -183,13 +190,23 @@ func (p *Provider) Create(ctx context.Context, req provider.CreateRequest) (stri
 		if err != nil {
 			return err
 		}
-		if err := p.ensureOverlay(log, api, pool, req.Name, basePath, src.Format); err != nil {
+		overlay, err := p.ensureOverlay(log, api, pool, req.Name, basePath, src.Format)
+		if err != nil {
 			return err
 		}
-		if err := p.writeSeed(log, api, pool, req.Name, req.CloudInit); err != nil {
+		seed, err := p.writeSeed(log, api, pool, req.Name, req.CloudInit)
+		if err != nil {
 			return err
 		}
-		dom, err = p.defineDomain(log, api, host, req, mt)
+		diskPath, err := api.StorageVolGetPath(overlay)
+		if err != nil {
+			return fmt.Errorf("resolving overlay path: %w", err)
+		}
+		seedPath, err := api.StorageVolGetPath(seed)
+		if err != nil {
+			return fmt.Errorf("resolving seed path: %w", err)
+		}
+		dom, err = p.defineDomain(log, api, host, req, mt, src.Firmware, diskPath, seedPath)
 		if err != nil {
 			return err
 		}
@@ -203,60 +220,71 @@ func (p *Provider) Create(ctx context.Context, req provider.CreateRequest) (stri
 	return id, nil
 }
 
-func (p *Provider) ensureOverlay(log logr.Logger, api libvirtAPI, pool libvirt.StoragePool, name, basePath, baseFormat string) error {
+// ensureOverlay creates the per-VM overlay volume, or returns the existing
+// one: creation is atomic, so an overlay that exists is complete.
+func (p *Provider) ensureOverlay(log logr.Logger, api libvirtAPI, pool libvirt.StoragePool, name, basePath, baseFormat string) (libvirt.StorageVol, error) {
 	xml, err := buildVolumeXML(overlayVolume(name, p.cfg.DiskSizeGB, basePath, baseFormat))
 	if err != nil {
-		return err
+		return libvirt.StorageVol{}, err
 	}
-	_, err = api.StorageVolCreateXML(pool, xml, 0)
+	vol, err := api.StorageVolCreateXML(pool, xml, 0)
 	switch {
 	case err == nil:
 		log.V(1).Info("overlay volume created", "volume", overlayVolumeName(name), "base", basePath)
+		return vol, nil
 	case hasCode(err, libvirt.ErrStorageVolExist):
 		log.V(1).Info("overlay volume already exists", "volume", overlayVolumeName(name))
+		vol, err := api.StorageVolLookupByName(pool, overlayVolumeName(name))
+		if err != nil {
+			return libvirt.StorageVol{}, fmt.Errorf("looking up existing overlay volume: %w", err)
+		}
+		return vol, nil
 	default:
-		return fmt.Errorf("creating overlay volume: %w", err)
+		return libvirt.StorageVol{}, fmt.Errorf("creating overlay volume: %w", err)
 	}
-	return nil
 }
 
 // writeSeed replaces the seed volume with a freshly built NoCloud ISO.
 // Only called while no domain references it.
-func (p *Provider) writeSeed(log logr.Logger, api libvirtAPI, pool libvirt.StoragePool, name, userData string) error {
+func (p *Provider) writeSeed(log logr.Logger, api libvirtAPI, pool libvirt.StoragePool, name, userData string) (libvirt.StorageVol, error) {
 	seedName := seedVolumeName(name)
 	if err := deleteIfExists(api, pool, seedName); err != nil {
-		return err
+		return libvirt.StorageVol{}, err
 	}
-	iso, err := buildSeedISO(name, userData, p.cfg.VendorData)
+	vendorData, err := buildVendorData(p.cfg.VendorData, p.cfg.InstallGuestAgent == nil || *p.cfg.InstallGuestAgent)
 	if err != nil {
-		return fmt.Errorf("%w: %v", provider.ErrPermanent, err)
+		return libvirt.StorageVol{}, fmt.Errorf("%w: %v", provider.ErrPermanent, err)
+	}
+	iso, err := buildSeedISO(name, userData, vendorData)
+	if err != nil {
+		return libvirt.StorageVol{}, fmt.Errorf("%w: %v", provider.ErrPermanent, err)
 	}
 	xml, err := buildVolumeXML(rawVolume(seedName, int64(len(iso))))
 	if err != nil {
-		return err
+		return libvirt.StorageVol{}, err
 	}
 	vol, err := api.StorageVolCreateXML(pool, xml, 0)
 	if err != nil {
-		return fmt.Errorf("creating seed volume: %w", err)
+		return libvirt.StorageVol{}, fmt.Errorf("creating seed volume: %w", err)
 	}
 	if err := api.StorageVolUpload(vol, bytes.NewReader(iso), 0, 0, 0); err != nil {
-		return fmt.Errorf("uploading seed volume: %w", err)
+		return libvirt.StorageVol{}, fmt.Errorf("uploading seed volume: %w", err)
 	}
 	log.V(1).Info("seed volume uploaded", "volume", seedName, "bytes", len(iso))
-	return nil
+	return vol, nil
 }
 
-func (p *Provider) defineDomain(log logr.Logger, api libvirtAPI, host provider.KVMHost, req provider.CreateRequest, mt provider.KVMMachineType) (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,
-		Pool:      host.Pool,
+		Firmware:  firmware,
 		Bridge:    host.Bridge,
-		DiskVol:   overlayVolumeName(req.Name),
+		DiskPath:  diskPath,
-		SeedVol:   seedVolumeName(req.Name),
+		SeedPath:  seedPath,
 	})
 	if err != nil {
 		return libvirt.Domain{}, err
@@ -265,7 +293,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
 }
 
@@ -303,7 +331,7 @@ func (p *Provider) Get(ctx context.Context, providerID string) (*provider.Instan
 	}
 	log := p.logger(ctx).WithValues("host", host.Name, "name", name)
 	var inst *provider.Instance
-	err = p.hosts.call(ctx, host, func(api libvirtAPI) error {
+	err = p.hosts.call(ctx, log, host, func(api libvirtAPI) error {
 		dom, err := api.DomainLookupByName(name)
 		if err != nil {
 			return fmt.Errorf("looking up domain: %w", err)
@@ -428,7 +456,8 @@ func mapState(s libvirt.DomainState, ip string) provider.InstanceState {
 // domain is not destroyed again, a missing domain is not undefined, and
 // the per-VM volumes are removed by name even when the domain is already
 // gone, so a crash between volume creation and define is cleaned up on the
-// next reconcile.
+// next reconcile. A providerID naming a host that is no longer configured
+// is a no-op success rather than an error.
 func (p *Provider) Delete(ctx context.Context, providerID string) error {
 	hostName, name, err := parseProviderID(providerID)
 	if err != nil {
@@ -436,10 +465,17 @@ func (p *Provider) Delete(ctx context.Context, providerID string) error {
 	}
 	host, err := p.host(hostName)
 	if err != nil {
-		return p.wrapErr("delete", providerID, err)
+		// The host is no longer configured, so this operator has no way to
+		// reach the domain and nothing left to clean. Same reasoning as
+		// removeVolumes over a missing pool: refusing would pin the Proxy's
+		// finalizer. Whatever is still running there is the admin's to remove,
+		// and orphan GC cannot see it either — see docs/kvm-provider.md.
+		p.logger(ctx).Info("host is not configured; delete has nothing to clean up",
+			"host", hostName, "providerID", providerID)
+		return nil
 	}
 	log := p.logger(ctx).WithValues("host", host.Name, "name", name)
-	err = p.hosts.call(ctx, host, func(api libvirtAPI) error {
+	err = p.hosts.call(ctx, log, host, func(api libvirtAPI) error {
 		if err := p.removeDomain(log, api, name); err != nil {
 			return err
 		}
@@ -511,7 +547,7 @@ func (p *Provider) ListByTag(ctx context.Context) ([]provider.Instance, error) {
 	var out []provider.Instance
 	for _, host := range p.cfg.Hosts {
 		hlog := log.WithValues("host", host.Name)
-		err := p.hosts.call(ctx, host, func(api libvirtAPI) error {
+		err := p.hosts.call(ctx, hlog, host, func(api libvirtAPI) error {
 			doms, _, err := api.ConnectListAllDomains(1, 0)
 			if err != nil {
 				return fmt.Errorf("listing domains: %w", err)

internal/provider/kvm/kvm_integration_test.go

@@ -25,6 +25,17 @@ package kvm
 //	KVM_TEST_SOCKET        libvirt socket path on the host
 //	KVM_TEST_TIMEOUT       overall budget incl. the image download (default 20m)
 //	KVM_TEST_KEEP          set to anything to leave the VM behind for inspection
+//	KVM_TEST_LOG_LEVEL     provider log verbosity shown in -v output (default 1:
+//	                       one line per libvirt call; 2 adds XML, metadata and
+//	                       addresses; 0 leaves only the t.Log lines)
+//
+// Provider logs are printed through t.Log in a compact form: time, logger
+// name, then the libvirt API function (or the message) and its fields.
+// The provider/host/name values that are the same on every line of a run
+// are dropped; the first t.Log line of the run states them. A run that
+// hits its timeout may log one more line from the RPC the pool abandoned,
+// after the test has ended; that is a symptom of the timeout, not a bug
+// in the tracer.
 
 import (
 	"context"
@@ -34,11 +45,15 @@ import (
 	"fmt"
 	"net"
 	"os"
+	"strconv"
+	"strings"
 	"testing"
 	"time"
 
 	libvirt "github.com/digitalocean/go-libvirt"
+	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/types"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider"
 )
@@ -53,6 +68,7 @@ type integrationEnv struct {
 	cfg      provider.ProviderConfig
 	timeout  time.Duration
 	keep     bool
+	logLevel int
 }
 
 func loadIntegrationEnv(t *testing.T) integrationEnv {
@@ -97,7 +113,95 @@ func loadIntegrationEnv(t *testing.T) integrationEnv {
 		}
 		timeout = d
 	}
-	return integrationEnv{cfg: pc, timeout: timeout, keep: os.Getenv("KVM_TEST_KEEP") != ""}
+	logLevel := 1
+	if v := os.Getenv("KVM_TEST_LOG_LEVEL"); v != "" {
+		n, err := strconv.Atoi(v)
+		if err != nil {
+			t.Fatalf("KVM_TEST_LOG_LEVEL: %v", err)
+		}
+		logLevel = n
+	}
+	return integrationEnv{cfg: pc, timeout: timeout, keep: os.Getenv("KVM_TEST_KEEP") != "", logLevel: logLevel}
+}
+
+// integrationLogger routes provider logs into the test output at the
+// requested verbosity, and installs it as controller-runtime's root logger
+// so nothing falls back to the unfulfilled delegating sink (which would
+// print its "SetLogger was never called" warning and swallow the lines).
+func integrationLogger(t *testing.T, verbosity int) logr.Logger {
+	t.Helper()
+	log := logr.New(&compactSink{t: t, verbosity: verbosity})
+	logf.SetLogger(log)
+	return log
+}
+
+// compactSink is a logr sink for test output: one line per record as
+// "HH:MM:SS.mmm name headline k=v ...", where the headline is the rpc
+// field when present (the libvirt API function) and the message
+// otherwise. Keys that are constant for the whole run are omitted.
+type compactSink struct {
+	t         *testing.T
+	verbosity int
+	name      string
+	values    []any
+}
+
+var compactOmit = map[string]bool{"provider": true, "host": true, "name": true}
+
+func (s *compactSink) Init(logr.RuntimeInfo) {}
+
+func (s *compactSink) Enabled(level int) bool { return level <= s.verbosity }
+
+// GetCallStackHelper lets logr mark its frames as helpers, so t.Log
+// attributes each line to the provider code that logged it.
+func (s *compactSink) GetCallStackHelper() func() { return s.t.Helper }
+
+func (s *compactSink) Info(_ int, msg string, kv ...any) {
+	s.t.Helper()
+	s.t.Log(s.format(msg, kv))
+}
+
+func (s *compactSink) Error(err error, msg string, kv ...any) {
+	s.t.Helper()
+	s.t.Log(s.format(msg, append(kv, "error", err)))
+}
+
+func (s *compactSink) WithValues(kv ...any) logr.LogSink {
+	c := *s
+	c.values = append(append([]any(nil), s.values...), kv...)
+	return &c
+}
+
+func (s *compactSink) WithName(name string) logr.LogSink {
+	c := *s
+	if c.name != "" {
+		name = c.name + "/" + name
+	}
+	c.name = name
+	return &c
+}
+
+func (s *compactSink) format(msg string, kv []any) string {
+	all := append(append([]any(nil), s.values...), kv...)
+	headline := msg
+	var b strings.Builder
+	for i := 0; i+1 < len(all); i += 2 {
+		key := fmt.Sprint(all[i])
+		if key == "rpc" {
+			headline = fmt.Sprint(all[i+1])
+			continue
+		}
+		if compactOmit[key] {
+			continue
+		}
+		val := fmt.Sprint(all[i+1])
+		if strings.ContainsAny(val, " \t\"=") {
+			val = strconv.Quote(val)
+		}
+		b.WriteString(" " + key + "=" + val)
+	}
+	name := s.name[strings.LastIndex(s.name, "/")+1:]
+	return fmt.Sprintf("%s %s %s%s", time.Now().Format("15:04:05.000"), name, headline, b.String())
 }
 
 func randomUID(t *testing.T) string {
@@ -114,7 +218,8 @@ func randomUID(t *testing.T) string {
 // stage depends on the state the previous one left on the hypervisor.
 func TestKVM_lifecycle(t *testing.T) {
 	env := loadIntegrationEnv(t)
-	ctx, cancel := context.WithTimeout(context.Background(), env.timeout)
+	log := integrationLogger(t, env.logLevel)
+	ctx, cancel := context.WithTimeout(logr.NewContext(context.Background(), log), env.timeout)
 	defer cancel()
 
 	prov, err := New(ctx, env.cfg)
@@ -143,7 +248,7 @@ func TestKVM_lifecycle(t *testing.T) {
 			t.Logf("KVM_TEST_KEEP set: leaving domain %s and its volumes on %s", req.Name, host.Address)
 			return
 		}
-		cctx, ccancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		cctx, ccancel := context.WithTimeout(logr.NewContext(context.Background(), log), 2*time.Minute)
 		defer ccancel()
 		if err := p.Delete(cctx, formatProviderID(host.Name, req.Name)); err != nil {
 			t.Errorf("cleanup Delete: %v", err)
@@ -217,7 +322,7 @@ func TestKVM_lifecycle(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = p.hosts.call(ctx, host, func(api libvirtAPI) error {
+	err = p.hosts.call(ctx, p.logger(ctx).WithValues("host", host.Name), host, func(api libvirtAPI) error {
 		pool, err := api.StoragePoolLookupByName(host.Pool)
 		if err != nil {
 			return err

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"},
@@ -30,12 +30,15 @@ func TestWithDefaults(t *testing.T) {
 	if cfg.DiskSizeGB != defaultDiskSizeGB {
 		t.Errorf("diskSizeGb = %d, want %d", cfg.DiskSizeGB, defaultDiskSizeGB)
 	}
-	if cfg.VendorData != defaultVendorData {
+	if cfg.VendorData != "" {
-		t.Errorf("vendorData = %q, want the default", cfg.VendorData)
+		t.Errorf("vendorData = %q, want none: the guest-agent bootstrap is merged into the user-data, not defaulted here", cfg.VendorData)
 	}
 	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)
 	}
@@ -92,7 +95,8 @@ func testConfig(hosts ...string) provider.ProviderConfig {
 }
 
 func newTestProvider(fakes hostFakes, hosts ...string) *Provider {
-	p := newWithDialer(testConfig(hosts...), fakes.dial, nil)
+	pc := testConfig(hosts...)
+	p := newWithDialer(pc.Name, *pc.KVM, fakes.dial, nil)
 	p.now = func() time.Time { return testNow }
 	return p
 }
@@ -100,7 +104,7 @@ func newTestProvider(fakes hostFakes, hosts ...string) *Provider {
 // cacheImage pre-populates the pool with the configured image and its
 // marker so Create never touches the network.
 func cacheImage(api *fakeAPI) {
-	src := imageSource{Key: "debian-13", URL: testImageURL, Format: formatQcow2}
+	src := imageSource{Key: "debian-13", KVMImage: provider.KVMImage{URL: testImageURL, Format: formatQcow2}}
 	api.putVol("default", src.volumeName(), formatQcow2, []byte("qcow2"))
 	api.putVol("default", src.markerName(), formatRaw, []byte(testImageSHA()))
 }
@@ -184,6 +188,13 @@ func assertCreated(t *testing.T, api *fakeAPI) {
 	if seed.format != formatRaw || len(seed.data) == 0 || int64(len(seed.data)) != seed.capacity {
 		t.Errorf("seed = format %s, %d bytes, capacity %d; want raw with capacity == uploaded size", seed.format, len(seed.data), seed.capacity)
 	}
+	_, files := readSeed(t, seed.data)
+	if files["user-data"] != testCloudInit {
+		t.Errorf("seed user-data = %q, want the Proxy's cloud-init verbatim", files["user-data"])
+	}
+	if !strings.Contains(files["vendor-data"], "qemu-guest-agent") {
+		t.Errorf("seed vendor-data = %q, want the guest-agent script", files["vendor-data"])
+	}
 }
 
 func TestCreate_freshHost(t *testing.T) {
@@ -210,6 +221,13 @@ func TestCreate_freshHost(t *testing.T) {
 	if d.Devices.Interfaces[0].Source.Bridge != "br0" {
 		t.Errorf("bridge = %q, want br0", d.Devices.Interfaces[0].Source.Bridge)
 	}
+	wantDisk, wantSeed := api.poolPath+"/"+overlayVolumeName(testName), api.poolPath+"/"+seedVolumeName(testName)
+	if got := d.Devices.Disks[0].Source.File; got != wantDisk {
+		t.Errorf("overlay disk source = %q, want the path libvirt reported, %q", got, wantDisk)
+	}
+	if got := d.Devices.Disks[1].Source.File; got != wantSeed {
+		t.Errorf("seed cdrom source = %q, want the path libvirt reported, %q", got, wantSeed)
+	}
 	if m := d.Metadata.Instance; m.UID != testUID || !m.managed() || !m.createdAt().Equal(testNow) {
 		t.Errorf("metadata = %+v, want managed, uid %s, createdAt %s", m, testUID, testNow)
 	}
@@ -224,7 +242,7 @@ func TestCreate_fetchesImageOnFirstUse(t *testing.T) {
 	api := newFakeAPI("default")
 	cfg := testConfig("nas1")
 	cfg.KVM.Images["debian-13"] = provider.KVMImage{URL: srv.URL + "/debian.qcow2", SHA256: testImageSHA()}
-	p := newWithDialer(cfg, hostFakes{"nas1": api}.dial, nil)
+	p := newWithDialer(cfg.Name, *cfg.KVM, hostFakes{"nas1": api}.dial, nil)
 
 	if _, err := p.Create(context.Background(), testRequest()); err != nil {
 		t.Fatalf("Create: %v", err)
@@ -462,28 +480,25 @@ func TestCreate_hostFailures(t *testing.T) {
 	t.Parallel()
 	cases := []struct {
 		name  string
-		setup func() (hostFakes, string)
+		setup func() hostFakes
 		class error
 	}{
 		{
 			name:  "host unreachable",
-			setup: func() (hostFakes, string) { return hostFakes{}, "nas1" },
+			setup: func() hostFakes { return hostFakes{} },
 			class: provider.ErrTransient,
 		},
 		{
 			name:  "pool missing",
-			setup: func() (hostFakes, string) {
+			setup: func() hostFakes { return hostFakes{"nas1": newFakeAPI("other")} },
-				api := newFakeAPI("other")
-				return hostFakes{"nas1": api}, "nas1"
-			},
 			class: provider.ErrPermanent,
 		},
 		{
 			name: "domain xml rejected",
-			setup: func() (hostFakes, string) {
+			setup: func() hostFakes {
 				api := readyHost()
 				api.fail["DomainDefineXML"] = lvError(libvirt.ErrConfigUnsupported, "no q35 here")
-				return hostFakes{"nas1": api}, "nas1"
+				return hostFakes{"nas1": api}
 			},
 			class: provider.ErrPermanent,
 		},
@@ -491,8 +506,7 @@ func TestCreate_hostFailures(t *testing.T) {
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
-			fakes, host := tc.setup()
+			p := newTestProvider(tc.setup(), "nas1")
-			p := newTestProvider(fakes, host)
 			_, err := p.Create(context.Background(), testRequest())
 			wantClass(t, err, tc.class)
 		})
@@ -712,6 +726,28 @@ func TestDelete_missingPoolIsNotAnError(t *testing.T) {
 	}
 }
 
+// A host dropped from the config leaves the operator no way to reach the
+// domain, so Delete has nothing left to clean. Failing instead would pin
+// the Proxy's finalizer forever, since the reconciler's delete path cannot
+// make progress without a Delete that can succeed.
+func TestDelete_unconfiguredHostIsNotAnError(t *testing.T) {
+	t.Parallel()
+	api := readyHost()
+	managedDomain(t, api, testName, testUID, true, nil)
+	p := newTestProvider(hostFakes{"nas1": api}, "nas1")
+
+	if err := p.Delete(context.Background(), "hosts/nas9/domains/"+testName); err != nil {
+		t.Fatalf("Delete over an unconfigured host: %v", err)
+	}
+	if api.domain(testName) == nil {
+		t.Error("domain on the configured host was touched; only the named host's state may change")
+	}
+	// Get still refuses, so the live path cannot mistake a config typo for a
+	// missing VM and recreate it somewhere else.
+	_, err := p.Get(context.Background(), "hosts/nas9/domains/"+testName)
+	wantClass(t, err, provider.ErrPermanent)
+}
+
 func TestDelete_errors(t *testing.T) {
 	t.Parallel()
 	cases := []struct {
@@ -721,7 +757,6 @@ func TestDelete_errors(t *testing.T) {
 		class error
 	}{
 		{"malformed id", "nope", func(*testing.T) hostFakes { return hostFakes{"nas1": readyHost()} }, provider.ErrPermanent},
-		{"host not configured", "hosts/nas9/domains/x", func(*testing.T) hostFakes { return hostFakes{"nas1": readyHost()} }, provider.ErrPermanent},
 		{"host unreachable", "hosts/nas1/domains/x", func(*testing.T) hostFakes { return hostFakes{} }, provider.ErrTransient},
 		{
 			"undefine fails", "hosts/nas1/domains/" + testName,
@@ -870,11 +905,95 @@ func TestMapState(t *testing.T) {
 	}
 }
 
-func TestNew_badKeyPathFailsFast(t *testing.T) {
+// runAllOps drives one full lifecycle over a ready fake host so logging
+// tests see every provider method and every libvirt call.
+func runAllOps(t *testing.T, ctx context.Context, req provider.CreateRequest) {
+	t.Helper()
+	p := newTestProvider(hostFakes{"nas1": readyHost()}, "nas1")
+	id, err := p.Create(ctx, req)
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	if _, err := p.Get(ctx, id); err != nil {
+		t.Fatalf("Get: %v", err)
+	}
+	if _, err := p.ListByTag(ctx); err != nil {
+		t.Fatalf("ListByTag: %v", err)
+	}
+	if err := p.Delete(ctx, id); err != nil {
+		t.Fatalf("Delete: %v", err)
+	}
+}
+
+func TestLogging_verbosityTiers(t *testing.T) {
 	t.Parallel()
-	cfg := testConfig("nas1")
+	tests := []struct {
-	cfg.KVM.SSHKeyFile = t.TempDir() + "/missing"
+		name        string
-	if _, err := New(context.Background(), cfg); err == nil {
+		verbosity   int
-		t.Fatal("New succeeded with a missing key file")
+		wantLines   []string
+		absentLines []string
+	}{
+		{
+			name:        "v0 stays silent",
+			verbosity:   0,
+			absentLines: []string{"libvirt call", "KVM domain ensured running"},
+		},
+		{
+			name:      "v1 logs one line per libvirt call plus the operation outcomes",
+			verbosity: 1,
+			wantLines: []string{
+				`kvm/libvirt "level"=1 "msg"="libvirt host connected"`,
+				`"msg"="libvirt call"`,
+				`"rpc"="virDomainLookupByName"`,
+				`"libvirtError"="ErrNoDomain"`,
+				`"rpc"="virStorageVolUpload"`,
+				`"rpc"="virDomainDefineXML"`,
+				`"rpc"="virDomainCreate"`,
+				`"rpc"="virDomainInterfaceAddresses"`,
+				`"rpc"="virConnectListAllDomains"`,
+				`"rpc"="virDomainUndefineFlags"`,
+				`"rpc"="virStorageVolDelete"`,
+				`"provider"="kvm-home"`,
+				`"host"="nas1"`,
+				`"name"="` + testName + `"`,
+				`"msg"="KVM domain ensured running"`,
+			},
+			absentLines: []string{`"xml"=`, `"metadata"=`},
+		},
+		{
+			name:      "v2 adds xml, metadata and addresses on the same lines",
+			verbosity: 2,
+			wantLines: []string{`"xml"="<domain`, `"xml"="<volume`, `"metadata"="<instance`, `"names"=[`},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			ctx, snapshot := captureContext(tc.verbosity)
+			req := testRequest()
+			const sentinel = "SENTINEL-cloud-init-must-never-be-logged"
+			req.CloudInit = "#cloud-config\nhostname: " + sentinel + "\n"
+
+			runAllOps(t, ctx, req)
+
+			lines := snapshot()
+			joined := strings.Join(lines, "\n")
+			if tc.verbosity == 0 && len(lines) != 0 {
+				t.Errorf("verbosity 0 logged %d lines:\n%s", len(lines), joined)
+			}
+			for _, want := range tc.wantLines {
+				if !strings.Contains(joined, want) {
+					t.Errorf("output missing %s:\n%s", want, joined)
+				}
+			}
+			for _, absent := range tc.absentLines {
+				if strings.Contains(joined, absent) {
+					t.Errorf("output unexpectedly contains %s:\n%s", absent, joined)
+				}
+			}
+			if strings.Contains(joined, sentinel) {
+				t.Errorf("cloud-init content leaked into logs:\n%s", joined)
+			}
+		})
 	}
 }

internal/provider/kvm/metadata.go

@@ -31,15 +31,19 @@ func newInstanceMetadata(uid string, createdAt time.Time) instanceMetadata {
 }
 
 // parseInstanceMetadata decodes the element as DomainGetMetadata returns it.
-// libvirt may re-serialise it with a namespace prefix rather than the default
+// libvirt selects the element by namespace URI but serialises it without the
-// namespace it was defined with; encoding/xml matches on namespace URI, so
+// declaration (observed on libvirt 11.6: a bare <instance>), and other
-// both spellings decode.
+// versions may keep a default or prefixed form. The URI is already checked
+// by the lookup, so only the local name is matched here.
 func parseInstanceMetadata(s string) (instanceMetadata, error) {
-	var m instanceMetadata
+	var raw struct {
-	if err := xml.Unmarshal([]byte(s), &m); err != nil {
+		XMLName xml.Name `xml:"instance"`
+		instanceMetadata
+	}
+	if err := xml.Unmarshal([]byte(s), &raw); err != nil {
 		return instanceMetadata{}, fmt.Errorf("parsing domain metadata: %w", err)
 	}
-	return m, nil
+	return raw.instanceMetadata, nil
 }
 
 func (m instanceMetadata) managed() bool {

internal/provider/kvm/metadata_test.go

@@ -29,32 +29,37 @@ func TestInstanceMetadata_roundTrip(t *testing.T) {
 	}
 }
 
-func TestParseInstanceMetadata_libvirtPrefixedForm(t *testing.T) {
+func TestParseInstanceMetadata(t *testing.T) {
-	t.Parallel()
-	// libvirt re-serialises the stored element with whatever prefix it
-	// chooses; only the namespace URI is contractual.
-	in := `<op:instance xmlns:op="` + metadataURI + `">` +
-		`<op:managed>true</op:managed><op:uid>uid-2</op:uid>` +
-		`<op:createdAt>2026-09-04T10:00:00Z</op:createdAt></op:instance>`
-	m, err := parseInstanceMetadata(in)
-	if err != nil {
-		t.Fatalf("parseInstanceMetadata() error = %v", err)
-	}
-	if !m.managed() || m.UID != "uid-2" || m.createdAt().Hour() != 10 {
-		t.Errorf("parsed = %+v", m)
-	}
-}
-
-func TestParseInstanceMetadata_notManaged(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
 		name        string
 		in          string
 		wantErr     bool
+		wantManaged bool
+		wantUID     string
+		wantCreated time.Time
 	}{
-		{"managed false", `<instance xmlns="` + metadataURI + `"><managed>false</managed><uid>u</uid></instance>`, false},
+		{
-		{"foreign element", `<foo xmlns="urn:other"><managed>true</managed></foo>`, true},
+			// libvirt re-serialises the stored element with whatever prefix it
-		{"garbage", `not xml`, true},
+			// chooses; only the namespace URI is contractual.
+			name: "libvirt prefixed form",
+			in: `<op:instance xmlns:op="` + metadataURI + `">` +
+				`<op:managed>true</op:managed><op:uid>uid-2</op:uid>` +
+				`<op:createdAt>2026-09-04T10:00:00Z</op:createdAt></op:instance>`,
+			wantManaged: true, wantUID: "uid-2", wantCreated: time.Date(2026, 9, 4, 10, 0, 0, 0, time.UTC),
+		},
+		{
+			// What libvirt 11.6 actually returns from DomainGetMetadata: the
+			// element selected by our URI, serialised without any namespace
+			// declaration.
+			name: "libvirt bare form",
+			in: `<instance><managed>true</managed><uid>uid-3</uid>` +
+				`<createdAt>2026-09-10T07:30:00Z</createdAt></instance>`,
+			wantManaged: true, wantUID: "uid-3", wantCreated: time.Date(2026, 9, 10, 7, 30, 0, 0, time.UTC),
+		},
+		{name: "managed false", in: `<instance xmlns="` + metadataURI + `"><managed>false</managed><uid>u</uid></instance>`, wantUID: "u"},
+		{name: "foreign element", in: `<foo xmlns="urn:other"><managed>true</managed></foo>`, wantErr: true},
+		{name: "garbage", in: `not xml`, wantErr: true},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
@@ -63,8 +68,8 @@ func TestParseInstanceMetadata_notManaged(t *testing.T) {
 			if (err != nil) != tc.wantErr {
 				t.Fatalf("error = %v, wantErr %v", err, tc.wantErr)
 			}
-			if m.managed() {
+			if m.managed() != tc.wantManaged || m.UID != tc.wantUID || !m.createdAt().Equal(tc.wantCreated) {
-				t.Errorf("managed() = true for %q", tc.in)
+				t.Errorf("parsed = %+v, want managed=%v uid=%q createdAt=%v", m, tc.wantManaged, tc.wantUID, tc.wantCreated)
 			}
 		})
 	}

internal/provider/kvm/seed.go

@@ -3,6 +3,9 @@ package kvm
 import (
 	"bytes"
 	"fmt"
+	"io"
+	"mime/multipart"
+	"net/textproto"
 	"strings"
 
 	"github.com/kdomanski/iso9660"
@@ -11,20 +14,97 @@ import (
 // seedLabel is the volume label cloud-init's NoCloud datasource looks for.
 const seedLabel = "cidata"
 
-// defaultVendorData is the cloud-init vendor-data shipped on every seed
+// guestAgentScript installs and starts the QEMU guest agent, which is how
-// unless the config overrides it. cloud-init merges it under the Proxy's own
+// the provider learns a VM's address before the host's ARP table has seen
-// user-data, so stock images grow a guest agent (which lets the provider
+// any traffic from it.
-// read the VM's IP before ARP sees traffic) without the user's cloud-init
+//
-// having to know about it.
+// It ships as vendor-data, and as a *script* rather than a cloud-config,
-const defaultVendorData = `#cloud-config
+// because that is the only shape cloud-init cannot drop. Vendor-data
-packages:
+// cloud-config is merged under the Proxy's own user-data with mergers that
-  - qemu-guest-agent
+// keep the user-data's value for every key both documents define and never
-runcmd:
+// append lists, so a Proxy carrying its own packages: silently lost the
-  - [systemctl, enable, --now, qemu-guest-agent]
+// agent. The strategy cannot be overridden from either document: merge_how
+// in user-data governs only the first source, and merge_how in vendor-data
+// is consumed and discarded before the document is stored. A vendor-data
+// script bypasses all of it — cloud-init gives vendor-data its own handler
+// chain, writes scripts to a separate directory and runs them from
+// cc_scripts_vendor, after the user's packages are installed and before the
+// user's runcmd.
+const guestAgentScript = `#!/bin/sh
+# Added by egress-proxies-operator so the hypervisor can report this VM's
+# address. Set installGuestAgent: false on the provider to leave it out.
+set -e
+if ! command -v qemu-ga >/dev/null 2>&1 && [ ! -x /usr/sbin/qemu-ga ]; then
+	if command -v apt-get >/dev/null 2>&1; then
+		apt-get update
+		DEBIAN_FRONTEND=noninteractive apt-get install -y qemu-guest-agent
+	elif command -v dnf >/dev/null 2>&1; then
+		dnf install -y qemu-guest-agent
+	elif command -v yum >/dev/null 2>&1; then
+		yum install -y qemu-guest-agent
+	elif command -v zypper >/dev/null 2>&1; then
+		zypper --non-interactive install qemu-guest-agent
+	elif command -v apk >/dev/null 2>&1; then
+		apk add qemu-guest-agent
+	else
+		echo "no known package manager; install qemu-guest-agent in the image" >&2
+		exit 1
+	fi
+fi
+if command -v systemctl >/dev/null 2>&1; then
+	systemctl enable --now qemu-guest-agent
+elif command -v rc-update >/dev/null 2>&1; then
+	rc-update add qemu-guest-agent default
+	rc-service qemu-guest-agent start
+fi
 `
 
-// buildSeedISO renders a NoCloud seed: user-data verbatim, vendor-data as
+// buildVendorData renders the seed's vendor-data. With nothing configured
-// given, and a meta-data whose instance-id is the deterministic VM name, so
+// it is the guest-agent script alone; with both, a MIME multipart carrying
+// the configured document and the script as separate parts, since
+// vendor-data is a single file and the two must not be concatenated.
+func buildVendorData(configured string, installAgent bool) (string, error) {
+	switch {
+	case !installAgent:
+		return configured, nil
+	case strings.TrimSpace(configured) == "":
+		return guestAgentScript, nil
+	default:
+		return multipartVendorData(configured, guestAgentScript)
+	}
+}
+
+// multipartVendorData packs several documents into one vendor-data blob.
+// cloud-init decides a payload is multipart by finding "mime-version:"
+// within its first 4096 bytes, so those headers lead. The configured
+// document goes in as text/plain on purpose: cloud-init re-sniffs that type
+// from the content's own header line, which keeps this code out of the
+// business of classifying what the user wrote.
+func multipartVendorData(configured, script string) (string, error) {
+	var body bytes.Buffer
+	w := multipart.NewWriter(&body)
+	parts := []struct{ contentType, content string }{
+		{"text/plain", configured},
+		{"text/x-shellscript", script},
+	}
+	for _, p := range parts {
+		part, err := w.CreatePart(textproto.MIMEHeader{"Content-Type": {p.contentType}})
+		if err != nil {
+			return "", fmt.Errorf("building vendor-data part: %w", err)
+		}
+		if _, err := io.WriteString(part, p.content); err != nil {
+			return "", fmt.Errorf("writing vendor-data part: %w", err)
+		}
+	}
+	if err := w.Close(); err != nil {
+		return "", fmt.Errorf("closing vendor-data: %w", err)
+	}
+	return fmt.Sprintf("MIME-Version: 1.0\r\nContent-Type: multipart/mixed; boundary=%q\r\n\r\n%s",
+		w.Boundary(), body.String()), nil
+}
+
+// buildSeedISO renders a NoCloud seed: user-data and vendor-data verbatim,
+// and a meta-data whose instance-id is the deterministic VM name, so
 // cloud-init never mistakes a reboot for a new instance. The ISO's volume
 // descriptor carries the build time, so two builds differ in a few
 // timestamp bytes; the files and label are identical.

internal/provider/kvm/seed_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"io"
 	"maps"
+	"mime"
+	"mime/multipart"
 	"strings"
 	"testing"
 
@@ -42,7 +44,7 @@ func readSeed(t *testing.T, iso []byte) (label string, files map[string]string)
 func TestBuildSeedISO_contents(t *testing.T) {
 	t.Parallel()
 	userData := "#cloud-config\npackages: [squid]\n"
-	iso, err := buildSeedISO("proxy-abc123def456ghij", userData, defaultVendorData)
+	iso, err := buildSeedISO("proxy-abc123def456ghij", userData, guestAgentScript)
 	if err != nil {
 		t.Fatalf("buildSeedISO() error = %v", err)
 	}
@@ -56,7 +58,7 @@ func TestBuildSeedISO_contents(t *testing.T) {
 	if files["user-data"] != userData {
 		t.Errorf("user-data = %q, want verbatim cloud-init", files["user-data"])
 	}
-	if files["vendor-data"] != defaultVendorData {
+	if files["vendor-data"] != guestAgentScript {
 		t.Errorf("vendor-data = %q", files["vendor-data"])
 	}
 	if got, want := files["meta-data"], "instance-id: proxy-abc123def456ghij\nlocal-hostname: proxy-abc123def456ghij\n"; got != want {
@@ -89,7 +91,7 @@ func TestBuildSeedISO_deterministicContent(t *testing.T) {
 
 func TestBuildSeedISO_emptyUserData(t *testing.T) {
 	t.Parallel()
-	iso, err := buildSeedISO("proxy-x", "", defaultVendorData)
+	iso, err := buildSeedISO("proxy-x", "", guestAgentScript)
 	if err != nil {
 		t.Fatalf("buildSeedISO() error = %v", err)
 	}
@@ -98,3 +100,134 @@ func TestBuildSeedISO_emptyUserData(t *testing.T) {
 		t.Errorf("user-data = %q (present=%v), want an empty file to still exist", body, ok)
 	}
 }
+
+// The whole point of shipping the agent as vendor-data: whatever the Proxy
+// wrote reaches the guest untouched, in any of cloud-init's formats. The
+// previous implementation parsed and re-rendered cloud-config user-data,
+// which dropped comments, reordered keys and re-typed unquoted scalars
+// (version: 1.10 arrived as 1.1).
+func TestBuildSeedISO_userDataIsNeverRewritten(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		userData string
+	}{
+		{"cloud-config with its own packages", "#cloud-config\n# keep me\npackages:\n  - squid\nversion: 1.10\nchpasswd: {list: 'u:0123456789'}\n"},
+		{"jinja template", "## template: jinja\n#cloud-config\nhostname: {{ v1.local_hostname }}\n"},
+		{"shell script", "#!/bin/sh\napt-get install -y squid\n"},
+		{"include", "#include\nhttps://example.invalid/x\n"},
+		{"mime multipart", "MIME-Version: 1.0\r\nContent-Type: multipart/mixed; boundary=\"x\"\r\n\r\n--x\r\nContent-Type: text/cloud-config\r\n\r\n#cloud-config\npackages: [squid]\n\r\n--x--\r\n"},
+		{"gzip payload", "\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03binary\x00bytes"},
+		{"leading blank line", "\n\n#cloud-config\npackages: [squid]\n"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			vendor, err := buildVendorData("", true)
+			if err != nil {
+				t.Fatalf("buildVendorData() error = %v", err)
+			}
+			iso, err := buildSeedISO("proxy-x", tc.userData, vendor)
+			if err != nil {
+				t.Fatalf("buildSeedISO() error = %v", err)
+			}
+			_, files := readSeed(t, iso)
+			if files["user-data"] != tc.userData {
+				t.Errorf("user-data was rewritten:\n got %q\nwant %q", files["user-data"], tc.userData)
+			}
+			if !strings.Contains(files["vendor-data"], "qemu-guest-agent") {
+				t.Errorf("vendor-data = %q, want the agent script", files["vendor-data"])
+			}
+		})
+	}
+}
+
+func TestBuildVendorData_agentScriptAlone(t *testing.T) {
+	t.Parallel()
+	got, err := buildVendorData("", true)
+	if err != nil {
+		t.Fatalf("buildVendorData() error = %v", err)
+	}
+	if got != guestAgentScript {
+		t.Errorf("vendor-data = %q, want the bare script", got)
+	}
+	if !strings.HasPrefix(got, "#!") {
+		t.Error("vendor-data must start with a shebang or cloud-init treats it as something other than a script")
+	}
+}
+
+func TestBuildVendorData_optOutKeepsConfiguredOnly(t *testing.T) {
+	t.Parallel()
+	configured := "#cloud-config\nntp: {enabled: true}\n"
+	tests := []struct {
+		name       string
+		configured string
+		want       string
+	}{
+		{"nothing configured", "", ""},
+		{"configured document passes through", configured, configured},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := buildVendorData(tc.configured, false)
+			if err != nil {
+				t.Fatalf("buildVendorData() error = %v", err)
+			}
+			if got != tc.want {
+				t.Errorf("vendor-data = %q, want %q", got, tc.want)
+			}
+			if strings.Contains(got, "qemu-guest-agent") {
+				t.Error("agent script present although installGuestAgent is off")
+			}
+		})
+	}
+}
+
+// Both documents have to survive in one file, and cloud-init only looks for
+// the multipart marker in the first 4096 bytes.
+func TestBuildVendorData_multipartCarriesBoth(t *testing.T) {
+	t.Parallel()
+	configured := "#cloud-config\nntp:\n  enabled: true\n"
+	got, err := buildVendorData(configured, true)
+	if err != nil {
+		t.Fatalf("buildVendorData() error = %v", err)
+	}
+	head := got
+	if len(head) > 4096 {
+		head = head[:4096]
+	}
+	if !strings.Contains(strings.ToLower(head), "mime-version:") {
+		t.Fatalf("no mime-version: in the first 4096 bytes; cloud-init would treat this as a single document:\n%s", head)
+	}
+	_, params, err := mime.ParseMediaType(strings.SplitN(got, "\r\n\r\n", 2)[0][strings.Index(got, "Content-Type:")+len("Content-Type: "):])
+	if err != nil {
+		t.Fatalf("parsing the multipart content type: %v", err)
+	}
+	body := strings.SplitN(got, "\r\n\r\n", 2)[1]
+	r := multipart.NewReader(strings.NewReader(body), params["boundary"])
+	var bodies, types []string
+	for {
+		part, err := r.NextPart()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatalf("reading part: %v", err)
+		}
+		content, err := io.ReadAll(part)
+		if err != nil {
+			t.Fatalf("reading part body: %v", err)
+		}
+		bodies = append(bodies, string(content))
+		types = append(types, part.Header.Get("Content-Type"))
+	}
+	if len(bodies) != 2 || bodies[0] != configured || bodies[1] != guestAgentScript {
+		t.Fatalf("parts = %q, want the configured document then the script", bodies)
+	}
+	// text/plain makes cloud-init re-sniff the user's document from its own
+	// header line, so this code never has to classify it.
+	if types[0] != "text/plain" || types[1] != "text/x-shellscript" {
+		t.Errorf("part types = %v, want [text/plain text/x-shellscript]", types)
+	}
+}

internal/provider/kvm/trace.go

@@ -0,0 +1,211 @@
+package kvm
+
+import (
+	"fmt"
+	"io"
+	"time"
+
+	libvirt "github.com/digitalocean/go-libvirt"
+	"github.com/go-logr/logr"
+)
+
+// tracingAPI wraps a libvirtAPI and logs one V(1) line per call after it
+// returns: the libvirt API function (rpc=virDomainLookupByName, ...), the
+// identifiers it was given, curated results, duration, and on failure the
+// error with libvirt's code name. At V(2) the same line also carries XML
+// bodies, metadata and address lists. Upload and download bodies are never
+// logged: the seed ISO carries cloud-init.
+type tracingAPI struct {
+	inner libvirtAPI
+	log   logr.Logger
+}
+
+type tracedCall struct {
+	t      *tracingAPI
+	method string
+	start  time.Time
+	kv     []any
+}
+
+// begin names the call by its libvirt API function: go-libvirt's method
+// names are the remote procedure names, which are the public vir* API
+// names without the prefix.
+func (t *tracingAPI) begin(method string, kv ...any) *tracedCall {
+	return &tracedCall{t: t, method: "vir" + method, start: time.Now(), kv: kv}
+}
+
+// detail adds key/values that are only worth the bytes at V(2).
+func (c *tracedCall) detail(kv ...any) *tracedCall {
+	if c.t.log.V(2).Enabled() {
+		c.kv = append(c.kv, kv...)
+	}
+	return c
+}
+
+func (c *tracedCall) done(err error, kv ...any) {
+	fields := make([]any, 0, 2+len(c.kv)+len(kv)+10)
+	fields = append(fields, "rpc", c.method)
+	fields = append(fields, c.kv...)
+	fields = append(fields, kv...)
+	fields = append(fields, "duration", time.Since(c.start).Round(time.Microsecond))
+	if err != nil {
+		fields = append(fields, "error", err.Error())
+		fields = append(fields, libvirtErrorFields(err)...)
+	}
+	c.t.log.V(1).Info("libvirt call", fields...)
+}
+
+// flagKV renders a flags value only when something is set; go-libvirt's
+// generated stringers do not know bitmask combinations.
+func flagKV(v int64) []any {
+	if v == 0 {
+		return nil
+	}
+	return []any{"flags", v}
+}
+
+func optString(s libvirt.OptString) string {
+	if len(s) == 0 {
+		return ""
+	}
+	return s[0]
+}
+
+func (t *tracingAPI) DomainLookupByName(name string) (libvirt.Domain, error) {
+	c := t.begin("DomainLookupByName", "domain", name)
+	d, err := t.inner.DomainLookupByName(name)
+	c.done(err)
+	return d, err
+}
+
+func (t *tracingAPI) DomainDefineXML(xml string) (libvirt.Domain, error) {
+	c := t.begin("DomainDefineXML", "xmlBytes", len(xml)).detail("xml", xml)
+	d, err := t.inner.DomainDefineXML(xml)
+	c.done(err, "domain", d.Name)
+	return d, err
+}
+
+func (t *tracingAPI) DomainCreate(d libvirt.Domain) error {
+	c := t.begin("DomainCreate", "domain", d.Name)
+	err := t.inner.DomainCreate(d)
+	c.done(err)
+	return err
+}
+
+func (t *tracingAPI) DomainDestroy(d libvirt.Domain) error {
+	c := t.begin("DomainDestroy", "domain", d.Name)
+	err := t.inner.DomainDestroy(d)
+	c.done(err)
+	return err
+}
+
+func (t *tracingAPI) DomainUndefineFlags(d libvirt.Domain, flags libvirt.DomainUndefineFlagsValues) error {
+	c := t.begin("DomainUndefineFlags", append([]any{"domain", d.Name}, flagKV(int64(flags))...)...)
+	err := t.inner.DomainUndefineFlags(d, flags)
+	c.done(err)
+	return err
+}
+
+func (t *tracingAPI) DomainGetState(d libvirt.Domain, flags uint32) (int32, int32, error) {
+	c := t.begin("DomainGetState", "domain", d.Name)
+	state, reason, err := t.inner.DomainGetState(d, flags)
+	c.done(err, "state", libvirt.DomainState(state), "reason", reason)
+	return state, reason, err
+}
+
+func (t *tracingAPI) DomainGetMetadata(d libvirt.Domain, typ int32, uri libvirt.OptString, flags libvirt.DomainModificationImpact) (string, error) {
+	c := t.begin("DomainGetMetadata", "domain", d.Name, "type", typ, "uri", optString(uri))
+	s, err := t.inner.DomainGetMetadata(d, typ, uri, flags)
+	c.detail("metadata", s).done(err, "metadataBytes", len(s))
+	return s, err
+}
+
+func (t *tracingAPI) DomainInterfaceAddresses(d libvirt.Domain, source uint32, flags uint32) ([]libvirt.DomainInterface, error) {
+	c := t.begin("DomainInterfaceAddresses", "domain", d.Name, "source", libvirt.DomainInterfaceAddressesSource(source))
+	ifaces, err := t.inner.DomainInterfaceAddresses(d, source, flags)
+	if t.log.V(2).Enabled() {
+		c.detail("addresses", formatAddresses(ifaces))
+	}
+	c.done(err, "interfaces", len(ifaces))
+	return ifaces, err
+}
+
+func formatAddresses(ifaces []libvirt.DomainInterface) []string {
+	var out []string
+	for _, iface := range ifaces {
+		for _, a := range iface.Addrs {
+			out = append(out, fmt.Sprintf("%s=%s/%d", iface.Name, a.Addr, a.Prefix))
+		}
+	}
+	return out
+}
+
+func (t *tracingAPI) ConnectListAllDomains(needResults int32, flags libvirt.ConnectListAllDomainsFlags) ([]libvirt.Domain, uint32, error) {
+	c := t.begin("ConnectListAllDomains", flagKV(int64(flags))...)
+	doms, n, err := t.inner.ConnectListAllDomains(needResults, flags)
+	if t.log.V(2).Enabled() {
+		names := make([]string, 0, len(doms))
+		for _, d := range doms {
+			names = append(names, d.Name)
+		}
+		c.detail("names", names)
+	}
+	c.done(err, "count", len(doms))
+	return doms, n, err
+}
+
+func (t *tracingAPI) StoragePoolLookupByName(name string) (libvirt.StoragePool, error) {
+	c := t.begin("StoragePoolLookupByName", "pool", name)
+	pool, err := t.inner.StoragePoolLookupByName(name)
+	c.done(err)
+	return pool, err
+}
+
+func (t *tracingAPI) StoragePoolRefresh(pool libvirt.StoragePool, flags uint32) error {
+	c := t.begin("StoragePoolRefresh", "pool", pool.Name)
+	err := t.inner.StoragePoolRefresh(pool, flags)
+	c.done(err)
+	return err
+}
+
+func (t *tracingAPI) StorageVolLookupByName(pool libvirt.StoragePool, name string) (libvirt.StorageVol, error) {
+	c := t.begin("StorageVolLookupByName", "pool", pool.Name, "volume", name)
+	vol, err := t.inner.StorageVolLookupByName(pool, name)
+	c.done(err)
+	return vol, err
+}
+
+func (t *tracingAPI) StorageVolGetPath(vol libvirt.StorageVol) (string, error) {
+	c := t.begin("StorageVolGetPath", "pool", vol.Pool, "volume", vol.Name)
+	path, err := t.inner.StorageVolGetPath(vol)
+	c.done(err, "path", path)
+	return path, err
+}
+
+func (t *tracingAPI) StorageVolCreateXML(pool libvirt.StoragePool, xml string, flags libvirt.StorageVolCreateFlags) (libvirt.StorageVol, error) {
+	c := t.begin("StorageVolCreateXML", append([]any{"pool", pool.Name, "xmlBytes", len(xml)}, flagKV(int64(flags))...)...).detail("xml", xml)
+	vol, err := t.inner.StorageVolCreateXML(pool, xml, flags)
+	c.done(err, "volume", vol.Name)
+	return vol, err
+}
+
+func (t *tracingAPI) StorageVolUpload(vol libvirt.StorageVol, r io.Reader, offset, length uint64, flags libvirt.StorageVolUploadFlags) error {
+	c := t.begin("StorageVolUpload", append([]any{"pool", vol.Pool, "volume", vol.Name, "offset", offset, "length", length}, flagKV(int64(flags))...)...)
+	err := t.inner.StorageVolUpload(vol, r, offset, length, flags)
+	c.done(err)
+	return err
+}
+
+func (t *tracingAPI) StorageVolDownload(vol libvirt.StorageVol, w io.Writer, offset, length uint64, flags libvirt.StorageVolDownloadFlags) error {
+	c := t.begin("StorageVolDownload", append([]any{"pool", vol.Pool, "volume", vol.Name, "offset", offset, "length", length}, flagKV(int64(flags))...)...)
+	err := t.inner.StorageVolDownload(vol, w, offset, length, flags)
+	c.done(err)
+	return err
+}
+
+func (t *tracingAPI) StorageVolDelete(vol libvirt.StorageVol, flags libvirt.StorageVolDeleteFlags) error {
+	c := t.begin("StorageVolDelete", append([]any{"pool", vol.Pool, "volume", vol.Name}, flagKV(int64(flags))...)...)
+	err := t.inner.StorageVolDelete(vol, flags)
+	c.done(err)
+	return err
+}

internal/provider/kvm/trace_test.go

@@ -0,0 +1,300 @@
+package kvm
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"reflect"
+	"strings"
+	"sync"
+	"testing"
+
+	libvirt "github.com/digitalocean/go-libvirt"
+	"github.com/go-logr/logr"
+	"github.com/go-logr/logr/funcr"
+)
+
+// captureLogger returns a funcr logger capped at verbosity (the test
+// stand-in for --zap-log-level=<verbosity>) and a snapshot function for the
+// lines it recorded. The sink is mutex-guarded: a cancelled call logs from
+// the abandoned goroutine.
+func captureLogger(verbosity int) (logr.Logger, func() []string) {
+	var mu sync.Mutex
+	var lines []string
+	log := funcr.New(func(prefix, args string) {
+		mu.Lock()
+		defer mu.Unlock()
+		lines = append(lines, prefix+" "+args)
+	}, funcr.Options{Verbosity: verbosity})
+	return log, func() []string {
+		mu.Lock()
+		defer mu.Unlock()
+		return append([]string(nil), lines...)
+	}
+}
+
+func captureContext(verbosity int) (context.Context, func() []string) {
+	log, snapshot := captureLogger(verbosity)
+	return logr.NewContext(context.Background(), log), snapshot
+}
+
+const stoppedName = "proxy-stopped"
+
+// tracedHost is a fake with one managed running domain and its volumes, so
+// every method of the seam has something to act on.
+func tracedHost(t *testing.T) *fakeAPI {
+	t.Helper()
+	api := readyHost()
+	managedDomain(t, api, testName, testUID, true, map[uint32][]libvirt.DomainInterface{
+		uint32(libvirt.DomainInterfaceAddressesSrcArp): {{Name: "vnet0", Addrs: []libvirt.DomainIPAddr{ipv4("192.168.1.50")}}},
+	})
+	api.putDomain(&fakeDomain{name: stoppedName})
+	return api
+}
+
+func TestTracingAPI_logsEveryMethod(t *testing.T) {
+	t.Parallel()
+	pool := libvirt.StoragePool{Name: "default"}
+	dom := libvirt.Domain{Name: testName}
+	overlay := libvirt.StorageVol{Pool: "default", Name: overlayVolumeName(testName)}
+	cases := []struct {
+		method string
+		call   func(api libvirtAPI) error
+	}{
+		{"DomainLookupByName", func(api libvirtAPI) error { _, err := api.DomainLookupByName(testName); return err }},
+		{"DomainDefineXML", func(api libvirtAPI) error {
+			xml, err := buildDomainXML(testDomainParams())
+			if err != nil {
+				return err
+			}
+			_, err = api.DomainDefineXML(xml)
+			return err
+		}},
+		{"DomainCreate", func(api libvirtAPI) error { return api.DomainCreate(libvirt.Domain{Name: stoppedName}) }},
+		{"DomainDestroy", func(api libvirtAPI) error { return api.DomainDestroy(dom) }},
+		{"DomainUndefineFlags", func(api libvirtAPI) error { return api.DomainUndefineFlags(dom, undefineFlags) }},
+		{"DomainGetState", func(api libvirtAPI) error { _, _, err := api.DomainGetState(dom, 0); return err }},
+		{"DomainGetMetadata", func(api libvirtAPI) error {
+			_, err := api.DomainGetMetadata(dom, int32(libvirt.DomainMetadataElement), libvirt.OptString{metadataURI}, 0)
+			return err
+		}},
+		{"DomainInterfaceAddresses", func(api libvirtAPI) error {
+			_, err := api.DomainInterfaceAddresses(dom, uint32(libvirt.DomainInterfaceAddressesSrcArp), 0)
+			return err
+		}},
+		{"ConnectListAllDomains", func(api libvirtAPI) error { _, _, err := api.ConnectListAllDomains(1, 0); return err }},
+		{"StoragePoolLookupByName", func(api libvirtAPI) error { _, err := api.StoragePoolLookupByName("default"); return err }},
+		{"StoragePoolRefresh", func(api libvirtAPI) error { return api.StoragePoolRefresh(pool, 0) }},
+		{"StorageVolLookupByName", func(api libvirtAPI) error { _, err := api.StorageVolLookupByName(pool, overlay.Name); return err }},
+		{"StorageVolGetPath", func(api libvirtAPI) error { _, err := api.StorageVolGetPath(overlay); return err }},
+		{"StorageVolCreateXML", func(api libvirtAPI) error {
+			xml, err := buildVolumeXML(rawVolume("new.iso", 3))
+			if err != nil {
+				return err
+			}
+			_, err = api.StorageVolCreateXML(pool, xml, 0)
+			return err
+		}},
+		{"StorageVolUpload", func(api libvirtAPI) error {
+			return api.StorageVolUpload(overlay, strings.NewReader("abc"), 0, 3, 0)
+		}},
+		{"StorageVolDownload", func(api libvirtAPI) error {
+			return api.StorageVolDownload(overlay, io.Discard, 0, 0, 0)
+		}},
+		{"StorageVolDelete", func(api libvirtAPI) error { return api.StorageVolDelete(overlay, 0) }},
+	}
+	if n := reflect.TypeOf((*libvirtAPI)(nil)).Elem().NumMethod(); n != len(cases) {
+		t.Fatalf("libvirtAPI has %d methods, this test covers %d; trace the new one", n, len(cases))
+	}
+	for _, tc := range cases {
+		t.Run(tc.method, func(t *testing.T) {
+			t.Parallel()
+			log, snapshot := captureLogger(1)
+			api := &tracingAPI{inner: tracedHost(t), log: log}
+			if err := tc.call(api); err != nil {
+				t.Fatalf("%s: %v", tc.method, err)
+			}
+			lines := snapshot()
+			if len(lines) != 1 {
+				t.Fatalf("logged %d lines, want exactly 1:\n%s", len(lines), strings.Join(lines, "\n"))
+			}
+			for _, want := range []string{`"msg"="libvirt call"`, `"rpc"="vir` + tc.method + `"`, `"duration"=`} {
+				if !strings.Contains(lines[0], want) {
+					t.Errorf("line missing %s:\n%s", want, lines[0])
+				}
+			}
+			if strings.Contains(lines[0], `"error"=`) {
+				t.Errorf("successful call logged an error:\n%s", lines[0])
+			}
+		})
+	}
+}
+
+func TestTracingAPI_passesResultsThrough(t *testing.T) {
+	t.Parallel()
+	inner := tracedHost(t)
+	log, _ := captureLogger(1)
+	traced := &tracingAPI{inner: inner, log: log}
+	dom := libvirt.Domain{Name: testName}
+
+	wantState, wantReason, err := inner.DomainGetState(dom, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	state, reason, err := traced.DomainGetState(dom, 0)
+	if err != nil || state != wantState || reason != wantReason {
+		t.Errorf("DomainGetState = %d/%d/%v, want %d/%d/nil", state, reason, err, wantState, wantReason)
+	}
+
+	overlay := libvirt.StorageVol{Pool: "default", Name: overlayVolumeName(testName)}
+	wantPath, _ := inner.StorageVolGetPath(overlay)
+	if path, err := traced.StorageVolGetPath(overlay); err != nil || path != wantPath {
+		t.Errorf("StorageVolGetPath = %q/%v, want %q", path, err, wantPath)
+	}
+
+	wantDoms, _, _ := inner.ConnectListAllDomains(1, 0)
+	doms, _, err := traced.ConnectListAllDomains(1, 0)
+	if err != nil || !reflect.DeepEqual(doms, wantDoms) {
+		t.Errorf("ConnectListAllDomains = %v/%v, want %v", doms, err, wantDoms)
+	}
+
+	inner.fail["DomainCreate"] = lvError(libvirt.ErrOperationInvalid, "already running")
+	if err := traced.DomainCreate(dom); !hasCode(err, libvirt.ErrOperationInvalid) {
+		t.Errorf("DomainCreate error = %v, want the injected libvirt error unchanged", err)
+	}
+}
+
+func TestTracingAPI_errorFields(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name    string
+		inject  error
+		want    []string
+		notWant []string
+	}{
+		{
+			name:   "libvirt error carries code, name and message",
+			inject: lvError(libvirt.ErrNoDomain, "Domain not found: no domain with matching name"),
+			want:   []string{`"error"="Domain not found`, `"libvirtError"="ErrNoDomain"`, `"libvirtCode"=42`},
+		},
+		{
+			name:    "transport error has no libvirt fields",
+			inject:  io.EOF,
+			want:    []string{`"error"="EOF"`},
+			notWant: []string{`"libvirtError"`, `"libvirtCode"`},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			inner := tracedHost(t)
+			inner.fail["DomainLookupByName"] = tc.inject
+			log, snapshot := captureLogger(1)
+			api := &tracingAPI{inner: inner, log: log}
+			if _, err := api.DomainLookupByName(testName); !errors.Is(err, tc.inject) {
+				t.Fatalf("error = %v, want the injected one", err)
+			}
+			joined := strings.Join(snapshot(), "\n")
+			for _, want := range tc.want {
+				if !strings.Contains(joined, want) {
+					t.Errorf("output missing %s:\n%s", want, joined)
+				}
+			}
+			for _, absent := range tc.notWant {
+				if strings.Contains(joined, absent) {
+					t.Errorf("output unexpectedly contains %s:\n%s", absent, joined)
+				}
+			}
+		})
+	}
+}
+
+func TestTracingAPI_detailOnlyAtV2(t *testing.T) {
+	t.Parallel()
+	dom := libvirt.Domain{Name: testName}
+	calls := []struct {
+		method string
+		call   func(api libvirtAPI) error
+		always string
+		detail string
+	}{
+		{"DomainDefineXML", func(api libvirtAPI) error {
+			xml, err := buildDomainXML(testDomainParams())
+			if err != nil {
+				return err
+			}
+			_, err = api.DomainDefineXML(xml)
+			return err
+		}, `"xmlBytes"=`, `"xml"="<domain`},
+		{"StorageVolCreateXML", func(api libvirtAPI) error {
+			xml, err := buildVolumeXML(rawVolume("new.iso", 3))
+			if err != nil {
+				return err
+			}
+			_, err = api.StorageVolCreateXML(libvirt.StoragePool{Name: "default"}, xml, 0)
+			return err
+		}, `"xmlBytes"=`, `"xml"="<volume`},
+		{"DomainGetMetadata", func(api libvirtAPI) error {
+			_, err := api.DomainGetMetadata(dom, int32(libvirt.DomainMetadataElement), libvirt.OptString{metadataURI}, 0)
+			return err
+		}, `"metadataBytes"=`, `"metadata"="<instance`},
+		{"DomainInterfaceAddresses", func(api libvirtAPI) error {
+			_, err := api.DomainInterfaceAddresses(dom, uint32(libvirt.DomainInterfaceAddressesSrcArp), 0)
+			return err
+		}, `"interfaces"=1`, `"addresses"=["vnet0=192.168.1.50/24"]`},
+		{"ConnectListAllDomains", func(api libvirtAPI) error {
+			_, _, err := api.ConnectListAllDomains(1, 0)
+			return err
+		}, `"count"=2`, `"names"=[`},
+	}
+	for _, verbosity := range []int{1, 2} {
+		for _, tc := range calls {
+			t.Run(tc.method+"/v"+string(rune('0'+verbosity)), func(t *testing.T) {
+				t.Parallel()
+				log, snapshot := captureLogger(verbosity)
+				api := &tracingAPI{inner: tracedHost(t), log: log}
+				if err := tc.call(api); err != nil {
+					t.Fatal(err)
+				}
+				joined := strings.Join(snapshot(), "\n")
+				if !strings.Contains(joined, tc.always) {
+					t.Errorf("v%d output missing %s:\n%s", verbosity, tc.always, joined)
+				}
+				if has := strings.Contains(joined, tc.detail); has != (verbosity >= 2) {
+					t.Errorf("v%d output contains %s = %v, want %v:\n%s", verbosity, tc.detail, has, verbosity >= 2, joined)
+				}
+			})
+		}
+	}
+}
+
+func TestTracingAPI_uploadLogsSizeNotContent(t *testing.T) {
+	t.Parallel()
+	const sentinel = "SENTINEL-seed-content-must-never-be-logged"
+	log, snapshot := captureLogger(2)
+	api := &tracingAPI{inner: tracedHost(t), log: log}
+	vol := libvirt.StorageVol{Pool: "default", Name: seedVolumeName(testName)}
+	if err := api.StorageVolUpload(vol, bytes.NewReader([]byte(sentinel)), 0, uint64(len(sentinel)), 0); err != nil {
+		t.Fatal(err)
+	}
+	joined := strings.Join(snapshot(), "\n")
+	if !strings.Contains(joined, `"length"=42`) {
+		t.Errorf("upload line missing the length:\n%s", joined)
+	}
+	if strings.Contains(joined, sentinel) {
+		t.Errorf("upload content leaked into logs:\n%s", joined)
+	}
+}
+
+func TestTracingAPI_silentBelowV1(t *testing.T) {
+	t.Parallel()
+	log, snapshot := captureLogger(0)
+	api := &tracingAPI{inner: tracedHost(t), log: log}
+	if _, err := api.DomainLookupByName(testName); err != nil {
+		t.Fatal(err)
+	}
+	if lines := snapshot(); len(lines) != 0 {
+		t.Errorf("verbosity 0 logged %d lines:\n%s", len(lines), strings.Join(lines, "\n"))
+	}
+}