From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=48705 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PrsLL-0008CB-At
	for qemu-devel@nongnu.org; Tue, 22 Feb 2011 08:31:56 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1PrsLG-00009M-CX
	for qemu-devel@nongnu.org; Tue, 22 Feb 2011 08:31:51 -0500
Received: from mail-yw0-f45.google.com ([209.85.213.45]:53054)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1PrsLG-00009I-8W
	for qemu-devel@nongnu.org; Tue, 22 Feb 2011 08:31:50 -0500
Received: by ywl41 with SMTP id 41so1018196ywl.4
	for <qemu-devel@nongnu.org>; Tue, 22 Feb 2011 05:31:49 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <1298375311.17006.165.camel@desk4.office.bytemark.co.uk>
References: <1298033729-17945-1-git-send-email-nick@bytemark.co.uk>
	<1298033729-17945-3-git-send-email-nick@bytemark.co.uk>
	<AANLkTi=CjNy2S0kbH=Wq8qQHh8OsuNw=ot+yMaYVLCtO@mail.gmail.com>
	<1298375311.17006.165.camel@desk4.office.bytemark.co.uk>
Date: Tue, 22 Feb 2011 13:31:49 +0000
Message-ID: <AANLkTimgBhceVocYDMKsKw--TBorvMvYE9O3cFENbRqT@mail.gmail.com>
Subject: Re: [Qemu-devel] Re: [PATCH 3/3] block/nbd: Make the NBD block device
	use the AIO interface
From: Stefan Hajnoczi <stefanha@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Nicholas Thomas <nick@bytemark.co.uk>
Cc: kwolf@redhat.com, qemu-devel@nongnu.org

On Tue, Feb 22, 2011 at 11:48 AM, Nicholas Thomas <nick@bytemark.co.uk> wro=
te:
>
>> > + * Send I/O requests to the server.
>> > + *
>> > + * This function sends requests to the server, links the requests to
>> > + * the outstanding_list in BDRVNBDState, and exits without waiting fo=
r
>> > + * the response. =A0The responses are received in the `aio_read_respo=
nse'
>> > + * function which is called from the main loop as a fd handler.
>> > + * If this is a write request and it's >1MB, split it into multiple A=
IOReqs
>> > + */
>> > +static void nbd_readv_writev_bh_cb(void *p)
>> > =A0{
>> > - =A0 =A0BDRVNBDState *s =3D bs->opaque;
>> > - =A0 =A0struct nbd_request request;
>> > + =A0 =A0NBDAIOCB *acb =3D p;
>> > + =A0 =A0int ret =3D 0;
>> >
>> > - =A0 =A0request.type =3D NBD_CMD_DISC;
>> > - =A0 =A0request.handle =3D (uint64_t)(intptr_t)bs;
>> > - =A0 =A0request.from =3D 0;
>> > - =A0 =A0request.len =3D 0;
>> > - =A0 =A0nbd_send_request(s->sock, &request);
>> > + =A0 =A0size_t len, done =3D 0;
>> > + =A0 =A0size_t total =3D acb->nb_sectors * SECTOR_SIZE;
>> > +
>> > + =A0 =A0/* Where the read/write starts from */
>> > + =A0 =A0size_t offset =3D acb->sector_num * SECTOR_SIZE;
>>
>> On 32-bit hosts size_t is only 32 bits wide. =A0It's not suitable for
>> storing the byte offset into the device. =A0Please use off_t.
>>
>> > + =A0 =A0BDRVNBDState *s =3D acb->common.bs->opaque;
>>
>> I don't follow this. =A0You have no guarantee that acb->common.bs->opaqu=
e
>> is the BDRVNBDState *.
>
> Another idiom from sheepdog. Seems qed, qcow2, qcow & vdi do the same.

My mistake, I ignored the "bs->".  This is fine but I read it as
acb->common.opaque.

Stefan