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

[Paolo Bonzini]

On 16/04/2015 09:16, Peter Lieven wrote:
> 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.

Correct.  So if you do shared access you _still_ cannot use
writeback/writethrough/unsafe, even for libiscsi.  You can use it for
migration if you implement bdrv_invalidate_cache and add a patch like
the following:

diff --git a/block.c b/block.c
index f2f8ae7..345e617 100644
--- a/block.c
+++ b/block.c
@@ -5119,7 +5119,12 @@ void bdrv_invalidate_cache(

     if (bs->drv->bdrv_invalidate_cache) {
         bs->drv->bdrv_invalidate_cache(bs, &local_err);
-    } else if (bs->file) {
+    }
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    if (bs->file) {
         bdrv_invalidate_cache(bs->file, &local_err);
     }
     if (local_err) {


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

Actually, if you use cache=none, it falls back to cache=directsync.

If I were you:

- I would switch to the cache.foo options, they're clearer and permit
finer-grained configuration.

- I would never use cache.direct=no if you know you're doing migration,
until bdrv_invalidate_cache is implemented.  After that, you can always
use cache.direct=no with libiscsi except if multiple servers are sharing
the disk.

- I would always use cache.writeback=yes if you know you're using
virtio-blk (which falls back to cache.writeback=no) or if the guest is
reasonably recent (2.6.32+ kernel; or any Windows as I'm not aware of
any change regarding flushes there).

- I would use file.cache.no-flush=yes if you know you're on
battery-backed storage.

Paolo

Paolo