All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Paul Brook <paul@codesourcery.com>, Avi Kivity <avi@redhat.com>,
	qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH] new scsi-generic abstraction, use SG_IO
Date: Wed, 18 Mar 2009 14:36:33 +0100	[thread overview]
Message-ID: <20090318133633.GA24254@lst.de> (raw)

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>

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 <sys/ioctl.h>
 #include <pthread.h>
 #include <unistd.h>
 #include <errno.h>
@@ -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);

             reply	other threads:[~2009-03-18 13:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-18 13:36 Christoph Hellwig [this message]
2009-03-18 13:46 ` [Qemu-devel] Re: [PATCH] new scsi-generic abstraction, use SG_IO Avi Kivity
2009-03-18 13:52   ` Christoph Hellwig
2009-03-28 17:30 ` [Qemu-devel] " Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090318133633.GA24254@lst.de \
    --to=hch@lst.de \
    --cc=avi@redhat.com \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.