disklayer_ram: add configurable sector size support#2927
Open
jstarks wants to merge 4 commits intomicrosoft:mainfrom
Open
disklayer_ram: add configurable sector size support#2927jstarks wants to merge 4 commits intomicrosoft:mainfrom
jstarks wants to merge 4 commits intomicrosoft:mainfrom
Conversation
Add support for non-512-byte sector sizes (e.g., 4096) to RamDiskLayer, which was previously hardcoded to 512-byte sectors. Changes: - Replace fixed Sector([u8; 512]) with Box<[u8]> per sector - Add sector_size and sector_shift fields to RamDiskLayer - Add new_with_sector_size() constructor; existing new() defaults to 512 - Use bitwise shifts for sector/byte conversions in all IO paths - LazyRamDiskLayer now inherits sector_size from the lower layer - Add sector_size field to RamDiskLayerHandle and wire through resolver - Add ram_disk_with_sector_size() convenience function - Add tests for 4096-byte sectors, validation, and lazy inheritance All existing callers are backward-compatible (sector_size: None = 512). Closes microsoft#1492 Supersedes microsoft#1892
Contributor
There was a problem hiding this comment.
Pull request overview
Adds configurable (power-of-two) sector-size support to the disklayer_ram RAM-backed disk layer, enabling 4K-sector ramdisks and propagating sector size through the resource handle/resolver stack for layered disks.
Changes:
- Extend
RamDiskLayer/LazyRamDiskLayerto support configurable sector sizes (defaulting to 512) and update IO math/storage accordingly. - Extend
RamDiskLayerHandlewith an optionalsector_sizeand update relevant call sites to populate the new field. - Add unit tests covering 4K read/write behavior, sector count calculation, and invalid sector-size validation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/x86_64/storage.rs | Updates ramdisk handle construction to include the new sector_size field. |
| vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs | Updates some ramdisk handle constructions for the new sector_size field (but one remains missing). |
| vm/devices/storage/disklayer_ram/src/resolver.rs | Resolver now always returns a LazyRamDiskLayer configured from handle fields (len/sector_size). |
| vm/devices/storage/disklayer_ram/src/lib.rs | Implements configurable sector sizing, updates read/write/unmap logic, adds ram_disk_with_sector_size, and adds tests. |
| vm/devices/storage/disk_backend_resources/src/layer.rs | Adds sector_size: Option<u32> to RamDiskLayerHandle with documentation. |
| petri/src/vm/openvmm/mod.rs | Updates RAM disk handle literals to include sector_size: None. |
| openvmm/openvmm_entry/src/lib.rs | Updates RAM disk handle literals to include sector_size: None. |
You can also share your feedback on Copilot code review. Take the survey.
mattkur
reviewed
Mar 10, 2026
| } | ||
|
|
||
| #[async_test] | ||
| async fn test_4096_sector_write_read() { |
Contributor
There was a problem hiding this comment.
@copilot: remind me, where is RMW handled (when the app wants to issue a 512B read to this device). Do we propagate this sector size all the way up to the guest?
(I'm curious to see if I can invoke copilot to reason for me, in pr comments)
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
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.
Add support for non-512-byte sector sizes (e.g., 4096) to RamDiskLayer, which was previously hardcoded to 512-byte sectors.
Closes #1492
Supersedes #1892