[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/21] scsi: Add 'hba_private' to SCSIRequest
From: |
Benjamin Herrenschmidt |
Subject: |
Re: [Qemu-devel] [PATCH 05/21] scsi: Add 'hba_private' to SCSIRequest |
Date: |
Tue, 19 Jul 2011 23:24:34 +1000 |
On Tue, 2011-07-19 at 15:06 +0200, Kevin Wolf wrote:
> Am 19.07.2011 14:43, schrieb Anthony Liguori:
> > On 07/19/2011 05:15 AM, Kevin Wolf wrote:
> >> From: Hannes Reinecke<address@hidden>
> >>
> >> 'tag' is just an abstraction to identify the command
> >> from the driver. So we should make that explicit by
> >> replacing 'tag' with a driver-defined pointer 'hba_private'.
> >> This saves the lookup for driver handling several commands
> >> in parallel.
> >> 'tag' is still being kept for tracing purposes.
> >>
> >> Signed-off-by: Hannes Reinecke<address@hidden>
> >> Acked-by: Paolo Bonzini<address@hidden>
> >> Signed-off-by: Kevin Wolf<address@hidden>
> >> ---
> >> hw/esp.c | 2 +-
> >> hw/lsi53c895a.c | 22 ++++++++--------------
> >> hw/scsi-bus.c | 9 ++++++---
> >> hw/scsi-disk.c | 4 ++--
> >> hw/scsi-generic.c | 5 +++--
> >> hw/scsi.h | 10 +++++++---
> >> hw/spapr_vscsi.c | 29 +++++++++--------------------
> >> hw/usb-msd.c | 9 +--------
> >> 8 files changed, 37 insertions(+), 53 deletions(-)
> >>
> >> diff --git a/hw/esp.c b/hw/esp.c
> >> index aa50800..9ddd637 100644
> >> --- a/hw/esp.c
> >> +++ b/hw/esp.c
> >> @@ -244,7 +244,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf,
> >> uint8_t busid)
> >>
> >> DPRINTF("do_busid_cmd: busid 0x%x\n", busid);
> >> lun = busid& 7;
> >> - s->current_req = scsi_req_new(s->current_dev, 0, lun);
> >> + s->current_req = scsi_req_new(s->current_dev, 0, lun, NULL);
> >> datalen = scsi_req_enqueue(s->current_req, buf);
> >> s->ti_size = datalen;
> >> if (datalen != 0) {
> >> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> >> index 940b43a..69eec1d 100644
> >> --- a/hw/lsi53c895a.c
> >> +++ b/hw/lsi53c895a.c
> >> @@ -661,7 +661,7 @@ static lsi_request *lsi_find_by_tag(LSIState *s,
> >> uint32_t tag)
> >> static void lsi_request_cancelled(SCSIRequest *req)
> >> {
> >> LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> >> - lsi_request *p;
> >> + lsi_request *p = req->hba_private;
> >>
> >> if (s->current&& req == s->current->req) {
> >> scsi_req_unref(req);
> >> @@ -670,7 +670,6 @@ static void lsi_request_cancelled(SCSIRequest *req)
> >> return;
> >> }
> >>
> >> - p = lsi_find_by_tag(s, req->tag);
> >> if (p) {
> >> QTAILQ_REMOVE(&s->queue, p, next);
> >> scsi_req_unref(req);
> >> @@ -680,18 +679,12 @@ static void lsi_request_cancelled(SCSIRequest *req)
> >>
> >> /* Record that data is available for a queued command. Returns zero if
> >> the device was reselected, nonzero if the IO is deferred. */
> >> -static int lsi_queue_tag(LSIState *s, uint32_t tag, uint32_t len)
> >> +static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len)
> >> {
> >> - lsi_request *p;
> >> -
> >> - p = lsi_find_by_tag(s, tag);
> >> - if (!p) {
> >> - BADF("IO with unknown tag %d\n", tag);
> >> - return 1;
> >> - }
> >> + lsi_request *p = req->hba_private;
> >>
> >> if (p->pending) {
> >> - BADF("Multiple IO pending for tag %d\n", tag);
> >> + BADF("Multiple IO pending for request %p\n", p);
> >> }
> >> p->pending = len;
> >> /* Reselect if waiting for it, or if reselection triggers an IRQ
> >> @@ -743,9 +736,9 @@ static void lsi_transfer_data(SCSIRequest *req,
> >> uint32_t len)
> >> LSIState *s = DO_UPCAST(LSIState, dev.qdev, req->bus->qbus.parent);
> >> int out;
> >>
> >> - if (s->waiting == 1 || !s->current || req->tag != s->current->tag ||
> >> + if (s->waiting == 1 || !s->current || req->hba_private != s->current
> >> ||
> >> (lsi_irq_on_rsl(s)&& !(s->scntl1& LSI_SCNTL1_CON))) {
> >> - if (lsi_queue_tag(s, req->tag, len)) {
> >> + if (lsi_queue_req(s, req, len)) {
> >> return;
> >> }
> >> }
> >> @@ -789,7 +782,8 @@ static void lsi_do_command(LSIState *s)
> >> assert(s->current == NULL);
> >> s->current = qemu_mallocz(sizeof(lsi_request));
> >> s->current->tag = s->select_tag;
> >> - s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun);
> >> + s->current->req = scsi_req_new(dev, s->current->tag, s->current_lun,
> >> + s->current);
> >>
> >> n = scsi_req_enqueue(s->current->req, buf);
> >> if (n) {
> >> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> >> index ad6a730..8b1a412 100644
> >> --- a/hw/scsi-bus.c
> >> +++ b/hw/scsi-bus.c
> >> @@ -131,7 +131,8 @@ int scsi_bus_legacy_handle_cmdline(SCSIBus *bus)
> >> return res;
> >> }
> >>
> >> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> >> uint32_t lun)
> >> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> >> + uint32_t lun, void *hba_private)
> >> {
> >> SCSIRequest *req;
> >>
> >> @@ -141,14 +142,16 @@ SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice
> >> *d, uint32_t tag, uint32_t l
> >> req->dev = d;
> >> req->tag = tag;
> >> req->lun = lun;
> >> + req->hba_private = hba_private;
> >> req->status = -1;
> >> trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
> >> return req;
> >> }
> >>
> >> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun)
> >> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >> + void *hba_private)
> >> {
> >> - return d->info->alloc_req(d, tag, lun);
> >> + return d->info->alloc_req(d, tag, lun, hba_private);
> >> }
> >>
> >> uint8_t *scsi_req_get_buf(SCSIRequest *req)
> >> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> >> index a8c7372..c2a99fe 100644
> >> --- a/hw/scsi-disk.c
> >> +++ b/hw/scsi-disk.c
> >> @@ -81,13 +81,13 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int
> >> error, int type);
> >> static int scsi_disk_emulate_command(SCSIDiskReq *r, uint8_t *outbuf);
> >>
> >> static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
> >> - uint32_t lun)
> >> + uint32_t lun, void *hba_private)
> >> {
> >> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, d);
> >> SCSIRequest *req;
> >> SCSIDiskReq *r;
> >>
> >> - req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun);
> >> + req = scsi_req_alloc(sizeof(SCSIDiskReq),&s->qdev, tag, lun,
> >> hba_private);
> >> r = DO_UPCAST(SCSIDiskReq, req, req);
> >> r->iov.iov_base = qemu_blockalign(s->bs, SCSI_DMA_BUF_SIZE);
> >> return req;
> >> diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
> >> index 8e59c7e..90345a7 100644
> >> --- a/hw/scsi-generic.c
> >> +++ b/hw/scsi-generic.c
> >> @@ -96,11 +96,12 @@ static int scsi_get_sense(SCSIRequest *req, uint8_t
> >> *outbuf, int len)
> >> return size;
> >> }
> >>
> >> -static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
> >> uint32_t lun)
> >> +static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag,
> >> uint32_t lun,
> >> + void *hba_private)
> >> {
> >> SCSIRequest *req;
> >>
> >> - req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun);
> >> + req = scsi_req_alloc(sizeof(SCSIGenericReq), d, tag, lun,
> >> hba_private);
> >> return req;
> >> }
> >>
> >> diff --git a/hw/scsi.h b/hw/scsi.h
> >> index c1dca35..6b15bbc 100644
> >> --- a/hw/scsi.h
> >> +++ b/hw/scsi.h
> >> @@ -43,6 +43,7 @@ struct SCSIRequest {
> >> } cmd;
> >> BlockDriverAIOCB *aiocb;
> >> bool enqueued;
> >> + void *hba_private;
> >> QTAILQ_ENTRY(SCSIRequest) next;
> >> };
> >>
> >> @@ -67,7 +68,8 @@ struct SCSIDeviceInfo {
> >> DeviceInfo qdev;
> >> scsi_qdev_initfn init;
> >> void (*destroy)(SCSIDevice *s);
> >> - SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun);
> >> + SCSIRequest *(*alloc_req)(SCSIDevice *s, uint32_t tag, uint32_t lun,
> >> + void *hba_private);
> >> void (*free_req)(SCSIRequest *req);
> >> int32_t (*send_command)(SCSIRequest *req, uint8_t *buf);
> >> void (*read_data)(SCSIRequest *req);
> >> @@ -138,8 +140,10 @@ extern const struct SCSISense sense_code_LUN_FAILURE;
> >> int scsi_build_sense(SCSISense sense, uint8_t *buf, int len, int fixed);
> >> int scsi_sense_valid(SCSISense sense);
> >>
> >> -SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> >> uint32_t lun);
> >> -SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun);
> >> +SCSIRequest *scsi_req_alloc(size_t size, SCSIDevice *d, uint32_t tag,
> >> + uint32_t lun, void *hba_private);
> >> +SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun,
> >> + void *hba_private);
> >> int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf);
> >> void scsi_req_free(SCSIRequest *req);
> >> SCSIRequest *scsi_req_ref(SCSIRequest *req);
> >> diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
> >> index 1c901ef..9e1cb2e 100644
> >> --- a/hw/spapr_vscsi.c
> >> +++ b/hw/spapr_vscsi.c
> >> @@ -121,7 +121,7 @@ static struct vscsi_req *vscsi_get_req(VSCSIState *s)
> >> return NULL;
> >> }
> >>
> >> -static void vscsi_put_req(VSCSIState *s, vscsi_req *req)
> >> +static void vscsi_put_req(vscsi_req *req)
> >> {
> >> if (req->sreq != NULL) {
> >> scsi_req_unref(req->sreq);
> >
> > This breaks the build:
> >
> > make[1]: Nothing to be done for `all'.
> > CC ppc64-softmmu/spapr_vscsi.o
> > /home/anthony/git/qemu/hw/spapr_vscsi.c: In function
> > ‘vscsi_command_complete’:
> > /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: error: ‘s’ undeclared
> > (first use in this function)
> > /home/anthony/git/qemu/hw/spapr_vscsi.c:535:34: note: each undeclared
> > identifier is reported only once for each function it appears in
>
> Adding Hannes to CC.
>
> > This file is only built when libfdt is installed which is probably why
> > you didn't catch it.
>
> Yes, I did build all targets, but probably don't have most optional
> dependencies installed. I guess I should install some more libraries on
> that machine.
>
> But as you said, there is no libfdt package in RHEL, so it will not be
> among them, and I'd like to avoid installing things from source.
Well, there isn't much we can do, those targets are going to require
libfdt, and if the device-tree stuff spreads on other architectures
(which is somewhat happening in the kernel now), expect that requirement
to grow beyond powerpc soon in qemu too :-)
I've spotted an effort to do a fedora package there:
https://bugzilla.redhat.com/show_bug.cgi?id=443882
I don't know much about distro packaging (I stay carefully away from
it), but maybe that could provide a basis for a RHEL one ?
Cheers,
Ben.
- [Qemu-devel] [PATCH 08/21] qemu-options.hx: Document missing -drive options, (continued)
- [Qemu-devel] [PATCH 08/21] qemu-options.hx: Document missing -drive options, Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 09/21] qemu-config: Document -drive options, Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 04/21] iov: Update parameter usage in iov_(to|from)_buf(), Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 06/21] scsi-disk: Fixup debugging statement, Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 12/21] VMDK: probe for monolithicFlat images, Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 10/21] VMDK: introduce VmdkExtent, Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 11/21] VMDK: bugfix, align offset to cluster in get_whole_cluster, Kevin Wolf, 2011/07/19
- [Qemu-devel] [PATCH 05/21] scsi: Add 'hba_private' to SCSIRequest, Kevin Wolf, 2011/07/19
- Re: [Qemu-devel] [PATCH 05/21] scsi: Add 'hba_private' to SCSIRequest, Benjamin Herrenschmidt, 2011/07/19
- Re: [Qemu-devel] [PATCH 05/21] scsi: Add 'hba_private' to SCSIRequest, David Gibson, 2011/07/20
[Qemu-devel] [PATCH 13/21] VMDK: separate vmdk_open by format version, Kevin Wolf, 2011/07/19
[Qemu-devel] [PATCH 02/21] qemu-io: Fix formatting, Kevin Wolf, 2011/07/19
[Qemu-devel] [PATCH 15/21] VMDK: flush multiple extents, Kevin Wolf, 2011/07/19
[Qemu-devel] [PATCH 07/21] scsi-disk: Mask out serial number EVPD, Kevin Wolf, 2011/07/19
[Qemu-devel] [PATCH 16/21] VMDK: move 'static' cid_update flag to bs field, Kevin Wolf, 2011/07/19
[Qemu-devel] [PATCH 17/21] VMDK: change get_cluster_offset return type, Kevin Wolf, 2011/07/19