Skip to content

Take guestbinary by ref instead of value#560

Closed
ludfjig wants to merge 1 commit intohyperlight-dev:mainfrom
ludfjig:take_guestbinary_by_ref
Closed

Take guestbinary by ref instead of value#560
ludfjig wants to merge 1 commit intohyperlight-dev:mainfrom
ludfjig:take_guestbinary_by_ref

Conversation

@ludfjig
Copy link
Copy Markdown
Contributor

@ludfjig ludfjig commented Jun 3, 2025

Depends on #559

Making Uninitialized::new take a GuestBinary by ref instead of value is more flexible, allowing reuse of the same guest binary across many different sandboxes. This PR allows passing either a ref or value, to be maximally flexible and not be a breaking change

@ludfjig ludfjig force-pushed the take_guestbinary_by_ref branch from af96a45 to 6698c2e Compare June 3, 2025 23:51
@ludfjig ludfjig added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Jun 3, 2025
dblnz
dblnz previously approved these changes Jun 4, 2025
Copy link
Copy Markdown
Contributor

@dblnz dblnz left a comment

Choose a reason for hiding this comment

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

Lgtm

Comment thread src/hyperlight_host/src/sandbox/uninitialized.rs Outdated
danbugs
danbugs previously approved these changes Jun 4, 2025
Copy link
Copy Markdown
Contributor

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

LGTM

@ludfjig ludfjig force-pushed the take_guestbinary_by_ref branch 3 times, most recently from 1b28b07 to 8ca12fc Compare June 9, 2025 17:41
jprendes
jprendes previously approved these changes Jun 9, 2025
@ludfjig ludfjig dismissed stale reviews from jprendes and danbugs via e2e1798 June 9, 2025 21:06
@ludfjig ludfjig force-pushed the take_guestbinary_by_ref branch 6 times, most recently from 94b2b19 to 02befa1 Compare June 9, 2025 23:55
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
@ludfjig ludfjig force-pushed the take_guestbinary_by_ref branch 2 times, most recently from 8692e1a to 6d8caed Compare June 10, 2025 00:34
@ludfjig ludfjig force-pushed the take_guestbinary_by_ref branch 2 times, most recently from f17d33d to 5174202 Compare June 10, 2025 17:59
@ludfjig
Copy link
Copy Markdown
Contributor Author

ludfjig commented Jun 16, 2025

I'm closing this for now, since #605 makes this PR non-trivial. The current state at least avoids copies of the guest binary since #559 has landed (but still requires a new struct for each sandbox - bad!) . Let's re-visit when we redesign the API.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants