Re: [PATCH, RFC] virtio_blk: add cache flush command

[Avi Kivity <avi@redhat.com>]

Christoph Hellwig wrote:
> On Mon, May 11, 2009 at 06:45:50PM +0300, Avi Kivity wrote:
>   
>>> Right now it's fsync.  By the time I'll submit the backend change it
>>> will still be fsync, but at least called from the posix-aio-compat
>>> thread pool.
>>>  
>>>       
>> I think if we have cache=writeback we should ignore this.
>>     
>
> It's only needed for cache=writeback, because without that there is no
> reason to flush a write cache.
>   

Maybe we should add a fourth cache= mode then.  But 
cache=writeback+fsync doesn't correspond to any real world drive; in the 
real world you're limited to power failures and a few megabytes of cache 
(typically less), cache=writeback+fsync can lose hundreds of megabytes 
due to power loss or software failure.

Oh, and cache=writeback+fsync doesn't work on qcow2, unless we add fsync 
after metadata updates.

>> For cache=none and cache=writethrough we don't really need fsync, but we 
>> do need to flush the inflight commands.
>>     
>
> What we do need for those modes is the basic barrier support because
> we can currently re-order requests.  The next version of my patch will
> implement a barriers without cache flush mode, although I don't think
> a fdatasync without any outstanding dirty data should cause problems.
>   

Yeah.  And maybe one day push the barrier into the kernel.

> (Or maybe ext3 actually is stupid enough to flush the whole fs even for
> that case

Sigh.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.