On Mon, May 14, 2012 at 10:04 PM, Kevin Wolf <kw...@redhat.com> wrote: > Am 14.05.2012 15:51, schrieb zwu.ker...@gmail.com: >> From: Zhi Yong Wu <wu...@linux.vnet.ibm.com> >> >> 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 <wu...@linux.vnet.ibm.com> > > 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
