Patchwork [2/4] virtio-scsi: Add basic request processing infrastructure

login
register
mail settings
Submitter Paolo Bonzini
Date Nov. 15, 2011, 5 p.m.
Message ID <1321376405-22776-3-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/125831/
State New
Headers show

Comments

Paolo Bonzini - Nov. 15, 2011, 5 p.m.
From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/virtio-scsi.c |  138 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 136 insertions(+), 2 deletions(-)
Zhiyong Wu - Nov. 16, 2011, 8:28 a.m.
On Wed, Nov 16, 2011 at 1:00 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio-scsi.c |  138 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 136 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
> index ff86376..7e6348a 100644
> --- a/hw/virtio-scsi.c
> +++ b/hw/virtio-scsi.c
> @@ -127,14 +127,148 @@ typedef struct {
>     uint32_t cdb_size;
>  } VirtIOSCSI;
>
> +typedef struct VirtIOSCSIReq {
> +    struct VirtIOSCSIReq *next;
> +    VirtIOSCSI *dev;
> +    VirtQueue *vq;
> +    VirtQueueElement elem;
> +    QEMUSGList qsgl;
> +    SCSIRequest *sreq;
> +    union {
> +        char                  *buf;
> +        VirtIOSCSICmdReq      *cmd;
> +        VirtIOSCSICtrlTMFReq  *tmf;
> +        VirtIOSCSICtrlANReq   *an;
> +    } req;
> +    union {
> +        char                  *buf;
> +        VirtIOSCSICmdResp     *cmd;
> +        VirtIOSCSICtrlTMFResp *tmf;
> +        VirtIOSCSICtrlANResp  *an;
> +        VirtIOSCSIEvent       *event;
> +    } resp;
> +} VirtIOSCSIReq;
> +
> +static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
> +{
> +    VirtQueue *vq = req->vq;
> +    virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
> +    qemu_sglist_destroy(&req->qsgl);
> +    if (req->sreq) {
> +        req->sreq->hba_private = NULL;
> +        scsi_req_unref(req->sreq);
> +    }
> +    g_free(req);
> +    virtio_notify(&req->dev->vdev, vq);
req is used-after-free?
> +}
> +
> +static void virtio_scsi_bad_req(void)
> +{
> +    error_report("wrong size for virtio-scsi headers");
> +    exit(1);
> +}
> +
> +static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg,
> +                                   target_phys_addr_t *addr, int num)
> +{
> +    memset(qsgl, 0, sizeof(*qsgl));
> +    while (num--) {
> +        qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
> +    }
> +}
> +
> +static VirtIOSCSIReq *virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq)
> +{
> +    VirtIOSCSIReq *req;
> +    req = g_malloc(sizeof(*req));
> +    if (!virtqueue_pop(vq, &req->elem)) {
> +        g_free(req);
> +        return NULL;
> +    }
> +
> +    assert(req->elem.out_num && req->elem.in_num);
> +    req->vq = vq;
> +    req->dev = s;
> +    req->next = NULL;
> +    req->sreq = NULL;
> +    req->req.buf = req->elem.out_sg[0].iov_base;
> +    req->resp.buf = req->elem.in_sg[0].iov_base;
> +
> +    if (req->elem.out_num > 1) {
> +        qemu_sgl_init_external(&req->qsgl, &req->elem.out_sg[1],
> +                               &req->elem.out_addr[1],
> +                               req->elem.out_num - 1);
> +    } else {
> +        qemu_sgl_init_external(&req->qsgl, &req->elem.in_sg[1],
> +                               &req->elem.in_addr[1],
> +                               req->elem.in_num - 1);
> +    }
> +
> +    return req;
> +}
> +
> +static void virtio_scsi_fail_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
> +{
> +    if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
> +        req->resp.tmf->response = VIRTIO_SCSI_S_FAILURE;
> +    } else {
> +        req->resp.an->response = VIRTIO_SCSI_S_FAILURE;
> +    }
> +
> +    virtio_scsi_complete_req(req);
> +}
> +
>  static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    /* TODO */
> +    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
> +    VirtIOSCSIReq *req;
> +
> +    while ((req = virtio_scsi_parse_req(s, vq))) {
> +        virtio_scsi_fail_ctrl_req(s, req);
> +    }
> +}
> +
> +static void virtio_scsi_fail_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
> +{
> +    req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE;
> +    virtio_scsi_complete_req(req);
>  }
>
>  static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    /* TODO */
> +    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
> +    VirtIOSCSIReq *req;
> +
> +    while ((req = virtio_scsi_parse_req(s, vq))) {
> +        int out_size, in_size;
> +        if (req->elem.out_num < 1 || req->elem.in_num < 1) {
> +            virtio_scsi_bad_req();
> +        }
> +
> +        out_size = req->elem.out_sg[0].iov_len;
> +        in_size = req->elem.in_sg[0].iov_len;
> +        if (out_size < sizeof(VirtIOSCSICmdReq) + VIRTIO_SCSI_CDB_SIZE ||
> +            in_size < sizeof(VirtIOSCSICmdResp) + VIRTIO_SCSI_SENSE_SIZE) {
> +            virtio_scsi_bad_req();
> +        }
> +
> +        if (req->elem.out_num > 1 && req->elem.in_num > 1) {
> +            virtio_scsi_fail_cmd_req(s, req);
> +            continue;
> +        }
> +
> +        req->resp.cmd->resid = 0;
> +        req->resp.cmd->status_qualifier = 0;
> +        req->resp.cmd->status = CHECK_CONDITION;
> +        req->resp.cmd->sense_len = 4;
> +        req->resp.cmd->sense[0] = 0xf0; /* Fixed format current sense */
> +        req->resp.cmd->sense[1] = ILLEGAL_REQUEST;
> +        req->resp.cmd->sense[2] = 0x20;
> +        req->resp.cmd->sense[3] = 0x00;
> +        req->resp.cmd->response = VIRTIO_SCSI_S_OK;
> +
> +        virtio_scsi_complete_req(req);
> +    }
>  }
>
>  static void virtio_scsi_get_config(VirtIODevice *vdev,
> --
> 1.7.7.1
>
>
>
>
Paolo Bonzini - Nov. 16, 2011, 8:29 a.m.
On 11/16/2011 09:28 AM, Zhi Yong Wu wrote:
>> >  +static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
>> >  +{
>> >  +    VirtQueue *vq = req->vq;
>> >  +    virtqueue_push(vq,&req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
>> >  +    qemu_sglist_destroy(&req->qsgl);
>> >  +    if (req->sreq) {
>> >  +        req->sreq->hba_private = NULL;
>> >  +        scsi_req_unref(req->sreq);
>> >  +    }
>> >  +    g_free(req);
>> >  +    virtio_notify(&req->dev->vdev, vq);
> req is used-after-free?

Yes, thanks for spotting it.

Paolo

Patch

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index ff86376..7e6348a 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -127,14 +127,148 @@  typedef struct {
     uint32_t cdb_size;
 } VirtIOSCSI;
 
+typedef struct VirtIOSCSIReq {
+    struct VirtIOSCSIReq *next;
+    VirtIOSCSI *dev;
+    VirtQueue *vq;
+    VirtQueueElement elem;
+    QEMUSGList qsgl;
+    SCSIRequest *sreq;
+    union {
+        char                  *buf;
+        VirtIOSCSICmdReq      *cmd;
+        VirtIOSCSICtrlTMFReq  *tmf;
+        VirtIOSCSICtrlANReq   *an;
+    } req;
+    union {
+        char                  *buf;
+        VirtIOSCSICmdResp     *cmd;
+        VirtIOSCSICtrlTMFResp *tmf;
+        VirtIOSCSICtrlANResp  *an;
+        VirtIOSCSIEvent       *event;
+    } resp;
+} VirtIOSCSIReq;
+
+static void virtio_scsi_complete_req(VirtIOSCSIReq *req)
+{
+    VirtQueue *vq = req->vq;
+    virtqueue_push(vq, &req->elem, req->qsgl.size + req->elem.in_sg[0].iov_len);
+    qemu_sglist_destroy(&req->qsgl);
+    if (req->sreq) {
+        req->sreq->hba_private = NULL;
+        scsi_req_unref(req->sreq);
+    }
+    g_free(req);
+    virtio_notify(&req->dev->vdev, vq);
+}
+
+static void virtio_scsi_bad_req(void)
+{
+    error_report("wrong size for virtio-scsi headers");
+    exit(1);
+}
+
+static void qemu_sgl_init_external(QEMUSGList *qsgl, struct iovec *sg,
+                                   target_phys_addr_t *addr, int num)
+{
+    memset(qsgl, 0, sizeof(*qsgl));
+    while (num--) {
+        qemu_sglist_add(qsgl, *(addr++), (sg++)->iov_len);
+    }
+}
+
+static VirtIOSCSIReq *virtio_scsi_parse_req(VirtIOSCSI *s, VirtQueue *vq)
+{
+    VirtIOSCSIReq *req;
+    req = g_malloc(sizeof(*req));
+    if (!virtqueue_pop(vq, &req->elem)) {
+        g_free(req);
+        return NULL;
+    }
+
+    assert(req->elem.out_num && req->elem.in_num);
+    req->vq = vq;
+    req->dev = s;
+    req->next = NULL;
+    req->sreq = NULL;
+    req->req.buf = req->elem.out_sg[0].iov_base;
+    req->resp.buf = req->elem.in_sg[0].iov_base;
+
+    if (req->elem.out_num > 1) {
+        qemu_sgl_init_external(&req->qsgl, &req->elem.out_sg[1],
+                               &req->elem.out_addr[1],
+                               req->elem.out_num - 1);
+    } else {
+        qemu_sgl_init_external(&req->qsgl, &req->elem.in_sg[1],
+                               &req->elem.in_addr[1],
+                               req->elem.in_num - 1);
+    }
+
+    return req;
+}
+
+static void virtio_scsi_fail_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
+{
+    if (req->req.tmf->type == VIRTIO_SCSI_T_TMF) {
+        req->resp.tmf->response = VIRTIO_SCSI_S_FAILURE;
+    } else {
+        req->resp.an->response = VIRTIO_SCSI_S_FAILURE;
+    }
+
+    virtio_scsi_complete_req(req);
+}
+
 static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
 {
-    /* TODO */
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    VirtIOSCSIReq *req;
+
+    while ((req = virtio_scsi_parse_req(s, vq))) {
+        virtio_scsi_fail_ctrl_req(s, req);
+    }
+}
+
+static void virtio_scsi_fail_cmd_req(VirtIOSCSI *s, VirtIOSCSIReq *req)
+{
+    req->resp.cmd->response = VIRTIO_SCSI_S_FAILURE;
+    virtio_scsi_complete_req(req);
 }
 
 static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq)
 {
-    /* TODO */
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    VirtIOSCSIReq *req;
+
+    while ((req = virtio_scsi_parse_req(s, vq))) {
+        int out_size, in_size;
+        if (req->elem.out_num < 1 || req->elem.in_num < 1) {
+            virtio_scsi_bad_req();
+        }
+
+        out_size = req->elem.out_sg[0].iov_len;
+        in_size = req->elem.in_sg[0].iov_len;
+        if (out_size < sizeof(VirtIOSCSICmdReq) + VIRTIO_SCSI_CDB_SIZE ||
+            in_size < sizeof(VirtIOSCSICmdResp) + VIRTIO_SCSI_SENSE_SIZE) {
+            virtio_scsi_bad_req();
+        }
+
+        if (req->elem.out_num > 1 && req->elem.in_num > 1) {
+            virtio_scsi_fail_cmd_req(s, req);
+            continue;
+        }
+
+        req->resp.cmd->resid = 0;
+        req->resp.cmd->status_qualifier = 0;
+        req->resp.cmd->status = CHECK_CONDITION;
+        req->resp.cmd->sense_len = 4;
+        req->resp.cmd->sense[0] = 0xf0; /* Fixed format current sense */
+        req->resp.cmd->sense[1] = ILLEGAL_REQUEST;
+        req->resp.cmd->sense[2] = 0x20;
+        req->resp.cmd->sense[3] = 0x00;
+        req->resp.cmd->response = VIRTIO_SCSI_S_OK;
+
+        virtio_scsi_complete_req(req);
+    }
 }
 
 static void virtio_scsi_get_config(VirtIODevice *vdev,