On 07/01/2011 09:42 AM, Hannes Reinecke wrote:
'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.
This makes tracing a bit harder to follow. Perhaps you can keep the
transport tag (a uint64_t) in the SCSIRequest for debugging purposes?
Signed-off-by: Hannes Reinecke<hare@xxxxxxx>
---
hw/esp.c | 2 +-
hw/lsi53c895a.c | 17 ++++++++---------
hw/scsi-bus.c | 22 +++++++++++-----------
hw/scsi-disk.c | 5 ++---
hw/scsi-generic.c | 4 ++--
hw/scsi.h | 8 ++++----
hw/spapr_vscsi.c | 41 ++++++++++++-----------------------------
hw/usb-msd.c | 10 +++++-----
trace-events | 14 +++++++-------
9 files changed, 52 insertions(+), 71 deletions(-)
diff --git a/hw/esp.c b/hw/esp.c
index 6d3f5d2..912ff89 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, lun, s);
Might as well pass NULL here. The hba_private value is basically
unnecessary when the adapter doesn't support tagged command queuing.
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 86582cc..4e2ea03 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -216,8 +216,8 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
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);
+ if (req->hba_private != s) {
+ fprintf(stderr, "usb-msd: Unexpected SCSI command 0x%p\n", req);
}
Same here, just pass NULL and remove these ifs.
Otherwise looks like a very good idea.
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
