Patchwork [v4,22/24] scsi: split command_complete callback in two

login
register
mail settings
Submitter Paolo Bonzini
Date May 23, 2011, 4:09 p.m.
Message ID <1306166949-19698-23-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/97010/
State New
Headers show

Comments

Paolo Bonzini - May 23, 2011, 4:09 p.m.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c         |   60 +++++++++++++++++---------------
 hw/lsi53c895a.c  |   48 +++++++++++++++-----------
 hw/scsi-bus.c    |    4 +-
 hw/scsi.h        |    9 +----
 hw/spapr_vscsi.c |  101 ++++++++++++++++++++++++++++++------------------------
 hw/usb-msd.c     |   69 +++++++++++++++++++++----------------
 6 files changed, 159 insertions(+), 132 deletions(-)
Blue Swirl - May 23, 2011, 7:38 p.m.
On Mon, May 23, 2011 at 7:09 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
>  hw/esp.c         |   60 +++++++++++++++++---------------
>  hw/lsi53c895a.c  |   48 +++++++++++++++-----------
>  hw/scsi-bus.c    |    4 +-
>  hw/scsi.h        |    9 +----
>  hw/spapr_vscsi.c |  101 ++++++++++++++++++++++++++++++------------------------
>  hw/usb-msd.c     |   69 +++++++++++++++++++++----------------
>  6 files changed, 159 insertions(+), 132 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index 0b80529..32ed327 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -395,38 +395,41 @@ static void esp_do_dma(ESPState *s)
>     esp_dma_done(s);
>  }
>
> -static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +static void esp_command_complete(SCSIRequest *req, uint32_t arg)
>  {
>     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
>
> -    if (reason == SCSI_REASON_DONE) {
> -        DPRINTF("SCSI Command complete\n");
> -        if (s->ti_size != 0)
> -            DPRINTF("SCSI command completed unexpectedly\n");
> -        s->ti_size = 0;
> -        s->dma_left = 0;
> -        s->async_len = 0;
> -        if (arg)
> -            DPRINTF("Command failed\n");
> -        s->status = arg;
> -        s->rregs[ESP_RSTAT] = STAT_ST;
> +    DPRINTF("SCSI Command complete\n");
> +    if (s->ti_size != 0)

While touching the code, please add braces.

> +        DPRINTF("SCSI command completed unexpectedly\n");
> +    s->ti_size = 0;
> +    s->dma_left = 0;
> +    s->async_len = 0;
> +    if (arg)
> +        DPRINTF("Command failed\n");
> +    s->status = arg;
> +    s->rregs[ESP_RSTAT] = STAT_ST;
> +    esp_dma_done(s);
> +    if (s->current_req) {
> +        scsi_req_unref(s->current_req);
> +        s->current_req = NULL;
> +        s->current_dev = NULL;
> +    }
> +}
> +
> +static void esp_transfer_data(SCSIRequest *req, uint32_t arg)
> +{
> +    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
> +
> +    DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
> +    s->async_len = arg;
> +    s->async_buf = scsi_req_get_buf(req);
> +    if (s->dma_left) {
> +        esp_do_dma(s);
> +    } else if (s->dma_counter != 0 && s->ti_size <= 0) {
> +        /* If this was the last part of a DMA transfer then the
> +           completion interrupt is deferred to here.  */
>         esp_dma_done(s);
> -       if (s->current_req) {
> -            scsi_req_unref(s->current_req);
> -            s->current_req = NULL;
> -            s->current_dev = NULL;
> -       }
> -    } else {
> -        DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
> -        s->async_len = arg;
> -        s->async_buf = scsi_req_get_buf(req);
> -        if (s->dma_left) {
> -            esp_do_dma(s);
> -        } else if (s->dma_counter != 0 && s->ti_size <= 0) {
> -            /* If this was the last part of a DMA transfer then the
> -               completion interrupt is deferred to here.  */
> -            esp_dma_done(s);
> -        }
>     }
>  }
>
> @@ -725,6 +728,7 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
>  }
>
>  static struct SCSIBusOps esp_scsi_ops = {
> +    .transfer_data = esp_transfer_data,
>     .complete = esp_command_complete,
>     .cancel = esp_request_cancelled
>  };
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index a6c7c5b..62fe191 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -711,40 +711,47 @@ static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
>         return 1;
>     }
>  }
> - /* Callback to indicate that the SCSI layer has completed a transfer.  */
> -static void lsi_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +
> + /* Callback to indicate that the SCSI layer has completed a command.  */
> +static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
>  {
>     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
>     int out;
>
>     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
> -    if (reason == SCSI_REASON_DONE) {
> -        DPRINTF("Command complete status=%d\n", (int)arg);
> -        s->status = arg;
> -        s->command_complete = 2;
> -        if (s->waiting && s->dbc != 0) {
> -            /* Raise phase mismatch for short transfers.  */
> -            lsi_bad_phase(s, out, PHASE_ST);
> -        } else {
> -            lsi_set_phase(s, PHASE_ST);
> -        }
> +    DPRINTF("Command complete status=%d\n", (int)arg);
> +    s->status = arg;
> +    s->command_complete = 2;
> +    if (s->waiting && s->dbc != 0) {
> +        /* Raise phase mismatch for short transfers.  */
> +        lsi_bad_phase(s, out, PHASE_ST);
> +    } else {
> +        lsi_set_phase(s, PHASE_ST);
> +    }
>
> -        if (req == s->current->req) {
> -            scsi_req_unref(s->current->req);
> -            qemu_free(s->current);
> -            s->current = NULL;
> -       }
> -        lsi_resume_script(s);
> -        return;
> +    if (req == s->current->req) {
> +        scsi_req_unref(s->current->req);
> +        qemu_free(s->current);
> +        s->current = NULL;
>     }
> +    lsi_resume_script(s);
> +}
> +
> + /* Callback to indicate that the SCSI layer has completed a transfer.  */
> +static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
> +{
> +    LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> +    int out;
>
>     if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
>         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
>         if (lsi_queue_tag(s, req->tag, arg)) {
>             return;
> -        }
> +       }
>     }
>
> +    out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
> +
>     /* host adapter (re)connected */
>     DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
>     s->current->dma_len = arg;
> @@ -2239,6 +2246,7 @@ static int lsi_scsi_uninit(PCIDevice *d)
>  }
>
>  static struct SCSIBusOps lsi_scsi_ops = {
> +    .transfer_data = lsi_transfer_data,
>     .complete = lsi_command_complete,
>     .cancel = lsi_request_cancelled
>  };
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index d4fd61d..086a998 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -628,7 +628,7 @@ void scsi_req_continue(SCSIRequest *req)
>  void scsi_req_data(SCSIRequest *req, int len)
>  {
>     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
> -    req->bus->ops->complete(req, SCSI_REASON_DATA, len);
> +    req->bus->ops->transfer_data(req, len);
>  }
>
>  void scsi_req_print(SCSIRequest *req)
> @@ -664,7 +664,7 @@ void scsi_req_complete(SCSIRequest *req)
>     assert(req->status != -1);
>     scsi_req_ref(req);
>     scsi_req_dequeue(req);
> -    req->bus->ops->complete(req, SCSI_REASON_DONE, req->status);
> +    req->bus->ops->complete(req, req->status);
>     scsi_req_unref(req);
>  }
>
> diff --git a/hw/scsi.h b/hw/scsi.h
> index fa9c9fa..70f9e89 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -9,12 +9,6 @@
>
>  #define SCSI_CMD_BUF_SIZE     16
>
> -/* scsi-disk.c */
> -enum scsi_reason {
> -    SCSI_REASON_DONE, /* Command complete.  */
> -    SCSI_REASON_DATA  /* Transfer complete, more data required.  */
> -};
> -
>  typedef struct SCSIBus SCSIBus;
>  typedef struct SCSIBusOps SCSIBusOps;
>  typedef struct SCSIDevice SCSIDevice;
> @@ -84,7 +78,8 @@ struct SCSIDeviceInfo {
>  };
>
>  struct SCSIBusOps {
> -    void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
> +    void (*transfer_data)(SCSIRequest *req, uint32_t arg);
> +    void (*complete)(SCSIRequest *req, uint32_t arg);
>     void (*cancel)(SCSIRequest *req);
>  };
>
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index f0e08e3..d84e10d 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -480,63 +480,34 @@ static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
>  }
>
>  /* Callback to indicate that the SCSI layer has completed a transfer.  */
> -static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
> +static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
>  {
>     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
>     vscsi_req *req = vscsi_find_req(s, sreq);
>     uint8_t *buf;
> -    int32_t res_in = 0, res_out = 0;
>     int len, rc = 0;
>
> -    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
> -            reason, sreq->tag, arg, req);
> +    dprintf("VSCSI: SCSI xfer complete tag=0x%x arg=0x%x, req=%p\n",
> +            sreq->tag, arg, req);
>     if (req == NULL) {
>         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
>         return;
>     }
>
>     if (req->sensing) {
> -        if (reason == SCSI_REASON_DONE) {
> -            dprintf("VSCSI: Sense done !\n");
> -            vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
> -            vscsi_put_req(s, req);
> -        } else {
> -            uint8_t *buf = scsi_req_get_buf(sreq);
> -
> -            len = MIN(arg, SCSI_SENSE_BUF_SIZE);
> -            dprintf("VSCSI: Sense data, %d bytes:\n", len);
> -            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> -                    buf[0], buf[1], buf[2], buf[3],
> -                    buf[4], buf[5], buf[6], buf[7]);
> -            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> -                    buf[8], buf[9], buf[10], buf[11],
> -                    buf[12], buf[13], buf[14], buf[15]);
> -            memcpy(req->sense, buf, len);
> -            req->senselen = len;
> -            scsi_req_continue(req->sreq);
> -        }
> -        return;
> -    }
> -
> -    if (reason == SCSI_REASON_DONE) {
> -        dprintf("VSCSI: Command complete err=%d\n", arg);
> -        if (arg == 0) {
> -            /* We handle overflows, not underflows for normal commands,
> -             * but hopefully nobody cares
> -             */
> -            if (req->writing) {
> -                res_out = req->data_len;
> -            } else {
> -                res_in = req->data_len;
> -            }
> -            vscsi_send_rsp(s, req, 0, res_in, res_out);
> -        } else if (arg == CHECK_CONDITION) {
> -            vscsi_send_request_sense(s, req);
> -            return;
> -        } else {
> -            vscsi_send_rsp(s, req, arg, 0, 0);
> -        }
> -        vscsi_put_req(s, req);
> +        uint8_t *buf = scsi_req_get_buf(sreq);
> +
> +        len = MIN(arg, SCSI_SENSE_BUF_SIZE);
> +        dprintf("VSCSI: Sense data, %d bytes:\n", len);
> +        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> +                buf[0], buf[1], buf[2], buf[3],
> +                buf[4], buf[5], buf[6], buf[7]);
> +        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
> +                buf[8], buf[9], buf[10], buf[11],
> +                buf[12], buf[13], buf[14], buf[15]);
> +        memcpy(req->sense, buf, len);
> +        req->senselen = len;
> +        scsi_req_continue(req->sreq);
>         return;
>     }
>
> @@ -559,6 +530,45 @@ static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
>     scsi_req_continue(sreq);
>  }
>
> +/* Callback to indicate that the SCSI layer has completed a transfer.  */
> +static void vscsi_command_complete(SCSIRequest *sreq, uint32_t arg)
> +{
> +    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> +    vscsi_req *req = vscsi_find_req(s, sreq);
> +    int32_t res_in = 0, res_out = 0;
> +
> +    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
> +            reason, sreq->tag, arg, req);
> +    if (req == NULL) {
> +        fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
> +        return;
> +    }
> +
> +    if (!req->sensing && arg == CHECK_CONDITION) {
> +        vscsi_send_request_sense(s, req);
> +        return;
> +    }
> +
> +    if (req->sensing) {
> +        dprintf("VSCSI: Sense done !\n");
> +        arg = CHECK_CONDITION;
> +    } else {
> +        dprintf("VSCSI: Command complete err=%d\n", arg);
> +        if (arg == 0) {
> +            /* We handle overflows, not underflows for normal commands,
> +             * but hopefully nobody cares
> +             */
> +            if (req->writing) {
> +                res_out = req->data_len;
> +            } else {
> +                res_in = req->data_len;
> +            }
> +        }
> +    }
> +    vscsi_send_rsp(s, req, 0, res_in, res_out);
> +    vscsi_put_req(s, req);
> +}
> +
>  static void vscsi_request_cancelled(SCSIRequest *sreq)
>  {
>     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
> @@ -916,6 +926,7 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
>  }
>
>  static struct SCSIBusOps vscsi_scsi_ops = {
> +    .transfer_data = vscsi_transfer_data,
>     .complete = vscsi_command_complete,
>     .cancel = vscsi_request_cancelled
>  };
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index 27a37fa..c808815 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -208,7 +208,7 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
>     memcpy(p->data, &csw, len);
>  }
>
> -static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
> +static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
>  {
>     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
>     USBPacket *p = s->packet;
> @@ -216,35 +216,7 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
>     if (req->tag != s->tag) {
>         fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
>     }
> -    if (reason == SCSI_REASON_DONE) {
> -        DPRINTF("Command complete %d\n", arg);
> -        s->residue = s->data_len;
> -        s->result = arg != 0;
> -        if (s->packet) {
> -            if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
> -                /* A deferred packet with no write data remaining must be
> -                   the status read packet.  */
> -                usb_msd_send_status(s, p);
> -                s->mode = USB_MSDM_CBW;
> -            } else {
> -                if (s->data_len) {
> -                    s->data_len -= s->usb_len;
> -                    if (s->mode == USB_MSDM_DATAIN)
> -                        memset(s->usb_buf, 0, s->usb_len);
> -                    s->usb_len = 0;
> -                }
> -                if (s->data_len == 0)
> -                    s->mode = USB_MSDM_CSW;
> -            }
> -            s->packet = NULL;
> -            usb_packet_complete(&s->dev, p);
> -        } else if (s->data_len == 0) {
> -            s->mode = USB_MSDM_CSW;
> -        }
> -        scsi_req_unref(req);
> -        s->req = NULL;
> -        return;
> -    }
> +
>     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
>     s->scsi_len = arg;
>     s->scsi_buf = scsi_req_get_buf(req);
> @@ -261,6 +233,42 @@ static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
>     }
>  }
>
> +static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
> +{
> +    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> +    USBPacket *p = s->packet;
> +
> +    if (req->tag != s->tag) {
> +        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
> +    }
> +    DPRINTF("Command complete %d\n", arg);
> +    s->residue = s->data_len;
> +    s->result = arg != 0;
> +    if (s->packet) {
> +        if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
> +            /* A deferred packet with no write data remaining must be
> +               the status read packet.  */
> +            usb_msd_send_status(s, p);
> +            s->mode = USB_MSDM_CBW;
> +        } else {
> +            if (s->data_len) {
> +                s->data_len -= s->usb_len;
> +                if (s->mode == USB_MSDM_DATAIN)
> +                    memset(s->usb_buf, 0, s->usb_len);
> +                s->usb_len = 0;
> +            }
> +            if (s->data_len == 0)
> +                s->mode = USB_MSDM_CSW;
> +        }
> +        s->packet = NULL;
> +        usb_packet_complete(&s->dev, p);
> +    } else if (s->data_len == 0) {
> +        s->mode = USB_MSDM_CSW;
> +    }
> +    scsi_req_unref(req);
> +    s->req = NULL;
> +}
> +
>  static void usb_msd_request_cancelled(SCSIRequest *req)
>  {
>     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
> @@ -494,6 +502,7 @@ static void usb_msd_password_cb(void *opaque, int err)
>  }
>
>  static struct SCSIBusOps usb_msd_scsi_ops = {
> +    .transfer_data = usb_msd_transfer_data,
>     .complete = usb_msd_command_complete,
>     .cancel = usb_msd_request_cancelled
>  };
> --
> 1.7.4.4
>
>
>
>
Paolo Bonzini - May 24, 2011, 7:32 a.m.
On 05/23/2011 09:38 PM, Blue Swirl wrote:
> While touching the code, please add braces.

Done.  However, both changes you requested touch several patches, so I 
cannot just send follow-ups for the two you replied to.  Is it ok to 
just send a pull request for v5?

Paolo

Patch

diff --git a/hw/esp.c b/hw/esp.c
index 0b80529..32ed327 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -395,38 +395,41 @@  static void esp_do_dma(ESPState *s)
     esp_dma_done(s);
 }
 
-static void esp_command_complete(SCSIRequest *req, int reason, uint32_t arg)
+static void esp_command_complete(SCSIRequest *req, uint32_t arg)
 {
     ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
 
-    if (reason == SCSI_REASON_DONE) {
-        DPRINTF("SCSI Command complete\n");
-        if (s->ti_size != 0)
-            DPRINTF("SCSI command completed unexpectedly\n");
-        s->ti_size = 0;
-        s->dma_left = 0;
-        s->async_len = 0;
-        if (arg)
-            DPRINTF("Command failed\n");
-        s->status = arg;
-        s->rregs[ESP_RSTAT] = STAT_ST;
+    DPRINTF("SCSI Command complete\n");
+    if (s->ti_size != 0)
+        DPRINTF("SCSI command completed unexpectedly\n");
+    s->ti_size = 0;
+    s->dma_left = 0;
+    s->async_len = 0;
+    if (arg)
+        DPRINTF("Command failed\n");
+    s->status = arg;
+    s->rregs[ESP_RSTAT] = STAT_ST;
+    esp_dma_done(s);
+    if (s->current_req) {
+        scsi_req_unref(s->current_req);
+        s->current_req = NULL;
+        s->current_dev = NULL;
+    }
+}
+
+static void esp_transfer_data(SCSIRequest *req, uint32_t arg)
+{
+    ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req->bus->qbus.parent);
+
+    DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
+    s->async_len = arg;
+    s->async_buf = scsi_req_get_buf(req);
+    if (s->dma_left) {
+        esp_do_dma(s);
+    } else if (s->dma_counter != 0 && s->ti_size <= 0) {
+        /* If this was the last part of a DMA transfer then the
+           completion interrupt is deferred to here.  */
         esp_dma_done(s);
-	if (s->current_req) {
-            scsi_req_unref(s->current_req);
-            s->current_req = NULL;
-            s->current_dev = NULL;
-	}
-    } else {
-        DPRINTF("transfer %d/%d\n", s->dma_left, s->ti_size);
-        s->async_len = arg;
-        s->async_buf = scsi_req_get_buf(req);
-        if (s->dma_left) {
-            esp_do_dma(s);
-        } else if (s->dma_counter != 0 && s->ti_size <= 0) {
-            /* If this was the last part of a DMA transfer then the
-               completion interrupt is deferred to here.  */
-            esp_dma_done(s);
-        }
     }
 }
 
@@ -725,6 +728,7 @@  void esp_init(target_phys_addr_t espaddr, int it_shift,
 }
 
 static struct SCSIBusOps esp_scsi_ops = {
+    .transfer_data = esp_transfer_data,
     .complete = esp_command_complete,
     .cancel = esp_request_cancelled
 };
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index a6c7c5b..62fe191 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -711,40 +711,47 @@  static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t arg)
         return 1;
     }
 }
- /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void lsi_command_complete(SCSIRequest *req, int reason, uint32_t arg)
+
+ /* Callback to indicate that the SCSI layer has completed a command.  */
+static void lsi_command_complete(SCSIRequest *req, uint32_t arg)
 {
     LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
     int out;
 
     out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
-    if (reason == SCSI_REASON_DONE) {
-        DPRINTF("Command complete status=%d\n", (int)arg);
-        s->status = arg;
-        s->command_complete = 2;
-        if (s->waiting && s->dbc != 0) {
-            /* Raise phase mismatch for short transfers.  */
-            lsi_bad_phase(s, out, PHASE_ST);
-        } else {
-            lsi_set_phase(s, PHASE_ST);
-        }
+    DPRINTF("Command complete status=%d\n", (int)arg);
+    s->status = arg;
+    s->command_complete = 2;
+    if (s->waiting && s->dbc != 0) {
+        /* Raise phase mismatch for short transfers.  */
+        lsi_bad_phase(s, out, PHASE_ST);
+    } else {
+        lsi_set_phase(s, PHASE_ST);
+    }
 
-        if (req == s->current->req) {
-            scsi_req_unref(s->current->req);
-            qemu_free(s->current);
-            s->current = NULL;
-	}
-        lsi_resume_script(s);
-        return;
+    if (req == s->current->req) {
+        scsi_req_unref(s->current->req);
+        qemu_free(s->current);
+        s->current = NULL;
     }
+    lsi_resume_script(s);
+}
+
+ /* Callback to indicate that the SCSI layer has completed a transfer.  */
+static void lsi_transfer_data(SCSIRequest *req, uint32_t arg)
+{
+    LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
+    int out;
 
     if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
         (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
         if (lsi_queue_tag(s, req->tag, arg)) {
             return;
-        }
+	}
     }
 
+    out = (s->sstat1 & PHASE_MASK) == PHASE_DO;
+
     /* host adapter (re)connected */
     DPRINTF("Data ready tag=0x%x len=%d\n", req->tag, arg);
     s->current->dma_len = arg;
@@ -2239,6 +2246,7 @@  static int lsi_scsi_uninit(PCIDevice *d)
 }
 
 static struct SCSIBusOps lsi_scsi_ops = {
+    .transfer_data = lsi_transfer_data,
     .complete = lsi_command_complete,
     .cancel = lsi_request_cancelled
 };
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index d4fd61d..086a998 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -628,7 +628,7 @@  void scsi_req_continue(SCSIRequest *req)
 void scsi_req_data(SCSIRequest *req, int len)
 {
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
-    req->bus->ops->complete(req, SCSI_REASON_DATA, len);
+    req->bus->ops->transfer_data(req, len);
 }
 
 void scsi_req_print(SCSIRequest *req)
@@ -664,7 +664,7 @@  void scsi_req_complete(SCSIRequest *req)
     assert(req->status != -1);
     scsi_req_ref(req);
     scsi_req_dequeue(req);
-    req->bus->ops->complete(req, SCSI_REASON_DONE, req->status);
+    req->bus->ops->complete(req, req->status);
     scsi_req_unref(req);
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index fa9c9fa..70f9e89 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -9,12 +9,6 @@ 
 
 #define SCSI_CMD_BUF_SIZE     16
 
-/* scsi-disk.c */
-enum scsi_reason {
-    SCSI_REASON_DONE, /* Command complete.  */
-    SCSI_REASON_DATA  /* Transfer complete, more data required.  */
-};
-
 typedef struct SCSIBus SCSIBus;
 typedef struct SCSIBusOps SCSIBusOps;
 typedef struct SCSIDevice SCSIDevice;
@@ -84,7 +78,8 @@  struct SCSIDeviceInfo {
 };
 
 struct SCSIBusOps {
-    void (*complete)(SCSIRequest *req, int reason, uint32_t arg);
+    void (*transfer_data)(SCSIRequest *req, uint32_t arg);
+    void (*complete)(SCSIRequest *req, uint32_t arg);
     void (*cancel)(SCSIRequest *req);
 };
 
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index f0e08e3..d84e10d 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -480,63 +480,34 @@  static void vscsi_send_request_sense(VSCSIState *s, vscsi_req *req)
 }
 
 /* Callback to indicate that the SCSI layer has completed a transfer.  */
-static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
+static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t arg)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
     vscsi_req *req = vscsi_find_req(s, sreq);
     uint8_t *buf;
-    int32_t res_in = 0, res_out = 0;
     int len, rc = 0;
 
-    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
-            reason, sreq->tag, arg, req);
+    dprintf("VSCSI: SCSI xfer complete tag=0x%x arg=0x%x, req=%p\n",
+            sreq->tag, arg, req);
     if (req == NULL) {
         fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
         return;
     }
 
     if (req->sensing) {
-        if (reason == SCSI_REASON_DONE) {
-            dprintf("VSCSI: Sense done !\n");
-            vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-            vscsi_put_req(s, req);
-        } else {
-            uint8_t *buf = scsi_req_get_buf(sreq);
-
-            len = MIN(arg, SCSI_SENSE_BUF_SIZE);
-            dprintf("VSCSI: Sense data, %d bytes:\n", len);
-            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
-                    buf[0], buf[1], buf[2], buf[3],
-                    buf[4], buf[5], buf[6], buf[7]);
-            dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
-                    buf[8], buf[9], buf[10], buf[11],
-                    buf[12], buf[13], buf[14], buf[15]);
-            memcpy(req->sense, buf, len);
-            req->senselen = len;
-            scsi_req_continue(req->sreq);
-        }
-        return;
-    }
-
-    if (reason == SCSI_REASON_DONE) {
-        dprintf("VSCSI: Command complete err=%d\n", arg);
-        if (arg == 0) {
-            /* We handle overflows, not underflows for normal commands,
-             * but hopefully nobody cares
-             */
-            if (req->writing) {
-                res_out = req->data_len;
-            } else {
-                res_in = req->data_len;
-            }
-            vscsi_send_rsp(s, req, 0, res_in, res_out);
-        } else if (arg == CHECK_CONDITION) {
-            vscsi_send_request_sense(s, req);
-            return;
-        } else {
-            vscsi_send_rsp(s, req, arg, 0, 0);
-        }
-        vscsi_put_req(s, req);
+        uint8_t *buf = scsi_req_get_buf(sreq);
+
+        len = MIN(arg, SCSI_SENSE_BUF_SIZE);
+        dprintf("VSCSI: Sense data, %d bytes:\n", len);
+        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
+                buf[0], buf[1], buf[2], buf[3],
+                buf[4], buf[5], buf[6], buf[7]);
+        dprintf("       %02x  %02x  %02x  %02x  %02x  %02x  %02x  %02x\n",
+                buf[8], buf[9], buf[10], buf[11],
+                buf[12], buf[13], buf[14], buf[15]);
+        memcpy(req->sense, buf, len);
+        req->senselen = len;
+        scsi_req_continue(req->sreq);
         return;
     }
 
@@ -559,6 +530,45 @@  static void vscsi_command_complete(SCSIRequest *sreq, int reason, uint32_t arg)
     scsi_req_continue(sreq);
 }
 
+/* Callback to indicate that the SCSI layer has completed a transfer.  */
+static void vscsi_command_complete(SCSIRequest *sreq, uint32_t arg)
+{
+    VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
+    vscsi_req *req = vscsi_find_req(s, sreq);
+    int32_t res_in = 0, res_out = 0;
+
+    dprintf("VSCSI: SCSI cmd complete, r=0x%x tag=0x%x arg=0x%x, req=%p\n",
+            reason, sreq->tag, arg, req);
+    if (req == NULL) {
+        fprintf(stderr, "VSCSI: Can't find request for tag 0x%x\n", sreq->tag);
+        return;
+    }
+
+    if (!req->sensing && arg == CHECK_CONDITION) {
+        vscsi_send_request_sense(s, req);
+        return;
+    }
+
+    if (req->sensing) {
+        dprintf("VSCSI: Sense done !\n");
+        arg = CHECK_CONDITION;
+    } else {
+        dprintf("VSCSI: Command complete err=%d\n", arg);
+        if (arg == 0) {
+            /* We handle overflows, not underflows for normal commands,
+             * but hopefully nobody cares
+             */
+            if (req->writing) {
+                res_out = req->data_len;
+            } else {
+                res_in = req->data_len;
+            }
+        }
+    }
+    vscsi_send_rsp(s, req, 0, res_in, res_out);
+    vscsi_put_req(s, req);
+}
+
 static void vscsi_request_cancelled(SCSIRequest *sreq)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq->bus->qbus.parent);
@@ -916,6 +926,7 @@  static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
 }
 
 static struct SCSIBusOps vscsi_scsi_ops = {
+    .transfer_data = vscsi_transfer_data,
     .complete = vscsi_command_complete,
     .cancel = vscsi_request_cancelled
 };
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 27a37fa..c808815 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -208,7 +208,7 @@  static void usb_msd_send_status(MSDState *s, USBPacket *p)
     memcpy(p->data, &csw, len);
 }
 
-static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
+static void usb_msd_transfer_data(SCSIRequest *req, uint32_t arg)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
     USBPacket *p = s->packet;
@@ -216,35 +216,7 @@  static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     if (req->tag != s->tag) {
         fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
     }
-    if (reason == SCSI_REASON_DONE) {
-        DPRINTF("Command complete %d\n", arg);
-        s->residue = s->data_len;
-        s->result = arg != 0;
-        if (s->packet) {
-            if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
-                /* A deferred packet with no write data remaining must be
-                   the status read packet.  */
-                usb_msd_send_status(s, p);
-                s->mode = USB_MSDM_CBW;
-            } else {
-                if (s->data_len) {
-                    s->data_len -= s->usb_len;
-                    if (s->mode == USB_MSDM_DATAIN)
-                        memset(s->usb_buf, 0, s->usb_len);
-                    s->usb_len = 0;
-                }
-                if (s->data_len == 0)
-                    s->mode = USB_MSDM_CSW;
-            }
-            s->packet = NULL;
-            usb_packet_complete(&s->dev, p);
-        } else if (s->data_len == 0) {
-            s->mode = USB_MSDM_CSW;
-        }
-        scsi_req_unref(req);
-        s->req = NULL;
-        return;
-    }
+
     assert((s->mode == USB_MSDM_DATAOUT) == (req->cmd.mode == SCSI_XFER_TO_DEV));
     s->scsi_len = arg;
     s->scsi_buf = scsi_req_get_buf(req);
@@ -261,6 +233,42 @@  static void usb_msd_command_complete(SCSIRequest *req, int reason, uint32_t arg)
     }
 }
 
+static void usb_msd_command_complete(SCSIRequest *req, uint32_t arg)
+{
+    MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
+    USBPacket *p = s->packet;
+
+    if (req->tag != s->tag) {
+        fprintf(stderr, "usb-msd: Unexpected SCSI Tag 0x%x\n", req->tag);
+    }
+    DPRINTF("Command complete %d\n", arg);
+    s->residue = s->data_len;
+    s->result = arg != 0;
+    if (s->packet) {
+        if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
+            /* A deferred packet with no write data remaining must be
+               the status read packet.  */
+            usb_msd_send_status(s, p);
+            s->mode = USB_MSDM_CBW;
+        } else {
+            if (s->data_len) {
+                s->data_len -= s->usb_len;
+                if (s->mode == USB_MSDM_DATAIN)
+                    memset(s->usb_buf, 0, s->usb_len);
+                s->usb_len = 0;
+            }
+            if (s->data_len == 0)
+                s->mode = USB_MSDM_CSW;
+        }
+        s->packet = NULL;
+        usb_packet_complete(&s->dev, p);
+    } else if (s->data_len == 0) {
+        s->mode = USB_MSDM_CSW;
+    }
+    scsi_req_unref(req);
+    s->req = NULL;
+}
+
 static void usb_msd_request_cancelled(SCSIRequest *req)
 {
     MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
@@ -494,6 +502,7 @@  static void usb_msd_password_cb(void *opaque, int err)
 }
 
 static struct SCSIBusOps usb_msd_scsi_ops = {
+    .transfer_data = usb_msd_transfer_data,
     .complete = usb_msd_command_complete,
     .cancel = usb_msd_request_cancelled
 };