[PATCH v4 00/12] block: fix the zone write granularity and the zone append limit

[PATCH v4 00/12] block: fix the zone write granularity and the zone append limit

[Niklas Cassel]

Hello Stefan, Kevin, and everyone else,

This series fixes how QEMU reports and enforces the two constraints a zoned
device places on a write to a sequential zone: the write granularity, and
the largest zone append it accepts. It also fixes two bugs in the zone
append emulation in file-posix, one of which a guest can reach.

Many of these patches are in preparation for Sam Li's zoned qcow2 series.
The first two patches in the series are taken directly from there, as they
are unrelated to qcow2.

Patch 3 is preparation with no functional change: deriving the zone that an
offset belongs to was open coded in five places, so it becomes a helper.

Patches 4 to 6 concern the write granularity. virtio-blk reported the
logical block size while the driver enforced the backend value, so on a
512e SMR disk a guest could be told that a request was valid and get an
I/O error for it. Writes to sequential zones were not checked against the
granularity at all, and zone appends had only their offset checked, not
their length. Patch 6 then refuses at realize a device whose write
pointers the configured logical block size cannot address, which needs no
emulated backend to provoke: write 512 bytes to a zone of a null_blk
device and attach it with logical_block_size=4096.

Patches 7 to 10 concern the append limit. The sector invariant moves to
bdrv_co_zone_append(), since a write pointer is tracked in sectors and
cannot represent anything finer, and file-posix drops its own check which
conflated that with the coarser granularity of the medium. file-posix then
stops reporting zone_append_max_bytes, which bounds REQ_OP_ZONE_APPEND, an
operation it never issues: it appends with an ordinary pwritev(), so
max_hw_transfer is the limit that applies. Finally virtio-blk derives what
it advertises rather than passing BlockLimits.max_append_sectors through,
which made an unset field mean "zone append unsupported" rather than "no
limit of its own", and Linux refuses to attach a zoned device that reports
zero.

Patches 11 and 12 fix the write pointer that raw_co_prw() substitutes for
the offset of an append. An offset that is never bounded against the
device derives an out of range zone index and reads past the write pointer
array, which qemu-io can reach. An append to a full zone uses a pointer
recorded at the end of the zone, so the data is written into the next zone
and success is returned; a guest can reach that one, because nothing in
virtio-blk checks whether a zone is full.


Changes since v3:
-Picked up tags from Damien.
-Replaced ctz64() with a zone_size_bits in struct BlockLimits.


Niklas Cassel (10):
  block: add a helper for the index of the zone an offset falls in
  virtio-blk: report the effective zone write granularity
  virtio-blk: check the write granularity of writes to sequential zones
  hw/block: reject a zoned device whose write pointers are unaddressable
  block: reject zone appends that are not a multiple of the sector size
  file-posix: remove the zone append write granularity check
  file-posix: base the zone append limit on the transfer limit
  virtio-blk: derive the maximum zone append size
  file-posix: reject a zone append past the device capacity
  file-posix: reject a zone append to a full or conventional zone

Sam Li (2):
  block: widen BlockLimits.zone_size to uint64_t
  virtio-blk: do not merge requests across a zone boundary

 block/block-backend.c             |  11 +++
 block/file-posix.c                |  71 +++++++++++++-------
 block/io.c                        |  31 +++++++++
 hw/block/block.c                  |  53 +++++++++++++++
 hw/block/virtio-blk.c             | 108 +++++++++++++++++++++++++-----
 include/block/block-io.h          |   8 +++
 include/block/block_int-common.h  |   9 ++-
 include/hw/block/block.h          |   9 +++
 include/system/block-backend-io.h |   1 +
 9 files changed, 260 insertions(+), 41 deletions(-)

-- 
2.55.0

[PATCH v4 01/12] block: widen BlockLimits.zone_size to uint64_t

[Niklas Cassel]

From: Sam Li <faithilikerun@gmail.com>

The zone-size field in BlockLimits is currently uint32_t, capping
expressible zone sizes at 4 GiB. Real zoned-device protocols
like NVMe ZNS allow larger zones. Widen BlockLimits.zone_size to
uint64_t to match.

Signed-off-by: Sam Li <faithilikerun@gmail.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c               | 2 +-
 include/block/block_int-common.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 9aad156aad..c0d6ee30e4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3598,7 +3598,7 @@ raw_co_zone_append(BlockDriverState *bs,
 
     if (*offset & zone_size_mask) {
         error_report("sector offset %" PRId64 " is not aligned to zone size "
-                     "%" PRId32 "", *offset / 512, bs->bl.zone_size / 512);
+                     "%" PRId64 "", *offset / 512, bs->bl.zone_size / 512);
         return -EINVAL;
     }
 
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 147c08155f..7571ed9968 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -901,7 +901,7 @@ typedef struct BlockLimits {
     BlockZoneModel zoned;
 
     /* zone size expressed in bytes */
-    uint32_t zone_size;
+    uint64_t zone_size;
 
     /* total number of zones */
     uint32_t nr_zones;
-- 
2.55.0

[PATCH v4 02/12] virtio-blk: do not merge requests across a zone boundary

[Niklas Cassel]

From: Sam Li <faithilikerun@gmail.com>

virtio_blk_submit_multireq() fuses adjacent in-zone requests into a
single request. On a zoned backend, a merged zone append request
that straddles a zone boundary is rejected by the device because
each write/read must stay within a single zone (virtio 1.4, 5.2.6.1).

Add a bail condition to the merge coalescer: if combining the
candidate request into the current batch would cross a zone
boundary, flush the current batch and start a new one.

Signed-off-by: Sam Li <faithilikerun@gmail.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/block-backend.c             | 11 +++++++++++
 hw/block/virtio-blk.c             | 22 +++++++++++++++++++++-
 include/system/block-backend-io.h |  1 +
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 37ba7e9fc4..049e70ddcb 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2326,6 +2326,17 @@ uint32_t blk_get_request_alignment(BlockBackend *blk)
     return bs ? bs->bl.request_alignment : BDRV_SECTOR_SIZE;
 }
 
+/*
+ * Returns the zone size in bytes for a zoned backend, or 0 if @blk does
+ * not present zoned geometry.
+ */
+uint64_t blk_get_zone_size(BlockBackend *blk)
+{
+    BlockDriverState *bs = blk_bs(blk);
+    IO_CODE();
+    return bs ? bs->bl.zone_size : 0;
+}
+
 /* Returns the optimal write zeroes alignment, in bytes; guaranteed nonzero */
 uint32_t blk_get_pwrite_zeroes_alignment(BlockBackend *blk)
 {
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6b92066aff..ffcd327803 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -294,6 +294,9 @@ static void virtio_blk_submit_multireq(VirtIOBlock *s, MultiReqBuffer *mrb)
     int i = 0, start = 0, num_reqs = 0, niov = 0, nb_sectors = 0;
     uint32_t max_transfer;
     int64_t sector_num = 0;
+    uint64_t zone_size = blk_get_zone_size(s->blk);
+    bool zone_cross;
+    int64_t zone_nr_sectors, end_sector;
 
     if (mrb->num_reqs == 1) {
         submit_requests(s, mrb, 0, 1, -1);
@@ -309,17 +312,34 @@ static void virtio_blk_submit_multireq(VirtIOBlock *s, MultiReqBuffer *mrb)
     for (i = 0; i < mrb->num_reqs; i++) {
         VirtIOBlockReq *req = mrb->reqs[i];
         if (num_reqs > 0) {
+            /*
+             * On zoned backends, a single backend write/read must not span
+             * a zone boundary. Bail out of merging if combining req into
+             * the current batch would straddle a zone.
+             */
+            if (zone_size > 0) {
+                zone_nr_sectors = zone_size / BDRV_SECTOR_SIZE;
+                end_sector = req->sector_num
+                                + req->qiov.size / BDRV_SECTOR_SIZE - 1;
+                zone_cross = (sector_num / zone_nr_sectors) !=
+                             (end_sector / zone_nr_sectors);
+            } else {
+                zone_cross = false;
+            }
+
             /*
              * NOTE: We cannot merge the requests in below situations:
              * 1. requests are not sequential
              * 2. merge would exceed maximum number of IOVs
              * 3. merge would exceed maximum transfer length of backend device
+             * 4. merge would cross a zone boundary on a zoned backend
              */
             if (sector_num + nb_sectors != req->sector_num ||
                 niov > blk_get_max_iov(s->blk) - req->qiov.niov ||
                 req->qiov.size > max_transfer ||
                 nb_sectors > (max_transfer -
-                              req->qiov.size) / BDRV_SECTOR_SIZE) {
+                              req->qiov.size) / BDRV_SECTOR_SIZE ||
+                zone_cross) {
                 submit_requests(s, mrb, start, num_reqs, niov);
                 num_reqs = 0;
             }
diff --git a/include/system/block-backend-io.h b/include/system/block-backend-io.h
index fd84723d9d..78d410b8df 100644
--- a/include/system/block-backend-io.h
+++ b/include/system/block-backend-io.h
@@ -121,6 +121,7 @@ uint32_t blk_get_request_alignment(BlockBackend *blk);
 uint32_t blk_get_pwrite_zeroes_alignment(BlockBackend *blk);
 uint32_t blk_get_max_transfer(BlockBackend *blk);
 uint64_t blk_get_max_hw_transfer(BlockBackend *blk);
+uint64_t blk_get_zone_size(BlockBackend *blk);
 
 int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
                                    BlockBackend *blk_out, int64_t off_out,
-- 
2.55.0

[PATCH v4 03/12] block: add a helper for the index of the zone an offset falls in

[Niklas Cassel]

Deriving the zone that an offset belongs to is open coded in five places,
in two files, as a division of the offset by BlockLimits.zone_size.

The zone size of a zoned device is always a power of two. Linux requires
that of every zoned device it accepts, in blk_revalidate_disk_zones() and
in each of the drivers that report one, and qcow2 rejects an image whose
header says otherwise. So the division is a shift, and the shift is worth
deriving once rather than at every use: record it in BlockLimits as
zone_size_bits, next to the size it comes from, and assert the property
that it relies on where it is computed.

Add bdrv_zone_index() and use it. No functional change; the count of zones
that a management operation spans stays a division, since it is a length
rather than an offset.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c               |  8 ++++----
 block/io.c                       | 12 ++++++++++++
 hw/block/virtio-blk.c            |  2 +-
 include/block/block-io.h         |  2 ++
 include/block/block_int-common.h |  7 +++++++
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index c0d6ee30e4..00323f7a9f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1359,7 +1359,7 @@ static int get_zones_wp(BlockDriverState *bs, int fd, int64_t offset,
     size_t rep_size;
     uint64_t sector = offset >> BDRV_SECTOR_BITS;
     BlockZoneWps *wps = bs->wps;
-    unsigned int j = offset / bs->bl.zone_size;
+    unsigned int j = bdrv_zone_index(bs, offset);
     unsigned int n = 0, i = 0;
     int ret;
     rep_size = sizeof(struct blk_zone_report) + nrz * sizeof(struct blk_zone);
@@ -2560,7 +2560,7 @@ raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr, uint64_t bytes,
         bs->bl.zoned != BLK_Z_NONE) {
         qemu_co_mutex_lock(&bs->wps->colock);
         if (type & QEMU_AIO_ZONE_APPEND) {
-            int index = offset / bs->bl.zone_size;
+            int index = bdrv_zone_index(bs, offset);
             offset = bs->wps->wp[index];
         }
     }
@@ -2615,7 +2615,7 @@ out:
         bs->bl.zoned != BLK_Z_NONE) {
         BlockZoneWps *wps = bs->wps;
         if (ret == 0) {
-            uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
+            uint64_t *wp = &wps->wp[bdrv_zone_index(bs, offset)];
             if (!BDRV_ZT_IS_CONV(*wp)) {
                 if (type & QEMU_AIO_ZONE_APPEND) {
                     *offset_ptr = *wp;
@@ -3513,7 +3513,7 @@ static int coroutine_fn raw_co_zone_mgmt(BlockDriverState *bs, BlockZoneOp op,
         return -EINVAL;
     }
 
-    uint32_t i = offset / bs->bl.zone_size;
+    uint32_t i = bdrv_zone_index(bs, offset);
     uint32_t nrz = len / bs->bl.zone_size;
     uint64_t *wp = &wps->wp[i];
     if (BDRV_ZT_IS_CONV(*wp) && len != capacity) {
diff --git a/block/io.c b/block/io.c
index a916b236c3..cef23ee5bc 100644
--- a/block/io.c
+++ b/block/io.c
@@ -227,6 +227,11 @@ void bdrv_refresh_limits(BlockDriverState *bs, Transaction *tran, Error **errp)
         }
     }
 
+    if (bs->bl.zone_size) {
+        assert(is_power_of_2(bs->bl.zone_size));
+        bs->bl.zone_size_bits = ctz64(bs->bl.zone_size);
+    }
+
     if (bs->bl.request_alignment > BDRV_MAX_ALIGNMENT) {
         error_setg(errp, "Driver requires too large request alignment");
     }
@@ -3361,6 +3366,13 @@ out:
     return co.ret;
 }
 
+uint32_t bdrv_zone_index(BlockDriverState *bs, uint64_t offset)
+{
+    IO_CODE();
+
+    return offset >> bs->bl.zone_size_bits;
+}
+
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
 {
     IO_CODE();
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index ffcd327803..8c5b87793e 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -527,7 +527,7 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
             }
         }
 
-        index = offset / bs->bl.zone_size;
+        index = bdrv_zone_index(bs, offset);
         if (BDRV_ZT_IS_CONV(bs->wps->wp[index])) {
             *status = VIRTIO_BLK_S_ZONE_INVALID_CMD;
             return false;
diff --git a/include/block/block-io.h b/include/block/block-io.h
index d34d846bb2..9d1c0eb7fb 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -126,6 +126,8 @@ int coroutine_fn GRAPH_RDLOCK bdrv_co_zone_append(BlockDriverState *bs,
                                                   int64_t *offset,
                                                   QEMUIOVector *qiov,
                                                   BdrvRequestFlags flags);
+/* The index of the zone that @offset falls in. */
+uint32_t bdrv_zone_index(BlockDriverState *bs, uint64_t offset);
 
 bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
 
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 7571ed9968..82aa0c1a6e 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -903,6 +903,13 @@ typedef struct BlockLimits {
     /* zone size expressed in bytes */
     uint64_t zone_size;
 
+    /*
+     * log2 of zone_size, derived by bdrv_refresh_limits(). A zoned device
+     * always has a zone size that is a power of two, so the zone an offset
+     * falls in is a shift rather than a division. See bdrv_zone_index().
+     */
+    uint32_t zone_size_bits;
+
     /* total number of zones */
     uint32_t nr_zones;
 
-- 
2.55.0

[PATCH v4 04/12] virtio-blk: report the effective zone write granularity

[Niklas Cassel]

For ZBC/ZAC devices the write granularity is the physical block size, so
a 512e SMR disk exposed through a host_device backend has a logical block
size of 512 and a zone write granularity of 4096. We told the guest driver 512
while raw_co_zone_append() rejects anything that is not 4096 byte
aligned, so the driver saw a plain I/O error for a request it had been
told was valid.

Report the larger of the backend granularity and the logical block size
instead. The guest driver cannot issue writes finer than the logical
size, so the larger of the two is the constraint that applies. This
matches what a Linux guest derives for itself: blk_validate_zoned_limits()
raises zone_write_granularity to the logical block size, and
blk_stack_limits() stacks it with max().

Add it as a helper next to blkconf_blocksizes(), since it is derived from
a BlockConf and the limits of the backend below it, and use it for the
zone append offset check in check_zoned_request(), which validated
against bs->bl.write_granularity. The value reported to the driver and
the value that requests are validated against then cannot drift apart.
The helper cannot return zero because blkconf_blocksizes() always leaves
a logical block size behind, so the check no longer needs to guard
against an unset granularity.

Fixes: 4f7366506a96 ("virtio-blk: add zoned storage emulation for zoned devices")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 hw/block/block.c         |  7 +++++++
 hw/block/virtio-blk.c    | 13 +++++++------
 include/hw/block/block.h |  8 ++++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/block/block.c b/hw/block/block.c
index f187fa025d..1c3135843d 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -201,6 +201,13 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp)
     return true;
 }
 
+uint32_t blkconf_zone_write_granularity(BlockConf *conf)
+{
+    BlockDriverState *bs = blk_bs(conf->blk);
+
+    return MAX(bs->bl.write_granularity, conf->logical_block_size);
+}
+
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
                                    bool resizable, Error **errp)
 {
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8c5b87793e..f6781ff5f6 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -520,11 +520,11 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
     }
 
     if (append) {
-        if (bs->bl.write_granularity) {
-            if ((offset % bs->bl.write_granularity) != 0) {
-                *status = VIRTIO_BLK_S_ZONE_UNALIGNED_WP;
-                return false;
-            }
+        uint32_t wg_mask = blkconf_zone_write_granularity(&s->conf.conf) - 1;
+
+        if (offset & wg_mask) {
+            *status = VIRTIO_BLK_S_ZONE_UNALIGNED_WP;
+            return false;
         }
 
         index = bdrv_zone_index(bs, offset);
@@ -1274,7 +1274,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
                      bs->bl.max_active_zones);
         virtio_stl_p(vdev, &blkcfg.zoned.max_open_zones,
                      bs->bl.max_open_zones);
-        virtio_stl_p(vdev, &blkcfg.zoned.write_granularity, blk_size);
+        virtio_stl_p(vdev, &blkcfg.zoned.write_granularity,
+                     blkconf_zone_write_granularity(conf));
         virtio_stl_p(vdev, &blkcfg.zoned.max_append_sectors,
                      bs->bl.max_append_sectors);
     } else {
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index df941df19f..f98525c01a 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -114,6 +114,14 @@ bool blkconf_geometry(BlockConf *conf, int *trans,
                       unsigned cyls_max, unsigned heads_max, unsigned secs_max,
                       Error **errp);
 bool blkconf_blocksizes(BlockConf *conf, Error **errp);
+/*
+ * The alignment constraint that applies to writes to a sequential zone. The
+ * medium may require a coarser granularity than the logical block size, while
+ * a guest cannot issue writes finer than the logical block size, so the
+ * constraint that applies is the larger of the two. A frontend must report this
+ * value to its guest and validate requests against it.
+ */
+uint32_t blkconf_zone_write_granularity(BlockConf *conf);
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
                                    bool resizable, Error **errp);
 
-- 
2.55.0

[PATCH v4 05/12] virtio-blk: check the write granularity of writes to sequential zones

[Niklas Cassel]

All VIRTIO_BLK_T_OUT requests issued to sequential zones and all
VIRTIO_BLK_T_ZONE_APPEND requests must have an offset and a data size
that are multiples of the write granularity reported by the device
(virtio 1.4, 5.2.6.1), and a violation is reported as
VIRTIO_BLK_S_ZONE_UNALIGNED_WP (virtio 1.4, 5.2.6).

Neither request type was fully checked. Zone appends validated only the
offset, while writes were not checked at all.

Check the size of the appended data, and both the offset and the size of
a write, against blkconf_zone_write_granularity(), so that every request the
device accepts is one that the guest driver was told is valid. Writes to
conventional zones keep no alignment constraint beyond the logical block
size. The write path performs the check after virtio_blk_sect_range_ok()
so that the zone index derived from the guest supplied sector is known to
be in range.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 hw/block/virtio-blk.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index f6781ff5f6..7d5a02a42c 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -398,6 +398,33 @@ static bool virtio_blk_sect_range_ok(VirtIOBlock *dev,
     return true;
 }
 
+/*
+ * Both the offset and the size of a write to a sequential zone must be a
+ * multiple of the write granularity that the device reports. Conventional
+ * zones are not constrained. The zone index is derived from a guest supplied
+ * sector, so this must only be called once virtio_blk_sect_range_ok() has
+ * bounded it.
+ */
+static bool virtio_blk_zone_write_granularity_ok(VirtIOBlock *dev,
+                                                 uint64_t sector, size_t size)
+{
+    BlockDriverState *bs = blk_bs(dev->blk);
+    uint64_t offset = sector << BDRV_SECTOR_BITS;
+    uint32_t wg_mask;
+
+    if (bs->bl.zoned == BLK_Z_NONE) {
+        return true;
+    }
+
+    if (BDRV_ZT_IS_CONV(bs->wps->wp[bdrv_zone_index(bs, offset)])) {
+        return true;
+    }
+
+    wg_mask = blkconf_zone_write_granularity(&dev->conf.conf) - 1;
+
+    return !(offset & wg_mask) && !(size & wg_mask);
+}
+
 static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req,
     struct virtio_blk_discard_write_zeroes *dwz_hdr, bool is_write_zeroes)
 {
@@ -522,7 +549,7 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
     if (append) {
         uint32_t wg_mask = blkconf_zone_write_granularity(&s->conf.conf) - 1;
 
-        if (offset & wg_mask) {
+        if (offset & wg_mask || len & wg_mask) {
             *status = VIRTIO_BLK_S_ZONE_UNALIGNED_WP;
             return false;
         }
@@ -911,6 +938,15 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
             return 0;
         }
 
+        if (is_write &&
+            !virtio_blk_zone_write_granularity_ok(s, req->sector_num,
+                                                  req->qiov.size)) {
+            virtio_blk_req_complete(req, VIRTIO_BLK_S_ZONE_UNALIGNED_WP);
+            block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);
+            g_free(req);
+            return 0;
+        }
+
         block_acct_start(blk_get_stats(s->blk), &req->acct, req->qiov.size,
                          is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ);
 
-- 
2.55.0

[PATCH v4 06/12] hw/block: reject a zoned device whose write pointers are unaddressable

[Niklas Cassel]

The write pointers of a zoned device outlive any particular use of it,
whether the device keeps them itself or a backend records them, while the
logical block size is a property of the frontend and is chosen afresh
every time the device is attached. Nothing ties the two together. A zone
written while the device was configured with logical_block_size=512
leaves a write pointer that is a multiple of 512, and attaching the same
device with logical_block_size=4096 makes that pointer unaddressable.

Such a pointer is not merely misaligned. The guest addresses the device
in logical blocks, and a zone report expresses the write pointer in 512
byte sectors, so the guest is told about a position that does not fall on
a logical block boundary. It can neither read nor write there, and the
zone can only be recovered by resetting it. The reverse direction is
harmless: a pointer laid down with a larger logical block size is still a
multiple of a smaller one.

On a zoned null_blk device with a logical block size of 512, a 512 byte
append to a sequential zone leaves the write pointer half a logical block
into it:

  $ qemu-io --image-opts -n driver=host_device,filename=/dev/nullb0 \
        -c "zap -p 0x20000000 0x200" -c "zrp 0x20000000 1"
  start: 0x100000, len 0x80000, cap 0x80000, wptr 0x100001, zcond:2

Attaching that disk with logical_block_size=4096 handed the guest a zone
it could not write to.

Check at realize time that the zone size and every write pointer of a
sequential zone are multiples of the write granularity that the device is
about to report, and refuse to start otherwise. The write pointers are
already held in memory by the driver, so this costs no I/O.

The check uses blkconf_zone_write_granularity(), the same value that a
frontend reports to its guest and validates requests against, so the
three cannot disagree.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 hw/block/block.c         | 46 ++++++++++++++++++++++++++++++++++++++++
 hw/block/virtio-blk.c    |  4 ++++
 include/hw/block/block.h |  1 +
 3 files changed, 51 insertions(+)

diff --git a/hw/block/block.c b/hw/block/block.c
index 1c3135843d..7663e385ca 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -208,6 +208,52 @@ uint32_t blkconf_zone_write_granularity(BlockConf *conf)
     return MAX(bs->bl.write_granularity, conf->logical_block_size);
 }
 
+bool blkconf_check_zoned_geometry(BlockConf *conf, Error **errp)
+{
+    BlockDriverState *bs = blk_bs(conf->blk);
+    uint32_t wg;
+
+    if (bs->bl.zoned == BLK_Z_NONE) {
+        return true;
+    }
+
+    wg = blkconf_zone_write_granularity(conf);
+
+    if (!QEMU_IS_ALIGNED(bs->bl.zone_size, wg)) {
+        error_setg(errp, "zone size %" PRIu64 " is not a multiple of the zone "
+                   "write granularity %" PRIu32, bs->bl.zone_size, wg);
+        return false;
+    }
+
+    /*
+     * A write pointer that is not a multiple of the write granularity does not
+     * fall on a logical block boundary, so the guest can neither read nor write
+     * at it and the zone can only be recovered by resetting it. A backend that
+     * records its write pointers, rather than reading them back from a device,
+     * can hand us such a pointer when the zones were written while the device
+     * was configured with a smaller logical block size.
+     */
+    for (uint32_t i = 0; i < bs->bl.nr_zones; i++) {
+        uint64_t wp = bs->wps->wp[i];
+
+        if (BDRV_ZT_IS_CONV(wp)) {
+            continue;
+        }
+
+        if (!QEMU_IS_ALIGNED(wp, wg)) {
+            error_setg(errp, "write pointer 0x%" PRIx64 " of zone %" PRIu32
+                       " is not a multiple of the zone write granularity %"
+                       PRIu32, wp, i, wg);
+            error_append_hint(errp, "The zones were written with a smaller "
+                              "logical_block_size. Reset them, or keep using "
+                              "the smaller size.\n");
+            return false;
+        }
+    }
+
+    return true;
+}
+
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
                                    bool resizable, Error **errp)
 {
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 7d5a02a42c..c2acf59c6d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1834,6 +1834,10 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (!blkconf_check_zoned_geometry(&conf->conf, errp)) {
+        return;
+    }
+
     bs = blk_bs(conf->conf.blk);
     if (bs->bl.zoned != BLK_Z_NONE) {
         virtio_add_feature(&s->host_features, VIRTIO_BLK_F_ZONED);
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index f98525c01a..69b0a085f0 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -122,6 +122,7 @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp);
  * value to its guest and validate requests against it.
  */
 uint32_t blkconf_zone_write_granularity(BlockConf *conf);
+bool blkconf_check_zoned_geometry(BlockConf *conf, Error **errp);
 bool blkconf_apply_backend_options(BlockConf *conf, bool readonly,
                                    bool resizable, Error **errp);
 
-- 
2.55.0

[PATCH v4 07/12] block: reject zone appends that are not a multiple of the sector size

[Niklas Cassel]

Zone write pointers are tracked and reported in units of
BDRV_SECTOR_SIZE, so an append whose data size is not a multiple of it
would leave a write pointer that cannot be represented, neither in a
BlockZoneDescriptor nor in the virtio and NVMe zone reports derived from
one.

Nothing states that invariant. file-posix, the only driver that carries
out an append itself, does enforce it, but only as a side effect of
checking each iovec against BlockLimits.write_granularity, which is never
smaller than the sector size. That conflates two constraints: the sector
granularity holds for every driver, whereas write_granularity describes a
coarser requirement of the medium below a driver, and only a driver that
has such a medium should express it.

Check the invariant once in bdrv_co_zone_append(), so that it no longer
rests on a driver that happens to have a granularity of its own to
report.

This is a lower bound, not the alignment that a guest has to observe. The
block layer cannot know that one, because it also depends on the logical
block size the device is configured with, which is a property of the
frontend. The alignment that applies to a guest is the larger of the two,
and it is the frontend that reports it, that validates requests against
it, and that has to refuse a device whose write pointers do not satisfy
it.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/io.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/io.c b/block/io.c
index cef23ee5bc..64e2f2b046 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3355,6 +3355,16 @@ int coroutine_fn bdrv_co_zone_append(BlockDriverState *bs, int64_t *offset,
         return ret;
     }
 
+    /*
+     * Zone write pointers are kept and reported in units of BDRV_SECTOR_SIZE,
+     * so an append that would leave a write pointer at a finer granularity
+     * cannot be represented. Drivers may impose a coarser granularity of their
+     * own, see BlockLimits.write_granularity.
+     */
+    if (!QEMU_IS_ALIGNED(qiov->size, BDRV_SECTOR_SIZE)) {
+        return -EINVAL;
+    }
+
     bdrv_inc_in_flight(bs);
     if (!drv || !drv->bdrv_co_zone_append || bs->bl.zoned == BLK_Z_NONE) {
         co.ret = -ENOTSUP;
-- 
2.55.0

[PATCH v4 08/12] file-posix: remove the zone append write granularity check

[Niklas Cassel]

The check rejects a zone append whose individual iovec lengths are not
multiples of the zone write granularity. That is stricter than the
constraint it is meant to enforce, which applies to the size of the
request as a whole. A request whose total is properly aligned but which
is split across, say, a 512 byte and a 3584 byte iovec is refused here,
even though the iovec boundaries do not survive into the scatter gather
list that reaches the device.

Nor is the driver the right place to enforce it. The kernel and the
device validate writes to a sequential zone themselves, which is why the
same function already passes the request length down without comparing it
against BlockLimits.max_append_sectors. The sector granularity that holds
for every backend is now checked once in bdrv_co_zone_append(), and a
frontend enforces the granularity it advertises to its guest.

Drop the check. BlockLimits.write_granularity is now set in one place, by
this driver from the zone_write_granularity queue attribute, and read in
one place, by the frontend that reports it.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 00323f7a9f..c1ac49d23d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3593,8 +3593,6 @@ raw_co_zone_append(BlockDriverState *bs,
                    BdrvRequestFlags flags) {
     assert(flags == 0);
     int64_t zone_size_mask = bs->bl.zone_size - 1;
-    int64_t iov_len = 0;
-    int64_t len = 0;
 
     if (*offset & zone_size_mask) {
         error_report("sector offset %" PRId64 " is not aligned to zone size "
@@ -3602,20 +3600,8 @@ raw_co_zone_append(BlockDriverState *bs,
         return -EINVAL;
     }
 
-    int64_t wg = bs->bl.write_granularity;
-    int64_t wg_mask = wg - 1;
-    for (int i = 0; i < qiov->niov; i++) {
-        iov_len = qiov->iov[i].iov_len;
-        if (iov_len & wg_mask) {
-            error_report("len of IOVector[%d] %" PRId64 " is not aligned to "
-                         "block size %" PRId64 "", i, iov_len, wg);
-            return -EINVAL;
-        }
-        len += iov_len;
-    }
-
     trace_zbd_zone_append(bs, *offset >> BDRV_SECTOR_BITS);
-    return raw_co_prw(bs, offset, len, qiov, QEMU_AIO_ZONE_APPEND, 0);
+    return raw_co_prw(bs, offset, qiov->size, qiov, QEMU_AIO_ZONE_APPEND, 0);
 }
 #endif
 
-- 
2.55.0

[PATCH v4 09/12] file-posix: base the zone append limit on the transfer limit

[Niklas Cassel]

The zone_append_max_bytes queue attribute is the largest
REQ_OP_ZONE_APPEND that the device accepts, and Linux has no interface
for issuing one from userspace: include/uapi has no such operation, and
every submitter of REQ_OP_ZONE_APPEND is in the kernel. Userspace writes
to a sequential zone with an ordinary write at the write pointer.

That is what this driver does. raw_co_zone_append() substitutes the write
pointer of the zone for the offset and hands the request to raw_co_prw(),
which reaches handle_aiocb_rw_vector() and issues a plain pwritev(). The
kernel never sees a zone append, so the attribute describes a limit on an
operation that is never issued.

Nor is it a limit that this driver runs into. The kernel splits a write
that exceeds the transfer limit rather than refusing it, so a larger
append succeeds: on a null_blk device whose zone_append_max_bytes is
130560, a 16 MiB append completes and advances the write pointer by
16 MiB. Reporting the attribute only understates what the driver can do,
because Linux derives it as a minimum that already includes max_sectors
and chunk_sectors.

Report max_hw_transfer instead, the limit that governs the write the
driver actually issues. There is no use in telling a guest that it may
append more than the device carries in one command, and a frontend then
does not have to reason about how this driver implements an append in
order to bound the value it advertises.

On a null_blk device with max_hw_sectors_kb of 127 and
zone_append_max_bytes of 130560, virtio-blk reports 255 sectors before
and after.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index c1ac49d23d..0e92ff8414 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1488,10 +1488,15 @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     }
     bs->bl.nr_zones = ret;
 
-    ret = get_sysfs_long_val(st, "zone_append_max_bytes");
-    if (ret > 0) {
-        bs->bl.max_append_sectors = ret >> BDRV_SECTOR_BITS;
-    }
+    /*
+     * raw_co_zone_append() carries out an append as an ordinary write at the
+     * write pointer, so the zone_append_max_bytes attribute, which bounds an
+     * operation that this driver never issues, does not apply. The kernel
+     * splits a write that is larger than the transfer limit rather than
+     * refusing it, but there is no use in telling a guest that it may append
+     * more than the device carries in one command.
+     */
+    bs->bl.max_append_sectors = bs->bl.max_hw_transfer >> BDRV_SECTOR_BITS;
 
     ret = get_sysfs_long_val(st, "zone_write_granularity");
     if (ret >= 0) {
-- 
2.55.0

[PATCH v4 10/12] virtio-blk: derive the maximum zone append size

[Niklas Cassel]

The max_append_sectors field of virtio_blk_zoned_characteristics must be
set by the device to the largest zone append request that can be issued
to it, and a value of zero tells the guest driver that zone append is not
supported at all (virtio 1.4, 5.2.5.2). Linux refuses to attach a zoned
device that reports zero.

We pass BlockLimits.max_append_sectors straight through, which makes that
field mean "zone append unsupported" when it is unset, rather than "this
backend imposes no limit of its own". Only a backend that has a limit of
its own has anything to put there.

Derive the value instead. A backend limit is honoured when there is one,
and otherwise the request is bounded by the zone size, since an append
cannot cross a zone boundary, and by the largest request the block layer
can carry. The result cannot be zero.

A backend that carries out an append itself, rather than passing it to a
device that has a limit of its own, is the one that knows how large a
request its implementation can take, so it reports that in
BlockLimits.max_append_sectors and this does not have to guess at it.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 hw/block/virtio-blk.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c2acf59c6d..131516a167 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -525,6 +525,27 @@ typedef struct ZoneCmdData {
     };
 } ZoneCmdData;
 
+/*
+ * The maximum zone append data size that the device reports to the driver in
+ * virtio_blk_zoned_characteristics, in 512 byte sectors.
+ *
+ * A backend that has no limit of its own leaves BlockLimits.max_append_sectors
+ * at zero, in which case the limit is whatever else bounds the request: an
+ * append cannot cross a zone boundary, and the block layer cannot carry a
+ * larger one. The result is never zero, which the driver would read as zone
+ * append not being supported at all.
+ */
+static uint32_t virtio_blk_max_append_sectors(VirtIOBlock *s)
+{
+    BlockDriverState *bs = blk_bs(s->blk);
+    uint64_t sectors;
+
+    sectors = MIN_NON_ZERO(bs->bl.zone_size >> BDRV_SECTOR_BITS,
+                           bs->bl.max_append_sectors);
+
+    return MIN_NON_ZERO(sectors, BDRV_REQUEST_MAX_SECTORS);
+}
+
 /*
  * check zoned_request: error checking before issuing requests. If all checks
  * passed, return true.
@@ -560,12 +581,8 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
             return false;
         }
 
-        if (len / 512 > bs->bl.max_append_sectors) {
-            if (bs->bl.max_append_sectors == 0) {
-                *status = VIRTIO_BLK_S_UNSUPP;
-            } else {
-                *status = VIRTIO_BLK_S_ZONE_INVALID_CMD;
-            }
+        if ((len >> BDRV_SECTOR_BITS) > virtio_blk_max_append_sectors(s)) {
+            *status = VIRTIO_BLK_S_ZONE_INVALID_CMD;
             return false;
         }
     }
@@ -1313,7 +1330,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
         virtio_stl_p(vdev, &blkcfg.zoned.write_granularity,
                      blkconf_zone_write_granularity(conf));
         virtio_stl_p(vdev, &blkcfg.zoned.max_append_sectors,
-                     bs->bl.max_append_sectors);
+                     virtio_blk_max_append_sectors(s));
     } else {
         blkcfg.zoned.model = VIRTIO_BLK_Z_NONE;
     }
-- 
2.55.0

[PATCH v4 11/12] file-posix: reject a zone append past the device capacity

[Niklas Cassel]

raw_co_zone_append() checks that the offset it is given is aligned to the
zone size, but not that it names a zone of the device. raw_co_prw() then
derives a zone index from it and reads that entry of the write pointer
array, so an offset past the end of the device reads past the end of the
array.

bdrv_co_zone_append() does not catch it either: bdrv_check_qiov_request()
bounds the request against BDRV_MAX_LENGTH, which has nothing to do with
the size of this device. A guest cannot reach it, because
check_zoned_request() in virtio-blk rejects an out of range offset first,
but qemu-io and any other caller of blk_co_zone_append() can:

  $ qemu-io --image-opts -n driver=host_device,filename=/dev/nullb0 \
        -c "zap -p 0x100000000000 0x1000"
  Segmentation fault

On a null_blk device with 1000 zones of 256 MiB, that offset yields zone
index 65536 and reads 512 KiB beyond an 8000 byte allocation.

Reject an offset that lies outside the device. That also bounds the
zone index that raw_co_prw() derives from it, so its write pointer
lookup stays inside the array.

Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 0e92ff8414..0e8ffe91c4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3597,8 +3597,15 @@ raw_co_zone_append(BlockDriverState *bs,
                    QEMUIOVector *qiov,
                    BdrvRequestFlags flags) {
     assert(flags == 0);
+    int64_t capacity = bs->total_sectors << BDRV_SECTOR_BITS;
     int64_t zone_size_mask = bs->bl.zone_size - 1;
 
+    if (*offset >= capacity) {
+        error_report("*offset %" PRId64 " is equal to or greater than the "
+                     "device capacity %" PRId64 "", *offset, capacity);
+        return -ENOSPC;
+    }
+
     if (*offset & zone_size_mask) {
         error_report("sector offset %" PRId64 " is not aligned to zone size "
                      "%" PRId64 "", *offset / 512, bs->bl.zone_size / 512);
-- 
2.55.0

[PATCH v4 12/12] file-posix: reject a zone append to a full or conventional zone

[Niklas Cassel]

raw_co_prw() replaces the offset of a zone append with the write pointer
of the addressed zone, which assumes that the stored value names a
position inside that zone. It does not in two cases.

A full zone has its write pointer recorded at the end of the zone, since
get_zones_wp() stores start + len for BLK_ZONE_COND_FULL. That is the
first sector of the following zone, so the append is submitted there. The
kernel accepts it whenever that zone is empty, because it is a legal
write at its write pointer, and the completion path advances the wrong
zone because it recomputes the zone index from the replaced offset. The
data is written to a zone that was never addressed and success is
returned:

  zone 2 finished, then a 4 KiB append to zone 2:
  After zap done, the append sector is 0x180000     <- zone 3
  zone 2: wptr 0x180000, zcond:14 (full)
  zone 3: wptr 0x180008                             <- advanced

A conventional zone has no write pointer at all, and its array entry
carries only the type marker in the top bit, so the offset becomes
negative and the write fails with EINVAL. That is harmless but it reports
nothing about the actual mistake.

Reject both while the write pointer lock is held, since the state has to
be read and acted on atomically. check_zoned_request() in virtio-blk
refuses an append to a conventional zone, so that case needs a caller
that goes to the driver directly, but nothing there examines whether a
zone is full, so a guest can reach the misdirected write.

Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 block/file-posix.c       | 27 +++++++++++++++++++++++++--
 block/io.c               |  9 +++++++++
 include/block/block-io.h |  6 ++++++
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 0e8ffe91c4..2cd16f58a2 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2565,8 +2565,31 @@ raw_co_prw(BlockDriverState *bs, int64_t *offset_ptr, uint64_t bytes,
         bs->bl.zoned != BLK_Z_NONE) {
         qemu_co_mutex_lock(&bs->wps->colock);
         if (type & QEMU_AIO_ZONE_APPEND) {
-            int index = bdrv_zone_index(bs, offset);
-            offset = bs->wps->wp[index];
+            uint32_t index = bdrv_zone_index(bs, offset);
+            uint64_t wp = bs->wps->wp[index];
+
+            /*
+             * The write pointer of the addressed zone becomes the offset of
+             * the write, so it has to name a position inside that zone. It
+             * does not for a conventional zone, which has no write pointer and
+             * stores a type marker in the top bit instead, and it does not for
+             * a full zone, whose write pointer is reported at the zone end.
+             * Either would send the data to a zone that was never addressed.
+             */
+            if (BDRV_ZT_IS_CONV(wp)) {
+                error_report("zone append at offset 0x%" PRIx64 " addresses a "
+                             "conventional zone", offset);
+                qemu_co_mutex_unlock(&bs->wps->colock);
+                return -EINVAL;
+            }
+            if (bdrv_zone_is_full(bs, index)) {
+                error_report("zone append at offset 0x%" PRIx64 " addresses a "
+                             "full zone", offset);
+                qemu_co_mutex_unlock(&bs->wps->colock);
+                return -ENOSPC;
+            }
+
+            offset = wp;
         }
     }
 #endif
diff --git a/block/io.c b/block/io.c
index 64e2f2b046..b657816688 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3383,6 +3383,15 @@ uint32_t bdrv_zone_index(BlockDriverState *bs, uint64_t offset)
     return offset >> bs->bl.zone_size_bits;
 }
 
+bool bdrv_zone_is_full(BlockDriverState *bs, uint32_t index)
+{
+    uint64_t zone_end = MIN((uint64_t)(index + 1) * bs->bl.zone_size,
+                            (uint64_t)bs->total_sectors << BDRV_SECTOR_BITS);
+    IO_CODE();
+
+    return bs->wps->wp[index] >= zone_end;
+}
+
 void *qemu_blockalign(BlockDriverState *bs, size_t size)
 {
     IO_CODE();
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 9d1c0eb7fb..c21dd6444e 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -128,6 +128,12 @@ int coroutine_fn GRAPH_RDLOCK bdrv_co_zone_append(BlockDriverState *bs,
                                                   BdrvRequestFlags flags);
 /* The index of the zone that @offset falls in. */
 uint32_t bdrv_zone_index(BlockDriverState *bs, uint64_t offset);
+/*
+ * True when the write pointer of a zone has reached the end of the writable
+ * part of that zone, so that nothing more can be written to it until it is
+ * reset. The write pointer lock must be held when called.
+ */
+bool bdrv_zone_is_full(BlockDriverState *bs, uint32_t index);
 
 bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
 
-- 
2.55.0

Re: [PATCH v4 01/12] block: widen BlockLimits.zone_size to uint64_t

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:36PM +0200, Niklas Cassel wrote:
> From: Sam Li <faithilikerun@gmail.com>
> 
> The zone-size field in BlockLimits is currently uint32_t, capping
> expressible zone sizes at 4 GiB. Real zoned-device protocols
> like NVMe ZNS allow larger zones. Widen BlockLimits.zone_size to
> uint64_t to match.
> 
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> Reviewed-by: Niklas Cassel <cassel@kernel.org>
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c               | 2 +-
>  include/block/block_int-common.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

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

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

Re: [PATCH v4 03/12] block: add a helper for the index of the zone an offset falls in

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:38PM +0200, Niklas Cassel wrote:
> Deriving the zone that an offset belongs to is open coded in five places,
> in two files, as a division of the offset by BlockLimits.zone_size.
> 
> The zone size of a zoned device is always a power of two. Linux requires
> that of every zoned device it accepts, in blk_revalidate_disk_zones() and
> in each of the drivers that report one, and qcow2 rejects an image whose
> header says otherwise. So the division is a shift, and the shift is worth
> deriving once rather than at every use: record it in BlockLimits as
> zone_size_bits, next to the size it comes from, and assert the property
> that it relies on where it is computed.
> 
> Add bdrv_zone_index() and use it. No functional change; the count of zones
> that a management operation spans stays a division, since it is a length
> rather than an offset.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c               |  8 ++++----
>  block/io.c                       | 12 ++++++++++++
>  hw/block/virtio-blk.c            |  2 +-
>  include/block/block-io.h         |  2 ++
>  include/block/block_int-common.h |  7 +++++++
>  5 files changed, 26 insertions(+), 5 deletions(-)

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

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

Re: [PATCH v4 04/12] virtio-blk: report the effective zone write granularity

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:39PM +0200, Niklas Cassel wrote:
> @@ -1274,7 +1274,8 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>                       bs->bl.max_active_zones);
>          virtio_stl_p(vdev, &blkcfg.zoned.max_open_zones,
>                       bs->bl.max_open_zones);
> -        virtio_stl_p(vdev, &blkcfg.zoned.write_granularity, blk_size);
> +        virtio_stl_p(vdev, &blkcfg.zoned.write_granularity,
> +                     blkconf_zone_write_granularity(conf));
>          virtio_stl_p(vdev, &blkcfg.zoned.max_append_sectors,
>                       bs->bl.max_append_sectors);
>      } else {

The device emulation code cannot expose the limit value from the host
device because that would break live migration: a guest device must
remain unchanged across migration and the VIRTIO Configuration Space
cannot change field values if the source and destination hosts have
different disks. The guest driver does not expect these values to
change once the device has been probed.

For example, it would be possible to migrate from a host with a write
granularity of 64 KB to a host with a write granularity of 4 KB. The
guest must still see a 64 KB write granularity the entire time (even
after migration).

For this reason, limits on guest devices must be configured explicitly
rather than automatically populated from the host device limits.

Could you implement an error when the guest limit has not been
configured appropriately for the given BlockBackend instead?

Stefan

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

Re: [PATCH v4 05/12] virtio-blk: check the write granularity of writes to sequential zones

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:40PM +0200, Niklas Cassel wrote:
> All VIRTIO_BLK_T_OUT requests issued to sequential zones and all
> VIRTIO_BLK_T_ZONE_APPEND requests must have an offset and a data size
> that are multiples of the write granularity reported by the device
> (virtio 1.4, 5.2.6.1), and a violation is reported as
> VIRTIO_BLK_S_ZONE_UNALIGNED_WP (virtio 1.4, 5.2.6).
> 
> Neither request type was fully checked. Zone appends validated only the
> offset, while writes were not checked at all.
> 
> Check the size of the appended data, and both the offset and the size of
> a write, against blkconf_zone_write_granularity(), so that every request the
> device accepts is one that the guest driver was told is valid. Writes to
> conventional zones keep no alignment constraint beyond the logical block
> size. The write path performs the check after virtio_blk_sect_range_ok()
> so that the zone index derived from the guest supplied sector is known to
> be in range.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  hw/block/virtio-blk.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)

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

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

Re: [PATCH v4 06/12] hw/block: reject a zoned device whose write pointers are unaddressable

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:41PM +0200, Niklas Cassel wrote:
> The write pointers of a zoned device outlive any particular use of it,
> whether the device keeps them itself or a backend records them, while the
> logical block size is a property of the frontend and is chosen afresh
> every time the device is attached. Nothing ties the two together. A zone
> written while the device was configured with logical_block_size=512
> leaves a write pointer that is a multiple of 512, and attaching the same
> device with logical_block_size=4096 makes that pointer unaddressable.
> 
> Such a pointer is not merely misaligned. The guest addresses the device
> in logical blocks, and a zone report expresses the write pointer in 512
> byte sectors, so the guest is told about a position that does not fall on
> a logical block boundary. It can neither read nor write there, and the
> zone can only be recovered by resetting it. The reverse direction is
> harmless: a pointer laid down with a larger logical block size is still a
> multiple of a smaller one.
> 
> On a zoned null_blk device with a logical block size of 512, a 512 byte
> append to a sequential zone leaves the write pointer half a logical block
> into it:
> 
>   $ qemu-io --image-opts -n driver=host_device,filename=/dev/nullb0 \
>         -c "zap -p 0x20000000 0x200" -c "zrp 0x20000000 1"
>   start: 0x100000, len 0x80000, cap 0x80000, wptr 0x100001, zcond:2
> 
> Attaching that disk with logical_block_size=4096 handed the guest a zone
> it could not write to.
> 
> Check at realize time that the zone size and every write pointer of a
> sequential zone are multiples of the write granularity that the device is
> about to report, and refuse to start otherwise. The write pointers are
> already held in memory by the driver, so this costs no I/O.
> 
> The check uses blkconf_zone_write_granularity(), the same value that a
> frontend reports to its guest and validates requests against, so the
> three cannot disagree.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  hw/block/block.c         | 46 ++++++++++++++++++++++++++++++++++++++++
>  hw/block/virtio-blk.c    |  4 ++++
>  include/hw/block/block.h |  1 +
>  3 files changed, 51 insertions(+)

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

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

Re: [PATCH v4 07/12] block: reject zone appends that are not a multiple of the sector size

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:42PM +0200, Niklas Cassel wrote:
> Zone write pointers are tracked and reported in units of
> BDRV_SECTOR_SIZE, so an append whose data size is not a multiple of it
> would leave a write pointer that cannot be represented, neither in a
> BlockZoneDescriptor nor in the virtio and NVMe zone reports derived from
> one.
> 
> Nothing states that invariant. file-posix, the only driver that carries
> out an append itself, does enforce it, but only as a side effect of
> checking each iovec against BlockLimits.write_granularity, which is never
> smaller than the sector size. That conflates two constraints: the sector
> granularity holds for every driver, whereas write_granularity describes a
> coarser requirement of the medium below a driver, and only a driver that
> has such a medium should express it.
> 
> Check the invariant once in bdrv_co_zone_append(), so that it no longer
> rests on a driver that happens to have a granularity of its own to
> report.
> 
> This is a lower bound, not the alignment that a guest has to observe. The
> block layer cannot know that one, because it also depends on the logical
> block size the device is configured with, which is a property of the
> frontend. The alignment that applies to a guest is the larger of the two,
> and it is the frontend that reports it, that validates requests against
> it, and that has to refuse a device whose write pointers do not satisfy
> it.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/io.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

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

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

Re: [PATCH v4 08/12] file-posix: remove the zone append write granularity check

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:43PM +0200, Niklas Cassel wrote:
> The check rejects a zone append whose individual iovec lengths are not
> multiples of the zone write granularity. That is stricter than the
> constraint it is meant to enforce, which applies to the size of the
> request as a whole. A request whose total is properly aligned but which
> is split across, say, a 512 byte and a 3584 byte iovec is refused here,
> even though the iovec boundaries do not survive into the scatter gather
> list that reaches the device.
> 
> Nor is the driver the right place to enforce it. The kernel and the
> device validate writes to a sequential zone themselves, which is why the
> same function already passes the request length down without comparing it
> against BlockLimits.max_append_sectors. The sector granularity that holds
> for every backend is now checked once in bdrv_co_zone_append(), and a
> frontend enforces the granularity it advertises to its guest.
> 
> Drop the check. BlockLimits.write_granularity is now set in one place, by
> this driver from the zone_write_granularity queue attribute, and read in
> one place, by the frontend that reports it.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)

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

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

Re: [PATCH v4 07/12] block: reject zone appends that are not a multiple of the sector size

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:42PM +0200, Niklas Cassel wrote:
> diff --git a/block/io.c b/block/io.c
> index cef23ee5bc..64e2f2b046 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3355,6 +3355,16 @@ int coroutine_fn bdrv_co_zone_append(BlockDriverState *bs, int64_t *offset,
>          return ret;
>      }
>  
> +    /*
> +     * Zone write pointers are kept and reported in units of BDRV_SECTOR_SIZE,
> +     * so an append that would leave a write pointer at a finer granularity
> +     * cannot be represented. Drivers may impose a coarser granularity of their
> +     * own, see BlockLimits.write_granularity.
> +     */
> +    if (!QEMU_IS_ALIGNED(qiov->size, BDRV_SECTOR_SIZE)) {
> +        return -EINVAL;
> +    }
> +
>      bdrv_inc_in_flight(bs);
>      if (!drv || !drv->bdrv_co_zone_append || bs->bl.zoned == BLK_Z_NONE) {
>          co.ret = -ENOTSUP;

When looking at bdrv_co_zone_append() and surrounding functions, I
noticed that the CoroutineIOCompletion co local variable is not used
except for the co.ret field. The co local could be replaced with "int
ret" to make the function more concise.

If you feel like it, feel free to add a patch that cleans up these
functions.

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

Re: [PATCH v4 09/12] file-posix: base the zone append limit on the transfer limit

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:44PM +0200, Niklas Cassel wrote:
> The zone_append_max_bytes queue attribute is the largest
> REQ_OP_ZONE_APPEND that the device accepts, and Linux has no interface
> for issuing one from userspace: include/uapi has no such operation, and
> every submitter of REQ_OP_ZONE_APPEND is in the kernel. Userspace writes
> to a sequential zone with an ordinary write at the write pointer.
> 
> That is what this driver does. raw_co_zone_append() substitutes the write
> pointer of the zone for the offset and hands the request to raw_co_prw(),
> which reaches handle_aiocb_rw_vector() and issues a plain pwritev(). The
> kernel never sees a zone append, so the attribute describes a limit on an
> operation that is never issued.
> 
> Nor is it a limit that this driver runs into. The kernel splits a write
> that exceeds the transfer limit rather than refusing it, so a larger
> append succeeds: on a null_blk device whose zone_append_max_bytes is
> 130560, a 16 MiB append completes and advances the write pointer by
> 16 MiB. Reporting the attribute only understates what the driver can do,
> because Linux derives it as a minimum that already includes max_sectors
> and chunk_sectors.
> 
> Report max_hw_transfer instead, the limit that governs the write the
> driver actually issues. There is no use in telling a guest that it may
> append more than the device carries in one command, and a frontend then
> does not have to reason about how this driver implements an append in
> order to bound the value it advertises.
> 
> On a null_blk device with max_hw_sectors_kb of 127 and
> zone_append_max_bytes of 130560, virtio-blk reports 255 sectors before
> and after.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

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

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

Re: [PATCH v4 10/12] virtio-blk: derive the maximum zone append size

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:45PM +0200, Niklas Cassel wrote:
> The max_append_sectors field of virtio_blk_zoned_characteristics must be
> set by the device to the largest zone append request that can be issued
> to it, and a value of zero tells the guest driver that zone append is not
> supported at all (virtio 1.4, 5.2.5.2). Linux refuses to attach a zoned
> device that reports zero.
> 
> We pass BlockLimits.max_append_sectors straight through, which makes that
> field mean "zone append unsupported" when it is unset, rather than "this
> backend imposes no limit of its own". Only a backend that has a limit of
> its own has anything to put there.
> 
> Derive the value instead. A backend limit is honoured when there is one,
> and otherwise the request is bounded by the zone size, since an append
> cannot cross a zone boundary, and by the largest request the block layer
> can carry. The result cannot be zero.
> 
> A backend that carries out an append itself, rather than passing it to a
> device that has a limit of its own, is the one that knows how large a
> request its implementation can take, so it reports that in
> BlockLimits.max_append_sectors and this does not have to guess at it.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  hw/block/virtio-blk.c | 31 ++++++++++++++++++++++++-------
>  1 file changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index c2acf59c6d..131516a167 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -525,6 +525,27 @@ typedef struct ZoneCmdData {
>      };
>  } ZoneCmdData;
>  
> +/*
> + * The maximum zone append data size that the device reports to the driver in
> + * virtio_blk_zoned_characteristics, in 512 byte sectors.
> + *
> + * A backend that has no limit of its own leaves BlockLimits.max_append_sectors
> + * at zero, in which case the limit is whatever else bounds the request: an
> + * append cannot cross a zone boundary, and the block layer cannot carry a
> + * larger one. The result is never zero, which the driver would read as zone
> + * append not being supported at all.
> + */
> +static uint32_t virtio_blk_max_append_sectors(VirtIOBlock *s)
> +{
> +    BlockDriverState *bs = blk_bs(s->blk);
> +    uint64_t sectors;
> +
> +    sectors = MIN_NON_ZERO(bs->bl.zone_size >> BDRV_SECTOR_BITS,
> +                           bs->bl.max_append_sectors);
> +
> +    return MIN_NON_ZERO(sectors, BDRV_REQUEST_MAX_SECTORS);
> +}
> +
>  /*
>   * check zoned_request: error checking before issuing requests. If all checks
>   * passed, return true.
> @@ -560,12 +581,8 @@ static bool check_zoned_request(VirtIOBlock *s, int64_t offset, int64_t len,
>              return false;
>          }
>  
> -        if (len / 512 > bs->bl.max_append_sectors) {
> -            if (bs->bl.max_append_sectors == 0) {
> -                *status = VIRTIO_BLK_S_UNSUPP;
> -            } else {
> -                *status = VIRTIO_BLK_S_ZONE_INVALID_CMD;
> -            }
> +        if ((len >> BDRV_SECTOR_BITS) > virtio_blk_max_append_sectors(s)) {
> +            *status = VIRTIO_BLK_S_ZONE_INVALID_CMD;
>              return false;
>          }
>      }
> @@ -1313,7 +1330,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>          virtio_stl_p(vdev, &blkcfg.zoned.write_granularity,
>                       blkconf_zone_write_granularity(conf));
>          virtio_stl_p(vdev, &blkcfg.zoned.max_append_sectors,
> -                     bs->bl.max_append_sectors);
> +                     virtio_blk_max_append_sectors(s));

This has the same issue as the write_granularity patch earlier in this
patch series, except that max_append_sectors was already exposing the
BlockBackends BlockLimits, which we shouldn't do.

>      } else {
>          blkcfg.zoned.model = VIRTIO_BLK_Z_NONE;
>      }
> -- 
> 2.55.0
> 

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

Re: [PATCH v4 11/12] file-posix: reject a zone append past the device capacity

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:46PM +0200, Niklas Cassel wrote:
> raw_co_zone_append() checks that the offset it is given is aligned to the
> zone size, but not that it names a zone of the device. raw_co_prw() then
> derives a zone index from it and reads that entry of the write pointer
> array, so an offset past the end of the device reads past the end of the
> array.
> 
> bdrv_co_zone_append() does not catch it either: bdrv_check_qiov_request()
> bounds the request against BDRV_MAX_LENGTH, which has nothing to do with
> the size of this device. A guest cannot reach it, because
> check_zoned_request() in virtio-blk rejects an out of range offset first,
> but qemu-io and any other caller of blk_co_zone_append() can:
> 
>   $ qemu-io --image-opts -n driver=host_device,filename=/dev/nullb0 \
>         -c "zap -p 0x100000000000 0x1000"
>   Segmentation fault
> 
> On a null_blk device with 1000 zones of 256 MiB, that offset yields zone
> index 65536 and reads 512 KiB beyond an 8000 byte allocation.
> 
> Reject an offset that lies outside the device. That also bounds the
> zone index that raw_co_prw() derives from it, so its write pointer
> lookup stays inside the array.
> 
> Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices")
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c | 7 +++++++
>  1 file changed, 7 insertions(+)

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

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

Re: [PATCH v4 12/12] file-posix: reject a zone append to a full or conventional zone

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:47PM +0200, Niklas Cassel wrote:
> raw_co_prw() replaces the offset of a zone append with the write pointer
> of the addressed zone, which assumes that the stored value names a
> position inside that zone. It does not in two cases.
> 
> A full zone has its write pointer recorded at the end of the zone, since
> get_zones_wp() stores start + len for BLK_ZONE_COND_FULL. That is the
> first sector of the following zone, so the append is submitted there. The
> kernel accepts it whenever that zone is empty, because it is a legal
> write at its write pointer, and the completion path advances the wrong
> zone because it recomputes the zone index from the replaced offset. The
> data is written to a zone that was never addressed and success is
> returned:
> 
>   zone 2 finished, then a 4 KiB append to zone 2:
>   After zap done, the append sector is 0x180000     <- zone 3
>   zone 2: wptr 0x180000, zcond:14 (full)
>   zone 3: wptr 0x180008                             <- advanced
> 
> A conventional zone has no write pointer at all, and its array entry
> carries only the type marker in the top bit, so the offset becomes
> negative and the write fails with EINVAL. That is harmless but it reports
> nothing about the actual mistake.
> 
> Reject both while the write pointer lock is held, since the state has to
> be read and acted on atomically. check_zoned_request() in virtio-blk
> refuses an append to a conventional zone, so that case needs a caller
> that goes to the driver directly, but nothing there examines whether a
> zone is full, so a guest can reach the misdirected write.
> 
> Fixes: 4751d09adcc3 ("block: introduce zone append write for zoned devices")
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  block/file-posix.c       | 27 +++++++++++++++++++++++++--
>  block/io.c               |  9 +++++++++
>  include/block/block-io.h |  6 ++++++
>  3 files changed, 40 insertions(+), 2 deletions(-)

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

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

Re: [PATCH v4 00/12] block: fix the zone write granularity and the zone append limit

[Stefan Hajnoczi]

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

On Mon, Sep 07, 2026 at 01:07:35PM +0200, Niklas Cassel wrote:
> Hello Stefan, Kevin, and everyone else,
> 
> This series fixes how QEMU reports and enforces the two constraints a zoned
> device places on a write to a sequential zone: the write granularity, and
> the largest zone append it accepts. It also fixes two bugs in the zone
> append emulation in file-posix, one of which a guest can reach.
> 
> Many of these patches are in preparation for Sam Li's zoned qcow2 series.
> The first two patches in the series are taken directly from there, as they
> are unrelated to qcow2.
> 
> Patch 3 is preparation with no functional change: deriving the zone that an
> offset belongs to was open coded in five places, so it becomes a helper.
> 
> Patches 4 to 6 concern the write granularity. virtio-blk reported the
> logical block size while the driver enforced the backend value, so on a
> 512e SMR disk a guest could be told that a request was valid and get an
> I/O error for it. Writes to sequential zones were not checked against the
> granularity at all, and zone appends had only their offset checked, not
> their length. Patch 6 then refuses at realize a device whose write
> pointers the configured logical block size cannot address, which needs no
> emulated backend to provoke: write 512 bytes to a zone of a null_blk
> device and attach it with logical_block_size=4096.
> 
> Patches 7 to 10 concern the append limit. The sector invariant moves to
> bdrv_co_zone_append(), since a write pointer is tracked in sectors and
> cannot represent anything finer, and file-posix drops its own check which
> conflated that with the coarser granularity of the medium. file-posix then
> stops reporting zone_append_max_bytes, which bounds REQ_OP_ZONE_APPEND, an
> operation it never issues: it appends with an ordinary pwritev(), so
> max_hw_transfer is the limit that applies. Finally virtio-blk derives what
> it advertises rather than passing BlockLimits.max_append_sectors through,
> which made an unset field mean "zone append unsupported" rather than "no
> limit of its own", and Linux refuses to attach a zoned device that reports
> zero.
> 
> Patches 11 and 12 fix the write pointer that raw_co_prw() substitutes for
> the offset of an append. An offset that is never bounded against the
> device derives an out of range zone index and reads past the write pointer
> array, which qemu-io can reach. An append to a full zone uses a pointer
> recorded at the end of the zone, so the data is written into the next zone
> and success is returned; a guest can reach that one, because nothing in
> virtio-blk checks whether a zone is full.
> 
> 
> Changes since v3:
> -Picked up tags from Damien.
> -Replaced ctz64() with a zone_size_bits in struct BlockLimits.

Hi Niklas,
Thank you for your patience. QEMU block layer code reviews are slow at
the moment and it took me a while to get to your series.

Overall it looks good. The only concern I have is exposing BlockBackend
BlockLimits fields directly in the emulated device. This breaks live
migration because guest drivers are not prepared for limits to change
suddenly at runtime. More about this in my replies.

Thanks,
Stefan

> 
> 
> Niklas Cassel (10):
>   block: add a helper for the index of the zone an offset falls in
>   virtio-blk: report the effective zone write granularity
>   virtio-blk: check the write granularity of writes to sequential zones
>   hw/block: reject a zoned device whose write pointers are unaddressable
>   block: reject zone appends that are not a multiple of the sector size
>   file-posix: remove the zone append write granularity check
>   file-posix: base the zone append limit on the transfer limit
>   virtio-blk: derive the maximum zone append size
>   file-posix: reject a zone append past the device capacity
>   file-posix: reject a zone append to a full or conventional zone
> 
> Sam Li (2):
>   block: widen BlockLimits.zone_size to uint64_t
>   virtio-blk: do not merge requests across a zone boundary
> 
>  block/block-backend.c             |  11 +++
>  block/file-posix.c                |  71 +++++++++++++-------
>  block/io.c                        |  31 +++++++++
>  hw/block/block.c                  |  53 +++++++++++++++
>  hw/block/virtio-blk.c             | 108 +++++++++++++++++++++++++-----
>  include/block/block-io.h          |   8 +++
>  include/block/block_int-common.h  |   9 ++-
>  include/hw/block/block.h          |   9 +++
>  include/system/block-backend-io.h |   1 +
>  9 files changed, 260 insertions(+), 41 deletions(-)
> 
> -- 
> 2.55.0
> 

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