virtio-blk: add async discard support#5935
Open
bacarrdy wants to merge 5 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds opt-in virtio-blk discard/TRIM support, wiring it through drive configuration into virtio request handling and the file I/O backends (sync + io_uring), plus updating docs and seccomp to permit the needed syscalls.
Changes:
- Add
discardto drive/block configs and propagate it through virtio-blk feature negotiation and persistence. - Implement discard request parsing/execution and backend support (sync via
BLKDISCARD/fallocate, async via io_uringFALLOCATEorURING_CMD). - Extend io_uring opcode support/probing and update docs + seccomp filters accordingly.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vmm/tests/integration_tests.rs | Update test drive configs to include new discard field. |
| src/vmm/src/vmm_config/drive.rs | Add discard to BlockDeviceConfig and update related tests/helpers. |
| src/vmm/src/resources.rs | Update resource tests to include new discard field. |
| src/vmm/src/io_uring/operation/mod.rs | Add io_uring opcodes + SQE layout support for fallocate / uring cmd. |
| src/vmm/src/io_uring/mod.rs | Allow callers to specify required opcodes when creating rings. |
| src/vmm/src/devices/virtio/block/virtio/test_utils.rs | Extend default block test config with discard. |
| src/vmm/src/devices/virtio/block/virtio/request.rs | Add VIRTIO_BLK_T_DISCARD parsing/validation + execution path. |
| src/vmm/src/devices/virtio/block/virtio/persist.rs | Persist/restore discard feature bit and config space changes. |
| src/vmm/src/devices/virtio/block/virtio/mod.rs | Define discard-related limits/alignment constants. |
| src/vmm/src/devices/virtio/block/virtio/io/sync_io.rs | Implement sync discard via BLKDISCARD or fallocate(PUNCH_HOLE). |
| src/vmm/src/devices/virtio/block/virtio/io/mod.rs | Add discard API to FileEngine and plumb discard flag into async engine creation. |
| src/vmm/src/devices/virtio/block/virtio/io/async_io.rs | Add async discard selection and submission via io_uring (fallocate / uring cmd). |
| src/vmm/src/devices/virtio/block/virtio/device.rs | Add discard to virtio-blk config, feature bits, config space, and tests. |
| src/vmm/src/devices/virtio/block/vhost_user/device.rs | Ensure vhost-user block configs reject/omit discard. |
| src/vmm/src/device_manager/mod.rs | Update device manager tests for new discard field. |
| src/vmm/src/builder.rs | Update builder tests for new discard field. |
| src/firecracker/swagger/firecracker.yaml | Document new discard API field for drives. |
| resources/seccomp/x86_64-unknown-linux-musl.json | Allow fallocate and ioctl(BLKDISCARD) for discard support. |
| resources/seccomp/aarch64-unknown-linux-musl.json | Allow fallocate and ioctl(BLKDISCARD) for discard support. |
| docs/api_requests/block-io-engine.md | Document async discard behavior/requirements. |
| docs/api_requests/block-discard.md | Add API usage documentation for discard. |
| CHANGELOG.md | Note new discard feature in changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cd7b1df to
67e39aa
Compare
Extend the virtio-blk config space through the discard limit fields so the device can report discard capability details when the feature is enabled. Signed-off-by: Jonas Savulionis <jonas@esnet.lt>
Add an opt-in discard drive flag and parse a single validated discard range without re-reading guest memory after validation. Execute discard with hole punching for regular files and BLKDISCARD for block devices on the sync engine. Reject discard with the async engine until an async implementation can be added. Signed-off-by: Jonas Savulionis <jonas@esnet.lt>
Allow fallocate hole punching and BLKDISCARD ioctl calls needed by the sync virtio-blk discard implementation. Signed-off-by: Jonas Savulionis <jonas@esnet.lt>
Add async discard execution for virtio-blk drives when the discard feature is enabled. Regular file backends use IORING_OP_FALLOCATE with hole punching. Block-device backends use BLOCK_URING_CMD_DISCARD through IORING_OP_URING_CMD when the host kernel supports it. Require async fsync support during io_uring initialization, document the async discard support matrix, and return VIRTIO_BLK_S_UNSUPP for guest DISCARD requests when the discard feature was not negotiated. Signed-off-by: Jonas Savulionis <jonas@esnet.lt>
d373e02 to
3007b1e
Compare
Store the validated discard byte offset and length in the existing Request fields instead of carrying a separate discard range. Signed-off-by: Jonas Savulionis <jonas@esnet.lt>
3007b1e to
465d79d
Compare
11 tasks
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.
Changes
discard=truesupport for writableAsyncIO engine virtio-block drives.IORING_OP_FALLOCATEfor regular backing files.BLOCK_URING_CMD_DISCARDfor block-device backing stores on host kernels that support it.VIRTIO_BLK_F_DISCARD, so guests that did not negotiate the feature receive an unsupported status.This PR intentionally depends on #5908. If #5908 is merged first, this branch should be rebased so the final diff only contains the async follow-up.
Reason
#5908 keeps the initial discard implementation focused on the
SyncIO engine. This follow-up adds the io_uring path so deployments using theAsyncIO engine can still expose guestfstrim/discard where the host kernel and backing storage support it.Validation
tools/devtool checkstylelocally after the latest update: passed (20 passed, 4 skipped).tools/devtool checkbuild --allwhile resolving thelibc::Ioctlreview thread.License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.