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

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Sep 10, 2018 at 10:35:21AM +0200, Paolo Bonzini wrote:
> 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

Right but it must not crash or hang or set NEEDS_RESET
as a result, right? It must use the request even if
it's with error.  I think spec should clarify that.

> > 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