Re: [Qemu-devel] [PATCH v2 06/16] block: do not parse BDRV_O_CACHE_WB in raw block drivers

[Paolo Bonzini]

Il 13/09/2012 21:04, Jeff Cody ha scritto:
>> > Perhaps we _should_ preserve that in bs->open_flags, while still using
>> > the initial value of BDRV_O_CACHE_WB to initialize bs->enable_write_cache.
> That would work.  Part of the problem is that BDRV_O_CACHE_WB seems
> overloaded; maybe bdrv_parse_cache_flags() should set a new flag, called
> BDRV_O_CACHE_WCE, which can be used in lieu of enable_write_cache
> (similar to getting rid of keep_read_only in favor of
>  BDRV_O_ALLOW_RDWR).  And then bdrv_parse_cache_flags() would just not
> set BDRV_O_CACHE_WB, which can then be used exclusively by the lower
> layers for their parsing (and bdrv_open_common would just set
> bs->open_flags to always have BDRV_O_CACHE_WB).
> 
> Then patch 2/16 would change to having bdrv_set_enable_write_cache()
> toggle BDRV_O_CACHE_WCE.
> 

Yeah, that would work.

Alternatively, we can keep this patch and leave bdrv_open_common as is;
but then I would also prefer if raw-{posix,win32} took care of setting
BDRV_O_CACHE_WB in bs->open_flags, so that the flags are consistent.
This setting of BDRV_O_CACHE_WB can be extended later to other formats.

Considering this and my comments to patch 3/16 we would have:

- after opening with cache=writethrough:

                           bs           bs->file
enable_write_cache        true           true
BDRV_O_CACHE_WB           false          true
bdrv_enable_write_cache() false          true


- after opening with cache=writeback:

                           bs           bs->file
enable_write_cache        true           true
BDRV_O_CACHE_WB           true           true
bdrv_enable_write_cache() true           true

Paolo