Patchwork [v4,04/24] scsi: introduce SCSIBusOps

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

Comments

Paolo Bonzini - May 23, 2011, 4:08 p.m.
There are more operations than a SCSI bus can handle, besides completing
commands.  The current callback in fact is overloaded and can be called
with two different meanings already.  Another example, which this series
will introduce, is cleaning up after a request is cancelled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 hw/esp.c          |    6 +++++-
 hw/lsi53c895a.c   |    6 +++++-
 hw/scsi-bus.c     |   12 ++++++------
 hw/scsi-generic.c |    2 +-
 hw/scsi.h         |   13 +++++++------
 hw/spapr_vscsi.c  |    6 +++++-
 hw/usb-msd.c      |    6 +++++-
 7 files changed, 34 insertions(+), 17 deletions(-)
Blue Swirl - May 23, 2011, 7:35 p.m.
On Mon, May 23, 2011 at 7:08 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> There are more operations than a SCSI bus can handle, besides completing
> commands.  The current callback in fact is overloaded and can be called
> with two different meanings already.  Another example, which this series
> will introduce, is cleaning up after a request is cancelled.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> ---
>  hw/esp.c          |    6 +++++-
>  hw/lsi53c895a.c   |    6 +++++-
>  hw/scsi-bus.c     |   12 ++++++------
>  hw/scsi-generic.c |    2 +-
>  hw/scsi.h         |   13 +++++++------
>  hw/spapr_vscsi.c  |    6 +++++-
>  hw/usb-msd.c      |    6 +++++-
>  7 files changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/hw/esp.c b/hw/esp.c
> index fa9d2a2..d8bba7a 100644
> --- a/hw/esp.c
> +++ b/hw/esp.c
> @@ -714,6 +714,10 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
>     *dma_enable = qdev_get_gpio_in(dev, 1);
>  }
>
> +static struct SCSIBusOps esp_scsi_ops = {

It looks like structs like this could be made 'const'.

> +    .complete = esp_command_complete
> +};
> +
>  static int esp_init1(SysBusDevice *dev)
>  {
>     ESPState *s = FROM_SYSBUS(ESPState, dev);
> @@ -728,7 +732,7 @@ static int esp_init1(SysBusDevice *dev)
>
>     qdev_init_gpio_in(&dev->qdev, esp_gpio_demux, 2);
>
> -    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
> +    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, &esp_scsi_ops);
>     return scsi_bus_legacy_handle_cmdline(&s->bus);
>  }
>
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index 2ce38a9..ccea6ad 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -2205,6 +2205,10 @@ static int lsi_scsi_uninit(PCIDevice *d)
>     return 0;
>  }
>
> +static struct SCSIBusOps lsi_scsi_ops = {
> +    .complete = lsi_command_complete
> +};
> +
>  static int lsi_scsi_init(PCIDevice *dev)
>  {
>     LSIState *s = DO_UPCAST(LSIState, dev, dev);
> @@ -2241,7 +2245,7 @@ static int lsi_scsi_init(PCIDevice *dev)
>                            PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_ram_mapfunc);
>     QTAILQ_INIT(&s->queue);
>
> -    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
> +    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, &lsi_scsi_ops);
>     if (!dev->qdev.hotplugged) {
>         return scsi_bus_legacy_handle_cmdline(&s->bus);
>     }
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index 191cbab..f21704f 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -21,13 +21,13 @@ static int next_scsi_bus;
>
>  /* Create a scsi bus, and attach devices to it.  */
>  void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
> -                  scsi_completionfn complete)
> +                  SCSIBusOps *ops)
>  {
>     qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
>     bus->busnr = next_scsi_bus++;
>     bus->tcq = tcq;
>     bus->ndev = ndev;
> -    bus->complete = complete;
> +    bus->ops = ops;
>     bus->qbus.allow_hotplug = 1;
>  }
>
> @@ -503,7 +503,7 @@ static const char *scsi_command_name(uint8_t cmd)
>  void scsi_req_data(SCSIRequest *req, int len)
>  {
>     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
> -    req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
> +    req->bus->ops->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
>  }
>
>  void scsi_req_print(SCSIRequest *req)
> @@ -538,9 +538,9 @@ void scsi_req_complete(SCSIRequest *req)
>  {
>     assert(req->status != -1);
>     scsi_req_dequeue(req);
> -    req->bus->complete(req->bus, SCSI_REASON_DONE,
> -                       req->tag,
> -                       req->status);
> +    req->bus->ops->complete(req->bus, SCSI_REASON_DONE,
> +                            req->tag,
> +                            req->status);
>  }
>
>  static char *scsibus_get_fw_dev_path(DeviceState *dev)
> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> index e4f1f30..f09458b 100644
> --- a/hw/scsi-generic.c
> +++ b/hw/scsi-generic.c
> @@ -335,7 +335,7 @@ static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
>         s->senselen = 7;
>         s->driver_status = SG_ERR_DRIVER_SENSE;
>         bus = scsi_bus_from_device(d);
> -        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
> +        bus->ops->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
>         return 0;
>     }
>
> diff --git a/hw/scsi.h b/hw/scsi.h
> index 7c09f32..20cf397 100644
> --- a/hw/scsi.h
> +++ b/hw/scsi.h
> @@ -16,10 +16,9 @@ enum scsi_reason {
>  };
>
>  typedef struct SCSIBus SCSIBus;
> +typedef struct SCSIBusOps SCSIBusOps;
>  typedef struct SCSIDevice SCSIDevice;
>  typedef struct SCSIDeviceInfo SCSIDeviceInfo;
> -typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, uint32_t tag,
> -                                  uint32_t arg);
>
>  enum SCSIXferMode {
>     SCSI_XFER_NONE,      /*  TEST_UNIT_READY, ...            */
> @@ -74,20 +73,22 @@ struct SCSIDeviceInfo {
>     uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
>  };
>
> -typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
> -              int unit);
> +struct SCSIBusOps {
> +    void (*complete)(SCSIBus *bus, int reason, uint32_t tag, uint32_t arg);
> +};
> +
>  struct SCSIBus {
>     BusState qbus;
>     int busnr;
>
>     int tcq, ndev;
> -    scsi_completionfn complete;
> +    SCSIBusOps *ops;
>
>     SCSIDevice *devs[MAX_SCSI_DEVS];
>  };
>
>  void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
> -                  scsi_completionfn complete);
> +                  SCSIBusOps *ops);
>  void scsi_qdev_register(SCSIDeviceInfo *info);
>
>  static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> index 9928334..9f5c154 100644
> --- a/hw/spapr_vscsi.c
> +++ b/hw/spapr_vscsi.c
> @@ -907,6 +907,10 @@ static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
>     return 0;
>  }
>
> +static struct SCSIBusOps vscsi_scsi_ops = {
> +    .complete = vscsi_command_complete
> +};
> +
>  static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>  {
>     VSCSIState *s = DO_UPCAST(VSCSIState, vdev, dev);
> @@ -923,7 +927,7 @@ static int spapr_vscsi_init(VIOsPAPRDevice *dev)
>     dev->crq.SendFunc = vscsi_do_crq;
>
>     scsi_bus_new(&s->bus, &dev->qdev, 1, VSCSI_REQ_LIMIT,
> -                 vscsi_command_complete);
> +                 &vscsi_scsi_ops);
>     if (!dev->qdev.hotplugged) {
>         scsi_bus_legacy_handle_cmdline(&s->bus);
>     }
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index bd1c3a4..7a07a12 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -487,6 +487,10 @@ static void usb_msd_password_cb(void *opaque, int err)
>         qdev_unplug(&s->dev.qdev);
>  }
>
> +static struct SCSIBusOps usb_msd_scsi_ops = {
> +    .complete = usb_msd_command_complete
> +};
> +
>  static int usb_msd_initfn(USBDevice *dev)
>  {
>     MSDState *s = DO_UPCAST(MSDState, dev, dev);
> @@ -516,7 +520,7 @@ static int usb_msd_initfn(USBDevice *dev)
>     }
>
>     usb_desc_init(dev);
> -    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
> +    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, &usb_msd_scsi_ops);
>     s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable);
>     if (!s->scsi_dev) {
>         return -1;
> --
> 1.7.4.4
>
>
>
>

Patch

diff --git a/hw/esp.c b/hw/esp.c
index fa9d2a2..d8bba7a 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -714,6 +714,10 @@  void esp_init(target_phys_addr_t espaddr, int it_shift,
     *dma_enable = qdev_get_gpio_in(dev, 1);
 }
 
+static struct SCSIBusOps esp_scsi_ops = {
+    .complete = esp_command_complete
+};
+
 static int esp_init1(SysBusDevice *dev)
 {
     ESPState *s = FROM_SYSBUS(ESPState, dev);
@@ -728,7 +732,7 @@  static int esp_init1(SysBusDevice *dev)
 
     qdev_init_gpio_in(&dev->qdev, esp_gpio_demux, 2);
 
-    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, esp_command_complete);
+    scsi_bus_new(&s->bus, &dev->qdev, 0, ESP_MAX_DEVS, &esp_scsi_ops);
     return scsi_bus_legacy_handle_cmdline(&s->bus);
 }
 
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 2ce38a9..ccea6ad 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -2205,6 +2205,10 @@  static int lsi_scsi_uninit(PCIDevice *d)
     return 0;
 }
 
+static struct SCSIBusOps lsi_scsi_ops = {
+    .complete = lsi_command_complete
+};
+
 static int lsi_scsi_init(PCIDevice *dev)
 {
     LSIState *s = DO_UPCAST(LSIState, dev, dev);
@@ -2241,7 +2245,7 @@  static int lsi_scsi_init(PCIDevice *dev)
                            PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_ram_mapfunc);
     QTAILQ_INIT(&s->queue);
 
-    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, lsi_command_complete);
+    scsi_bus_new(&s->bus, &dev->qdev, 1, LSI_MAX_DEVS, &lsi_scsi_ops);
     if (!dev->qdev.hotplugged) {
         return scsi_bus_legacy_handle_cmdline(&s->bus);
     }
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 191cbab..f21704f 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -21,13 +21,13 @@  static int next_scsi_bus;
 
 /* Create a scsi bus, and attach devices to it.  */
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
-                  scsi_completionfn complete)
+                  SCSIBusOps *ops)
 {
     qbus_create_inplace(&bus->qbus, &scsi_bus_info, host, NULL);
     bus->busnr = next_scsi_bus++;
     bus->tcq = tcq;
     bus->ndev = ndev;
-    bus->complete = complete;
+    bus->ops = ops;
     bus->qbus.allow_hotplug = 1;
 }
 
@@ -503,7 +503,7 @@  static const char *scsi_command_name(uint8_t cmd)
 void scsi_req_data(SCSIRequest *req, int len)
 {
     trace_scsi_req_data(req->dev->id, req->lun, req->tag, len);
-    req->bus->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
+    req->bus->ops->complete(req->bus, SCSI_REASON_DATA, req->tag, len);
 }
 
 void scsi_req_print(SCSIRequest *req)
@@ -538,9 +538,9 @@  void scsi_req_complete(SCSIRequest *req)
 {
     assert(req->status != -1);
     scsi_req_dequeue(req);
-    req->bus->complete(req->bus, SCSI_REASON_DONE,
-                       req->tag,
-                       req->status);
+    req->bus->ops->complete(req->bus, SCSI_REASON_DONE,
+                            req->tag,
+                            req->status);
 }
 
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index e4f1f30..f09458b 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -335,7 +335,7 @@  static int32_t scsi_send_command(SCSIDevice *d, uint32_t tag,
         s->senselen = 7;
         s->driver_status = SG_ERR_DRIVER_SENSE;
         bus = scsi_bus_from_device(d);
-        bus->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
+        bus->ops->complete(bus, SCSI_REASON_DONE, tag, CHECK_CONDITION);
         return 0;
     }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 7c09f32..20cf397 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -16,10 +16,9 @@  enum scsi_reason {
 };
 
 typedef struct SCSIBus SCSIBus;
+typedef struct SCSIBusOps SCSIBusOps;
 typedef struct SCSIDevice SCSIDevice;
 typedef struct SCSIDeviceInfo SCSIDeviceInfo;
-typedef void (*scsi_completionfn)(SCSIBus *bus, int reason, uint32_t tag,
-                                  uint32_t arg);
 
 enum SCSIXferMode {
     SCSI_XFER_NONE,      /*  TEST_UNIT_READY, ...            */
@@ -74,20 +73,22 @@  struct SCSIDeviceInfo {
     uint8_t *(*get_buf)(SCSIDevice *s, uint32_t tag);
 };
 
-typedef void (*SCSIAttachFn)(DeviceState *host, BlockDriverState *bdrv,
-              int unit);
+struct SCSIBusOps {
+    void (*complete)(SCSIBus *bus, int reason, uint32_t tag, uint32_t arg);
+};
+
 struct SCSIBus {
     BusState qbus;
     int busnr;
 
     int tcq, ndev;
-    scsi_completionfn complete;
+    SCSIBusOps *ops;
 
     SCSIDevice *devs[MAX_SCSI_DEVS];
 };
 
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, int tcq, int ndev,
-                  scsi_completionfn complete);
+                  SCSIBusOps *ops);
 void scsi_qdev_register(SCSIDeviceInfo *info);
 
 static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 9928334..9f5c154 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -907,6 +907,10 @@  static int vscsi_do_crq(struct VIOsPAPRDevice *dev, uint8_t *crq_data)
     return 0;
 }
 
+static struct SCSIBusOps vscsi_scsi_ops = {
+    .complete = vscsi_command_complete
+};
+
 static int spapr_vscsi_init(VIOsPAPRDevice *dev)
 {
     VSCSIState *s = DO_UPCAST(VSCSIState, vdev, dev);
@@ -923,7 +927,7 @@  static int spapr_vscsi_init(VIOsPAPRDevice *dev)
     dev->crq.SendFunc = vscsi_do_crq;
 
     scsi_bus_new(&s->bus, &dev->qdev, 1, VSCSI_REQ_LIMIT,
-                 vscsi_command_complete);
+                 &vscsi_scsi_ops);
     if (!dev->qdev.hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus);
     }
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index bd1c3a4..7a07a12 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -487,6 +487,10 @@  static void usb_msd_password_cb(void *opaque, int err)
         qdev_unplug(&s->dev.qdev);
 }
 
+static struct SCSIBusOps usb_msd_scsi_ops = {
+    .complete = usb_msd_command_complete
+};
+
 static int usb_msd_initfn(USBDevice *dev)
 {
     MSDState *s = DO_UPCAST(MSDState, dev, dev);
@@ -516,7 +520,7 @@  static int usb_msd_initfn(USBDevice *dev)
     }
 
     usb_desc_init(dev);
-    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, usb_msd_command_complete);
+    scsi_bus_new(&s->bus, &s->dev.qdev, 0, 1, &usb_msd_scsi_ops);
     s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable);
     if (!s->scsi_dev) {
         return -1;