Repository navigation
fix(virtio): sync disk data on virtio-blk FLUSH - #545
Merged
Merged
Conversation
VirtioMio::handle_queue() previously handled BlkRequest::Flush by calling <&File as Write>::flush(), which is a no-op in the Rust standard library and never synchronizes data to disk. Per VirtIO 1.4 Sec 5.2.6.2, a block device backed by persistent storage must ensure writes are committed to it before reporting completion of a VIRTIO_BLK_T_FLUSH request. Call File::sync_data() instead. A flush only needs the written data and the metadata required to read it back, such as the file size and block allocation, not timestamps. So also set IORING_FSYNC_DATASYNC on the fsync in the io_uring worker, which makes both workers use fdatasync(2) on Linux. On macOS, sync_data() issues F_FULLFSYNC, the same as sync_all(). Fixes: 9a3857f ("fix(blk): flush disk file on FLUSH requests") Signed-off-by: Changyuan Lyu <changyuanl@google.com>
Coverage Report for CI Build 42962744932Coverage decreased (-0.01%) to 43.821%Details
Uncovered Changes
Coverage Regressions2 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
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.
Split from #544.
VirtioMio::handle_queue()previously handledBlkRequest::Flushwith<&File as Write>::flush(), which is a no-op in the Rust standard library and never invokesfsync. Per VirtIO 1.4 Sec 5.2.6.2, a block device backed by persistent storage must ensure writes are committed to it before reporting completion of aVIRTIO_BLK_T_FLUSHrequest. CallFile::sync_data()instead.A flush only needs the written data and the metadata required to read it back (file size, block allocation), not timestamps, so the
io_uringworker now also setsIORING_FSYNC_DATASYNC. Both workers usefdatasync(2)on Linux; on macOS,sync_data()issuesF_FULLFSYNClikesync_all().cargo +nightly fmt --check,cargo clippy --all-targets -D warningson x86_64-unknown-linux-gnu/musl, aarch64-unknown-linux-gnu and aarch64-apple-darwin, andcargo test --workspacepass locally.