Skip to content

Unify block chunking across v1 and v2 in copy_block#803

Open
demoray wants to merge 2 commits into
mainfrom
bcaswell/unify-block-chunking
Open

Unify block chunking across v1 and v2 in copy_block#803
demoray wants to merge 2 commits into
mainfrom
bcaswell/unify-block-chunking

Conversation

@demoray
Copy link
Copy Markdown
Collaborator

@demoray demoray commented May 18, 2026

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.

Change

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:

pub fn copy_block(&mut self, range: Range<u64>) -> Result<()> {
    let mut start = range.start;
    while start < range.end {
        let end = range.end.min(
            start.checked_add(MAX_BLOCK_SIZE).ok_or(Error::TooLarge)?,
        );
        self.copy_if_nonzero(start..end)?;
        start = end;
    }
    Ok(())
}

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/iomem are not contiguous on real
hardware). 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.
  • avml-convert v1↔v2 sparse roundtrip tests
    (convert_sparse_raw_between_lime_and_compressed_formats,
    trailing_zero_block_is_dropped_from_raw_roundtrip) —
    encode_raw_image already pre-chunks before calling
    copy_block, so each call is a single iteration of the new
    loop.

Documented the 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

demoray added 2 commits May 18, 2026 21:14
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
@demoray demoray enabled auto-merge (squash) May 18, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant