Re: [PATCH 2/3] scsi: replace 'tag' with 'hba_private' pointer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/01/2011 10:27 AM, Paolo Bonzini wrote:
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?

Sure. Anything to get the patches accepted :-)

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.

Ok, I'll be resending both.

Cheers,

Hannes
--
Dr. Hannes Reinecke		      zSeries & Storage
hare@xxxxxxx			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux