From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41829) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c99Vz-00027x-8Y for qemu-devel@nongnu.org; Tue, 22 Nov 2016 06:45:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c99Vy-0004nI-8P for qemu-devel@nongnu.org; Tue, 22 Nov 2016 06:45:31 -0500 Date: Tue, 22 Nov 2016 12:45:20 +0100 From: Kevin Wolf Message-ID: <20161122114520.GC5615@noname.redhat.com> References: <1479749488-31808-1-git-send-email-kwolf@redhat.com> <1479749488-31808-8-git-send-email-kwolf@redhat.com> <122bc834-8ab3-ce9d-2b22-8459afe6feee@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="i0/AhcQY5QxfSsSZ" Content-Disposition: inline In-Reply-To: <122bc834-8ab3-ce9d-2b22-8459afe6feee@redhat.com> Subject: Re: [Qemu-devel] [PATCH 7/8] quorum: Implement .bdrv_co_preadv/pwritev() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-block@nongnu.org, berto@igalia.com, mreitz@redhat.com, qemu-devel@nongnu.org --i0/AhcQY5QxfSsSZ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 21.11.2016 um 21:04 hat Eric Blake geschrieben: > On 11/21/2016 11:31 AM, Kevin Wolf wrote: > > This enables byte granularity requests on quorum nodes. > >=20 > > Note that the QMP events emitted by the driver are an external API that > > we were careless enough to define as sector based. The offset and length > > of requests reported in events are rounded therefore. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > block/quorum.c | 79 ++++++++++++++++++++++++++------------------------= -------- > > 1 file changed, 36 insertions(+), 43 deletions(-) > >=20 >=20 > > -static void quorum_report_bad(QuorumOpType type, uint64_t sector_num, > > - int nb_sectors, char *node_name, int ret) > > +static void quorum_report_bad(QuorumOpType type, uint64_t offset, > > + uint64_t bytes, char *node_name, int ret) > > { > > const char *msg =3D NULL; > > + int64_t start_sector =3D offset / BDRV_SECTOR_SIZE; > > + int64_t end_sector =3D DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SI= ZE); >=20 > This one looks correct, >=20 > > + > > if (ret < 0) { > > msg =3D strerror(-ret); > > } > > =20 > > - qapi_event_send_quorum_report_bad(type, !!msg, msg, node_name, > > - sector_num, nb_sectors, &error_a= bort); > > + qapi_event_send_quorum_report_bad(type, !!msg, msg, node_name, sta= rt_sector, > > + end_sector - start_sector, &erro= r_abort); > > } > > =20 > > static void quorum_report_failure(QuorumAIOCB *acb) > > { > > const char *reference =3D bdrv_get_device_or_node_name(acb->bs); > > - qapi_event_send_quorum_failure(reference, acb->sector_num, > > - acb->nb_sectors, &error_abort); > > + qapi_event_send_quorum_failure(reference, > > + acb->offset / BDRV_SECTOR_SIZE, > > + acb->bytes / BDRV_SECTOR_SIZE, >=20 > but this one still looks like it could give unexpected results for > acb->bytes < BDRV_SECTOR_SIZE. Thanks, I missed this one. I'll send a v2. > > -static int quorum_co_readv(BlockDriverState *bs, > > - int64_t sector_num, int nb_sectors, > > - QEMUIOVector *qiov) > > +static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset, > > + uint64_t bytes, QEMUIOVector *qiov, int fl= ags) > > { >=20 > Is it worth adding assert(!flags)? For now, the block layer doesn't > have any defined flags (and if it did, we'd probably want to add a > .supported_read_flags to parallel the existing .supported_write_flags). I don't think we need to assert this, no other driver does that. We have =2Esupported_write_flags and I would indeed add .supported_read_flags if/when we start using flags for read requests, so we can be reasonably sure that only those flags are set even without asserting it. > [Huh - side thought: right now, we don't have any defined semantics for > BDRV_REQUEST_FUA on reads (although we modeled it in part after SCSI, > which does have it defined for reads). But quorum rewrites on read > might be an interesting application of where we can trigger a write > during reads, and where we may want to guarantee FUA semantics on those > writes, thus making a potentially plausible use of the flag on read] Makes sense to me, but that's something for a different series. And actually, I'm not sure who would even send a read with FUA set today. Can this even happen yet? Kevin --i0/AhcQY5QxfSsSZ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJYNC/QAAoJEH8JsnLIjy/WVMMP/jiqNW/apDOb8772waFp0w5V S5ffZGXuioGGHbXoPBt+o64ynaKpYXGTvLWvX/7K+2fZYIWoVqqrpHiKcknJeCtg FyjiDQsLdNTbkqvojRiiotlPY2RtBcif8nP+lmOJlRPT1FHotUupawi4js4cFPVW dqPUC/I6QxHZatM0VwJziELC6RvRM6usfUC9GAbG2zKZBsP25sxF5x3unJavQrLE OOW9f5Ob3s3L/AhOT+3TIB+1sAjeQ8HjaNl/cmRCB061hricgCvipIdjzOE31wzU SWGkM0Ozys+VEYk61CXLIi8dlW6NDik2F10+6BGcFuEBM+cBH1ZJkAqLdBG9sDE7 +LFtL/mPCe8vkn4XTG2HFrgHAygjT7Eiy3P0O0VuML0t+joZFj2PoGq1V+9nuEts QJPcb1gH16Z4ClJyz3OeqSjEXbCm6njqOCQpwpsEe8qz7Wz/Q31jsJ78ErP0Kbuj qUc8rAJwK99H+q80nTDRGadvYePNxVmUxzskrZ37HWthAQXMyLAVUdHKAohUhAt4 JCLHK11T6SEotAYxTZF5DG3FQiG8NSFXzxuDpsu9zlfPgwbIB5St6Yjn5swlbulU nW7Lra+FWbZ0qeoe0+rk7wV2rj2iqPexztLDLS0sPbU78vcFqeTpXND+gAGs3V+X fHTpPTQS8scR1zTqSntH =Jhfu -----END PGP SIGNATURE----- --i0/AhcQY5QxfSsSZ--