fix(hypervisors): correct memory unit conversion and validation#826
Open
Nachiket-Roy wants to merge 1 commit into
Open
fix(hypervisors): correct memory unit conversion and validation#826Nachiket-Roy wants to merge 1 commit into
Nachiket-Roy wants to merge 1 commit into
Conversation
Update various VMM backends (QEMU, Firecracker, HVT, SPT, and Cloud Hypervisor) to correctly convert and validate memory sizes to their expected units (bytes/MiB) instead of MB. - QEMU and Cloud Hypervisor now use raw bytes. - Firecracker, HVT, and SPT now use MiB. - Low memory values (<1 MiB) for Solo5 (HVT/SPT) and Firecracker are clamped to 1 MiB instead of falling back to default 256 MiB. - Replace utility BytesToStringMB with BytesToBString and BytesToMiBString, and update tests accordingly. Fixes: urunc-dev#818 Fixes: urunc-dev#819 Signed-off-by: Nachiket Roy <nachiket.roy.2@gmail.com>
👷 Deploy Preview for urunc processing.
|
✅ Deploy Preview for urunc canceled.
|
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.
Description
This PR fixes the memory handling issues reported in #818 and #819 by replacing the previous global conversion with monitor-specific handling.
Previously, OCI memory limits were converted using decimal MB for all monitors, which could cause the guest to receive more memory than requested. Additionally, requests that rounded to
0were treated as unspecified and silently replaced withDefaultMemory(256MB).This PR updates each backend to use the most accurate representation it supports:
-m <bytes>B).--memory size=<bytes>).1 MiBto the minimum representable value.1 MiBto satisfy its CLI validation.An unspecified (
0) memory request continues to fall back toDefaultMemory, while valid non-zero requests are preserved as accurately as each monitor allows.Related issues
How was this tested?
go test -v ./pkg/....LLM usage
Claude (Anthropic) was used to discuss implementation trade-offs. All implementation details and behavior were independently verified through source inspection, documentation, and runtime testing.
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).