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

[Peter Lieven]

Am 15.04.2015 um 11:03 schrieb 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.

Thanks for the clarification. At first glance its not that obvious.

There is one thing I want to mention none is not exactly writeback
and directsync not exactly writethrough. We do not enable the
iscsi->allocationmap if BRDV_O_NOCACHE is in the flags. And this
flag seems to be equivalent to cache.direct.

> > 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.

Or if you use libnfs with the NFS driver for accessing the QCOWs on
your NFS share.

My current strategy is to use always cache=none except if I have
an iSCSI target where I either know that the target has battery backup
or if I use virtio-blk (which falls back to writethrough if there is no support
for flushes).

> > 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.

Okay, so it seems setting FUA in the READ commands is nothing we can address
with our current cache modes.

Peter