Conversation
ming1
commented
Jun 10, 2026
ming1
commented
Collaborator
- big refactor
- small bug fixes
Update CLAUDE.md to match the current code: document the four-method Qcow2IoOps trait, the discard/fallocate/drop test suites, and a new "Device Setup & Public API" section covering Qcow2DevParams, the qcow2_setup_dev_* helpers, automatic backing-chain resolution, and the public Qcow2Dev methods. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Remove byteorder (no references anywhere in the crate) and clap_derive (clap is already pulled with the "derive" feature, which re-exports Parser/Subcommand/Args). Also de-duplicate tempfile, which was declared in both [dependencies] and [dev-dependencies]; it stays in [dependencies] since the public make_temp_qcow2_img helper uses it. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
The sync, tokio and io_uring backends each carried byte-for-byte copies of the Linux hole-punch flag mapping + nix fallocate call, the O_DIRECT fcntl, and the zero-write discard fallback. Hoist these into ops.rs as linux_punch_hole(), set_direct_io() and zeroed_io_buf(), so each backend impl is now a thin delegate. The macOS F_PUNCHHOLE soft-fail path in the tokio backend keeps its specialized logic and only reuses zeroed_io_buf. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Qcow2DevParams stored read_only and backing behind RefCell, but the only mutation site (the setup helpers) already clones the params before calling mark_backing_dev, so plain fields with &mut self setters work and remove the runtime borrow checking. Likewise Qcow2IoSync held its File in a RefCell even though the field is only an ownership anchor keeping the fd valid; a plain File suffices. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Qcow2DevParams::set_read_only and mark_backing_dev changed receiver from &self to &mut self, dropping interior mutability. That is a source-level breaking change: callers that mutated params through a shared reference or an immutable binding no longer compile. Cargo treats "0.1" as ^0.1, so a 0.1.x release would silently break downstream consumers on update; bump the minor to 0.2.0 to make the break opt-in. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Mechanical, behavior-preserving idiom modernization flagged by clippy: inline format args, manual (a+b-1)/b -> u64::div_ceil, io::Error::new (ErrorKind::Other, _) -> io::Error::other, drop a needless as_bytes(), single-arm match -> if let, and == false -> negation. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
clippy::repr_packed_without_abi flags bare #[repr(packed)], which defaults to #[repr(Rust, packed)] and has no stable ABI. Qcow2RawHeader and Qcow2HeaderExtensionHeader are (de)serialized field-by-field via bincode, so struct memory layout does not define the on-disk format; making the existing layout explicit as #[repr(Rust, packed)] silences the lint with zero behavior change. (repr(C, packed) is deliberately avoided, as that would reorder fields.) Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Reformat lines left non-canonical by earlier edits (a clippy one-liner, a struct literal that now fits one line) and a stale import order in the basic test. Formatting only, no behavior change. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move src/meta.rs to src/meta/mod.rs and split the Table/TableEntry traits plus the table-impl macros into a new meta/table.rs submodule. The macros are re-exported with pub(crate) use and imported back into mod.rs (all of them, since impl_table_traits! expands to unqualified calls of the helper macros that must resolve at the invocation site). No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move the guest-offset address-splitting type out of meta/mod.rs into a dedicated meta/addr.rs submodule, re-exported from mod.rs. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move Qcow2RawHeader, the header-extension types, the feature-type enum and Qcow2Header (with its parse/format/serialize logic) into a new meta/header.rs submodule, re-exported from mod.rs. The remaining table types stay in mod.rs and header.rs pulls the refcount table types it needs for format_qcow2 from super. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move L1Entry, L1Table, their trait impls and the test_l1_table unit test into a new meta/l1.rs submodule. The test moves with the code because it constructs L1Entry via its private tuple field, which is only reachable from within the type's own module. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move L2Entry, Mapping, MappingSource, L2Table, their trait impls and the test_l2_table unit test into a new meta/l2.rs submodule. The test moves with the code because it reads L2Table's private cluster_bits field. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move RefTableEntry, RefTable, RefBlockEntry, RefBlock, their trait impls and the three refcount unit tests into a new meta/refcount.rs submodule. test_refcount_block2 moves with the code because it builds RefBlockEntry via its private tuple field. meta/mod.rs is now reduced to module declarations and re-exports. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Relocate the format/parse round-trip unit test next to the Qcow2Header code it exercises. meta/mod.rs is now purely module declarations and re-exports. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move src/dev.rs to src/dev/mod.rs and split the standalone Qcow2Info and Qcow2DevParams data types into a new dev/info.rs submodule, re-exported from mod.rs. Two Qcow2Info helper methods used by the allocator (rb_slice_entries, is_read_only) are widened to pub(crate) since they are now accessed across the module boundary. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move HostCluster and the refcount-based cluster allocator (grow_reftable, free_clusters, refblock cache helpers, allocate_clusters and friends) plus the three allocator unit tests into a new dev/alloc.rs submodule as an impl extension of Qcow2Dev. Methods and HostCluster called across the new module boundary are widened from private to pub(crate); this is the mechanical cost of splitting the single Qcow2Dev impl block. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move the read/mapping path (get_mapping, get_l2_entry/entries, the do_read_* helpers and read_at) into a new dev/read.rs submodule as an impl extension of Qcow2Dev. do_read_compressed and get_l2_entry are widened to pub(crate) since the COW write path and checker call them. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move the write path (ensure_l2_offset, the COW helpers, write-mapping construction and write_at) into a new dev/write.rs submodule as an impl extension of Qcow2Dev. No visibility changes were needed: it calls the already-pub(crate) read/alloc helpers and the parent's private cache kernel, and exposes only the existing pub write_at. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
The grow_reftable doc comment was left behind in mod.rs when the allocator was extracted; reunite it with the function it describes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move discard and __discard_one_cluster into a new dev/discard.rs submodule as an impl extension of Qcow2Dev. No visibility changes needed. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move the cluster-usage accounting (qcow2_cluster_usage and its add_*_clusters helpers) and the integrity checker (check and its check_*/cluster_is_allocated helpers) into a new dev/check.rs submodule as an impl extension of Qcow2Dev. No visibility changes needed. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Move the IO-op wrappers (call_*), table loaders, L2-slice cache management and the soft-update flush logic (flush_*, flush_meta) into a new dev/cache.rs submodule as an impl extension of Qcow2Dev. The kernel methods invoked by the read/write/alloc/check/discard submodules are widened to pub(crate); call_read keeps its #[inline] and mark_need_flush keeps #[inline(always)]. dev/mod.rs is now just the struct, constructor, lifecycle helpers and prep/flush flags. No logic changes. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
The allocator tests (alloc.rs) and the io test (mod.rs) each inherited the full test-module import block; drop the imports each no longer uses. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
The blocks_before/after_write and blocks_after_discard bindings in discard_releases_host_cluster feed only #[cfg(unix)] assertions, so they were unused (and warned) on non-unix targets. Gate the let bindings with #[cfg(unix)] to match their asserts; host_blocks already has a not(unix) stub. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Reflect the extraction of the monolithic src/dev.rs and src/meta.rs into the src/dev/ and src/meta/ submodule trees. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
These pre-existing issues became visible while reviewing the relocated code; none are regressions from the refactor itself. - meta/refcount: __get/__set shifted 2/4-bit refcount entries by the entry index instead of its in-byte bit offset, causing entries to alias. Shift by (index % 4) * 2 and (index % 2) * 4 respectively. - ops: FALLOC_FL_PUNCH_HOLE|FALLOC_FL_ZERO_RANGE is an invalid combo and PUNCH_HOLE requires KEEP_SIZE, so both branches returned EINVAL. Use ZERO_RANGE|KEEP_SIZE and PUNCH_HOLE|KEEP_SIZE; also fix the FALLOCATE_ZERO_RAGE -> FALLOCATE_ZERO_RANGE typo across callers. - dev/read: clamp EOF-straddling reads to the in-image length (vsize - offset) rather than the past-EOF overflow, and replace the unreachable i-1 == res.len() check with i == res.len() - 1. - dev/write: propagate allocate_clusters/allocate_cluster errors with ? instead of unwrap()/expect() that panic on Ok(None). Signed-off-by: Ming Lei <tom.leiming@gmail.com>
The prior fix mapped FALLOCATE_ZERO_RANGE to FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE, which only zeroes the range. On filesystems that don't deallocate zeroed extents (e.g. overlayfs on GitHub CI) the call succeeds without freeing blocks, so discard stops reclaiming space and the fallocate_punch_hole_shrinks_st_blocks test fails (before == after). Use FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, which deallocates the range and reads back as zero, satisfying both the discard and reads-as-zero contracts and matching the macOS F_PUNCHHOLE backend. Unlike ZERO_RANGE, PUNCH_HOLE either frees blocks or errors, so it never silently succeeds without reclaiming space. Signed-off-by: Ming Lei <tom.leiming@gmail.com>
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.