Messages in this thread | | | Date | Mon, 19 Dec 2011 10:00:34 -0500 | From | Douglas Gilbert <> | Subject | Re: [PATCH v3 1/2] virtio-scsi: first version |
| |
On 11-12-19 08:11 AM, Paolo Bonzini wrote: > On 12/19/2011 01:22 PM, Hannes Reinecke wrote: >>> + case VIRTIO_SCSI_S_UNDERRUN: >>> + set_host_byte(sc, DID_ERROR); >>> + break; >> Hmm. Not sure this is correct. >> UNDERRUN could be a valid state, eg it is quite common to send >> an INQUIRY with a length of 255 bytes. And only evaluate the >> bytes which are actually send back. > > This happens when you send an INQUIRY with a sglist of 255 bytes and an > ALLOCATION LENGTH of 300 bytes. The spec says "VIRTIO_SCSI_S_UNDERRUN [is > returned] if the content of the CDB requires transferring more data than is > available in the data buffers".
Overrun! That is overrun.
Underrun conventionally refers to the data-in buffer containing less data than the transport has allowed for. It is detected at the completion of a SCSI command and is used to report to the application client that bytes beyond a certain position in the data-in buffer are not valid (i.e. ignore them).
A mismatch between the allocation length field inside a SCSI command (a difficult and dangerous area to snoop in) and the data-in buffer length indicated by sglist should be called something else. Arguably it should not be treated as an error until an overrun occurs.
I think the 'case VIRTIO_SCSI_S_UNDERRUN:' line deserves a comment due to its misleading use of the word "underrun". It is reporting a potential or actual _overrun_.
Doug Gilbert
| |