qcow2-rs: fallocate via F_PUNCHHOLE on macOS#11
Merged
Conversation
On macOS the existing Qcow2IoTokio::fallocate path wrote zeros via pwrite, which preserves the reads-as-zero contract but does not release host extents. This commit replaces that fallback with a real fcntl(F_PUNCHHOLE, &fpunchhole_t) punch, available on macOS since 10.10 and behaviorally equivalent to fallocate(FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE) on Linux: file size unchanged, punched range reads as zero, allocated host extents released. APFS requires fp_offset and fp_length to be multiples of the volume block size (4096). Sub-block ranges return EINVAL, which the implementation catches alongside EOPNOTSUPP and ENOSYS and falls back to the pre-existing zero-write path so the reads-as-zero contract still holds — the host file just doesn't shrink for that one call. Mechanically the fd: i32 field on Qcow2IoTokio is now populated on macOS too (previously Linux-only); the new() cfg arms are merged accordingly. The Linux fallocate path and the non-Linux non-macOS fallback are unchanged. Assisted-by: Claude Opus 4.7 (1M context)
GHA Ubuntu runners' filesystem (likely overlay-backed) returns EOPNOTSUPP for `FALLOC_FL_PUNCH_HOLE | FALLOC_FL_ZERO_RANGE`, the combination `Qcow2OpsFlags::FALLOCATE_ZERO_RAGE` maps to. `Qcow2Dev::call_fallocate` already has a write-zeros fallback for exactly this case; the test was wrong to be strict. Now the test accepts the soft-fail path: on EOPNOTSUPP / "Operation not supported" / "Unsupported", skip the strict shrinkage assertion with an eprintln note. The read-as-zero contract is still asserted unconditionally — that's the user-facing guarantee on both paths. macOS test path unchanged (APFS supports F_PUNCHHOLE; the test gates strict shrinkage only when the punch reported success). Assisted-by: Claude Opus 4.7 (1M context)
sandrewh
added a commit
to sandrewh/qcow2-rs
that referenced
this pull request
May 14, 2026
Collaborator
|
Maybe something like below is needed for fixing the test failure? |
Follow-up to review feedback from @ming1 on PR ublk-org#11. The previous fix replaced the `.expect()` panic with a soft-fail skip, but `Qcow2IoTokio::fallocate` is the raw IO layer and has no write-zeros fallback (that lives in `Qcow2Dev::call_fallocate`). So on the EOPNOTSUPP path the range stayed the 0xAB prefill and the reads-as-zero assertion would correctly fail — the test was still broken, just at a different line. Now the not-punched branch replicates exactly what production does: write zeros to the range, then fsync. The reads-as-zero contract then genuinely holds on both the punch path and the fallback path. Assisted-by: Claude Opus 4.7 (1M context)
Contributor
Author
|
Thanks @ming1 — good catch, and you're exactly right. Applied your suggested approach in |
sandrewh
added a commit
to sandrewh/qcow2-rs
that referenced
this pull request
May 15, 2026
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.
qcow2-rs: fallocate via F_PUNCHHOLE on macOS
Summary
Replace the macOS fallback in
Qcow2IoTokio::fallocate(which previously wrote zeros viapwrite) with a real hole-punch usingfcntl(F_PUNCHHOLE, &fpunchhole_t). This makes discard semantics actually reclaim host file space on Darwin hosts, mirroring the existing Linuxfallocate(FALLOC_FL_PUNCH_HOLE)behavior.Motivation
Qcow2IoOps::fallocateis the file-level punch primitive thatQcow2Devcalls (viacall_fallocate) when it wants to zero a range AND release the host extents. On Linux this works as intended. On macOS the existing implementation intokio_io.rsfalls back to writing zeros viapwrite, which preserves the reads-as-zero contract but doesn't shrink the file — wasting disk space when callers discard large regions.F_PUNCHHOLEhas been available on macOS since 10.10 (Yosemite, 2014). It's the canonical macOS analogue of Linux'sFALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE.Behavior
fcntl(F_PUNCHHOLE, &fpunchhole_t). File size unchanged, punched range reads as zeros, allocated host extents released.fp_offsetandfp_lengthto be multiples of the volume block size (4096). Sub-block ranges returnEINVAL. The implementation catchesEINVAL/EOPNOTSUPP/ENOSYSand falls back to the existing zero-write path so the reads-as-zero contract still holds — the host file just doesn't shrink for that one call.Tests
Three new tests in
tests/fallocate.rs, platform-gated:fallocate_punch_hole_shrinks_st_blocks_on_linuxfallocate(PUNCH_HOLE)shrinksst_blocks; logical file size unchanged; punched range reads as zero; bytes outside untouchedfallocate_punch_hole_shrinks_st_blocks_on_macosF_PUNCHHOLEon a 4-KiB-aligned rangefallocate_sub_block_range_falls_back_to_zero_write_on_macosReproduce
On macOS:
On Linux:
Out of scope (intentional)
Qcow2IoOpstrait methods or flags.Qcow2Devcluster-allocator semantics.Disclosure
Co-developed with Claude Code (Claude Opus 4.7, 1M context). I reviewed every line and can answer questions about the design or implementation.