Skip to content

Add cpu vendor to snapshots#1587

Merged
ludfjig merged 1 commit into
hyperlight-dev:mainfrom
ludfjig:cpu-vendor
Jun 26, 2026
Merged

Add cpu vendor to snapshots#1587
ludfjig merged 1 commit into
hyperlight-dev:mainfrom
ludfjig:cpu-vendor

Conversation

@ludfjig

@ludfjig ludfjig commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Error on loading intel snapshots on amd and vice versa (tried my best for aarch64 but not sure if correct please review). The goal is to relax this requirement later, but will need more investigation.

@ludfjig ludfjig added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Jun 26, 2026
@ludfjig ludfjig marked this pull request as ready for review June 26, 2026 07:21
Copilot AI review requested due to automatic review settings June 26, 2026 07:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens snapshot portability rules by recording the host CPU vendor in the OCI snapshot config (and mirroring it into the manifest descriptor annotations) so snapshots taken on one CPU vendor (e.g., Intel) are rejected when loaded on a different vendor (e.g., AMD), reducing the risk of resuming incompatible CPU state.

Changes:

  • Add cpu_vendor to the snapshot config and validate it at load time.
  • Emit a new dev.hyperlight.snapshot.cpu.vendor advisory annotation on the manifest descriptor.
  • Update documentation and add a test to ensure vendor mismatches are rejected.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/hyperlight_host/src/sandbox/snapshot/file/mod.rs Writes the CPU vendor annotation and includes cpu_vendor in the saved config.
src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs Adds the new CPU vendor annotation key and updates annotation docs.
src/hyperlight_host/src/sandbox/snapshot/file/config.rs Introduces CpuVendor and enforces CPU vendor matching during load validation.
src/hyperlight_host/src/sandbox/snapshot/file_tests.rs Adds a unit test asserting vendor mismatches are rejected.
docs/snapshot-oci-format.md Documents the new CPU vendor binding and annotation.

Comment thread src/hyperlight_host/src/sandbox/snapshot/file/config.rs
Comment thread src/hyperlight_host/src/sandbox/snapshot/file/config.rs
dblnz
dblnz previously approved these changes Jun 26, 2026

@dblnz dblnz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jprendes jprendes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I think the copilot comment is on point, we don't want to accidentally crash on Windows on ARM.

@danbugs danbugs added the ready-for-review PR is ready for (re-)review label Jun 26, 2026

@danbugs danbugs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, but I think you still need to address the copilot review. Plus—what are your thoughts on providing an escape hatch on this requirement? For example, a skip_cpu_vendor_check option (or similar) on the load path that defaults to false but lets callers explicitly opt out when they've validated cross-vendor compatibility for their use case. That way the default is strict, but users aren't hard-blocked if they know what they're doing—and it gives us a natural knob to flip when we're ready to relax the restriction across the board.

@ludfjig ludfjig force-pushed the cpu-vendor branch 3 times, most recently from 1f80796 to 1f069c2 Compare June 26, 2026 17:34
@ludfjig

ludfjig commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Mostly LGTM, but I think you still need to address the copilot review. Plus—what are your thoughts on providing an escape hatch on this requirement? For example, a skip_cpu_vendor_check option (or similar) on the load path that defaults to false but lets callers explicitly opt out when they've validated cross-vendor compatibility for their use case. That way the default is strict, but users aren't hard-blocked if they know what they're doing—and it gives us a natural knob to flip when we're ready to relax the restriction across the board.

I don't think I am a fan for this release. We should just do the work to allow it (or not) and fix it and allow it

…ding

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
@ludfjig ludfjig merged commit 83f01c4 into hyperlight-dev:main Jun 26, 2026
82 of 96 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. ready-for-review PR is ready for (re-)review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants