On Fri, Feb 18, 2011 at 12:55:29PM +0000, Nick Thomas wrote: > +static inline AIOReq *alloc_aio_req(BDRVNBDState *s, NBDAIOCB *acb, > + size_t data_len, > + off_t offset, > + off_t iov_offset) > +{ > + AIOReq *aio_req; > > - if (reply.handle != request.handle) > + aio_req = qemu_malloc(sizeof(*aio_req)); > + aio_req->aiocb = acb; > + aio_req->iov_offset = iov_offset; > + aio_req->offset = offset; > + aio_req->data_len = data_len; > + aio_req->handle = s->aioreq_seq_num++; /* FIXME: Trivially guessable */
Does it matter at all that this number is guessable? I think we can drop the FIXME comment. > +/* > + * 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 *. > + > + AIOReq *aio_req; > > - close(s->sock); > + qemu_bh_delete(acb->bh); > + acb->bh = NULL; > + > + while (done != total) { > + len = (total - done); > + > + /* Split write requests into 1MiB segments */ > + if(acb->aiocb_type == AIOCB_WRITE_UDATA && len > MAX_NBD_WRITE) { > + len = MAX_NBD_WRITE; > + } We need to be more conservative than 1 MB because qemu-nbd.c checks the following: if (request.len + NBD_REPLY_SIZE > data_size) { LOG("len (%u) is larger than max len (%u)", request.len + NBD_REPLY_SIZE, data_size); errno = EINVAL; return -1; } and NBD_REPLY_SIZE is 16. Stefan