On 11/24/2010 05:52 PM, Christoph Hellwig wrote: > On Wed, Nov 24, 2010 at 12:16:08PM +0100, Hannes Reinecke wrote: >> Add callback to create a request with a predefined iovec. >> This is required for drivers which can use the iovec >> of a command directly. > > What happend to my comment that the iov and non-iov case should share > code? Also what happened to the other comment about not naming the > method implementation different from the method name. > Looked into it. Sure I could be doing it for scsi-disk; for scsi-generic it won't work, though. And it's not much of a code-share to have from it; you'll end up with something like:
static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun) { struct iovec *iov; uint8_t *buf; SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); SCSIRequest *req; SCSIDiskReq *r; buf = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE); iov = qemu_mallocz(sizeof(struct iovec)); iov[0].iov_base = buf; req = scsi_new_request_iovec(d, tag, lun, iov, 1); r = DO_UPCAST(SCSIDiskReq, req, req); r->iov_buf = buf; return req; } which doesn't look better than the original: static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d); SCSIRequest *req; SCSIDiskReq *r; req = scsi_req_alloc(sizeof(SCSIDiskReq), d, tag, lun); r = DO_UPCAST(SCSIDiskReq, req, req); r->iov_buf = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE); r->iov = qemu_mallocz(sizeof(struct iovec)); r->iov[0].iov_base = r->iov_buf; r->iov_num = 1; return req; } But I'm open to suggestions. And for the naming: The SCSI stack is using 'req' for every function accepting SCSIRequest as an argument: hw/scsi.h: SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag, uint32_t lun); void scsi_req_free(SCSIRequest *req); int scsi_req_parse(SCSIRequest *req, uint8_t *buf); void scsi_req_print(SCSIRequest *req); void scsi_req_complete(SCSIRequest *req); So using 'alloc_req' and 'free_req' is in line with this naming scheme. Using 'alloc_request' and 'free_request' really looked odd in the light of the general usage. Hence I didn't do it. But again, I'm open to suggestions here. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Markus Rex, HRB 16746 (AG Nürnberg)