From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36368) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yqi7G-000723-Qo for qemu-devel@nongnu.org; Fri, 08 May 2015 09:14:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yqi7F-0002Tb-PO for qemu-devel@nongnu.org; Fri, 08 May 2015 09:14:58 -0400 Message-ID: <554CB6C6.3060809@redhat.com> Date: Fri, 08 May 2015 15:14:46 +0200 From: Max Reitz MIME-Version: 1.0 References: <1430971496-32659-1-git-send-email-phoeagon@gmail.com> <1431011818-15822-1-git-send-email-phoeagon@gmail.com> In-Reply-To: <1431011818-15822-1-git-send-email-phoeagon@gmail.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4] block/vdi: Use bdrv_flush after metadata updates List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhe Qiu , qemu-devel@nongnu.org Cc: kwolf@redhat.com, sw@weilnetz.de, qemu-block@nongnu.org On 07.05.2015 17:16, Zhe Qiu wrote: > In reference to b0ad5a45...078a458e, metadata writes to > qcow2/cow/qcow/vpc/vmdk are all synced prior to succeeding writes. > > Only when write is successful that bdrv_flush is called. > > Signed-off-by: Zhe Qiu > --- > block/vdi.c | 3 +++ > 1 file changed, 3 insertions(+) I missed Kevin's arguments before, but I think that adding this is more correct than not having it; and when thinking about speed, this is vdi, a format supported for compatibility. Writing isn't our most important concern anyway (especially considering that it's even to disable write support for vdi at compile time). So if we wanted to optimize it, we'd probably have to cache multiple allocations, do them at once and then flush afterwards (like the metadata cache we have in qcow2?), but that is complicated (like the metadata cache in qcow2), certainly too complicated for a format supported for compatibility (unless someone really wants to implement it). Reviewed-by: Max Reitz > > diff --git a/block/vdi.c b/block/vdi.c > index 7642ef3..dfe8ade 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -713,6 +713,9 @@ static int vdi_co_write(BlockDriverState *bs, > logout("will write %u block map sectors starting from entry %u\n", > n_sectors, bmap_first); > ret = bdrv_write(bs->file, offset, base, n_sectors); > + if (ret >= 0) { > + ret = bdrv_flush(bs->file); > + } > } > > return ret;