new scsi-generic abstraction, use SG_IO (Christoph Hellwig)

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 <hch@lst.de> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6895 c046a42c-6fe2-441c-8c8c-71466251a162

new scsi-generic abstraction, use SG_IO (Christoph Hellwig)
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 <hch@lst.de> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6895 c046a42c-6fe2-441c-8c8c-71466251a162
aliguori
1 parent 64a7fde8
Showing 7 changed files with 115 additions and 137 deletions
block-raw-posix.c
block.c
block.h
block_int.h
hw/scsi-generic.c
posix-aio-compat.c
posix-aio-compat.h
@@ -1209,6 +1209,26 @@ static int raw_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
     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;
+}
+
 #elif defined(__FreeBSD__)
 static int fd_open(BlockDriverState *bs)
@@ -1349,33 +1369,14 @@ static int raw_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
 {
     return -ENOTSUP;
 }
-#endif /* !linux && !FreeBSD */
-
-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)
-{
-    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);
-}
-static BlockDriverAIOCB *raw_sg_aio_write(BlockDriverState *bs,
-                                          void *buf, int count,
-                                          BlockDriverCompletionFunc *cb,
-                                          void *opaque)
+static BlockDriverAIOCB *raw_aio_ioctl(BlockDriverState *bs,
+        unsigned long int req, void *buf,
+        BlockDriverCompletionFunc *cb, void *opaque)
 {
-    return raw_aio_write(bs, 0, buf, -(int64_t)count, cb, opaque);
+    return -ENOTSUP;
 }
+#endif /* !linux && !FreeBSD */
 BlockDriver bdrv_host_device = {
     .format_name	= "host_device",
@@ -1402,8 +1403,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,
 };
@@ -1633,24 +1633,13 @@ int bdrv_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
     return -ENOTSUP;
 }
-int bdrv_sg_send_command(BlockDriverState *bs, void *buf, int count)
-{
-    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);
-}
-
-BlockDriverAIOCB *bdrv_sg_aio_read(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)
 {
-    return bs->drv->bdrv_sg_aio_read(bs, buf, count, cb, opaque);
-}
+    BlockDriver *drv = bs->drv;
-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;
 }
@@ -102,12 +102,10 @@ BlockDriverAIOCB *bdrv_aio_write(BlockDriverState *bs, int64_t sector_num,
 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(BlockDriverState *bs, const char *snapshot_id);
 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);
@@ -86,16 +86,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);
     AIOPool aio_pool;
     struct BlockDriver *next;
@@ -201,8 +201,6 @@ static int execute_command(BlockDriverState *bdrv,
                            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(BlockDriverState *bdrv,
     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(BlockDriverState *bdrv)
     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(BlockDriverState *bdrv)
     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;
@@ -11,6 +11,7 @@
  *
  */
+#include <sys/ioctl.h>
 #include <pthread.h>
 #include <unistd.h>
 #include <errno.h>
@@ -75,6 +76,47 @@ static void thread_create(pthread_t *thread, pthread_attr_t *attr,
     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 *aioinit)
     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_paiocb *aiocb, int is_write)
 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)
@@ -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);