[RFC v2 0/4] virtio-blk: Add support for FUA write requests

[RFC v2 0/4] virtio-blk: Add support for FUA write requests

[Alberto Faria]

There is a proposal to introduce virtio-blk fua write support to the virtio
spec [1], and a matching kernel RFC [2]. This series implements the proposal by
adding VIRTIO_BLK_{F,T}_OUT_FUA uapi definitions and corresponding virtio-blk,
vhost-user-blk, and vduse-blk support.

[1] https://lore.kernel.org/virtio-comment/20250507152602.3993258-1-afaria@redhat.com/
[2] https://lore.kernel.org/linux-block/20250508001951.421467-1-afaria@redhat.com/

v2:
- Include missing hw/block/virtio-blk.c changes actually implementing
  VIRTIO_BLK_T_OUT_FUA support.

Alberto Faria (4):
  include: Add tentative virtio-blk FUA write definitions
  virtio-blk: Add VIRTIO_BLK_T_OUT_FUA command support
  vhost-user-blk: Add VIRTIO_BLK_T_OUT_FUA command support
  vduse-blk: Add VIRTIO_BLK_T_OUT_FUA command support

 block/export/vduse-blk.c                    |  3 +-
 block/export/vhost-user-blk-server.c        |  1 +
 block/export/virtio-blk-handler.c           |  7 ++-
 contrib/vhost-user-blk/vhost-user-blk.c     | 16 ++++--
 hw/block/vhost-user-blk.c                   |  3 ++
 hw/block/virtio-blk.c                       | 41 +++++++--------
 hw/core/machine.c                           |  5 +-
 hw/virtio/virtio-qmp.c                      |  2 +
 include/hw/virtio/virtio-blk.h              |  1 +
 include/standard-headers/linux/virtio_blk.h |  4 ++
 tests/qtest/vhost-user-blk-test.c           | 56 +++++++++++++++++++++
 tests/qtest/virtio-blk-test.c               | 56 +++++++++++++++++++++
 12 files changed, 168 insertions(+), 27 deletions(-)

-- 
2.49.0

[RFC v2 1/4] include: Add tentative virtio-blk FUA write definitions

[Alberto Faria]

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 include/standard-headers/linux/virtio_blk.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/standard-headers/linux/virtio_blk.h b/include/standard-headers/linux/virtio_blk.h
index d7be3cf5e42..67a373e7967 100644
--- a/include/standard-headers/linux/virtio_blk.h
+++ b/include/standard-headers/linux/virtio_blk.h
@@ -42,6 +42,7 @@
 #define VIRTIO_BLK_F_WRITE_ZEROES	14	/* WRITE ZEROES is supported */
 #define VIRTIO_BLK_F_SECURE_ERASE	16 /* Secure Erase is supported */
 #define VIRTIO_BLK_F_ZONED		17	/* Zoned block device */
+#define VIRTIO_BLK_F_OUT_FUA		18	/* FUA write is supported */
 
 /* Legacy feature bits */
 #ifndef VIRTIO_BLK_NO_LEGACY
@@ -204,6 +205,9 @@ struct virtio_blk_config {
 /* Reset All zones command */
 #define VIRTIO_BLK_T_ZONE_RESET_ALL 26
 
+/* FUA write command */
+#define VIRTIO_BLK_T_OUT_FUA        27
+
 #ifndef VIRTIO_BLK_NO_LEGACY
 /* Barrier before this op. */
 #define VIRTIO_BLK_T_BARRIER	0x80000000
-- 
2.49.0

[RFC v2 2/4] virtio-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Alberto Faria]

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 block/export/virtio-blk-handler.c |  7 ++--
 hw/block/virtio-blk.c             | 41 +++++++++++-----------
 hw/core/machine.c                 |  4 ++-
 hw/virtio/virtio-qmp.c            |  2 ++
 include/hw/virtio/virtio-blk.h    |  1 +
 tests/qtest/virtio-blk-test.c     | 56 +++++++++++++++++++++++++++++++
 6 files changed, 88 insertions(+), 23 deletions(-)

diff --git a/block/export/virtio-blk-handler.c b/block/export/virtio-blk-handler.c
index bc1cec67570..db40529fa68 100644
--- a/block/export/virtio-blk-handler.c
+++ b/block/export/virtio-blk-handler.c
@@ -169,11 +169,13 @@ int coroutine_fn virtio_blk_process_req(VirtioBlkHandler *handler,
     type = le32_to_cpu(out.type);
     switch (type & ~VIRTIO_BLK_T_BARRIER) {
     case VIRTIO_BLK_T_IN:
-    case VIRTIO_BLK_T_OUT: {
+    case VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_OUT_FUA: {
         QEMUIOVector qiov;
         int64_t offset;
         ssize_t ret = 0;
         bool is_write = type & VIRTIO_BLK_T_OUT;
+        bool is_fua = type == VIRTIO_BLK_T_OUT_FUA;
         int64_t sector_num = le64_to_cpu(out.sector);
 
         if (is_write && !handler->writable) {
@@ -197,7 +199,8 @@ int coroutine_fn virtio_blk_process_req(VirtioBlkHandler *handler,
         offset = sector_num << VIRTIO_BLK_SECTOR_BITS;
 
         if (is_write) {
-            ret = blk_co_pwritev(blk, offset, qiov.size, &qiov, 0);
+            ret = blk_co_pwritev(blk, offset, qiov.size, &qiov,
+                                 is_fua ? BDRV_REQ_FUA : 0);
         } else {
             ret = blk_co_preadv(blk, offset, qiov.size, &qiov, 0);
         }
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index b54d01d3a24..48eb468f3bd 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -220,6 +220,7 @@ static inline void submit_requests(VirtIOBlock *s, MultiReqBuffer *mrb,
     QEMUIOVector *qiov = &mrb->reqs[start]->qiov;
     int64_t sector_num = mrb->reqs[start]->sector_num;
     bool is_write = mrb->is_write;
+    bool is_fua = mrb->is_fua;
     BdrvRequestFlags flags = 0;
 
     if (num_reqs > 1) {
@@ -256,6 +257,9 @@ static inline void submit_requests(VirtIOBlock *s, MultiReqBuffer *mrb,
     }
 
     if (is_write) {
+        if (is_fua) {
+            flags |= BDRV_REQ_FUA;
+        }
         blk_aio_pwritev(blk, sector_num << BDRV_SECTOR_BITS, qiov,
                         flags, virtio_blk_rw_complete,
                         mrb->reqs[start]);
@@ -828,13 +832,16 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
 
     type = virtio_ldl_p(vdev, &req->out.type);
 
-    /* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER
-     * is an optional flag. Although a guest should not send this flag if
-     * not negotiated we ignored it in the past. So keep ignoring it. */
-    switch (type & ~(VIRTIO_BLK_T_OUT | VIRTIO_BLK_T_BARRIER)) {
+    /* VIRTIO_BLK_T_BARRIER is an optional flag. Although a guest should not
+     * send this flag if not negotiated we ignored it in the past. So keep
+     * ignoring it. */
+    switch (type & ~VIRTIO_BLK_T_BARRIER) {
     case VIRTIO_BLK_T_IN:
+    case VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_OUT_FUA:
     {
         bool is_write = type & VIRTIO_BLK_T_OUT;
+        bool is_fua = (type & ~VIRTIO_BLK_T_BARRIER) == VIRTIO_BLK_T_OUT_FUA;
         req->sector_num = virtio_ldq_p(vdev, &req->out.sector);
 
         if (is_write) {
@@ -858,10 +865,10 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
         block_acct_start(blk_get_stats(s->blk), &req->acct, req->qiov.size,
                          is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);
 
-        /* merge would exceed maximum number of requests or IO direction
-         * changes */
+        /* merge would exceed maximum number of requests or type changes */
         if (mrb->num_reqs > 0 && (mrb->num_reqs == VIRTIO_BLK_MAX_MERGE_REQS ||
                                   is_write != mrb->is_write ||
+                                  is_fua != mrb->is_fua ||
                                   !s->conf.request_merging)) {
             virtio_blk_submit_multireq(s, mrb);
         }
@@ -869,6 +876,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
         assert(mrb->num_reqs < VIRTIO_BLK_MAX_MERGE_REQS);
         mrb->reqs[mrb->num_reqs++] = req;
         mrb->is_write = is_write;
+        mrb->is_fua = is_fua;
         break;
     }
     case VIRTIO_BLK_T_FLUSH:
@@ -910,7 +918,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
         g_free(req);
         break;
     }
-    case VIRTIO_BLK_T_ZONE_APPEND & ~VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_ZONE_APPEND:
         /*
          * Passing out_iov/out_num and in_iov/in_num is not safe
          * to access req->elem.out_sg directly because it may be
@@ -918,13 +926,8 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
          */
         virtio_blk_handle_zone_append(req, out_iov, in_iov, out_num, in_num);
         break;
-    /*
-     * VIRTIO_BLK_T_DISCARD and VIRTIO_BLK_T_WRITE_ZEROES are defined with
-     * VIRTIO_BLK_T_OUT flag set. We masked this flag in the switch statement,
-     * so we must mask it for these requests, then we will check if it is set.
-     */
-    case VIRTIO_BLK_T_DISCARD & ~VIRTIO_BLK_T_OUT:
-    case VIRTIO_BLK_T_WRITE_ZEROES & ~VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_DISCARD:
+    case VIRTIO_BLK_T_WRITE_ZEROES:
     {
         struct virtio_blk_discard_write_zeroes dwz_hdr;
         size_t out_len = iov_size(out_iov, out_num);
@@ -932,12 +935,8 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
                                VIRTIO_BLK_T_WRITE_ZEROES;
         uint8_t err_status;
 
-        /*
-         * Unsupported if VIRTIO_BLK_T_OUT is not set or the request contains
-         * more than one segment.
-         */
-        if (unlikely(!(type & VIRTIO_BLK_T_OUT) ||
-                     out_len > sizeof(dwz_hdr))) {
+        /* Unsupported if the request contains more than one segment. */
+        if (unlikely(out_len > sizeof(dwz_hdr))) {
             virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
             g_free(req);
             return 0;
@@ -1882,6 +1881,8 @@ static const Property virtio_blk_properties[] = {
                        conf.max_discard_sectors, BDRV_REQUEST_MAX_SECTORS),
     DEFINE_PROP_UINT32("max-write-zeroes-sectors", VirtIOBlock,
                        conf.max_write_zeroes_sectors, BDRV_REQUEST_MAX_SECTORS),
+    DEFINE_PROP_BIT64("fua-write", VirtIOBlock, host_features,
+                      VIRTIO_BLK_F_OUT_FUA, true),
     DEFINE_PROP_BOOL("x-enable-wce-if-config-wce", VirtIOBlock,
                      conf.x_enable_wce_if_config_wce, true),
 };
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ed01798d37c..8439b094904 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,7 +37,9 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "audio/audio.h"
 
-GlobalProperty hw_compat_10_0[] = {};
+GlobalProperty hw_compat_10_0[] = {
+    { "virtio-blk-device", "fua-write", "false" },
+};
 const size_t hw_compat_10_0_len = G_N_ELEMENTS(hw_compat_10_0);
 
 GlobalProperty hw_compat_9_2[] = {
diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 3b6377cf0d2..9cbdd543623 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -173,6 +173,8 @@ static const qmp_virtio_feature_map_t virtio_blk_feature_map[] = {
             "VIRTIO_BLK_F_SECURE_ERASE: Secure erase supported"),
     FEATURE_ENTRY(VIRTIO_BLK_F_ZONED, \
             "VIRTIO_BLK_F_ZONED: Zoned block devices"),
+    FEATURE_ENTRY(VIRTIO_BLK_F_OUT_FUA, \
+            "VIRTIO_BLK_F_OUT_FUA: FUA write command supported"),
 #ifndef VIRTIO_BLK_NO_LEGACY
     FEATURE_ENTRY(VIRTIO_BLK_F_BARRIER, \
             "VIRTIO_BLK_F_BARRIER: Request barriers supported"),
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 3d8dee7ec15..916df26b6f0 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -98,6 +98,7 @@ typedef struct MultiReqBuffer {
     VirtIOBlockReq *reqs[VIRTIO_BLK_MAX_MERGE_REQS];
     unsigned int num_reqs;
     bool is_write;
+    bool is_fua;
 } MultiReqBuffer;
 
 typedef struct VirtIOBlkClass {
diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c
index 98c906ebb4a..a90838b8912 100644
--- a/tests/qtest/virtio-blk-test.c
+++ b/tests/qtest/virtio-blk-test.c
@@ -88,6 +88,7 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
     switch (req->type) {
     case VIRTIO_BLK_T_IN:
     case VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_OUT_FUA:
         g_assert_cmpuint(data_size % 512, ==, 0);
         break;
     case VIRTIO_BLK_T_DISCARD:
@@ -280,6 +281,61 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
         guest_free(alloc, req_addr);
     }
 
+    if (features & (1u << VIRTIO_BLK_F_OUT_FUA)) {
+        /* FUA write and read with 3 descriptor layout */
+        /* FUA write request */
+        req.type = VIRTIO_BLK_T_OUT_FUA;
+        req.ioprio = 1;
+        req.sector = 0;
+        req.data = g_malloc0(512);
+        strcpy(req.data, "test");
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                            QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        guest_free(alloc, req_addr);
+
+        /* Read request */
+        req.type = VIRTIO_BLK_T_IN;
+        req.ioprio = 1;
+        req.sector = 0;
+        req.data = g_malloc0(512);
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                            QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        data = g_malloc0(512);
+        memread(req_addr + 16, data, 512);
+        g_assert_cmpstr(data, ==, "test");
+        g_free(data);
+
+        guest_free(alloc, req_addr);
+    }
+
     if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
         /* Write and read with 2 descriptor layout */
         /* Write request */
-- 
2.49.0

[RFC v2 3/4] vhost-user-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Alberto Faria]

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 block/export/vhost-user-blk-server.c    |  1 +
 contrib/vhost-user-blk/vhost-user-blk.c | 16 +++++--
 hw/block/vhost-user-blk.c               |  3 ++
 hw/core/machine.c                       |  1 +
 tests/qtest/vhost-user-blk-test.c       | 56 +++++++++++++++++++++++++
 5 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
index d9d2014d9b7..fad9560b219 100644
--- a/block/export/vhost-user-blk-server.c
+++ b/block/export/vhost-user-blk-server.c
@@ -125,6 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev)
                1ull << VIRTIO_BLK_F_FLUSH |
                1ull << VIRTIO_BLK_F_DISCARD |
                1ull << VIRTIO_BLK_F_WRITE_ZEROES |
+               1ull << VIRTIO_BLK_F_OUT_FUA |
                1ull << VIRTIO_BLK_F_CONFIG_WCE |
                1ull << VIRTIO_BLK_F_MQ |
                1ull << VIRTIO_F_VERSION_1 |
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index 6cc18a1c04f..8d732bbe468 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -154,7 +154,7 @@ vub_readv(VubReq *req, struct iovec *iov, uint32_t iovcnt)
 }
 
 static ssize_t
-vub_writev(VubReq *req, struct iovec *iov, uint32_t iovcnt)
+vub_writev(VubReq *req, struct iovec *iov, uint32_t iovcnt, int flags)
 {
     VubDev *vdev_blk = req->vdev_blk;
     ssize_t rc;
@@ -271,12 +271,19 @@ static int vub_virtio_process_req(VubDev *vdev_blk,
     type = le32_to_cpu(req->out->type);
     switch (type & ~VIRTIO_BLK_T_BARRIER) {
     case VIRTIO_BLK_T_IN:
-    case VIRTIO_BLK_T_OUT: {
+    case VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_OUT_FUA: {
         ssize_t ret = 0;
         bool is_write = type & VIRTIO_BLK_T_OUT;
+        int flags = 0;
         req->sector_num = le64_to_cpu(req->out->sector);
         if (is_write) {
-            ret  = vub_writev(req, &elem->out_sg[1], out_num);
+            #ifdef RWF_SYNC
+            if (type == VIRTIO_BLK_T_OUT_FUA) {
+                flags |= RWF_SYNC;
+            }
+            #endif
+            ret  = vub_writev(req, &elem->out_sg[1], out_num, flags);
         } else {
             ret = vub_readv(req, &elem->in_sg[0], in_num);
         }
@@ -379,6 +386,9 @@ vub_get_features(VuDev *dev)
                1ull << VIRTIO_BLK_F_DISCARD |
                1ull << VIRTIO_BLK_F_WRITE_ZEROES |
                #endif
+               #ifdef RWF_SYNC
+               1ull << VIRTIO_BLK_F_OUT_FUA |
+               #endif
                1ull << VIRTIO_BLK_F_CONFIG_WCE;
 
     if (vdev_blk->enable_ro) {
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 4bb5ed299e7..988be625969 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -44,6 +44,7 @@ static const int user_feature_bits[] = {
     VIRTIO_BLK_F_CONFIG_WCE,
     VIRTIO_BLK_F_DISCARD,
     VIRTIO_BLK_F_WRITE_ZEROES,
+    VIRTIO_BLK_F_OUT_FUA,
     VIRTIO_F_VERSION_1,
     VIRTIO_RING_F_INDIRECT_DESC,
     VIRTIO_RING_F_EVENT_IDX,
@@ -581,6 +582,8 @@ static const Property vhost_user_blk_properties[] = {
                       VIRTIO_BLK_F_DISCARD, true),
     DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, parent_obj.host_features,
                       VIRTIO_BLK_F_WRITE_ZEROES, true),
+    DEFINE_PROP_BIT64("fua-write", VHostUserBlk, parent_obj.host_features,
+                      VIRTIO_BLK_F_OUT_FUA, true),
 };
 
 static void vhost_user_blk_class_init(ObjectClass *klass, const void *data)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 8439b094904..bcf4f3423cb 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,7 @@
 
 GlobalProperty hw_compat_10_0[] = {
     { "virtio-blk-device", "fua-write", "false" },
+    { "vhost-user-blk", "fua-write", "false" },
 };
 const size_t hw_compat_10_0_len = G_N_ELEMENTS(hw_compat_10_0);
 
diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
index ea90d41232e..4c68702e0b0 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -72,6 +72,7 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
     switch (req->type) {
     case VIRTIO_BLK_T_IN:
     case VIRTIO_BLK_T_OUT:
+    case VIRTIO_BLK_T_OUT_FUA:
         g_assert_cmpuint(data_size % 512, ==, 0);
         break;
     case VIRTIO_BLK_T_DISCARD:
@@ -389,6 +390,61 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
                                           VIRTIO_BLK_T_DISCARD);
     }
 
+    if (features & (1u << VIRTIO_BLK_F_OUT_FUA)) {
+        /* FUA write and read with 3 descriptor layout */
+        /* FUA write request */
+        req.type = VIRTIO_BLK_T_OUT_FUA;
+        req.ioprio = 1;
+        req.sector = 0;
+        req.data = g_malloc0(512);
+        strcpy(req.data, "test");
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                            QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        guest_free(alloc, req_addr);
+
+        /* Read request */
+        req.type = VIRTIO_BLK_T_IN;
+        req.ioprio = 1;
+        req.sector = 0;
+        req.data = g_malloc0(512);
+
+        req_addr = virtio_blk_request(alloc, dev, &req, 512);
+
+        g_free(req.data);
+
+        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
+        qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
+        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
+
+        qvirtqueue_kick(qts, dev, vq, free_head);
+
+        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
+                            QVIRTIO_BLK_TIMEOUT_US);
+        status = readb(req_addr + 528);
+        g_assert_cmpint(status, ==, 0);
+
+        data = g_malloc0(512);
+        qtest_memread(qts, req_addr + 16, data, 512);
+        g_assert_cmpstr(data, ==, "test");
+        g_free(data);
+
+        guest_free(alloc, req_addr);
+    }
+
     if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
         /* Write and read with 2 descriptor layout */
         /* Write request */
-- 
2.49.0

[RFC v2 4/4] vduse-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Alberto Faria]

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 block/export/vduse-blk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
index bd852e538dc..36c3c0a56e6 100644
--- a/block/export/vduse-blk.c
+++ b/block/export/vduse-blk.c
@@ -329,7 +329,8 @@ static int vduse_blk_exp_create(BlockExport *exp, BlockExportOptions *opts,
                (1ULL << VIRTIO_BLK_F_BLK_SIZE) |
                (1ULL << VIRTIO_BLK_F_FLUSH) |
                (1ULL << VIRTIO_BLK_F_DISCARD) |
-               (1ULL << VIRTIO_BLK_F_WRITE_ZEROES);
+               (1ULL << VIRTIO_BLK_F_WRITE_ZEROES) |
+               (1ULL << VIRTIO_BLK_F_OUT_FUA);
 
     if (num_queues > 1) {
         features |= 1ULL << VIRTIO_BLK_F_MQ;
-- 
2.49.0

Re: [RFC v2 1/4] include: Add tentative virtio-blk FUA write definitions

[Stefan Hajnoczi]

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

On Thu, May 08, 2025 at 05:20:41PM +0100, Alberto Faria wrote:
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  include/standard-headers/linux/virtio_blk.h | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

Re: [RFC v2 2/4] virtio-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Stefan Hajnoczi]

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

On Thu, May 08, 2025 at 05:20:42PM +0100, Alberto Faria wrote:
> @@ -828,13 +832,16 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
>  
>      type = virtio_ldl_p(vdev, &req->out.type);
>  
> -    /* VIRTIO_BLK_T_OUT defines the command direction. VIRTIO_BLK_T_BARRIER
> -     * is an optional flag. Although a guest should not send this flag if
> -     * not negotiated we ignored it in the past. So keep ignoring it. */
> -    switch (type & ~(VIRTIO_BLK_T_OUT | VIRTIO_BLK_T_BARRIER)) {
> +    /* VIRTIO_BLK_T_BARRIER is an optional flag. Although a guest should not
> +     * send this flag if not negotiated we ignored it in the past. So keep
> +     * ignoring it. */
> +    switch (type & ~VIRTIO_BLK_T_BARRIER) {

This changes the behavior of the device. VIRTIO_BLK_T_FLUSH |
VIRTIO_BLK_T_OUT is now treated as an unsupported command instead of a
flush. The same is true for the other command types as well (like zoned
devices, discard, etc).

Buggy guest drivers might depend on this behavior. From a user
perspective it's QEMU's fault if existing guests break, even if the
guest driver violates the VIRTIO specification. I would treat this as a
stable ABI that third-party virtio-blk drivers depend on unless there is
a strong reason to change existing behavior.

Can you add "case VIRTIO_BLK_T_OUT_FUA & ~VIRTIO_BLK_T_OUT:" instead of
changing existing behavior?

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

Re: [RFC v2 3/4] vhost-user-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Stefan Hajnoczi]

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

On Thu, May 08, 2025 at 05:20:43PM +0100, Alberto Faria wrote:
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  block/export/vhost-user-blk-server.c    |  1 +
>  contrib/vhost-user-blk/vhost-user-blk.c | 16 +++++--
>  hw/block/vhost-user-blk.c               |  3 ++
>  hw/core/machine.c                       |  1 +
>  tests/qtest/vhost-user-blk-test.c       | 56 +++++++++++++++++++++++++
>  5 files changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/block/export/vhost-user-blk-server.c b/block/export/vhost-user-blk-server.c
> index d9d2014d9b7..fad9560b219 100644
> --- a/block/export/vhost-user-blk-server.c
> +++ b/block/export/vhost-user-blk-server.c
> @@ -125,6 +125,7 @@ static uint64_t vu_blk_get_features(VuDev *dev)
>                 1ull << VIRTIO_BLK_F_FLUSH |
>                 1ull << VIRTIO_BLK_F_DISCARD |
>                 1ull << VIRTIO_BLK_F_WRITE_ZEROES |
> +               1ull << VIRTIO_BLK_F_OUT_FUA |
>                 1ull << VIRTIO_BLK_F_CONFIG_WCE |
>                 1ull << VIRTIO_BLK_F_MQ |
>                 1ull << VIRTIO_F_VERSION_1 |
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 6cc18a1c04f..8d732bbe468 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -154,7 +154,7 @@ vub_readv(VubReq *req, struct iovec *iov, uint32_t iovcnt)
>  }
>  
>  static ssize_t
> -vub_writev(VubReq *req, struct iovec *iov, uint32_t iovcnt)
> +vub_writev(VubReq *req, struct iovec *iov, uint32_t iovcnt, int flags)
>  {
>      VubDev *vdev_blk = req->vdev_blk;
>      ssize_t rc;

Where is flags used?

> @@ -271,12 +271,19 @@ static int vub_virtio_process_req(VubDev *vdev_blk,
>      type = le32_to_cpu(req->out->type);
>      switch (type & ~VIRTIO_BLK_T_BARRIER) {
>      case VIRTIO_BLK_T_IN:
> -    case VIRTIO_BLK_T_OUT: {
> +    case VIRTIO_BLK_T_OUT:
> +    case VIRTIO_BLK_T_OUT_FUA: {
>          ssize_t ret = 0;
>          bool is_write = type & VIRTIO_BLK_T_OUT;
> +        int flags = 0;
>          req->sector_num = le64_to_cpu(req->out->sector);
>          if (is_write) {
> -            ret  = vub_writev(req, &elem->out_sg[1], out_num);
> +            #ifdef RWF_SYNC
> +            if (type == VIRTIO_BLK_T_OUT_FUA) {
> +                flags |= RWF_SYNC;
> +            }
> +            #endif
> +            ret  = vub_writev(req, &elem->out_sg[1], out_num, flags);
>          } else {
>              ret = vub_readv(req, &elem->in_sg[0], in_num);
>          }
> @@ -379,6 +386,9 @@ vub_get_features(VuDev *dev)
>                 1ull << VIRTIO_BLK_F_DISCARD |
>                 1ull << VIRTIO_BLK_F_WRITE_ZEROES |
>                 #endif
> +               #ifdef RWF_SYNC
> +               1ull << VIRTIO_BLK_F_OUT_FUA |
> +               #endif
>                 1ull << VIRTIO_BLK_F_CONFIG_WCE;
>  
>      if (vdev_blk->enable_ro) {
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 4bb5ed299e7..988be625969 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -44,6 +44,7 @@ static const int user_feature_bits[] = {
>      VIRTIO_BLK_F_CONFIG_WCE,
>      VIRTIO_BLK_F_DISCARD,
>      VIRTIO_BLK_F_WRITE_ZEROES,
> +    VIRTIO_BLK_F_OUT_FUA,
>      VIRTIO_F_VERSION_1,
>      VIRTIO_RING_F_INDIRECT_DESC,
>      VIRTIO_RING_F_EVENT_IDX,
> @@ -581,6 +582,8 @@ static const Property vhost_user_blk_properties[] = {
>                        VIRTIO_BLK_F_DISCARD, true),
>      DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, parent_obj.host_features,
>                        VIRTIO_BLK_F_WRITE_ZEROES, true),
> +    DEFINE_PROP_BIT64("fua-write", VHostUserBlk, parent_obj.host_features,
> +                      VIRTIO_BLK_F_OUT_FUA, true),
>  };
>  
>  static void vhost_user_blk_class_init(ObjectClass *klass, const void *data)
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 8439b094904..bcf4f3423cb 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -39,6 +39,7 @@
>  
>  GlobalProperty hw_compat_10_0[] = {
>      { "virtio-blk-device", "fua-write", "false" },
> +    { "vhost-user-blk", "fua-write", "false" },
>  };
>  const size_t hw_compat_10_0_len = G_N_ELEMENTS(hw_compat_10_0);
>  
> diff --git a/tests/qtest/vhost-user-blk-test.c b/tests/qtest/vhost-user-blk-test.c
> index ea90d41232e..4c68702e0b0 100644
> --- a/tests/qtest/vhost-user-blk-test.c
> +++ b/tests/qtest/vhost-user-blk-test.c
> @@ -72,6 +72,7 @@ static uint64_t virtio_blk_request(QGuestAllocator *alloc, QVirtioDevice *d,
>      switch (req->type) {
>      case VIRTIO_BLK_T_IN:
>      case VIRTIO_BLK_T_OUT:
> +    case VIRTIO_BLK_T_OUT_FUA:
>          g_assert_cmpuint(data_size % 512, ==, 0);
>          break;
>      case VIRTIO_BLK_T_DISCARD:
> @@ -389,6 +390,61 @@ static QVirtQueue *test_basic(QVirtioDevice *dev, QGuestAllocator *alloc)
>                                            VIRTIO_BLK_T_DISCARD);
>      }
>  
> +    if (features & (1u << VIRTIO_BLK_F_OUT_FUA)) {
> +        /* FUA write and read with 3 descriptor layout */
> +        /* FUA write request */
> +        req.type = VIRTIO_BLK_T_OUT_FUA;
> +        req.ioprio = 1;
> +        req.sector = 0;
> +        req.data = g_malloc0(512);
> +        strcpy(req.data, "test");
> +
> +        req_addr = virtio_blk_request(alloc, dev, &req, 512);
> +
> +        g_free(req.data);
> +
> +        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> +        qvirtqueue_add(qts, vq, req_addr + 16, 512, false, true);
> +        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> +
> +        qvirtqueue_kick(qts, dev, vq, free_head);
> +
> +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> +                            QVIRTIO_BLK_TIMEOUT_US);
> +        status = readb(req_addr + 528);
> +        g_assert_cmpint(status, ==, 0);
> +
> +        guest_free(alloc, req_addr);
> +
> +        /* Read request */
> +        req.type = VIRTIO_BLK_T_IN;
> +        req.ioprio = 1;
> +        req.sector = 0;
> +        req.data = g_malloc0(512);
> +
> +        req_addr = virtio_blk_request(alloc, dev, &req, 512);
> +
> +        g_free(req.data);
> +
> +        free_head = qvirtqueue_add(qts, vq, req_addr, 16, false, true);
> +        qvirtqueue_add(qts, vq, req_addr + 16, 512, true, true);
> +        qvirtqueue_add(qts, vq, req_addr + 528, 1, true, false);
> +
> +        qvirtqueue_kick(qts, dev, vq, free_head);
> +
> +        qvirtio_wait_used_elem(qts, dev, vq, free_head, NULL,
> +                            QVIRTIO_BLK_TIMEOUT_US);
> +        status = readb(req_addr + 528);
> +        g_assert_cmpint(status, ==, 0);
> +
> +        data = g_malloc0(512);
> +        qtest_memread(qts, req_addr + 16, data, 512);
> +        g_assert_cmpstr(data, ==, "test");
> +        g_free(data);
> +
> +        guest_free(alloc, req_addr);
> +    }
> +
>      if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
>          /* Write and read with 2 descriptor layout */
>          /* Write request */
> -- 
> 2.49.0
> 

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

Re: [RFC v2 4/4] vduse-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Stefan Hajnoczi]

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

On Thu, May 08, 2025 at 05:20:44PM +0100, Alberto Faria wrote:

If you respin, please add something to the commit description explaining
that an earlier commit already updated
block/export/virtio-blk-handler.c:virtio_blk_process_req() to handle FUA
requests.

> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  block/export/vduse-blk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
> index bd852e538dc..36c3c0a56e6 100644
> --- a/block/export/vduse-blk.c
> +++ b/block/export/vduse-blk.c
> @@ -329,7 +329,8 @@ static int vduse_blk_exp_create(BlockExport *exp, BlockExportOptions *opts,
>                 (1ULL << VIRTIO_BLK_F_BLK_SIZE) |
>                 (1ULL << VIRTIO_BLK_F_FLUSH) |
>                 (1ULL << VIRTIO_BLK_F_DISCARD) |
> -               (1ULL << VIRTIO_BLK_F_WRITE_ZEROES);
> +               (1ULL << VIRTIO_BLK_F_WRITE_ZEROES) |
> +               (1ULL << VIRTIO_BLK_F_OUT_FUA);
>  
>      if (num_queues > 1) {
>          features |= 1ULL << VIRTIO_BLK_F_MQ;
> -- 
> 2.49.0
> 

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

Re: [RFC v2 4/4] vduse-blk: Add VIRTIO_BLK_T_OUT_FUA command support

[Stefan Hajnoczi]

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

On Thu, May 08, 2025 at 05:20:44PM +0100, Alberto Faria wrote:
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  block/export/vduse-blk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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