Conversation
The translation of Linux's fallocate semantics to macOS was badly broken. While we can't fully preserve the original semantics, since macOS doesn't have posix_fallocate and F_PREALLOCATE doesn't allow to allocate arbitrary ranges, we can do something close enough to work fine for the vast majority of guest applications. Signed-off-by: Sergio Lopez <slp@redhat.com>
slp
requested review from
MatiasVara,
dorindabassey,
jakecorrenti,
mtjhrc and
tylerfanelli
as code owners
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request significantly improves the fallocate semantics for macOS by correctly translating Linux fallocate flags to their closest macOS equivalents. The implementation now properly handles LINUX_FALLOC_FL_ALLOCATE_RANGE and LINUX_FALLOC_FL_PUNCH_HOLE flags, including the LINUX_FALLOC_FL_KEEP_SIZE modifier. Error handling for unsupported flags and invalid flag combinations is also correctly implemented, making the virtio-fs behavior more robust and aligned with Linux expectations on macOS.
|
@mtjhrc @dorindabassey @mz-pdm PTAL. If you can't test on macOS, a visual inspection is still appreciated. |
|
@slp I'm still getting acquainted with the codebase. I ran this through Claude, so take it with a grain of salt, but it poitned out the following:
|
| let new_length = (offset + length) as i64; | ||
|
|
||
| if keep_size { | ||
| // Check the number of allocate blocks instead of the file size. |
According to https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html, Claude is right. Other than that (and the possible typo in the comment), the PR looks good to me (but I know nothing about the topic, just using available documentation). |
3 participants
The translation of Linux's fallocate semantics to macOS was badly broken. While we can't fully preserve the original semantics, since macOS doesn't have posix_fallocate and F_PREALLOCATE doesn't allow to allocate arbitrary ranges, we can do something close enough to work fine for the vast majority of guest applications.