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
