Virtuozzo QEMU development (svt-core@virtuozzo.com)
 help / color / mirror / Atom feed
* [PATCH hci-8.1 v3 0/4] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls
@ 2026-10-28 12:33 Alexander Ivanov
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op Alexander Ivanov
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Alexander Ivanov @ 2026-10-28 12:33 UTC (permalink / raw)
  To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

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 makes
vhost-blk create its num-threads workers with the mainstream ioctls,
keeping the same worker layout, and fall back to a single thread on a
kernel without them. Patch 4 drops VHOST_SET_NWORKERS from
linux-headers.

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 (4):
  vhost-backend: add vhost_set_fork_from_owner op
  vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES
  vhost-blk: create kernel threads with the mainstream worker ioctls
  linux-headers: drop VHOST_SET_NWORKERS

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

-- 
2.53.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op
  2026-10-28 12:33 [PATCH hci-8.1 v3 0/4] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
@ 2026-10-28 12:33 ` Alexander Ivanov
  2026-10-28 12:36   ` no-reply
  2026-10-28 15:30   ` Andrey Drobyshev
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES Alexander Ivanov
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Alexander Ivanov @ 2026-10-28 12:33 UTC (permalink / raw)
  To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

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>
---
 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES
  2026-10-28 12:33 [PATCH hci-8.1 v3 0/4] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op Alexander Ivanov
@ 2026-10-28 12:33 ` Alexander Ivanov
  2026-10-28 12:35   ` no-reply
  2026-10-28 15:31   ` Andrey Drobyshev
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls Alexander Ivanov
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
  3 siblings, 2 replies; 13+ messages in thread
From: Alexander Ivanov @ 2026-10-28 12:33 UTC (permalink / raw)
  To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

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 fails VHOST_SET_VRING_NUM on any vq beyond them,
so such a device realizes fine and then never starts. Refuse it at
realize instead.

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

Signed-off-by: Alexander Ivanov <alexander.ivanov@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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls
  2026-10-28 12:33 [PATCH hci-8.1 v3 0/4] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op Alexander Ivanov
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES Alexander Ivanov
@ 2026-10-28 12:33 ` Alexander Ivanov
  2026-10-28 12:38   ` no-reply
  2026-10-28 15:31   ` Andrey Drobyshev
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
  3 siblings, 2 replies; 13+ messages in thread
From: Alexander Ivanov @ 2026-10-28 12:33 UTC (permalink / raw)
  To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

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. On such a kernel, or one without
VHOST_SET_FORK_FROM_OWNER, warn and run a single thread rather than
fail to start vhost-blk.

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

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

diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
index 8a015d54fa7..cc05eb057df 100644
--- a/hw/block/vhost-blk.c
+++ b/hw/block/vhost-blk.c
@@ -33,9 +33,60 @@
 #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 int vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
+{
+    uint32_t ids[VHOST_BLK_MAX_QUEUES];
+    int i, ret;
+
+    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;
+        }
+
+        /* vq i < nworkers is the first user of thread i: create it here */
+        if (i < nworkers) {
+            struct vhost_worker_state worker = { };
+
+            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
+            if (ret == -ENOTTY) {
+                /* vhost-blk predating worker ioctls, fails on vq 1 only */
+                warn_report_once("vhost-blk: kernel does not support worker "
+                                 "ioctls, using a single kernel thread");
+                return 0;
+            } else if (ret < 0) {
+                error_report("vhost-blk: unable to create kernel thread: %s",
+                             strerror(-ret));
+                return ret;
+            }
+            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) {
+            error_report("vhost-blk: unable to attach kernel thread to "
+                         "queue %d: %s", i, strerror(-ret));
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
 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)));
@@ -51,15 +102,36 @@ static int vhost_blk_start(VirtIODevice *vdev)
         return 0;
     }
 
+    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
+    if (nworkers > 1) {
+        /*
+         * VHOST_NEW_WORKER needs vhost_task mode; our kernel defaults to
+         * kthreads and VHOST_RESET_OWNER brings that default back, so ask
+         * on every start. Must precede VHOST_SET_OWNER.
+         */
+        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
+                                                         VHOST_FORK_OWNER_TASK);
+        if (ret == -ENOTTY) {
+            warn_report_once("vhost-blk: kernel does not support vhost tasks, "
+                             "using a single kernel thread");
+            nworkers = 1;
+        } else if (ret < 0) {
+            error_report("vhost-blk: unable to set vhost task mode: %s",
+                         strerror(-ret));
+            return ret;
+        }
+    }
+
     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) {
+        ret = vhost_blk_set_workers(hdev, nworkers);
+        if (ret < 0) {
+            return ret;
+        }
     }
 
     if (s->conf.serial) {
@@ -70,22 +142,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 +172,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 +185,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");
     }
-- 
2.53.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS
  2026-10-28 12:33 [PATCH hci-8.1 v3 0/4] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
                   ` (2 preceding siblings ...)
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls Alexander Ivanov
@ 2026-10-28 12:33 ` Alexander Ivanov
  2026-10-28 12:36   ` no-reply
  2026-10-28 15:31   ` Andrey Drobyshev
  3 siblings, 2 replies; 13+ messages in thread
From: Alexander Ivanov @ 2026-10-28 12:33 UTC (permalink / raw)
  To: andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

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>
---
 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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES Alexander Ivanov
@ 2026-10-28 12:35   ` no-reply
  2026-10-28 15:31   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: no-reply @ 2026-10-28 12:35 UTC (permalink / raw)
  To: Alexander Ivanov, andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

> 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 fails VHOST_SET_VRING_NUM on any vq beyond them,
> so such a device realizes fine and then never starts. Refuse it at
> realize instead.
> 
> 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/20261028123333.3145676-1-alexander.ivanov@virtuozzo.com?part=2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op Alexander Ivanov
@ 2026-10-28 12:36   ` no-reply
  2026-10-28 15:30   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: no-reply @ 2026-10-28 12:36 UTC (permalink / raw)
  To: andrey.zhadchenko, Alexander Ivanov; +Cc: andrey.drobyshev, svt-core

> 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/20261028123333.3145676-1-alexander.ivanov@virtuozzo.com?part=1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
@ 2026-10-28 12:36   ` no-reply
  2026-10-28 15:31   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: no-reply @ 2026-10-28 12:36 UTC (permalink / raw)
  To: andrey.zhadchenko, Alexander Ivanov; +Cc: svt-core, andrey.drobyshev

> 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/20261028123333.3145676-1-alexander.ivanov@virtuozzo.com?part=4


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls Alexander Ivanov
@ 2026-10-28 12:38   ` no-reply
  2026-10-28 15:31   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: no-reply @ 2026-10-28 12:38 UTC (permalink / raw)
  To: Alexander Ivanov, andrey.zhadchenko; +Cc: andrey.drobyshev, svt-core

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] vhost-blk: single-thread fallback only handles -ENOTTY from unknown vhost ioctls
- [Low] vhost-blk: ENOTTY worker fallback leaves device in vhost_task mode
- [Low] vhost-blk: num-threads=0 silently accepted as a single kernel thread
--

commit bce7b559a829ea8e18f3184e167eeedc9027e77f
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 creates num-threads - 1 workers and attaches
them to the virtqueues round-robin. It also switches the device to
vhost_task mode with VHOST_SET_FORK_FROM_OWNER before VHOST_SET_OWNER. If
the kernel lacks these ioctls, it warns and runs a single thread.

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 int vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
> +{

[ ... ]

> +            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
> +            if (ret == -ENOTTY) {
> +                /* vhost-blk predating worker ioctls, fails on vq 1 only */
> +                warn_report_once("vhost-blk: kernel does not support worker "
> +                                 "ioctls, using a single kernel thread");
> +                return 0;
> +            } else if (ret < 0) {
> +                error_report("vhost-blk: unable to create kernel thread: %s",
> +                             strerror(-ret));
> +                return ret;
> +            }

[Severity: Medium]
Is -ENOTTY the only errno an older kernel can return here? The fallback to
a single thread is taken only for an exact -ENOTTY from
vhost_blk_set_workers()->vhost_new_worker(). Any other errno goes through
error_report() and fails vhost_blk_start().

The same applies in vhost_blk_start() to the result of
vhost_set_fork_from_owner():

    ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
                                                     VHOST_FORK_OWNER_TASK);
    if (ret == -ENOTTY) {
        ...
        nworkers = 1;
    } else if (ret < 0) {
        error_report("vhost-blk: unable to set vhost task mode: %s", ...);
        return ret;

Both go through vhost_kernel_call(), which returns the raw -errno from
ioctl().

The comment in vhost_kernel_set_features() says that for unknown vhost
ioctls the kernel interprets the argument as a virtio queue id and most
likely errors out validating that id, instead of reporting an unknown
operation.

If VHOST_SET_FORK_FROM_OWNER falls through to the vring ioctl handler on a
kernel without it, could the result be -ENOBUFS instead of -ENOTTY? In that
case the commit message promise to "warn and run a single thread rather than
fail to start vhost-blk" would not hold when num-threads > 1.

The commit message also notes that VHOST_NEW_WORKER fails with -EFAULT in
kthread mode, so the errno is not uniform across kernel configurations.

I couldn't check the kernel side, so this is based on the in-tree comment.

> +    }
> +
> +    return 0;
> +}

[ ... ]

> +    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
> +    if (nworkers > 1) {

[Severity: Low]
If num-threads is 0, nworkers becomes 0 here, so both nworkers > 1 blocks
are skipped. Doesn't vhost_blk_start() then silently run a single kthread
with no diagnostic?

The old code passed num_threads to VHOST_SET_NWORKERS whenever it was not 1,
and failed with -EINVAL if that ioctl failed. vhost_blk_device_realize()
validates num_queues but not num_threads, and the num-threads property is a
DEFINE_PROP_UINT16 with a default of 1 and no range check.

Should num-threads=0 be rejected at realize time, or is the silent fallback
intended? The commit message doesn't mention this change in behaviour.

> +        /*
> +         * VHOST_NEW_WORKER needs vhost_task mode; our kernel defaults to
> +         * kthreads and VHOST_RESET_OWNER brings that default back, so ask
> +         * on every start. Must precede VHOST_SET_OWNER.
> +         */
> +        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
> +                                                         VHOST_FORK_OWNER_TASK);

[Severity: Low]
On a released vz10 kernel that has VHOST_SET_FORK_FROM_OWNER but whose
vhost-blk lacks the worker ioctls, this call succeeds and switches the
device to vhost_task mode. Later, vhost_blk_set_workers() gets -ENOTTY from
vhost_new_worker() on vq 1, warns, and returns 0.

Does anything restore VHOST_FORK_OWNER_KTHREAD on that path? The fallback
then runs one thread in vhost_task mode, while num-threads=1 runs one thread
in kthread mode.

The commit message says "With num-threads=1 nothing changes: no worker
ioctls, kthread mode", but the fallback case isn't described as differing
from it. Is that difference intended?

I couldn't confirm from this tree whether task mode matters for vhost-blk on
those kernels.

> +        if (ret == -ENOTTY) {
> +            warn_report_once("vhost-blk: kernel does not support vhost tasks, "
> +                             "using a single kernel thread");
> +            nworkers = 1;
> +        } else if (ret < 0) {

[ ... ]

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op Alexander Ivanov
  2026-10-28 12:36   ` no-reply
@ 2026-10-28 15:30   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: Andrey Drobyshev @ 2026-10-28 15:30 UTC (permalink / raw)
  To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core

On 10/28/26 2:33 PM, Alexander Ivanov wrote:
> 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>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES Alexander Ivanov
  2026-10-28 12:35   ` no-reply
@ 2026-10-28 15:31   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: Andrey Drobyshev @ 2026-10-28 15:31 UTC (permalink / raw)
  To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core

On 10/28/26 2:33 PM, Alexander Ivanov wrote:
> 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 fails VHOST_SET_VRING_NUM on any vq beyond them,
> so such a device realizes fine and then never starts.

This claim isn't quite accurate:

> -device vhost-blk-pci,devpath=/dev/loop0,num-queues=33
>   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

So realize doesn't even work.  Just adjust the commit message.

> Refuse it at realize instead.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
>

Let's also add the 'Fixes: 0ff3cb5ff28f ("...")' tag here.

> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  [...]
With that:

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
  2026-10-28 12:36   ` no-reply
@ 2026-10-28 15:31   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: Andrey Drobyshev @ 2026-10-28 15:31 UTC (permalink / raw)
  To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core

On 10/28/26 2:33 PM, Alexander Ivanov wrote:
> 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>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls
  2026-10-28 12:33 ` [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls Alexander Ivanov
  2026-10-28 12:38   ` no-reply
@ 2026-10-28 15:31   ` Andrey Drobyshev
  1 sibling, 0 replies; 13+ messages in thread
From: Andrey Drobyshev @ 2026-10-28 15:31 UTC (permalink / raw)
  To: Alexander Ivanov, andrey.zhadchenko; +Cc: svt-core

On 10/28/26 2:33 PM, 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. On such a kernel, or one without
> VHOST_SET_FORK_FROM_OWNER, warn and run a single thread rather than
> fail to start vhost-blk.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-107537
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  hw/block/vhost-blk.c | 100 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 86 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> index 8a015d54fa7..cc05eb057df 100644
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c
> @@ -33,9 +33,60 @@
>  #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 int vhost_blk_set_workers(struct vhost_dev *hdev, int nworkers)
> +{
> +    uint32_t ids[VHOST_BLK_MAX_QUEUES];
> +    int i, ret;
> +
> +    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;
> +        }
> +
> +        /* vq i < nworkers is the first user of thread i: create it here */
> +        if (i < nworkers) {
> +            struct vhost_worker_state worker = { };
> +
> +            ret = hdev->vhost_ops->vhost_new_worker(hdev, &worker);
> +            if (ret == -ENOTTY) {
> +                /* vhost-blk predating worker ioctls, fails on vq 1 only */
> +                warn_report_once("vhost-blk: kernel does not support worker "
> +                                 "ioctls, using a single kernel thread");
> +                return 0;

AFAIU Sashiko is right about failure not always being indicated by
-ENOTTY.  We might get EPERM, ENOBUFS, EINVAL, EFAULT.

Maybe let's just stop keying on exact error:

>       ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev, VHOST_FORK_OWNER_TASK);
>       if (ret < 0) {
>           warn_report_once("vhost-blk: unable to use multiple kernel threads: %s",
>                            strerror(-ret));
>           nworkers = 1;
>       }

And the same for vhost_new_worker() inside the loop.

@Andrey Zhadchenko can you confirm that's the right error-handling
approach here?

> +            } else if (ret < 0) {
> +                error_report("vhost-blk: unable to create kernel thread: %s",
> +                             strerror(-ret));
> +                return ret;
> +            }
> +            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) {
> +            error_report("vhost-blk: unable to attach kernel thread to "
> +                         "queue %d: %s", i, strerror(-ret));
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  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)));
> @@ -51,15 +102,36 @@ static int vhost_blk_start(VirtIODevice *vdev)
>          return 0;
>      }
>  
> +    nworkers = MIN(s->conf.num_threads, hdev->nvqs);
> +    if (nworkers > 1) {
> +        /*
> +         * VHOST_NEW_WORKER needs vhost_task mode; our kernel defaults to
> +         * kthreads and VHOST_RESET_OWNER brings that default back, so ask
> +         * on every start. Must precede VHOST_SET_OWNER.
> +         */
> +        ret = hdev->vhost_ops->vhost_set_fork_from_owner(hdev,
> +                                                         VHOST_FORK_OWNER_TASK);
> +        if (ret == -ENOTTY) {
> +            warn_report_once("vhost-blk: kernel does not support vhost tasks, "
> +                             "using a single kernel thread");
> +            nworkers = 1;
> +        } else if (ret < 0) {
> +            error_report("vhost-blk: unable to set vhost task mode: %s",
> +                         strerror(-ret));
> +            return ret;
> +        }
> +    }
> +
>      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) {
> +        ret = vhost_blk_set_workers(hdev, nworkers);
> +        if (ret < 0) {
> +            return ret;
> +        }
>      }
>  
>      if (s->conf.serial) {
> @@ -70,22 +142,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);

Nit: these s->dev -> hdev replacements generally don't belong here as
they aren't part of the changing logic.  Split it into another patch
(maybe covering stop()/reset() as well?) and add 'Fixes: ...' tag, so I
can squash it during the next rebase keeping only the meaningful history.

Andrey

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-10-28 15:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 12:33 [PATCH hci-8.1 v3 0/4] vhost-blk: switch from VHOST_SET_NWORKERS to the mainstream worker ioctls Alexander Ivanov
2026-10-28 12:33 ` [PATCH hci-8.1 v3 1/4] vhost-backend: add vhost_set_fork_from_owner op Alexander Ivanov
2026-10-28 12:36   ` no-reply
2026-10-28 15:30   ` Andrey Drobyshev
2026-10-28 12:33 ` [PATCH hci-8.1 v3 2/4] vhost-blk: reject num-queues above VHOST_BLK_MAX_QUEUES Alexander Ivanov
2026-10-28 12:35   ` no-reply
2026-10-28 15:31   ` Andrey Drobyshev
2026-10-28 12:33 ` [PATCH hci-8.1 v3 3/4] vhost-blk: create kernel threads with the mainstream worker ioctls Alexander Ivanov
2026-10-28 12:38   ` no-reply
2026-10-28 15:31   ` Andrey Drobyshev
2026-10-28 12:33 ` [PATCH hci-8.1 v3 4/4] linux-headers: drop VHOST_SET_NWORKERS Alexander Ivanov
2026-10-28 12:36   ` no-reply
2026-10-28 15:31   ` Andrey Drobyshev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox