[PATCH v3] virtio-blk: Add support for "Force Unit Access" writes

[PATCH v3] virtio-blk: Add support for "Force Unit Access" writes

[Alberto Faria]

Add a VIRTIO_BLK_F_REQ_FLAGS feature bit converting the current
`virtio_blk_req::reserved` field into a `flags` bit field, which can be
used to modify the behavior of an entire request.

Define one of the bits as signaling that a VIRTIO_BLK_T_OUT request
should be a "Force Unit Access" (FUA) write, i.e., should become stable
once the request completes. FUA writes enable better performance
compared to the alternative of waiting for a write to complete and
subsequently submitting a flush.

Leave the remaining 31 bits reserved for now.

Also add a VIRTIO_BLK_F_REQ_FLAGS_FUA feature bit indicating device
support for the aforementioned FUA bit.

This approach allows for future expansion to other request-level flags
that may each be applicable to all or just a subset of request types.
The VIRTIO_BLK_F_REQ_FLAGS feature bit ensures compatibility with legacy
devices/drivers that interpret the previously-`reserved` field as a
priority indicator.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---

QEMU + kernel patchsets and some performance numbers to follow.

v3:
- Changed to a more future-proof approach somewhat similar to what was
  suggested by Stefan.
- Included a brief rationale for the introduction of FUA write requests,
  as suggested by Michael.

v2:
- Redefine VIRTIO_BLK_T_OUT_FUA to 27 since 15 is already in use.
- Clarify that the cache mode has no impact on VIRTIO_BLK_T_OUT_FUA
  semantics.
- Allow drivers to negotiate VIRTIO_BLK_F_OUT_FUA even if they are
  incapable of sending VIRTIO_BLK_T_OUT_FUA commands.

 device-types/blk/description.tex | 50 +++++++++++++++++++++++++++-----
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
index 2712ada..6f80451 100644
--- a/device-types/blk/description.tex
+++ b/device-types/blk/description.tex
@@ -66,6 +66,13 @@ \subsection{Feature bits}\label{sec:Device Types / Block Device / Feature bits}
 	(ZNS). For brevity, these standard documents are referred as "ZBD standards"
 	from this point on in the text.
 
+\item[VIRTIO_BLK_F_REQ_FLAGS (18)] Device can interpret the \field{flags} field
+    in the \field{virtio_blk_req} structure.
+
+\item[VIRTIO_BLK_F_REQ_FLAGS_FUA (19)] Device supports the Force Unit Access
+    (FUA) flag in the \field{flags} field of the \field{virtio_blk_req}
+    structure for VIRTIO_BLK_T_OUT requests.
+
 \end{description}
 
 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Block Device / Feature bits / Legacy Interface: Feature bits}
@@ -402,6 +409,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
 \item the device MUST initialize padding bytes \field{unused2} to 0.
 \end{itemize}
 
+If VIRTIO_BLK_F_REQ_FLAGS is not negotiated, then VIRTIO_BLK_F_REQ_FLAGS_FUA
+also MUST NOT be negotiated.
+
 \subsubsection{Legacy Interface: Device Initialization}\label{sec:Device Types / Block Device / Device Initialization / Legacy Interface: Device Initialization}
 
 Because legacy devices do not have FEATURES_OK, transitional devices
@@ -434,7 +444,10 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
 \begin{lstlisting}
 struct virtio_blk_req {
         le32 type;
-        le32 reserved;
+        struct {
+                le32 fua:1;
+                le32 reserved:31;
+        } flags;
         le64 sector;
         u8 data[];
         u8 status;
@@ -459,6 +472,9 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
 #define VIRTIO_BLK_T_SECURE_ERASE   14
 \end{lstlisting}
 
+The \field{flags} field is entirely reserved unless VIRTIO_BLK_F_REQ_FLAGS is
+negotiated.
+
 The \field{sector} number indicates the offset (multiplied by 512) where
 the read or write is to occur. This field is unused and set to 0 for
 commands other than read, write and some zone operations.
@@ -873,6 +889,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
 
 A driver SHOULD accept the VIRTIO_BLK_F_RO feature if offered.
 
+If VIRTIO_BLK_F_REQ_FLAGS is negotiated:
+
+\begin{itemize}
+\item A driver MUST NOT set the \field{fua} bit in \field{flags} unless the
+    VIRTIO_BLK_F_REQ_FLAGS_FUA feature is negotiated and the request type is
+    VIRTIO_BLK_T_OUT.
+\end{itemize}
+
 A driver MUST set \field{sector} to 0 for a VIRTIO_BLK_T_FLUSH request.
 A driver SHOULD NOT include any data in a VIRTIO_BLK_T_FLUSH request.
 
@@ -962,9 +986,15 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
 operation of the device. When this happens, the device SHOULD trigger a
 configuration change notification.
 
-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.
+A device MUST set the \field{status} byte to VIRTIO_BLK_S_IOERR for a write
+request and MUST NOT write any data if any of the following conditions is true:
+
+\begin{itemize}
+\item the VIRTIO_BLK_F_RO feature if offered.
+
+\item the VIRTIO_BLK_F_REQ_FLAGS feature was negotiated and the \field{fua} bit
+    in \field{flags} is set but VIRTIO_BLK_F_REQ_FLAGS_FUA was not negotiated.
+\end{itemize}
 
 The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
 discard, secure erase and write zeroes commands if any unknown flag is set.
@@ -1000,14 +1030,20 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
   \field{writeback} field in configuration space was 0 \textbf{all the time between
   the submission of the write and its completion};
 
-\item\label{item:flush3} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
+\item\label{item:flush3} the VIRTIO_BLK_F_REQ_FLAGS_FUA feature was negotiated
+  and the \field{fua} bit in \field{flags} was set in the write request
+  (regardless of whether the VIRTIO_BLK_F_FLUSH or VIRTIO_BLK_F_CONFIG_WCE
+  features were negotiated, and regardless of the current cache mode as
+  expressed by the value of the \field{writeback} field in configuration space).
+
+\item\label{item:flush4} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
   completed} and is completed itself.
 \end{enumerate}
 
 If the device is backed by persistent storage, the device MUST ensure that
 stable writes are committed to it, before reporting completion of the write
-(cases~\ref{item:flush1} and~\ref{item:flush2}) or the flush
-(case~\ref{item:flush3}).  Failure to do so can cause data loss
+(cases~\ref{item:flush1}, \ref{item:flush2} and~\ref{item:flush3}) or the flush
+(case~\ref{item:flush4}).  Failure to do so can cause data loss
 in case of a crash.
 
 If the driver changes \field{writeback} between the submission of the write
-- 
2.51.0

Re: [PATCH v3] virtio-blk: Add support for "Force Unit Access" writes

[Stefan Hajnoczi]

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

On Wed, Sep 17, 2025 at 08:12:49AM +0100, Alberto Faria wrote:
> Add a VIRTIO_BLK_F_REQ_FLAGS feature bit converting the current
> `virtio_blk_req::reserved` field into a `flags` bit field, which can be
> used to modify the behavior of an entire request.
> 
> Define one of the bits as signaling that a VIRTIO_BLK_T_OUT request
> should be a "Force Unit Access" (FUA) write, i.e., should become stable
> once the request completes. FUA writes enable better performance
> compared to the alternative of waiting for a write to complete and
> subsequently submitting a flush.
> 
> Leave the remaining 31 bits reserved for now.
> 
> Also add a VIRTIO_BLK_F_REQ_FLAGS_FUA feature bit indicating device
> support for the aforementioned FUA bit.
> 
> This approach allows for future expansion to other request-level flags
> that may each be applicable to all or just a subset of request types.
> The VIRTIO_BLK_F_REQ_FLAGS feature bit ensures compatibility with legacy
> devices/drivers that interpret the previously-`reserved` field as a
> priority indicator.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
> 
> QEMU + kernel patchsets and some performance numbers to follow.
> 
> v3:
> - Changed to a more future-proof approach somewhat similar to what was
>   suggested by Stefan.
> - Included a brief rationale for the introduction of FUA write requests,
>   as suggested by Michael.
> 
> v2:
> - Redefine VIRTIO_BLK_T_OUT_FUA to 27 since 15 is already in use.
> - Clarify that the cache mode has no impact on VIRTIO_BLK_T_OUT_FUA
>   semantics.
> - Allow drivers to negotiate VIRTIO_BLK_F_OUT_FUA even if they are
>   incapable of sending VIRTIO_BLK_T_OUT_FUA commands.
> 
>  device-types/blk/description.tex | 50 +++++++++++++++++++++++++++-----
>  1 file changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
> index 2712ada..6f80451 100644
> --- a/device-types/blk/description.tex
> +++ b/device-types/blk/description.tex
> @@ -66,6 +66,13 @@ \subsection{Feature bits}\label{sec:Device Types / Block Device / Feature bits}
>  	(ZNS). For brevity, these standard documents are referred as "ZBD standards"
>  	from this point on in the text.
>  
> +\item[VIRTIO_BLK_F_REQ_FLAGS (18)] Device can interpret the \field{flags} field
> +    in the \field{virtio_blk_req} structure.
> +
> +\item[VIRTIO_BLK_F_REQ_FLAGS_FUA (19)] Device supports the Force Unit Access
> +    (FUA) flag in the \field{flags} field of the \field{virtio_blk_req}
> +    structure for VIRTIO_BLK_T_OUT requests.
> +
>  \end{description}
>  
>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Block Device / Feature bits / Legacy Interface: Feature bits}
> @@ -402,6 +409,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
>  \item the device MUST initialize padding bytes \field{unused2} to 0.
>  \end{itemize}
>  
> +If VIRTIO_BLK_F_REQ_FLAGS is not negotiated, then VIRTIO_BLK_F_REQ_FLAGS_FUA
> +also MUST NOT be negotiated.

Please structure this as:

  \drivernormative
  ...
  The driver MUST NOT negotiate VIRTIO_BLK_F_REQ_FLAGS_FUA without
  VIRTIO_BLK_F_REQ_FLAGS.

  \devicenormative
  ...
  The device MUST NOT acknowledge FEATURES_OK if the driver sets
  VIRTIO_BLK_F_REQ_FLAGS_FUA without VIRTIO_BLK_F_REQ_FLAGS.

That way driver authors also have a conformance clause informing them
that FUA requires request flags.

> +
>  \subsubsection{Legacy Interface: Device Initialization}\label{sec:Device Types / Block Device / Device Initialization / Legacy Interface: Device Initialization}
>  
>  Because legacy devices do not have FEATURES_OK, transitional devices
> @@ -434,7 +444,10 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>  \begin{lstlisting}
>  struct virtio_blk_req {
>          le32 type;
> -        le32 reserved;
> +        struct {
> +                le32 fua:1;
> +                le32 reserved:31;
> +        } flags;

FUA only has meaning for certain request types. I think it's worth
stating that the meaning of the flags field is specific to each request
type. That way there are more bits available in the future and there is
no need to specify how invalid flag bits are handled for the other
request types.

>          le64 sector;
>          u8 data[];
>          u8 status;
> @@ -459,6 +472,9 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>  #define VIRTIO_BLK_T_SECURE_ERASE   14
>  \end{lstlisting}
>  
> +The \field{flags} field is entirely reserved unless VIRTIO_BLK_F_REQ_FLAGS is
> +negotiated.

This statement might be interpreted as "the flags field is zero when
VIRTIO_BLK_F_REQ_FLAGS is not negotiated" by device implementors. The
Linux driver populates it with the request's I/O priority even for
non-legacy devices, so it might not be zero.

How about:

  The \field{flags} field is ignored by the device unless
  VIRTIO_BLK_F_REQ_FLAGS is negotiated.

> +
>  The \field{sector} number indicates the offset (multiplied by 512) where
>  the read or write is to occur. This field is unused and set to 0 for
>  commands other than read, write and some zone operations.
> @@ -873,6 +889,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>  
>  A driver SHOULD accept the VIRTIO_BLK_F_RO feature if offered.
>  
> +If VIRTIO_BLK_F_REQ_FLAGS is negotiated:
> +
> +\begin{itemize}
> +\item A driver MUST NOT set the \field{fua} bit in \field{flags} unless the
> +    VIRTIO_BLK_F_REQ_FLAGS_FUA feature is negotiated and the request type is
> +    VIRTIO_BLK_T_OUT.
> +\end{itemize}
> +
>  A driver MUST set \field{sector} to 0 for a VIRTIO_BLK_T_FLUSH request.
>  A driver SHOULD NOT include any data in a VIRTIO_BLK_T_FLUSH request.
>  
> @@ -962,9 +986,15 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>  operation of the device. When this happens, the device SHOULD trigger a
>  configuration change notification.
>  
> -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.
> +A device MUST set the \field{status} byte to VIRTIO_BLK_S_IOERR for a write
> +request and MUST NOT write any data if any of the following conditions is true:
> +
> +\begin{itemize}
> +\item the VIRTIO_BLK_F_RO feature if offered.
> +
> +\item the VIRTIO_BLK_F_REQ_FLAGS feature was negotiated and the \field{fua} bit
> +    in \field{flags} is set but VIRTIO_BLK_F_REQ_FLAGS_FUA was not negotiated.

This statement is in conflict with backwards compatibility. Consider the
case where a new request flag called FOO is added to the spec after this
patch is merged and it uses the same language:

  \item the VIRTIO_BLK_F_REQ_FLAGS feature was negotiated and the
  \field{foo} bit in \field{flags} is set but VIRTIO_BLK_F_REQ_FLAGS_FOO
  was not negotiated.

Existing devices don't know about REQ_FLAGS_FOO but the spec now says
they must fail the request with VIRTIO_BLK_S_IOERR if the foo bit is set
without VIRTIO_BLK_F_REQ_FLAGS_FOO.

New behavior for FUA can only be introduced when
VIRTIO_BLK_F_REQ_FLAGS_FUA has been negotiated. I think that requires
dropping this statement from this patch. (Technically it works in this
patch because VIRTIO_BLK_F_REQ_FLAGS is also introduced in the same
patch, but it won't be possible for future flags bits and therefore it
is strange to treat FUA differently.)

> +\end{itemize}
>  
>  The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
>  discard, secure erase and write zeroes commands if any unknown flag is set.
> @@ -1000,14 +1030,20 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
>    \field{writeback} field in configuration space was 0 \textbf{all the time between
>    the submission of the write and its completion};
>  
> -\item\label{item:flush3} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
> +\item\label{item:flush3} the VIRTIO_BLK_F_REQ_FLAGS_FUA feature was negotiated
> +  and the \field{fua} bit in \field{flags} was set in the write request
> +  (regardless of whether the VIRTIO_BLK_F_FLUSH or VIRTIO_BLK_F_CONFIG_WCE
> +  features were negotiated, and regardless of the current cache mode as
> +  expressed by the value of the \field{writeback} field in configuration space).

Missing "and the write request is completed"?

> +
> +\item\label{item:flush4} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
>    completed} and is completed itself.
>  \end{enumerate}
>  
>  If the device is backed by persistent storage, the device MUST ensure that
>  stable writes are committed to it, before reporting completion of the write
> -(cases~\ref{item:flush1} and~\ref{item:flush2}) or the flush
> -(case~\ref{item:flush3}).  Failure to do so can cause data loss
> +(cases~\ref{item:flush1}, \ref{item:flush2} and~\ref{item:flush3}) or the flush
> +(case~\ref{item:flush4}).  Failure to do so can cause data loss
>  in case of a crash.
>  
>  If the driver changes \field{writeback} between the submission of the write
> -- 
> 2.51.0
> 

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

Re: [PATCH v3] virtio-blk: Add support for "Force Unit Access" writes

[Alberto Faria]

On Thu, Sep 18, 2025 at 2:38 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Wed, Sep 17, 2025 at 08:12:49AM +0100, Alberto Faria wrote:
> > Add a VIRTIO_BLK_F_REQ_FLAGS feature bit converting the current
> > `virtio_blk_req::reserved` field into a `flags` bit field, which can be
> > used to modify the behavior of an entire request.
> >
> > Define one of the bits as signaling that a VIRTIO_BLK_T_OUT request
> > should be a "Force Unit Access" (FUA) write, i.e., should become stable
> > once the request completes. FUA writes enable better performance
> > compared to the alternative of waiting for a write to complete and
> > subsequently submitting a flush.
> >
> > Leave the remaining 31 bits reserved for now.
> >
> > Also add a VIRTIO_BLK_F_REQ_FLAGS_FUA feature bit indicating device
> > support for the aforementioned FUA bit.
> >
> > This approach allows for future expansion to other request-level flags
> > that may each be applicable to all or just a subset of request types.
> > The VIRTIO_BLK_F_REQ_FLAGS feature bit ensures compatibility with legacy
> > devices/drivers that interpret the previously-`reserved` field as a
> > priority indicator.
> >
> > Signed-off-by: Alberto Faria <afaria@redhat.com>
> > ---
> >
> > QEMU + kernel patchsets and some performance numbers to follow.
> >
> > v3:
> > - Changed to a more future-proof approach somewhat similar to what was
> >   suggested by Stefan.
> > - Included a brief rationale for the introduction of FUA write requests,
> >   as suggested by Michael.
> >
> > v2:
> > - Redefine VIRTIO_BLK_T_OUT_FUA to 27 since 15 is already in use.
> > - Clarify that the cache mode has no impact on VIRTIO_BLK_T_OUT_FUA
> >   semantics.
> > - Allow drivers to negotiate VIRTIO_BLK_F_OUT_FUA even if they are
> >   incapable of sending VIRTIO_BLK_T_OUT_FUA commands.
> >
> >  device-types/blk/description.tex | 50 +++++++++++++++++++++++++++-----
> >  1 file changed, 43 insertions(+), 7 deletions(-)
> >
> > diff --git a/device-types/blk/description.tex b/device-types/blk/description.tex
> > index 2712ada..6f80451 100644
> > --- a/device-types/blk/description.tex
> > +++ b/device-types/blk/description.tex
> > @@ -66,6 +66,13 @@ \subsection{Feature bits}\label{sec:Device Types / Block Device / Feature bits}
> >       (ZNS). For brevity, these standard documents are referred as "ZBD standards"
> >       from this point on in the text.
> >
> > +\item[VIRTIO_BLK_F_REQ_FLAGS (18)] Device can interpret the \field{flags} field
> > +    in the \field{virtio_blk_req} structure.
> > +
> > +\item[VIRTIO_BLK_F_REQ_FLAGS_FUA (19)] Device supports the Force Unit Access
> > +    (FUA) flag in the \field{flags} field of the \field{virtio_blk_req}
> > +    structure for VIRTIO_BLK_T_OUT requests.
> > +
> >  \end{description}
> >
> >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Block Device / Feature bits / Legacy Interface: Feature bits}
> > @@ -402,6 +409,9 @@ \subsection{Device Initialization}\label{sec:Device Types / Block Device / Devic
> >  \item the device MUST initialize padding bytes \field{unused2} to 0.
> >  \end{itemize}
> >
> > +If VIRTIO_BLK_F_REQ_FLAGS is not negotiated, then VIRTIO_BLK_F_REQ_FLAGS_FUA
> > +also MUST NOT be negotiated.
>
> Please structure this as:
>
>   \drivernormative
>   ...
>   The driver MUST NOT negotiate VIRTIO_BLK_F_REQ_FLAGS_FUA without
>   VIRTIO_BLK_F_REQ_FLAGS.
>
>   \devicenormative
>   ...
>   The device MUST NOT acknowledge FEATURES_OK if the driver sets
>   VIRTIO_BLK_F_REQ_FLAGS_FUA without VIRTIO_BLK_F_REQ_FLAGS.
>
> That way driver authors also have a conformance clause informing them
> that FUA requires request flags.

Will do.

> > +
> >  \subsubsection{Legacy Interface: Device Initialization}\label{sec:Device Types / Block Device / Device Initialization / Legacy Interface: Device Initialization}
> >
> >  Because legacy devices do not have FEATURES_OK, transitional devices
> > @@ -434,7 +444,10 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >  \begin{lstlisting}
> >  struct virtio_blk_req {
> >          le32 type;
> > -        le32 reserved;
> > +        struct {
> > +                le32 fua:1;
> > +                le32 reserved:31;
> > +        } flags;
>
> FUA only has meaning for certain request types. I think it's worth
> stating that the meaning of the flags field is specific to each request
> type. That way there are more bits available in the future and there is
> no need to specify how invalid flag bits are handled for the other
> request types.

I think this adds some complexity that may not be worth it if we're
confident 32 bits shared across all requests will always be enough.
Will do it regardless in the next version so we can compare.

> >          le64 sector;
> >          u8 data[];
> >          u8 status;
> > @@ -459,6 +472,9 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >  #define VIRTIO_BLK_T_SECURE_ERASE   14
> >  \end{lstlisting}
> >
> > +The \field{flags} field is entirely reserved unless VIRTIO_BLK_F_REQ_FLAGS is
> > +negotiated.
>
> This statement might be interpreted as "the flags field is zero when
> VIRTIO_BLK_F_REQ_FLAGS is not negotiated" by device implementors. The
> Linux driver populates it with the request's I/O priority even for
> non-legacy devices, so it might not be zero.
>
> How about:
>
>   The \field{flags} field is ignored by the device unless
>   VIRTIO_BLK_F_REQ_FLAGS is negotiated.

Sounds good.

> > +
> >  The \field{sector} number indicates the offset (multiplied by 512) where
> >  the read or write is to occur. This field is unused and set to 0 for
> >  commands other than read, write and some zone operations.
> > @@ -873,6 +889,14 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >
> >  A driver SHOULD accept the VIRTIO_BLK_F_RO feature if offered.
> >
> > +If VIRTIO_BLK_F_REQ_FLAGS is negotiated:
> > +
> > +\begin{itemize}
> > +\item A driver MUST NOT set the \field{fua} bit in \field{flags} unless the
> > +    VIRTIO_BLK_F_REQ_FLAGS_FUA feature is negotiated and the request type is
> > +    VIRTIO_BLK_T_OUT.
> > +\end{itemize}
> > +
> >  A driver MUST set \field{sector} to 0 for a VIRTIO_BLK_T_FLUSH request.
> >  A driver SHOULD NOT include any data in a VIRTIO_BLK_T_FLUSH request.
> >
> > @@ -962,9 +986,15 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >  operation of the device. When this happens, the device SHOULD trigger a
> >  configuration change notification.
> >
> > -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.
> > +A device MUST set the \field{status} byte to VIRTIO_BLK_S_IOERR for a write
> > +request and MUST NOT write any data if any of the following conditions is true:
> > +
> > +\begin{itemize}
> > +\item the VIRTIO_BLK_F_RO feature if offered.
> > +
> > +\item the VIRTIO_BLK_F_REQ_FLAGS feature was negotiated and the \field{fua} bit
> > +    in \field{flags} is set but VIRTIO_BLK_F_REQ_FLAGS_FUA was not negotiated.
>
> This statement is in conflict with backwards compatibility. Consider the
> case where a new request flag called FOO is added to the spec after this
> patch is merged and it uses the same language:
>
>   \item the VIRTIO_BLK_F_REQ_FLAGS feature was negotiated and the
>   \field{foo} bit in \field{flags} is set but VIRTIO_BLK_F_REQ_FLAGS_FOO
>   was not negotiated.
>
> Existing devices don't know about REQ_FLAGS_FOO but the spec now says
> they must fail the request with VIRTIO_BLK_S_IOERR if the foo bit is set
> without VIRTIO_BLK_F_REQ_FLAGS_FOO.
>
> New behavior for FUA can only be introduced when
> VIRTIO_BLK_F_REQ_FLAGS_FUA has been negotiated. I think that requires
> dropping this statement from this patch. (Technically it works in this
> patch because VIRTIO_BLK_F_REQ_FLAGS is also introduced in the same
> patch, but it won't be possible for future flags bits and therefore it
> is strange to treat FUA differently.)

Will fix.

> > +\end{itemize}
> >
> >  The device MUST set the \field{status} byte to VIRTIO_BLK_S_UNSUPP for
> >  discard, secure erase and write zeroes commands if any unknown flag is set.
> > @@ -1000,14 +1030,20 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> >    \field{writeback} field in configuration space was 0 \textbf{all the time between
> >    the submission of the write and its completion};
> >
> > -\item\label{item:flush3} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
> > +\item\label{item:flush3} the VIRTIO_BLK_F_REQ_FLAGS_FUA feature was negotiated
> > +  and the \field{fua} bit in \field{flags} was set in the write request
> > +  (regardless of whether the VIRTIO_BLK_F_FLUSH or VIRTIO_BLK_F_CONFIG_WCE
> > +  features were negotiated, and regardless of the current cache mode as
> > +  expressed by the value of the \field{writeback} field in configuration space).
>
> Missing "and the write request is completed"?

This should be implied by the paragraph above the itemize.

Thanks for the review,
Alberto

> > +
> > +\item\label{item:flush4} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
> >    completed} and is completed itself.
> >  \end{enumerate}
> >
> >  If the device is backed by persistent storage, the device MUST ensure that
> >  stable writes are committed to it, before reporting completion of the write
> > -(cases~\ref{item:flush1} and~\ref{item:flush2}) or the flush
> > -(case~\ref{item:flush3}).  Failure to do so can cause data loss
> > +(cases~\ref{item:flush1}, \ref{item:flush2} and~\ref{item:flush3}) or the flush
> > +(case~\ref{item:flush4}).  Failure to do so can cause data loss
> >  in case of a crash.
> >
> >  If the driver changes \field{writeback} between the submission of the write
> > --
> > 2.51.0
> >

Re: [PATCH v3] virtio-blk: Add support for "Force Unit Access" writes

[Stefan Hajnoczi]

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

On Wed, Nov 12, 2025 at 05:30:40AM +0000, Alberto Faria wrote:
> On Thu, Sep 18, 2025 at 2:38 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Wed, Sep 17, 2025 at 08:12:49AM +0100, Alberto Faria wrote:
> > > @@ -1000,14 +1030,20 @@ \subsection{Device Operation}\label{sec:Device Types / Block Device / Device Ope
> > >    \field{writeback} field in configuration space was 0 \textbf{all the time between
> > >    the submission of the write and its completion};
> > >
> > > -\item\label{item:flush3} a VIRTIO_BLK_T_FLUSH request is sent \textbf{after the write is
> > > +\item\label{item:flush3} the VIRTIO_BLK_F_REQ_FLAGS_FUA feature was negotiated
> > > +  and the \field{fua} bit in \field{flags} was set in the write request
> > > +  (regardless of whether the VIRTIO_BLK_F_FLUSH or VIRTIO_BLK_F_CONFIG_WCE
> > > +  features were negotiated, and regardless of the current cache mode as
> > > +  expressed by the value of the \field{writeback} field in configuration space).
> >
> > Missing "and the write request is completed"?
> 
> This should be implied by the paragraph above the itemize.

You are right, write completion is already mentioned:

  A write becomes stable once it is completed and one or more of the
  following conditions is true:
  - ...

Stefan

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