[PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Keiichi Watanabe]

This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
the device's ENOSPC error to the driver so the driver can treat it
in a different way from other IO errors.
More motivation is explained in fs-devel mail [1].

[2] is a prototype implementation in crosvm and [3] is in virtio-blk
driver. Idealy, the error should be propagated to the guest file system
and handled smartly, but the part is still under discussion.

[1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
[2]: https://crrev.com/c/5599649
[3]: https://crrev.com/c/5600325


*** BLURB HERE ***

Keiichi Watanabe (1):
  virito-blk: Support NOSPC error

 device-types/blk/description.tex | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

-- 
2.45.2.627.g7a2c4fd464-goog

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

[Keiichi Watanabe]

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

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

[Michael S. Tsirkin]

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?



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

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

[Michael S. Tsirkin]

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

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

[Parav Pandit]

Hi Keiichi,

> From: Keiichi Watanabe <keiichiw@chromium.org>
> Sent: Tuesday, June 18, 2024 1:49 PM
> To: virtio-comment@lists.linux.dev
> Cc: keiichiw@chromium.org; uekawa@chromium.org;
> takayas@chromium.org; dverkamp@chromium.org; tytso@google.com
> Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> 
> 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
s/stroage/storage

> 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>
> ---
>  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
>  \end{lstlisting}
> 
With recent work from Michael and others in admin command area,

we try to use existing Linux error codes referenced by the spec [errno] in the "Normative References" section.
So for this new error code also, it would be good to reuse value 28 (instead of 7) of [2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h#n32

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

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

[Michael S. Tsirkin]

On Tue, Jun 18, 2024 at 11:54:10AM +0000, Parav Pandit wrote:
> Hi Keiichi,
> 
> > From: Keiichi Watanabe <keiichiw@chromium.org>
> > Sent: Tuesday, June 18, 2024 1:49 PM
> > To: virtio-comment@lists.linux.dev
> > Cc: keiichiw@chromium.org; uekawa@chromium.org;
> > takayas@chromium.org; dverkamp@chromium.org; tytso@google.com
> > Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> > 
> > 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
> s/stroage/storage
> 
> > 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>
> > ---
> >  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
> >  \end{lstlisting}
> > 
> With recent work from Michael and others in admin command area,
> 
> we try to use existing Linux error codes referenced by the spec [errno] in the "Normative References" section.
> So for this new error code also, it would be good to reuse value 28 (instead of 7) of [2].
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h#n32


The problem with this idea, is that e.g. EIO is 5 which is already used
up by zone flags. And errno values go beyond 127 so with a single byte
status we can't reserve a bit for that, either.


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

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

[Keiichi Watanabe]

Thank you for the feedback. Having the new feature bit makes sense.
So, I think we can update the spec as follows:

* Define VIRTIO_BLK_F_ERRNO feature bit
* If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the following
virito_blk_req_with_errno instead of virtio_blk_req.
* If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status to
VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.

struct virtio_blk_req_with_errno {
  le32 type;
  le32 reserved;
  le64 sector;
  u8 data[];
  u8 status;
  u8 padding[3];
  le32 errno;
};

Does it make sense? If so, I'll make v2 patch.
Note that I used le32 for errno above because POSIX spec [1] says
errno is int and int is usually 4 bytes.

[1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html

Keiichi


On Tue, Jun 18, 2024 at 9:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Jun 18, 2024 at 11:54:10AM +0000, Parav Pandit wrote:
> > Hi Keiichi,
> >
> > > From: Keiichi Watanabe <keiichiw@chromium.org>
> > > Sent: Tuesday, June 18, 2024 1:49 PM
> > > To: virtio-comment@lists.linux.dev
> > > Cc: keiichiw@chromium.org; uekawa@chromium.org;
> > > takayas@chromium.org; dverkamp@chromium.org; tytso@google.com
> > > Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> > >
> > > 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
> > s/stroage/storage
> >
> > > 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>
> > > ---
> > >  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
> > >  \end{lstlisting}
> > >
> > With recent work from Michael and others in admin command area,
> >
> > we try to use existing Linux error codes referenced by the spec [errno] in the "Normative References" section.
> > So for this new error code also, it would be good to reuse value 28 (instead of 7) of [2].
> >
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h#n32
>
>
> The problem with this idea, is that e.g. EIO is 5 which is already used
> up by zone flags. And errno values go beyond 127 so with a single byte
> status we can't reserve a bit for that, either.
>
>
> > >  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
> > >
> >
>

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

[Michael S. Tsirkin]

On Wed, Jun 19, 2024 at 10:09:19PM +0900, Keiichi Watanabe wrote:
> Thank you for the feedback. Having the new feature bit makes sense.
> So, I think we can update the spec as follows:
> 
> * Define VIRTIO_BLK_F_ERRNO feature bit
> * If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the following
> virito_blk_req_with_errno instead of virtio_blk_req.
> * If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status to
> VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.
> 
> struct virtio_blk_req_with_errno {
>   le32 type;
>   le32 reserved;
>   le64 sector;
>   u8 data[];
>   u8 status;
>   u8 padding[3];
>   le32 errno;
> };
> 
> Does it make sense? If so, I'll make v2 patch.
> Note that I used le32 for errno above because POSIX spec [1] says
> errno is int and int is usually 4 bytes.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
> 
> Keiichi

Okay. But what exactly are the errno values we are going to use?

Also, how to we add errno values?

Parav, wouldn't you say the complexity is exploding, this
is a 1st new value in a very long while, maybe
the original approach is ok?

> 
> On Tue, Jun 18, 2024 at 9:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Jun 18, 2024 at 11:54:10AM +0000, Parav Pandit wrote:
> > > Hi Keiichi,
> > >
> > > > From: Keiichi Watanabe <keiichiw@chromium.org>
> > > > Sent: Tuesday, June 18, 2024 1:49 PM
> > > > To: virtio-comment@lists.linux.dev
> > > > Cc: keiichiw@chromium.org; uekawa@chromium.org;
> > > > takayas@chromium.org; dverkamp@chromium.org; tytso@google.com
> > > > Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> > > >
> > > > 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
> > > s/stroage/storage
> > >
> > > > 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>
> > > > ---
> > > >  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
> > > >  \end{lstlisting}
> > > >
> > > With recent work from Michael and others in admin command area,
> > >
> > > we try to use existing Linux error codes referenced by the spec [errno] in the "Normative References" section.
> > > So for this new error code also, it would be good to reuse value 28 (instead of 7) of [2].
> > >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h#n32
> >
> >
> > The problem with this idea, is that e.g. EIO is 5 which is already used
> > up by zone flags. And errno values go beyond 127 so with a single byte
> > status we can't reserve a bit for that, either.
> >
> >
> > > >  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
> > > >
> > >
> >

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, July 4, 2024 4:25 AM
> 
> On Wed, Jun 19, 2024 at 10:09:19PM +0900, Keiichi Watanabe wrote:
> > Thank you for the feedback. Having the new feature bit makes sense.
> > So, I think we can update the spec as follows:
> >
> > * Define VIRTIO_BLK_F_ERRNO feature bit
> > * If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the following
> > virito_blk_req_with_errno instead of virtio_blk_req.
> > * If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status to
> > VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.
> >
> > struct virtio_blk_req_with_errno {
> >   le32 type;
> >   le32 reserved;
> >   le64 sector;
> >   u8 data[];
> >   u8 status;
> >   u8 padding[3];
> >   le32 errno;
> > };
> >
> > Does it make sense? If so, I'll make v2 patch.
> > Note that I used le32 for errno above because POSIX spec [1] says
> > errno is int and int is usually 4 bytes.
> >
> > [1]:
> >
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.ht
> ml
> >
> > Keiichi
> 
> Okay. But what exactly are the errno values we are going to use?
> 
> Also, how to we add errno values?
> 
> Parav, wouldn't you say the complexity is exploding, this is a 1st new value in a
> very long while, maybe the original approach is ok?
> 
I agree with Michael's point on adding new field errno is unnecessary complexity to me.
I do not understand the need for it.
Because 8-bit status is already sufficient to reflect the NOSPC error.
This can be easily done by utilizing either (a) errno.h value or (b) some arbitrary error code.
Just because values 1 and 2 are taken away in the current spec, that doesn't mean we must give up reusing values from errno.h.

Errno.h value ENOENT is not applicable to virtio-blk device anyway, so I don’t see it can conflict in future.
EPERM is valid that will be needed for a write IO on the read only disks.
Since EPERM is already taken some arbitrary high value or some other unrelated value can be defined to reflect EPERM behavor.

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

[Michael S. Tsirkin]

On Wed, Jun 19, 2024 at 10:09:19PM +0900, Keiichi Watanabe wrote:
> Thank you for the feedback. Having the new feature bit makes sense.
> So, I think we can update the spec as follows:
> 
> * Define VIRTIO_BLK_F_ERRNO feature bit
> * If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the following
> virito_blk_req_with_errno instead of virtio_blk_req.
> * If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status to
> VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.
> 
> struct virtio_blk_req_with_errno {
>   le32 type;
>   le32 reserved;
>   le64 sector;
>   u8 data[];
>   u8 status;
>   u8 padding[3];
>   le32 errno;
> };
> 
> Does it make sense? If so, I'll make v2 patch.
> Note that I used le32 for errno above because POSIX spec [1] says
> errno is int and int is usually 4 bytes.
> 
> [1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
> 
> Keiichi

Thought hard about it. I think expanding the request for errno is
unwelcome. What we can do, if you really want to do it, is when
VIRTIO_BLK_F_ERRNO is negotiated, then make error codes different.

./include/uapi/asm-generic/errno.h has numbers that only go up
to about 60. So ok for now.

As to whether just keep your approach, can we see some
analysis which errors are likely to happen at the block level?
If there are many more of them, then reusing errno begins to
make sense.

> 
> On Tue, Jun 18, 2024 at 9:18 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Tue, Jun 18, 2024 at 11:54:10AM +0000, Parav Pandit wrote:
> > > Hi Keiichi,
> > >
> > > > From: Keiichi Watanabe <keiichiw@chromium.org>
> > > > Sent: Tuesday, June 18, 2024 1:49 PM
> > > > To: virtio-comment@lists.linux.dev
> > > > Cc: keiichiw@chromium.org; uekawa@chromium.org;
> > > > takayas@chromium.org; dverkamp@chromium.org; tytso@google.com
> > > > Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> > > >
> > > > 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
> > > s/stroage/storage
> > >
> > > > 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>
> > > > ---
> > > >  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
> > > >  \end{lstlisting}
> > > >
> > > With recent work from Michael and others in admin command area,
> > >
> > > we try to use existing Linux error codes referenced by the spec [errno] in the "Normative References" section.
> > > So for this new error code also, it would be good to reuse value 28 (instead of 7) of [2].
> > >
> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/errno-base.h#n32
> >
> >
> > The problem with this idea, is that e.g. EIO is 5 which is already used
> > up by zone flags. And errno values go beyond 127 so with a single byte
> > status we can't reserve a bit for that, either.
> >
> >
> > > >  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
> > > >
> > >
> >

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, July 4, 2024 1:34 PM
> 
> On Wed, Jun 19, 2024 at 10:09:19PM +0900, Keiichi Watanabe wrote:
> > Thank you for the feedback. Having the new feature bit makes sense.
> > So, I think we can update the spec as follows:
> >
> > * Define VIRTIO_BLK_F_ERRNO feature bit
> > * If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the following
> > virito_blk_req_with_errno instead of virtio_blk_req.
> > * If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status to
> > VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.
> >
> > struct virtio_blk_req_with_errno {
> >   le32 type;
> >   le32 reserved;
> >   le64 sector;
> >   u8 data[];
> >   u8 status;
> >   u8 padding[3];
> >   le32 errno;
> > };
> >
> > Does it make sense? If so, I'll make v2 patch.
> > Note that I used le32 for errno above because POSIX spec [1] says
> > errno is int and int is usually 4 bytes.
> >
> > [1]:
> >
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.ht
> ml
> >
> > Keiichi
> 
> Thought hard about it. I think expanding the request for errno is unwelcome.
True, seems overkill.

> What we can do, if you really want to do it, is when VIRTIO_BLK_F_ERRNO is
> negotiated, then make error codes different.
> 
Do we also need a feature bit?
If the device reports req.status == ENOSPC convert into the NOSPC error code.
If it returns VIRTIO_BLK_S_IOERR, than generic EIO?

This is because there is no gain in telling the device to return special error code on hitting a special error.
It returns the error if it has support for it. that’s it.

I am unable to see the benefit of extra negotiation.
Would this simplicity work?

> ./include/uapi/asm-generic/errno.h has numbers that only go up to about 60.
> So ok for now.
> 
> As to whether just keep your approach, can we see some analysis which errors
> are likely to happen at the block level?
> If there are many more of them, then reusing errno begins to make sense.
> 
> >
> > On Tue, Jun 18, 2024 at 9:18 PM Michael S. Tsirkin <mst@redhat.com>
> wrote:
> > >
> > > On Tue, Jun 18, 2024 at 11:54:10AM +0000, Parav Pandit wrote:
> > > > Hi Keiichi,
> > > >
> > > > > From: Keiichi Watanabe <keiichiw@chromium.org>
> > > > > Sent: Tuesday, June 18, 2024 1:49 PM
> > > > > To: virtio-comment@lists.linux.dev
> > > > > Cc: keiichiw@chromium.org; uekawa@chromium.org;
> > > > > takayas@chromium.org; dverkamp@chromium.org;
> tytso@google.com
> > > > > Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> > > > >
> > > > > 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
> > > > s/stroage/storage
> > > >
> > > > > 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>
> > > > > ---
> > > > >  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
> > > > >  \end{lstlisting}
> > > > >
> > > > With recent work from Michael and others in admin command area,
> > > >
> > > > we try to use existing Linux error codes referenced by the spec [errno] in
> the "Normative References" section.
> > > > So for this new error code also, it would be good to reuse value 28
> (instead of 7) of [2].
> > > >
> > > > [1]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > > /tree/include/uapi/asm-generic/errno-base.h
> > > > [2]
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > > /tree/include/uapi/asm-generic/errno-base.h#n32
> > >
> > >
> > > The problem with this idea, is that e.g. EIO is 5 which is already
> > > used up by zone flags. And errno values go beyond 127 so with a
> > > single byte status we can't reserve a bit for that, either.
> > >
> > >
> > > > >  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
> > > > >
> > > >
> > >

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

[Michael S. Tsirkin]

On Thu, Jul 04, 2024 at 09:52:52AM +0000, Parav Pandit wrote:
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Thursday, July 4, 2024 1:34 PM
> > 
> > On Wed, Jun 19, 2024 at 10:09:19PM +0900, Keiichi Watanabe wrote:
> > > Thank you for the feedback. Having the new feature bit makes sense.
> > > So, I think we can update the spec as follows:
> > >
> > > * Define VIRTIO_BLK_F_ERRNO feature bit
> > > * If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the following
> > > virito_blk_req_with_errno instead of virtio_blk_req.
> > > * If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status to
> > > VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.
> > >
> > > struct virtio_blk_req_with_errno {
> > >   le32 type;
> > >   le32 reserved;
> > >   le64 sector;
> > >   u8 data[];
> > >   u8 status;
> > >   u8 padding[3];
> > >   le32 errno;
> > > };
> > >
> > > Does it make sense? If so, I'll make v2 patch.
> > > Note that I used le32 for errno above because POSIX spec [1] says
> > > errno is int and int is usually 4 bytes.
> > >
> > > [1]:
> > >
> > https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.ht
> > ml
> > >
> > > Keiichi
> > 
> > Thought hard about it. I think expanding the request for errno is unwelcome.
> True, seems overkill.
> 
> > What we can do, if you really want to do it, is when VIRTIO_BLK_F_ERRNO is
> > negotiated, then make error codes different.
> > 
> Do we also need a feature bit?
> If the device reports req.status == ENOSPC convert into the NOSPC error code.
> If it returns VIRTIO_BLK_S_IOERR, than generic EIO?
> 
> This is because there is no gain in telling the device to return special error code on hitting a special error.
> It returns the error if it has support for it. that’s it.
> 
> I am unable to see the benefit of extra negotiation.
> Would this simplicity work?

No - old drivers won't know how to handle the new ENOSPC.


> > ./include/uapi/asm-generic/errno.h has numbers that only go up to about 60.
> > So ok for now.
> > 
> > As to whether just keep your approach, can we see some analysis which errors
> > are likely to happen at the block level?
> > If there are many more of them, then reusing errno begins to make sense.
> > 
> > >
> > > On Tue, Jun 18, 2024 at 9:18 PM Michael S. Tsirkin <mst@redhat.com>
> > wrote:
> > > >
> > > > On Tue, Jun 18, 2024 at 11:54:10AM +0000, Parav Pandit wrote:
> > > > > Hi Keiichi,
> > > > >
> > > > > > From: Keiichi Watanabe <keiichiw@chromium.org>
> > > > > > Sent: Tuesday, June 18, 2024 1:49 PM
> > > > > > To: virtio-comment@lists.linux.dev
> > > > > > Cc: keiichiw@chromium.org; uekawa@chromium.org;
> > > > > > takayas@chromium.org; dverkamp@chromium.org;
> > tytso@google.com
> > > > > > Subject: [PATCH 1/1] virito-blk: Support NOSPC error
> > > > > >
> > > > > > 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
> > > > > s/stroage/storage
> > > > >
> > > > > > 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>
> > > > > > ---
> > > > > >  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
> > > > > >  \end{lstlisting}
> > > > > >
> > > > > With recent work from Michael and others in admin command area,
> > > > >
> > > > > we try to use existing Linux error codes referenced by the spec [errno] in
> > the "Normative References" section.
> > > > > So for this new error code also, it would be good to reuse value 28
> > (instead of 7) of [2].
> > > > >
> > > > > [1]
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > > > /tree/include/uapi/asm-generic/errno-base.h
> > > > > [2]
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > > > /tree/include/uapi/asm-generic/errno-base.h#n32
> > > >
> > > >
> > > > The problem with this idea, is that e.g. EIO is 5 which is already
> > > > used up by zone flags. And errno values go beyond 127 so with a
> > > > single byte status we can't reserve a bit for that, either.
> > > >
> > > >
> > > > > >  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
> > > > > >
> > > > >
> > > >
> 

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

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, July 4, 2024 4:49 PM
> To: Parav Pandit <parav@nvidia.com>
> Cc: Keiichi Watanabe <keiichiw@chromium.org>; virtio-
> comment@lists.linux.dev; uekawa@chromium.org; takayas@chromium.org;
> dverkamp@chromium.org; tytso@google.com
> Subject: Re: [PATCH 1/1] virito-blk: Support NOSPC error
> 
> On Thu, Jul 04, 2024 at 09:52:52AM +0000, Parav Pandit wrote:
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, July 4, 2024 1:34 PM
> > >
> > > On Wed, Jun 19, 2024 at 10:09:19PM +0900, Keiichi Watanabe wrote:
> > > > Thank you for the feedback. Having the new feature bit makes sense.
> > > > So, I think we can update the spec as follows:
> > > >
> > > > * Define VIRTIO_BLK_F_ERRNO feature bit
> > > > * If VIRTIO_BLK_F_ERRNO is negotiated, the driver sends the
> > > > following virito_blk_req_with_errno instead of virtio_blk_req.
> > > > * If VIRTIO_BLK_F_ERRNO is negotiated, when the device sets status
> > > > to VIRTIO_BLK_S_IOERR, the device may set errno to a positive value.
> > > >
> > > > struct virtio_blk_req_with_errno {
> > > >   le32 type;
> > > >   le32 reserved;
> > > >   le64 sector;
> > > >   u8 data[];
> > > >   u8 status;
> > > >   u8 padding[3];
> > > >   le32 errno;
> > > > };
> > > >
> > > > Does it make sense? If so, I'll make v2 patch.
> > > > Note that I used le32 for errno above because POSIX spec [1] says
> > > > errno is int and int is usually 4 bytes.
> > > >
> > > > [1]:
> > > >
> > >
> https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.ht
> > > ml
> > > >
> > > > Keiichi
> > >
> > > Thought hard about it. I think expanding the request for errno is
> unwelcome.
> > True, seems overkill.
> >
> > > What we can do, if you really want to do it, is when
> > > VIRTIO_BLK_F_ERRNO is negotiated, then make error codes different.
> > >
> > Do we also need a feature bit?
> > If the device reports req.status == ENOSPC convert into the NOSPC error
> code.
> > If it returns VIRTIO_BLK_S_IOERR, than generic EIO?
> >
> > This is because there is no gain in telling the device to return special error
> code on hitting a special error.
> > It returns the error if it has support for it. that’s it.
> >
> > I am unable to see the benefit of extra negotiation.
> > Would this simplicity work?
> 
> No - old drivers won't know how to handle the new ENOSPC.
> 
Ok. got it.
yeah, feature bit can tell this to the device.
Thanks.

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Michael S. Tsirkin]

On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> the device's ENOSPC error to the driver so the driver can treat it
> in a different way from other IO errors.
> More motivation is explained in fs-devel mail [1].
> 
> [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> driver. Idealy, the error should be propagated to the guest file system
> and handled smartly, but the part is still under discussion.
> 
> [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
> [2]: https://crrev.com/c/5599649
> [3]: https://crrev.com/c/5600325
> 

> *** BLURB HERE ***


it's a single patch, pls just send directly no cover letter needed.

> Keiichi Watanabe (1):
>   virito-blk: Support NOSPC error
> 
>  device-types/blk/description.tex | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> -- 
> 2.45.2.627.g7a2c4fd464-goog
> 

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Keiichi Watanabe]

Sorry, I'll do so next time.

On Tue, Jun 18, 2024 at 7:54 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> > This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> > the device's ENOSPC error to the driver so the driver can treat it
> > in a different way from other IO errors.
> > More motivation is explained in fs-devel mail [1].
> >
> > [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> > driver. Idealy, the error should be propagated to the guest file system
> > and handled smartly, but the part is still under discussion.
> >
> > [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
> > [2]: https://crrev.com/c/5599649
> > [3]: https://crrev.com/c/5600325
> >
>
> > *** BLURB HERE ***
>
>
> it's a single patch, pls just send directly no cover letter needed.
>
> > Keiichi Watanabe (1):
> >   virito-blk: Support NOSPC error
> >
> >  device-types/blk/description.tex | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > --
> > 2.45.2.627.g7a2c4fd464-goog
> >
>

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> the device's ENOSPC error to the driver so the driver can treat it
> in a different way from other IO errors.
> More motivation is explained in fs-devel mail [1].
> 
> [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> driver. Idealy, the error should be propagated to the guest file system
> and handled smartly, but the part is still under discussion.
> 
> [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u

I have replied on linux-fsdevel so we can discuss a bit more about the
context and get input from storage and file systems people.

Thanks,
Stefan

> [2]: https://crrev.com/c/5599649
> [3]: https://crrev.com/c/5600325
> 
> 
> *** BLURB HERE ***
> 
> Keiichi Watanabe (1):
>   virito-blk: Support NOSPC error
> 
>  device-types/blk/description.tex | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> -- 
> 2.45.2.627.g7a2c4fd464-goog
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Michael S. Tsirkin]

On Wed, Jun 19, 2024 at 10:00:24AM -0400, Stefan Hajnoczi wrote:
> On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> > This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> > the device's ENOSPC error to the driver so the driver can treat it
> > in a different way from other IO errors.
> > More motivation is explained in fs-devel mail [1].
> > 
> > [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> > driver. Idealy, the error should be propagated to the guest file system
> > and handled smartly, but the part is still under discussion.
> > 
> > [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
> 
> I have replied on linux-fsdevel so we can discuss a bit more about the
> context and get input from storage and file systems people.
> 
> Thanks,
> Stefan

Stefan could you also comment on whether we are likely to add many more
errno values soonish?


> > [2]: https://crrev.com/c/5599649
> > [3]: https://crrev.com/c/5600325
> > 
> > 
> > *** BLURB HERE ***
> > 
> > Keiichi Watanabe (1):
> >   virito-blk: Support NOSPC error
> > 
> >  device-types/blk/description.tex | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > -- 
> > 2.45.2.627.g7a2c4fd464-goog
> > 
> > 

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]

On Wed, Jul 03, 2024 at 06:55:38PM -0400, Michael S. Tsirkin wrote:
> On Wed, Jun 19, 2024 at 10:00:24AM -0400, Stefan Hajnoczi wrote:
> > On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> > > This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> > > the device's ENOSPC error to the driver so the driver can treat it
> > > in a different way from other IO errors.
> > > More motivation is explained in fs-devel mail [1].
> > > 
> > > [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> > > driver. Idealy, the error should be propagated to the guest file system
> > > and handled smartly, but the part is still under discussion.
> > > 
> > > [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
> > 
> > I have replied on linux-fsdevel so we can discuss a bit more about the
> > context and get input from storage and file systems people.
> > 
> > Thanks,
> > Stefan
> 
> Stefan could you also comment on whether we are likely to add many more
> errno values soonish?

I'm not aware of other in-flight or upcoming work that will add new
virtio-blk error codes.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Michael S. Tsirkin]

On Tue, Jul 09, 2024 at 02:14:47PM +0200, Stefan Hajnoczi wrote:
> On Wed, Jul 03, 2024 at 06:55:38PM -0400, Michael S. Tsirkin wrote:
> > On Wed, Jun 19, 2024 at 10:00:24AM -0400, Stefan Hajnoczi wrote:
> > > On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> > > > This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> > > > the device's ENOSPC error to the driver so the driver can treat it
> > > > in a different way from other IO errors.
> > > > More motivation is explained in fs-devel mail [1].
> > > > 
> > > > [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> > > > driver. Idealy, the error should be propagated to the guest file system
> > > > and handled smartly, but the part is still under discussion.
> > > > 
> > > > [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
> > > 
> > > I have replied on linux-fsdevel so we can discuss a bit more about the
> > > context and get input from storage and file systems people.
> > > 
> > > Thanks,
> > > Stefan
> > 
> > Stefan could you also comment on whether we are likely to add many more
> > errno values soonish?
> 
> I'm not aware of other in-flight or upcoming work that will add new
> virtio-blk error codes.
> 
> Stefan

I'd guess the original approach is ok then, if it's going to just be
this error for the next X years, we are better off with a simple
change.

-- 
MST

Re: [PATCH 0/1] A new virtio-blk error code for host-side ENOSPC

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]

On Tue, Jul 09, 2024 at 09:31:08AM -0400, Michael S. Tsirkin wrote:
> On Tue, Jul 09, 2024 at 02:14:47PM +0200, Stefan Hajnoczi wrote:
> > On Wed, Jul 03, 2024 at 06:55:38PM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Jun 19, 2024 at 10:00:24AM -0400, Stefan Hajnoczi wrote:
> > > > On Tue, Jun 18, 2024 at 05:18:57PM +0900, Keiichi Watanabe wrote:
> > > > > This patch proposes a new error code VIRTIO_BLK_S_NOSPC to report
> > > > > the device's ENOSPC error to the driver so the driver can treat it
> > > > > in a different way from other IO errors.
> > > > > More motivation is explained in fs-devel mail [1].
> > > > > 
> > > > > [2] is a prototype implementation in crosvm and [3] is in virtio-blk
> > > > > driver. Idealy, the error should be propagated to the guest file system
> > > > > and handled smartly, but the part is still under discussion.
> > > > > 
> > > > > [1]: https://lore.kernel.org/linux-fsdevel/CAD90VcZybb0ZOVrhE-MqDPwEya8878uzA1RBwd68U7x4CufkTQ@mail.gmail.com/T/#u
> > > > 
> > > > I have replied on linux-fsdevel so we can discuss a bit more about the
> > > > context and get input from storage and file systems people.
> > > > 
> > > > Thanks,
> > > > Stefan
> > > 
> > > Stefan could you also comment on whether we are likely to add many more
> > > errno values soonish?
> > 
> > I'm not aware of other in-flight or upcoming work that will add new
> > virtio-blk error codes.
> > 
> > Stefan
> 
> I'd guess the original approach is ok then, if it's going to just be
> this error for the next X years, we are better off with a simple
> change.

Let's wait to see how logical block exhaustion support in Linux is
accepted before adding VIRTIO_BLK_S_NOSPC to the virtio-blk spec. I
think it will be pretty straightforward to merge the virtio-blk spec
once the file systems-level questions about been discussed.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]