Re: [Qemu-devel] [PATCH 1.2 5/7] block: add bdrv_set_enable_write_cache

[Paolo Bonzini]

Il 31/05/2012 13:44, Kevin Wolf ha scritto:
> Am 22.05.2012 18:17, schrieb Paolo Bonzini:
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>  block.c |    5 +++++
>>  block.h |    1 +
>>  2 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index b682764..bb709c2 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2369,6 +2369,11 @@ int bdrv_enable_write_cache(BlockDriverState *bs)
>>      return bs->enable_write_cache;
>>  }
>>  
>> +void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)
>> +{
>> +    bs->enable_write_cache = wce;
>> +}
> 
> I think we should have a flush here if bs->enable_write_cache && !wce,
> it's not really something that could differ between device models.

But what varies is the mechanics of doing the flush asynchronously.
IDE, SCSI, etc. have different ways to report completion to the guest.

> I see
> that you're doing it in your IDE patch by calling ide_flush_cache()
> which looks like this:
> 
> void ide_flush_cache(IDEState *s)
> {
>     if (s->bs == NULL) {
>         ide_flush_cb(s, 0);
>         return;
>     }
> 
>     bdrv_acct_start(s->bs, &s->acct, 0, BDRV_ACCT_FLUSH);
>     bdrv_aio_flush(s->bs, ide_flush_cb, s);
> }
> 
> I don't think this flush should be accounted, because it's only
> indirectly initiated by the guest.

True, but then a guest could do flushes by flipping the WCE bit, and no
time would be accounted to flushes.  This is going to happen at most
once or twice in sane guests, so I don't think it matters.

Paolo