From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47666) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yq37R-0004Ps-Eq for qemu-devel@nongnu.org; Wed, 06 May 2015 13:28:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yq37N-00069e-Vp for qemu-devel@nongnu.org; Wed, 06 May 2015 13:28:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40653) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yq37N-00069F-Is for qemu-devel@nongnu.org; Wed, 06 May 2015 13:28:21 -0400 Message-ID: <554A4F34.4080505@redhat.com> Date: Wed, 06 May 2015 11:28:20 -0600 From: Eric Blake MIME-Version: 1.0 References: <554A4458.7090901@redhat.com> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Bdf3IoV94WnkqBRm2xpmFrwj9XH7qBags" Subject: Re: [Qemu-devel] [PATCH] use bdrv_flush to provide barrier semantic in block/vdi.c for metadata updates List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: phoeagon , Max Reitz , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Bdf3IoV94WnkqBRm2xpmFrwj9XH7qBags Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/06/2015 11:23 AM, phoeagon wrote: > Thanks for your input. >=20 > So I changed it to: > 1. Only call bdrv_flush when bdrv_pwrite was successful > 2. Only if bdrv_flush was unsuccessful that the return value of > vdi_co_write is updated. > In this way we try to avoid messing up any potential return value check= s > possible while still propagating bdrv_flush errors. > That return value was a catch and I admit I'm no pro with the return va= lue > convention in QEMU. bdrv_pwrite doesn't return the same value as > bdrv_pwrite_sync I assume (they do return negative values when fail, bu= t > different values when successful) > --- >=20 The text above [1]... >=20 > Signed-off-by: Zhe Qiu This S-o-b is still broken. >=20 >>>From 19b2fabbe00765b418362d8c1891f266091621f3 Mon Sep 17 00:00:00 2001 When sending a revised patch, it's better to send it as a new top-level thread, and with 'v2' somewhere in the subject line (hint: git send-email -v2). Your placement of the Signed-off-by line before the From: attribution line is incorrect > From: phoeagon > Date: Thu, 7 May 2015 01:09:38 +0800 > Subject: [PATCH] block/vdi: Use bdrv_flush after metadata updates >=20 > In reference to > b0ad5a455d7e5352d4c86ba945112011dbeadfb8~078a458e077d6b0db262c4b05fee51= d01de2d1d2, > metadata writes to qcow2/cow/qcow/vpc/vmdk are all synced prior to > succeeding writes. >=20 > --- =2E..[1] is more useful here, after the commit message body. I highly suggest you read http://wiki.qemu.org/Contribute/SubmitAPatch; it is also a good idea to use 'git send-email' to send a patch to yourself, then 'git am' on that message to see if it survived the round trip through email, before sending to the list. > block/vdi.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) >=20 > diff --git a/block/vdi.c b/block/vdi.c > index 5d09b36..54a5fa8 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -713,7 +713,11 @@ static int vdi_co_write(BlockDriverState *bs, > logout("will write %u block map sectors starting from entry %u= \n", > n_sectors, bmap_first); > ret =3D bdrv_write(bs->file, offset, base, n_sectors); > + if (!(ret < 0)) { This looks odd. Better might be: 'if (ret >=3D 0) {' > + int flush_ret =3D bdrv_flush(bs->file); > + if (flush_ret < 0) > + ret =3D flush_ret; Missing {} (hint: scripts/checkpatch.pl is an important part of good patch submission) > + } > } >=20 > return ret; >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Bdf3IoV94WnkqBRm2xpmFrwj9XH7qBags Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVSk80AAoJEKeha0olJ0NqKZAH/3wi93VMkTstX2ZZp2Mvaj/N LiwAAzk77VZ4nYnRi04PcnE5t3sbKiPb9/z4q2C2tUxb/RP0HuT79F5iNUCzdoq/ INz1DXR3cNXk8yB6pLPqrRZFxa9dAAIv9pUF+wG1Rr6L6GtBet/UJ6MfD4cNznDO zeun7a02cu8K2KSdl5cP+26UTUvSdwXCV9pft3DSTdc8D8lx3O8aGkxHEMrcUp98 iQQcLmUPLA1KZKc8sq+Wr/oUpxqXPcDpjx6+4xPmgNutialJG3gp4fWUtL71h1Lp CO5lSypa6T+17gmttRp7MebQWf6WqaC5s381QjlzLEeG2lqXkPly0yVjP4Yu8bw= =LmeW -----END PGP SIGNATURE----- --Bdf3IoV94WnkqBRm2xpmFrwj9XH7qBags--