From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STwFg-000414-S9 for qemu-devel@nongnu.org; Mon, 14 May 2012 10:28:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1STwFa-0001bw-K4 for qemu-devel@nongnu.org; Mon, 14 May 2012 10:27:56 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:56046) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STwFa-0001bI-AU for qemu-devel@nongnu.org; Mon, 14 May 2012 10:27:50 -0400 Received: by pbbro12 with SMTP id ro12so8701812pbb.4 for ; Mon, 14 May 2012 07:27:48 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4FB110E1.90103@redhat.com> References: <1337003481-30215-1-git-send-email-zwu.kernel@gmail.com> <4FB110E1.90103@redhat.com> Date: Mon, 14 May 2012 22:27:48 +0800 Message-ID: From: Zhi Yong Wu Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qcow2: adjust qcow2_co_flush_to_os -> qcow2_co_flush_to_disk List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pbonzini@redhat.com, wuzhy@linux.vnet.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On Mon, May 14, 2012 at 10:04 PM, Kevin Wolf wrote: > Am 14.05.2012 15:51, schrieb zwu.kernel@gmail.com: >> From: Zhi Yong Wu >> >> qcow2_co_flush_to_os() actually flush all cached data to the disk. To ke= ep its name consistent with its actual function, adjust its name accordingl= y. >> >> Signed-off-by: Zhi Yong Wu > > 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: > > =A0 =A0/* > =A0 =A0 * Flushes all data that was already written to the OS all the way > down to > =A0 =A0 * the disk (for example raw-posix calls fsync()). > =A0 =A0 */ > =A0 =A0int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs); > > =A0 =A0/* > =A0 =A0 * Flushes all internal caches to the OS. The data may still sit i= n a > =A0 =A0 * writeback cache of the host OS, but it will survive a crash of > the qemu > =A0 =A0 * process. > =A0 =A0 */ > =A0 =A0int 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 --=20 Regards, Zhi Yong Wu