Re: [PATCH 1/1] virito-blk: Support NOSPC error

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

On Tue, Jun 18, 2024 at 06:58:40AM -0400, Michael S. Tsirkin wrote:
> On Tue, Jun 18, 2024 at 05:18:58PM +0900, Keiichi Watanabe wrote:
> > Add a status code to indicate that the storage is full on the virtio-blk
> > device side.
> > 
> > This is useful in scenarios where a virtio-blk disk image is provided on a
> > sparse disk and the host storage is full.
> > In this scenario, the driver cannot create a new file to the disk image
> > because the sparse disk cannot be expanded.
> > However, if the host have a service that clean up host stroage regularly
> > by wiping cache files, the guest may want to retry the request.
> > For this type of special handling, this error should have a separate
> > error code in virito-blk.
> > 
> > Signed-off-by: Keiichi Watanabe <keiichiw@chromium.org>
> 
> Unfortunately we did not think about it and did not specify how
> drivers should handle any new error status.
> So I think we need to burn a feature bit on this.
> 
> 
> > ---
> >  device-types/blk/description.tex | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
> > index f04c932..5a34bd5 100644
> > --- a/device-types/blk/description.tex
> > +++ b/device-types/blk/description.tex
> > @@ -532,12 +532,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >  
> >  The final \field{status} byte is written by the device: either
> >  VIRTIO_BLK_S_OK for success, VIRTIO_BLK_S_IOERR for device or driver
> > -error or VIRTIO_BLK_S_UNSUPP for a request unsupported by device:
> > +error, VIRTIO_BLK_S_UNSUPP for a request unsupported by device or
> > +VIRTIO_BLK_S_NOSPC for an error that device doesn't have enough space.
> >  
> >  \begin{lstlisting}
> >  #define VIRTIO_BLK_S_OK        0
> >  #define VIRTIO_BLK_S_IOERR     1
> >  #define VIRTIO_BLK_S_UNSUPP    2
> > +#define VIRTIO_BLK_S_NOSPC     7
> 
> 
> Hmm. It's the last free bit.
> Seems like a bad idea to burn it on a limited use-case like this -
> I would say we need to make bit 7 mean "any other
> status" and then report the actual status somewhere else.
> No?

Ugh it's not a bit, I was confused ;)
Pls ignore.


> 
> 
> 
> >  \end{lstlisting}
> >  
> >  The status of individual segments is indeterminate when a discard or write zero
> > @@ -567,10 +569,11 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >  Requests of type VIRTIO_BLK_T_OUT, VIRTIO_BLK_T_ZONE_OPEN,
> >  VIRTIO_BLK_T_ZONE_CLOSE, VIRTIO_BLK_T_ZONE_FINISH, VIRTIO_BLK_T_ZONE_APPEND,
> >  VIRTIO_BLK_T_ZONE_RESET or VIRTIO_BLK_T_ZONE_RESET_ALL may be completed by the
> > -device with VIRTIO_BLK_S_OK, VIRTIO_BLK_S_IOERR or VIRTIO_BLK_S_UNSUPP
> > -\field{status}, or, additionally, with  VIRTIO_BLK_S_ZONE_INVALID_CMD,
> > -VIRTIO_BLK_S_ZONE_UNALIGNED_WP, VIRTIO_BLK_S_ZONE_OPEN_RESOURCE or
> > -VIRTIO_BLK_S_ZONE_ACTIVE_RESOURCE ZBD-specific status codes.
> > +device with VIRTIO_BLK_S_OK, VIRTIO_BLK_S_IOERR, VIRTIO_BLK_S_UNSUPP or
> > +VIRTIO_BLK_S_NOSPC \field{status}, or, additionally, with
> > +VIRTIO_BLK_S_ZONE_INVALID_CMD, VIRTIO_BLK_S_ZONE_UNALIGNED_WP,
> > +VIRTIO_BLK_S_ZONE_OPEN_RESOURCE or VIRTIO_BLK_S_ZONE_ACTIVE_RESOURCE ZBD-specific
> > +status codes.
> >  
> >  Besides the request status, VIRTIO_BLK_T_ZONE_APPEND requests return the
> >  starting sector of the appended data back to the driver. For this reason,
> > -- 
> > 2.45.2.627.g7a2c4fd464-goog
> >