Series comparison

-[PULL 0/5] Block patches
+[PULL 00/14] Block patches
-The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81:
+The following changes since commit f5fe7c17ac4e309e47e78f0f9761aebc8d2f2c81:
-  Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into staging (2021-07-04 14:04:12 +0100)
+  Merge tag 'pull-tcg-20230823-2' of https://gitlab.com/rth7680/qemu into staging (2023-08-28 16:07:04 -0400)
 are available in the Git repository at:
-  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-09-01
-for you to fetch changes up to 9f460c64e13897117f35ffb61f6f5e0102cabc70:
+for you to fetch changes up to 380448464dd89291cf7fd7434be6c225482a334d:
-  block/io: Merge discard request alignments (2021-07-06 14:28:55 +0100)
+  tests/file-io-error: New test (2023-08-29 13:01:24 +0200)
 ----------------------------------------------------------------
-Pull request
+Block patches
 - Fix for file-posix's zoning code crashing on I/O errors
 - Throttling refactoring
 ----------------------------------------------------------------
+Hanna Czenczek (5):
+  file-posix: Clear bs->bl.zoned on error
+  file-posix: Check bs->bl.zoned for zone info
+  file-posix: Fix zone update in I/O error path
+  file-posix: Simplify raw_co_prw's 'out' zone code
+  tests/file-io-error: New test
-Akihiko Odaki (3):
+Zhenwei Pi (9):
-  block/file-posix: Optimize for macOS
+  throttle: introduce enum ThrottleDirection
-  block: Add backend_defaults property
+  test-throttle: use enum ThrottleDirection
-  block/io: Merge discard request alignments
+  throttle: support read-only and write-only
   test-throttle: test read only and write only
   cryptodev: use NULL throttle timer cb for read direction
   throttle: use enum ThrottleDirection instead of bool is_write
   throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code
   fsdev: Use ThrottleDirection instread of bool is_write
   block/throttle-groups: Use ThrottleDirection instread of bool is_write
-Stefan Hajnoczi (2):
+ fsdev/qemu-fsdev-throttle.h                |   4 +-
-  util/async: add a human-readable name to BHs for debugging
+ include/block/throttle-groups.h            |   6 +-
-  util/async: print leaked BH name when AioContext finalizes
+ include/qemu/throttle.h                    |  16 +-
+ backends/cryptodev.c                       |  12 +-
- include/block/aio.h            | 31 ++++++++++++++++++++++---
+ block/block-backend.c                      |   4 +-
- include/hw/block/block.h       |  3 +++
+ block/file-posix.c                         |  42 +++---
- include/qemu/main-loop.h       |  4 +++-
+ block/throttle-groups.c                    | 163 +++++++++++----------
- block/file-posix.c             | 27 ++++++++++++++++++++--
+ block/throttle.c                           |   8 +-
- block/io.c                     |  2 ++
+ fsdev/qemu-fsdev-throttle.c                |  18 ++-
- hw/block/block.c               | 42 ++++++++++++++++++++++++++++++----
+ hw/9pfs/cofile.c                           |   4 +-
- tests/unit/ptimer-test-stubs.c |  2 +-
+ tests/unit/test-throttle.c                 |  76 +++++++++-
- util/async.c                   | 25 ++++++++++++++++----
+ util/throttle.c                            |  84 +++++++----
- util/main-loop.c               |  4 ++--
+ tests/qemu-iotests/tests/file-io-error     | 119 +++++++++++++++
- tests/qemu-iotests/172.out     | 38 ++++++++++++++++++++++++++++++
+ tests/qemu-iotests/tests/file-io-error.out |  33 +++++
-files changed, 161 insertions(+), 17 deletions(-)
+files changed, 418 insertions(+), 171 deletions(-)
  create mode 100755 tests/qemu-iotests/tests/file-io-error
  create mode 100644 tests/qemu-iotests/tests/file-io-error.out
 --
-.31.1
+.41.0

-New patch
+[PULL 01/14] throttle: introduce enum ThrottleDirection
+From: zhenwei pi <pizhenwei@bytedance.com>
+Use enum ThrottleDirection instead of number index.
+Reviewed-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+Message-Id: <20230728022006.1098509-2-pizhenwei@bytedance.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+---
+ include/qemu/throttle.h | 11 ++++++++---
+ util/throttle.c         | 16 +++++++++-------
+files changed, 17 insertions(+), 10 deletions(-)
+diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/qemu/throttle.h
++++ b/include/qemu/throttle.h
+@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleState {
+     int64_t previous_leak;    /* timestamp of the last leak done */
+ } ThrottleState;
++typedef enum {
++    THROTTLE_READ = 0,
++    THROTTLE_WRITE,
++    THROTTLE_MAX
++} ThrottleDirection;
++
+ typedef struct ThrottleTimers {
+-    QEMUTimer *timers[2];     /* timers used to do the throttling */
++    QEMUTimer *timers[THROTTLE_MAX];    /* timers used to do the throttling */
+     QEMUClockType clock_type; /* the clock used */
+     /* Callbacks */
+-    QEMUTimerCB *read_timer_cb;
+-    QEMUTimerCB *write_timer_cb;
++    QEMUTimerCB *timer_cb[THROTTLE_MAX];
+     void *timer_opaque;
+ } ThrottleTimers;
+diff --git a/util/throttle.c b/util/throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/throttle.c
++++ b/util/throttle.c
+@@ -XXX,XX +XXX,XX @@ static bool throttle_compute_timer(ThrottleState *ts,
+ void throttle_timers_attach_aio_context(ThrottleTimers *tt,
+                                         AioContext *new_context)
+ {
+-    tt->timers[0] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+-                                  tt->read_timer_cb, tt->timer_opaque);
+-    tt->timers[1] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+-                                  tt->write_timer_cb, tt->timer_opaque);
++    tt->timers[THROTTLE_READ] =
++        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
++                      tt->timer_cb[THROTTLE_READ], tt->timer_opaque);
++    tt->timers[THROTTLE_WRITE] =
++        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
++                      tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque);
+ }
+ /*
+@@ -XXX,XX +XXX,XX @@ void throttle_timers_init(ThrottleTimers *tt,
+     memset(tt, 0, sizeof(ThrottleTimers));
+     tt->clock_type = clock_type;
+-    tt->read_timer_cb = read_timer_cb;
+-    tt->write_timer_cb = write_timer_cb;
++    tt->timer_cb[THROTTLE_READ] = read_timer_cb;
++    tt->timer_cb[THROTTLE_WRITE] = write_timer_cb;
+     tt->timer_opaque = timer_opaque;
+     throttle_timers_attach_aio_context(tt, aio_context);
+ }
+@@ -XXX,XX +XXX,XX @@ void throttle_timers_detach_aio_context(ThrottleTimers *tt)
+ {
+     int i;
+-    for (i = 0; i < 2; i++) {
++    for (i = 0; i < THROTTLE_MAX; i++) {
+         throttle_timer_destroy(&tt->timers[i]);
+     }
+ }
+--
+.41.0

-New patch
+[PULL 02/14] test-throttle: use enum ThrottleDirection
+From: zhenwei pi <pizhenwei@bytedance.com>
+Use enum ThrottleDirection instead in the throttle test codes.
+Reviewed-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+Message-Id: <20230728022006.1098509-3-pizhenwei@bytedance.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+---
+ tests/unit/test-throttle.c | 6 +++---
+file changed, 3 insertions(+), 3 deletions(-)
+diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/unit/test-throttle.c
++++ b/tests/unit/test-throttle.c
+@@ -XXX,XX +XXX,XX @@ static void test_init(void)
+     /* check initialized fields */
+     g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
+-    g_assert(tt->timers[0]);
+-    g_assert(tt->timers[1]);
++    g_assert(tt->timers[THROTTLE_READ]);
++    g_assert(tt->timers[THROTTLE_WRITE]);
+     /* check other fields where cleared */
+     g_assert(!ts.previous_leak);
+@@ -XXX,XX +XXX,XX @@ static void test_destroy(void)
+     throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
+                          read_timer_cb, write_timer_cb, &ts);
+     throttle_timers_destroy(tt);
+-    for (i = 0; i < 2; i++) {
++    for (i = 0; i < THROTTLE_MAX; i++) {
+         g_assert(!tt->timers[i]);
+     }
+ }
+--
+.41.0

-[PULL 1/5] util/async: add a human-readable name to BHs for debugging
+[PULL 03/14] throttle: support read-only and write-only
-It can be difficult to debug issues with BHs in production environments.
+From: zhenwei pi <pizhenwei@bytedance.com>
 Although BHs can usually be identified by looking up their ->cb()
 function pointer, this requires debug information for the program. It is
 also not possible to print human-readable diagnostics about BHs because
 they have no identifier.
-This patch adds a name to each BH. The name is not unique per instance
+Only one direction is necessary in several scenarios:
-but differentiates between cb() functions, which is usually enough. It's
+- a read-only disk
-done by changing aio_bh_new() and friends to macros that stringify cb.
+- operations on a device are considered as *write* only. For example,
   encrypt/decrypt/sign/verify operations on a cryptodev use a single
   *write* timer(read timer callback is defined, but never invoked).
-The next patch will use the name field when reporting leaked BHs.
+Allow a single direction in throttle, this reduces memory, and uplayer
 does not need a dummy callback any more.
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+Reviewed-by: Alberto Garcia <berto@igalia.com>
-Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
-Message-Id: <20210414200247.917496-2-stefanha@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
 Message-Id: <20230728022006.1098509-4-pizhenwei@bytedance.com>
 Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
 ---
- include/block/aio.h            | 31 ++++++++++++++++++++++++++++---
+ util/throttle.c | 42 ++++++++++++++++++++++++++++--------------
- include/qemu/main-loop.h       |  4 +++-
+file changed, 28 insertions(+), 14 deletions(-)
  tests/unit/ptimer-test-stubs.c |  2 +-
  util/async.c                   |  9 +++++++--
  util/main-loop.c               |  4 ++--
 files changed, 41 insertions(+), 9 deletions(-)
-diff --git a/include/block/aio.h b/include/block/aio.h
+diff --git a/util/throttle.c b/util/throttle.c
 index XXXXXXX..XXXXXXX 100644
---- a/include/block/aio.h
+--- a/util/throttle.c
-+++ b/include/block/aio.h
++++ b/util/throttle.c
-@@ -XXX,XX +XXX,XX @@ void aio_context_acquire(AioContext *ctx);
+@@ -XXX,XX +XXX,XX @@ static bool throttle_compute_timer(ThrottleState *ts,
- /* Relinquish ownership of the AioContext. */
+ void throttle_timers_attach_aio_context(ThrottleTimers *tt,
- void aio_context_release(AioContext *ctx);
+                                         AioContext *new_context)
+ {
-+/**
+-    tt->timers[THROTTLE_READ] =
-+ * aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will
+-        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-+ * run only once and as soon as possible.
+-                      tt->timer_cb[THROTTLE_READ], tt->timer_opaque);
-+ *
+-    tt->timers[THROTTLE_WRITE] =
-+ * @name: A human-readable identifier for debugging purposes.
+-        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-+ */
+-                      tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque);
-+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
++    ThrottleDirection dir;
 +                                  const char *name);
 +
- /**
++    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
-  * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run
++        if (tt->timer_cb[dir]) {
-  * only once and as soon as possible.
++            tt->timers[dir] =
-+ *
++                aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-+ * A convenience wrapper for aio_bh_schedule_oneshot_full() that uses cb as the
++                              tt->timer_cb[dir], tt->timer_opaque);
-+ * name string.
++        }
-  */
++    }
--void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+ }
-+#define aio_bh_schedule_oneshot(ctx, cb, opaque) \
-+    aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)))
+ /*
+@@ -XXX,XX +XXX,XX @@ void throttle_timers_init(ThrottleTimers *tt,
- /**
+                           QEMUTimerCB *write_timer_cb,
-- * aio_bh_new: Allocate a new bottom half structure.
+                           void *timer_opaque)
-+ * aio_bh_new_full: Allocate a new bottom half structure.
+ {
-  *
++    assert(read_timer_cb || write_timer_cb);
-  * Bottom halves are lightweight callbacks whose invocation is guaranteed
+     memset(tt, 0, sizeof(ThrottleTimers));
-  * to be wait-free, thread-safe and signal-safe.  The #QEMUBH structure
-  * is opaque and must be allocated prior to its use.
+     tt->clock_type = clock_type;
-+ *
+@@ -XXX,XX +XXX,XX @@ void throttle_timers_init(ThrottleTimers *tt,
-+ * @name: A human-readable identifier for debugging purposes.
+ /* destroy a timer */
-  */
+ static void throttle_timer_destroy(QEMUTimer **timer)
--QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+ {
-+QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
+-    assert(*timer != NULL);
-+                        const char *name);
++    if (*timer == NULL) {
 +        return;
 +    }
      timer_free(*timer);
      *timer = NULL;
@@ -XXX,XX +XXX,XX @@ static void throttle_timer_destroy(QEMUTimer **timer)
  /* Remove timers from event loop */
  void throttle_timers_detach_aio_context(ThrottleTimers *tt)
  {
 -    int i;
 +    ThrottleDirection dir;
 -    for (i = 0; i < THROTTLE_MAX; i++) {
 -        throttle_timer_destroy(&tt->timers[i]);
 +    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
 +        throttle_timer_destroy(&tt->timers[dir]);
      }
  }
@@ -XXX,XX +XXX,XX @@ void throttle_timers_destroy(ThrottleTimers *tt)
  /* is any throttling timer configured */
  bool throttle_timers_are_initialized(ThrottleTimers *tt)
  {
 -    if (tt->timers[0]) {
 -        return true;
 +    ThrottleDirection dir;
 +
-+/**
++    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
-+ * aio_bh_new: Allocate a new bottom half structure
++        if (tt->timers[dir]) {
-+ *
++            return true;
-+ * A convenience wrapper for aio_bh_new_full() that uses the cb as the name
++        }
-+ * string.
+     }
-+ */
-+#define aio_bh_new(ctx, cb, opaque) \
+     return false;
-+    aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb)))
+@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
+ {
- /**
+     int64_t now = qemu_clock_get_ns(tt->clock_type);
-  * aio_notify: Force processing of pending events.
+     int64_t next_timestamp;
-diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
++    QEMUTimer *timer;
-index XXXXXXX..XXXXXXX 100644
+     bool must_wait;
---- a/include/qemu/main-loop.h
-+++ b/include/qemu/main-loop.h
++    timer = is_write ? tt->timers[THROTTLE_WRITE] : tt->timers[THROTTLE_READ];
-@@ -XXX,XX +XXX,XX @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
++    assert(timer);
++
- void qemu_fd_register(int fd);
+     must_wait = throttle_compute_timer(ts,
+                                        is_write,
--QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
+                                        now,
-+#define qemu_bh_new(cb, opaque) \
+@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
-+    qemu_bh_new_full((cb), (opaque), (stringify(cb)))
+     }
-+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
- void qemu_bh_schedule_idle(QEMUBH *bh);
+     /* request throttled and timer pending -> do nothing */
+-    if (timer_pending(tt->timers[is_write])) {
- enum {
++    if (timer_pending(timer)) {
-diff --git a/tests/unit/ptimer-test-stubs.c b/tests/unit/ptimer-test-stubs.c
+         return true;
-index XXXXXXX..XXXXXXX 100644
+     }
---- a/tests/unit/ptimer-test-stubs.c
-+++ b/tests/unit/ptimer-test-stubs.c
+     /* request throttled and timer not pending -> arm timer */
-@@ -XXX,XX +XXX,XX @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type, int attr_mask)
+-    timer_mod(tt->timers[is_write], next_timestamp);
-     return deadline;
++    timer_mod(timer, next_timestamp);
      return true;
  }
--QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
-+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name)
- {
-     QEMUBH *bh = g_new(QEMUBH, 1);
-diff --git a/util/async.c b/util/async.c
-index XXXXXXX..XXXXXXX 100644
---- a/util/async.c
-+++ b/util/async.c
-@@ -XXX,XX +XXX,XX @@ enum {
- struct QEMUBH {
-     AioContext *ctx;
-+    const char *name;
-     QEMUBHFunc *cb;
-     void *opaque;
-     QSLIST_ENTRY(QEMUBH) next;
-@@ -XXX,XX +XXX,XX @@ static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags)
-     return bh;
- }
--void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
-+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
-+                                  void *opaque, const char *name)
- {
-     QEMUBH *bh;
-     bh = g_new(QEMUBH, 1);
-@@ -XXX,XX +XXX,XX @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
-         .ctx = ctx,
-         .cb = cb,
-         .opaque = opaque,
-+        .name = name,
-     };
-     aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
- }
--QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
-+QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
-+                        const char *name)
- {
-     QEMUBH *bh;
-     bh = g_new(QEMUBH, 1);
-@@ -XXX,XX +XXX,XX @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
-         .ctx = ctx,
-         .cb = cb,
-         .opaque = opaque,
-+        .name = name,
-     };
-     return bh;
- }
-diff --git a/util/main-loop.c b/util/main-loop.c
-index XXXXXXX..XXXXXXX 100644
---- a/util/main-loop.c
-+++ b/util/main-loop.c
-@@ -XXX,XX +XXX,XX @@ void main_loop_wait(int nonblocking)
- /* Functions to operate on the main QEMU AioContext.  */
--QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
-+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name)
- {
--    return aio_bh_new(qemu_aio_context, cb, opaque);
-+    return aio_bh_new_full(qemu_aio_context, cb, opaque, name);
- }
- /*
 --
-.31.1
+.41.0

-New patch
+[PULL 04/14] test-throttle: test read only and write only
+From: zhenwei pi <pizhenwei@bytedance.com>
+Reviewed-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+Message-Id: <20230728022006.1098509-5-pizhenwei@bytedance.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+---
+ tests/unit/test-throttle.c | 66 ++++++++++++++++++++++++++++++++++++++
+file changed, 66 insertions(+)
+diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/unit/test-throttle.c
++++ b/tests/unit/test-throttle.c
+@@ -XXX,XX +XXX,XX @@ static void test_init(void)
+     throttle_timers_destroy(tt);
+ }
++static void test_init_readonly(void)
++{
++    int i;
++
++    tt = &tgm.throttle_timers;
++
++    /* fill the structures with crap */
++    memset(&ts, 1, sizeof(ts));
++    memset(tt, 1, sizeof(*tt));
++
++    /* init structures */
++    throttle_init(&ts);
++    throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
++                         read_timer_cb, NULL, &ts);
++
++    /* check initialized fields */
++    g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
++    g_assert(tt->timers[THROTTLE_READ]);
++    g_assert(!tt->timers[THROTTLE_WRITE]);
++
++    /* check other fields where cleared */
++    g_assert(!ts.previous_leak);
++    g_assert(!ts.cfg.op_size);
++    for (i = 0; i < BUCKETS_COUNT; i++) {
++        g_assert(!ts.cfg.buckets[i].avg);
++        g_assert(!ts.cfg.buckets[i].max);
++        g_assert(!ts.cfg.buckets[i].level);
++    }
++
++    throttle_timers_destroy(tt);
++}
++
++static void test_init_writeonly(void)
++{
++    int i;
++
++    tt = &tgm.throttle_timers;
++
++    /* fill the structures with crap */
++    memset(&ts, 1, sizeof(ts));
++    memset(tt, 1, sizeof(*tt));
++
++    /* init structures */
++    throttle_init(&ts);
++    throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
++                         NULL, write_timer_cb, &ts);
++
++    /* check initialized fields */
++    g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
++    g_assert(!tt->timers[THROTTLE_READ]);
++    g_assert(tt->timers[THROTTLE_WRITE]);
++
++    /* check other fields where cleared */
++    g_assert(!ts.previous_leak);
++    g_assert(!ts.cfg.op_size);
++    for (i = 0; i < BUCKETS_COUNT; i++) {
++        g_assert(!ts.cfg.buckets[i].avg);
++        g_assert(!ts.cfg.buckets[i].max);
++        g_assert(!ts.cfg.buckets[i].level);
++    }
++
++    throttle_timers_destroy(tt);
++}
++
+ static void test_destroy(void)
+ {
+     int i;
+@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
+     g_test_add_func("/throttle/leak_bucket",        test_leak_bucket);
+     g_test_add_func("/throttle/compute_wait",       test_compute_wait);
+     g_test_add_func("/throttle/init",               test_init);
++    g_test_add_func("/throttle/init_readonly",      test_init_readonly);
++    g_test_add_func("/throttle/init_writeonly",     test_init_writeonly);
+     g_test_add_func("/throttle/destroy",            test_destroy);
+     g_test_add_func("/throttle/have_timer",         test_have_timer);
+     g_test_add_func("/throttle/detach_attach",      test_detach_attach);
+--
+.41.0

-New patch
+[PULL 05/14] cryptodev: use NULL throttle timer cb for read direction
+From: zhenwei pi <pizhenwei@bytedance.com>
+Operations on a cryptodev are considered as *write* only, the callback
+of read direction is never invoked. Use NULL instead of an unreachable
+path(cryptodev_backend_throttle_timer_cb on read direction).
+The dummy read timer(never invoked) is already removed here, it means
+that the 'FIXME' tag is no longer needed.
+Reviewed-by: Alberto Garcia <berto@igalia.com>
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+Message-Id: <20230728022006.1098509-6-pizhenwei@bytedance.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+---
+ backends/cryptodev.c | 3 +--
+file changed, 1 insertion(+), 2 deletions(-)
+diff --git a/backends/cryptodev.c b/backends/cryptodev.c
+index XXXXXXX..XXXXXXX 100644
+--- a/backends/cryptodev.c
++++ b/backends/cryptodev.c
+@@ -XXX,XX +XXX,XX @@ static void cryptodev_backend_set_throttle(CryptoDevBackend *backend, int field,
+     if (!enabled) {
+         throttle_init(&backend->ts);
+         throttle_timers_init(&backend->tt, qemu_get_aio_context(),
+-                             QEMU_CLOCK_REALTIME,
+-                             cryptodev_backend_throttle_timer_cb, /* FIXME */
++                             QEMU_CLOCK_REALTIME, NULL,
+                              cryptodev_backend_throttle_timer_cb, backend);
+     }
+--
+.41.0

-New patch
+[PULL 06/14] throttle: use enum ThrottleDirection instead of bool is_write
+From: zhenwei pi <pizhenwei@bytedance.com>
+enum ThrottleDirection is already there, use ThrottleDirection instead
+of 'bool is_write' for throttle API, also modify related codes from
+block, fsdev, cryptodev and tests.
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+Message-Id: <20230728022006.1098509-7-pizhenwei@bytedance.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+---
+ include/qemu/throttle.h     |  5 +++--
+ backends/cryptodev.c        |  9 +++++----
+ block/throttle-groups.c     |  6 ++++--
+ fsdev/qemu-fsdev-throttle.c |  8 +++++---
+ tests/unit/test-throttle.c  |  4 ++--
+ util/throttle.c             | 31 +++++++++++++++++--------------
+files changed, 36 insertions(+), 27 deletions(-)
+diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/qemu/throttle.h
++++ b/include/qemu/throttle.h
+@@ -XXX,XX +XXX,XX @@ void throttle_config_init(ThrottleConfig *cfg);
+ /* usage */
+ bool throttle_schedule_timer(ThrottleState *ts,
+                              ThrottleTimers *tt,
+-                             bool is_write);
++                             ThrottleDirection direction);
+-void throttle_account(ThrottleState *ts, bool is_write, uint64_t size);
++void throttle_account(ThrottleState *ts, ThrottleDirection direction,
++                      uint64_t size);
+ void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg,
+                                Error **errp);
+ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var);
+diff --git a/backends/cryptodev.c b/backends/cryptodev.c
+index XXXXXXX..XXXXXXX 100644
+--- a/backends/cryptodev.c
++++ b/backends/cryptodev.c
+@@ -XXX,XX +XXX,XX @@ static void cryptodev_backend_throttle_timer_cb(void *opaque)
+             continue;
+         }
+-        throttle_account(&backend->ts, true, ret);
++        throttle_account(&backend->ts, THROTTLE_WRITE, ret);
+         cryptodev_backend_operation(backend, op_info);
+         if (throttle_enabled(&backend->tc) &&
+-            throttle_schedule_timer(&backend->ts, &backend->tt, true)) {
++            throttle_schedule_timer(&backend->ts, &backend->tt,
++                                    THROTTLE_WRITE)) {
+             break;
+         }
+     }
+@@ -XXX,XX +XXX,XX @@ int cryptodev_backend_crypto_operation(
+         goto do_account;
+     }
+-    if (throttle_schedule_timer(&backend->ts, &backend->tt, true) ||
++    if (throttle_schedule_timer(&backend->ts, &backend->tt, THROTTLE_WRITE) ||
+         !QTAILQ_EMPTY(&backend->opinfos)) {
+         QTAILQ_INSERT_TAIL(&backend->opinfos, op_info, next);
+         return 0;
+@@ -XXX,XX +XXX,XX @@ do_account:
+         return ret;
+     }
+-    throttle_account(&backend->ts, true, ret);
++    throttle_account(&backend->ts, THROTTLE_WRITE, ret);
+     return cryptodev_backend_operation(backend, op_info);
+ }
+diff --git a/block/throttle-groups.c b/block/throttle-groups.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/throttle-groups.c
++++ b/block/throttle-groups.c
+@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
+     ThrottleState *ts = tgm->throttle_state;
+     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
+     ThrottleTimers *tt = &tgm->throttle_timers;
++    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
+     bool must_wait;
+     if (qatomic_read(&tgm->io_limits_disabled)) {
+@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
+         return true;
+     }
+-    must_wait = throttle_schedule_timer(ts, tt, is_write);
++    must_wait = throttle_schedule_timer(ts, tt, direction);
+     /* If a timer just got armed, set tgm as the current token */
+     if (must_wait) {
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm
+     bool must_wait;
+     ThrottleGroupMember *token;
+     ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
++    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
+     assert(bytes >= 0);
+@@ -XXX,XX +XXX,XX @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm
+     }
+     /* The I/O will be executed, so do the accounting */
+-    throttle_account(tgm->throttle_state, is_write, bytes);
++    throttle_account(tgm->throttle_state, direction, bytes);
+     /* Schedule the next request */
+     schedule_next_request(tgm, is_write);
+diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/fsdev/qemu-fsdev-throttle.c
++++ b/fsdev/qemu-fsdev-throttle.c
+@@ -XXX,XX +XXX,XX @@ void fsdev_throttle_init(FsThrottle *fst)
+ void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
+                                             struct iovec *iov, int iovcnt)
+ {
++    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
++
+     if (throttle_enabled(&fst->cfg)) {
+-        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) ||
++        if (throttle_schedule_timer(&fst->ts, &fst->tt, direction) ||
+             !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
+             qemu_co_queue_wait(&fst->throttled_reqs[is_write], NULL);
+         }
+-        throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
++        throttle_account(&fst->ts, direction, iov_size(iov, iovcnt));
+         if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
+-            !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
++            !throttle_schedule_timer(&fst->ts, &fst->tt, direction)) {
+             qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+         }
+     }
+diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/tests/unit/test-throttle.c
++++ b/tests/unit/test-throttle.c
+@@ -XXX,XX +XXX,XX @@ static bool do_test_accounting(bool is_ops, /* are we testing bps or ops */
+     throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &cfg);
+     /* account a read */
+-    throttle_account(&ts, false, size);
++    throttle_account(&ts, THROTTLE_READ, size);
+     /* account a write */
+-    throttle_account(&ts, true, size);
++    throttle_account(&ts, THROTTLE_WRITE, size);
+     /* check total result */
+     index = to_test[is_ops][0];
+diff --git a/util/throttle.c b/util/throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/throttle.c
++++ b/util/throttle.c
+@@ -XXX,XX +XXX,XX @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
+ /* This function compute the time that must be waited while this IO
+  *
+- * @is_write:   true if the current IO is a write, false if it's a read
++ * @direction:  throttle direction
+  * @ret:        time to wait
+  */
+ static int64_t throttle_compute_wait_for(ThrottleState *ts,
+-                                         bool is_write)
++                                         ThrottleDirection direction)
+ {
+     BucketType to_check[2][4] = { {THROTTLE_BPS_TOTAL,
+                                    THROTTLE_OPS_TOTAL,
+@@ -XXX,XX +XXX,XX @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
+     int i;
+     for (i = 0; i < 4; i++) {
+-        BucketType index = to_check[is_write][i];
++        BucketType index = to_check[direction][i];
+         wait = throttle_compute_wait(&ts->cfg.buckets[index]);
+         if (wait > max_wait) {
+             max_wait = wait;
+@@ -XXX,XX +XXX,XX @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
+ /* compute the timer for this type of operation
+  *
+- * @is_write:   the type of operation
++ * @direction:  throttle direction
+  * @now:        the current clock timestamp
+  * @next_timestamp: the resulting timer
+  * @ret:        true if a timer must be set
+  */
+ static bool throttle_compute_timer(ThrottleState *ts,
+-                                   bool is_write,
++                                   ThrottleDirection direction,
+                                    int64_t now,
+                                    int64_t *next_timestamp)
+ {
+@@ -XXX,XX +XXX,XX @@ static bool throttle_compute_timer(ThrottleState *ts,
+     throttle_do_leak(ts, now);
+     /* compute the wait time if any */
+-    wait = throttle_compute_wait_for(ts, is_write);
++    wait = throttle_compute_wait_for(ts, direction);
+     /* if the code must wait compute when the next timer should fire */
+     if (wait) {
+@@ -XXX,XX +XXX,XX @@ void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg)
+  * NOTE: this function is not unit tested due to it's usage of timer_mod
+  *
+  * @tt:       the timers structure
+- * @is_write: the type of operation (read/write)
++ * @direction: throttle direction
+  * @ret:      true if the timer has been scheduled else false
+  */
+ bool throttle_schedule_timer(ThrottleState *ts,
+                              ThrottleTimers *tt,
+-                             bool is_write)
++                             ThrottleDirection direction)
+ {
+     int64_t now = qemu_clock_get_ns(tt->clock_type);
+     int64_t next_timestamp;
+     QEMUTimer *timer;
+     bool must_wait;
+-    timer = is_write ? tt->timers[THROTTLE_WRITE] : tt->timers[THROTTLE_READ];
++    assert(direction < THROTTLE_MAX);
++    timer = tt->timers[direction];
+     assert(timer);
+     must_wait = throttle_compute_timer(ts,
+-                                       is_write,
++                                       direction,
+                                        now,
+                                        &next_timestamp);
+@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
+ /* do the accounting for this operation
+  *
+- * @is_write: the type of operation (read/write)
++ * @direction: throttle direction
+  * @size:     the size of the operation
+  */
+-void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
++void throttle_account(ThrottleState *ts, ThrottleDirection direction,
++                      uint64_t size)
+ {
+     const BucketType bucket_types_size[2][2] = {
+         { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ },
+@@ -XXX,XX +XXX,XX @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
+     double units = 1.0;
+     unsigned i;
++    assert(direction < THROTTLE_MAX);
+     /* if cfg.op_size is defined and smaller than size we compute unit count */
+     if (ts->cfg.op_size && size > ts->cfg.op_size) {
+         units = (double) size / ts->cfg.op_size;
+@@ -XXX,XX +XXX,XX @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
+     for (i = 0; i < 2; i++) {
+         LeakyBucket *bkt;
+-        bkt = &ts->cfg.buckets[bucket_types_size[is_write][i]];
++        bkt = &ts->cfg.buckets[bucket_types_size[direction][i]];
+         bkt->level += size;
+         if (bkt->burst_length > 1) {
+             bkt->burst_level += size;
+         }
+-        bkt = &ts->cfg.buckets[bucket_types_units[is_write][i]];
++        bkt = &ts->cfg.buckets[bucket_types_units[direction][i]];
+         bkt->level += units;
+         if (bkt->burst_length > 1) {
+             bkt->burst_level += units;
+--
+.41.0

-New patch
+[PULL 07/14] throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code
+From: zhenwei pi <pizhenwei@bytedance.com>
+The first dimension of both to_check and
+bucket_types_size/bucket_types_units is used as throttle direction,
+use THROTTLE_MAX instead of hard coded number. Also use ARRAY_SIZE()
+to avoid hard coded number for the second dimension.
+Hanna noticed that the two array should be static. Yes, turn them
+into static variables.
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
+Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
+Message-Id: <20230728022006.1098509-8-pizhenwei@bytedance.com>
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+---
+ util/throttle.c | 11 ++++++-----
+file changed, 6 insertions(+), 5 deletions(-)
+diff --git a/util/throttle.c b/util/throttle.c
+index XXXXXXX..XXXXXXX 100644
+--- a/util/throttle.c
++++ b/util/throttle.c
+@@ -XXX,XX +XXX,XX @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
+ static int64_t throttle_compute_wait_for(ThrottleState *ts,
+                                          ThrottleDirection direction)
+ {
+-    BucketType to_check[2][4] = { {THROTTLE_BPS_TOTAL,
++    static const BucketType to_check[THROTTLE_MAX][4] = {
++                                  {THROTTLE_BPS_TOTAL,
+                                    THROTTLE_OPS_TOTAL,
+                                    THROTTLE_BPS_READ,
+                                    THROTTLE_OPS_READ},
+@@ -XXX,XX +XXX,XX @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
+     int64_t wait, max_wait = 0;
+     int i;
+-    for (i = 0; i < 4; i++) {
++    for (i = 0; i < ARRAY_SIZE(to_check[THROTTLE_READ]); i++) {
+         BucketType index = to_check[direction][i];
+         wait = throttle_compute_wait(&ts->cfg.buckets[index]);
+         if (wait > max_wait) {
+@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
+ void throttle_account(ThrottleState *ts, ThrottleDirection direction,
+                       uint64_t size)
+ {
+-    const BucketType bucket_types_size[2][2] = {
++    static const BucketType bucket_types_size[THROTTLE_MAX][2] = {
+         { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ },
+         { THROTTLE_BPS_TOTAL, THROTTLE_BPS_WRITE }
+     };
+-    const BucketType bucket_types_units[2][2] = {
++    static const BucketType bucket_types_units[THROTTLE_MAX][2] = {
+         { THROTTLE_OPS_TOTAL, THROTTLE_OPS_READ },
+         { THROTTLE_OPS_TOTAL, THROTTLE_OPS_WRITE }
+     };
+@@ -XXX,XX +XXX,XX @@ void throttle_account(ThrottleState *ts, ThrottleDirection direction,
+         units = (double) size / ts->cfg.op_size;
+     }
+-    for (i = 0; i < 2; i++) {
++    for (i = 0; i < ARRAY_SIZE(bucket_types_size[THROTTLE_READ]); i++) {
+         LeakyBucket *bkt;
+         bkt = &ts->cfg.buckets[bucket_types_size[direction][i]];
+--
+.41.0

-[PULL 4/5] block: Add backend_defaults property
+[PULL 08/14] fsdev: Use ThrottleDirection instread of bool is_write
-From: Akihiko Odaki <akihiko.odaki@gmail.com>
+From: zhenwei pi <pizhenwei@bytedance.com>
-backend_defaults property allow users to control if default block
+'bool is_write' style is obsolete from throttle framework, adapt
-properties should be decided with backend information.
+fsdev to the new style.
-If it is off, any backend information will be discarded, which is
+Cc: Greg Kurz <groug@kaod.org>
-suitable if you plan to perform live migration to a different disk backend.
+Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
 Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
 Message-Id: <20230728022006.1098509-9-pizhenwei@bytedance.com>
 Reviewed-by: Greg Kurz <groug@kaod.org>
 Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
 ---
  fsdev/qemu-fsdev-throttle.h |  4 ++--
  fsdev/qemu-fsdev-throttle.c | 14 +++++++-------
  hw/9pfs/cofile.c            |  4 ++--
 files changed, 11 insertions(+), 11 deletions(-)
-If it is on, a block device may utilize backend information more
+diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
 aggressively.
 By default, it is auto, which uses backend information for block
 sizes and ignores the others, which is consistent with the older
 versions.
 Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
 Message-id: 20210705130458.97642-2-akihiko.odaki@gmail.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
  include/hw/block/block.h   |  3 +++
  hw/block/block.c           | 42 ++++++++++++++++++++++++++++++++++----
  tests/qemu-iotests/172.out | 38 ++++++++++++++++++++++++++++++++++
 files changed, 79 insertions(+), 4 deletions(-)
 diff --git a/include/hw/block/block.h b/include/hw/block/block.h
 index XXXXXXX..XXXXXXX 100644
---- a/include/hw/block/block.h
+--- a/fsdev/qemu-fsdev-throttle.h
-+++ b/include/hw/block/block.h
++++ b/fsdev/qemu-fsdev-throttle.h
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ typedef struct FsThrottle {
+     ThrottleState ts;
- typedef struct BlockConf {
+     ThrottleTimers tt;
-     BlockBackend *blk;
+     ThrottleConfig cfg;
-+    OnOffAuto backend_defaults;
+-    CoQueue      throttled_reqs[2];
-     uint32_t physical_block_size;
++    CoQueue      throttled_reqs[THROTTLE_MAX];
-     uint32_t logical_block_size;
+ } FsThrottle;
-     uint32_t min_io_size;
-@@ -XXX,XX +XXX,XX @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
+ int fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *, Error **);
  void fsdev_throttle_init(FsThrottle *);
 -void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
 +void coroutine_fn fsdev_co_throttle_request(FsThrottle *, ThrottleDirection ,
                                              struct iovec *, int);
  void fsdev_throttle_cleanup(FsThrottle *);
 diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
 index XXXXXXX..XXXXXXX 100644
 --- a/fsdev/qemu-fsdev-throttle.c
 +++ b/fsdev/qemu-fsdev-throttle.c
@@ -XXX,XX +XXX,XX @@ void fsdev_throttle_init(FsThrottle *fst)
      }
  }
- #define DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)                     \
+-void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
-+    DEFINE_PROP_ON_OFF_AUTO("backend_defaults", _state,                 \
++void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst,
-+                            _conf.backend_defaults, ON_OFF_AUTO_AUTO),  \
++                                            ThrottleDirection direction,
-     DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
+                                             struct iovec *iov, int iovcnt)
                            _conf.logical_block_size),                    \
      DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
 diff --git a/hw/block/block.c b/hw/block/block.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/block/block.c
 +++ b/hw/block/block.c
@@ -XXX,XX +XXX,XX @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp)
  {
-     BlockBackend *blk = conf->blk;
+-    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
-     BlockSizes blocksizes;
+-
--    int backend_ret;
++    assert(direction < THROTTLE_MAX);
-+    BlockDriverState *bs;
+     if (throttle_enabled(&fst->cfg)) {
-+    bool use_blocksizes;
+         if (throttle_schedule_timer(&fst->ts, &fst->tt, direction) ||
-+    bool use_bs;
+-            !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
-+
+-            qemu_co_queue_wait(&fst->throttled_reqs[is_write], NULL);
-+    switch (conf->backend_defaults) {
++            !qemu_co_queue_empty(&fst->throttled_reqs[direction])) {
-+    case ON_OFF_AUTO_AUTO:
++            qemu_co_queue_wait(&fst->throttled_reqs[direction], NULL);
-+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
+         }
-+        use_bs = false;
-+        break;
+         throttle_account(&fst->ts, direction, iov_size(iov, iovcnt));
-+
-+    case ON_OFF_AUTO_ON:
+-        if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
-+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
++        if (!qemu_co_queue_empty(&fst->throttled_reqs[direction]) &&
-+        bs = blk_bs(blk);
+             !throttle_schedule_timer(&fst->ts, &fst->tt, direction)) {
-+        use_bs = bs;
+-            qemu_co_queue_next(&fst->throttled_reqs[is_write]);
-+        break;
++            qemu_co_queue_next(&fst->throttled_reqs[direction]);
 +
 +    case ON_OFF_AUTO_OFF:
 +        use_blocksizes = false;
 +        use_bs = false;
 +        break;
 +
 +    default:
 +        abort();
 +    }
 -    backend_ret = blk_probe_blocksizes(blk, &blocksizes);
      /* fill in detected values if they are not defined via qemu command line */
      if (!conf->physical_block_size) {
 -        if (!backend_ret) {
 +        if (use_blocksizes) {
             conf->physical_block_size = blocksizes.phys;
          } else {
              conf->physical_block_size = BDRV_SECTOR_SIZE;
          }
      }
-     if (!conf->logical_block_size) {
+ }
--        if (!backend_ret) {
+diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
-+        if (use_blocksizes) {
+index XXXXXXX..XXXXXXX 100644
-             conf->logical_block_size = blocksizes.log;
+--- a/hw/9pfs/cofile.c
-         } else {
++++ b/hw/9pfs/cofile.c
-             conf->logical_block_size = BDRV_SECTOR_SIZE;
+@@ -XXX,XX +XXX,XX @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
-         }
+     if (v9fs_request_cancelled(pdu)) {
          return -EINTR;
      }
-+    if (use_bs) {
+-    fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
-+        if (!conf->opt_io_size) {
++    fsdev_co_throttle_request(s->ctx.fst, THROTTLE_WRITE, iov, iovcnt);
-+            conf->opt_io_size = bs->bl.opt_transfer;
+     v9fs_co_run_in_worker(
-+        }
+         {
-+        if (conf->discard_granularity == -1) {
+             err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
-+            if (bs->bl.pdiscard_alignment) {
+@@ -XXX,XX +XXX,XX @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
-+                conf->discard_granularity = bs->bl.pdiscard_alignment;
+     if (v9fs_request_cancelled(pdu)) {
-+            } else if (bs->bl.request_alignment != 1) {
+         return -EINTR;
-+                conf->discard_granularity = bs->bl.request_alignment;
+     }
-+            }
+-    fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
-+        }
++    fsdev_co_throttle_request(s->ctx.fst, THROTTLE_READ, iov, iovcnt);
-+    }
+     v9fs_co_run_in_worker(
+         {
-     if (conf->logical_block_size > conf->physical_block_size) {
+             err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
          error_setg(errp,
 diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
 index XXXXXXX..XXXXXXX 100644
 --- a/tests/qemu-iotests/172.out
 +++ b/tests/qemu-iotests/172.out
@@ -XXX,XX +XXX,XX @@ Testing:
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "none1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "floppy1"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
                dev: floppy, id ""
                  unit = 1 (0x1)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "floppy0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = ""
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=120
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = ""
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=144
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = ""
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=288
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = ""
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica
                dev: floppy, id ""
                  unit = 0 (0x0)
                  drive = "none0"
 +                backend_defaults = "auto"
                  logical_block_size = 512 (512 B)
                  physical_block_size = 512 (512 B)
                  min_io_size = 0 (0 B)
 --
-.31.1
+.41.0

-New patch
+[PULL 09/14] block/throttle-groups: Use ThrottleDirection instread of bool is_write
+From: zhenwei pi <pizhenwei@bytedance.com>
 'bool is_write' style is obsolete from throttle framework, adapt
 block throttle groups to the new style:
 - use ThrottleDirection instead of 'bool is_write'. Ex,
   schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
   -> schedule_next_request(ThrottleGroupMember *tgm, ThrottleDirection direction)
 - use THROTTLE_MAX instead of hard code. Ex, ThrottleGroupMember *tokens[2]
   -> ThrottleGroupMember *tokens[THROTTLE_MAX]
 - use ThrottleDirection instead of hard code on iteration. Ex, (i = 0; i < 2; i++)
   -> for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++)
 Use a simple python script to test the new style:
  #!/usr/bin/python3
 import subprocess
 import random
 import time
 commands = ['virsh blkdeviotune jammy vda --write-bytes-sec ', \
             'virsh blkdeviotune jammy vda --write-iops-sec ', \
             'virsh blkdeviotune jammy vda --read-bytes-sec ', \
             'virsh blkdeviotune jammy vda --read-iops-sec ']
 for loop in range(1, 1000):
     time.sleep(random.randrange(3, 5))
     command = commands[random.randrange(0, 3)] + str(random.randrange(0, 1000000))
     subprocess.run(command, shell=True, check=True)
 This works fine.
 Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
 Message-Id: <20230728022006.1098509-10-pizhenwei@bytedance.com>
 Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
 Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
 ---
  include/block/throttle-groups.h |   6 +-
  block/block-backend.c           |   4 +-
  block/throttle-groups.c         | 161 ++++++++++++++++----------------
  block/throttle.c                |   8 +-
 files changed, 90 insertions(+), 89 deletions(-)
 diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/block/throttle-groups.h
 +++ b/include/block/throttle-groups.h
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleGroupMember {
      AioContext   *aio_context;
      /* throttled_reqs_lock protects the CoQueues for throttled requests.  */
      CoMutex      throttled_reqs_lock;
 -    CoQueue      throttled_reqs[2];
 +    CoQueue      throttled_reqs[THROTTLE_MAX];
      /* Nonzero if the I/O limits are currently being ignored; generally
       * it is zero.  Accessed with atomic operations.
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleGroupMember {
       * throttle_state tells us if I/O limits are configured. */
      ThrottleState *throttle_state;
      ThrottleTimers throttle_timers;
 -    unsigned       pending_reqs[2];
 +    unsigned       pending_reqs[THROTTLE_MAX];
      QLIST_ENTRY(ThrottleGroupMember) round_robin;
  } ThrottleGroupMember;
@@ -XXX,XX +XXX,XX @@ void throttle_group_restart_tgm(ThrottleGroupMember *tgm);
  void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
                                                          int64_t bytes,
 -                                                        bool is_write);
 +                                                        ThrottleDirection direction);
  void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
                                         AioContext *new_context);
  void throttle_group_detach_aio_context(ThrottleGroupMember *tgm);
 diff --git a/block/block-backend.c b/block/block-backend.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/block-backend.c
 +++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ blk_co_do_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes,
      /* throttling disk I/O */
      if (blk->public.throttle_group_member.throttle_state) {
          throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member,
 -                bytes, false);
 +                bytes, THROTTLE_READ);
      }
      ret = bdrv_co_preadv_part(blk->root, offset, bytes, qiov, qiov_offset,
@@ -XXX,XX +XXX,XX @@ blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
      /* throttling disk I/O */
      if (blk->public.throttle_group_member.throttle_state) {
          throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member,
 -                bytes, true);
 +                bytes, THROTTLE_WRITE);
      }
      if (!blk->enable_write_cache) {
 diff --git a/block/throttle-groups.c b/block/throttle-groups.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/throttle-groups.c
 +++ b/block/throttle-groups.c
@@ -XXX,XX +XXX,XX @@
  static void throttle_group_obj_init(Object *obj);
  static void throttle_group_obj_complete(UserCreatable *obj, Error **errp);
 -static void timer_cb(ThrottleGroupMember *tgm, bool is_write);
 +static void timer_cb(ThrottleGroupMember *tgm, ThrottleDirection direction);
  /* The ThrottleGroup structure (with its ThrottleState) is shared
   * among different ThrottleGroupMembers and it's independent from
@@ -XXX,XX +XXX,XX @@ struct ThrottleGroup {
      QemuMutex lock; /* This lock protects the following four fields */
      ThrottleState ts;
      QLIST_HEAD(, ThrottleGroupMember) head;
 -    ThrottleGroupMember *tokens[2];
 -    bool any_timer_armed[2];
 +    ThrottleGroupMember *tokens[THROTTLE_MAX];
 +    bool any_timer_armed[THROTTLE_MAX];
      QEMUClockType clock_type;
      /* This field is protected by the global QEMU mutex */
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *throttle_group_next_tgm(ThrottleGroupMember *tgm)
   * This assumes that tg->lock is held.
   *
   * @tgm:        the ThrottleGroupMember
 - * @is_write:   the type of operation (read/write)
 + * @direction:  the ThrottleDirection
   * @ret:        whether the ThrottleGroupMember has pending requests.
   */
  static inline bool tgm_has_pending_reqs(ThrottleGroupMember *tgm,
 -                                        bool is_write)
 +                                        ThrottleDirection direction)
  {
 -    return tgm->pending_reqs[is_write];
 +    return tgm->pending_reqs[direction];
  }
  /* Return the next ThrottleGroupMember in the round-robin sequence with pending
@@ -XXX,XX +XXX,XX @@ static inline bool tgm_has_pending_reqs(ThrottleGroupMember *tgm,
   * This assumes that tg->lock is held.
   *
   * @tgm:       the current ThrottleGroupMember
 - * @is_write:  the type of operation (read/write)
 + * @direction: the ThrottleDirection
   * @ret:       the next ThrottleGroupMember with pending requests, or tgm if
   *             there is none.
   */
  static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
 -                                                bool is_write)
 +                                                ThrottleDirection direction)
  {
      ThrottleState *ts = tgm->throttle_state;
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
       * it's being drained. Skip the round-robin search and return tgm
       * immediately if it has pending requests. Otherwise we could be
       * forcing it to wait for other member's throttled requests. */
 -    if (tgm_has_pending_reqs(tgm, is_write) &&
 +    if (tgm_has_pending_reqs(tgm, direction) &&
          qatomic_read(&tgm->io_limits_disabled)) {
          return tgm;
      }
 -    start = token = tg->tokens[is_write];
 +    start = token = tg->tokens[direction];
      /* get next bs round in round robin style */
      token = throttle_group_next_tgm(token);
 -    while (token != start && !tgm_has_pending_reqs(token, is_write)) {
 +    while (token != start && !tgm_has_pending_reqs(token, direction)) {
          token = throttle_group_next_tgm(token);
      }
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
       * then decide the token is the current tgm because chances are
       * the current tgm got the current request queued.
       */
 -    if (token == start && !tgm_has_pending_reqs(token, is_write)) {
 +    if (token == start && !tgm_has_pending_reqs(token, direction)) {
          token = tgm;
      }
      /* Either we return the original TGM, or one with pending requests */
 -    assert(token == tgm || tgm_has_pending_reqs(token, is_write));
 +    assert(token == tgm || tgm_has_pending_reqs(token, direction));
      return token;
  }
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
   * This assumes that tg->lock is held.
   *
   * @tgm:        the current ThrottleGroupMember
 - * @is_write:   the type of operation (read/write)
 + * @direction:  the ThrottleDirection
   * @ret:        whether the I/O request needs to be throttled or not
   */
  static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
 -                                          bool is_write)
 +                                          ThrottleDirection direction)
  {
      ThrottleState *ts = tgm->throttle_state;
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
      ThrottleTimers *tt = &tgm->throttle_timers;
 -    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
      bool must_wait;
      if (qatomic_read(&tgm->io_limits_disabled)) {
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
      }
      /* Check if any of the timers in this group is already armed */
 -    if (tg->any_timer_armed[is_write]) {
 +    if (tg->any_timer_armed[direction]) {
          return true;
      }
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
      /* If a timer just got armed, set tgm as the current token */
      if (must_wait) {
 -        tg->tokens[is_write] = tgm;
 -        tg->any_timer_armed[is_write] = true;
 +        tg->tokens[direction] = tgm;
 +        tg->any_timer_armed[direction] = true;
      }
      return must_wait;
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
   * any request was actually pending.
   *
   * @tgm:       the current ThrottleGroupMember
 - * @is_write:  the type of operation (read/write)
 + * @direction: the ThrottleDirection
   */
  static bool coroutine_fn throttle_group_co_restart_queue(ThrottleGroupMember *tgm,
 -                                                         bool is_write)
 +                                                         ThrottleDirection direction)
  {
      bool ret;
      qemu_co_mutex_lock(&tgm->throttled_reqs_lock);
 -    ret = qemu_co_queue_next(&tgm->throttled_reqs[is_write]);
 +    ret = qemu_co_queue_next(&tgm->throttled_reqs[direction]);
      qemu_co_mutex_unlock(&tgm->throttled_reqs_lock);
      return ret;
@@ -XXX,XX +XXX,XX @@ static bool coroutine_fn throttle_group_co_restart_queue(ThrottleGroupMember *tg
   * This assumes that tg->lock is held.
   *
   * @tgm:       the current ThrottleGroupMember
 - * @is_write:  the type of operation (read/write)
 + * @direction: the ThrottleDirection
   */
 -static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
 +static void schedule_next_request(ThrottleGroupMember *tgm,
 +                                  ThrottleDirection direction)
  {
      ThrottleState *ts = tgm->throttle_state;
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -XXX,XX +XXX,XX @@ static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
      ThrottleGroupMember *token;
      /* Check if there's any pending request to schedule next */
 -    token = next_throttle_token(tgm, is_write);
 -    if (!tgm_has_pending_reqs(token, is_write)) {
 +    token = next_throttle_token(tgm, direction);
 +    if (!tgm_has_pending_reqs(token, direction)) {
          return;
      }
      /* Set a timer for the request if it needs to be throttled */
 -    must_wait = throttle_group_schedule_timer(token, is_write);
 +    must_wait = throttle_group_schedule_timer(token, direction);
      /* If it doesn't have to wait, queue it for immediate execution */
      if (!must_wait) {
          /* Give preference to requests from the current tgm */
          if (qemu_in_coroutine() &&
 -            throttle_group_co_restart_queue(tgm, is_write)) {
 +            throttle_group_co_restart_queue(tgm, direction)) {
              token = tgm;
          } else {
              ThrottleTimers *tt = &token->throttle_timers;
              int64_t now = qemu_clock_get_ns(tg->clock_type);
 -            timer_mod(tt->timers[is_write], now);
 -            tg->any_timer_armed[is_write] = true;
 +            timer_mod(tt->timers[direction], now);
 +            tg->any_timer_armed[direction] = true;
          }
 -        tg->tokens[is_write] = token;
 +        tg->tokens[direction] = token;
      }
  }
@@ -XXX,XX +XXX,XX @@ static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
   *
   * @tgm:       the current ThrottleGroupMember
   * @bytes:     the number of bytes for this I/O
 - * @is_write:  the type of operation (read/write)
 + * @direction: the ThrottleDirection
   */
  void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
                                                          int64_t bytes,
 -                                                        bool is_write)
 +                                                        ThrottleDirection direction)
  {
      bool must_wait;
      ThrottleGroupMember *token;
      ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
 -    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
      assert(bytes >= 0);
 +    assert(direction < THROTTLE_MAX);
      qemu_mutex_lock(&tg->lock);
      /* First we check if this I/O has to be throttled. */
 -    token = next_throttle_token(tgm, is_write);
 -    must_wait = throttle_group_schedule_timer(token, is_write);
 +    token = next_throttle_token(tgm, direction);
 +    must_wait = throttle_group_schedule_timer(token, direction);
      /* Wait if there's a timer set or queued requests of this type */
 -    if (must_wait || tgm->pending_reqs[is_write]) {
 -        tgm->pending_reqs[is_write]++;
 +    if (must_wait || tgm->pending_reqs[direction]) {
 +        tgm->pending_reqs[direction]++;
          qemu_mutex_unlock(&tg->lock);
          qemu_co_mutex_lock(&tgm->throttled_reqs_lock);
 -        qemu_co_queue_wait(&tgm->throttled_reqs[is_write],
 +        qemu_co_queue_wait(&tgm->throttled_reqs[direction],
                             &tgm->throttled_reqs_lock);
          qemu_co_mutex_unlock(&tgm->throttled_reqs_lock);
          qemu_mutex_lock(&tg->lock);
 -        tgm->pending_reqs[is_write]--;
 +        tgm->pending_reqs[direction]--;
      }
      /* The I/O will be executed, so do the accounting */
      throttle_account(tgm->throttle_state, direction, bytes);
      /* Schedule the next request */
 -    schedule_next_request(tgm, is_write);
 +    schedule_next_request(tgm, direction);
      qemu_mutex_unlock(&tg->lock);
  }
  typedef struct {
      ThrottleGroupMember *tgm;
 -    bool is_write;
 +    ThrottleDirection direction;
  } RestartData;
  static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
      ThrottleGroupMember *tgm = data->tgm;
      ThrottleState *ts = tgm->throttle_state;
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
 -    bool is_write = data->is_write;
 +    ThrottleDirection direction = data->direction;
      bool empty_queue;
 -    empty_queue = !throttle_group_co_restart_queue(tgm, is_write);
 +    empty_queue = !throttle_group_co_restart_queue(tgm, direction);
      /* If the request queue was empty then we have to take care of
       * scheduling the next one */
      if (empty_queue) {
          qemu_mutex_lock(&tg->lock);
 -        schedule_next_request(tgm, is_write);
 +        schedule_next_request(tgm, direction);
          qemu_mutex_unlock(&tg->lock);
      }
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
      aio_wait_kick();
  }
 -static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write)
 +static void throttle_group_restart_queue(ThrottleGroupMember *tgm,
 +                                        ThrottleDirection direction)
  {
      Coroutine *co;
      RestartData *rd = g_new0(RestartData, 1);
      rd->tgm = tgm;
 -    rd->is_write = is_write;
 +    rd->direction = direction;
      /* This function is called when a timer is fired or when
       * throttle_group_restart_tgm() is called. Either way, there can
       * be no timer pending on this tgm at this point */
 -    assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
 +    assert(!timer_pending(tgm->throttle_timers.timers[direction]));
      qatomic_inc(&tgm->restart_pending);
@@ -XXX,XX +XXX,XX @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
  void throttle_group_restart_tgm(ThrottleGroupMember *tgm)
  {
 -    int i;
 +    ThrottleDirection dir;
      if (tgm->throttle_state) {
 -        for (i = 0; i < 2; i++) {
 -            QEMUTimer *t = tgm->throttle_timers.timers[i];
 +        for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
 +            QEMUTimer *t = tgm->throttle_timers.timers[dir];
              if (timer_pending(t)) {
                  /* If there's a pending timer on this tgm, fire it now */
                  timer_del(t);
 -                timer_cb(tgm, i);
 +                timer_cb(tgm, dir);
              } else {
                  /* Else run the next request from the queue manually */
 -                throttle_group_restart_queue(tgm, i);
 +                throttle_group_restart_queue(tgm, dir);
              }
          }
      }
@@ -XXX,XX +XXX,XX @@ void throttle_group_get_config(ThrottleGroupMember *tgm, ThrottleConfig *cfg)
   * because it had been throttled.
   *
   * @tgm:       the ThrottleGroupMember whose request had been throttled
 - * @is_write:  the type of operation (read/write)
 + * @direction: the ThrottleDirection
   */
 -static void timer_cb(ThrottleGroupMember *tgm, bool is_write)
 +static void timer_cb(ThrottleGroupMember *tgm, ThrottleDirection direction)
  {
      ThrottleState *ts = tgm->throttle_state;
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
      /* The timer has just been fired, so we can update the flag */
      qemu_mutex_lock(&tg->lock);
 -    tg->any_timer_armed[is_write] = false;
 +    tg->any_timer_armed[direction] = false;
      qemu_mutex_unlock(&tg->lock);
      /* Run the request that was waiting for this timer */
 -    throttle_group_restart_queue(tgm, is_write);
 +    throttle_group_restart_queue(tgm, direction);
  }
  static void read_timer_cb(void *opaque)
  {
 -    timer_cb(opaque, false);
 +    timer_cb(opaque, THROTTLE_READ);
  }
  static void write_timer_cb(void *opaque)
  {
 -    timer_cb(opaque, true);
 +    timer_cb(opaque, THROTTLE_WRITE);
  }
  /* Register a ThrottleGroupMember from the throttling group, also initializing
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
                                   const char *groupname,
                                   AioContext *ctx)
  {
 -    int i;
 +    ThrottleDirection dir;
      ThrottleState *ts = throttle_group_incref(groupname);
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
      QEMU_LOCK_GUARD(&tg->lock);
      /* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
 -    for (i = 0; i < 2; i++) {
 -        if (!tg->tokens[i]) {
 -            tg->tokens[i] = tgm;
 +    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
 +        if (!tg->tokens[dir]) {
 +            tg->tokens[dir] = tgm;
          }
 +        qemu_co_queue_init(&tgm->throttled_reqs[dir]);
      }
      QLIST_INSERT_HEAD(&tg->head, tgm, round_robin);
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
                           write_timer_cb,
                           tgm);
      qemu_co_mutex_init(&tgm->throttled_reqs_lock);
 -    qemu_co_queue_init(&tgm->throttled_reqs[0]);
 -    qemu_co_queue_init(&tgm->throttled_reqs[1]);
  }
  /* Unregister a ThrottleGroupMember from its group, removing it from the list,
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
      ThrottleState *ts = tgm->throttle_state;
      ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
      ThrottleGroupMember *token;
 -    int i;
 +    ThrottleDirection dir;
      if (!ts) {
          /* Discard already unregistered tgm */
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
      AIO_WAIT_WHILE(tgm->aio_context, qatomic_read(&tgm->restart_pending) > 0);
      WITH_QEMU_LOCK_GUARD(&tg->lock) {
 -        for (i = 0; i < 2; i++) {
 -            assert(tgm->pending_reqs[i] == 0);
 -            assert(qemu_co_queue_empty(&tgm->throttled_reqs[i]));
 -            assert(!timer_pending(tgm->throttle_timers.timers[i]));
 -            if (tg->tokens[i] == tgm) {
 +        for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
 +            assert(tgm->pending_reqs[dir] == 0);
 +            assert(qemu_co_queue_empty(&tgm->throttled_reqs[dir]));
 +            assert(!timer_pending(tgm->throttle_timers.timers[dir]));
 +            if (tg->tokens[dir] == tgm) {
                  token = throttle_group_next_tgm(tgm);
                  /* Take care of the case where this is the last tgm in the group */
                  if (token == tgm) {
                      token = NULL;
                  }
 -                tg->tokens[i] = token;
 +                tg->tokens[dir] = token;
              }
          }
@@ -XXX,XX +XXX,XX @@ void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
  {
      ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
      ThrottleTimers *tt = &tgm->throttle_timers;
 -    int i;
 +    ThrottleDirection dir;
      /* Requests must have been drained */
 -    assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0);
 -    assert(qemu_co_queue_empty(&tgm->throttled_reqs[0]));
 -    assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
 +    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
 +        assert(tgm->pending_reqs[dir] == 0);
 +        assert(qemu_co_queue_empty(&tgm->throttled_reqs[dir]));
 +    }
      /* Kick off next ThrottleGroupMember, if necessary */
      WITH_QEMU_LOCK_GUARD(&tg->lock) {
 -        for (i = 0; i < 2; i++) {
 -            if (timer_pending(tt->timers[i])) {
 -                tg->any_timer_armed[i] = false;
 -                schedule_next_request(tgm, i);
 +        for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
 +            if (timer_pending(tt->timers[dir])) {
 +                tg->any_timer_armed[dir] = false;
 +                schedule_next_request(tgm, dir);
              }
          }
      }
 diff --git a/block/throttle.c b/block/throttle.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/throttle.c
 +++ b/block/throttle.c
@@ -XXX,XX +XXX,XX @@ throttle_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
  {
      ThrottleGroupMember *tgm = bs->opaque;
 -    throttle_group_co_io_limits_intercept(tgm, bytes, false);
 +    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_READ);
      return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
  }
@@ -XXX,XX +XXX,XX @@ throttle_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                      QEMUIOVector *qiov, BdrvRequestFlags flags)
  {
      ThrottleGroupMember *tgm = bs->opaque;
 -    throttle_group_co_io_limits_intercept(tgm, bytes, true);
 +    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
      return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
  }
@@ -XXX,XX +XXX,XX @@ throttle_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
                            BdrvRequestFlags flags)
  {
      ThrottleGroupMember *tgm = bs->opaque;
 -    throttle_group_co_io_limits_intercept(tgm, bytes, true);
 +    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
      return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
  }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn GRAPH_RDLOCK
  throttle_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
  {
      ThrottleGroupMember *tgm = bs->opaque;
 -    throttle_group_co_io_limits_intercept(tgm, bytes, true);
 +    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
      return bdrv_co_pdiscard(bs->file, offset, bytes);
  }
 --
 .41.0

-New patch
+[PULL 10/14] file-posix: Clear bs->bl.zoned on error
+bs->bl.zoned is what indicates whether the zone information is present
+and valid; it is the only thing that raw_refresh_zoned_limits() sets if
+CONFIG_BLKZONED is not defined, and it is also the only thing that it
+sets if CONFIG_BLKZONED is defined, but there are no zones.
+Make sure that it is always set to BLK_Z_NONE if there is an error
+anywhere in raw_refresh_zoned_limits() so that we do not accidentally
+announce zones while our information is incomplete or invalid.
+This also fixes a memory leak in the last error path in
+raw_refresh_zoned_limits().
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Message-Id: <20230824155345.109765-2-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
+---
+ block/file-posix.c | 21 ++++++++++++---------
+file changed, 12 insertions(+), 9 deletions(-)
+diff --git a/block/file-posix.c b/block/file-posix.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/file-posix.c
++++ b/block/file-posix.c
+@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     BlockZoneModel zoned;
+     int ret;
+-    bs->bl.zoned = BLK_Z_NONE;
+-
+     ret = get_sysfs_zoned_model(st, &zoned);
+     if (ret < 0 || zoned == BLK_Z_NONE) {
+-        return;
++        goto no_zoned;
+     }
+     bs->bl.zoned = zoned;
+@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     if (ret < 0) {
+         error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
+                                      "sysfs attribute");
+-        return;
++        goto no_zoned;
+     } else if (!ret) {
+         error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
+-        return;
++        goto no_zoned;
+     }
+     bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
+@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     if (ret < 0) {
+         error_setg_errno(errp, -ret, "Unable to read nr_zones "
+                                      "sysfs attribute");
+-        return;
++        goto no_zoned;
+     } else if (!ret) {
+         error_setg(errp, "Read 0 from nr_zones sysfs attribute");
+-        return;
++        goto no_zoned;
+     }
+     bs->bl.nr_zones = ret;
+@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+     ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
+     if (ret < 0) {
+         error_setg_errno(errp, -ret, "report wps failed");
+-        bs->wps = NULL;
+-        return;
++        goto no_zoned;
+     }
+     qemu_co_mutex_init(&bs->wps->colock);
++    return;
++
++no_zoned:
++    bs->bl.zoned = BLK_Z_NONE;
++    g_free(bs->wps);
++    bs->wps = NULL;
+ }
+ #else /* !defined(CONFIG_BLKZONED) */
+ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
+--
+.41.0

-[PULL 3/5] block/file-posix: Optimize for macOS
+[PULL 11/14] file-posix: Check bs->bl.zoned for zone info
-From: Akihiko Odaki <akihiko.odaki@gmail.com>
+Instead of checking bs->wps or bs->bl.zone_size for whether zone
 information is present, check bs->bl.zoned.  That is the flag that
 raw_refresh_zoned_limits() reliably sets to indicate zone support.  If
 it is set to something other than BLK_Z_NONE, other values and objects
 like bs->wps and bs->bl.zone_size must be non-null/zero and valid; if it
 is not, we cannot rely on their validity.
-This commit introduces "punch hole" operation and optimizes transfer
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
-block size for macOS.
+Message-Id: <20230824155345.109765-3-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
 Thanks to Konstantin Nazarov for detailed analysis of a flaw in an
 old version of this change:
 https://gist.github.com/akihikodaki/87df4149e7ca87f18dc56807ec5a1bc5#gistcomment-3654667
 Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
 Message-id: 20210705130458.97642-1-akihiko.odaki@gmail.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/file-posix.c | 27 +++++++++++++++++++++++++--
+ block/file-posix.c | 12 +++++++-----
-file changed, 25 insertions(+), 2 deletions(-)
+file changed, 7 insertions(+), 5 deletions(-)
 diff --git a/block/file-posix.c b/block/file-posix.c
 index XXXXXXX..XXXXXXX 100644
 --- a/block/file-posix.c
 +++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
- #if defined(HAVE_HOST_BLOCK_DEVICE)
+     if (fd_open(bs) < 0)
- #include <paths.h>
+         return -EIO;
- #include <sys/param.h>
+ #if defined(CONFIG_BLKZONED)
-+#include <sys/mount.h>
+-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && bs->wps) {
- #include <IOKit/IOKitLib.h>
++    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
- #include <IOKit/IOBSD.h>
++        bs->bl.zoned != BLK_Z_NONE) {
- #include <IOKit/storage/IOMediaBSDClient.h>
+         qemu_co_mutex_lock(&bs->wps->colock);
-@@ -XXX,XX +XXX,XX @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
+-        if (type & QEMU_AIO_ZONE_APPEND && bs->bl.zone_size) {
-         return;
++        if (type & QEMU_AIO_ZONE_APPEND) {
              int index = offset / bs->bl.zone_size;
              offset = bs->wps->wp[index];
          }
@@ -XXX,XX +XXX,XX @@ out:
  {
      BlockZoneWps *wps = bs->wps;
      if (ret == 0) {
 -        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
 -            && wps && bs->bl.zone_size) {
 +        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
 +            bs->bl.zoned != BLK_Z_NONE) {
              uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
              if (!BDRV_ZT_IS_CONV(*wp)) {
                  if (type & QEMU_AIO_ZONE_APPEND) {
@@ -XXX,XX +XXX,XX @@ out:
          }
      }
-+#if defined(__APPLE__) && (__MACH__)
+-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) {
-+    struct statfs buf;
++    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
-+
++        bs->blk.zoned != BLK_Z_NONE) {
-+    if (!fstatfs(s->fd, &buf)) {
+         qemu_co_mutex_unlock(&wps->colock);
 +        bs->bl.opt_transfer = buf.f_iosize;
 +        bs->bl.pdiscard_alignment = buf.f_bsize;
 +    }
 +#endif
 +
      if (bs->sg || S_ISBLK(st.st_mode)) {
          int ret = hdev_get_max_hw_transfer(s->fd, &st);
@@ -XXX,XX +XXX,XX @@ out:
      }
  }
-+#if defined(CONFIG_FALLOCATE) || defined(BLKZEROOUT) || defined(BLKDISCARD)
- static int translate_err(int err)
- {
-     if (err == -ENODEV || err == -ENOSYS || err == -EOPNOTSUPP ||
-@@ -XXX,XX +XXX,XX @@ static int translate_err(int err)
-     }
-     return err;
- }
-+#endif
- #ifdef CONFIG_FALLOCATE
- static int do_fallocate(int fd, int mode, off_t offset, off_t len)
-@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_discard(void *opaque)
-             }
-         } while (errno == EINTR);
--        ret = -errno;
-+        ret = translate_err(-errno);
- #endif
-     } else {
- #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
-         ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-                            aiocb->aio_offset, aiocb->aio_nbytes);
-+        ret = translate_err(-errno);
-+#elif defined(__APPLE__) && (__MACH__)
-+        fpunchhole_t fpunchhole;
-+        fpunchhole.fp_flags = 0;
-+        fpunchhole.reserved = 0;
-+        fpunchhole.fp_offset = aiocb->aio_offset;
-+        fpunchhole.fp_length = aiocb->aio_nbytes;
-+        if (fcntl(s->fd, F_PUNCHHOLE, &fpunchhole) == -1) {
-+            ret = errno == ENODEV ? -ENOTSUP : -errno;
-+        } else {
-+            ret = 0;
-+        }
- #endif
-     }
--    ret = translate_err(ret);
-     if (ret == -ENOTSUP) {
-         s->has_discard = false;
-     }
 --
-.31.1
+.41.0

-[PULL 2/5] util/async: print leaked BH name when AioContext finalizes
+[PULL 12/14] file-posix: Fix zone update in I/O error path
-BHs must be deleted before the AioContext is finalized. If not, it's a
+We must check that zone information is present before running
-bug and probably indicates that some part of the program still expects
+update_zones_wp().
 the BH to run in the future. That can lead to memory leaks, inconsistent
 state, or just hangs.
-Unfortunately the assert(flags & BH_DELETED) call in aio_ctx_finalize()
+Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2234374
-is difficult to debug because the assertion failure contains no
+Fixes: Coverity CID 1512459
-information about the BH!
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
 Message-Id: <20230824155345.109765-4-hreitz@redhat.com>
 Reviewed-by: Sam Li <faithilikerun@gmail.com>
 ---
  block/file-posix.c | 3 ++-
 file changed, 2 insertions(+), 1 deletion(-)
-Use the QEMUBH name field added in the previous patch to show a useful
+diff --git a/block/file-posix.c b/block/file-posix.c
 error when a leaked BH is detected.
 Suggested-by: Eric Ernst <eric.g.ernst@gmail.com>
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-Id: <20210414200247.917496-3-stefanha@redhat.com>
 ---
  util/async.c | 16 ++++++++++++++--
 file changed, 14 insertions(+), 2 deletions(-)
 diff --git a/util/async.c b/util/async.c
 index XXXXXXX..XXXXXXX 100644
---- a/util/async.c
+--- a/block/file-posix.c
-+++ b/util/async.c
++++ b/block/file-posix.c
-@@ -XXX,XX +XXX,XX @@ aio_ctx_finalize(GSource     *source)
+@@ -XXX,XX +XXX,XX @@ out:
-     assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list));
+             }
+         }
-     while ((bh = aio_bh_dequeue(&ctx->bh_list, &flags))) {
+     } else {
--        /* qemu_bh_delete() must have been called on BHs in this AioContext */
+-        if (type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) {
--        assert(flags & BH_DELETED);
++        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
-+        /*
++            bs->bl.zoned != BLK_Z_NONE) {
-+         * qemu_bh_delete() must have been called on BHs in this AioContext. In
+             update_zones_wp(bs, s->fd, 0, 1);
-+         * many cases memory leaks, hangs, or inconsistent state occur when a
+         }
 +         * BH is leaked because something still expects it to run.
 +         *
 +         * If you hit this, fix the lifecycle of the BH so that
 +         * qemu_bh_delete() and any associated cleanup is called before the
 +         * AioContext is finalized.
 +         */
 +        if (unlikely(!(flags & BH_DELETED))) {
 +            fprintf(stderr, "%s: BH '%s' leaked, aborting...\n",
 +                    __func__, bh->name);
 +            abort();
 +        }
          g_free(bh);
      }
 --
-.31.1
+.41.0

-New patch
+[PULL 13/14] file-posix: Simplify raw_co_prw's 'out' zone code
+We duplicate the same condition three times here, pull it out to the top
+level.
+Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
+Message-Id: <20230824155345.109765-5-hreitz@redhat.com>
+Reviewed-by: Sam Li <faithilikerun@gmail.com>
+---
+ block/file-posix.c | 18 +++++-------------
+file changed, 5 insertions(+), 13 deletions(-)
+diff --git a/block/file-posix.c b/block/file-posix.c
+index XXXXXXX..XXXXXXX 100644
+--- a/block/file-posix.c
++++ b/block/file-posix.c
+@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
+ out:
+ #if defined(CONFIG_BLKZONED)
+-{
+-    BlockZoneWps *wps = bs->wps;
+-    if (ret == 0) {
+-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+-            bs->bl.zoned != BLK_Z_NONE) {
++    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
++        bs->bl.zoned != BLK_Z_NONE) {
++        BlockZoneWps *wps = bs->wps;
++        if (ret == 0) {
+             uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
+             if (!BDRV_ZT_IS_CONV(*wp)) {
+                 if (type & QEMU_AIO_ZONE_APPEND) {
+@@ -XXX,XX +XXX,XX @@ out:
+                     *wp = offset + bytes;
+                 }
+             }
+-        }
+-    } else {
+-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+-            bs->bl.zoned != BLK_Z_NONE) {
++        } else {
+             update_zones_wp(bs, s->fd, 0, 1);
+         }
+-    }
+-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+-        bs->blk.zoned != BLK_Z_NONE) {
+         qemu_co_mutex_unlock(&wps->colock);
+     }
+-}
+ #endif
+     return ret;
+ }
+--
+.41.0

-[PULL 5/5] block/io: Merge discard request alignments
+[PULL 14/14] tests/file-io-error: New test
-From: Akihiko Odaki <akihiko.odaki@gmail.com>
+This is a regression test for
 https://bugzilla.redhat.com/show_bug.cgi?id=2234374.
-Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
+All this test needs to do is trigger an I/O error inside of file-posix
-Message-id: 20210705130458.97642-3-akihiko.odaki@gmail.com
+(specifically raw_co_prw()).  One reliable way to do this without
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
+requiring special privileges is to use a FUSE export, which allows us to
 inject any error that we want, e.g. via blkdebug.
 Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
 Message-Id: <20230824155345.109765-6-hreitz@redhat.com>
 [hreitz: Fixed test to be skipped when there is no FUSE support, to
          suppress fusermount's allow_other warning, and to be skipped
          with $IMGOPTSSYNTAX enabled]
 Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
 ---
- block/io.c | 2 ++
+ tests/qemu-iotests/tests/file-io-error     | 119 +++++++++++++++++++++
-file changed, 2 insertions(+)
+ tests/qemu-iotests/tests/file-io-error.out |  33 ++++++
 files changed, 152 insertions(+)
  create mode 100755 tests/qemu-iotests/tests/file-io-error
  create mode 100644 tests/qemu-iotests/tests/file-io-error.out
-diff --git a/block/io.c b/block/io.c
+diff --git a/tests/qemu-iotests/tests/file-io-error b/tests/qemu-iotests/tests/file-io-error
-index XXXXXXX..XXXXXXX 100644
+new file mode 100755
---- a/block/io.c
+index XXXXXXX..XXXXXXX
-+++ b/block/io.c
+--- /dev/null
-@@ -XXX,XX +XXX,XX @@ void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll)
++++ b/tests/qemu-iotests/tests/file-io-error
+@@ -XXX,XX +XXX,XX @@
- static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src)
++#!/usr/bin/env bash
- {
++# group: rw
-+    dst->pdiscard_alignment = MAX(dst->pdiscard_alignment,
++#
-+                                  src->pdiscard_alignment);
++# Produce an I/O error in file-posix, and hope that it is not catastrophic.
-     dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer);
++# Regression test for: https://bugzilla.redhat.com/show_bug.cgi?id=2234374
-     dst->max_transfer = MIN_NON_ZERO(dst->max_transfer, src->max_transfer);
++#
-     dst->max_hw_transfer = MIN_NON_ZERO(dst->max_hw_transfer,
++# Copyright (C) 2023 Red Hat, Inc.
 +#
 +# This program is free software; you can redistribute it and/or modify
 +# it under the terms of the GNU General Public License as published by
 +# the Free Software Foundation; either version 2 of the License, or
 +# (at your option) any later version.
 +#
 +# This program is distributed in the hope that it will be useful,
 +# but WITHOUT ANY WARRANTY; without even the implied warranty of
 +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 +# GNU General Public License for more details.
 +#
 +# You should have received a copy of the GNU General Public License
 +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 +#
 +
 +seq=$(basename "$0")
 +echo "QA output created by $seq"
 +
 +status=1    # failure is the default!
 +
 +_cleanup()
 +{
 +    _cleanup_qemu
 +    rm -f "$TEST_DIR/fuse-export"
 +}
 +trap "_cleanup; exit \$status" 0 1 2 3 15
 +
 +# get standard environment, filters and checks
 +. ../common.rc
 +. ../common.filter
 +. ../common.qemu
 +
 +# Format-agnostic (we do not use any), but we do test the file protocol
 +_supported_proto file
 +_require_drivers blkdebug null-co
 +
 +if [ "$IMGOPTSSYNTAX" = "true" ]; then
 +    # We need `$QEMU_IO -f file` to work; IMGOPTSSYNTAX uses --image-opts,
 +    # breaking -f.
 +    _unsupported_fmt $IMGFMT
 +fi
 +
 +# This is a regression test of a bug in which flie-posix would access zone
 +# information in case of an I/O error even when there is no zone information,
 +# resulting in a division by zero.
 +# To reproduce the problem, we need to trigger an I/O error inside of
 +# file-posix, which can be done (rootless) by providing a FUSE export that
 +# presents only errors when accessed.
 +
 +_launch_qemu
 +_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'qmp_capabilities'}" \
 +    'return'
 +
 +_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'blockdev-add',
 +      'arguments': {
 +          'driver': 'blkdebug',
 +          'node-name': 'node0',
 +          'inject-error': [{'event': 'none'}],
 +          'image': {
 +              'driver': 'null-co'
 +          }
 +      }}" \
 +    'return'
 +
 +# FUSE mountpoint must exist and be a regular file
 +touch "$TEST_DIR/fuse-export"
 +
 +# The grep -v to filter fusermount's (benign) error when /etc/fuse.conf does
 +# not contain user_allow_other and the subsequent check for missing FUSE support
 +# have both been taken from iotest 308.
 +output=$(_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'block-export-add',
 +      'arguments': {
 +          'id': 'exp0',
 +          'type': 'fuse',
 +          'node-name': 'node0',
 +          'mountpoint': '$TEST_DIR/fuse-export',
 +          'writable': true
 +      }}" \
 +    'return' \
 +    | grep -v 'option allow_other only allowed if')
 +
 +if echo "$output" | grep -q "Parameter 'type' does not accept value 'fuse'"; then
 +    _notrun 'No FUSE support'
 +fi
 +echo "$output"
 +
 +echo
 +# This should fail, but gracefully, i.e. just print an I/O error, not crash.
 +$QEMU_IO -f file -c 'write 0 64M' "$TEST_DIR/fuse-export" | _filter_qemu_io
 +echo
 +
 +_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'block-export-del',
 +      'arguments': {'id': 'exp0'}}" \
 +    'return'
 +
 +_send_qemu_cmd $QEMU_HANDLE \
 +    '' \
 +    'BLOCK_EXPORT_DELETED'
 +
 +_send_qemu_cmd $QEMU_HANDLE \
 +    "{'execute': 'blockdev-del',
 +      'arguments': {'node-name': 'node0'}}" \
 +    'return'
 +
 +# success, all done
 +echo "*** done"
 +rm -f $seq.full
 +status=0
 diff --git a/tests/qemu-iotests/tests/file-io-error.out b/tests/qemu-iotests/tests/file-io-error.out
 new file mode 100644
 index XXXXXXX..XXXXXXX
 --- /dev/null
 +++ b/tests/qemu-iotests/tests/file-io-error.out
@@ -XXX,XX +XXX,XX @@
 +QA output created by file-io-error
 +{'execute': 'qmp_capabilities'}
 +{"return": {}}
 +{'execute': 'blockdev-add',
 +      'arguments': {
 +          'driver': 'blkdebug',
 +          'node-name': 'node0',
 +          'inject-error': [{'event': 'none'}],
 +          'image': {
 +              'driver': 'null-co'
 +          }
 +      }}
 +{"return": {}}
 +{'execute': 'block-export-add',
 +      'arguments': {
 +          'id': 'exp0',
 +          'type': 'fuse',
 +          'node-name': 'node0',
 +          'mountpoint': 'TEST_DIR/fuse-export',
 +          'writable': true
 +      }}
 +{"return": {}}
 +
 +write failed: Input/output error
 +
 +{'execute': 'block-export-del',
 +      'arguments': {'id': 'exp0'}}
 +{"return": {}}
 +{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "exp0"}}
 +{'execute': 'blockdev-del',
 +      'arguments': {'node-name': 'node0'}}
 +{"return": {}}
 +*** done
 --
-.31.1
+.41.0

The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81:

Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into staging (2021-07-04 14:04:12 +0100)

are available in the Git repository at:

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 9f460c64e13897117f35ffb61f6f5e0102cabc70:

block/io: Merge discard request alignments (2021-07-06 14:28:55 +0100)

----------------------------------------------------------------
Pull request

----------------------------------------------------------------

Akihiko Odaki (3):
  block/file-posix: Optimize for macOS
  block: Add backend_defaults property
  block/io: Merge discard request alignments

Stefan Hajnoczi (2):
  util/async: add a human-readable name to BHs for debugging
  util/async: print leaked BH name when AioContext finalizes

-- 
2.31.1

It can be difficult to debug issues with BHs in production environments.
Although BHs can usually be identified by looking up their ->cb()
function pointer, this requires debug information for the program. It is
also not possible to print human-readable diagnostics about BHs because
they have no identifier.

This patch adds a name to each BH. The name is not unique per instance
but differentiates between cb() functions, which is usually enough. It's
done by changing aio_bh_new() and friends to macros that stringify cb.

The next patch will use the name field when reporting leaked BHs.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210414200247.917496-2-stefanha@redhat.com>
---
 include/block/aio.h            | 31 ++++++++++++++++++++++++++++---
 include/qemu/main-loop.h       |  4 +++-
 tests/unit/ptimer-test-stubs.c |  2 +-
 util/async.c                   |  9 +++++++--
 util/main-loop.c               |  4 ++--
 5 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -XXX,XX +XXX,XX @@ void aio_context_acquire(AioContext *ctx);
 /* Relinquish ownership of the AioContext. */
 void aio_context_release(AioContext *ctx);
 
+/**
+ * aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will
+ * run only once and as soon as possible.
+ *
+ * @name: A human-readable identifier for debugging purposes.
+ */
+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
+                                  const char *name);
+
 /**
  * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run
  * only once and as soon as possible.
+ *
+ * A convenience wrapper for aio_bh_schedule_oneshot_full() that uses cb as the
+ * name string.
  */
-void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+#define aio_bh_schedule_oneshot(ctx, cb, opaque) \
+    aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)))
 
 /**
- * aio_bh_new: Allocate a new bottom half structure.
+ * aio_bh_new_full: Allocate a new bottom half structure.
  *
  * Bottom halves are lightweight callbacks whose invocation is guaranteed
  * to be wait-free, thread-safe and signal-safe.  The #QEMUBH structure
  * is opaque and must be allocated prior to its use.
+ *
+ * @name: A human-readable identifier for debugging purposes.
  */
-QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
+                        const char *name);
+
+/**
+ * aio_bh_new: Allocate a new bottom half structure
+ *
+ * A convenience wrapper for aio_bh_new_full() that uses the cb as the name
+ * string.
+ */
+#define aio_bh_new(ctx, cb, opaque) \
+    aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb)))
 
 /**
  * aio_notify: Force processing of pending events.
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -XXX,XX +XXX,XX @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
 
 void qemu_fd_register(int fd);
 
-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
+#define qemu_bh_new(cb, opaque) \
+    qemu_bh_new_full((cb), (opaque), (stringify(cb)))
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
 void qemu_bh_schedule_idle(QEMUBH *bh);
 
 enum {
diff --git a/tests/unit/ptimer-test-stubs.c b/tests/unit/ptimer-test-stubs.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/ptimer-test-stubs.c
+++ b/tests/unit/ptimer-test-stubs.c
@@ -XXX,XX +XXX,XX @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type, int attr_mask)
     return deadline;
 }
 
-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name)
 {
     QEMUBH *bh = g_new(QEMUBH, 1);
 
diff --git a/util/async.c b/util/async.c
index XXXXXXX..XXXXXXX 100644
--- a/util/async.c
+++ b/util/async.c
@@ -XXX,XX +XXX,XX @@ enum {
 
 struct QEMUBH {
     AioContext *ctx;
+    const char *name;
     QEMUBHFunc *cb;
     void *opaque;
     QSLIST_ENTRY(QEMUBH) next;
@@ -XXX,XX +XXX,XX @@ static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags)
     return bh;
 }
 
-void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
+                                  void *opaque, const char *name)
 {
     QEMUBH *bh;
     bh = g_new(QEMUBH, 1);
@@ -XXX,XX +XXX,XX @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
         .ctx = ctx,
         .cb = cb,
         .opaque = opaque,
+        .name = name,
     };
     aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
 }
 
-QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
+QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
+                        const char *name)
 {
     QEMUBH *bh;
     bh = g_new(QEMUBH, 1);
@@ -XXX,XX +XXX,XX @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
         .ctx = ctx,
         .cb = cb,
         .opaque = opaque,
+        .name = name,
     };
     return bh;
 }
diff --git a/util/main-loop.c b/util/main-loop.c
index XXXXXXX..XXXXXXX 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -XXX,XX +XXX,XX @@ void main_loop_wait(int nonblocking)
 
 /* Functions to operate on the main QEMU AioContext.  */
 
-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name)
 {
-    return aio_bh_new(qemu_aio_context, cb, opaque);
+    return aio_bh_new_full(qemu_aio_context, cb, opaque, name);
 }
 
 /*
-- 
2.31.1

BHs must be deleted before the AioContext is finalized. If not, it's a
bug and probably indicates that some part of the program still expects
the BH to run in the future. That can lead to memory leaks, inconsistent
state, or just hangs.

Unfortunately the assert(flags & BH_DELETED) call in aio_ctx_finalize()
is difficult to debug because the assertion failure contains no
information about the BH!

Use the QEMUBH name field added in the previous patch to show a useful
error when a leaked BH is detected.

Suggested-by: Eric Ernst <eric.g.ernst@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20210414200247.917496-3-stefanha@redhat.com>
---
 util/async.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/util/async.c b/util/async.c
index XXXXXXX..XXXXXXX 100644
--- a/util/async.c
+++ b/util/async.c
@@ -XXX,XX +XXX,XX @@ aio_ctx_finalize(GSource     *source)
     assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list));
 
     while ((bh = aio_bh_dequeue(&ctx->bh_list, &flags))) {
-        /* qemu_bh_delete() must have been called on BHs in this AioContext */
-        assert(flags & BH_DELETED);
+        /*
+         * qemu_bh_delete() must have been called on BHs in this AioContext. In
+         * many cases memory leaks, hangs, or inconsistent state occur when a
+         * BH is leaked because something still expects it to run.
+         *
+         * If you hit this, fix the lifecycle of the BH so that
+         * qemu_bh_delete() and any associated cleanup is called before the
+         * AioContext is finalized.
+         */
+        if (unlikely(!(flags & BH_DELETED))) {
+            fprintf(stderr, "%s: BH '%s' leaked, aborting...\n",
+                    __func__, bh->name);
+            abort();
+        }
 
         g_free(bh);
     }
-- 
2.31.1

From: Akihiko Odaki <akihiko.odaki@gmail.com>

This commit introduces "punch hole" operation and optimizes transfer
block size for macOS.

Thanks to Konstantin Nazarov for detailed analysis of a flaw in an
old version of this change:
https://gist.github.com/akihikodaki/87df4149e7ca87f18dc56807ec5a1bc5#gistcomment-3654667

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Message-id: 20210705130458.97642-1-akihiko.odaki@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/file-posix.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@
 #if defined(HAVE_HOST_BLOCK_DEVICE)
 #include <paths.h>
 #include <sys/param.h>
+#include <sys/mount.h>
 #include <IOKit/IOKitLib.h>
 #include <IOKit/IOBSD.h>
 #include <IOKit/storage/IOMediaBSDClient.h>
@@ -XXX,XX +XXX,XX @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
         return;
     }
 
+#if defined(__APPLE__) && (__MACH__)
+    struct statfs buf;
+
+    if (!fstatfs(s->fd, &buf)) {
+        bs->bl.opt_transfer = buf.f_iosize;
+        bs->bl.pdiscard_alignment = buf.f_bsize;
+    }
+#endif
+
     if (bs->sg || S_ISBLK(st.st_mode)) {
         int ret = hdev_get_max_hw_transfer(s->fd, &st);
 
@@ -XXX,XX +XXX,XX @@ out:
     }
 }
 
+#if defined(CONFIG_FALLOCATE) || defined(BLKZEROOUT) || defined(BLKDISCARD)
 static int translate_err(int err)
 {
     if (err == -ENODEV || err == -ENOSYS || err == -EOPNOTSUPP ||
@@ -XXX,XX +XXX,XX @@ static int translate_err(int err)
     }
     return err;
 }
+#endif
 
 #ifdef CONFIG_FALLOCATE
 static int do_fallocate(int fd, int mode, off_t offset, off_t len)
@@ -XXX,XX +XXX,XX @@ static int handle_aiocb_discard(void *opaque)
             }
         } while (errno == EINTR);
 
-        ret = -errno;
+        ret = translate_err(-errno);
 #endif
     } else {
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
         ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
                            aiocb->aio_offset, aiocb->aio_nbytes);
+        ret = translate_err(-errno);
+#elif defined(__APPLE__) && (__MACH__)
+        fpunchhole_t fpunchhole;
+        fpunchhole.fp_flags = 0;
+        fpunchhole.reserved = 0;
+        fpunchhole.fp_offset = aiocb->aio_offset;
+        fpunchhole.fp_length = aiocb->aio_nbytes;
+        if (fcntl(s->fd, F_PUNCHHOLE, &fpunchhole) == -1) {
+            ret = errno == ENODEV ? -ENOTSUP : -errno;
+        } else {
+            ret = 0;
+        }
 #endif
     }
 
-    ret = translate_err(ret);
     if (ret == -ENOTSUP) {
         s->has_discard = false;
     }
-- 
2.31.1

From: Akihiko Odaki <akihiko.odaki@gmail.com>

backend_defaults property allow users to control if default block
properties should be decided with backend information.

If it is off, any backend information will be discarded, which is
suitable if you plan to perform live migration to a different disk backend.

If it is on, a block device may utilize backend information more
aggressively.

By default, it is auto, which uses backend information for block
sizes and ignores the others, which is consistent with the older
versions.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Message-id: 20210705130458.97642-2-akihiko.odaki@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/hw/block/block.h   |  3 +++
 hw/block/block.c           | 42 ++++++++++++++++++++++++++++++++++----
 tests/qemu-iotests/172.out | 38 ++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -XXX,XX +XXX,XX @@
 
 typedef struct BlockConf {
     BlockBackend *blk;
+    OnOffAuto backend_defaults;
     uint32_t physical_block_size;
     uint32_t logical_block_size;
     uint32_t min_io_size;
@@ -XXX,XX +XXX,XX @@ static inline unsigned int get_physical_block_exp(BlockConf *conf)
 }
 
 #define DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf)                     \
+    DEFINE_PROP_ON_OFF_AUTO("backend_defaults", _state,                 \
+                            _conf.backend_defaults, ON_OFF_AUTO_AUTO),  \
     DEFINE_PROP_BLOCKSIZE("logical_block_size", _state,                 \
                           _conf.logical_block_size),                    \
     DEFINE_PROP_BLOCKSIZE("physical_block_size", _state,                \
diff --git a/hw/block/block.c b/hw/block/block.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -XXX,XX +XXX,XX @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp)
 {
     BlockBackend *blk = conf->blk;
     BlockSizes blocksizes;
-    int backend_ret;
+    BlockDriverState *bs;
+    bool use_blocksizes;
+    bool use_bs;
+
+    switch (conf->backend_defaults) {
+    case ON_OFF_AUTO_AUTO:
+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
+        use_bs = false;
+        break;
+
+    case ON_OFF_AUTO_ON:
+        use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes);
+        bs = blk_bs(blk);
+        use_bs = bs;
+        break;
+
+    case ON_OFF_AUTO_OFF:
+        use_blocksizes = false;
+        use_bs = false;
+        break;
+
+    default:
+        abort();
+    }
 
-    backend_ret = blk_probe_blocksizes(blk, &blocksizes);
     /* fill in detected values if they are not defined via qemu command line */
     if (!conf->physical_block_size) {
-        if (!backend_ret) {
+        if (use_blocksizes) {
            conf->physical_block_size = blocksizes.phys;
         } else {
             conf->physical_block_size = BDRV_SECTOR_SIZE;
         }
     }
     if (!conf->logical_block_size) {
-        if (!backend_ret) {
+        if (use_blocksizes) {
             conf->logical_block_size = blocksizes.log;
         } else {
             conf->logical_block_size = BDRV_SECTOR_SIZE;
         }
     }
+    if (use_bs) {
+        if (!conf->opt_io_size) {
+            conf->opt_io_size = bs->bl.opt_transfer;
+        }
+        if (conf->discard_granularity == -1) {
+            if (bs->bl.pdiscard_alignment) {
+                conf->discard_granularity = bs->bl.pdiscard_alignment;
+            } else if (bs->bl.request_alignment != 1) {
+                conf->discard_granularity = bs->bl.request_alignment;
+            }
+        }
+    }
 
     if (conf->logical_block_size > conf->physical_block_size) {
         error_setg(errp,
diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out
index XXXXXXX..XXXXXXX 100644
--- a/tests/qemu-iotests/172.out
+++ b/tests/qemu-iotests/172.out
@@ -XXX,XX +XXX,XX @@ Testing:
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "none1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "floppy1"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
               dev: floppy, id ""
                 unit = 1 (0x1)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "floppy0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = ""
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=120
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = ""
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=144
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = ""
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=288
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = ""
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
@@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica
               dev: floppy, id ""
                 unit = 0 (0x0)
                 drive = "none0"
+                backend_defaults = "auto"
                 logical_block_size = 512 (512 B)
                 physical_block_size = 512 (512 B)
                 min_io_size = 0 (0 B)
-- 
2.31.1

The following changes since commit f5fe7c17ac4e309e47e78f0f9761aebc8d2f2c81:

Merge tag 'pull-tcg-20230823-2' of https://gitlab.com/rth7680/qemu into staging (2023-08-28 16:07:04 -0400)

are available in the Git repository at:

https://gitlab.com/hreitz/qemu.git tags/pull-block-2023-09-01

for you to fetch changes up to 380448464dd89291cf7fd7434be6c225482a334d:

tests/file-io-error: New test (2023-08-29 13:01:24 +0200)

----------------------------------------------------------------
Block patches

- Fix for file-posix's zoning code crashing on I/O errors
- Throttling refactoring

----------------------------------------------------------------
Hanna Czenczek (5):
  file-posix: Clear bs->bl.zoned on error
  file-posix: Check bs->bl.zoned for zone info
  file-posix: Fix zone update in I/O error path
  file-posix: Simplify raw_co_prw's 'out' zone code
  tests/file-io-error: New test

Zhenwei Pi (9):
  throttle: introduce enum ThrottleDirection
  test-throttle: use enum ThrottleDirection
  throttle: support read-only and write-only
  test-throttle: test read only and write only
  cryptodev: use NULL throttle timer cb for read direction
  throttle: use enum ThrottleDirection instead of bool is_write
  throttle: use THROTTLE_MAX/ARRAY_SIZE for hard code
  fsdev: Use ThrottleDirection instread of bool is_write
  block/throttle-groups: Use ThrottleDirection instread of bool is_write

fsdev/qemu-fsdev-throttle.h                |   4 +-
 include/block/throttle-groups.h            |   6 +-
 include/qemu/throttle.h                    |  16 +-
 backends/cryptodev.c                       |  12 +-
 block/block-backend.c                      |   4 +-
 block/file-posix.c                         |  42 +++---
 block/throttle-groups.c                    | 163 +++++++++++----------
 block/throttle.c                           |   8 +-
 fsdev/qemu-fsdev-throttle.c                |  18 ++-
 hw/9pfs/cofile.c                           |   4 +-
 tests/unit/test-throttle.c                 |  76 +++++++++-
 util/throttle.c                            |  84 +++++++----
 tests/qemu-iotests/tests/file-io-error     | 119 +++++++++++++++
 tests/qemu-iotests/tests/file-io-error.out |  33 +++++
 14 files changed, 418 insertions(+), 171 deletions(-)
 create mode 100755 tests/qemu-iotests/tests/file-io-error
 create mode 100644 tests/qemu-iotests/tests/file-io-error.out

-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

Use enum ThrottleDirection instead of number index.

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-2-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 include/qemu/throttle.h | 11 ++++++++---
 util/throttle.c         | 16 +++++++++-------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleState {
     int64_t previous_leak;    /* timestamp of the last leak done */
 } ThrottleState;
 
+typedef enum {
+    THROTTLE_READ = 0,
+    THROTTLE_WRITE,
+    THROTTLE_MAX
+} ThrottleDirection;
+
 typedef struct ThrottleTimers {
-    QEMUTimer *timers[2];     /* timers used to do the throttling */
+    QEMUTimer *timers[THROTTLE_MAX];    /* timers used to do the throttling */
     QEMUClockType clock_type; /* the clock used */
 
     /* Callbacks */
-    QEMUTimerCB *read_timer_cb;
-    QEMUTimerCB *write_timer_cb;
+    QEMUTimerCB *timer_cb[THROTTLE_MAX];
     void *timer_opaque;
 } ThrottleTimers;
 
diff --git a/util/throttle.c b/util/throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -XXX,XX +XXX,XX @@ static bool throttle_compute_timer(ThrottleState *ts,
 void throttle_timers_attach_aio_context(ThrottleTimers *tt,
                                         AioContext *new_context)
 {
-    tt->timers[0] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-                                  tt->read_timer_cb, tt->timer_opaque);
-    tt->timers[1] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-                                  tt->write_timer_cb, tt->timer_opaque);
+    tt->timers[THROTTLE_READ] =
+        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                      tt->timer_cb[THROTTLE_READ], tt->timer_opaque);
+    tt->timers[THROTTLE_WRITE] =
+        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                      tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque);
 }
 
 /*
@@ -XXX,XX +XXX,XX @@ void throttle_timers_init(ThrottleTimers *tt,
     memset(tt, 0, sizeof(ThrottleTimers));
 
     tt->clock_type = clock_type;
-    tt->read_timer_cb = read_timer_cb;
-    tt->write_timer_cb = write_timer_cb;
+    tt->timer_cb[THROTTLE_READ] = read_timer_cb;
+    tt->timer_cb[THROTTLE_WRITE] = write_timer_cb;
     tt->timer_opaque = timer_opaque;
     throttle_timers_attach_aio_context(tt, aio_context);
 }
@@ -XXX,XX +XXX,XX @@ void throttle_timers_detach_aio_context(ThrottleTimers *tt)
 {
     int i;
 
-    for (i = 0; i < 2; i++) {
+    for (i = 0; i < THROTTLE_MAX; i++) {
         throttle_timer_destroy(&tt->timers[i]);
     }
 }
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

Use enum ThrottleDirection instead in the throttle test codes.

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-3-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 tests/unit/test-throttle.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -XXX,XX +XXX,XX @@ static void test_init(void)
 
     /* check initialized fields */
     g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
-    g_assert(tt->timers[0]);
-    g_assert(tt->timers[1]);
+    g_assert(tt->timers[THROTTLE_READ]);
+    g_assert(tt->timers[THROTTLE_WRITE]);
 
     /* check other fields where cleared */
     g_assert(!ts.previous_leak);
@@ -XXX,XX +XXX,XX @@ static void test_destroy(void)
     throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
                          read_timer_cb, write_timer_cb, &ts);
     throttle_timers_destroy(tt);
-    for (i = 0; i < 2; i++) {
+    for (i = 0; i < THROTTLE_MAX; i++) {
         g_assert(!tt->timers[i]);
     }
 }
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

Only one direction is necessary in several scenarios:
- a read-only disk
- operations on a device are considered as *write* only. For example,
  encrypt/decrypt/sign/verify operations on a cryptodev use a single
  *write* timer(read timer callback is defined, but never invoked).

Allow a single direction in throttle, this reduces memory, and uplayer
does not need a dummy callback any more.

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-4-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 util/throttle.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/util/throttle.c b/util/throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -XXX,XX +XXX,XX @@ static bool throttle_compute_timer(ThrottleState *ts,
 void throttle_timers_attach_aio_context(ThrottleTimers *tt,
                                         AioContext *new_context)
 {
-    tt->timers[THROTTLE_READ] =
-        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-                      tt->timer_cb[THROTTLE_READ], tt->timer_opaque);
-    tt->timers[THROTTLE_WRITE] =
-        aio_timer_new(new_context, tt->clock_type, SCALE_NS,
-                      tt->timer_cb[THROTTLE_WRITE], tt->timer_opaque);
+    ThrottleDirection dir;
+
+    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+        if (tt->timer_cb[dir]) {
+            tt->timers[dir] =
+                aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+                              tt->timer_cb[dir], tt->timer_opaque);
+        }
+    }
 }
 
 /*
@@ -XXX,XX +XXX,XX @@ void throttle_timers_init(ThrottleTimers *tt,
                           QEMUTimerCB *write_timer_cb,
                           void *timer_opaque)
 {
+    assert(read_timer_cb || write_timer_cb);
     memset(tt, 0, sizeof(ThrottleTimers));
 
     tt->clock_type = clock_type;
@@ -XXX,XX +XXX,XX @@ void throttle_timers_init(ThrottleTimers *tt,
 /* destroy a timer */
 static void throttle_timer_destroy(QEMUTimer **timer)
 {
-    assert(*timer != NULL);
+    if (*timer == NULL) {
+        return;
+    }
 
     timer_free(*timer);
     *timer = NULL;
@@ -XXX,XX +XXX,XX @@ static void throttle_timer_destroy(QEMUTimer **timer)
 /* Remove timers from event loop */
 void throttle_timers_detach_aio_context(ThrottleTimers *tt)
 {
-    int i;
+    ThrottleDirection dir;
 
-    for (i = 0; i < THROTTLE_MAX; i++) {
-        throttle_timer_destroy(&tt->timers[i]);
+    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+        throttle_timer_destroy(&tt->timers[dir]);
     }
 }
 
@@ -XXX,XX +XXX,XX @@ void throttle_timers_destroy(ThrottleTimers *tt)
 /* is any throttling timer configured */
 bool throttle_timers_are_initialized(ThrottleTimers *tt)
 {
-    if (tt->timers[0]) {
-        return true;
+    ThrottleDirection dir;
+
+    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+        if (tt->timers[dir]) {
+            return true;
+        }
     }
 
     return false;
@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
 {
     int64_t now = qemu_clock_get_ns(tt->clock_type);
     int64_t next_timestamp;
+    QEMUTimer *timer;
     bool must_wait;
 
+    timer = is_write ? tt->timers[THROTTLE_WRITE] : tt->timers[THROTTLE_READ];
+    assert(timer);
+
     must_wait = throttle_compute_timer(ts,
                                        is_write,
                                        now,
@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
     }
 
     /* request throttled and timer pending -> do nothing */
-    if (timer_pending(tt->timers[is_write])) {
+    if (timer_pending(timer)) {
         return true;
     }
 
     /* request throttled and timer not pending -> arm timer */
-    timer_mod(tt->timers[is_write], next_timestamp);
+    timer_mod(timer, next_timestamp);
     return true;
 }
 
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-5-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 tests/unit/test-throttle.c | 66 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -XXX,XX +XXX,XX @@ static void test_init(void)
     throttle_timers_destroy(tt);
 }
 
+static void test_init_readonly(void)
+{
+    int i;
+
+    tt = &tgm.throttle_timers;
+
+    /* fill the structures with crap */
+    memset(&ts, 1, sizeof(ts));
+    memset(tt, 1, sizeof(*tt));
+
+    /* init structures */
+    throttle_init(&ts);
+    throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
+                         read_timer_cb, NULL, &ts);
+
+    /* check initialized fields */
+    g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
+    g_assert(tt->timers[THROTTLE_READ]);
+    g_assert(!tt->timers[THROTTLE_WRITE]);
+
+    /* check other fields where cleared */
+    g_assert(!ts.previous_leak);
+    g_assert(!ts.cfg.op_size);
+    for (i = 0; i < BUCKETS_COUNT; i++) {
+        g_assert(!ts.cfg.buckets[i].avg);
+        g_assert(!ts.cfg.buckets[i].max);
+        g_assert(!ts.cfg.buckets[i].level);
+    }
+
+    throttle_timers_destroy(tt);
+}
+
+static void test_init_writeonly(void)
+{
+    int i;
+
+    tt = &tgm.throttle_timers;
+
+    /* fill the structures with crap */
+    memset(&ts, 1, sizeof(ts));
+    memset(tt, 1, sizeof(*tt));
+
+    /* init structures */
+    throttle_init(&ts);
+    throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
+                         NULL, write_timer_cb, &ts);
+
+    /* check initialized fields */
+    g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
+    g_assert(!tt->timers[THROTTLE_READ]);
+    g_assert(tt->timers[THROTTLE_WRITE]);
+
+    /* check other fields where cleared */
+    g_assert(!ts.previous_leak);
+    g_assert(!ts.cfg.op_size);
+    for (i = 0; i < BUCKETS_COUNT; i++) {
+        g_assert(!ts.cfg.buckets[i].avg);
+        g_assert(!ts.cfg.buckets[i].max);
+        g_assert(!ts.cfg.buckets[i].level);
+    }
+
+    throttle_timers_destroy(tt);
+}
+
 static void test_destroy(void)
 {
     int i;
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
     g_test_add_func("/throttle/leak_bucket",        test_leak_bucket);
     g_test_add_func("/throttle/compute_wait",       test_compute_wait);
     g_test_add_func("/throttle/init",               test_init);
+    g_test_add_func("/throttle/init_readonly",      test_init_readonly);
+    g_test_add_func("/throttle/init_writeonly",     test_init_writeonly);
     g_test_add_func("/throttle/destroy",            test_destroy);
     g_test_add_func("/throttle/have_timer",         test_have_timer);
     g_test_add_func("/throttle/detach_attach",      test_detach_attach);
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

Operations on a cryptodev are considered as *write* only, the callback
of read direction is never invoked. Use NULL instead of an unreachable
path(cryptodev_backend_throttle_timer_cb on read direction).

The dummy read timer(never invoked) is already removed here, it means
that the 'FIXME' tag is no longer needed.

Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-6-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 backends/cryptodev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index XXXXXXX..XXXXXXX 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -XXX,XX +XXX,XX @@ static void cryptodev_backend_set_throttle(CryptoDevBackend *backend, int field,
     if (!enabled) {
         throttle_init(&backend->ts);
         throttle_timers_init(&backend->tt, qemu_get_aio_context(),
-                             QEMU_CLOCK_REALTIME,
-                             cryptodev_backend_throttle_timer_cb, /* FIXME */
+                             QEMU_CLOCK_REALTIME, NULL,
                              cryptodev_backend_throttle_timer_cb, backend);
     }
 
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

enum ThrottleDirection is already there, use ThrottleDirection instead
of 'bool is_write' for throttle API, also modify related codes from
block, fsdev, cryptodev and tests.

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-7-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 include/qemu/throttle.h     |  5 +++--
 backends/cryptodev.c        |  9 +++++----
 block/throttle-groups.c     |  6 ++++--
 fsdev/qemu-fsdev-throttle.c |  8 +++++---
 tests/unit/test-throttle.c  |  4 ++--
 util/throttle.c             | 31 +++++++++++++++++--------------
 6 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index XXXXXXX..XXXXXXX 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -XXX,XX +XXX,XX @@ void throttle_config_init(ThrottleConfig *cfg);
 /* usage */
 bool throttle_schedule_timer(ThrottleState *ts,
                              ThrottleTimers *tt,
-                             bool is_write);
+                             ThrottleDirection direction);
 
-void throttle_account(ThrottleState *ts, bool is_write, uint64_t size);
+void throttle_account(ThrottleState *ts, ThrottleDirection direction,
+                      uint64_t size);
 void throttle_limits_to_config(ThrottleLimits *arg, ThrottleConfig *cfg,
                                Error **errp);
 void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var);
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index XXXXXXX..XXXXXXX 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -XXX,XX +XXX,XX @@ static void cryptodev_backend_throttle_timer_cb(void *opaque)
             continue;
         }
 
-        throttle_account(&backend->ts, true, ret);
+        throttle_account(&backend->ts, THROTTLE_WRITE, ret);
         cryptodev_backend_operation(backend, op_info);
         if (throttle_enabled(&backend->tc) &&
-            throttle_schedule_timer(&backend->ts, &backend->tt, true)) {
+            throttle_schedule_timer(&backend->ts, &backend->tt,
+                                    THROTTLE_WRITE)) {
             break;
         }
     }
@@ -XXX,XX +XXX,XX @@ int cryptodev_backend_crypto_operation(
         goto do_account;
     }
 
-    if (throttle_schedule_timer(&backend->ts, &backend->tt, true) ||
+    if (throttle_schedule_timer(&backend->ts, &backend->tt, THROTTLE_WRITE) ||
         !QTAILQ_EMPTY(&backend->opinfos)) {
         QTAILQ_INSERT_TAIL(&backend->opinfos, op_info, next);
         return 0;
@@ -XXX,XX +XXX,XX @@ do_account:
         return ret;
     }
 
-    throttle_account(&backend->ts, true, ret);
+    throttle_account(&backend->ts, THROTTLE_WRITE, ret);
 
     return cryptodev_backend_operation(backend, op_info);
 }
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index XXXXXXX..XXXXXXX 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
     ThrottleTimers *tt = &tgm->throttle_timers;
+    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
     bool must_wait;
 
     if (qatomic_read(&tgm->io_limits_disabled)) {
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
         return true;
     }
 
-    must_wait = throttle_schedule_timer(ts, tt, is_write);
+    must_wait = throttle_schedule_timer(ts, tt, direction);
 
     /* If a timer just got armed, set tgm as the current token */
     if (must_wait) {
@@ -XXX,XX +XXX,XX @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm
     bool must_wait;
     ThrottleGroupMember *token;
     ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
+    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
 
     assert(bytes >= 0);
 
@@ -XXX,XX +XXX,XX @@ void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm
     }
 
     /* The I/O will be executed, so do the accounting */
-    throttle_account(tgm->throttle_state, is_write, bytes);
+    throttle_account(tgm->throttle_state, direction, bytes);
 
     /* Schedule the next request */
     schedule_next_request(tgm, is_write);
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -XXX,XX +XXX,XX @@ void fsdev_throttle_init(FsThrottle *fst)
 void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
                                             struct iovec *iov, int iovcnt)
 {
+    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
+
     if (throttle_enabled(&fst->cfg)) {
-        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) ||
+        if (throttle_schedule_timer(&fst->ts, &fst->tt, direction) ||
             !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
             qemu_co_queue_wait(&fst->throttled_reqs[is_write], NULL);
         }
 
-        throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
+        throttle_account(&fst->ts, direction, iov_size(iov, iovcnt));
 
         if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
-            !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
+            !throttle_schedule_timer(&fst->ts, &fst->tt, direction)) {
             qemu_co_queue_next(&fst->throttled_reqs[is_write]);
         }
     }
diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -XXX,XX +XXX,XX @@ static bool do_test_accounting(bool is_ops, /* are we testing bps or ops */
     throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &cfg);
 
     /* account a read */
-    throttle_account(&ts, false, size);
+    throttle_account(&ts, THROTTLE_READ, size);
     /* account a write */
-    throttle_account(&ts, true, size);
+    throttle_account(&ts, THROTTLE_WRITE, size);
 
     /* check total result */
     index = to_test[is_ops][0];
diff --git a/util/throttle.c b/util/throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -XXX,XX +XXX,XX @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
 
 /* This function compute the time that must be waited while this IO
  *
- * @is_write:   true if the current IO is a write, false if it's a read
+ * @direction:  throttle direction
  * @ret:        time to wait
  */
 static int64_t throttle_compute_wait_for(ThrottleState *ts,
-                                         bool is_write)
+                                         ThrottleDirection direction)
 {
     BucketType to_check[2][4] = { {THROTTLE_BPS_TOTAL,
                                    THROTTLE_OPS_TOTAL,
@@ -XXX,XX +XXX,XX @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
     int i;
 
     for (i = 0; i < 4; i++) {
-        BucketType index = to_check[is_write][i];
+        BucketType index = to_check[direction][i];
         wait = throttle_compute_wait(&ts->cfg.buckets[index]);
         if (wait > max_wait) {
             max_wait = wait;
@@ -XXX,XX +XXX,XX @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
 
 /* compute the timer for this type of operation
  *
- * @is_write:   the type of operation
+ * @direction:  throttle direction
  * @now:        the current clock timestamp
  * @next_timestamp: the resulting timer
  * @ret:        true if a timer must be set
  */
 static bool throttle_compute_timer(ThrottleState *ts,
-                                   bool is_write,
+                                   ThrottleDirection direction,
                                    int64_t now,
                                    int64_t *next_timestamp)
 {
@@ -XXX,XX +XXX,XX @@ static bool throttle_compute_timer(ThrottleState *ts,
     throttle_do_leak(ts, now);
 
     /* compute the wait time if any */
-    wait = throttle_compute_wait_for(ts, is_write);
+    wait = throttle_compute_wait_for(ts, direction);
 
     /* if the code must wait compute when the next timer should fire */
     if (wait) {
@@ -XXX,XX +XXX,XX @@ void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg)
  * NOTE: this function is not unit tested due to it's usage of timer_mod
  *
  * @tt:       the timers structure
- * @is_write: the type of operation (read/write)
+ * @direction: throttle direction
  * @ret:      true if the timer has been scheduled else false
  */
 bool throttle_schedule_timer(ThrottleState *ts,
                              ThrottleTimers *tt,
-                             bool is_write)
+                             ThrottleDirection direction)
 {
     int64_t now = qemu_clock_get_ns(tt->clock_type);
     int64_t next_timestamp;
     QEMUTimer *timer;
     bool must_wait;
 
-    timer = is_write ? tt->timers[THROTTLE_WRITE] : tt->timers[THROTTLE_READ];
+    assert(direction < THROTTLE_MAX);
+    timer = tt->timers[direction];
     assert(timer);
 
     must_wait = throttle_compute_timer(ts,
-                                       is_write,
+                                       direction,
                                        now,
                                        &next_timestamp);
 
@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
 
 /* do the accounting for this operation
  *
- * @is_write: the type of operation (read/write)
+ * @direction: throttle direction
  * @size:     the size of the operation
  */
-void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
+void throttle_account(ThrottleState *ts, ThrottleDirection direction,
+                      uint64_t size)
 {
     const BucketType bucket_types_size[2][2] = {
         { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ },
@@ -XXX,XX +XXX,XX @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
     double units = 1.0;
     unsigned i;
 
+    assert(direction < THROTTLE_MAX);
     /* if cfg.op_size is defined and smaller than size we compute unit count */
     if (ts->cfg.op_size && size > ts->cfg.op_size) {
         units = (double) size / ts->cfg.op_size;
@@ -XXX,XX +XXX,XX @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
     for (i = 0; i < 2; i++) {
         LeakyBucket *bkt;
 
-        bkt = &ts->cfg.buckets[bucket_types_size[is_write][i]];
+        bkt = &ts->cfg.buckets[bucket_types_size[direction][i]];
         bkt->level += size;
         if (bkt->burst_length > 1) {
             bkt->burst_level += size;
         }
 
-        bkt = &ts->cfg.buckets[bucket_types_units[is_write][i]];
+        bkt = &ts->cfg.buckets[bucket_types_units[direction][i]];
         bkt->level += units;
         if (bkt->burst_length > 1) {
             bkt->burst_level += units;
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

The first dimension of both to_check and
bucket_types_size/bucket_types_units is used as throttle direction,
use THROTTLE_MAX instead of hard coded number. Also use ARRAY_SIZE()
to avoid hard coded number for the second dimension.

Hanna noticed that the two array should be static. Yes, turn them
into static variables.

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-8-pizhenwei@bytedance.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 util/throttle.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/util/throttle.c b/util/throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -XXX,XX +XXX,XX @@ int64_t throttle_compute_wait(LeakyBucket *bkt)
 static int64_t throttle_compute_wait_for(ThrottleState *ts,
                                          ThrottleDirection direction)
 {
-    BucketType to_check[2][4] = { {THROTTLE_BPS_TOTAL,
+    static const BucketType to_check[THROTTLE_MAX][4] = {
+                                  {THROTTLE_BPS_TOTAL,
                                    THROTTLE_OPS_TOTAL,
                                    THROTTLE_BPS_READ,
                                    THROTTLE_OPS_READ},
@@ -XXX,XX +XXX,XX @@ static int64_t throttle_compute_wait_for(ThrottleState *ts,
     int64_t wait, max_wait = 0;
     int i;
 
-    for (i = 0; i < 4; i++) {
+    for (i = 0; i < ARRAY_SIZE(to_check[THROTTLE_READ]); i++) {
         BucketType index = to_check[direction][i];
         wait = throttle_compute_wait(&ts->cfg.buckets[index]);
         if (wait > max_wait) {
@@ -XXX,XX +XXX,XX @@ bool throttle_schedule_timer(ThrottleState *ts,
 void throttle_account(ThrottleState *ts, ThrottleDirection direction,
                       uint64_t size)
 {
-    const BucketType bucket_types_size[2][2] = {
+    static const BucketType bucket_types_size[THROTTLE_MAX][2] = {
         { THROTTLE_BPS_TOTAL, THROTTLE_BPS_READ },
         { THROTTLE_BPS_TOTAL, THROTTLE_BPS_WRITE }
     };
-    const BucketType bucket_types_units[2][2] = {
+    static const BucketType bucket_types_units[THROTTLE_MAX][2] = {
         { THROTTLE_OPS_TOTAL, THROTTLE_OPS_READ },
         { THROTTLE_OPS_TOTAL, THROTTLE_OPS_WRITE }
     };
@@ -XXX,XX +XXX,XX @@ void throttle_account(ThrottleState *ts, ThrottleDirection direction,
         units = (double) size / ts->cfg.op_size;
     }
 
-    for (i = 0; i < 2; i++) {
+    for (i = 0; i < ARRAY_SIZE(bucket_types_size[THROTTLE_READ]); i++) {
         LeakyBucket *bkt;
 
         bkt = &ts->cfg.buckets[bucket_types_size[direction][i]];
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

'bool is_write' style is obsolete from throttle framework, adapt
fsdev to the new style.

Cc: Greg Kurz <groug@kaod.org>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-9-pizhenwei@bytedance.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 fsdev/qemu-fsdev-throttle.h |  4 ++--
 fsdev/qemu-fsdev-throttle.c | 14 +++++++-------
 hw/9pfs/cofile.c            |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
index XXXXXXX..XXXXXXX 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -XXX,XX +XXX,XX @@ typedef struct FsThrottle {
     ThrottleState ts;
     ThrottleTimers tt;
     ThrottleConfig cfg;
-    CoQueue      throttled_reqs[2];
+    CoQueue      throttled_reqs[THROTTLE_MAX];
 } FsThrottle;
 
 int fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *, Error **);
 
 void fsdev_throttle_init(FsThrottle *);
 
-void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *, ThrottleDirection ,
                                             struct iovec *, int);
 
 void fsdev_throttle_cleanup(FsThrottle *);
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -XXX,XX +XXX,XX @@ void fsdev_throttle_init(FsThrottle *fst)
     }
 }
 
-void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst,
+                                            ThrottleDirection direction,
                                             struct iovec *iov, int iovcnt)
 {
-    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
-
+    assert(direction < THROTTLE_MAX);
     if (throttle_enabled(&fst->cfg)) {
         if (throttle_schedule_timer(&fst->ts, &fst->tt, direction) ||
-            !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
-            qemu_co_queue_wait(&fst->throttled_reqs[is_write], NULL);
+            !qemu_co_queue_empty(&fst->throttled_reqs[direction])) {
+            qemu_co_queue_wait(&fst->throttled_reqs[direction], NULL);
         }
 
         throttle_account(&fst->ts, direction, iov_size(iov, iovcnt));
 
-        if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
+        if (!qemu_co_queue_empty(&fst->throttled_reqs[direction]) &&
             !throttle_schedule_timer(&fst->ts, &fst->tt, direction)) {
-            qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+            qemu_co_queue_next(&fst->throttled_reqs[direction]);
         }
     }
 }
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -XXX,XX +XXX,XX @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
     if (v9fs_request_cancelled(pdu)) {
         return -EINTR;
     }
-    fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
+    fsdev_co_throttle_request(s->ctx.fst, THROTTLE_WRITE, iov, iovcnt);
     v9fs_co_run_in_worker(
         {
             err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
@@ -XXX,XX +XXX,XX @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
     if (v9fs_request_cancelled(pdu)) {
         return -EINTR;
     }
-    fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
+    fsdev_co_throttle_request(s->ctx.fst, THROTTLE_READ, iov, iovcnt);
     v9fs_co_run_in_worker(
         {
             err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
-- 
2.41.0

From: zhenwei pi <pizhenwei@bytedance.com>

'bool is_write' style is obsolete from throttle framework, adapt
block throttle groups to the new style:
- use ThrottleDirection instead of 'bool is_write'. Ex,
  schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
  -> schedule_next_request(ThrottleGroupMember *tgm, ThrottleDirection direction)

- use THROTTLE_MAX instead of hard code. Ex, ThrottleGroupMember *tokens[2]
  -> ThrottleGroupMember *tokens[THROTTLE_MAX]

- use ThrottleDirection instead of hard code on iteration. Ex, (i = 0; i < 2; i++)
  -> for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++)

Use a simple python script to test the new style:
 #!/usr/bin/python3
import subprocess
import random
import time

commands = ['virsh blkdeviotune jammy vda --write-bytes-sec ', \
            'virsh blkdeviotune jammy vda --write-iops-sec ', \
            'virsh blkdeviotune jammy vda --read-bytes-sec ', \
            'virsh blkdeviotune jammy vda --read-iops-sec ']

for loop in range(1, 1000):
    time.sleep(random.randrange(3, 5))
    command = commands[random.randrange(0, 3)] + str(random.randrange(0, 1000000))
    subprocess.run(command, shell=True, check=True)

This works fine.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
Message-Id: <20230728022006.1098509-10-pizhenwei@bytedance.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 include/block/throttle-groups.h |   6 +-
 block/block-backend.c           |   4 +-
 block/throttle-groups.c         | 161 ++++++++++++++++----------------
 block/throttle.c                |   8 +-
 4 files changed, 90 insertions(+), 89 deletions(-)

diff --git a/include/block/throttle-groups.h b/include/block/throttle-groups.h
index XXXXXXX..XXXXXXX 100644
--- a/include/block/throttle-groups.h
+++ b/include/block/throttle-groups.h
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleGroupMember {
     AioContext   *aio_context;
     /* throttled_reqs_lock protects the CoQueues for throttled requests.  */
     CoMutex      throttled_reqs_lock;
-    CoQueue      throttled_reqs[2];
+    CoQueue      throttled_reqs[THROTTLE_MAX];
 
     /* Nonzero if the I/O limits are currently being ignored; generally
      * it is zero.  Accessed with atomic operations.
@@ -XXX,XX +XXX,XX @@ typedef struct ThrottleGroupMember {
      * throttle_state tells us if I/O limits are configured. */
     ThrottleState *throttle_state;
     ThrottleTimers throttle_timers;
-    unsigned       pending_reqs[2];
+    unsigned       pending_reqs[THROTTLE_MAX];
     QLIST_ENTRY(ThrottleGroupMember) round_robin;
 
 } ThrottleGroupMember;
@@ -XXX,XX +XXX,XX @@ void throttle_group_restart_tgm(ThrottleGroupMember *tgm);
 
 void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
                                                         int64_t bytes,
-                                                        bool is_write);
+                                                        ThrottleDirection direction);
 void throttle_group_attach_aio_context(ThrottleGroupMember *tgm,
                                        AioContext *new_context);
 void throttle_group_detach_aio_context(ThrottleGroupMember *tgm);
diff --git a/block/block-backend.c b/block/block-backend.c
index XXXXXXX..XXXXXXX 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -XXX,XX +XXX,XX @@ blk_co_do_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes,
     /* throttling disk I/O */
     if (blk->public.throttle_group_member.throttle_state) {
         throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member,
-                bytes, false);
+                bytes, THROTTLE_READ);
     }
 
     ret = bdrv_co_preadv_part(blk->root, offset, bytes, qiov, qiov_offset,
@@ -XXX,XX +XXX,XX @@ blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
     /* throttling disk I/O */
     if (blk->public.throttle_group_member.throttle_state) {
         throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member,
-                bytes, true);
+                bytes, THROTTLE_WRITE);
     }
 
     if (!blk->enable_write_cache) {
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index XXXXXXX..XXXXXXX 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -XXX,XX +XXX,XX @@
 
 static void throttle_group_obj_init(Object *obj);
 static void throttle_group_obj_complete(UserCreatable *obj, Error **errp);
-static void timer_cb(ThrottleGroupMember *tgm, bool is_write);
+static void timer_cb(ThrottleGroupMember *tgm, ThrottleDirection direction);
 
 /* The ThrottleGroup structure (with its ThrottleState) is shared
  * among different ThrottleGroupMembers and it's independent from
@@ -XXX,XX +XXX,XX @@ struct ThrottleGroup {
     QemuMutex lock; /* This lock protects the following four fields */
     ThrottleState ts;
     QLIST_HEAD(, ThrottleGroupMember) head;
-    ThrottleGroupMember *tokens[2];
-    bool any_timer_armed[2];
+    ThrottleGroupMember *tokens[THROTTLE_MAX];
+    bool any_timer_armed[THROTTLE_MAX];
     QEMUClockType clock_type;
 
     /* This field is protected by the global QEMU mutex */
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *throttle_group_next_tgm(ThrottleGroupMember *tgm)
  * This assumes that tg->lock is held.
  *
  * @tgm:        the ThrottleGroupMember
- * @is_write:   the type of operation (read/write)
+ * @direction:  the ThrottleDirection
  * @ret:        whether the ThrottleGroupMember has pending requests.
  */
 static inline bool tgm_has_pending_reqs(ThrottleGroupMember *tgm,
-                                        bool is_write)
+                                        ThrottleDirection direction)
 {
-    return tgm->pending_reqs[is_write];
+    return tgm->pending_reqs[direction];
 }
 
 /* Return the next ThrottleGroupMember in the round-robin sequence with pending
@@ -XXX,XX +XXX,XX @@ static inline bool tgm_has_pending_reqs(ThrottleGroupMember *tgm,
  * This assumes that tg->lock is held.
  *
  * @tgm:       the current ThrottleGroupMember
- * @is_write:  the type of operation (read/write)
+ * @direction: the ThrottleDirection
  * @ret:       the next ThrottleGroupMember with pending requests, or tgm if
  *             there is none.
  */
 static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
-                                                bool is_write)
+                                                ThrottleDirection direction)
 {
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
      * it's being drained. Skip the round-robin search and return tgm
      * immediately if it has pending requests. Otherwise we could be
      * forcing it to wait for other member's throttled requests. */
-    if (tgm_has_pending_reqs(tgm, is_write) &&
+    if (tgm_has_pending_reqs(tgm, direction) &&
         qatomic_read(&tgm->io_limits_disabled)) {
         return tgm;
     }
 
-    start = token = tg->tokens[is_write];
+    start = token = tg->tokens[direction];
 
     /* get next bs round in round robin style */
     token = throttle_group_next_tgm(token);
-    while (token != start && !tgm_has_pending_reqs(token, is_write)) {
+    while (token != start && !tgm_has_pending_reqs(token, direction)) {
         token = throttle_group_next_tgm(token);
     }
 
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
      * then decide the token is the current tgm because chances are
      * the current tgm got the current request queued.
      */
-    if (token == start && !tgm_has_pending_reqs(token, is_write)) {
+    if (token == start && !tgm_has_pending_reqs(token, direction)) {
         token = tgm;
     }
 
     /* Either we return the original TGM, or one with pending requests */
-    assert(token == tgm || tgm_has_pending_reqs(token, is_write));
+    assert(token == tgm || tgm_has_pending_reqs(token, direction));
 
     return token;
 }
@@ -XXX,XX +XXX,XX @@ static ThrottleGroupMember *next_throttle_token(ThrottleGroupMember *tgm,
  * This assumes that tg->lock is held.
  *
  * @tgm:        the current ThrottleGroupMember
- * @is_write:   the type of operation (read/write)
+ * @direction:  the ThrottleDirection
  * @ret:        whether the I/O request needs to be throttled or not
  */
 static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
-                                          bool is_write)
+                                          ThrottleDirection direction)
 {
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
     ThrottleTimers *tt = &tgm->throttle_timers;
-    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
     bool must_wait;
 
     if (qatomic_read(&tgm->io_limits_disabled)) {
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
     }
 
     /* Check if any of the timers in this group is already armed */
-    if (tg->any_timer_armed[is_write]) {
+    if (tg->any_timer_armed[direction]) {
         return true;
     }
 
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
 
     /* If a timer just got armed, set tgm as the current token */
     if (must_wait) {
-        tg->tokens[is_write] = tgm;
-        tg->any_timer_armed[is_write] = true;
+        tg->tokens[direction] = tgm;
+        tg->any_timer_armed[direction] = true;
     }
 
     return must_wait;
@@ -XXX,XX +XXX,XX @@ static bool throttle_group_schedule_timer(ThrottleGroupMember *tgm,
  * any request was actually pending.
  *
  * @tgm:       the current ThrottleGroupMember
- * @is_write:  the type of operation (read/write)
+ * @direction: the ThrottleDirection
  */
 static bool coroutine_fn throttle_group_co_restart_queue(ThrottleGroupMember *tgm,
-                                                         bool is_write)
+                                                         ThrottleDirection direction)
 {
     bool ret;
 
     qemu_co_mutex_lock(&tgm->throttled_reqs_lock);
-    ret = qemu_co_queue_next(&tgm->throttled_reqs[is_write]);
+    ret = qemu_co_queue_next(&tgm->throttled_reqs[direction]);
     qemu_co_mutex_unlock(&tgm->throttled_reqs_lock);
 
     return ret;
@@ -XXX,XX +XXX,XX @@ static bool coroutine_fn throttle_group_co_restart_queue(ThrottleGroupMember *tg
  * This assumes that tg->lock is held.
  *
  * @tgm:       the current ThrottleGroupMember
- * @is_write:  the type of operation (read/write)
+ * @direction: the ThrottleDirection
  */
-static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
+static void schedule_next_request(ThrottleGroupMember *tgm,
+                                  ThrottleDirection direction)
 {
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
@@ -XXX,XX +XXX,XX @@ static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
     ThrottleGroupMember *token;
 
     /* Check if there's any pending request to schedule next */
-    token = next_throttle_token(tgm, is_write);
-    if (!tgm_has_pending_reqs(token, is_write)) {
+    token = next_throttle_token(tgm, direction);
+    if (!tgm_has_pending_reqs(token, direction)) {
         return;
     }
 
     /* Set a timer for the request if it needs to be throttled */
-    must_wait = throttle_group_schedule_timer(token, is_write);
+    must_wait = throttle_group_schedule_timer(token, direction);
 
     /* If it doesn't have to wait, queue it for immediate execution */
     if (!must_wait) {
         /* Give preference to requests from the current tgm */
         if (qemu_in_coroutine() &&
-            throttle_group_co_restart_queue(tgm, is_write)) {
+            throttle_group_co_restart_queue(tgm, direction)) {
             token = tgm;
         } else {
             ThrottleTimers *tt = &token->throttle_timers;
             int64_t now = qemu_clock_get_ns(tg->clock_type);
-            timer_mod(tt->timers[is_write], now);
-            tg->any_timer_armed[is_write] = true;
+            timer_mod(tt->timers[direction], now);
+            tg->any_timer_armed[direction] = true;
         }
-        tg->tokens[is_write] = token;
+        tg->tokens[direction] = token;
     }
 }
 
@@ -XXX,XX +XXX,XX @@ static void schedule_next_request(ThrottleGroupMember *tgm, bool is_write)
  *
  * @tgm:       the current ThrottleGroupMember
  * @bytes:     the number of bytes for this I/O
- * @is_write:  the type of operation (read/write)
+ * @direction: the ThrottleDirection
  */
 void coroutine_fn throttle_group_co_io_limits_intercept(ThrottleGroupMember *tgm,
                                                         int64_t bytes,
-                                                        bool is_write)
+                                                        ThrottleDirection direction)
 {
     bool must_wait;
     ThrottleGroupMember *token;
     ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
-    ThrottleDirection direction = is_write ? THROTTLE_WRITE : THROTTLE_READ;
 
     assert(bytes >= 0);
+    assert(direction < THROTTLE_MAX);
 
     qemu_mutex_lock(&tg->lock);
 
     /* First we check if this I/O has to be throttled. */
-    token = next_throttle_token(tgm, is_write);
-    must_wait = throttle_group_schedule_timer(token, is_write);
+    token = next_throttle_token(tgm, direction);
+    must_wait = throttle_group_schedule_timer(token, direction);
 
     /* Wait if there's a timer set or queued requests of this type */
-    if (must_wait || tgm->pending_reqs[is_write]) {
-        tgm->pending_reqs[is_write]++;
+    if (must_wait || tgm->pending_reqs[direction]) {
+        tgm->pending_reqs[direction]++;
         qemu_mutex_unlock(&tg->lock);
         qemu_co_mutex_lock(&tgm->throttled_reqs_lock);
-        qemu_co_queue_wait(&tgm->throttled_reqs[is_write],
+        qemu_co_queue_wait(&tgm->throttled_reqs[direction],
                            &tgm->throttled_reqs_lock);
         qemu_co_mutex_unlock(&tgm->throttled_reqs_lock);
         qemu_mutex_lock(&tg->lock);
-        tgm->pending_reqs[is_write]--;
+        tgm->pending_reqs[direction]--;
     }
 
     /* The I/O will be executed, so do the accounting */
     throttle_account(tgm->throttle_state, direction, bytes);
 
     /* Schedule the next request */
-    schedule_next_request(tgm, is_write);
+    schedule_next_request(tgm, direction);
 
     qemu_mutex_unlock(&tg->lock);
 }
 
 typedef struct {
     ThrottleGroupMember *tgm;
-    bool is_write;
+    ThrottleDirection direction;
 } RestartData;
 
 static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
     ThrottleGroupMember *tgm = data->tgm;
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
-    bool is_write = data->is_write;
+    ThrottleDirection direction = data->direction;
     bool empty_queue;
 
-    empty_queue = !throttle_group_co_restart_queue(tgm, is_write);
+    empty_queue = !throttle_group_co_restart_queue(tgm, direction);
 
     /* If the request queue was empty then we have to take care of
      * scheduling the next one */
     if (empty_queue) {
         qemu_mutex_lock(&tg->lock);
-        schedule_next_request(tgm, is_write);
+        schedule_next_request(tgm, direction);
         qemu_mutex_unlock(&tg->lock);
     }
 
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn throttle_group_restart_queue_entry(void *opaque)
     aio_wait_kick();
 }
 
-static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write)
+static void throttle_group_restart_queue(ThrottleGroupMember *tgm,
+                                        ThrottleDirection direction)
 {
     Coroutine *co;
     RestartData *rd = g_new0(RestartData, 1);
 
     rd->tgm = tgm;
-    rd->is_write = is_write;
+    rd->direction = direction;
 
     /* This function is called when a timer is fired or when
      * throttle_group_restart_tgm() is called. Either way, there can
      * be no timer pending on this tgm at this point */
-    assert(!timer_pending(tgm->throttle_timers.timers[is_write]));
+    assert(!timer_pending(tgm->throttle_timers.timers[direction]));
 
     qatomic_inc(&tgm->restart_pending);
 
@@ -XXX,XX +XXX,XX @@ static void throttle_group_restart_queue(ThrottleGroupMember *tgm, bool is_write
 
 void throttle_group_restart_tgm(ThrottleGroupMember *tgm)
 {
-    int i;
+    ThrottleDirection dir;
 
     if (tgm->throttle_state) {
-        for (i = 0; i < 2; i++) {
-            QEMUTimer *t = tgm->throttle_timers.timers[i];
+        for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+            QEMUTimer *t = tgm->throttle_timers.timers[dir];
             if (timer_pending(t)) {
                 /* If there's a pending timer on this tgm, fire it now */
                 timer_del(t);
-                timer_cb(tgm, i);
+                timer_cb(tgm, dir);
             } else {
                 /* Else run the next request from the queue manually */
-                throttle_group_restart_queue(tgm, i);
+                throttle_group_restart_queue(tgm, dir);
             }
         }
     }
@@ -XXX,XX +XXX,XX @@ void throttle_group_get_config(ThrottleGroupMember *tgm, ThrottleConfig *cfg)
  * because it had been throttled.
  *
  * @tgm:       the ThrottleGroupMember whose request had been throttled
- * @is_write:  the type of operation (read/write)
+ * @direction: the ThrottleDirection
  */
-static void timer_cb(ThrottleGroupMember *tgm, bool is_write)
+static void timer_cb(ThrottleGroupMember *tgm, ThrottleDirection direction)
 {
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
 
     /* The timer has just been fired, so we can update the flag */
     qemu_mutex_lock(&tg->lock);
-    tg->any_timer_armed[is_write] = false;
+    tg->any_timer_armed[direction] = false;
     qemu_mutex_unlock(&tg->lock);
 
     /* Run the request that was waiting for this timer */
-    throttle_group_restart_queue(tgm, is_write);
+    throttle_group_restart_queue(tgm, direction);
 }
 
 static void read_timer_cb(void *opaque)
 {
-    timer_cb(opaque, false);
+    timer_cb(opaque, THROTTLE_READ);
 }
 
 static void write_timer_cb(void *opaque)
 {
-    timer_cb(opaque, true);
+    timer_cb(opaque, THROTTLE_WRITE);
 }
 
 /* Register a ThrottleGroupMember from the throttling group, also initializing
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
                                  const char *groupname,
                                  AioContext *ctx)
 {
-    int i;
+    ThrottleDirection dir;
     ThrottleState *ts = throttle_group_incref(groupname);
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
 
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
 
     QEMU_LOCK_GUARD(&tg->lock);
     /* If the ThrottleGroup is new set this ThrottleGroupMember as the token */
-    for (i = 0; i < 2; i++) {
-        if (!tg->tokens[i]) {
-            tg->tokens[i] = tgm;
+    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+        if (!tg->tokens[dir]) {
+            tg->tokens[dir] = tgm;
         }
+        qemu_co_queue_init(&tgm->throttled_reqs[dir]);
     }
 
     QLIST_INSERT_HEAD(&tg->head, tgm, round_robin);
@@ -XXX,XX +XXX,XX @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
                          write_timer_cb,
                          tgm);
     qemu_co_mutex_init(&tgm->throttled_reqs_lock);
-    qemu_co_queue_init(&tgm->throttled_reqs[0]);
-    qemu_co_queue_init(&tgm->throttled_reqs[1]);
 }
 
 /* Unregister a ThrottleGroupMember from its group, removing it from the list,
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
     ThrottleState *ts = tgm->throttle_state;
     ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
     ThrottleGroupMember *token;
-    int i;
+    ThrottleDirection dir;
 
     if (!ts) {
         /* Discard already unregistered tgm */
@@ -XXX,XX +XXX,XX @@ void throttle_group_unregister_tgm(ThrottleGroupMember *tgm)
     AIO_WAIT_WHILE(tgm->aio_context, qatomic_read(&tgm->restart_pending) > 0);
 
     WITH_QEMU_LOCK_GUARD(&tg->lock) {
-        for (i = 0; i < 2; i++) {
-            assert(tgm->pending_reqs[i] == 0);
-            assert(qemu_co_queue_empty(&tgm->throttled_reqs[i]));
-            assert(!timer_pending(tgm->throttle_timers.timers[i]));
-            if (tg->tokens[i] == tgm) {
+        for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+            assert(tgm->pending_reqs[dir] == 0);
+            assert(qemu_co_queue_empty(&tgm->throttled_reqs[dir]));
+            assert(!timer_pending(tgm->throttle_timers.timers[dir]));
+            if (tg->tokens[dir] == tgm) {
                 token = throttle_group_next_tgm(tgm);
                 /* Take care of the case where this is the last tgm in the group */
                 if (token == tgm) {
                     token = NULL;
                 }
-                tg->tokens[i] = token;
+                tg->tokens[dir] = token;
             }
         }
 
@@ -XXX,XX +XXX,XX @@ void throttle_group_detach_aio_context(ThrottleGroupMember *tgm)
 {
     ThrottleGroup *tg = container_of(tgm->throttle_state, ThrottleGroup, ts);
     ThrottleTimers *tt = &tgm->throttle_timers;
-    int i;
+    ThrottleDirection dir;
 
     /* Requests must have been drained */
-    assert(tgm->pending_reqs[0] == 0 && tgm->pending_reqs[1] == 0);
-    assert(qemu_co_queue_empty(&tgm->throttled_reqs[0]));
-    assert(qemu_co_queue_empty(&tgm->throttled_reqs[1]));
+    for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+        assert(tgm->pending_reqs[dir] == 0);
+        assert(qemu_co_queue_empty(&tgm->throttled_reqs[dir]));
+    }
 
     /* Kick off next ThrottleGroupMember, if necessary */
     WITH_QEMU_LOCK_GUARD(&tg->lock) {
-        for (i = 0; i < 2; i++) {
-            if (timer_pending(tt->timers[i])) {
-                tg->any_timer_armed[i] = false;
-                schedule_next_request(tgm, i);
+        for (dir = THROTTLE_READ; dir < THROTTLE_MAX; dir++) {
+            if (timer_pending(tt->timers[dir])) {
+                tg->any_timer_armed[dir] = false;
+                schedule_next_request(tgm, dir);
             }
         }
     }
diff --git a/block/throttle.c b/block/throttle.c
index XXXXXXX..XXXXXXX 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -XXX,XX +XXX,XX @@ throttle_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes,
 {
 
     ThrottleGroupMember *tgm = bs->opaque;
-    throttle_group_co_io_limits_intercept(tgm, bytes, false);
+    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_READ);
 
     return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
 }
@@ -XXX,XX +XXX,XX @@ throttle_co_pwritev(BlockDriverState *bs, int64_t offset, int64_t bytes,
                     QEMUIOVector *qiov, BdrvRequestFlags flags)
 {
     ThrottleGroupMember *tgm = bs->opaque;
-    throttle_group_co_io_limits_intercept(tgm, bytes, true);
+    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
 
     return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
 }
@@ -XXX,XX +XXX,XX @@ throttle_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
                           BdrvRequestFlags flags)
 {
     ThrottleGroupMember *tgm = bs->opaque;
-    throttle_group_co_io_limits_intercept(tgm, bytes, true);
+    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
 
     return bdrv_co_pwrite_zeroes(bs->file, offset, bytes, flags);
 }
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn GRAPH_RDLOCK
 throttle_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
     ThrottleGroupMember *tgm = bs->opaque;
-    throttle_group_co_io_limits_intercept(tgm, bytes, true);
+    throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
 
     return bdrv_co_pdiscard(bs->file, offset, bytes);
 }
-- 
2.41.0

bs->bl.zoned is what indicates whether the zone information is present
and valid; it is the only thing that raw_refresh_zoned_limits() sets if
CONFIG_BLKZONED is not defined, and it is also the only thing that it
sets if CONFIG_BLKZONED is defined, but there are no zones.

Make sure that it is always set to BLK_Z_NONE if there is an error
anywhere in raw_refresh_zoned_limits() so that we do not accidentally
announce zones while our information is incomplete or invalid.

This also fixes a memory leak in the last error path in
raw_refresh_zoned_limits().

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-2-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
---
 block/file-posix.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     BlockZoneModel zoned;
     int ret;
 
-    bs->bl.zoned = BLK_Z_NONE;
-
     ret = get_sysfs_zoned_model(st, &zoned);
     if (ret < 0 || zoned == BLK_Z_NONE) {
-        return;
+        goto no_zoned;
     }
     bs->bl.zoned = zoned;
 
@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Unable to read chunk_sectors "
                                      "sysfs attribute");
-        return;
+        goto no_zoned;
     } else if (!ret) {
         error_setg(errp, "Read 0 from chunk_sectors sysfs attribute");
-        return;
+        goto no_zoned;
     }
     bs->bl.zone_size = ret << BDRV_SECTOR_BITS;
 
@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Unable to read nr_zones "
                                      "sysfs attribute");
-        return;
+        goto no_zoned;
     } else if (!ret) {
         error_setg(errp, "Read 0 from nr_zones sysfs attribute");
-        return;
+        goto no_zoned;
     }
     bs->bl.nr_zones = ret;
 
@@ -XXX,XX +XXX,XX @@ static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
     ret = get_zones_wp(bs, s->fd, 0, bs->bl.nr_zones, 0);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "report wps failed");
-        bs->wps = NULL;
-        return;
+        goto no_zoned;
     }
     qemu_co_mutex_init(&bs->wps->colock);
+    return;
+
+no_zoned:
+    bs->bl.zoned = BLK_Z_NONE;
+    g_free(bs->wps);
+    bs->wps = NULL;
 }
 #else /* !defined(CONFIG_BLKZONED) */
 static void raw_refresh_zoned_limits(BlockDriverState *bs, struct stat *st,
-- 
2.41.0

Instead of checking bs->wps or bs->bl.zone_size for whether zone
information is present, check bs->bl.zoned.  That is the flag that
raw_refresh_zoned_limits() reliably sets to indicate zone support.  If
it is set to something other than BLK_Z_NONE, other values and objects
like bs->wps and bs->bl.zone_size must be non-null/zero and valid; if it
is not, we cannot rely on their validity.

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-3-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
---
 block/file-posix.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
     if (fd_open(bs) < 0)
         return -EIO;
 #if defined(CONFIG_BLKZONED)
-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && bs->wps) {
+    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+        bs->bl.zoned != BLK_Z_NONE) {
         qemu_co_mutex_lock(&bs->wps->colock);
-        if (type & QEMU_AIO_ZONE_APPEND && bs->bl.zone_size) {
+        if (type & QEMU_AIO_ZONE_APPEND) {
             int index = offset / bs->bl.zone_size;
             offset = bs->wps->wp[index];
         }
@@ -XXX,XX +XXX,XX @@ out:
 {
     BlockZoneWps *wps = bs->wps;
     if (ret == 0) {
-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND))
-            && wps && bs->bl.zone_size) {
+        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+            bs->bl.zoned != BLK_Z_NONE) {
             uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
             if (!BDRV_ZT_IS_CONV(*wp)) {
                 if (type & QEMU_AIO_ZONE_APPEND) {
@@ -XXX,XX +XXX,XX @@ out:
         }
     }
 
-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) && wps) {
+    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+        bs->blk.zoned != BLK_Z_NONE) {
         qemu_co_mutex_unlock(&wps->colock);
     }
 }
-- 
2.41.0

We duplicate the same condition three times here, pull it out to the top
level.

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-5-hreitz@redhat.com>
Reviewed-by: Sam Li <faithilikerun@gmail.com>
---
 block/file-posix.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index XXXXXXX..XXXXXXX 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
 
 out:
 #if defined(CONFIG_BLKZONED)
-{
-    BlockZoneWps *wps = bs->wps;
-    if (ret == 0) {
-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
-            bs->bl.zoned != BLK_Z_NONE) {
+    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
+        bs->bl.zoned != BLK_Z_NONE) {
+        BlockZoneWps *wps = bs->wps;
+        if (ret == 0) {
             uint64_t *wp = &wps->wp[offset / bs->bl.zone_size];
             if (!BDRV_ZT_IS_CONV(*wp)) {
                 if (type & QEMU_AIO_ZONE_APPEND) {
@@ -XXX,XX +XXX,XX @@ out:
                     *wp = offset + bytes;
                 }
             }
-        }
-    } else {
-        if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
-            bs->bl.zoned != BLK_Z_NONE) {
+        } else {
             update_zones_wp(bs, s->fd, 0, 1);
         }
-    }
 
-    if ((type & (QEMU_AIO_WRITE | QEMU_AIO_ZONE_APPEND)) &&
-        bs->blk.zoned != BLK_Z_NONE) {
         qemu_co_mutex_unlock(&wps->colock);
     }
-}
 #endif
     return ret;
 }
-- 
2.41.0

This is a regression test for
https://bugzilla.redhat.com/show_bug.cgi?id=2234374.

All this test needs to do is trigger an I/O error inside of file-posix
(specifically raw_co_prw()).  One reliable way to do this without
requiring special privileges is to use a FUSE export, which allows us to
inject any error that we want, e.g. via blkdebug.

Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230824155345.109765-6-hreitz@redhat.com>
[hreitz: Fixed test to be skipped when there is no FUSE support, to
         suppress fusermount's allow_other warning, and to be skipped
         with $IMGOPTSSYNTAX enabled]
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
---
 tests/qemu-iotests/tests/file-io-error     | 119 +++++++++++++++++++++
 tests/qemu-iotests/tests/file-io-error.out |  33 ++++++
 2 files changed, 152 insertions(+)
 create mode 100755 tests/qemu-iotests/tests/file-io-error
 create mode 100644 tests/qemu-iotests/tests/file-io-error.out

diff --git a/tests/qemu-iotests/tests/file-io-error b/tests/qemu-iotests/tests/file-io-error
new file mode 100755
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/tests/file-io-error
@@ -XXX,XX +XXX,XX @@
+#!/usr/bin/env bash
+# group: rw
+#
+# Produce an I/O error in file-posix, and hope that it is not catastrophic.
+# Regression test for: https://bugzilla.redhat.com/show_bug.cgi?id=2234374
+#
+# Copyright (C) 2023 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+seq=$(basename "$0")
+echo "QA output created by $seq"
+
+status=1	# failure is the default!
+
+_cleanup()
+{
+    _cleanup_qemu
+    rm -f "$TEST_DIR/fuse-export"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ../common.rc
+. ../common.filter
+. ../common.qemu
+
+# Format-agnostic (we do not use any), but we do test the file protocol
+_supported_proto file
+_require_drivers blkdebug null-co
+
+if [ "$IMGOPTSSYNTAX" = "true" ]; then
+    # We need `$QEMU_IO -f file` to work; IMGOPTSSYNTAX uses --image-opts,
+    # breaking -f.
+    _unsupported_fmt $IMGFMT
+fi
+
+# This is a regression test of a bug in which flie-posix would access zone
+# information in case of an I/O error even when there is no zone information,
+# resulting in a division by zero.
+# To reproduce the problem, we need to trigger an I/O error inside of
+# file-posix, which can be done (rootless) by providing a FUSE export that
+# presents only errors when accessed.
+
+_launch_qemu
+_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'qmp_capabilities'}" \
+    'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'blockdev-add',
+      'arguments': {
+          'driver': 'blkdebug',
+          'node-name': 'node0',
+          'inject-error': [{'event': 'none'}],
+          'image': {
+              'driver': 'null-co'
+          }
+      }}" \
+    'return'
+
+# FUSE mountpoint must exist and be a regular file
+touch "$TEST_DIR/fuse-export"
+
+# The grep -v to filter fusermount's (benign) error when /etc/fuse.conf does
+# not contain user_allow_other and the subsequent check for missing FUSE support
+# have both been taken from iotest 308.
+output=$(_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'block-export-add',
+      'arguments': {
+          'id': 'exp0',
+          'type': 'fuse',
+          'node-name': 'node0',
+          'mountpoint': '$TEST_DIR/fuse-export',
+          'writable': true
+      }}" \
+    'return' \
+    | grep -v 'option allow_other only allowed if')
+
+if echo "$output" | grep -q "Parameter 'type' does not accept value 'fuse'"; then
+    _notrun 'No FUSE support'
+fi
+echo "$output"
+
+echo
+# This should fail, but gracefully, i.e. just print an I/O error, not crash.
+$QEMU_IO -f file -c 'write 0 64M' "$TEST_DIR/fuse-export" | _filter_qemu_io
+echo
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'block-export-del',
+      'arguments': {'id': 'exp0'}}" \
+    'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    '' \
+    'BLOCK_EXPORT_DELETED'
+
+_send_qemu_cmd $QEMU_HANDLE \
+    "{'execute': 'blockdev-del',
+      'arguments': {'node-name': 'node0'}}" \
+    'return'
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/tests/file-io-error.out b/tests/qemu-iotests/tests/file-io-error.out
new file mode 100644
index XXXXXXX..XXXXXXX
--- /dev/null
+++ b/tests/qemu-iotests/tests/file-io-error.out
@@ -XXX,XX +XXX,XX @@
+QA output created by file-io-error
+{'execute': 'qmp_capabilities'}
+{"return": {}}
+{'execute': 'blockdev-add',
+      'arguments': {
+          'driver': 'blkdebug',
+          'node-name': 'node0',
+          'inject-error': [{'event': 'none'}],
+          'image': {
+              'driver': 'null-co'
+          }
+      }}
+{"return": {}}
+{'execute': 'block-export-add',
+      'arguments': {
+          'id': 'exp0',
+          'type': 'fuse',
+          'node-name': 'node0',
+          'mountpoint': 'TEST_DIR/fuse-export',
+          'writable': true
+      }}
+{"return": {}}
+
+write failed: Input/output error
+
+{'execute': 'block-export-del',
+      'arguments': {'id': 'exp0'}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "exp0"}}
+{'execute': 'blockdev-del',
+      'arguments': {'node-name': 'node0'}}
+{"return": {}}
+*** done
-- 
2.41.0