oxidecomputer / oxidecomputer/propolis

nvme: write/flush on a read-only disk could be short-circuited

Open
#1,182 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

this issue is a "we could" and "we probably shouldn't" and "this will probably be digested into a doc comment on nvme/mod.rs", not "we should".

from the NVMe spec (1.0e):

The Flush command shall commit data and metadata associated with the specified namespace(s) to non-volatile media. The flush applies to all commands completed prior to the submission of the Flush command. The controller may also flush additional data and/or metadata from any namespace.

likewise,

The Write command writes data and metadata, if applicable, to the NVM controller for the logical blocks indicated. [...]

.. if we've decided the storage medium is read-only, we could just handle these operations in the emulated controller for that storage medium. flushes always succeed immediately (there's no data or metadata to flush), writes always fail immediately.

I don't really think this is worth doing: it adds complexity in the controller to either do the same thing as the storage medium or hide bugs in the storage medium. it is only valuable if we think the storage medium will get read-onlyness wrong or guests are issuing lots of writes or flushes to read-only devices and we want them to have better throughput than on read-write devices. I personally am not interested in optimizing for Propolis' ability to kick write errors back to guests.. :)

not doing this means errors manifest in potentially-interesting ways: https://github.com/oxidecomputer/propolis/issues/1181 is a case where flushes to a read-only Crucible volume succeed with >=2 downstairs active, but with only one downstairs the flush fails. Crucible only needs one downstairs active to serve reads, though, so after activating a so-afflicted volume we actually have a volume that's OK for reads but fails on flushes. OVMF then tries to send an NVMe flush when closing the FAT filesystem of the ESP, that flush fails, and OVMF decides the volume is broken because the disk returned an error on an I/O.

maybe we're well-suited by a debug check that the storage medium fails writes and always succeeds on flushes, but we're not particularly close to having a good test that would exercise that, and we're not exactly shipping debug binaries anywhere. so such a check wouldn't do all that much for us. my conclusion is mostly that https://github.com/oxidecomputer/propolis/pull/966 would want to include "flush always works on a read-only disk" as a property to test, that we do that kind of fuzzing on NVMe controllers by Crucible, and that we try Crucible in different degrees of up-ness in that kind of test.

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 by reading the NVMe-related context in nvme/mod.rs, then review issue 1181 and pull request 966 for the surrounding read-only, flush, and fuzzing concerns. The issue does not establish an implementation plan; done would first require agreement on whether this behavior should be implemented or documented and tested.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
operating-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.