Re: [Qemu-devel] Re: [PATCH 3/3] block/nbd: Make the NBD block device use the AIO interface

[Stefan Hajnoczi]

On Tue, Feb 22, 2011 at 11:48 AM, Nicholas Thomas <address@hidden> wrote:
>
>> > + * 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 for
>> > + * the response.  The responses are received in the `aio_read_response'
>> > + * 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 
>> > AIOReqs
>> > + */
>> > +static void nbd_readv_writev_bh_cb(void *p)
>> >  {
>> > -    BDRVNBDState *s = bs->opaque;
>> > -    struct nbd_request request;
>> > +    NBDAIOCB *acb = p;
>> > +    int ret = 0;
>> >
>> > -    request.type = NBD_CMD_DISC;
>> > -    request.handle = (uint64_t)(intptr_t)bs;
>> > -    request.from = 0;
>> > -    request.len = 0;
>> > -    nbd_send_request(s->sock, &request);
>> > +    size_t len, done = 0;
>> > +    size_t total = acb->nb_sectors * SECTOR_SIZE;
>> > +
>> > +    /* Where the read/write starts from */
>> > +    size_t offset = acb->sector_num * SECTOR_SIZE;
>>
>> On 32-bit hosts size_t is only 32 bits wide.  It's not suitable for
>> storing the byte offset into the device.  Please use off_t.
>>
>> > +    BDRVNBDState *s = acb->common.bs->opaque;
>>
>> I don't follow this.  You have no guarantee that acb->common.bs->opaque
>> 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