[PATCH hci-8.1 v4 0/5] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls

[PATCH hci-8.1 v4 0/5] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls

[Alexander Ivanov]

The kernel drops our VHOST_SET_NWORKERS ioctl in favour of the
mainstream VHOST_NEW_WORKER/VHOST_ATTACH_VRING_WORKER, which vhost-blk
now supports.

Patch 1 wraps VHOST_SET_FORK_FROM_OWNER in a vhost backend op. Patch 2
refuses num-queues above VHOST_BLK_MAX_QUEUES at realize. Patch 3 is a
cleanup taking &s->dev into a local pointer. Patch 4 makes vhost-blk
create its num-threads workers with the mainstream ioctls, keeping the
same worker layout, and fall back to fewer threads when the kernel
cannot provide them. Patch 5 drops VHOST_SET_NWORKERS from
linux-headers.

v4:
- new patch 3: split the &s->dev -> hdev conversion out of patch 4
- patch 2: quote the error realize fails with, add Fixes:
- patch 4: do not key on the errno; on any worker ioctl failure warn
  and continue with the threads created so far instead of failing
  to start vhost-blk
- patch 4: drop the comments, explain in the commit message instead

v3:
- new patch 1: call VHOST_SET_FORK_FROM_OWNER, VHOST_NEW_WORKER and
  VHOST_ATTACH_VRING_WORKER through vhost_ops instead of raw ioctls
- new patch 2: refuse num-queues above VHOST_BLK_MAX_QUEUES at realize
  instead of clamping num-threads in patch 3
- patch 3: create each worker right before attaching its first vq

v2:
- patch 1: clamp num-threads to VHOST_BLK_MAX_QUEUES
- patch 1: note in the commit message that our kernel, unlike
  mainline, defaults to kthreads

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Alexander Ivanov (5):
  vhost-backend: add vhost_set_fork_from_owner op
  vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES
  vhost-blk: use a local vhost_dev pointer in start and stop
  vhost-blk: create kernel threads with the mainstream worker ioctls
  linux-headers: drop VHOST_SET_NWORKERS

 hw/block/vhost-blk.c              | 97 +++++++++++++++++++++++++------
 hw/virtio/vhost-backend.c         |  7 +++
 include/hw/virtio/vhost-backend.h |  3 +
 linux-headers/linux/vhost.h       |  9 ---
 4 files changed, 88 insertions(+), 28 deletions(-)

-- 
2.53.0

[PATCH hci-8.1 v4 1/5] vhost-backend: add vhost_set_fork_from_owner op

[Alexander Ivanov]

Wrap VHOST_SET_FORK_FROM_OWNER for the kernel backend next to the
worker ops, so devices switch the vhost worker mode the same way they
create and attach workers.

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/virtio/vhost-backend.c         | 7 +++++++
 include/hw/virtio/vhost-backend.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 1447d129630..7f8c731a607 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -187,6 +187,12 @@ static int vhost_kernel_get_vring_worker(struct vhost_dev *dev,
     return vhost_kernel_call(dev, VHOST_GET_VRING_WORKER, worker);
 }
 
+static int vhost_kernel_set_fork_from_owner(struct vhost_dev *dev,
+                                            uint8_t fork_owner)
+{
+    return vhost_kernel_call(dev, VHOST_SET_FORK_FROM_OWNER, &fork_owner);
+}
+
 static int vhost_kernel_set_backend_cap(struct vhost_dev *dev)
 {
     uint64_t features;
@@ -386,6 +392,7 @@ const VhostOps kernel_ops = {
         .vhost_attach_vring_worker = vhost_kernel_attach_vring_worker,
         .vhost_new_worker = vhost_kernel_new_worker,
         .vhost_free_worker = vhost_kernel_free_worker,
+        .vhost_set_fork_from_owner = vhost_kernel_set_fork_from_owner,
         .vhost_set_features_ex = vhost_kernel_set_features,
         .vhost_get_features_ex = vhost_kernel_get_features,
         .vhost_set_backend_cap = vhost_kernel_set_backend_cap,
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index 18ce5ea9a07..78cd66378ef 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -95,6 +95,8 @@ typedef int (*vhost_new_worker_op)(struct vhost_dev *dev,
                                    struct vhost_worker_state *worker);
 typedef int (*vhost_free_worker_op)(struct vhost_dev *dev,
                                     struct vhost_worker_state *worker);
+typedef int (*vhost_set_fork_from_owner_op)(struct vhost_dev *dev,
+                                            uint8_t fork_owner);
 typedef int (*vhost_set_features_ex_op)(struct vhost_dev *dev,
                                         const uint64_t *features);
 typedef int (*vhost_get_features_ex_op)(struct vhost_dev *dev,
@@ -190,6 +192,7 @@ typedef struct VhostOps {
     vhost_free_worker_op vhost_free_worker;
     vhost_get_vring_worker_op vhost_get_vring_worker;
     vhost_attach_vring_worker_op vhost_attach_vring_worker;
+    vhost_set_fork_from_owner_op vhost_set_fork_from_owner;
     vhost_set_features_ex_op vhost_set_features_ex;
     vhost_get_features_ex_op vhost_get_features_ex;
     vhost_set_features_op vhost_set_features;
-- 
2.53.0

Re: [PATCH hci-8.1 v4 1/5] vhost-backend: add vhost_set_fork_from_owner op

[no-reply]

> Wrap VHOST_SET_FORK_FROM_OWNER for the kernel backend next to the
> worker ops, so devices switch the vhost worker mode the same way they
> create and attach workers.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029092301.3286535-1-alexander.ivanov@virtuozzo.com?part=1

[PATCH hci-8.1 v4 2/5] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES

[Alexander Ivanov]

Only the automatic num-queues value is capped at VHOST_BLK_MAX_QUEUES,
an explicit one is taken as is. The kernel vhost-blk device has that
many virtqueues and rejects any vq beyond them, so realize fails with
an error that does not point at num-queues:

  qemu-system-x86_64: vhost_set_vring_call failed: No buffer space available (105)
  qemu-system-x86_64: vhost-blk: vhost initialization failed: No buffer space available

Check num-queues against the limit up front instead.

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Fixes: 0ff3cb5ff28f ("block: add vhost-blk backend")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 hw/block/vhost-blk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
index abddff02c6d..8a015d54fa7 100644
--- a/hw/block/vhost-blk.c
+++ b/hw/block/vhost-blk.c
@@ -598,8 +598,9 @@ static void vhost_blk_device_realize(DeviceState *dev, Error **errp)
                                VHOST_BLK_MAX_QUEUES);
     }
 
-    if (!conf->num_queues) {
-        error_setg(errp, "vhost-blk: num-queues property must be larger than 0");
+    if (!conf->num_queues || conf->num_queues > VHOST_BLK_MAX_QUEUES) {
+        error_setg(errp, "vhost-blk: invalid num-queues property (%" PRIu16 "), "
+                   "must be 1-%d", conf->num_queues, VHOST_BLK_MAX_QUEUES);
         return;
     }
 
-- 
2.53.0

Re: [PATCH hci-8.1 v4 2/5] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES

[no-reply]

> Only the automatic num-queues value is capped at VHOST_BLK_MAX_QUEUES,
> an explicit one is taken as is. The kernel vhost-blk device has that
> many virtqueues and rejects any vq beyond them, so realize fails with
> an error that does not point at num-queues:
> 
>   qemu-system-x86_64: vhost_set_vring_call failed: No buffer space available (105)
>   qemu-system-x86_64: vhost-blk: vhost initialization failed: No buffer space available
> 
> Check num-queues against the limit up front instead.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Fixes: 0ff3cb5ff28f ("block: add vhost-blk backend")
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029092301.3286535-1-alexander.ivanov@virtuozzo.com?part=2

[PATCH hci-8.1 v4 3/5] vhost-blk: use a local vhost_dev pointer in start and stop

[Alexander Ivanov]

Take &s->dev once into hdev in vhost_blk_start() and vhost_blk_stop()
instead of spelling it out at every use. No functional change.

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Fixes: 0ff3cb5ff28f ("block: add vhost-blk backend")
Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 hw/block/vhost-blk.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
index 8a015d54fa7..b3eb181ceae 100644
--- a/hw/block/vhost-blk.c
+++ b/hw/block/vhost-blk.c
@@ -36,6 +36,7 @@
 static int vhost_blk_start(VirtIODevice *vdev)
 {
     VHostBlk *s = VHOST_BLK(vdev);
+    struct vhost_dev *hdev = &s->dev;
     struct vhost_vring_file backend;
     int ret, i, nworkers;
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
@@ -70,22 +71,22 @@ static int vhost_blk_start(VirtIODevice *vdev)
         }
     }
 
-    ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, true);
+    ret = k->set_guest_notifiers(qbus->parent, hdev->nvqs, true);
     if (ret < 0) {
         error_report("vhost-blk: unable to bind guest notifiers");
         return ret;
     }
 
-    ret = vhost_dev_enable_notifiers(&s->dev, vdev);
+    ret = vhost_dev_enable_notifiers(hdev, vdev);
     if (ret < 0) {
         error_report("vhost-blk: unable to enable dev notifiers: %s",
                      strerror(errno));
         goto err_set;
     }
 
-    s->dev.acked_features = vdev->guest_features & s->dev.features;
+    hdev->acked_features = vdev->guest_features & hdev->features;
 
-    ret = vhost_dev_start(&s->dev, vdev, false);
+    ret = vhost_dev_start(hdev, vdev, false);
     if (ret < 0) {
         error_report("vhost-blk: unable to start vhost dev");
         goto err_disable;
@@ -100,11 +101,11 @@ static int vhost_blk_start(VirtIODevice *vdev)
         goto err_stop;
     }
 
-    for (i = 0; i < s->dev.nvqs; i++) {
-        vhost_virtqueue_mask(&s->dev, vdev, i, false);
+    for (i = 0; i < hdev->nvqs; i++) {
+        vhost_virtqueue_mask(hdev, vdev, i, false);
     }
 
-    for (i = 0; i < s->dev.nvqs; i++) {
+    for (i = 0; i < hdev->nvqs; i++) {
         event_notifier_set(virtio_queue_get_host_notifier(virtio_get_queue(vdev, i)));
     }
 
@@ -113,11 +114,11 @@ static int vhost_blk_start(VirtIODevice *vdev)
     return 0;
 
 err_stop:
-    vhost_dev_stop(&s->dev, vdev, false);
+    vhost_dev_stop(hdev, vdev, false);
 err_disable:
-    vhost_dev_disable_notifiers(&s->dev, vdev);
+    vhost_dev_disable_notifiers(hdev, vdev);
 err_set:
-    ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
+    ret = k->set_guest_notifiers(qbus->parent, hdev->nvqs, false);
     if (ret < 0) {
         error_report("vhost-blk: unable to unbind guest notifiers");
     }
@@ -131,6 +132,7 @@ static void vhost_blk_stop(VirtIODevice *vdev)
     BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     VHostBlk *s = VHOST_BLK(vdev);
+    struct vhost_dev *hdev = &s->dev;
     struct vhost_vring_file backend;
     int ret;
 
@@ -138,11 +140,11 @@ static void vhost_blk_stop(VirtIODevice *vdev)
         return;
     }
 
-    ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, false);
+    ret = k->set_guest_notifiers(qbus->parent, hdev->nvqs, false);
     if (ret < 0) {
         error_report("vhost-blk: unable to unbind guest notifiers");
     }
-    vhost_dev_disable_notifiers(&s->dev, vdev);
+    vhost_dev_disable_notifiers(hdev, vdev);
 
     /*
      * Detach the backend before vhost_dev_stop() samples the vring base,
@@ -156,7 +158,7 @@ static void vhost_blk_stop(VirtIODevice *vdev)
     ret = ioctl(s->vhostfd, VHOST_BLK_SET_BACKEND, &backend);
     assert(ret == 0);
 
-    vhost_dev_stop(&s->dev, vdev, false);
+    vhost_dev_stop(hdev, vdev, false);
 
     s->vhost_started = false;
 }
-- 
2.53.0

Re: [PATCH hci-8.1 v4 3/5] vhost-blk: use a local vhost_dev pointer in start and stop

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] vhost-blk: Fixes tag on a no-functional-change refactor
--

commit e32c7620df63e11379f613aefad8997664c58582
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

vhost-blk: use a local vhost_dev pointer in start and stop

This patch adds a local struct vhost_dev pointer, hdev, in vhost_blk_start()
and vhost_blk_stop(). It replaces each use of &s->dev with hdev. The commit
message says there is no functional change.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-107537

This isn't a bug in the code, but the commit message has a problem.

> vhost-blk: use a local vhost_dev pointer in start and stop
>
> Take &s->dev once into hdev in vhost_blk_start() and vhost_blk_stop()
> instead of spelling it out at every use. No functional change.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
>
> Fixes: 0ff3cb5ff28f ("block: add vhost-blk backend")
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

[Severity: Low]
The message says "No functional change", yet it carries a Fixes: tag
pointing at 0ff3cb5ff28f ("block: add vhost-blk backend").

The only edits are the new local alias in vhost_blk_start() and
vhost_blk_stop():

    struct vhost_dev *hdev = &s->dev;

and the replacement of &s->dev with hdev at each use. Nothing in the diff
fixes a bug.

Could the Fixes: tag be dropped? Stable-backport tooling may otherwise pick
up this cosmetic cleanup.

If a real bug is being fixed, could the commit message describe it?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029092301.3286535-1-alexander.ivanov@virtuozzo.com?part=3

Re: [PATCH hci-8.1 v4 3/5] vhost-blk: use a local vhost_dev pointer in start and stop

[Alexander Ivanov]

> [Severity: Low]
> The message says "No functional change", yet it carries a Fixes: tag
> pointing at 0ff3cb5ff28f ("block: add vhost-blk backend").
>
> The only edits are the new local alias in vhost_blk_start() and
> vhost_blk_stop():
>
>      struct vhost_dev *hdev = &s->dev;
>
> and the replacement of &s->dev with hdev at each use. Nothing in the diff
> fixes a bug.
>
> Could the Fixes: tag be dropped? Stable-backport tooling may otherwise pick
> up this cosmetic cleanup.
>
> If a real bug is being fixed, could the commit message describe it?
The Fixes: tag is intentional: the maintainer asked for this cleanup to 
be split
out with a Fixes: tag so it can be squashed into 0ff3cb5ff28f on the 
next rebase.

Re: [PATCH hci-8.1 v4 3/5] vhost-blk: use a local vhost_dev pointer in start and stop

[Andrey Drobyshev]

On 10/29/26 11:22 AM, Alexander Ivanov wrote:
> Take &s->dev once into hdev in vhost_blk_start() and vhost_blk_stop()
> instead of spelling it out at every use. No functional change.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Fixes: 0ff3cb5ff28f ("block: add vhost-blk backend")
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  [...]
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

[PATCH hci-8.1 v4 4/5] vhost-blk: create kernel threads with the mainstream worker ioctls

[Alexander Ivanov]

num-threads was passed to our own VHOST_SET_NWORKERS kernel ioctl. The
kernel drops it with the revert of "drivers/vhost: add ioctl to
increase the number of workers" in favour of the mainstream worker
ioctls, which vhost-blk now supports. Create num-threads - 1 workers
with VHOST_NEW_WORKER and attach them to the virtqueues round-robin
with VHOST_ATTACH_VRING_WORKER, the layout VHOST_SET_NWORKERS produced.

Our kernel, unlike mainline, defaults to kthread mode, where
VHOST_NEW_WORKER fails with -EFAULT rather than -ENOTTY. So switch the
device to vhost_task mode with VHOST_SET_FORK_FROM_OWNER before
VHOST_SET_OWNER. With num-threads=1 nothing changes: no worker ioctls,
kthread mode.

Released vz10 kernels have VHOST_SET_FORK_FROM_OWNER, but their
vhost-blk does not handle the worker ioctls and fails VHOST_NEW_WORKER
with -ENOTTY. Other errors are possible as well, so do not key on the
errno: if VHOST_SET_FORK_FROM_OWNER fails, keep all virtqueues on the
default thread; if VHOST_NEW_WORKER fails partway, spread the
remaining virtqueues over the threads created so far; if attaching a
virtqueue fails, it stays on the default thread. Either way warn rather
than fail to start vhost-blk. Unless VHOST_SET_FORK_FROM_OWNER failed,
the default thread stays a vhost_task, as the mode cannot be changed
after VHOST_SET_OWNER.

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 hw/block/vhost-blk.c | 64 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 60 insertions(+), 4 deletions(-)

diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
index b3eb181ceae..9416c3ac07e 100644
--- a/hw/block/vhost-blk.c
+++ b/hw/block/vhost-blk.c
@@ -33,6 +33,53 @@
 #include "migration/cpr.h"
 #include "system/runstate.h"
 
+/*
+ * Spread the virtqueues round-robin over nworkers kernel threads. Thread 0 is
+ * the default worker VHOST_SET_OWNER created; vq 0 and every nworkers-th vq
+ * after it stay on it.
+ */
+static void vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
+{
+    uint32_t ids[VHOST_BLK_MAX_QUEUES];
+    int i, ret, err = 0, nfailed = 0;
+
+    assert(nworkers <= ARRAY_SIZE(ids));
+
+    for (i = 1; i < hdev->nvqs; i++) {
+        int w = i % nworkers;
+        struct vhost_vring_worker vq_worker = { .index = i };
+
+        if (w == 0) {
+            continue;
+        }
+
+        if (i < nworkers) {
+            struct vhost_worker_state worker = { };
+
+            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
+            if (ret < 0) {
+                warn_report("vhost-blk: unable to create kernel thread: %s",
+                            strerror(-ret));
+                nworkers = i;
+                continue;
+            }
+            ids[w] = worker.worker_id;
+        }
+
+        vq_worker.worker_id = ids[w];
+        ret = hdev->vhost_ops->vhost_attach_vring_worker(hdev, &vq_worker);
+        if (ret < 0) {
+            err = ret;
+            nfailed++;
+        }
+    }
+
+    if (nfailed) {
+        warn_report("vhost-blk: unable to attach kernel threads to %d "
+                    "queues: %s", nfailed, strerror(-err));
+    }
+}
+
 static int vhost_blk_start(VirtIODevice *vdev)
 {
     VHostBlk *s = VHOST_BLK(vdev);
@@ -52,15 +99,24 @@ static int vhost_blk_start(VirtIODevice *vdev)
         return 0;
     }
 
+    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
+    if (nworkers > 1) {
+        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
+                                                         VHOST_FORK_OWNER_TASK);
+        if (ret < 0) {
+            warn_report("vhost-blk: unable to use multiple kernel threads: "
+                        "%s", strerror(-ret));
+            nworkers = 1;
+        }
+    }
+
     if (ioctl(s->vhostfd, VHOST_SET_OWNER, NULL)) {
         error_report("vhost-blk: unable to set owner");
         return -ENOSYS;
     }
 
-    nworkers = s->conf.num_threads;
-    if (nworkers != 1 && ioctl(s->vhostfd, VHOST_SET_NWORKERS, &nworkers)) {
-        error_report("vhost-blk: unable to set number of kernel threads");
-        return -EINVAL;
+    if (nworkers > 1) {
+        vhost_blk_set_workers(hdev, nworkers);
     }
 
     if (s->conf.serial) {
-- 
2.53.0

Re: [PATCH hci-8.1 v4 4/5] vhost-blk: create kernel threads with the mainstream worker ioctls

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] vhost-blk: vhost_blk_set_workers() leaves vq on default thread on NEW_WORKER failure
- [Low] vhost-blk: num-threads=0 is silently accepted instead of rejected
- [Low] vhost-blk: workers from vhost_blk_set_workers() not released on vhost_blk_start() error paths
--

commit 23b6bc10b68c2d486e285cf836111089db385f0f
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

vhost-blk: create kernel threads with the mainstream worker ioctls

This patch replaces the VHOST_SET_NWORKERS ioctl with VHOST_NEW_WORKER and VHOST_ATTACH_VRING_WORKER. It spreads the virtqueues round-robin over num-threads - 1 extra workers. It also switches the device to vhost_task mode with VHOST_SET_FORK_FROM_OWNER before VHOST_SET_OWNER.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-107537

> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c

[ ... ]

> +static void vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
> +{
> +    uint32_t ids[VHOST_BLK_MAX_QUEUES];
> +    int i, ret, err = 0, nfailed = 0;
> +
> +    assert(nworkers <= ARRAY_SIZE(ids));
> +
> +    for (i = 1; i < hdev->nvqs; i++) {
> +        int w = i % nworkers;
> +        struct vhost_vring_worker vq_worker = { .index = i };
> +
> +        if (w == 0) {
> +            continue;
> +        }
> +
> +        if (i < nworkers) {
> +            struct vhost_worker_state worker = { };
> +
> +            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
> +            if (ret < 0) {
> +                warn_report("vhost-blk: unable to create kernel thread: %s",
> +                            strerror(-ret));
> +                nworkers = i;
> +                continue;
> +            }

[Severity: Low]
When vhost_new_worker() fails for queue i, the code sets nworkers = i and
does continue.  Doesn't that skip the attach step for queue i itself?

Queue i then stays on the default thread, even though workers 1..i-1
already exist and could take it.  Later queues are spread correctly,
because w = i % nworkers uses the reduced nworkers.

The commit message says "if VHOST_NEW_WORKER fails partway, spread the
remaining virtqueues over the threads created so far".  Should the failing
queue also be attached to one of the existing workers, for example by
recomputing w and falling through to the attach step?

> +            ids[w] = worker.worker_id;
> +        }
> +
> +        vq_worker.worker_id = ids[w];
> +        ret = hdev->vhost_ops->vhost_attach_vring_worker(hdev, &vq_worker);
> +        if (ret < 0) {
> +            err = ret;
> +            nfailed++;
> +        }
> +    }

[ ... ]

> @@ -52,15 +99,24 @@ static int vhost_blk_start(VirtIODevice *vdev)
>          return 0;
>      }
>  
> +    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
> +    if (nworkers > 1) {

[Severity: Low]
With -device vhost-blk,num-threads=0, nworkers = MIN(0, nvqs) = 0, so
every "nworkers > 1" branch is skipped and no warning is printed.

The old code did this:

    nworkers = s->conf.num_threads;
    if (nworkers != 1 && ioctl(s->vhostfd, VHOST_SET_NWORKERS, &nworkers)) {
        error_report("vhost-blk: unable to set number of kernel threads");
        return -EINVAL;
    }

So 0 was passed to the kernel, and the device failed to start if the ioctl
rejected it.  I could not check the kernel handler.

The num-threads property is a plain DEFINE_PROP_UINT16 with default 1, and
I found no realize-time check that rejects 0.  Is silently running
num-threads=0 on the single default thread intended?  The commit message
only says that num-threads=1 is unchanged.

> +        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
> +                                                         VHOST_FORK_OWNER_TASK);

[ ... ]

> +    if (nworkers > 1) {
> +        vhost_blk_set_workers(hdev, nworkers);
>      }
>  
>      if (s->conf.serial) {

[Severity: Low]
Can the extra workers created by vhost_blk_set_workers() outlive a failed
start?

Once vhost_blk_set_workers() has run, later failures in vhost_blk_start()
do not issue VHOST_RESET_OWNER or VHOST_FREE_WORKER.  These are the
VHOST_BLK_SET_SERIAL failure, set_guest_notifiers, and the err_set,
err_disable and err_stop paths.

The only place that does the teardown is vhost_blk_reset():

    ret = ioctl(s->vhostfd, VHOST_RESET_OWNER, NULL);

So after a failed start with num-threads > 1 and at least 2 virtqueues, the
owner and the extra worker threads seem to stay alive until a later reset
or until the vhost fd is closed.

The missing owner reset on these error paths was there before this patch.
But this patch adds worker threads to what stays alive on them.  I could not
verify how the vz kernel handles a retried VHOST_SET_OWNER.  Should the
error paths release the workers?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029092301.3286535-1-alexander.ivanov@virtuozzo.com?part=4

Re: [PATCH hci-8.1 v4 4/5] vhost-blk: create kernel threads with the mainstream worker ioctls

[Alexander Ivanov]

>> +static void vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
>> +{
>> +    uint32_t ids[VHOST_BLK_MAX_QUEUES];
>> +    int i, ret, err = 0, nfailed = 0;
>> +
>> +    assert(nworkers <= ARRAY_SIZE(ids));
>> +
>> +    for (i = 1; i < hdev->nvqs; i++) {
>> +        int w = i % nworkers;
>> +        struct vhost_vring_worker vq_worker = { .index = i };
>> +
>> +        if (w == 0) {
>> +            continue;
>> +        }
>> +
>> +        if (i < nworkers) {
>> +            struct vhost_worker_state worker = { };
>> +
>> +            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
>> +            if (ret < 0) {
>> +                warn_report("vhost-blk: unable to create kernel thread: %s",
>> +                            strerror(-ret));
>> +                nworkers = i;
>> +                continue;
>> +            }
> [Severity: Low]
> When vhost_new_worker() fails for queue i, the code sets nworkers = i and
> does continue.  Doesn't that skip the attach step for queue i itself?
>
> Queue i then stays on the default thread, even though workers 1..i-1
> already exist and could take it.  Later queues are spread correctly,
> because w = i % nworkers uses the reduced nworkers.
>
> The commit message says "if VHOST_NEW_WORKER fails partway, spread the
> remaining virtqueues over the threads created so far".  Should the failing
> queue also be attached to one of the existing workers, for example by
> recomputing w and falling through to the attach step?
Queue i lands on thread 0, which is exactly where round-robin over
the reduced nworkers puts it (i % i == 0); attaching it would be a no-op.
>> @@ -52,15 +99,24 @@ static int vhost_blk_start(VirtIODevice *vdev)
>>           return 0;
>>       }
>>   
>> +    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
>> +    if (nworkers > 1) {
> [Severity: Low]
> With -device vhost-blk,num-threads=0, nworkers = MIN(0, nvqs) = 0, so
> every "nworkers > 1" branch is skipped and no warning is printed.
>
> The old code did this:
>
>      nworkers = s->conf.num_threads;
>      if (nworkers != 1 && ioctl(s->vhostfd, VHOST_SET_NWORKERS, &nworkers)) {
>          error_report("vhost-blk: unable to set number of kernel threads");
>          return -EINVAL;
>      }
>
> So 0 was passed to the kernel, and the device failed to start if the ioctl
> rejected it.  I could not check the kernel handler.
>
> The num-threads property is a plain DEFINE_PROP_UINT16 with default 1, and
> I found no realize-time check that rejects 0.  Is silently running
> num-threads=0 on the single default thread intended?  The commit message
> only says that num-threads=1 is unchanged.
This is intended and unchanged: the old VHOST_SET_NWORKERS handler
accepted 0 without error (its worker-creation loop starts at 1, so all
virtqueues stayed on the default thread), and MIN(num_threads, nvqs) with
the nworkers > 1 checks keeps exactly that.
>> +    if (nworkers > 1) {
>> +        vhost_blk_set_workers(hdev, nworkers);
>>       }
>>   
>>       if (s->conf.serial) {
> [Severity: Low]
> Can the extra workers created by vhost_blk_set_workers() outlive a failed
> start?
>
> Once vhost_blk_set_workers() has run, later failures in vhost_blk_start()
> do not issue VHOST_RESET_OWNER or VHOST_FREE_WORKER.  These are the
> VHOST_BLK_SET_SERIAL failure, set_guest_notifiers, and the err_set,
> err_disable and err_stop paths.
>
> The only place that does the teardown is vhost_blk_reset():
>
>      ret = ioctl(s->vhostfd, VHOST_RESET_OWNER, NULL);
>
> So after a failed start with num-threads > 1 and at least 2 virtqueues, the
> owner and the extra worker threads seem to stay alive until a later reset
> or until the vhost fd is closed.
>
> The missing owner reset on these error paths was there before this patch.
> But this patch adds worker threads to what stays alive on them.  I could not
> verify how the vz kernel handles a retried VHOST_SET_OWNER.  Should the
> error paths release the workers?
The extra workers live exactly as long as the owner, which a failed 
start already
leaks, so adding VHOST_RESET_OWNER to the error paths is a separate fix.

Re: [PATCH hci-8.1 v4 4/5] vhost-blk: create kernel threads with the mainstream worker ioctls

[Andrey Drobyshev]

On 10/29/26 11:23 AM, Alexander Ivanov wrote:
> num-threads was passed to our own VHOST_SET_NWORKERS kernel ioctl. The
> kernel drops it with the revert of "drivers/vhost: add ioctl to
> increase the number of workers" in favour of the mainstream worker
> ioctls, which vhost-blk now supports. Create num-threads - 1 workers
> with VHOST_NEW_WORKER and attach them to the virtqueues round-robin
> with VHOST_ATTACH_VRING_WORKER, the layout VHOST_SET_NWORKERS produced.
> 
> Our kernel, unlike mainline, defaults to kthread mode, where
> VHOST_NEW_WORKER fails with -EFAULT rather than -ENOTTY. So switch the
> device to vhost_task mode with VHOST_SET_FORK_FROM_OWNER before
> VHOST_SET_OWNER. With num-threads=1 nothing changes: no worker ioctls,
> kthread mode.
> 
> Released vz10 kernels have VHOST_SET_FORK_FROM_OWNER, but their
> vhost-blk does not handle the worker ioctls and fails VHOST_NEW_WORKER
> with -ENOTTY. Other errors are possible as well, so do not key on the
> errno: if VHOST_SET_FORK_FROM_OWNER fails, keep all virtqueues on the
> default thread; if VHOST_NEW_WORKER fails partway, spread the
> remaining virtqueues over the threads created so far; if attaching a
> virtqueue fails, it stays on the default thread. Either way warn rather
> than fail to start vhost-blk. Unless VHOST_SET_FORK_FROM_OWNER failed,
> the default thread stays a vhost_task, as the mode cannot be changed
> after VHOST_SET_OWNER.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  [...]

LGTM this time.

Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

@Andrey Zhadchenko I'd like your R-b on this one before merging.

Andrey

Re: [PATCH hci-8.1 v4 4/5] vhost-blk: create kernel threads with the mainstream worker ioctls

[Andrey Zhadchenko]

I like VHOST_FORK_OWNER_TASK, but I would rather avoid treating worker 
creation as non-fatal error. This will go on silently and we will never 
know.
Let's just fail when any of the IOCTLs do.

Also let's set this mode for any amount of workers. The difference is 
more or less negligible but we will have uniformity across nworkers and 
better worker isolation.

On 10/29/26 10:23, Alexander Ivanov wrote:
> num-threads was passed to our own VHOST_SET_NWORKERS kernel ioctl. The
> kernel drops it with the revert of "drivers/vhost: add ioctl to
> increase the number of workers" in favour of the mainstream worker
> ioctls, which vhost-blk now supports. Create num-threads - 1 workers
> with VHOST_NEW_WORKER and attach them to the virtqueues round-robin
> with VHOST_ATTACH_VRING_WORKER, the layout VHOST_SET_NWORKERS produced.
> 
> Our kernel, unlike mainline, defaults to kthread mode, where
> VHOST_NEW_WORKER fails with -EFAULT rather than -ENOTTY. So switch the
> device to vhost_task mode with VHOST_SET_FORK_FROM_OWNER before
> VHOST_SET_OWNER. With num-threads=1 nothing changes: no worker ioctls,
> kthread mode.
> 
> Released vz10 kernels have VHOST_SET_FORK_FROM_OWNER, but their
> vhost-blk does not handle the worker ioctls and fails VHOST_NEW_WORKER
> with -ENOTTY. Other errors are possible as well, so do not key on the
> errno: if VHOST_SET_FORK_FROM_OWNER fails, keep all virtqueues on the
> default thread; if VHOST_NEW_WORKER fails partway, spread the
> remaining virtqueues over the threads created so far; if attaching a
> virtqueue fails, it stays on the default thread. Either way warn rather
> than fail to start vhost-blk. Unless VHOST_SET_FORK_FROM_OWNER failed,
> the default thread stays a vhost_task, as the mode cannot be changed
> after VHOST_SET_OWNER.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   hw/block/vhost-blk.c | 64 +++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> index b3eb181ceae..9416c3ac07e 100644
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c
> @@ -33,6 +33,53 @@
>   #include "migration/cpr.h"
>   #include "system/runstate.h"
>   
> +/*
> + * Spread the virtqueues round-robin over nworkers kernel threads. Thread 0 is
> + * the default worker VHOST_SET_OWNER created; vq 0 and every nworkers-th vq
> + * after it stay on it.
> + */
> +static void vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
> +{
> +    uint32_t ids[VHOST_BLK_MAX_QUEUES];
> +    int i, ret, err = 0, nfailed = 0;
> +
> +    assert(nworkers <= ARRAY_SIZE(ids));
> +
> +    for (i = 1; i < hdev->nvqs; i++) {
> +        int w = i % nworkers;
> +        struct vhost_vring_worker vq_worker = { .index = i };
> +
> +        if (w == 0) {
> +            continue;
> +        }
> +
> +        if (i < nworkers) {
> +            struct vhost_worker_state worker = { };
> +
> +            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
> +            if (ret < 0) {
> +                warn_report("vhost-blk: unable to create kernel thread: %s",
> +                            strerror(-ret));
> +                nworkers = i;
> +                continue;
> +            }
> +            ids[w] = worker.worker_id;
> +        }
> +
> +        vq_worker.worker_id = ids[w];
> +        ret = hdev->vhost_ops->vhost_attach_vring_worker(hdev, &vq_worker);
> +        if (ret < 0) {
> +            err = ret;
> +            nfailed++;
> +        }
> +    }
> +
> +    if (nfailed) {
> +        warn_report("vhost-blk: unable to attach kernel threads to %d "
> +                    "queues: %s", nfailed, strerror(-err));
> +    }
> +}
> +
>   static int vhost_blk_start(VirtIODevice *vdev)
>   {
>       VHostBlk *s = VHOST_BLK(vdev);
> @@ -52,15 +99,24 @@ static int vhost_blk_start(VirtIODevice *vdev)
>           return 0;
>       }
>   
> +    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
> +    if (nworkers > 1) {
> +        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
> +                                                         VHOST_FORK_OWNER_TASK);
> +        if (ret < 0) {
> +            warn_report("vhost-blk: unable to use multiple kernel threads: "
> +                        "%s", strerror(-ret));
> +            nworkers = 1;
> +        }
> +    }
> +
>       if (ioctl(s->vhostfd, VHOST_SET_OWNER, NULL)) {
>           error_report("vhost-blk: unable to set owner");
>           return -ENOSYS;
>       }
>   
> -    nworkers = s->conf.num_threads;
> -    if (nworkers != 1 && ioctl(s->vhostfd, VHOST_SET_NWORKERS, &nworkers)) {
> -        error_report("vhost-blk: unable to set number of kernel threads");
> -        return -EINVAL;
> +    if (nworkers > 1) {
> +        vhost_blk_set_workers(hdev, nworkers);
>       }
>   
>       if (s->conf.serial) {

[PATCH hci-8.1 v4 5/5] linux-headers: drop VHOST_SET_NWORKERS

[Alexander Ivanov]

vhost-blk no longer uses it, and the kernel drops the ioctl.

https://virtuozzo.atlassian.net/browse/VSTOR-107537

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
 linux-headers/linux/vhost.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index baad06412d4..b6eb628d973 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -102,15 +102,6 @@
 #define VHOST_GET_VRING_WORKER _IOWR(VHOST_VIRTIO, 0x16,		\
 				     struct vhost_vring_worker)
 
-/* Set number of vhost workers
- * Currently number of vhost workers can only be increased.
- * All workers are freed upon reset.
- * If the value is too big it is silently truncated to the maximum number of
- * supported vhost workers
- * Even if the error is returned it is possible that some workers were created
- */
-#define VHOST_SET_NWORKERS _IOW(VHOST_VIRTIO, 0x1F, int)
-
 /* The following ioctls use eventfd file descriptors to signal and poll
  * for events. */
 
-- 
2.53.0

Re: [PATCH hci-8.1 v4 5/5] linux-headers: drop VHOST_SET_NWORKERS

[no-reply]

> vhost-blk no longer uses it, and the kernel drops the ioctl.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261029092301.3286535-1-alexander.ivanov@virtuozzo.com?part=5