qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] qcow2: adjust qcow2_co_flush_to_os -> qcow2_co_


From: Zhi Yong Wu
Subject: Re: [Qemu-devel] [PATCH] qcow2: adjust qcow2_co_flush_to_os -> qcow2_co_flush_to_disk
Date: Mon, 14 May 2012 22:27:48 +0800

On Mon, May 14, 2012 at 10:04 PM, Kevin Wolf <address@hidden> wrote:
> Am 14.05.2012 15:51, schrieb address@hidden:
>> From: Zhi Yong Wu <address@hidden>
>>
>> qcow2_co_flush_to_os() actually flush all cached data to the disk. To keep 
>> its name consistent with its actual function, adjust its name accordingly.
>>
>> Signed-off-by: Zhi Yong Wu <address@hidden>
>
> This patch is plain wrong.
>
> You're aware that you're not changing a name, but functionality here?
Sure, i know this.

> Have a look at block_int.h for the semantics of each function:
>
>    /*
>     * Flushes all data that was already written to the OS all the way
> down to
>     * the disk (for example raw-posix calls fsync()).
>     */
>    int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
>
>    /*
>     * Flushes all internal caches to the OS. The data may still sit in a
>     * writeback cache of the host OS, but it will survive a crash of
> the qemu
>     * process.
>     */
>    int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
>
> Apart from that, it's not even intentional that qcow2 does a
> bdrv_flush() even if it didn't write out any cache entries. If we
Since bdrv_flush() is invoked now, qcow2_co_flush_to_os() is not
strictly alligned to its expected semantics, but the semantics of 'int
coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs)', so i
suggested adjusting its name.

> optimise the cache code a bit, this might disappear in the future.
You mean that bdrv_flush() will be not invoked here in the future?
>
> Kevin



-- 
Regards,

Zhi Yong Wu



reply via email to

[Prev in Thread] Current Thread [Next in Thread]