One definition of the machine, imported instead of spelled again - #81
Merged
Conversation
spinbox kept its own copy of what a VM is: a command-line builder, a kernel
command line, and a MachineIdentity beside them that spelled the same four
arguments a second time so a template's fingerprint could be computed before
there was a command line to read. The two were kept in step by a shared
machineShape function and a comment asking the next person not to break it,
because nothing checks a restore at run time — a template loaded into a machine
of another shape is memory and device state going into hardware it did not come
from, and it does not fail, it misbehaves.
There is one machine.Spec now. The command line is Args() of it and the
fingerprint is Fingerprint() of it, so the two cannot disagree because there is
nothing left to disagree.
The two definitions had already drifted, in both directions. Ours hashed
runtime.GOARCH, which cannot differ without the QEMU binary differing. It hashed
the host CPU unconditionally, which is right under `-cpu host` and wrong the
moment a model is named — it would partition templates per machine exactly when
the point of naming one is that they cross machines. And it did not hash the
device list at all, so adding the balloon left every existing template matching
a machine it could no longer be restored into.
**Memory grows through virtio-mem.** The `-m` line has no slots any more, so the
pc-dimm path went with them: a backend object and a device per step, a table of
which of the eight slots were in use, LIFO ordering so unplug took the newest, an
RPC into the guest to online what arrived and another to offline what was
leaving, and a rollback for each of the four ways that could half-fail. One
qom-set replaces it. The guest onlines by itself (memhp_default_state=online) and
shrinking is the same call with a smaller number. What the device reports is read
back rather than assumed: a request is a negotiation, and recording the number we
asked for would leave the controller believing in memory the VM does not have.
**Every disk is a qcow2.** The format was worked out from the file extension
while the command line was being built — a guess in the one place nobody looks.
There is one format because there is one thing that produces them: a chain with a
read-only base many VMs map at once and a read-write tip that is this VM's.
**One layout.** The release tree is the layout everywhere now, so hack/spin-machine
copies instead of rearranging, and internal/paths stops guessing: no candidate
lists ending in /usr/bin, no second kernel name. A host with no release used to
run *a* QEMU, with different devices and a different fingerprint, and find out by
way of a guest that would not start.
Deleted rather than adapted: qemu_command_test.go and the fingerprint and
machineShape tests, which now test machine.Spec upstream and better; and
qmp_memory_test.go, whose every test was either a 128 MB alignment rule that no
longer exists or a t.Skip("manual integration test") that has never run.
Verified: task lint, go test -race ./..., and hack/spin-machine against a sibling
checkout.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a
go.mod carried spin-machine as indirect while three files import it; `go mod tidy` says what is true. The pin is a branch commit again and not a tag, for the reason the last one was: spin-stack/spin-machine#2 carries Spec.Identity, which this repository's template cache keys on, and which the merge of #1 did not take. It moves to a release once that lands. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…t assumed
Hard-coding qcow2 was wrong today. It is where the disks are going — one chain,
a read-only base many VMs map and a read-write tip that is this VM's — and it is
not where they come from: containerd's snapshotter hands this shim a merged.vmdk
and a raw rwlayer.img, and QEMU refused the first of them outright.
qemu-system-x86_64: -drive file=…/merged.vmdk,…,format=qcow2:
Image is not in qcow2 format
The guess it replaced was no better — the format was worked out from the file
extension while the command line was being built, by code that had been handed
the answer and dropped it: `MountConfig.Vmdk` was already set by the one party
that knows, and `addDisk` ignored it.
So the option says the format instead of saying a format's name: WithFormat
replaces WithVmdk, DiskConfig carries it, and vm.DefaultDiskFormat is what a disk
is when nobody says otherwise. That constant is raw, and turning it into qcow2 is
the whole of the change when the disks stop coming from a snapshotter.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a
A rename left template_build_integration_test.go calling a function that no longer existed. `task lint` was green and `go test ./...` was green, because neither compiles what is behind a build tag; the break surfaced in CI as `[build failed]` after five minutes of setting up a VM — the slowest lane in the repository reporting the fastest kind of error there is. `go vet -tags=integration,e2e` beside the linter. Not golangci-lint with the tags: the break to catch is a build break, and turning the full linter loose on files it has never seen reports seven pre-existing style findings that belong to nobody running this today. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
spinbox kept its own copy of what a VM is: a command-line builder (402 lines), a kernel command line (257), and a
MachineIdentitybeside them that spelled the same four arguments a second time so a template's fingerprint could be computed before there was a command line to read. The two were kept in step by a sharedmachineShapeand a comment asking the next person not to break it — because nothing checks a restore at run time. A template loaded into a machine of another shape is memory and device state going into hardware it did not come from, and it does not fail, it misbehaves.There is one
machine.Specnow. The command line isArgs()of it and the fingerprint isFingerprint()of it, so the two cannot disagree because there is nothing left to disagree.Net −2,361 lines.
The two definitions had already drifted, in both directions
runtime.GOARCH, which cannot differ without the QEMU binary differing.-cpu host, wrong the moment a model is named, where it would partition templates per machine exactly when the point of naming one is that they cross machines.Memory grows through virtio-mem
The
-mline has noslots=any more, so the pc-dimm path went with them: a backend object and a device per step, a table of which of the eight slots were in use, LIFO ordering so unplug took the newest, an RPC into the guest to online what arrived and another to offline what was leaving, and a rollback for each of the four ways that could half-fail. Oneqom-setreplaces all of it — the guest onlines by itself (memhp_default_state=online) and shrinking is the same call with a smaller number.What the device reports is read back rather than assumed. A request is a negotiation: virtio-mem plugs as the guest accepts and unplugs only what the guest has released, so recording the number we asked for would leave the controller believing in memory the VM does not have, and every threshold after that computed against it. There is a test for exactly that.
Every disk is a qcow2
The format was worked out from the file extension while the command line was being built — a guess in the one place nobody looks, and a wrong one is a guest that boots and finds a disk full of nothing. There is one format because there is one thing that produces them: a chain with a read-only base many VMs map at once and a read-write tip that is this VM's.
One layout
The release tree is the layout everywhere, so
hack/spin-machinecopies instead of rearranging, andinternal/pathsstops guessing — no candidate lists ending in/usr/bin, no second kernel name, nodiscoverQemuPath. A host with no release used to run a QEMU, with different devices and a different fingerprint, and find out by way of a guest that would not start. Config validation now opens the release, so a host missing one of the four files is told which one, once.Deleted rather than adapted
qemu_command_test.goand the fingerprint/machineShapetests: they testmachine.Specupstream now, against more properties than these did.qmp_memory_test.go: every test in it was either a 128 MB alignment rule that no longer exists or at.Skip("manual integration test")that has never run.Verified
task lint(0 issues),go test -race ./..., andhack/spin-machineagainst a sibling checkout.Before merging
go.modpins spin-machine at a branch commit — this needs spin-stack/spin-machine#1, and the pin must be moved to a tagged release once that merges. This invalidates every template on every host, which is what changing the machine means; nothing in production consumes one yet.The
Systemservice still carriesOnlineMemory/OfflineMemoryand the guest still implements them. Nothing on the host calls them any more; removing them is a proto change and belongs in its own commit.🤖 Generated with Claude Code
https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a