Repository navigation
Support suspending and resuming Linux VMs - #1297
randvoorhies wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d2a3fcca1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| private func regenerateMachineIdentifier() throws { |
There was a problem hiding this comment.
When a Linux .tvm archive is imported more than once, or imported on a host that still has the exported VM, Import.swift only fixes MAC collisions; the new persisted VZGenericMachineIdentifier is copied unchanged from config.json. Since this helper is private and only the clone path can call it, those imported local Linux VMs remain suspendable with the same supposedly unique platform identifier, defeating the uniqueness invariant this change adds for save/restore. Please make this available to import and regenerate it when a local Linux identifier collision exists.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. I updated Import to regenerate the VZGenericMachineIdentifier when it collides with an existing local Linux VM. Regeneration also removes state.vzvmsave if a new identifier is minted, since that saved state is tied to the old identifier and cannot be restored using the new one.
I added an import test using distinct MAC addresses to isolate the machine-identifier collision. It verifies that the identifier changes, the MAC address remains unchanged, and the incompatible saved state is
removed.
By the way, I think the same collision risk may exist for macOS VMs. Darwin's ecid is a VZMacMachineIdentifier, and the clone and import paths appear to preserve it. I kept that preexisting behavior out of this PR, but it may be worth unifying the identity handling in a follow-up.
There was a problem hiding this comment.
I took a step back to think about why this would be any different for the Linux vs Darwin platforms, and ended up simplifying the whole PR to make the new behavior more consistent with how Darwin currently handles its ecid. Existing identifiers are now preserved just like Darwin, and the only Linux-specific clone behavior is initializing the missing identifier for a stopped legacy config.
I agree that Import should probably do the machineIdentity/ecid collision check, but it should be done consistently for Darwin and Linux and that felt out of scope for this PR.
0d2a3fc to
769f6a2
Compare
|
I'm very excited about getting this merged in. My org has a suite of playwright tests that run against a containerized stack of services running on a Linux host. Bringing that stack up and down to reset state for each test is expensive at around 30s per respin. Some initial experiments with code behind this new PR show that we can take a snapshot of a VM running our system, and then resume it in ~2s before each test. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 769f6a21be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if populateCache { | ||
| let expectedPushedVMDir = try ociStorage.create(pushedRemoteName) | ||
| try localVMDir.clone(to: expectedPushedVMDir, generateMAC: false) | ||
| try localVMDir.clone(to: expectedPushedVMDir, generateMAC: false, generateMachineIdentifier: false) |
There was a problem hiding this comment.
When --populate-cache is used after pushing a suspended Linux VM, this cache clone still copies state.vzvmsave, even though pushToRegistry only uploads config, disk, and NVRAM. A later tart clone <that ref> can then open the populated OCI cache, see the cached VM as .Suspended, preserve the same Linux machine identifier, and produce a local clone that restores a snapshot that was never part of the pushed image; strip the saved state from the pushed-image cache (or clone without it) before linking the ref.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, but that's really an issue that pre-dates this PR and also applies to Darwin. I removed all of my changes to Push and Clone (other than the new machineIdentifier generation for legacy configs) so this PR doesn't change that behavior at all.
Similar to the other comment, this is probably a good idea to fix consistently for both platforms but I think it's out of scope for the current PR.
769f6a2 to
01d9359
Compare
|
Would be great to get this in |
|
@randvoorhies can you rebase and I will review |
|
@yzhuang-oai yup no problem I'll get to it in the next few days |
01d9359 to
6b6a326
Compare
|
Ok @yzhuang-oai this MR is rebased and should be good to go. |
|
|
||
| if sourceState != .Suspended, | ||
| let linux = sourceConfig.platform as? Linux, | ||
| linux.machineIdentifier == nil { |
There was a problem hiding this comment.
with the check linux.machineIdentifier == nil
for a stopped VM that has an identifier, it would keep the identifier in its config, and the clone above would copy its identifier to the clone and would not initialize a new identifier here because linux.machineIdentifier == nil is false
this would result in 2 VMs having the same identifier (the stopped one and the cloned one).
There was a problem hiding this comment.
@yzhuang-oai almost done with this - it's been a busy week at my day job 😅
I was aiming to mirror the behavior of the Darwin ecid handling (see my prior responses to the AI reviewer). I think you're right though that stopped clones shouldn't share an id, so I'm refactoring to instead handle this machineIdentifier more like how you all handle MACs. I should have it done by this weekend.
|
@randvoorhies can you check the review comment? almost ready to merge. |
Restoring saved state requires the VZGenericMachineIdentifier that the
VM was saved with, but Linux VMs were getting a new one every time they
started (including when trying to restore from saved state). The
identifier is now persisted in the VM config and reused on every start.
Apple's SDK header for VZGenericPlatformConfiguration.machineIdentifier
says:
> Running two virtual machines concurrently with the same identifier
> results in undefined behavior in the guest operating system. When
> restoring a virtual machine from saved state, this `machineIdentifier`
> must match the `machineIdentifier` of the saved virtual machine.
So identifiers for Linux guests are now treated like MAC addresses:
- Cloning a non-suspended VM generates a new id for the clone.
- Suspended clones keep their id because their saved state requires
it.
- Running a VM whose id matches an already running VM generates a new
id.
- If the colliding VM being run was suspended, its saved state will
be discarded.
Configurations without an identifier will still load, but can't be
suspended.
Suspendable mode for Linux VMs omits the pointing device because
VZUSBScreenCoordinatePointingDeviceConfiguration makes restores fail.
6b6a326 to
b01bd45
Compare
|
@yzhuang-oai updated and rebased onto the latest
I had originally wanted to follow how Running several VMs with the same |
This branch has not been deployed
3 participants
Fixes #1177
Summary
Previous investigation in #796 and #1177 implied that restoring suspended Linux VMs wasn't supported by Virtualization.framework. Some testing on current macOS shows that Linux restoration does actually work properly, and that Tart's existing suspend/restore mechanisms were very close to working.
The
Darwinplatform implementation persists the VM's ECID and assigns it toVZMacPlatformConfiguration.machineIdentifier. However, theLinuxplatform didn't specify amachineIdentifierwhen constructing itsVZGenericPlatformConfiguration, which caused a new identifier to be generated each time the platform was instantiated.Unfortunately, Virtualization.framework returns a very opaque error if saved machine state is restored with a mismatched identifier:
This PR generates and persists a
VZGenericMachineIdentifierin new Linux VM config files, then reuses it whenever the platform is constructed. The goal is to get the Linux behavior as close as possible to the existing Darwin behavior (with the exception of handling legacy config files with no machineIdentifier). Future work could probably align these more closely, but I'd like to keep this PR as minimal as I can.Now, existing Linux configs without a
machineIdentifierare loadable for normal use but can't be suspended. When cloning a stopped legacy Linux VM, the missingmachineIdentifieris initialized in the new clone. Existing identifiers are preserved, matching Tart's current behavior for Darwin ECIDs, and suspended clones keep their original identity and saved state.The
Linuxplatform now conforms to PlatformSuspendable. Suspendable Linux VMs retain USB keyboard support but omit VZUSBScreenCoordinatePointingDeviceConfiguration, which passes validateSaveRestoreSupport() but causes restoration to fail.Testing
The matching boot ID and PID with a continuously incrementing counter confirm that saved machine state was restored rather than cold-booted.