Re: [Qemu-devel] virtio scsi host draft specification, v2

[Stefan Hajnoczi]

On Mon, May 30, 2011 at 10:22 AM, Paolo Bonzini <address@hidden> wrote:
> On 05/28/2011 09:33 PM, Stefan Hajnoczi wrote:
>>> Requests have the following format:
>>>
>>>     struct virtio_scsi_req_cmd {
>>>         u8 lun[8];
>>>         u64 id;
>>>         u8 task_attr;
>>>         u8 prio;
>>>         u8 crn;
>>>         u32 num_dataout, num_datain;
>>
>> These fields can be discovered from the in and out counts that virtio
>> provides.  They seem redundant to me.
>
> I'm not sure, perhaps in the future more variable-sized fields may be added.
>  I added a note that requests will be failed if the driver detects
> inconsistencies between the actual number of buffers, and the count
> specified in num_dataout/num_datain.

Okay, you're right that it will be necessary if more multi-buffer data is added.

>> SAMr4 5.1 The Execute Command procedure call:
>> "The CRN value zero shall be reserved for use as defined by the SCSI
>> transport protocol."
>>
>> FWIW the SRP spec simply doesn't include CRN and I think we could do the
>> same.  I don't know what it is actually used for in other transports...
>
> I wasn't sure of what would happen in the case of SCSI passthrough to
> protocols that do use CRN.  It seems "free" to leave it in.

I don't know the answer here.

>>> The control receiveq is used by the device to report information on
>>> logical units that are attached to it.  The driver should always
>>> leave a few (?) buffers ready in the control receiveq.  The device may
>>
>> "The driver should always leave buffers ready in the control receiveq"
>>
>> Also, I think it should say "the device must drop events if it finds no
>> buffer ready".  The spec goes into detail on how to notify about dropped
>> events, using "must" instead of "may" seems right.
>
> "Must" seems too strong.  Dropped events are a racy event, so it is not
> really possible to guve any guarantee.  I changed it to "will" though.

Okay thanks, I was more worried about allowing the device to queue
when event buffers are not available.  If the dropping behavior is
part of the spec then it is clear what the device implementation
should do.

Stefan