From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40812) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aygaq-0005xL-WD for qemu-devel@nongnu.org; Fri, 06 May 2016 10:19:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aygae-0003BH-Dy for qemu-devel@nongnu.org; Fri, 06 May 2016 10:18:55 -0400 References: <1462406126-22946-1-git-send-email-eblake@redhat.com> <1462406126-22946-8-git-send-email-eblake@redhat.com> <20160506125041.GF5093@noname.redhat.com> From: Eric Blake Message-ID: <572CA7A4.5000402@redhat.com> Date: Fri, 6 May 2016 08:18:12 -0600 MIME-Version: 1.0 In-Reply-To: <20160506125041.GF5093@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tU3eLu9lv81E4NFPAJFnDN4FrTsChRT7v" Subject: Re: [Qemu-devel] [PATCH v6 07/20] scsi-disk: Switch to byte-based aio block access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Paolo Bonzini This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --tU3eLu9lv81E4NFPAJFnDN4FrTsChRT7v Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 05/06/2016 06:50 AM, Kevin Wolf wrote: > Am 05.05.2016 um 01:55 hat Eric Blake geschrieben: >> Sector-based blk_aio_readv() and blk_aio_writev() should die; switch >> to byte-based blk_aio_preadv() and blk_aio_pwritev() instead. >> >> @@ -343,8 +343,9 @@ static void scsi_do_read(SCSIDiskReq *r, int ret) >> n =3D scsi_init_iovec(r, SCSI_DMA_BUF_SIZE); >> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct, >> n * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ); >=20 > If you replace n * BDRV_SECTOR_SIZE here as well, n is unused and can g= o > away (you actually did that already for writes). Sure, I can add that in. >> @@ -1730,13 +1730,11 @@ static void scsi_write_same_complete(void *opa= que, int ret) >> if (data->iov.iov_len) { >> block_acct_start(blk_get_stats(s->qdev.conf.blk), &r->acct, >> data->iov.iov_len, BLOCK_ACCT_WRITE); >> - /* blk_aio_write doesn't like the qiov size being different f= rom >> - * nb_sectors, make sure they match. >> - */ >=20 > Wouldn't it be better to update the comment instead of deleting it? If = I > understand correctly, this additional qemu_iovec_init_external() is for= > the last part of an unaligned WRITE SAME request, where the qiov can > become shorter than in the previous iterations. The comment mattered when you were passing two sizes to blk_aio_writev (one embedded in data->qiov, and one in terms of sectors as a direct argument); the block layer asserted the two were equal. But now that we are handling bytes, we pass in a single size (that of data->qiov), so the comment no longer makes sense. >=20 >> qemu_iovec_init_external(&data->qiov, &data->iov, 1); >> - r->req.aiocb =3D blk_aio_writev(s->qdev.conf.blk, data->secto= r, >> - &data->qiov, data->iov.iov_len = / 512, >> - scsi_write_same_complete, data)= ; >> + r->req.aiocb =3D blk_aio_pwritev(s->qdev.conf.blk, >> + data->sector << BDRV_SECTOR_BI= TS, >> + &data->qiov, 0, It caught me more than once while writing the patch that my new function replaced nb_sectors (the duplicated size) by flags, rather than adding a parameter (as it was harder to get the compiler to gripe about incomplete conversions, such as forgetting to s/write/pwrite/). But by the end of the series, when the old interface is gone, everything still compiles under the new name, so at least we're assured that the series worked on that front. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --tU3eLu9lv81E4NFPAJFnDN4FrTsChRT7v 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/ iQEcBAEBCAAGBQJXLKekAAoJEKeha0olJ0NqTnUIALDCZ3/M9uQAmKhU7ZJcGkmD ZDZcfRITArvFxm/Q+yAVzK+a+ST0DHKt5o6oxv9zvPQGFueuEVO1y2b2cFEs1iT0 DrLUaBZHIKemmwW5zLJianbgTP95PZpZjhQabfANa5PzaGidDbk+HB0Bz0UmpoWy 7tTzKp9EDno3EKC8/l76gG8WO29XYI3Lrxgmxs2vcsl8tbHkgC62byxiDTuyQmtn TOI5SalzkjTm9hgTUaRKu2qxeLiwvDhKt7C64dzpnUHOcrngfhMTuCd/7BFnt/zR TChzw0U0IlO5c0rmIE4VvTDurFZ5uhYW0/6XEJggS16dPb0GxLkLruMkN7TLQiE= =YafW -----END PGP SIGNATURE----- --tU3eLu9lv81E4NFPAJFnDN4FrTsChRT7v--