* [PATCH] vhost-user-blk: add seg-max-adjust flag
@ 2026-04-01 23:05 Sergei Heifetz
2026-04-15 15:24 ` Raphael Norwitz
0 siblings, 1 reply; 5+ messages in thread
From: Sergei Heifetz @ 2026-04-01 23:05 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefano Garzarella, Raphael Norwitz, Sergei Heifetz
The virtio specification is not completely clear about seg_max and its
relationship with queue_size. Some drivers (for example, the modern
Linux kernel driver) rely on seg_max to set the maximum number of
segments used in a request. If seg_max is set larger than queue_size,
such a driver might overwhelm a virtqueue by trying to send more
segments than it can handle. As a result, it either hangs or faults.
One might argue that it is the vhost-user server's responsibility to set
a valid seg_max value. However, due to the issue described in the
previous paragraph, this value should generally depend on queue_size.
That's why it may be necessary to control it on QEMU's side.
This patch adds the seg-max-adjust flag. A flag with the same name
already exists for virtio-blk (and it exists to solve the same problem,
except that here we have a vhost-user server to consult).
If seg-max-adjust is set, the final seg_max is the minimum of the value
provided by the vhost-user server and (queue_size - 2). It is not
enabled by default.
Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
---
hw/block/vhost-user-blk.c | 11 +++++++++++
include/hw/virtio/vhost-user-blk.h | 1 +
2 files changed, 12 insertions(+)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index c151e836770..23f910d9fe3 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
/* Our num_queues overrides the device backend */
virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
+ if (s->seg_max_adjust) {
+ uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
+ virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
+ }
+
memcpy(config, &s->blkcfg, vdev->config_len);
}
@@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
error_setg(errp, "queue size must be non-zero");
return;
}
+ if (s->queue_size < 4 && s->seg_max_adjust) {
+ error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
+ return;
+ }
if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
error_setg(errp, "queue size must not exceed %d",
VIRTQUEUE_MAX_SIZE);
@@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
+ DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
+ false),
DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
VIRTIO_BLK_F_CONFIG_WCE, true),
DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index 1e41a2bcdf6..dee848cfd81 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -34,6 +34,7 @@ struct VHostUserBlk {
struct virtio_blk_config blkcfg;
uint16_t num_queues;
uint32_t queue_size;
+ bool seg_max_adjust;
struct vhost_dev dev;
struct vhost_inflight *inflight;
VhostUserState vhost_user;
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
2026-04-01 23:05 [PATCH] vhost-user-blk: add seg-max-adjust flag Sergei Heifetz
@ 2026-04-15 15:24 ` Raphael Norwitz
2026-06-15 17:19 ` Vladimir Sementsov-Ogievskiy
2026-07-04 9:17 ` Vladimir Sementsov-Ogievskiy
0 siblings, 2 replies; 5+ messages in thread
From: Raphael Norwitz @ 2026-04-15 15:24 UTC (permalink / raw)
To: Sergei Heifetz, qemu-devel
Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefano Garzarella
Looks in keeping with how virtio-blk does things with seg_max_adjust, so
I’m happy in principle.
Just a nit and a question.
On 4/2/26 1:05 AM, Sergei Heifetz wrote:
> [You don't often get email from heifetz@yandex-team.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> The virtio specification is not completely clear about seg_max and its
> relationship with queue_size. Some drivers (for example, the modern
> Linux kernel driver) rely on seg_max to set the maximum number of
> segments used in a request. If seg_max is set larger than queue_size,
> such a driver might overwhelm a virtqueue by trying to send more
> segments than it can handle. As a result, it either hangs or faults.
>
> One might argue that it is the vhost-user server's responsibility to set
> a valid seg_max value. However, due to the issue described in the
> previous paragraph, this value should generally depend on queue_size.
> That's why it may be necessary to control it on QEMU's side.
>
> This patch adds the seg-max-adjust flag. A flag with the same name
> already exists for virtio-blk (and it exists to solve the same problem,
> except that here we have a vhost-user server to consult).
>
> If seg-max-adjust is set, the final seg_max is the minimum of the value
> provided by the vhost-user server and (queue_size - 2). It is not
> enabled by default.
>
> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
> ---
> hw/block/vhost-user-blk.c | 11 +++++++++++
> include/hw/virtio/vhost-user-blk.h | 1 +
> 2 files changed, 12 insertions(+)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index c151e836770..23f910d9fe3 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> /* Our num_queues overrides the device backend */
> virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
>
> + if (s->seg_max_adjust) {
NIT: declaration at the top of the function
> + uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
> + virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
> + }
> +
> memcpy(config, &s->blkcfg, vdev->config_len);
> }
>
> @@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> error_setg(errp, "queue size must be non-zero");
> return;
> }
Can you explain why queue size must be greater than 4 if seg_max_adjust
is set? Sorry if I'm missing something obvious.
> + if (s->queue_size < 4 && s->seg_max_adjust) {
> + error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
> + return;
> + }
> if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
> error_setg(errp, "queue size must not exceed %d",
> VIRTQUEUE_MAX_SIZE);
> @@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
> DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
> VHOST_USER_BLK_AUTO_NUM_QUEUES),
> DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
> + DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
> + false),
> DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
> VIRTIO_BLK_F_CONFIG_WCE, true),
> DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
> index 1e41a2bcdf6..dee848cfd81 100644
> --- a/include/hw/virtio/vhost-user-blk.h
> +++ b/include/hw/virtio/vhost-user-blk.h
> @@ -34,6 +34,7 @@ struct VHostUserBlk {
> struct virtio_blk_config blkcfg;
> uint16_t num_queues;
> uint32_t queue_size;
> + bool seg_max_adjust;
> struct vhost_dev dev;
> struct vhost_inflight *inflight;
> VhostUserState vhost_user;
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
2026-04-15 15:24 ` Raphael Norwitz
@ 2026-06-15 17:19 ` Vladimir Sementsov-Ogievskiy
2026-07-04 9:17 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 5+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-06-15 17:19 UTC (permalink / raw)
To: Raphael Norwitz, qemu-devel
Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefano Garzarella, Daniil Tatianin
15.04.26 18:24, Raphael Norwitz пишет:
> Looks in keeping with how virtio-blk does things with seg_max_adjust, so I’m happy in principle.
>
> Just a nit and a question.
>
> On 4/2/26 1:05 AM, Sergei Heifetz wrote:
>> [You don't often get email from heifetz@yandex-team.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> The virtio specification is not completely clear about seg_max and its
>> relationship with queue_size. Some drivers (for example, the modern
>> Linux kernel driver) rely on seg_max to set the maximum number of
>> segments used in a request. If seg_max is set larger than queue_size,
>> such a driver might overwhelm a virtqueue by trying to send more
>> segments than it can handle. As a result, it either hangs or faults.
>>
>> One might argue that it is the vhost-user server's responsibility to set
>> a valid seg_max value. However, due to the issue described in the
>> previous paragraph, this value should generally depend on queue_size.
>> That's why it may be necessary to control it on QEMU's side.
>>
>> This patch adds the seg-max-adjust flag. A flag with the same name
>> already exists for virtio-blk (and it exists to solve the same problem,
>> except that here we have a vhost-user server to consult).
>>
>> If seg-max-adjust is set, the final seg_max is the minimum of the value
>> provided by the vhost-user server and (queue_size - 2). It is not
>> enabled by default.
>>
>> Signed-off-by: Sergei Heifetz <heifetz@yandex-team.com>
>> ---
>> hw/block/vhost-user-blk.c | 11 +++++++++++
>> include/hw/virtio/vhost-user-blk.h | 1 +
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>> index c151e836770..23f910d9fe3 100644
>> --- a/hw/block/vhost-user-blk.c
>> +++ b/hw/block/vhost-user-blk.c
>> @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>> /* Our num_queues overrides the device backend */
>> virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
>>
>> + if (s->seg_max_adjust) {
>
> NIT: declaration at the top of the function
>
>> + uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
>> + virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
>> + }
>> +
>> memcpy(config, &s->blkcfg, vdev->config_len);
>> }
>>
>> @@ -474,6 +479,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>> error_setg(errp, "queue size must be non-zero");
>> return;
>> }
>
> Can you explain why queue size must be greater than 4 if seg_max_adjust is set? Sorry if I'm missing something obvious.
Because we want seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2),
and queue size must be power of two. So, for seg_max be > 0, we need queue_size at least 4.
>
>> + if (s->queue_size < 4 && s->seg_max_adjust) {
>> + error_setg(errp, "queue size must be >= 4 when seg-max-adjust is set");
>> + return;
>> + }
>> if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
>> error_setg(errp, "queue size must not exceed %d",
>> VIRTQUEUE_MAX_SIZE);
>> @@ -608,6 +617,8 @@ static const Property vhost_user_blk_properties[] = {
>> DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
>> VHOST_USER_BLK_AUTO_NUM_QUEUES),
>> DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
>> + DEFINE_PROP_BOOL("seg-max-adjust", VHostUserBlk, seg_max_adjust,
>> + false),
>> DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
>> VIRTIO_BLK_F_CONFIG_WCE, true),
>> DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
>> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
>> index 1e41a2bcdf6..dee848cfd81 100644
>> --- a/include/hw/virtio/vhost-user-blk.h
>> +++ b/include/hw/virtio/vhost-user-blk.h
>> @@ -34,6 +34,7 @@ struct VHostUserBlk {
>> struct virtio_blk_config blkcfg;
>> uint16_t num_queues;
>> uint32_t queue_size;
>> + bool seg_max_adjust;
>> struct vhost_dev dev;
>> struct vhost_inflight *inflight;
>> VhostUserState vhost_user;
>> --
>> 2.53.0
>>
>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
2026-04-15 15:24 ` Raphael Norwitz
2026-06-15 17:19 ` Vladimir Sementsov-Ogievskiy
@ 2026-07-04 9:17 ` Vladimir Sementsov-Ogievskiy
2026-07-04 9:57 ` Michael S. Tsirkin
1 sibling, 1 reply; 5+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2026-07-04 9:17 UTC (permalink / raw)
To: Raphael Norwitz, Sergei Heifetz, qemu-devel
Cc: qemu-block, Michael S. Tsirkin, Kevin Wolf, Hanna Reitz,
Stefano Garzarella
On 15.04.26 18:24, Raphael Norwitz wrote:
>> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
>> index c151e836770..23f910d9fe3 100644
>> --- a/hw/block/vhost-user-blk.c
>> +++ b/hw/block/vhost-user-blk.c
>> @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>> /* Our num_queues overrides the device backend */
>> virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
>>
>> + if (s->seg_max_adjust) {
>
> NIT: declaration at the top of the function
Hmm, no, accordingly to docs/dev/style.rst, "at the beginning of blocks",
so, it's OK to declare it at top of if-block, it's often used in the code,
look at output of `git grep -B 3 '^ int '` for example.
>
>> + uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
>> + virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
>> + }
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] vhost-user-blk: add seg-max-adjust flag
2026-07-04 9:17 ` Vladimir Sementsov-Ogievskiy
@ 2026-07-04 9:57 ` Michael S. Tsirkin
0 siblings, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2026-07-04 9:57 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: Raphael Norwitz, Sergei Heifetz, qemu-devel, qemu-block,
Kevin Wolf, Hanna Reitz, Stefano Garzarella
On Sat, Jul 04, 2026 at 12:17:40PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 15.04.26 18:24, Raphael Norwitz wrote:
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index c151e836770..23f910d9fe3 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -65,6 +65,11 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
> > > /* Our num_queues overrides the device backend */
> > > virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
> > >
> > > + if (s->seg_max_adjust) {
> >
> > NIT: declaration at the top of the function
>
> Hmm, no, accordingly to docs/dev/style.rst, "at the beginning of blocks",
> so, it's OK to declare it at top of if-block, it's often used in the code,
> look at output of `git grep -B 3 '^ int '` for example.
indeed.
> >
> > > + uint32_t seg_max = MIN(s->blkcfg.seg_max, s->queue_size - 2);
if you respon pls add an empty line here.
> > > + virtio_stl_p(vdev, &s->blkcfg.seg_max, seg_max);
> > > + }
>
>
> --
> Best regards,
> Vladimir
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-04 9:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-01 23:05 [PATCH] vhost-user-blk: add seg-max-adjust flag Sergei Heifetz
2026-04-15 15:24 ` Raphael Norwitz
2026-06-15 17:19 ` Vladimir Sementsov-Ogievskiy
2026-07-04 9:17 ` Vladimir Sementsov-Ogievskiy
2026-07-04 9:57 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.