Skip to content

virtio: add peek support to VirtioQueue#2992

Merged
jstarks merged 4 commits intomicrosoft:mainfrom
jstarks:virtio-peek
Mar 15, 2026
Merged

virtio: add peek support to VirtioQueue#2992
jstarks merged 4 commits intomicrosoft:mainfrom
jstarks:virtio-peek

Conversation

@jstarks
Copy link
Member

@jstarks jstarks commented Mar 13, 2026

Add try_peek and peek methods to VirtioQueue that return a PeekedWork handle for inspecting the next available descriptor without advancing the available index. This allows a device to examine payload contents before deciding whether to consume the descriptor. Dropping a PeekedWork without calling consume is a no-op -- the descriptor stays in the available ring for the next call.

To support this, refactor the queue internals to separate availability checking from index advancement:

  • Split QueueCore::try_next_work into try_peek_work + advance.
  • Add SplitQueueGetWork::advance and PackedQueueGetWork::advance methods that each handle their own index bookkeeping.
  • Hoist descriptor_index out of per-queue-type completion contexts into QueueWork directly, since both split and packed queues need it.
  • Extract payload-reading helpers (read_from_payload, read_from_payload_at_offset, readable_payload_length) as free functions shared between VirtioQueueCallbackWork and PeekedWork.

Also simplify the Stream implementation: poll_next_buffer now returns Poll<Result<..., Error>> instead of wrapping in an extra Option, since the stream never yields None before an error.

@jstarks jstarks requested a review from a team as a code owner March 13, 2026 22:44
Copilot AI review requested due to automatic review settings March 13, 2026 22:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds “peek” support to the virtio queue abstraction so devices can inspect the next available descriptor without consuming it, and refactors queue internals to separate availability checks from index advancement.

Changes:

  • Add VirtioQueue::{try_peek, peek} and a PeekedWork handle that can later be consume()d into a normal VirtioQueueCallbackWork.
  • Refactor QueueCoreGetWork and split/packed queue implementations to split “peek” vs “advance” behavior and move descriptor_index into QueueWork.
  • Extract shared payload-reading helpers and simplify the Stream implementation to never yield None.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
vm/devices/virtio/virtio/src/queue/split.rs Split queue: make availability checking non-advancing and add explicit advance().
vm/devices/virtio/virtio/src/queue/packed.rs Packed queue: add advance(count) and adjust completion context construction/access.
vm/devices/virtio/virtio/src/queue.rs Core queue refactor: introduce try_peek_work + advance, move descriptor index into QueueWork.
vm/devices/virtio/virtio/src/common.rs Add PeekedWork, shared payload read helpers, and streamline Stream polling.

You can also share your feedback on Copilot code review. Take the survey.

@jstarks jstarks requested a review from Copilot March 15, 2026 16:18
@github-actions github-actions bot added the unsafe Related to unsafe code label Mar 15, 2026
@github-actions
Copy link

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds “peek” support to the virtio queue abstraction so devices can inspect the next available descriptor without consuming it, enabling conditional consumption based on payload contents. It refactors queue internals to separate availability checking from advancing the available index, and updates the async Stream surface to remove an unnecessary Option layer.

Changes:

  • Add VirtioQueue::try_peek / VirtioQueue::peek returning a PeekedWork handle, with consume() to advance and convert into normal completion work.
  • Refactor QueueCoreGetWork into try_peek_work + advance, and move descriptor_index onto QueueWork.
  • Add tests validating that peeking does not advance for both split and packed queues.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
vm/devices/virtio/virtio/src/common.rs Adds PeekedWork, shared payload read helpers, and stream/poll refactor integration in VirtioQueue.
vm/devices/virtio/virtio/src/queue.rs Splits “peek vs advance” mechanics in QueueCoreGetWork and adjusts QueueWork/completion context layout.
vm/devices/virtio/virtio/src/queue/split.rs Refactors split queue availability checking to avoid advancing the index until advance().
vm/devices/virtio/virtio/src/queue/packed.rs Refactors packed queue advancement and adjusts completion context to support deferred advancement.
vm/devices/virtio/virtio/src/tests.rs Adds split/packed tests for peek semantics (no-advance, peek-then-next behavior).

You can also share your feedback on Copilot code review. Take the survey.

jstarks added 3 commits March 15, 2026 19:15
Add `try_peek` and `peek` methods to `VirtioQueue` that return a
`PeekedWork` handle for inspecting the next available descriptor without
advancing the available index. This allows a device to examine payload
contents before deciding whether to consume the descriptor. Dropping a
`PeekedWork` without calling `consume` is a no-op -- the descriptor
stays in the available ring for the next call.

To support this, refactor the queue internals to separate availability
checking from index advancement:

- Split `QueueCore::try_next_work` into `try_peek_work` + `advance`.
- Add `SplitQueueGetWork::advance` and `PackedQueueGetWork::advance`
  methods that each handle their own index bookkeeping.
- Hoist `descriptor_index` out of per-queue-type completion contexts
  into `QueueWork` directly, since both split and packed queues need it.
- Extract payload-reading helpers (`read_from_payload`,
  `read_from_payload_at_offset`, `readable_payload_length`) as free
  functions shared between `VirtioQueueCallbackWork` and `PeekedWork`.

Also simplify the `Stream` implementation: `poll_next_buffer` now returns
`Poll<Result<..., Error>>` instead of wrapping in an extra `Option`,
since the stream never yields `None` before an error.
- Remove empty SplitQueueCompletionContext struct and its re-export
- Fix payload.address + skip overflow in read_from_payload_at_offset
  using saturating_add to prevent GPA wraparound on guest-provided
  addresses
- Add unit tests for try_peek/consume for both split and packed queues
Address PR feedback: reword try_peek_work, try_peek, and peek doc
comments to clarify that advance is needed to consume/progress (not
before re-peeking), and that the guest may modify descriptor memory
between peeks so payload contents are not guaranteed to be stable.
Copilot AI review requested due to automatic review settings March 15, 2026 19:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds non-consuming “peek” support to the core virtio queue API (VirtioQueue) by introducing try_peek/peek returning a PeekedWork handle, and refactors queue internals so availability checks are separated from index advancement. This enables devices to inspect the next descriptor payload before deciding to consume it.

Changes:

  • Add VirtioQueue::{try_peek, peek} and PeekedWork with consume() to explicitly advance after peeking.
  • Refactor queue internals: split try_next_work into try_peek_work + advance, and move descriptor index into QueueWork.
  • Add unit tests validating that peeking does not advance for both split and packed queues.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
vm/devices/virtio/virtio/src/common.rs Introduces PeekedWork, adds try_peek/peek, and extracts shared payload-reading helpers.
vm/devices/virtio/virtio/src/queue.rs Refactors core work retrieval into peek + explicit advancement; moves descriptor index into QueueWork.
vm/devices/virtio/virtio/src/queue/split.rs Splits availability checking from index advancement for split queues; adjusts completion API.
vm/devices/virtio/virtio/src/queue/packed.rs Adds advance(count) for packed queues and refactors completion context construction.
vm/devices/virtio/virtio/src/tests.rs Adds new tests covering peek behavior for split and packed queues.

You can also share your feedback on Copilot code review. Take the survey.

- Clarify peek/try_peek doc comments: descriptor metadata is snapshotted
  at peek time; only guest buffer contents may change between peek and
  consume.
- Add verify_packed_peek_linked_multi_descriptor test exercising the
  descriptor_count/advance path with a 3-descriptor linked buffer on
  packed queues.
@jstarks jstarks merged commit 3a79304 into microsoft:main Mar 15, 2026
56 checks passed
@jstarks jstarks deleted the virtio-peek branch March 15, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants