[PATCH 6/6] block/io: keep zero flag for head/tail parts of misaligned zero write when possible

Fiona Ebner posted 6 patches 1 month ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[PATCH 6/6] block/io: keep zero flag for head/tail parts of misaligned zero write when possible
Posted by Fiona Ebner 1 month ago
This fixes io-test 179 with qcow2. Otherwise, there would be more 'data'
sectors.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Not sure if we should even attempt this without considering
detect-zeroes. While it does keep 179 working, it is a change in
behavior, since it could turn previously data sectors (with zeroes)
into zero sectors when they are part of the head or tail of a zero
write.

Would it even be tolerable from a performance perspective?


Remaining test failures after this patch are:
154 177 204 271

177 and 204 use opt-write-zero=15M with blkdebug, making an
is_power_of_2(align) assertion fail.

Changes in the output of 154 and 271 might be expected, but I didn't
look into it in detail yet, as I wanted to discuss everything first.

 block/io.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/block/io.c b/block/io.c
index d92b30bce5..34b523c951 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2179,10 +2179,14 @@ bdrv_co_do_zero_pwritev(BdrvChild *child, int64_t offset, int64_t bytes,
             int64_t write_bytes = pad.merge_reads ? pad.buf_len : align;
 
             if (total_length >= aligned_offset + write_bytes) {
+                BdrvRequestFlags head_flags =
+                    buffer_is_zero(pad.buf, write_bytes) ?
+                    flags : flags & ~BDRV_REQ_ZERO_WRITE;
+
                 qemu_iovec_init_buf(&local_qiov, pad.buf, write_bytes);
                 ret = bdrv_aligned_pwritev(child, req, aligned_offset,
                                            write_bytes, align, &local_qiov, 0,
-                                           flags & ~BDRV_REQ_ZERO_WRITE);
+                                           head_flags);
             } else {
                 write_bytes = total_length - aligned_offset;
                 qemu_iovec_init_buf(&local_qiov, pad.buf, write_bytes);
@@ -2220,10 +2224,13 @@ bdrv_co_do_zero_pwritev(BdrvChild *child, int64_t offset, int64_t bytes,
         if (total_length >= offset + align) {
             assert(align == pad.tail + bytes);
 
+            BdrvRequestFlags tail_flags =
+                buffer_is_zero(pad.tail_buf + bytes, pad.tail) ?
+                flags : flags & ~BDRV_REQ_ZERO_WRITE;
+
             qemu_iovec_init_buf(&local_qiov, pad.tail_buf, align);
             ret = bdrv_aligned_pwritev(child, req, offset, align, align,
-                                       &local_qiov, 0,
-                                       flags & ~BDRV_REQ_ZERO_WRITE);
+                                       &local_qiov, 0, tail_flags);
         } else {
             int64_t write_bytes = total_length - offset;
             qemu_iovec_init_buf(&local_qiov, pad.tail_buf, write_bytes);
-- 
2.47.3
Re: [PATCH 6/6] block/io: keep zero flag for head/tail parts of misaligned zero write when possible
Posted by Stefan Hajnoczi 1 week ago
On Fri, Jan 09, 2026 at 01:08:33PM +0100, Fiona Ebner wrote:
> This fixes io-test 179 with qcow2. Otherwise, there would be more 'data'
> sectors.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
> 
> Not sure if we should even attempt this without considering
> detect-zeroes. While it does keep 179 working, it is a change in
> behavior, since it could turn previously data sectors (with zeroes)
> into zero sectors when they are part of the head or tail of a zero
> write.
> 
> Would it even be tolerable from a performance perspective?

179 compares qemu-img map output and this makes changes to allocation status
fragile. In a case like this I would either adjust the test case and
regenerate the output, or if the output variance is now unavoidable then
a more sophisticated filter function is needed.

I'd avoid buffer_is_zero() in bdrv_co_do_zero_pwritev() since it eats
CPU cycles.

Stefan