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

[ronnie sahlberg]

Sounds good to me.

On Tue, Apr 14, 2015 at 12:59 PM, Peter Lieven <address@hidden> wrote:
> Am 14.04.2015 um 21:55 schrieb Peter Lieven:
>> 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. Ronnie? SBC-2 says about FUA and WRITE: "Write commands shall not 
>> return GOOD status until the logical blocks have actually been written on 
>> the media (i.e., the data is not write cached)."
>>
>> Please correct me if I am wrong, but for iSCSI
>> cachemodes none, directsync, writethrough and none are identical.
>> cachemode unsafe avoids the explicit flush which is no good idea as
>> we all would agree.
>>
>> cachemode writeback is the only one that enables bs->enable_write_cache.
>> I use it for our enterprise iSCSI SANs that have battery backup and I use it
>> only in conjunction with Virtio which falls back to writethrough if there is
>> no support for flushes from the guest.
>>
>> 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.
>
> e) In iscsi_co_readv: If iscsilun->dpofua && (bs->open_flags & 
> BDRV_O_NOCACHE) set FUA bit in Read 10/16.
>
> This explicitly tells the iSCSI storage to bypass its read cache.
>
> Peter
>