On Wed, Jun 12, 2024 at 10:01:18AM +0200, Roger Pau Monné wrote: > On Tue, Jun 11, 2024 at 07:19:10AM +0200, Christoph Hellwig wrote: > > blkfront always had a robust negotiation protocol for detecting a write > > cache. Stop simply disabling cache flushes when they fail as that is > > a grave error. > > It's my understanding the current code attempts to cover up for the > lack of guarantees the feature itself provides: > So even when the feature is exposed, the backend might return > EOPNOTSUPP for the flush/barrier operations. How is this supposed to work? I mean in the worst case we could just immediately complete the flush requests in the driver, but we're really lying to any upper layer. > Such failure is tied on whether the underlying blkback storage > supports REQ_OP_WRITE with REQ_PREFLUSH operation. blkback will > expose "feature-barrier" and/or "feature-flush-cache" without knowing > whether the underlying backend supports those operations, hence the > weird fallback in blkfront. If we are just talking about the Linux blkback driver (I know there probably are a few other implementations) it won't every do that. I see it has code to do so, but the Linux block layer doesn't allow the flush operation to randomly fail if it was previously advertised. Note that even blkfront conforms to this as it fixes up the return value when it gets this notsupp error to ok. > Overall blkback should ensure that REQ_PREFLUSH is supported before > exposing "feature-barrier" or "feature-flush-cache", as then the > exposed features would really match what the underlying backend > supports (rather than the commands blkback knows about). Yes. The in-tree xen-blkback does that, but even without that the Linux block layer actually makes sure flushes sent by upper layers always succeed even when not supported.
