From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Ljvws-0000xj-Ls for qemu-devel@nongnu.org; Wed, 18 Mar 2009 09:36:46 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Ljvwn-0000xH-2Y for qemu-devel@nongnu.org; Wed, 18 Mar 2009 09:36:45 -0400 Received: from [199.232.76.173] (port=57355 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Ljvwm-0000xD-T9 for qemu-devel@nongnu.org; Wed, 18 Mar 2009 09:36:40 -0400 Received: from verein.lst.de ([213.95.11.210]:49340) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.60) (envelope-from ) id 1Ljvwm-0004qD-6X for qemu-devel@nongnu.org; Wed, 18 Mar 2009 09:36:40 -0400 Date: Wed, 18 Mar 2009 14:36:33 +0100 From: Christoph Hellwig Message-ID: <20090318133633.GA24254@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] [PATCH] new scsi-generic abstraction, use SG_IO Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Brook , Avi Kivity , qemu-devel@nongnu.org Okay, I started looking into how to handle scsi-generic I/O in the new world order. I think the best is to use the SG_IO ioctl instead of the read/write interface as that allows us to support scsi passthrough on disk/cdrom devices, too. See Hannes patch on the kvm list from August for an example. Now that we always do ioctls we don't need another abstraction than bdrv_ioctl for the synchronous requests for now, and for asynchronous requests I've added a aio_ioctl abstraction keeping it simple. Long-term we might want to move the ops to a higher-level abstraction and let the low-level code fill out the request header, but I'm lazy enough to leave that to the people trying to support scsi-passthrough on a non-Linux OS. Tested lightly by issuing various sg_ commands from sg3-utils in a guest to a host CDROM device. Signed-off-by: Christoph Hellwig Index: qemu/block-raw-posix.c =================================================================== --- qemu.orig/block-raw-posix.c 2009-03-18 14:24:29.970978694 +0100 +++ qemu/block-raw-posix.c 2009-03-18 14:30:55.982934019 +0100 @@ -1164,6 +1164,26 @@ static int raw_ioctl(BlockDriverState *b return ioctl(s->fd, req, buf); } + +static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, + unsigned long int req, void *buf, + BlockDriverCompletionFunc *cb, void *opaque) +{ + RawAIOCB *acb; + + acb = raw_aio_setup(bs, 0, buf, 0, cb, opaque); + if (!acb) + return NULL; + + acb->aiocb.aio_ioctl_cmd = req; + if (qemu_paio_ioctl(&acb->aiocb) < 0) { + raw_aio_remove(acb); + return NULL; + } + + return &acb->common; +} + #else static int fd_open(BlockDriverState *bs) @@ -1195,33 +1215,15 @@ static int raw_ioctl(BlockDriverState *b { return -ENOTSUP; } -#endif /* !linux */ - -static int raw_sg_send_command(BlockDriverState *bs, void *buf, int count) -{ - return raw_pwrite(bs, -1, buf, count); -} -static int raw_sg_recv_response(BlockDriverState *bs, void *buf, int count) +static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs, + unsigned long int req, void *buf, + BlockDriverCompletionFunc *cb, void *opaque) { - return raw_pread(bs, -1, buf, count); -} - -static BlockDriverAIOCB *raw_sg_aio_read(BlockDriverState *bs, - void *buf, int count, - BlockDriverCompletionFunc *cb, - void *opaque) -{ - return raw_aio_read(bs, 0, buf, -(int64_t)count, cb, opaque); + return -ENOTSUP; } -static BlockDriverAIOCB *raw_sg_aio_write(BlockDriverState *bs, - void *buf, int count, - BlockDriverCompletionFunc *cb, - void *opaque) -{ - return raw_aio_write(bs, 0, buf, -(int64_t)count, cb, opaque); -} +#endif /* !linux */ BlockDriver bdrv_host_device = { .format_name = "host_device", @@ -1248,8 +1250,5 @@ BlockDriver bdrv_host_device = { .bdrv_set_locked = raw_set_locked, /* generic scsi device */ .bdrv_ioctl = raw_ioctl, - .bdrv_sg_send_command = raw_sg_send_command, - .bdrv_sg_recv_response = raw_sg_recv_response, - .bdrv_sg_aio_read = raw_sg_aio_read, - .bdrv_sg_aio_write = raw_sg_aio_write, + .bdrv_aio_ioctl = raw_aio_ioctl, }; Index: qemu/block.c =================================================================== --- qemu.orig/block.c 2009-03-18 14:24:29.974978644 +0100 +++ qemu/block.c 2009-03-18 14:27:46.842978340 +0100 @@ -1601,24 +1601,13 @@ int bdrv_ioctl(BlockDriverState *bs, uns return -ENOTSUP; } -int bdrv_sg_send_command(BlockDriverState *bs, void *buf, int count) +BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, + unsigned long int req, void *buf, + BlockDriverCompletionFunc *cb, void *opaque) { - return bs->drv->bdrv_sg_send_command(bs, buf, count); -} - -int bdrv_sg_recv_response(BlockDriverState *bs, void *buf, int count) -{ - return bs->drv->bdrv_sg_recv_response(bs, buf, count); -} + BlockDriver *drv = bs->drv; -BlockDriverAIOCB *bdrv_sg_aio_read(BlockDriverState *bs, void *buf, int count, - BlockDriverCompletionFunc *cb, void *opaque) -{ - return bs->drv->bdrv_sg_aio_read(bs, buf, count, cb, opaque); -} - -BlockDriverAIOCB *bdrv_sg_aio_write(BlockDriverState *bs, void *buf, int count, - BlockDriverCompletionFunc *cb, void *opaque) -{ - return bs->drv->bdrv_sg_aio_write(bs, buf, count, cb, opaque); + if (drv && drv->bdrv_aio_ioctl) + return drv->bdrv_aio_ioctl(bs, req, buf, cb, opaque); + return NULL; } Index: qemu/block.h =================================================================== --- qemu.orig/block.h 2009-03-18 14:24:29.978978316 +0100 +++ qemu/block.h 2009-03-18 14:27:46.843978188 +0100 @@ -102,12 +102,10 @@ BlockDriverAIOCB *bdrv_aio_write(BlockDr void bdrv_aio_cancel(BlockDriverAIOCB *acb); /* sg packet commands */ -int bdrv_sg_send_command(BlockDriverState *bs, void *buf, int count); -int bdrv_sg_recv_response(BlockDriverState *bs, void *buf, int count); -BlockDriverAIOCB *bdrv_sg_aio_read(BlockDriverState *bs, void *buf, int count, - BlockDriverCompletionFunc *cb, void *opaque); -BlockDriverAIOCB *bdrv_sg_aio_write(BlockDriverState *bs, void *buf, int count, - BlockDriverCompletionFunc *cb, void *opaque); +int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf); +BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs, + unsigned long int req, void *buf, + BlockDriverCompletionFunc *cb, void *opaque); /* Ensure contents are flushed to disk. */ void bdrv_flush(BlockDriverState *bs); @@ -169,7 +167,6 @@ int bdrv_snapshot_delete(BlockDriverStat int bdrv_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_info); char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn); -int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf); char *get_human_readable_size(char *buf, int buf_size, int64_t size); int path_is_absolute(const char *path); Index: qemu/block_int.h =================================================================== --- qemu.orig/block_int.h 2009-03-18 14:24:29.983978743 +0100 +++ qemu/block_int.h 2009-03-18 14:27:46.843978188 +0100 @@ -80,16 +80,9 @@ struct BlockDriver { /* to control generic scsi devices */ int (*bdrv_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf); - int (*bdrv_sg_send_command)(BlockDriverState *bs, void *buf, int count); - int (*bdrv_sg_recv_response)(BlockDriverState *bs, void *buf, int count); - BlockDriverAIOCB *(*bdrv_sg_aio_read)(BlockDriverState *bs, - void *buf, int count, - BlockDriverCompletionFunc *cb, - void *opaque); - BlockDriverAIOCB *(*bdrv_sg_aio_write)(BlockDriverState *bs, - void *buf, int count, - BlockDriverCompletionFunc *cb, - void *opaque); + BlockDriverAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs, + unsigned long int req, void *buf, + BlockDriverCompletionFunc *cb, void *opaque); BlockDriverAIOCB *free_aiocb; struct BlockDriver *next; Index: qemu/hw/scsi-generic.c =================================================================== --- qemu.orig/hw/scsi-generic.c 2009-03-18 14:24:29.988979030 +0100 +++ qemu/hw/scsi-generic.c 2009-03-18 14:27:46.844978943 +0100 @@ -201,8 +201,6 @@ static int execute_command(BlockDriverSt SCSIRequest *r, int direction, BlockDriverCompletionFunc *complete) { - int ret; - r->io_header.interface_id = 'S'; r->io_header.dxfer_direction = direction; r->io_header.dxferp = r->buf; @@ -215,27 +213,7 @@ static int execute_command(BlockDriverSt r->io_header.usr_ptr = r; r->io_header.flags |= SG_FLAG_DIRECT_IO; - ret = bdrv_sg_send_command(bdrv, &r->io_header, sizeof(r->io_header)); - if (ret < 0) { - BADF("execute_command: write failed ! (%d)\n", errno); - return -1; - } - if (complete == NULL) { - int ret; - r->aiocb = NULL; - while ((ret = bdrv_sg_recv_response(bdrv, &r->io_header, - sizeof(r->io_header))) < 0 && - ret == -EINTR) - ; - if (ret < 0) { - BADF("execute_command: read failed !\n"); - return -1; - } - return 0; - } - - r->aiocb = bdrv_sg_aio_read(bdrv, (uint8_t*)&r->io_header, - sizeof(r->io_header), complete, r); + r->aiocb = bdrv_aio_ioctl(bdrv, SG_IO, &r->io_header, complete, r); if (r->aiocb == NULL) { BADF("execute_command: read failed !\n"); return -1; @@ -637,14 +615,7 @@ static int get_blocksize(BlockDriverStat io_header.sbp = sensebuf; io_header.timeout = 6000; /* XXX */ - ret = bdrv_sg_send_command(bdrv, &io_header, sizeof(io_header)); - if (ret < 0) - return -1; - - while ((ret = bdrv_sg_recv_response(bdrv, &io_header, sizeof(io_header))) < 0 && - ret == -EINTR) - ; - + ret = bdrv_ioctl(bdrv, SG_IO, &io_header); if (ret < 0) return -1; @@ -675,14 +646,7 @@ static int get_stream_blocksize(BlockDri io_header.sbp = sensebuf; io_header.timeout = 6000; /* XXX */ - ret = bdrv_sg_send_command(bdrv, &io_header, sizeof(io_header)); - if (ret < 0) - return -1; - - while ((ret = bdrv_sg_recv_response(bdrv, &io_header, sizeof(io_header))) < 0 && - ret == -EINTR) - ; - + ret = bdrv_ioctl(bdrv, SG_IO, &io_header); if (ret < 0) return -1; Index: qemu/posix-aio-compat.c =================================================================== --- qemu.orig/posix-aio-compat.c 2009-03-18 14:24:29.994978746 +0100 +++ qemu/posix-aio-compat.c 2009-03-18 14:27:46.844978943 +0100 @@ -11,6 +11,7 @@ * */ +#include #include #include #include @@ -75,6 +76,47 @@ static void thread_create(pthread_t *thr if (ret) die2(ret, "pthread_create"); } +static size_t handle_aiocb_readwrite(struct qemu_paiocb *aiocb) +{ + size_t offset = 0; + ssize_t len; + + while (offset < aiocb->aio_nbytes) { + if (aiocb->aio_type == QEMU_PAIO_WRITE) + len = pwrite(aiocb->aio_fildes, + (const char *)aiocb->aio_buf + offset, + aiocb->aio_nbytes - offset, + aiocb->aio_offset + offset); + else + len = pread(aiocb->aio_fildes, + (char *)aiocb->aio_buf + offset, + aiocb->aio_nbytes - offset, + aiocb->aio_offset + offset); + + if (len == -1 && errno == EINTR) + continue; + else if (len == -1) { + offset = -errno; + break; + } else if (len == 0) + break; + + offset += len; + } + + return offset; +} + +static size_t handle_aiocb_ioctl(struct qemu_paiocb *aiocb) +{ + int ret; + + ret = ioctl(aiocb->aio_fildes, aiocb->aio_ioctl_cmd, aiocb->aio_buf); + if (ret == -1) + return -errno; + return ret; +} + static void *aio_thread(void *unused) { pid_t pid; @@ -88,8 +130,7 @@ static void *aio_thread(void *unused) while (1) { struct qemu_paiocb *aiocb; - size_t offset; - int ret = 0; + size_t ret = 0; qemu_timeval tv; struct timespec ts; @@ -109,40 +150,26 @@ static void *aio_thread(void *unused) aiocb = TAILQ_FIRST(&request_list); TAILQ_REMOVE(&request_list, aiocb, node); - - offset = 0; aiocb->active = 1; - idle_threads--; mutex_unlock(&lock); - while (offset < aiocb->aio_nbytes) { - ssize_t len; - - if (aiocb->is_write) - len = pwrite(aiocb->aio_fildes, - (const char *)aiocb->aio_buf + offset, - aiocb->aio_nbytes - offset, - aiocb->aio_offset + offset); - else - len = pread(aiocb->aio_fildes, - (char *)aiocb->aio_buf + offset, - aiocb->aio_nbytes - offset, - aiocb->aio_offset + offset); - - if (len == -1 && errno == EINTR) - continue; - else if (len == -1) { - offset = -errno; - break; - } else if (len == 0) - break; - - offset += len; - } + switch (aiocb->aio_type) { + case QEMU_PAIO_READ: + case QEMU_PAIO_WRITE: + ret = handle_aiocb_readwrite(aiocb); + break; + case QEMU_PAIO_IOCTL: + ret = handle_aiocb_ioctl(aiocb); + break; + default: + fprintf(stderr, "invalid aio request (0x%x)\n", aiocb->aio_type); + ret = -EINVAL; + break; + } mutex_lock(&lock); - aiocb->ret = offset; + aiocb->ret = ret; idle_threads++; mutex_unlock(&lock); @@ -178,9 +205,9 @@ int qemu_paio_init(struct qemu_paioinit return 0; } -static int qemu_paio_submit(struct qemu_paiocb *aiocb, int is_write) +static int qemu_paio_submit(struct qemu_paiocb *aiocb, int type) { - aiocb->is_write = is_write; + aiocb->aio_type = type; aiocb->ret = -EINPROGRESS; aiocb->active = 0; mutex_lock(&lock); @@ -195,12 +222,17 @@ static int qemu_paio_submit(struct qemu_ int qemu_paio_read(struct qemu_paiocb *aiocb) { - return qemu_paio_submit(aiocb, 0); + return qemu_paio_submit(aiocb, QEMU_PAIO_READ); } int qemu_paio_write(struct qemu_paiocb *aiocb) { - return qemu_paio_submit(aiocb, 1); + return qemu_paio_submit(aiocb, QEMU_PAIO_WRITE); +} + +int qemu_paio_ioctl(struct qemu_paiocb *aiocb) +{ + return qemu_paio_submit(aiocb, QEMU_PAIO_IOCTL); } ssize_t qemu_paio_return(struct qemu_paiocb *aiocb) Index: qemu/posix-aio-compat.h =================================================================== --- qemu.orig/posix-aio-compat.h 2009-03-18 14:24:29.999978685 +0100 +++ qemu/posix-aio-compat.h 2009-03-18 14:27:46.844978943 +0100 @@ -29,12 +29,16 @@ struct qemu_paiocb int aio_fildes; void *aio_buf; size_t aio_nbytes; +#define aio_ioctl_cmd aio_nbytes /* for QEMU_PAIO_IOCTL */ int ev_signo; off_t aio_offset; /* private */ TAILQ_ENTRY(qemu_paiocb) node; - int is_write; + int aio_type; +#define QEMU_PAIO_READ 0x01 +#define QEMU_PAIO_WRITE 0x02 +#define QEMU_PAIO_IOCTL 0x03 ssize_t ret; int active; }; @@ -49,6 +53,7 @@ struct qemu_paioinit int qemu_paio_init(struct qemu_paioinit *aioinit); int qemu_paio_read(struct qemu_paiocb *aiocb); int qemu_paio_write(struct qemu_paiocb *aiocb); +int qemu_paio_ioctl(struct qemu_paiocb *aiocb); int qemu_paio_error(struct qemu_paiocb *aiocb); ssize_t qemu_paio_return(struct qemu_paiocb *aiocb); int qemu_paio_cancel(int fd, struct qemu_paiocb *aiocb);