oxidecomputer / oxidecomputer/propolis

virtio-block needs `VIRTIO_BLK_F_FLUSH` / `VIRTIO_BLK_T_FLUSH` support.

Open
#492 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

storage
Dominant language
Rust
Stars
270
Forks
42
Avg merge
4d 5h
Merged PRs (30d)
6

Description

While investigating seemingly much worse performance for the nvme device compared to virtio-block, @rmustacc pointed out we set the Volatile Write Cache bit for nvme devices but not the similar flush capability for virtio-block.

As a quick test, I tried clearing the vwc bit and rerunning pgbench with an nvme device and got similar (if not very slightly better) results compared to virtio-block (both using the file backend). Given the fact the file backend also doesn't use any sync flags on open, the speedup we were seeing on virtio-block makes sense: turns out it's faster to just assume things are synchronous (when in actually they're not) and never call flush/fsync.

The issue is we don't ever try to negotiate the VIRTIO_BLK_F_FLUSH feature today. Per the VIRTIO spec[^1]:

An implementation that does not offer VIRTIO_BLK_F_FLUSH and does not commit completed writes will not be resilient to data loss in case of crashes.

In addition to advertising and trying to negotiate VIRTIO_BLK_F_FLUSH, we then subsequently need to support VIRTIO_BLK_T_FLUSH commands and forward appropriately to the backend.

Note: implementation wise we can also just choose to always commit writes even without flush support:

If VIRTIO_BLK_F_FLUSH was not offered by the device, the device MAY also commit writes to persistent device backend storage before reporting their completion.

But this relies on better support on the backend's side as well.

[^1]: 5.2.6.2 Device Requirements: Device Operation
[^2]: note at this point adding flush support would seemingly regress virtio-block performance but that's just a reflection of no longer trading in reliability. Regardless, there's separate ongoing work to improve the performance overall.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in lib/propolis/src/hw/virtio/block.rs at the feature negotiation code around lines 163-174, then trace how block requests are dispatched to the backend. Add negotiation for VIRTIO_BLK_F_FLUSH and handling for VIRTIO_BLK_T_FLUSH, forwarding the operation appropriately; done means flush requests reach persistent backend storage without silently sacrificing reliability.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.