Re: [virtio] [PATCH 2/4] virtio-blk: clarify what seg_max and size_max are for

[Paolo Bonzini <pbonzini@redhat.com>]

On 28/08/2018 14:27, Michael S. Tsirkin wrote:
> On Fri, Aug 17, 2018 at 07:04:42PM +0200, Paolo Bonzini wrote:
>> Define them in terms of descriptors, not segments, and explain what the
>> driver and device are to use them for.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  content.tex | 17 +++++++++++++----
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/content.tex b/content.tex
>> index 4cdfeef..fd3a3c2 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -3515,11 +3515,11 @@ device except where noted.
>>  \subsection{Feature bits}\label{sec:Device Types / Block Device / Feature bits}
>>  
>>  \begin{description}
>> -\item[VIRTIO_BLK_F_SIZE_MAX (1)] Maximum size of any single segment is
>> -    in \field{size_max}.
>> +\item[VIRTIO_BLK_F_SIZE_MAX (1)] Maximum size of any single descriptor in
>> +    the data portion of a request ("segment") is in \field{size_max}.
>>  
>> -\item[VIRTIO_BLK_F_SEG_MAX (2)] Maximum number of segments in a
>> -    request is in \field{seg_max}.
>> +\item[VIRTIO_BLK_F_SEG_MAX (2)] Maximum number of descriptors in the
>> +    data portion of a request ("segments") is in \field{seg_max}.
>>  
>>  \item[VIRTIO_BLK_F_GEOMETRY (4)] Disk-style geometry specified in
>>      \field{geometry}.
>> @@ -3762,6 +3762,10 @@ error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
>>  A driver SHOULD NOT submit a request which would cause a read or write
>>  beyond \field{capacity}.
>>  
>> +A driver SHOULD NOT submit a request which has more than \field{seg_max}
>> +descriptors in the data portion, or which has more than \field{size_max}
>> +bytes in a single descriptor in the data portion.
>> +
>>  A driver SHOULD accept the VIRTIO_BLK_F_RO feature if offered.
>>  
>>  A driver MUST set \field{sector} to 0 for a VIRTIO_BLK_T_FLUSH request.
>> @@ -3782,6 +3786,11 @@ a range of sectors has been discarded.
>>  A device MUST set the \field{status} byte to VIRTIO_BLK_S_IOERR for a
>>  request which would cause a read or write beyond \field{capacity}.
>>  
>> +A device MAY set the \field{status} byte to VIRTIO_BLK_S_IOERR for a
>> +request
> 
> Well device can always do this for any request, right?
> I think the distinction is that it can do this for all such requests?
> 
>> which has more than \field{seg_max} descriptors in the data
>> +portion, or which has more than \field{size_max} bytes in a single
>> +descriptor in the data portion.
>> +
> 
> Since it's SHOULD NOT for driver, I think another assumption is that
> device must handle such huge requests anyway then?

Not really, the idea was that it's okay if the driver sends huge
requests, but it has to expect the error case.  OTOH the device is free
to handle huge requests if it wishes to.

Paolo

> Do all host implementations actually satisfy this requirement?
> 
>>  A device MUST set the \field{status} byte to VIRTIO_BLK_S_IOERR
>>  for a write request if the VIRTIO_BLK_F_RO feature if offered, and MUST NOT
>>  write any data.
>> -- 
>> 2.17.1
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe from this mail list, you must leave the OASIS TC that 
>> generates this mail.  Follow this link to all your TCs in OASIS at:
>> https://www.oasis-open.org/apps/org/workgroup/portal/my_workgroups.php