Unify block chunking across v1 and v2 in copy_block#803
Open
demoray wants to merge 2 commits into
Open
Conversation
copy_block previously pre-chunked v2 ranges to MAX_BLOCK_SIZE before dispatching to copy_block_impl, while v1 dispatched the whole range as-is. copy_block_impl then split on size: ranges larger than MAX_BLOCK_SIZE went to copy_large_block (streaming, no zero-elision), smaller ranges went to copy_if_nonzero (buffered, zero-elided). Because v2 always pre-chunked to <= MAX_BLOCK_SIZE, copy_large_block was unreachable for v2 and only fired on v1 with iomem ranges > 16 MiB. The asymmetry had two consequences: 1. v1 silently lost zero-elision on any contiguous iomem range above 16 MiB. v2 always got it. 2. copy_large_block looked dead to readers — `if version == 1` was the only path that reached it, and that fact was not obvious from the call graph. Move the chunking into copy_block and apply it to both versions. copy_block_impl and copy_large_block are deleted; copy_if_nonzero becomes the single inner step. v1 large ranges now produce multiple LiME records in the output (the format is a sequence of records walked in order, and existing v1 output already contains gaps between iomem ranges, so readers that handle the existing gaps handle the new ones the same way). All existing tests pass without modification: - image::tests::copy_block_skips_all_zero_ranges (small range, one loop iteration) - avml-convert v1<->v2 sparse roundtrip tests (already pre-chunk before calling copy_block) Documented the v1 wire-format change in the copy_block doc comment. Verified with: - cargo fmt --check - cargo clippy --all-features --all-targets -- -D warnings - cargo test --all-features
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.
copy_blockpreviously pre-chunked v2 ranges toMAX_BLOCK_SIZEbefore dispatching to
copy_block_impl, while v1 dispatched thewhole range as-is.
copy_block_implthen split on size: rangeslarger than
MAX_BLOCK_SIZEwent tocopy_large_block(streaming,no zero-elision), smaller ranges went to
copy_if_nonzero(buffered, zero-elided). Because v2 always pre-chunked to
<= MAX_BLOCK_SIZE,copy_large_blockwas unreachable for v2 andonly fired on v1 with iomem ranges > 16 MiB.
The asymmetry had two consequences:
above 16 MiB. v2 always got it.
copy_large_blocklooked dead to readers —if version == 1was the only path that reached it, and that fact was not
obvious from the call graph.
Change
Move the chunking into
copy_blockand apply it to both versions.copy_block_implandcopy_large_blockare deleted;copy_if_nonzerobecomes the single inner step:Wire-format change for v1
v1 acquisitions of a single contiguous iomem range > 16 MiB now
produce multiple LiME records in the output instead of one,
and intra-range gaps appear where chunks were entirely zero.
The LiME format is a sequence of records walked in order, and
existing v1 output already contains gaps between iomem ranges
(System RAM ranges in
/proc/iomemare not contiguous on realhardware). Readers that handle the existing inter-range gaps will
handle the new intra-range gaps the same way. Tools that assume a
strict one-record-per-iomem-range mapping — if any exist — will
see a behavior change.
Worth flagging in release notes.
Compatibility checks
All existing tests pass without modification:
image::tests::copy_block_skips_all_zero_ranges— small range,one loop iteration; unchanged behavior.
(
convert_sparse_raw_between_lime_and_compressed_formats,trailing_zero_block_is_dropped_from_raw_roundtrip) —encode_raw_imagealready pre-chunks before callingcopy_block, so each call is a single iteration of the newloop.
Documented the wire-format change in the
copy_blockdoc comment.Verified with:
cargo fmt --checkcargo clippy --all-features --all-targets -- -D warningscargo test --all-features