Re: [Qemu-block] RFC iscsi: set FUA and DPO if !bs->enable_write_cache

[Paolo Bonzini]

On 14/04/2015 21:55, Peter Lieven wrote:
> Am 14.04.2015 um 18:15 schrieb Paolo Bonzini:
>>
>> On 14/04/2015 08:49, Peter Lieven wrote:
>>> Hi,
>>>
>>> Ronnie came up with an idea to reduce latency if !bs->enable_write_cache
>>> for an iSCSI device.
>>>
>>> If !bs->enable_write_cache Qemu sends a flush after every single write.
>>> What could be done is
>>> the following:
>>>
>>> if (!bs->enable_write_cache)
>>>  set FUA (force unit access) and DPO (disable page out) bits in every
>>> write cmd
>>>  make iscsi_co_flush a NOOP in this case.
>>>
>>> Your thoughts?
>> Yes, that would work.  In fact I'm not even sure you need DPO.
>>
>> speed of cache=writethrough in general doesn't matter much, except if
>> whoever runs the guest knows that the host has battery-backed cache.  In
>> that case this trick would improve latency.  You could get the same with
>> -drive file.cache.no-flush=on but this would just work.
> 
> You could, but this would not set the FUA bit. The DPO was Ronnies
> idea.

Yes, note that I mentioned battery-backed cache.  In that case, the FUA
bit in practice is treated as a no-op by the storage.

> Please correct me if I am wrong, but for iSCSI
> cachemodes none, directsync, writethrough and none are identical.

Nope, for iSCSI cache=none and cache=writeback are the same.  Consider
the parameter space as follows?

              cache.direct      cache.writeback      cache.no-flush
none               X                     X
directsync         X
writeback                                X
writethrough
unsafe                                   X                  X

Since iSCSI ignores cache.direct (it's always direct!), none==writeback
and directsync==writethrough.

> cachemode unsafe avoids the explicit flush which is no good idea as
> we all would agree.

Actually, in the case of battery-backed cache file.cache.no-flush=on
(aka file.cache=unsafe) _is_ a good idea, because arrays with
battery-backed cache ignore flushes (just like FUAs).

Of course cache=unsafe and cache.no-flush=on are not a good idea because
you want to flush the qcow2 caches for example.

> cachemode writeback is the only one that enables bs->enable_write_cache.

None, too.  I would use cache=none just to be safe: cache=writeback
works with migration only because you are using the iSCSI driver.

> What I would implement in the iSCSI driver is the following:
> 
> a) check support for DPOFUA in the MODE SENSE page. Store it in 
> iscsilun->dpofua.
> b) make iscsi_co_flush a NOOP if iscsilun->dpofua && !bs->enable_write_cache
> c) In iscsi_co_writev
>     If iscsilun->dpofua && !bs->enable_write_cache set the FUA bit in the 
> write command.
> d) in iscsi_co_write_zeroes
>    If iscsilun->dpofua && !bs->enable_write_cache manually issue a 
> iscsi_co_flush as the WRITESAME10/16 commands lack the FUA bit.

Makes sense.  I think DPO is too much: it would bypass the read cache,
which is not writethrough but rather no caching at all.

Paolo