* [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits).
@ 2011-11-21 13:39 Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 1/8] usb-storage: move status debug message to usb_msd_send_status Gerd Hoffmann
` (7 more replies)
0 siblings, 8 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
Hi,
This pull fixes SYNCRONIZE_CACHE handling in both scsi and usb code.
The SYNCRONIZE_CACHE scsi command is different from all other ones:
It doesn't transfer any data, but still doesn't complete instantly.
Both scsi-disk and usb-storage failed to handle this case correctly,
resulting in all sorts of funky assert failures and segfaults when
the guest writes to a virtual usb stick and flushes stuff.
Patches 1-4 are minor cleanups for usb-storage (slighly improved scsi
request tracking). Also Patch 6 depends on these.
Patch 5 is the actual bugfix for scsi-disk.
Patch 6 is the actual bugfix for usb-storage.
Patches 7+8 are bonus fixes for Coverity-spotted issues which are
sitting in the usb patch queue for a while already.
The following changes since commit bc4268998d154b9b3cc86a7b6bd932cc974591c9:
x86: fix pcmpestrm and pcmpistrm (2011-11-19 13:51:27 +0000)
are available in the git repository at:
git://git.kraxel.org/qemu usb.30
Gerd Hoffmann (8):
usb-storage: move status debug message to usb_msd_send_status.
usb-storage: fill status in complete callback.
usb-storage: drop tag from device state.
usb-storage: drop result from device state.
scsi-disk: don't call scsi_req_complete twice.
usb-storage: don't try to send the status early.
ehci: add assert
usb-linux: fix /proc/bus/usb/devices scan
hw/scsi-disk.c | 5 ++-
hw/usb-ehci.c | 1 +
hw/usb-msd.c | 65 ++++++++++++++++++++++++++++++-------------------------
usb-linux.c | 7 +++++-
4 files changed, 45 insertions(+), 33 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 1/8] usb-storage: move status debug message to usb_msd_send_status.
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
@ 2011-11-21 13:39 ` Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 2/8] usb-storage: fill status in complete callback Gerd Hoffmann
` (6 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
usb_msd_send_status can be called from different code paths, move the
debug message into the function to make sure it is printed
unconditionally.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb-msd.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index b734177..8dde421 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -194,6 +194,8 @@ static void usb_msd_send_status(MSDState *s, USBPacket *p)
struct usb_msd_csw csw;
int len;
+ DPRINTF("Command status %d tag 0x%x, len %zd\n",
+ s->result, s->tag, p->iov.size);
csw.sig = cpu_to_le32(0x53425355);
csw.tag = cpu_to_le32(s->tag);
csw.residue = s->residue;
@@ -432,8 +434,6 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
break;
case USB_MSDM_CSW:
- DPRINTF("Command status %d tag 0x%x, len %zd\n",
- s->result, s->tag, p->iov.size);
if (p->iov.size < 13) {
goto fail;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 2/8] usb-storage: fill status in complete callback.
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 1/8] usb-storage: move status debug message to usb_msd_send_status Gerd Hoffmann
@ 2011-11-21 13:39 ` Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 3/8] usb-storage: drop tag from device state Gerd Hoffmann
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
Put status word into device state, fill it in command_complete, have
usb_msd_send_status just send it out.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb-msd.c | 38 +++++++++++++++++++++-----------------
1 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 8dde421..5dc4afc 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -38,6 +38,13 @@ enum USBMSDMode {
USB_MSDM_CSW /* Command Status. */
};
+struct usb_msd_csw {
+ uint32_t sig;
+ uint32_t tag;
+ uint32_t residue;
+ uint8_t status;
+};
+
typedef struct {
USBDevice dev;
enum USBMSDMode mode;
@@ -46,6 +53,7 @@ typedef struct {
uint32_t data_len;
uint32_t residue;
uint32_t tag;
+ struct usb_msd_csw csw;
SCSIRequest *req;
SCSIBus bus;
BlockConf conf;
@@ -67,13 +75,6 @@ struct usb_msd_cbw {
uint8_t cmd[16];
};
-struct usb_msd_csw {
- uint32_t sig;
- uint32_t tag;
- uint32_t residue;
- uint8_t status;
-};
-
enum {
STR_MANUFACTURER = 1,
STR_PRODUCT,
@@ -191,19 +192,15 @@ static void usb_msd_copy_data(MSDState *s, USBPacket *p)
static void usb_msd_send_status(MSDState *s, USBPacket *p)
{
- struct usb_msd_csw csw;
int len;
DPRINTF("Command status %d tag 0x%x, len %zd\n",
- s->result, s->tag, p->iov.size);
- csw.sig = cpu_to_le32(0x53425355);
- csw.tag = cpu_to_le32(s->tag);
- csw.residue = s->residue;
- csw.status = s->result;
-
- len = MIN(sizeof(csw), p->iov.size);
- usb_packet_copy(p, &csw, len);
- p->result = len;
+ s->csw.status, s->csw.tag, p->iov.size);
+
+ assert(s->csw.sig == 0x53425355);
+ len = MIN(sizeof(s->csw), p->iov.size);
+ usb_packet_copy(p, &s->csw, len);
+ memset(&s->csw, 0, sizeof(s->csw));
}
static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
@@ -236,6 +233,12 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
DPRINTF("Command complete %d\n", status);
s->residue = s->data_len;
s->result = status != 0;
+
+ s->csw.sig = cpu_to_le32(0x53425355);
+ s->csw.tag = cpu_to_le32(s->tag);
+ s->csw.residue = s->residue;
+ s->csw.status = s->result;
+
if (s->packet) {
if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
/* A deferred packet with no write data remaining must be
@@ -257,6 +260,7 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
} else if (s->data_len == 0) {
s->mode = USB_MSDM_CSW;
}
+
scsi_req_unref(req);
s->req = NULL;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 3/8] usb-storage: drop tag from device state.
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 1/8] usb-storage: move status debug message to usb_msd_send_status Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 2/8] usb-storage: fill status in complete callback Gerd Hoffmann
@ 2011-11-21 13:39 ` Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 4/8] usb-storage: drop result " Gerd Hoffmann
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
scsi keeps track of the tag in SCSIRequest,
no need to store a separate copy.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb-msd.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 5dc4afc..089c23c 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -52,7 +52,6 @@ typedef struct {
uint8_t *scsi_buf;
uint32_t data_len;
uint32_t residue;
- uint32_t tag;
struct usb_msd_csw csw;
SCSIRequest *req;
SCSIBus bus;
@@ -230,12 +229,12 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
USBPacket *p = s->packet;
- DPRINTF("Command complete %d\n", status);
+ DPRINTF("Command complete %d tag 0x%x\n", status, req->tag);
s->residue = s->data_len;
s->result = status != 0;
s->csw.sig = cpu_to_le32(0x53425355);
- s->csw.tag = cpu_to_le32(s->tag);
+ s->csw.tag = cpu_to_le32(req->tag);
s->csw.residue = s->residue;
s->csw.status = s->result;
@@ -260,7 +259,6 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
} else if (s->data_len == 0) {
s->mode = USB_MSDM_CSW;
}
-
scsi_req_unref(req);
s->req = NULL;
}
@@ -340,6 +338,7 @@ static void usb_msd_cancel_io(USBDevice *dev, USBPacket *p)
static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
{
MSDState *s = (MSDState *)dev;
+ uint32_t tag;
int ret = 0;
struct usb_msd_cbw cbw;
uint8_t devep = p->devep;
@@ -366,7 +365,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
fprintf(stderr, "usb-msd: Bad LUN %d\n", cbw.lun);
goto fail;
}
- s->tag = le32_to_cpu(cbw.tag);
+ tag = le32_to_cpu(cbw.tag);
s->data_len = le32_to_cpu(cbw.data_len);
if (s->data_len == 0) {
s->mode = USB_MSDM_CSW;
@@ -376,10 +375,10 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
s->mode = USB_MSDM_DATAOUT;
}
DPRINTF("Command tag 0x%x flags %08x len %d data %d\n",
- s->tag, cbw.flags, cbw.cmd_len, s->data_len);
+ tag, cbw.flags, cbw.cmd_len, s->data_len);
s->residue = 0;
s->scsi_len = 0;
- s->req = scsi_req_new(s->scsi_dev, s->tag, 0, cbw.cmd, NULL);
+ s->req = scsi_req_new(s->scsi_dev, tag, 0, cbw.cmd, NULL);
scsi_req_enqueue(s->req);
/* ??? Should check that USB and SCSI data transfer
directions match. */
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 4/8] usb-storage: drop result from device state.
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
` (2 preceding siblings ...)
2011-11-21 13:39 ` [Qemu-devel] [PATCH 3/8] usb-storage: drop tag from device state Gerd Hoffmann
@ 2011-11-21 13:39 ` Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 5/8] scsi-disk: don't call scsi_req_complete twice Gerd Hoffmann
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb-msd.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 089c23c..6f32a0e 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -59,7 +59,6 @@ typedef struct {
char *serial;
SCSIDevice *scsi_dev;
uint32_t removable;
- int result;
/* For async completion. */
USBPacket *packet;
} MSDState;
@@ -231,12 +230,11 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status)
DPRINTF("Command complete %d tag 0x%x\n", status, req->tag);
s->residue = s->data_len;
- s->result = status != 0;
s->csw.sig = cpu_to_le32(0x53425355);
s->csw.tag = cpu_to_le32(req->tag);
s->csw.residue = s->residue;
- s->csw.status = s->result;
+ s->csw.status = status != 0;
if (s->packet) {
if (s->data_len == 0 && s->mode == USB_MSDM_DATAOUT) {
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 5/8] scsi-disk: don't call scsi_req_complete twice.
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
` (3 preceding siblings ...)
2011-11-21 13:39 ` [Qemu-devel] [PATCH 4/8] usb-storage: drop result " Gerd Hoffmann
@ 2011-11-21 13:39 ` Gerd Hoffmann
2011-11-21 13:49 ` Paolo Bonzini
2011-11-21 13:40 ` [Qemu-devel] [PATCH 6/8] usb-storage: don't try to send the status early Gerd Hoffmann
` (2 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:39 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
In case the guest sends a SYNCHRONIZE_CACHE command scsi_req_complete()
is called twice: Once because there is no data to transfer and
scsi-disk thinks it is done with the command, and once when the flush is
actually finished ...
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/scsi-disk.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 62f538f..f3c75b3 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -291,7 +291,7 @@ static void scsi_write_complete(void * opaque, int ret)
scsi_req_complete(&r->req, GOOD);
} else {
scsi_init_iovec(r);
- DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
+ DPRINTF("Write complete tag=0x%x more=%zd\n", r->req.tag, r->qiov.size);
scsi_req_data(&r->req, r->qiov.size);
}
@@ -1421,7 +1421,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
return 0;
}
- if (r->sector_count == 0 && r->iov.iov_len == 0) {
+ if (r->sector_count == 0 && r->iov.iov_len == 0 &&
+ command != SYNCHRONIZE_CACHE) {
scsi_req_complete(&r->req, GOOD);
}
len = r->sector_count * 512 + r->iov.iov_len;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 6/8] usb-storage: don't try to send the status early.
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
` (4 preceding siblings ...)
2011-11-21 13:39 ` [Qemu-devel] [PATCH 5/8] scsi-disk: don't call scsi_req_complete twice Gerd Hoffmann
@ 2011-11-21 13:40 ` Gerd Hoffmann
2011-11-21 14:10 ` Paolo Bonzini
2011-11-21 13:40 ` [Qemu-devel] [PATCH 7/8] ehci: add assert Gerd Hoffmann
2011-11-21 13:40 ` [Qemu-devel] [PATCH 8/8] usb-linux: fix /proc/bus/usb/devices scan Gerd Hoffmann
7 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:40 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
Until recently all scsi commands sent to scsi-disk did either transfer
data or finished instantly. The correct implementation of
SYNCRONIZE_CACHE changed the picture though, and usb-storage needs
a fix to handle that case correctly.
---
hw/usb-msd.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index 6f32a0e..68e3756 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -378,9 +378,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
s->scsi_len = 0;
s->req = scsi_req_new(s->scsi_dev, tag, 0, cbw.cmd, NULL);
scsi_req_enqueue(s->req);
- /* ??? Should check that USB and SCSI data transfer
- directions match. */
- if (s->mode != USB_MSDM_CSW && s->residue == 0) {
+ if (s->req && s->req->cmd.xfer != SCSI_XFER_NONE) {
scsi_req_continue(s->req);
}
ret = p->result;
@@ -439,9 +437,15 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
goto fail;
}
- usb_msd_send_status(s, p);
- s->mode = USB_MSDM_CBW;
- ret = 13;
+ if (s->req) {
+ /* still in flight */
+ s->packet = p;
+ ret = USB_RET_ASYNC;
+ } else {
+ usb_msd_send_status(s, p);
+ s->mode = USB_MSDM_CBW;
+ ret = 13;
+ }
break;
case USB_MSDM_DATAIN:
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 7/8] ehci: add assert
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
` (5 preceding siblings ...)
2011-11-21 13:40 ` [Qemu-devel] [PATCH 6/8] usb-storage: don't try to send the status early Gerd Hoffmann
@ 2011-11-21 13:40 ` Gerd Hoffmann
2011-11-21 13:40 ` [Qemu-devel] [PATCH 8/8] usb-linux: fix /proc/bus/usb/devices scan Gerd Hoffmann
7 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:40 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann
Coverity thinks q could be NULL there and warns.
I believe it can't be NULL there.
Add assert to prove it.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/usb-ehci.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index cdd5aae..3eea94d 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -2046,6 +2046,7 @@ static void ehci_advance_state(EHCIState *ehci,
break;
case EST_WRITEBACK:
+ assert(q != NULL);
again = ehci_state_writeback(q, async);
break;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 8/8] usb-linux: fix /proc/bus/usb/devices scan
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
` (6 preceding siblings ...)
2011-11-21 13:40 ` [Qemu-devel] [PATCH 7/8] ehci: add assert Gerd Hoffmann
@ 2011-11-21 13:40 ` Gerd Hoffmann
2011-11-21 17:17 ` Markus Armbruster
7 siblings, 1 reply; 12+ messages in thread
From: Gerd Hoffmann @ 2011-11-21 13:40 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gerd Hoffmann, Markus Armbruster
Commit 0c402e5abb8c2755390eee864b43a98280fc2453 is incomplete
and misses one of the two function pointer calls in
usb_host_scan_dev(). Add the additional port handling logic
to the other call too.
Spotted by Coverity.
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
usb-linux.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index f086d57..d4426ea 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1568,7 +1568,12 @@ static int usb_host_scan_dev(void *opaque, USBScanFunc *func)
if (line[0] == 'T' && line[1] == ':') {
if (device_count && (vendor_id || product_id)) {
/* New device. Add the previously discovered device. */
- ret = func(opaque, bus_num, addr, 0, class_id, vendor_id,
+ if (port > 0) {
+ snprintf(buf, sizeof(buf), "%d", port);
+ } else {
+ snprintf(buf, sizeof(buf), "?");
+ }
+ ret = func(opaque, bus_num, addr, buf, class_id, vendor_id,
product_id, product_name, speed);
if (ret) {
goto the_end;
--
1.7.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] scsi-disk: don't call scsi_req_complete twice.
2011-11-21 13:39 ` [Qemu-devel] [PATCH 5/8] scsi-disk: don't call scsi_req_complete twice Gerd Hoffmann
@ 2011-11-21 13:49 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2011-11-21 13:49 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 11/21/2011 02:39 PM, Gerd Hoffmann wrote:
> In case the guest sends a SYNCHRONIZE_CACHE command scsi_req_complete()
> is called twice: Once because there is no data to transfer and
> scsi-disk thinks it is done with the command, and once when the flush is
> actually finished ...
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
> ---
> hw/scsi-disk.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 62f538f..f3c75b3 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -291,7 +291,7 @@ static void scsi_write_complete(void * opaque, int ret)
> scsi_req_complete(&r->req, GOOD);
> } else {
> scsi_init_iovec(r);
> - DPRINTF("Write complete tag=0x%x more=%d\n", r->req.tag, r->qiov.size);
> + DPRINTF("Write complete tag=0x%x more=%zd\n", r->req.tag, r->qiov.size);
> scsi_req_data(&r->req, r->qiov.size);
> }
>
> @@ -1421,7 +1421,8 @@ static int32_t scsi_send_command(SCSIRequest *req, uint8_t *buf)
> scsi_check_condition(r, SENSE_CODE(LBA_OUT_OF_RANGE));
> return 0;
> }
> - if (r->sector_count == 0&& r->iov.iov_len == 0) {
> + if (r->sector_count == 0&& r->iov.iov_len == 0&&
> + command != SYNCHRONIZE_CACHE) {
> scsi_req_complete(&r->req, GOOD);
> }
> len = r->sector_count * 512 + r->iov.iov_len;
/me is confused :)
case SYNCHRONIZE_CACHE:
/* The request is used as the AIO opaque value, so add a ref. */
scsi_req_ref(&r->req);
bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_flush_complete, r);
if (r->req.aiocb == NULL) {
scsi_flush_complete(r, -EIO);
}
return 0;
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] usb-storage: don't try to send the status early.
2011-11-21 13:40 ` [Qemu-devel] [PATCH 6/8] usb-storage: don't try to send the status early Gerd Hoffmann
@ 2011-11-21 14:10 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2011-11-21 14:10 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On 11/21/2011 02:40 PM, Gerd Hoffmann wrote:
> Until recently all scsi commands sent to scsi-disk did either transfer
> data or finished instantly. The correct implementation of
> SYNCRONIZE_CACHE changed the picture though, and usb-storage needs
> a fix to handle that case correctly.
> ---
> hw/usb-msd.c | 16 ++++++++++------
> 1 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
> index 6f32a0e..68e3756 100644
> --- a/hw/usb-msd.c
> +++ b/hw/usb-msd.c
> @@ -378,9 +378,7 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
> s->scsi_len = 0;
> s->req = scsi_req_new(s->scsi_dev, tag, 0, cbw.cmd, NULL);
> scsi_req_enqueue(s->req);
> - /* ??? Should check that USB and SCSI data transfer
> - directions match. */
> - if (s->mode != USB_MSDM_CSW&& s->residue == 0) {
> + if (s->req&& s->req->cmd.xfer != SCSI_XFER_NONE) {
> scsi_req_continue(s->req);
> }
> ret = p->result;
> @@ -439,9 +437,15 @@ static int usb_msd_handle_data(USBDevice *dev, USBPacket *p)
> goto fail;
> }
>
> - usb_msd_send_status(s, p);
> - s->mode = USB_MSDM_CBW;
> - ret = 13;
> + if (s->req) {
> + /* still in flight */
> + s->packet = p;
> + ret = USB_RET_ASYNC;
> + } else {
> + usb_msd_send_status(s, p);
> + s->mode = USB_MSDM_CBW;
> + ret = 13;
> + }
> break;
>
> case USB_MSDM_DATAIN:
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
... even without 5/8.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 8/8] usb-linux: fix /proc/bus/usb/devices scan
2011-11-21 13:40 ` [Qemu-devel] [PATCH 8/8] usb-linux: fix /proc/bus/usb/devices scan Gerd Hoffmann
@ 2011-11-21 17:17 ` Markus Armbruster
0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2011-11-21 17:17 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: pbonzini, qemu-devel
Gerd Hoffmann <kraxel@redhat.com> writes:
> Commit 0c402e5abb8c2755390eee864b43a98280fc2453 is incomplete
> and misses one of the two function pointer calls in
> usb_host_scan_dev(). Add the additional port handling logic
> to the other call too.
>
> Spotted by Coverity.
>
> Cc: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-11-21 17:17 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-21 13:39 [Qemu-devel] [PULL 1.0] usb patch queue (with scsi bits) Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 1/8] usb-storage: move status debug message to usb_msd_send_status Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 2/8] usb-storage: fill status in complete callback Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 3/8] usb-storage: drop tag from device state Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 4/8] usb-storage: drop result " Gerd Hoffmann
2011-11-21 13:39 ` [Qemu-devel] [PATCH 5/8] scsi-disk: don't call scsi_req_complete twice Gerd Hoffmann
2011-11-21 13:49 ` Paolo Bonzini
2011-11-21 13:40 ` [Qemu-devel] [PATCH 6/8] usb-storage: don't try to send the status early Gerd Hoffmann
2011-11-21 14:10 ` Paolo Bonzini
2011-11-21 13:40 ` [Qemu-devel] [PATCH 7/8] ehci: add assert Gerd Hoffmann
2011-11-21 13:40 ` [Qemu-devel] [PATCH 8/8] usb-linux: fix /proc/bus/usb/devices scan Gerd Hoffmann
2011-11-21 17:17 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.