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: #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 #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.
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):
likewise,
.. 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: #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 #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.