From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58792) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ajv4j-0004Xk-Ao for qemu-devel@nongnu.org; Sat, 26 Mar 2016 16:44:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ajv4i-00052b-AB for qemu-devel@nongnu.org; Sat, 26 Mar 2016 16:44:49 -0400 References: <1458325289-17848-1-git-send-email-kwolf@redhat.com> <1458325289-17848-14-git-send-email-kwolf@redhat.com> From: Max Reitz Message-ID: <56F6F4B7.8010909@redhat.com> Date: Sat, 26 Mar 2016 21:44:39 +0100 MIME-Version: 1.0 In-Reply-To: <1458325289-17848-14-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qKHsPoF30A3VpDxMbsffV1Xe06NlCqA7b" Subject: Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --qKHsPoF30A3VpDxMbsffV1Xe06NlCqA7b Content-Type: multipart/mixed; boundary="msnAERmRgoEEvXaQv594d6KqOaXU7TUow" From: Max Reitz To: Kevin Wolf , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org Message-ID: <56F6F4B7.8010909@redhat.com> Subject: Re: [PATCH 13/20] iscsi: Support BDRV_REQ_FUA References: <1458325289-17848-1-git-send-email-kwolf@redhat.com> <1458325289-17848-14-git-send-email-kwolf@redhat.com> In-Reply-To: <1458325289-17848-14-git-send-email-kwolf@redhat.com> --msnAERmRgoEEvXaQv594d6KqOaXU7TUow Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 18.03.2016 19:21, Kevin Wolf wrote: > This replaces the existing hack in the iscsi driver that sent the FUA > bit in writethrough mode and ignored the following flush in order to > optimise the number of roundtrips (see commit 73b5394e). >=20 > Signed-off-by: Kevin Wolf > --- > block/iscsi.c | 24 +++++++----------------- > 1 file changed, 7 insertions(+), 17 deletions(-) >=20 > diff --git a/block/iscsi.c b/block/iscsi.c > index 3b54536..4f75204 100644 > --- a/block/iscsi.c > +++ b/block/iscsi.c [...] > @@ -1851,7 +1840,8 @@ static BlockDriver bdrv_iscsi =3D { > .bdrv_co_discard =3D iscsi_co_discard, > .bdrv_co_write_zeroes =3D iscsi_co_write_zeroes, > .bdrv_co_readv =3D iscsi_co_readv, > - .bdrv_co_writev =3D iscsi_co_writev, > + .bdrv_co_writev_flags =3D iscsi_co_writev_flags, > + .supported_write_flags =3D BDRV_REQ_FUA, > .bdrv_co_flush_to_disk =3D iscsi_co_flush, > =20 > #ifdef __linux__ >=20 Hm, wait, maybe not R-b. I can see three places in block/io.c which call bdrv_co_writev(), and only one of them diverts to bdrv_co_writev_flags() if that is available. Maybe we don't need to care about the bounce-buffer case for write_zeroes, but I do think we need to care about the COR case. Of course bdrv_co_writev() can trivially be forwarded to bdrv_co_writev_flags(), but I'm not sure who is supposed to do this forwarding. I can imagine three ways: (1) Keep a wrapper per block driver. Simple, but not so elegant. (2) Make all bdrv_co_writev() callers call bdrv_co_writev_flags() if the former is not available but the latter is. (3) Introduce a generic function replacing every drv->bdrv_co_writev() call which then decides which driver function to invoke. Max --msnAERmRgoEEvXaQv594d6KqOaXU7TUow-- --qKHsPoF30A3VpDxMbsffV1Xe06NlCqA7b 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 iQEcBAEBCAAGBQJW9vS3AAoJEDuxQgLoOKytjbgH/jUCGqPot1nxDyaLDvMD13v4 6qC4cN0mHpuwB7ypPT0pEvLzVesDDRlPB8Ji37yLfEAilGQUEvg6nytIiuY4vMqO lVxMgDh87/DgKcEizcH0seGhsarW6kTrCX1Fij4WmNJKH3rW9fMZuVU37+sC1liP dUWufoTNFL7/+ONn9L6jGi9QO23aTEWSvQFEfqOfVxoiF/HKMSN/EvwlaGHUgT5i sDdHM4cAHTQc6yOJAlKL9O1seHZQ920nd3q/dbsV0/CKMTy7KDRnGxSMBhuzVzn+ zk3fU4CLa8ol5mOwBwUoxCpwcyeQo+vIiB0rLEpgUCqJl8GuM3/QPfDAxq4N4Bc= =eiBL -----END PGP SIGNATURE----- --qKHsPoF30A3VpDxMbsffV1Xe06NlCqA7b--